diff --git a/changes/issue-12633-required-password-reset b/changes/issue-12633-required-password-reset new file mode 100644 index 0000000000..54dc590e69 --- /dev/null +++ b/changes/issue-12633-required-password-reset @@ -0,0 +1 @@ +* Fixed response status code to 403 when a user cannot change their password due to not being requested to by the admin or due to this user having Single-Sign-On (SSO) enabled. diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index 4a21685a3c..b1f4e3639a 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -3360,7 +3360,7 @@ func (s *integrationTestSuite) TestUsers() { s.DoJSON("POST", "/api/latest/fleet/perform_required_password_reset", performRequiredPasswordResetRequest{ Password: newRawPwd, ID: u.ID, - }, http.StatusInternalServerError, &perfPwdResetResp) // TODO: should be 40?, see #4406 + }, http.StatusForbidden, &perfPwdResetResp) s.token = s.getTestAdminToken() // login as that user to verify that the new password is active (userRawPwd was updated to the new pwd) diff --git a/server/service/integration_sso_test.go b/server/service/integration_sso_test.go index a0c7de1516..ade53abc18 100644 --- a/server/service/integration_sso_test.go +++ b/server/service/integration_sso_test.go @@ -13,10 +13,13 @@ import ( "strings" "testing" + "github.com/fleetdm/fleet/v4/server/datastore/mysql" "github.com/fleetdm/fleet/v4/server/datastore/redis/redistest" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/ptr" "github.com/fleetdm/fleet/v4/server/sso" + "github.com/fleetdm/fleet/v4/server/test" + "github.com/jmoiron/sqlx" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -196,6 +199,70 @@ func (s *integrationSSOTestSuite) TestSSOLogin() { }) } +func (s *integrationSSOTestSuite) TestPerformRequiredPasswordResetWithSSO() { + // ensure that on exit, the admin token is used + defer func() { s.token = s.getTestAdminToken() }() + + t := s.T() + + // create a non-SSO user + var createResp createUserResponse + userRawPwd := test.GoodPassword + params := fleet.UserPayload{ + Name: ptr.String("extra"), + Email: ptr.String("extra@asd.com"), + Password: ptr.String(userRawPwd), + GlobalRole: ptr.String(fleet.RoleObserver), + } + s.DoJSON("POST", "/api/latest/fleet/users/admin", params, http.StatusOK, &createResp) + assert.NotZero(t, createResp.User.ID) + assert.True(t, createResp.User.AdminForcedPasswordReset) + nonSSOUser := *createResp.User + + // enable SSO + acResp := appConfigResponse{} + s.DoJSON("PATCH", "/api/latest/fleet/config", json.RawMessage(`{ + "sso_settings": { + "enable_sso": true, + "entity_id": "https://localhost:8080", + "issuer_uri": "http://localhost:8080/simplesaml/saml2/idp/SSOService.php", + "idp_name": "SimpleSAML", + "metadata_url": "http://localhost:9080/simplesaml/saml2/idp/metadata.php" + } + }`), http.StatusOK, &acResp) + require.NotNil(t, acResp) + + // perform a required password change using the non-SSO user, works + s.token = s.getTestToken(nonSSOUser.Email, userRawPwd) + perfPwdResetResp := performRequiredPasswordResetResponse{} + newRawPwd := "new_password2!" + s.DoJSON("POST", "/api/latest/fleet/perform_required_password_reset", performRequiredPasswordResetRequest{ + Password: newRawPwd, + ID: nonSSOUser.ID, + }, http.StatusOK, &perfPwdResetResp) + require.False(t, perfPwdResetResp.User.AdminForcedPasswordReset) + + // trick the user into one with SSO enabled (we could create that user but it + // won't have a password nor an API token to use for the request, so we mock + // it in the DB) + mysql.ExecAdhocSQL(t, s.ds, func(db sqlx.ExtContext) error { + _, err := db.ExecContext( + context.Background(), + "UPDATE users SET sso_enabled = 1, admin_forced_password_reset = 1 WHERE id = ?", + nonSSOUser.ID, + ) + return err + }) + + // perform a required password change using the mocked SSO user, disallowed + perfPwdResetResp = performRequiredPasswordResetResponse{} + newRawPwd = "new_password2!" + s.DoJSON("POST", "/api/latest/fleet/perform_required_password_reset", performRequiredPasswordResetRequest{ + Password: newRawPwd, + ID: nonSSOUser.ID, + }, http.StatusForbidden, &perfPwdResetResp) +} + func inflate(t *testing.T, s string) *sso.AuthnRequest { t.Helper() diff --git a/server/service/users.go b/server/service/users.go index 9da533673a..f3841fbfdf 100644 --- a/server/service/users.go +++ b/server/service/users.go @@ -835,6 +835,7 @@ func (svc *Service) PerformRequiredPasswordReset(ctx context.Context, password s return nil, fleet.ErrNoContext } if !vc.CanPerformPasswordReset() { + svc.authz.SkipAuthorization(ctx) return nil, fleet.NewPermissionError("cannot reset password") } user := vc.User @@ -844,10 +845,16 @@ func (svc *Service) PerformRequiredPasswordReset(ctx context.Context, password s } if user.SSOEnabled { - return nil, ctxerr.New(ctx, "password reset for single sign on user not allowed") + // should never happen because this would get caught by the + // CanPerformPasswordReset check above + err := fleet.NewPermissionError("password reset for single sign on user not allowed") + return nil, ctxerr.Wrap(ctx, err) } if !user.IsAdminForcedPasswordReset() { - return nil, ctxerr.New(ctx, "user does not require password reset") + // should never happen because this would get caught by the + // CanPerformPasswordReset check above + err := fleet.NewPermissionError("cannot reset password") + return nil, ctxerr.Wrap(ctx, err) } // prevent setting the same password