-
Notifications
You must be signed in to change notification settings - Fork 31
Allow disabling local password through broker.conf #1234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -24,6 +24,18 @@ client_id = <CLIENT_ID> | |||||
| ## if the identity provider is unreachable (e.g. due to network issues). | ||||||
| #force_provider_authentication = false | ||||||
|
|
||||||
| ## Disable local password authentication, requiring users to always perform | ||||||
| ## device authentication with the identity provider. | ||||||
| ## | ||||||
| ## When enabled: | ||||||
| ## - Users will not be able to create or use local passwords | ||||||
| ## - Device authentication will be required for every login | ||||||
| ## - Local password authentication mode will not be offered | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to self: We should probably point users to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, Maybe we can add an explanation mentioning about when |
||||||
| ## | ||||||
| ## Important: Enabling this option prevents offline login entirely. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| ## Users must have network connectivity to authenticate. | ||||||
| #disable_local_password = false | ||||||
|
|
||||||
| [users] | ||||||
| ## The directory where the home directories of new users are created. | ||||||
| ## Existing users will keep their current home directory. | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -305,6 +305,11 @@ func (b *Broker) availableAuthModes(session session) (availableModes []string, e | |||||
| func (b *Broker) authModeIsAvailable(session session, authMode string) bool { | ||||||
| switch authMode { | ||||||
| case authmodes.Password: | ||||||
| if b.cfg.disableLocalPassword { | ||||||
| log.Debugf(context.Background(), "Local password authentication is disabled") | ||||||
| return false | ||||||
| } | ||||||
|
|
||||||
| if !tokenExists(session) { | ||||||
| log.Debugf(context.Background(), "Token does not exist for user %q, so local password authentication is not available", session.username) | ||||||
| return false | ||||||
|
|
@@ -716,9 +721,14 @@ func (b *Broker) deviceAuth(ctx context.Context, session *session) (string, isAu | |||||
| // Store the auth info in the session so that we can use it when handling the | ||||||
| // next IsAuthenticated call for the new password mode. | ||||||
| session.authInfo = authInfo | ||||||
| session.nextAuthModes = []string{authmodes.NewPassword} | ||||||
|
|
||||||
| return AuthNext, nil | ||||||
| // Only require password creation if local password authentication is not disabled | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| if !b.cfg.disableLocalPassword { | ||||||
| session.nextAuthModes = []string{authmodes.NewPassword} | ||||||
| return AuthNext, nil | ||||||
| } | ||||||
|
Comment on lines
+726
to
+729
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another consequence of not creating a local password is that the keyring can't be unlocked. #1190 also won't fix it in that case.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay. We can discuss about this. I will post here if I get an idea about this.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made a proposal here: #1190 (comment)
Comment on lines
+726
to
+729
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to self: Manually test:
Ideally we should add e2e tests for those. |
||||||
|
|
||||||
| return b.finishAuth(session, authInfo) | ||||||
| } | ||||||
|
|
||||||
| func (b *Broker) passwordAuth(ctx context.Context, session *session, secret string) (string, isAuthenticatedDataResponse) { | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Definitely a token |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| access: granted | ||
| data: '{"userinfo":{"name":"[email protected]","uuid":"test-user-id","dir":"/home/[email protected]","shell":"/usr/bin/bash","gecos":"[email protected]","groups":[{"name":"remote-test-group","ugid":"12345"},{"name":"local-test-group","ugid":""}]}}' | ||
| err: <nil> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Definitely a token |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| access: granted | ||
| data: '{"userinfo":{"name":"[email protected]","uuid":"test-user-id","dir":"/home/[email protected]","shell":"/usr/bin/bash","gecos":"[email protected]","groups":[{"name":"remote-test-group","ugid":"12345"},{"name":"local-test-group","ugid":""}]}}' | ||
| err: <nil> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Definitely a hashed password |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Definitely a token |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| access: granted | ||
| data: '{"userinfo":{"name":"[email protected]","uuid":"test-user-id","dir":"/home/[email protected]","shell":"/usr/bin/bash","gecos":"[email protected]","groups":[{"name":"remote-test-group","ugid":"12345"},{"name":"local-test-group","ugid":""}]}}' | ||
| err: <nil> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Definitely a hashed password |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Definitely a token |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| access: next | ||
| data: '{}' | ||
| err: <nil> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.