From 0e99c8356a4b22582fdf14e7a3904e5058dec70d Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Mon, 22 Jul 2024 14:46:27 +0200 Subject: [PATCH] fix: correctly check user auth methods and enable button (#8342) # Which Problems Are Solved #8291 added backwards compatibilty for users who were created through the user V2 API and want to sign in to the login UI. There were however to issues, where users might be prompted to set a password even if they already had one set or they would not be able to submit the email verification code. # How the Problems Are Solved - Replaced `SearchUserAuthMethods `with `ListUserAuthMethodTypes` to check for set up auth methods. - Fixed page / javascript to disable submit button. # Additional Changes - Changed `ListActiveUserAuthMethodTypes ` to `ListUserAuthMethodTypes` and a `activeOnly` boolean parameter # Additional Context - relates to #8291 - noticed internally on QA --- internal/api/grpc/user/v2/user.go | 2 +- internal/api/ui/login/mail_verify_handler.go | 14 ++++---- .../static/templates/mail_verification.html | 6 +++- internal/query/user_auth_method.go | 20 ++++++----- internal/query/user_auth_method_test.go | 36 ++++++++++++++----- 5 files changed, 52 insertions(+), 26 deletions(-) diff --git a/internal/api/grpc/user/v2/user.go b/internal/api/grpc/user/v2/user.go index dc91fe3ef..dd96f3107 100644 --- a/internal/api/grpc/user/v2/user.go +++ b/internal/api/grpc/user/v2/user.go @@ -590,7 +590,7 @@ func (s *Server) checkIntentToken(token string, intentID string) error { } func (s *Server) ListAuthenticationMethodTypes(ctx context.Context, req *user.ListAuthenticationMethodTypesRequest) (*user.ListAuthenticationMethodTypesResponse, error) { - authMethods, err := s.query.ListActiveUserAuthMethodTypes(ctx, req.GetUserId()) + authMethods, err := s.query.ListUserAuthMethodTypes(ctx, req.GetUserId(), true) if err != nil { return nil, err } diff --git a/internal/api/ui/login/mail_verify_handler.go b/internal/api/ui/login/mail_verify_handler.go index 85c611457..a14aa197a 100644 --- a/internal/api/ui/login/mail_verify_handler.go +++ b/internal/api/ui/login/mail_verify_handler.go @@ -4,12 +4,12 @@ import ( "context" "net/http" "net/url" + "slices" "github.com/zitadel/logging" http_mw "github.com/zitadel/zitadel/internal/api/http/middleware" "github.com/zitadel/zitadel/internal/domain" - "github.com/zitadel/zitadel/internal/query" "github.com/zitadel/zitadel/internal/zerrors" ) @@ -72,16 +72,16 @@ func (l *Login) handleMailVerification(w http.ResponseWriter, r *http.Request) { } func (l *Login) checkUserNoFirstFactor(ctx context.Context, userID string) bool { - userIDQuery, err := query.NewUserAuthMethodUserIDSearchQuery(userID) - logging.WithFields("userID", userID).OnError(err).Warn("error creating NewUserAuthMethodUserIDSearchQuery") - authMethodsQuery, err := query.NewUserAuthMethodTypesSearchQuery(domain.UserAuthMethodTypeIDP, domain.UserAuthMethodTypePassword, domain.UserAuthMethodTypePasswordless) - logging.WithFields("userID", userID).OnError(err).Warn("error creating NewUserAuthMethodTypesSearchQuery") - authMethods, err := l.query.SearchUserAuthMethods(ctx, &query.UserAuthMethodSearchQueries{Queries: []query.SearchQuery{userIDQuery, authMethodsQuery}}, false) + authMethods, err := l.query.ListUserAuthMethodTypes(setUserContext(ctx, userID, ""), userID, false) if err != nil { logging.WithFields("userID", userID).OnError(err).Warn("unable to load user's auth methods for mail verification") return false } - return len(authMethods.AuthMethods) == 0 + return !slices.ContainsFunc(authMethods.AuthMethodTypes, func(m domain.UserAuthMethodType) bool { + return m == domain.UserAuthMethodTypeIDP || + m == domain.UserAuthMethodTypePassword || + m == domain.UserAuthMethodTypePasswordless + }) } func (l *Login) handleMailVerificationCheck(w http.ResponseWriter, r *http.Request) { diff --git a/internal/api/ui/login/static/templates/mail_verification.html b/internal/api/ui/login/static/templates/mail_verification.html index 011947c17..961718567 100644 --- a/internal/api/ui/login/static/templates/mail_verification.html +++ b/internal/api/ui/login/static/templates/mail_verification.html @@ -39,7 +39,7 @@ {{ template "error-message" .}}
- @@ -56,7 +56,11 @@
+{{ if .PasswordInit }} +{{ else }} + +{{ end }} {{template "main-bottom" .}} diff --git a/internal/query/user_auth_method.go b/internal/query/user_auth_method.go index f1e2721ba..243c465a8 100644 --- a/internal/query/user_auth_method.go +++ b/internal/query/user_auth_method.go @@ -146,7 +146,7 @@ func (q *Queries) SearchUserAuthMethods(ctx context.Context, queries *UserAuthMe return userAuthMethods, err } -func (q *Queries) ListActiveUserAuthMethodTypes(ctx context.Context, userID string) (userAuthMethodTypes *AuthMethodTypes, err error) { +func (q *Queries) ListUserAuthMethodTypes(ctx context.Context, userID string, activeOnly bool) (userAuthMethodTypes *AuthMethodTypes, err error) { ctxData := authz.GetCtxData(ctx) if ctxData.UserID != userID { if err := q.checkPermission(ctx, domain.PermissionUserRead, ctxData.OrgID, userID); err != nil { @@ -156,7 +156,7 @@ func (q *Queries) ListActiveUserAuthMethodTypes(ctx context.Context, userID stri ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() - query, scan := prepareActiveUserAuthMethodTypesQuery(ctx, q.client) + query, scan := prepareUserAuthMethodTypesQuery(ctx, q.client, activeOnly) eq := sq.Eq{ UserIDCol.identifier(): userID, UserInstanceIDCol.identifier(): authz.GetInstance(ctx).InstanceID(), @@ -353,8 +353,8 @@ func prepareUserAuthMethodsQuery(ctx context.Context, db prepareDatabase) (sq.Se } } -func prepareActiveUserAuthMethodTypesQuery(ctx context.Context, db prepareDatabase) (sq.SelectBuilder, func(*sql.Rows) (*AuthMethodTypes, error)) { - authMethodsQuery, authMethodsArgs, err := prepareAuthMethodQuery() +func prepareUserAuthMethodTypesQuery(ctx context.Context, db prepareDatabase, activeOnly bool) (sq.SelectBuilder, func(*sql.Rows) (*AuthMethodTypes, error)) { + authMethodsQuery, authMethodsArgs, err := prepareAuthMethodQuery(activeOnly) if err != nil { return sq.SelectBuilder{}, nil } @@ -468,14 +468,16 @@ func prepareAuthMethodsIDPsQuery() (string, error) { return idpsQuery, err } -func prepareAuthMethodQuery() (string, []interface{}, error) { - return sq.Select( +func prepareAuthMethodQuery(activeOnly bool) (string, []interface{}, error) { + q := sq.Select( "DISTINCT("+authMethodTypeType.identifier()+")", authMethodTypeUserID.identifier(), authMethodTypeInstanceID.identifier()). - From(authMethodTypeTable.identifier()). - Where(sq.Eq{authMethodTypeState.identifier(): domain.MFAStateReady}). - ToSql() + From(authMethodTypeTable.identifier()) + if activeOnly { + q = q.Where(sq.Eq{authMethodTypeState.identifier(): domain.MFAStateReady}) + } + return q.ToSql() } func prepareAuthMethodsForceMFAQuery() (string, error) { diff --git a/internal/query/user_auth_method_test.go b/internal/query/user_auth_method_test.go index 59004f5da..95f9a0229 100644 --- a/internal/query/user_auth_method_test.go +++ b/internal/query/user_auth_method_test.go @@ -217,8 +217,13 @@ func Test_UserAuthMethodPrepares(t *testing.T) { object: (*AuthMethodTypes)(nil), }, { - name: "prepareActiveUserAuthMethodTypesQuery no result", - prepare: prepareActiveUserAuthMethodTypesQuery, + name: "prepareUserAuthMethodTypesQuery no result", + prepare: func(ctx context.Context, db prepareDatabase) (sq.SelectBuilder, func(*sql.Rows) (*AuthMethodTypes, error)) { + builder, scan := prepareUserAuthMethodTypesQuery(ctx, db, true) + return builder, func(rows *sql.Rows) (*AuthMethodTypes, error) { + return scan(rows) + } + }, want: want{ sqlExpectations: mockQueries( regexp.QuoteMeta(prepareActiveAuthMethodTypesStmt), @@ -229,8 +234,13 @@ func Test_UserAuthMethodPrepares(t *testing.T) { object: &AuthMethodTypes{AuthMethodTypes: []domain.UserAuthMethodType{}}, }, { - name: "prepareActiveUserAuthMethodTypesQuery one second factor", - prepare: prepareActiveUserAuthMethodTypesQuery, + name: "prepareUserAuthMethodTypesQuery one second factor", + prepare: func(ctx context.Context, db prepareDatabase) (sq.SelectBuilder, func(*sql.Rows) (*AuthMethodTypes, error)) { + builder, scan := prepareUserAuthMethodTypesQuery(ctx, db, true) + return builder, func(rows *sql.Rows) (*AuthMethodTypes, error) { + return scan(rows) + } + }, want: want{ sqlExpectations: mockQueries( regexp.QuoteMeta(prepareActiveAuthMethodTypesStmt), @@ -256,8 +266,13 @@ func Test_UserAuthMethodPrepares(t *testing.T) { }, }, { - name: "prepareActiveUserAuthMethodTypesQuery multiple second factors", - prepare: prepareActiveUserAuthMethodTypesQuery, + name: "prepareUserAuthMethodTypesQuery multiple second factors", + prepare: func(ctx context.Context, db prepareDatabase) (sq.SelectBuilder, func(*sql.Rows) (*AuthMethodTypes, error)) { + builder, scan := prepareUserAuthMethodTypesQuery(ctx, db, true) + return builder, func(rows *sql.Rows) (*AuthMethodTypes, error) { + return scan(rows) + } + }, want: want{ sqlExpectations: mockQueries( regexp.QuoteMeta(prepareActiveAuthMethodTypesStmt), @@ -289,8 +304,13 @@ func Test_UserAuthMethodPrepares(t *testing.T) { }, }, { - name: "prepareActiveUserAuthMethodTypesQuery sql err", - prepare: prepareActiveUserAuthMethodTypesQuery, + name: "prepareUserAuthMethodTypesQuery sql err", + prepare: func(ctx context.Context, db prepareDatabase) (sq.SelectBuilder, func(*sql.Rows) (*AuthMethodTypes, error)) { + builder, scan := prepareUserAuthMethodTypesQuery(ctx, db, true) + return builder, func(rows *sql.Rows) (*AuthMethodTypes, error) { + return scan(rows) + } + }, want: want{ sqlExpectations: mockQueryErr( regexp.QuoteMeta(prepareActiveAuthMethodTypesStmt),