Add ability to disable local auth. #2575
Conversation
russjones
force-pushed
the
rjones/disable-local-auth
branch
2 times, most recently
from
3edd64d to
add4140
Compare
|
I've migrated this model to protobuf in my branch, let's hold on on this change before I merge to avoid changing the model twice. |
russjones
force-pushed
the
rjones/disable-local-auth
branch
2 times, most recently
from
5aab69a to
dea9522
Compare
| if err != nil { | ||
| return "", trace.Wrap(err) | ||
| } | ||
| if clusterConfig.GetLocalAuth() == false { |
There was a problem hiding this comment.
I think this condition can be omitted here, the logic of creating a signup token does not need to be aware of this detail.
| @@ -263,6 +279,14 @@ func (s *AuthServer) CreateUserWithOTP(token string, password string, otpToken s | |||
|
|
|||
| // CreateUserWithoutOTP creates an account with the provided password and deletes the token afterwards. | |||
| func (s *AuthServer) CreateUserWithoutOTP(token string, password string) (services.WebSession, error) { | |||
| clusterConfig, err := s.GetClusterConfig() | |||
There was a problem hiding this comment.
From the first glance it looks like it should be handled using AuthPreferences rather than a stand alone configuration property.
There was a problem hiding this comment.
I think we are moving auth preferences in the cluster config in the longer term, so kind of makes sense
| @@ -291,6 +315,14 @@ func (s *AuthServer) CreateUserWithoutOTP(token string, password string) (servic | |||
| } | |||
|
|
|||
| func (s *AuthServer) CreateUserWithU2FToken(token string, password string, response u2f.RegisterResponse) (services.WebSession, error) { | |||
There was a problem hiding this comment.
Instead of checking for this condition in all these methods:
CreateSignupToken
CreateUserWithU2FToken
CreateUserWithOTP
CreateUserWithoutOTP
We can do it in 1 method, say CreateUser.
This is how we do it in gravity https://github.com/gravitational/gravity/blob/master/lib/users/usersservice/usersservice.go#L1340
where all requests for creating a new local user go through 1 method which has all the checks.
There was a problem hiding this comment.
I kind of agree that having one place is nice, not sure if worth refactoring it all right now makes sense
lib/auth/new_web_user.go
Outdated
| return nil, trace.Wrap(err) | ||
| } | ||
| if clusterConfig.GetLocalAuth() == false { | ||
| return nil, trace.AccessDenied("local auth disabled") |
lib/auth/new_web_user.go
Outdated
| return "", trace.Wrap(err) | ||
| } | ||
| if clusterConfig.GetLocalAuth() == false { | ||
| return "", trace.AccessDenied("local auth disabled") |
| @@ -263,6 +279,14 @@ func (s *AuthServer) CreateUserWithOTP(token string, password string, otpToken s | |||
|
|
|||
| // CreateUserWithoutOTP creates an account with the provided password and deletes the token afterwards. | |||
| func (s *AuthServer) CreateUserWithoutOTP(token string, password string) (services.WebSession, error) { | |||
| clusterConfig, err := s.GetClusterConfig() | |||
There was a problem hiding this comment.
I think we are moving auth preferences in the cluster config in the longer term, so kind of makes sense
| @@ -291,6 +315,14 @@ func (s *AuthServer) CreateUserWithoutOTP(token string, password string) (servic | |||
| } | |||
|
|
|||
| func (s *AuthServer) CreateUserWithU2FToken(token string, password string, response u2f.RegisterResponse) (services.WebSession, error) { | |||
There was a problem hiding this comment.
I kind of agree that having one place is nice, not sure if worth refactoring it all right now makes sense
Added "local_auth" to file configuration and "LocalAuth" to services.ClusterConfig to control cluster-wide local authentication. Check local auth settings when generating signup tokens, creating local users, and login.
russjones
force-pushed
the
rjones/disable-local-auth
branch
from
dea9522 to
5bd8e21
Compare
Description
Added "local_auth" to file configuration and "LocalAuth" to
services.ClusterConfigto control cluster-wide local authentication. Check local auth settings when generating signup tokens, creating local users, and login.Related Issues
Fixes https://github.com/gravitational/teleport.e/issues/105