diff --git a/app/boot_levels.go b/app/boot_levels.go index 02113d94b..56bad08c9 100644 --- a/app/boot_levels.go +++ b/app/boot_levels.go @@ -521,6 +521,7 @@ func updateAuthSettings(svc authServicer, current *types.AppSettings) { SignupEnabled: current.Auth.Internal.Signup.Enabled, EmailConfirmationRequired: current.Auth.Internal.Signup.EmailConfirmationRequired, PasswordResetEnabled: current.Auth.Internal.PasswordReset.Enabled, + SplitCredentialsCheck: current.Auth.Internal.SplitCredentialsCheck, ExternalEnabled: current.Auth.External.Enabled, MultiFactor: authSettings.MultiFactor{ TOTP: authSettings.TOTP{ diff --git a/auth/assets/templates/login.html.tpl b/auth/assets/templates/login.html.tpl index f8498ebd2..f8b7f5803 100644 --- a/auth/assets/templates/login.html.tpl +++ b/auth/assets/templates/login.html.tpl @@ -27,6 +27,7 @@ autocomplete="username" aria-label="Email"> + {{ if not .form.splitCredentialsCheck }}
+ {{ else }} +
+
+ +
+
+ {{ end }}
{{ if .settings.PasswordResetEnabled }} diff --git a/auth/assets/templates/scenarios.yaml b/auth/assets/templates/scenarios.yaml index b814bd75b..e682b81a9 100644 --- a/auth/assets/templates/scenarios.yaml +++ b/auth/assets/templates/scenarios.yaml @@ -39,6 +39,11 @@ login: SignupEnabled: false PasswordResetEnabled: false + With Split credentials check / username: + form: + splitCredentialsCheck: true + settings: + LocalEnabled: true With errors after submit: settings: diff --git a/auth/auth.go b/auth/auth.go index 590e48797..412790fbf 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -301,6 +301,10 @@ func (svc *service) UpdateSettings(s *settings.Settings) { svc.log.Debug("setting changed", zap.Bool("passwordResetEnabled", s.PasswordResetEnabled)) } + if svc.settings.SplitCredentialsCheck != s.SplitCredentialsCheck { + svc.log.Debug("setting changed", zap.Bool("splitCredentialsCheck", s.SplitCredentialsCheck)) + } + if svc.settings.ExternalEnabled != s.ExternalEnabled { svc.log.Debug("setting changed", zap.Bool("externalEnabled", s.ExternalEnabled)) } diff --git a/auth/handlers/handle_login.go b/auth/handlers/handle_login.go index 423a48835..7b9b9016c 100644 --- a/auth/handlers/handle_login.go +++ b/auth/handlers/handle_login.go @@ -1,6 +1,8 @@ package handlers import ( + "fmt" + "github.com/cortezaproject/corteza-server/auth/request" "github.com/cortezaproject/corteza-server/pkg/auth" "github.com/cortezaproject/corteza-server/pkg/errors" @@ -17,26 +19,66 @@ type ( func (h *AuthHandlers) loginForm(req *request.AuthReq) error { req.Template = TmplLogin - req.Data["form"] = req.PopKV() + kv := req.PopKV() + + if kv == nil && h.Settings.SplitCredentialsCheck { + // Force login form to show only email input + // + // KV is nil (means that this is first load of the form) + // and credentials check is split into two parts (email first then credentials) + kv = map[string]string{ + "splitCredentialsCheck": "split", + } + } + + req.Data["form"] = kv return nil } func (h *AuthHandlers) loginProc(req *request.AuthReq) (err error) { + // In most cases, we want to redirect back to login req.RedirectTo = GetLinks().Login req.SetKV(nil) var ( - user *types.User - email = req.Request.PostFormValue("email") + user *types.User + email = req.Request.PostFormValue("email") + password = req.Request.PostFormValue("password") ) - user, err = h.AuthService.InternalLogin( - req.Context(), - email, - req.Request.PostFormValue("password"), - ) + err = func() (err error) { + if len(email) > 0 && len(password) == 0 && h.Settings.SplitCredentialsCheck { + // Email provided but no password and the + // split credentials check enabled + // + // SetKV w/ email will prevent the login form to show only email input + req.SetKV(map[string]string{"email": email}) + + // If user does not have password set (or in case there is no user with such email) + // and there is exactly one IdP we redirect user to that IdP + if !h.AuthService.PasswordSet(req.Context(), email) && len(h.Settings.Providers) == 1 { + // User w/o the password + // In case there is one single IdP automatically redirect user there + req.RedirectTo = fmt.Sprintf("%s/%s", GetLinks().External, h.Settings.Providers[0].Handle) + + // for the clarity of the flow + // keeping this optional return here + return + } + + // This user has existing password credentials or with more than one IdP + // + // Take user back to the login form and ask for the password + // or any other kind of login + return + } + + user, err = h.AuthService.InternalLogin(req.Context(), email, password) + + if err != nil { + return + } - if err == nil { var ( isPerm = len(req.Request.PostFormValue("keep-session")) > 0 lifetime = h.Opt.SessionLifetime @@ -69,8 +111,8 @@ func (h *AuthHandlers) loginProc(req *request.AuthReq) (err error) { handleSuccessfulAuth(req) - return nil - } + return + }() switch { case service.AuthErrInternalLoginDisabledByConfig().Is(err): diff --git a/auth/handlers/handle_login_test.go b/auth/handlers/handle_login_test.go index cd2d120d4..56d8e30bb 100644 --- a/auth/handlers/handle_login_test.go +++ b/auth/handlers/handle_login_test.go @@ -152,6 +152,41 @@ func Test_loginProc(t *testing.T) { } }, }, + { + name: "split credentials check", + payload: map[string]string{"email": "mockuser@example.tld"}, + alerts: []request.Alert(nil), + link: GetLinks().Login, + fn: func(authSettings *settings.Settings) { + req.PostForm.Add("email", "mockuser@example.tld") + authSettings.SplitCredentialsCheck = true + + authService = &authServiceMocked{ + passwordSet: func(ctx context.Context, email string) bool { + return false + }, + } + }, + }, + { + name: "split credentials check with providers", + payload: map[string]string{"email": "mockuser@example.tld"}, + alerts: []request.Alert(nil), + link: GetLinks().External + "/test-idp", + fn: func(authSettings *settings.Settings) { + req.PostForm.Add("email", "mockuser@example.tld") + authSettings.SplitCredentialsCheck = true + authSettings.Providers = []settings.Provider{ + {Handle: "test-idp"}, + } + + authService = &authServiceMocked{ + passwordSet: func(ctx context.Context, email string) bool { + return false + }, + } + }, + }, } for _, tc := range tcc { diff --git a/auth/handlers/handler.go b/auth/handlers/handler.go index e355d2e95..db650dab9 100644 --- a/auth/handlers/handler.go +++ b/auth/handlers/handler.go @@ -35,7 +35,7 @@ type ( SendEmailAddressConfirmationToken(ctx context.Context, u *types.User) (err error) SendPasswordResetToken(ctx context.Context, email string) (err error) GetProviders() types.ExternalAuthProviderSet - + PasswordSet(ctx context.Context, email string) (is bool) ValidateTOTP(ctx context.Context, code string) (err error) ConfigureTOTP(ctx context.Context, secret string, code string) (u *types.User, err error) RemoveTOTP(ctx context.Context, userID uint64, code string) (u *types.User, err error) diff --git a/auth/settings/settings.go b/auth/settings/settings.go index e417b5895..ace7474b0 100644 --- a/auth/settings/settings.go +++ b/auth/settings/settings.go @@ -7,6 +7,7 @@ type ( EmailConfirmationRequired bool PasswordResetEnabled bool ExternalEnabled bool + SplitCredentialsCheck bool Providers []Provider Saml SAML MultiFactor MultiFactor diff --git a/system/rest.yaml b/system/rest.yaml index c4032b328..dd3d91656 100644 --- a/system/rest.yaml +++ b/system/rest.yaml @@ -595,9 +595,8 @@ endpoints: post: - name: password type: string - required: true sensitive: true - title: New password + title: New password or empty to unset - name: membershipList method: GET diff --git a/system/rest/request/user.go b/system/rest/request/user.go index b1e93d52e..6990fe59b 100644 --- a/system/rest/request/user.go +++ b/system/rest/request/user.go @@ -221,7 +221,7 @@ type ( // Password POST parameter // - // New password + // New password or empty to unset Password string } diff --git a/system/service/auth.go b/system/service/auth.go index 259afaf15..1d3323f1e 100644 --- a/system/service/auth.go +++ b/system/service/auth.go @@ -617,26 +617,13 @@ func (svc auth) CheckPasswordStrength(password string) bool { func (svc auth) SetPasswordCredentials(ctx context.Context, userID uint64, password string) (err error) { var ( hash []byte - cc types.CredentialsSet - f = types.CredentialsFilter{Kind: credentialsTypePassword, OwnerID: userID} ) if hash, err = svc.hashPassword(password); err != nil { return } - if cc, _, err = store.SearchCredentials(ctx, svc.store, f); err != nil { - return nil - } - - // Mark all credentials as deleted - _ = cc.Walk(func(c *types.Credentials) error { - c.DeletedAt = now() - return nil - }) - - // Do a partial update and soft-delete all - if err = store.UpdateCredentials(ctx, svc.store, cc...); err != nil { + if err = svc.removePasswordCredentials(ctx, userID); err != nil { return } @@ -652,6 +639,33 @@ func (svc auth) SetPasswordCredentials(ctx context.Context, userID uint64, passw return store.CreateCredentials(ctx, svc.store, c) } +// RemovePasswordCredentials (soft) deletes old password entry +func (svc auth) RemovePasswordCredentials(ctx context.Context, userID uint64) (err error) { + // Do a partial update and soft-delete all + return svc.removePasswordCredentials(ctx, userID) +} + +// RemovePasswordCredentials (soft) deletes old password entry +func (svc auth) removePasswordCredentials(ctx context.Context, userID uint64) (err error) { + var ( + cc types.CredentialsSet + f = types.CredentialsFilter{Kind: credentialsTypePassword, OwnerID: userID} + ) + + if cc, _, err = store.SearchCredentials(ctx, svc.store, f); err != nil { + return nil + } + + // Mark all credentials as deleted + _ = cc.Walk(func(c *types.Credentials) error { + c.DeletedAt = now() + return nil + }) + + // Do a partial update and soft-delete all + return store.UpdateCredentials(ctx, svc.store, cc...) +} + // ValidateEmailConfirmationToken issues a validation token that can be used for func (svc auth) ValidateEmailConfirmationToken(ctx context.Context, token string) (user *types.User, err error) { return svc.loadFromTokenAndConfirmEmail(ctx, token, credentialsTypeEmailAuthToken) @@ -662,6 +676,31 @@ func (svc auth) ValidatePasswordResetToken(ctx context.Context, token string) (u return svc.loadFromTokenAndConfirmEmail(ctx, token, credentialsTypeResetPasswordToken) } +// PasswordSet checks and returns true if user's password is set +// False is also returned in case user does not exist. +func (svc *auth) PasswordSet(ctx context.Context, email string) (is bool) { + + //svc.settings.Auth.External.Enabled + u, err := store.LookupUserByEmail(ctx, svc.store, email) + if err != nil { + return + } + + cc, _, err := store.SearchCredentials(ctx, svc.store, types.CredentialsFilter{ + OwnerID: u.ID, + Kind: credentialsTypePassword, + }) + if err != nil { + return + } + + if len(cc) > 0 && svc.settings.Auth.Internal.Enabled { + return true + } + + return +} + // loadFromTokenAndConfirmEmail loads token, confirms user's func (svc auth) loadFromTokenAndConfirmEmail(ctx context.Context, token, tokenType string) (u *types.User, err error) { var ( diff --git a/system/service/user.go b/system/service/user.go index 45d926aa7..e8e412e92 100644 --- a/system/service/user.go +++ b/system/service/user.go @@ -47,6 +47,7 @@ type ( userAuth interface { CheckPasswordStrength(string) bool SetPasswordCredentials(context.Context, uint64, string) error + RemovePasswordCredentials(context.Context, uint64) error } userAccessController interface { @@ -674,11 +675,12 @@ func (svc user) Unsuspend(ctx context.Context, userID uint64) (err error) { // SetPassword sets new password for a user // -// Expecting setter to have permissions to update modify users and internal authentication enabled +// Expecting setter to have permissions to update users func (svc user) SetPassword(ctx context.Context, userID uint64, newPassword string) (err error) { var ( u *types.User uaProps = &userActionProps{user: &types.User{ID: userID}} + a = UserActionSetPassword ) err = func() error { @@ -696,6 +698,11 @@ func (svc user) SetPassword(ctx context.Context, userID uint64, newPassword stri return UserErrNotAllowedToUpdate() } + if newPassword == "" { + a = UserActionRemovePassword + return svc.auth.RemovePasswordCredentials(ctx, userID) + } + if !svc.auth.CheckPasswordStrength(newPassword) { return UserErrPasswordNotSecure() } @@ -707,7 +714,7 @@ func (svc user) SetPassword(ctx context.Context, userID uint64, newPassword stri return nil }() - return svc.recordAction(ctx, uaProps, UserActionSetPassword, err) + return svc.recordAction(ctx, uaProps, a, err) } diff --git a/system/service/user_actions.gen.go b/system/service/user_actions.gen.go index 05f01e45e..de43c94b8 100644 --- a/system/service/user_actions.gen.go +++ b/system/service/user_actions.gen.go @@ -506,6 +506,26 @@ func UserActionSetPassword(props ...*userActionProps) *userAction { return a } +// UserActionRemovePassword returns "system:user.removePassword" action +// +// This function is auto-generated. +// +func UserActionRemovePassword(props ...*userActionProps) *userAction { + a := &userAction{ + timestamp: time.Now(), + resource: "system:user", + action: "removePassword", + log: "password removed for {user}", + severity: actionlog.Notice, + } + + if len(props) > 0 { + a.props = props[0] + } + + return a +} + // UserActionDeleteAuthTokens returns "system:user.deleteAuthTokens" action // // This function is auto-generated. diff --git a/system/service/user_actions.yaml b/system/service/user_actions.yaml index 35a2824da..23d1d84db 100644 --- a/system/service/user_actions.yaml +++ b/system/service/user_actions.yaml @@ -59,6 +59,9 @@ actions: - action: setPassword log: "password changed for {user}" + - action: removePassword + log: "password removed for {user}" + - action: deleteAuthTokens log: "deleted auth tokens of {user}" diff --git a/system/types/app_settings.go b/system/types/app_settings.go index 6ab9602f9..4b14d62fb 100644 --- a/system/types/app_settings.go +++ b/system/types/app_settings.go @@ -49,6 +49,12 @@ type ( // Can users reset their passwords PasswordReset struct{ Enabled bool } `kv:"password-reset"` + + // Splits credentials check into 2 parts + // If user has password credentials it offers him to enter the password + // Otherwise we offer the user to choose among the enabled external providers + // If only one ext. provider is enabled, user is automatically redirected there + SplitCredentialsCheck bool `kv:"split-credentials-check"` } External struct {