From 7f8ccf7143d49b3ba2b849a2e54644e2528377f1 Mon Sep 17 00:00:00 2001 From: joerger Date: Fri, 15 Dec 2023 18:43:15 -0800 Subject: [PATCH] Automatically set passwordless connector from Auth service instead of Proxy service. --- lib/auth/auth_with_roles.go | 45 ++++++++++- lib/web/apiserver.go | 44 ----------- lib/web/apiserver_test.go | 154 ++++++++++++++++++------------------ 3 files changed, 121 insertions(+), 122 deletions(-) diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index f9cbd14eec68d..6146fe7084a4c 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -3146,7 +3146,49 @@ func (a *ServerWithRoles) GetResetPasswordToken(ctx context.Context, tokenID str // ChangeUserAuthentication is implemented by AuthService.ChangeUserAuthentication. func (a *ServerWithRoles) ChangeUserAuthentication(ctx context.Context, req *proto.ChangeUserAuthenticationRequest) (*proto.ChangeUserAuthenticationResponse, error) { // Token is it's own authentication, no need to double check. - return a.authServer.ChangeUserAuthentication(ctx, req) + resp, err := a.authServer.ChangeUserAuthentication(ctx, req) + if err != nil { + return nil, trace.Wrap(err) + } + + // We use the presence of a WebAuthn response, along with the absence of a + // password, as a proxy to determine that a passwordless registration took + // place, as it is not possible to infer that just from the WebAuthn response. + isPasswordless := req.NewMFARegisterResponse != nil && len(req.NewPassword) == 0 + if isPasswordless && modules.GetModules().Features().Cloud { + if err := a.trySettingConnectorNameToPasswordless(ctx); err != nil { + log.WithError(err).Error("Failed to set passwordless as connector name.") + } + } + + return resp, nil +} + +// trySettingConnectorNameToPasswordless sets cluster_auth_preference connectorName to `passwordless` when the first cloud user chooses passwordless as the authentication method. +// This simplifies UX for cloud users, as they will not need to select a passwordless connector when logging in. +func (a *ServerWithRoles) trySettingConnectorNameToPasswordless(ctx context.Context) error { + users, err := a.authServer.GetUsers(ctx, false) + if err != nil { + return trace.Wrap(err) + } + + // Only set the connector name on the first user registration. + if len(users) != 1 { + return nil + } + + authPreference, err := a.authServer.GetAuthPreference(ctx) + if err != nil { + return trace.Wrap(err) + } + + // Don't overwrite an existing connector name. + if connector := authPreference.GetConnectorName(); connector != "" && connector != constants.LocalConnector { + return nil + } + + authPreference.SetConnectorName(constants.PasswordlessConnector) + return trace.Wrap(a.authServer.SetAuthPreference(ctx, authPreference)) } // UpdateUser updates an existing user in a backend. @@ -6869,7 +6911,6 @@ func checkOktaLockTarget(ctx context.Context, authzCtx *authz.Context, users ser // checkOktaLockAccess gates access to update operations on lock records based // on the origin label on the supplied user record. func checkOktaLockAccess(ctx context.Context, authzCtx *authz.Context, locks services.LockGetter, existingLockName string, verb string) error { - existingLock, err := locks.GetLock(ctx, existingLockName) if err != nil && !trace.IsNotFound(err) { return trace.Wrap(err) diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index ffec5624b2c77..cbbccefa30363 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -2271,11 +2271,6 @@ func (h *Handler) changeUserAuthentication(w http.ResponseWriter, r *http.Reques return nil, trace.Wrap(err) } - err = h.trySettingConnectorNameToPasswordless(r.Context(), ctx, req) - if err != nil { - h.log.WithError(err).Error("Failed to set passwordless as connector name.") - } - if err := websession.SetCookie(w, sess.GetUser(), sess.GetName()); err != nil { return nil, trace.Wrap(err) } @@ -2297,45 +2292,6 @@ func (h *Handler) changeUserAuthentication(w http.ResponseWriter, r *http.Reques }, nil } -// trySettingConnectorNameToPasswordless sets cluster_auth_preference connectorName to `passwordless` when the first cloud user chooses passwordless as the authentication method. -// This simplifies UX for cloud users, as they will not need to select a passwordless connector when logging in. -func (h *Handler) trySettingConnectorNameToPasswordless(ctx context.Context, sessCtx *SessionContext, req changeUserAuthenticationRequest) error { - // We use the presence of a WebAuthn response, along with the absence of a - // password, as a proxy to determine that a passwordless registration took - // place, as it is not possible to infer that just from the WebAuthn response. - isPasswordlessRegistration := req.WebauthnCreationResponse != nil && len(req.Password) == 0 - if !isPasswordlessRegistration { - return nil - } - - if !h.ClusterFeatures.GetCloud() { - return nil - } - - authPreference, err := sessCtx.cfg.RootClient.GetAuthPreference(ctx) - if err != nil { - return nil - } - - if connector := authPreference.GetConnectorName(); connector != "" && connector != constants.LocalConnector { - return nil - } - - users, err := h.cfg.ProxyClient.GetUsers(ctx, false) - if err != nil { - return trace.Wrap(err) - } - - if len(users) != 1 { - return nil - } - - authPreference.SetConnectorName(constants.PasswordlessConnector) - - err = sessCtx.cfg.RootClient.SetAuthPreference(ctx, authPreference) - return trace.Wrap(err) -} - // createResetPasswordToken allows a UI user to reset a user's password. // This handler is also required for after creating new users. func (h *Handler) createResetPasswordToken(w http.ResponseWriter, r *http.Request, _ httprouter.Params, ctx *SessionContext) (interface{}, error) { diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index f7ecebb478a01..2df9e86f391a9 100644 --- a/lib/web/apiserver_test.go +++ b/lib/web/apiserver_test.go @@ -5569,7 +5569,7 @@ func TestChangeUserAuthentication_settingDefaultClusterAuthPreference(t *testing initialConnectorName string resultConnectorName string }{{ - name: "first cloud sign-in changes connector to `passwordless`", + name: "first cloud sign-in changes connector to passwordless", cloud: true, numberOfUsers: 1, authPreferenceType: constants.Local, @@ -5607,107 +5607,109 @@ func TestChangeUserAuthentication_settingDefaultClusterAuthPreference(t *testing }} for _, tc := range tt { - modules.SetTestModules(t, &modules.TestModules{ - TestFeatures: modules.Features{ - Cloud: tc.cloud, - }, - }) + t.Run(tc.name, func(t *testing.T) { + modules.SetTestModules(t, &modules.TestModules{ + TestFeatures: modules.Features{ + Cloud: tc.cloud, + }, + }) - const RPID = "localhost" + const RPID = "localhost" - s := newWebSuiteWithConfig(t, webSuiteConfig{ - authPreferenceSpec: &types.AuthPreferenceSpecV2{ - Type: tc.authPreferenceType, - ConnectorName: tc.initialConnectorName, - SecondFactor: constants.SecondFactorOn, - Webauthn: &types.Webauthn{ - RPID: RPID, + s := newWebSuiteWithConfig(t, webSuiteConfig{ + authPreferenceSpec: &types.AuthPreferenceSpecV2{ + Type: tc.authPreferenceType, + ConnectorName: tc.initialConnectorName, + SecondFactor: constants.SecondFactorOn, + Webauthn: &types.Webauthn{ + RPID: RPID, + }, }, - }, - }) + }) - // user and role - users := make([]types.User, tc.numberOfUsers) + // user and role + users := make([]types.User, tc.numberOfUsers) - for i := 0; i < tc.numberOfUsers; i++ { - user, err := types.NewUser(fmt.Sprintf("test_user_%v", i)) - require.NoError(t, err) + for i := 0; i < tc.numberOfUsers; i++ { + user, err := types.NewUser(fmt.Sprintf("test_user_%v", i)) + require.NoError(t, err) - user.SetCreatedBy(types.CreatedBy{ - User: types.UserRef{Name: "other_user"}, - }) + user.SetCreatedBy(types.CreatedBy{ + User: types.UserRef{Name: "other_user"}, + }) - role := services.RoleForUser(user) + role := services.RoleForUser(user) - role, err = s.server.Auth().UpsertRole(s.ctx, role) - require.NoError(t, err) + role, err = s.server.Auth().UpsertRole(s.ctx, role) + require.NoError(t, err) - user.AddRole(role.GetName()) + user.AddRole(role.GetName()) - user, err = s.server.Auth().CreateUser(s.ctx, user) - require.NoError(t, err) + user, err = s.server.Auth().CreateUser(s.ctx, user) + require.NoError(t, err) - users[i] = user - } + users[i] = user + } - initialUser := users[0] + initialUser := users[0] - clt := s.client(t) + clt := s.client(t) - // create register challenge - token, err := s.server.Auth().CreateResetPasswordToken(s.ctx, auth.CreateUserTokenRequest{ - Name: initialUser.GetName(), - }) - require.NoError(t, err) + // create register challenge + token, err := s.server.Auth().CreateResetPasswordToken(s.ctx, auth.CreateUserTokenRequest{ + Name: initialUser.GetName(), + }) + require.NoError(t, err) - res, err := s.server.Auth().CreateRegisterChallenge(s.ctx, &authproto.CreateRegisterChallengeRequest{ - TokenID: token.GetName(), - DeviceType: authproto.DeviceType_DEVICE_TYPE_WEBAUTHN, - DeviceUsage: authproto.DeviceUsage_DEVICE_USAGE_PASSWORDLESS, - }) - require.NoError(t, err) + res, err := s.server.Auth().CreateRegisterChallenge(s.ctx, &authproto.CreateRegisterChallengeRequest{ + TokenID: token.GetName(), + DeviceType: authproto.DeviceType_DEVICE_TYPE_WEBAUTHN, + DeviceUsage: authproto.DeviceUsage_DEVICE_USAGE_PASSWORDLESS, + }) + require.NoError(t, err) - cc := wantypes.CredentialCreationFromProto(res.GetWebauthn()) + cc := wantypes.CredentialCreationFromProto(res.GetWebauthn()) - // use passwordless as auth method - device, err := mocku2f.Create() - require.NoError(t, err) + // use passwordless as auth method + device, err := mocku2f.Create() + require.NoError(t, err) - device.SetPasswordless() + device.SetPasswordless() - ccr, err := device.SignCredentialCreation("https://"+RPID, cc) - require.NoError(t, err) + ccr, err := device.SignCredentialCreation("https://"+RPID, cc) + require.NoError(t, err) - // send sign-in response to server - body, err := json.Marshal(changeUserAuthenticationRequest{ - WebauthnCreationResponse: ccr, - TokenID: token.GetName(), - DeviceName: "passwordless-device", - Password: tc.password, - }) - require.NoError(t, err) + // send sign-in response to server + body, err := json.Marshal(changeUserAuthenticationRequest{ + WebauthnCreationResponse: ccr, + TokenID: token.GetName(), + DeviceName: "passwordless-device", + Password: tc.password, + }) + require.NoError(t, err) - req, err := http.NewRequest("PUT", clt.Endpoint("webapi", "users", "password", "token"), bytes.NewBuffer(body)) - require.NoError(t, err) + req, err := http.NewRequest("PUT", clt.Endpoint("webapi", "users", "password", "token"), bytes.NewBuffer(body)) + require.NoError(t, err) - csrfToken, err := csrf.GenerateToken() - require.NoError(t, err) - addCSRFCookieToReq(req, csrfToken) - req.Header.Set(csrf.HeaderName, csrfToken) - req.Header.Set("Content-Type", "application/json") + csrfToken, err := csrf.GenerateToken() + require.NoError(t, err) + addCSRFCookieToReq(req, csrfToken) + req.Header.Set(csrf.HeaderName, csrfToken) + req.Header.Set("Content-Type", "application/json") - re, err := clt.Client.RoundTrip(func() (*http.Response, error) { - return clt.Client.HTTPClient().Do(req) - }) + re, err := clt.Client.RoundTrip(func() (*http.Response, error) { + return clt.Client.HTTPClient().Do(req) + }) - require.NoError(t, err) - require.Equal(t, http.StatusOK, re.Code()) + require.NoError(t, err) + require.Equal(t, http.StatusOK, re.Code()) - // check if auth preference connectorName is set - authPreference, err := s.server.Auth().GetAuthPreference(s.ctx) - require.NoError(t, err) + // check if auth preference connectorName is set + authPreference, err := s.server.Auth().GetAuthPreference(s.ctx) + require.NoError(t, err) - require.Equal(t, authPreference.GetConnectorName(), tc.resultConnectorName, "Found unexpected auth connector name") + require.Equal(t, tc.resultConnectorName, authPreference.GetConnectorName(), "Found unexpected auth connector name") + }) } }