From 97cd2f8aecc4e2c5fff5cb85af180117d1ef1503 Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Tue, 13 Oct 2026 15:22:57 +1300 Subject: [PATCH] feat: optional different deploy approver, and admin Reset MFA - A global setting, deploy_approval_require_different_approver (off by default), refuses approving or extending a deploy approval by the user who created the request or owns its API token, with a 403 that says which. It fails closed when the setting can't be read, and the approval's audit entry records that the rule applied. It's on the Settings page and settable with RGW_SETTING_*. Deploy approval requests now load their token's owner (TargetUserID was never set, so the owner of the target token couldn't view a request someone else created for it) (#131) - Admins can reset another user's MFA (POST /api/v1/users/:email/mfa/reset, Reset MFA on the user's page): it removes their MFA methods, backup codes and trusted devices and revokes their sessions in one transaction, keeps their API tokens, needs MFA on the request, is audited as user.mfa_reset, and emails the user (#135) Fixes #131, fixes #135 Co-Authored-By: Claude Opus 5.5 --- .../configuration/environment-variables.mdx | 1 + .../docs/development/api-reference.mdx | 1 + .../integrations/deploy-approvals/index.mdx | 10 ++ .../deploy-approvals/workflow.mdx | 9 +- .../docs/operations/troubleshooting.mdx | 5 +- .../content/docs/security/compliance/soc2.mdx | 8 +- .../docs/user-guide/mfa/backup-codes.mdx | 5 +- .../user-guide/web-ui/user-management.mdx | 13 ++ internal/gateway/audit/constants.go | 1 + internal/gateway/db/deploy_approval_scan.go | 7 +- internal/gateway/db/mfa.go | 16 ++- internal/gateway/handlers/admin_users_lock.go | 6 +- .../gateway/handlers/admin_users_mfa_reset.go | 81 +++++++++++ .../handlers/admin_users_mfa_reset_test.go | 118 +++++++++++++++ .../gateway/handlers/deploy_approval_admin.go | 28 ++-- .../handlers/deploy_approval_approver.go | 82 +++++++++++ .../handlers/deploy_approval_approver_test.go | 136 ++++++++++++++++++ .../gateway/openapi/generated/swagger.json | 67 +++++++++ internal/gateway/rbac/audit_names.go | 1 + internal/gateway/rbac/gateway_routes.go | 1 + internal/gateway/routes/route_registration.go | 1 + internal/gateway/settings/defaults.go | 20 +-- internal/gateway/settings/groups.go | 1 + internal/gateway/settings/service_deploy.go | 31 ++-- web/src/api/generated.ts | 23 +++ web/src/api/openapi.json | 90 ++++++++++++ web/src/api/types.generated.ts | 88 ++++++++++++ .../components/user-mfa-reset-dialog.test.tsx | 57 ++++++++ web/src/components/user-mfa-reset-dialog.tsx | 71 +++++++++ web/src/lib/api.ts | 4 + web/src/lib/generated/mfa-requirements.ts | 6 + web/src/pages/settings-page.test.tsx | 25 ++++ .../pages/settings/deploy-approvals-card.tsx | 135 ++++++++--------- web/src/pages/user-page.tsx | 9 ++ web/src/pages/user/header-section.tsx | 21 ++- web/src/pages/user/use-user-danger-zone.ts | 5 + 36 files changed, 1053 insertions(+), 130 deletions(-) create mode 100644 internal/gateway/handlers/admin_users_mfa_reset.go create mode 100644 internal/gateway/handlers/admin_users_mfa_reset_test.go create mode 100644 internal/gateway/handlers/deploy_approval_approver.go create mode 100644 internal/gateway/handlers/deploy_approval_approver_test.go create mode 100644 web/src/components/user-mfa-reset-dialog.test.tsx create mode 100644 web/src/components/user-mfa-reset-dialog.tsx diff --git a/docs/src/content/docs/configuration/environment-variables.mdx b/docs/src/content/docs/configuration/environment-variables.mdx index af896a3f..257bd585 100644 --- a/docs/src/content/docs/configuration/environment-variables.mdx +++ b/docs/src/content/docs/configuration/environment-variables.mdx @@ -194,6 +194,7 @@ startup log lists them, and the settings page marks them and lets an admin clear | `RGW_SETTING_MFA_TRUSTED_DEVICE_TTL_DAYS` | Trusted device TTL | | `RGW_SETTING_DEPLOY_APPROVALS_ENABLED` | Enable deploy approvals | | `RGW_SETTING_DEPLOY_APPROVAL_WINDOW_MINUTES` | Approval window | +| `RGW_SETTING_DEPLOY_APPROVAL_REQUIRE_DIFFERENT_APPROVER` | Require a different approver | | `RGW_SETTING_ALLOW_DESTRUCTIVE_ACTIONS` | Allow destructive actions | | `RGW_SETTING_DEFAULT_CI_PROVIDER` | Default CI provider | | `RGW_SETTING_DEFAULT_VCS_PROVIDER` | Default VCS provider | diff --git a/docs/src/content/docs/development/api-reference.mdx b/docs/src/content/docs/development/api-reference.mdx index 3f7d24c8..931ef751 100644 --- a/docs/src/content/docs/development/api-reference.mdx +++ b/docs/src/content/docs/development/api-reference.mdx @@ -100,6 +100,7 @@ All under `/auth/mfa`: - `POST /users/:email/sessions/:sessionID/revoke` - `POST /users/:email/sessions/revoke_all` - `POST /users/:email/lock`, `POST /users/:email/unlock` +- `POST /users/:email/mfa/reset`: removes another user's MFA methods, backup codes and trusted devices and signs them out - `GET /api-tokens`, `GET /api-tokens/:tokenID`, `POST /api-tokens` - `PUT /api-tokens/:tokenID`, `DELETE /api-tokens/:tokenID` - `GET /api-tokens/permissions` diff --git a/docs/src/content/docs/integrations/deploy-approvals/index.mdx b/docs/src/content/docs/integrations/deploy-approvals/index.mdx index 91d6bd43..ea97f530 100644 --- a/docs/src/content/docs/integrations/deploy-approvals/index.mdx +++ b/docs/src/content/docs/integrations/deploy-approvals/index.mdx @@ -138,6 +138,15 @@ Approving, rejecting and extending a request ask for MFA every time, even within - Auditable proof of human authorization - API tokens can't approve +### Different Approver (Optional) + +For segregation of duties, turn on **Require a different approver** (Settings > Deploy Approvals, or +`RGW_SETTING_DEPLOY_APPROVAL_REQUIRE_DIFFERENT_APPROVER=true`). Nobody can then approve or extend a deploy they +asked for: the user who created the request, or the owner of the API token it's for. They get a 403 that says +which. The approval's audit entry records `different_approver_required: true`. + +It's off by default, so a team with one admin can still deploy. + ### Time-Limited Approvals Approvals expire after a configurable window (default 15 minutes): @@ -203,6 +212,7 @@ The `deploy_approval_requests` table tracks the complete lifecycle: |---------|---------|-------------| | `RGW_SETTING_DEPLOY_APPROVALS_ENABLED` | `true` | Enable/disable approval checks | | `RGW_SETTING_DEPLOY_APPROVAL_WINDOW_MINUTES` | `15` | How long approvals remain valid | +| `RGW_SETTING_DEPLOY_APPROVAL_REQUIRE_DIFFERENT_APPROVER` | `false` | Refuse approvals and extensions by the request's creator or its token's owner | ### Per-App Settings diff --git a/docs/src/content/docs/integrations/deploy-approvals/workflow.mdx b/docs/src/content/docs/integrations/deploy-approvals/workflow.mdx index 9b44f8a0..01e14ddf 100644 --- a/docs/src/content/docs/integrations/deploy-approvals/workflow.mdx +++ b/docs/src/content/docs/integrations/deploy-approvals/workflow.mdx @@ -142,11 +142,16 @@ The review UI shows: Server validates the MFA response -4. **Request marked approved** +4. **Approver checked** + + With [Require a different approver](/integrations/deploy-approvals/#different-approver-optional) on, the + request's creator and its API token's owner are refused with 403 + +5. **Request marked approved** Status changes, expiration timer starts -5. **CI auto-approval triggered** +6. **CI auto-approval triggered** If configured, gateway calls CI API diff --git a/docs/src/content/docs/operations/troubleshooting.mdx b/docs/src/content/docs/operations/troubleshooting.mdx index fd5f2795..fa58f011 100644 --- a/docs/src/content/docs/operations/troubleshooting.mdx +++ b/docs/src/content/docs/operations/troubleshooting.mdx @@ -148,8 +148,9 @@ user's. ### Lost MFA Device 1. Sign in with a backup code, then remove the lost method and enroll a new one under **Account Security** -2. Without any method or backup code, an admin must delete the user and add them again. That deletes their API - tokens too, so CI using them needs new tokens +2. Without any method or backup code, an admin chooses **Reset MFA** on the user's page (after confirming who's + asking). It removes their methods, backup codes and trusted devices and signs them out; their API tokens are kept. + They set up a new method when they next sign in. If the account was locked by failed attempts, unlock it too ## Database Issues diff --git a/docs/src/content/docs/security/compliance/soc2.mdx b/docs/src/content/docs/security/compliance/soc2.mdx index 2971091d..83c07ca4 100644 --- a/docs/src/content/docs/security/compliance/soc2.mdx +++ b/docs/src/content/docs/security/compliance/soc2.mdx @@ -141,9 +141,11 @@ graph LR - CI/CD deployments require human approval - Only users with the approve permission (admins) can approve, with MFA on the request - Full audit trail of approvals +- Optionally, a different approver: with `deploy_approval_require_different_approver` on, nobody can approve or + extend a deploy they requested (they created the request, or own its API token), and each approval's audit + entry records that the rule applied -The gateway doesn't stop a token's owner approving their own pipeline's request. If your controls require a -different approver, give CI tokens to a user who isn't an approver, or check it in your review process. +The setting is off by default. Turn it on if your controls require segregation of duties. ### CC9: Risk Mitigation @@ -240,7 +242,7 @@ curl -b "session_token=TOKEN" \ ### Change Management (CC8) - [ ] Deploy approvals enabled -- [ ] Approvers are not the owners of the CI tokens they approve +- [ ] Different approver required (`RGW_SETTING_DEPLOY_APPROVAL_REQUIRE_DIFFERENT_APPROVER=true`) - [ ] CI/CD uses dedicated tokens - [ ] Change log maintained diff --git a/docs/src/content/docs/user-guide/mfa/backup-codes.mdx b/docs/src/content/docs/user-guide/mfa/backup-codes.mdx index 7c5e7972..49b92b98 100644 --- a/docs/src/content/docs/user-guide/mfa/backup-codes.mdx +++ b/docs/src/content/docs/user-guide/mfa/backup-codes.mdx @@ -15,8 +15,9 @@ Backup codes provide emergency access if you lose your primary MFA device. - **Backup Codes** in Account Security shows how many unused codes you have and when they were generated ## Getting Backup Codes diff --git a/docs/src/content/docs/user-guide/web-ui/user-management.mdx b/docs/src/content/docs/user-guide/web-ui/user-management.mdx index 54bd769b..a4769eac 100644 --- a/docs/src/content/docs/user-guide/web-ui/user-management.mdx +++ b/docs/src/content/docs/user-guide/web-ui/user-management.mdx @@ -67,6 +67,19 @@ unlocked. A lock needs a reason: the user's email includes it, and in the web UI **Unlock Account** reverses it. The user is emailed when locked and when unlocked. You can't lock or delete your own account. +## Resetting MFA (Admin) + +For a user who has lost every MFA method and their backup codes, choose **Reset MFA** on their detail page (shown for +users who have MFA set up) and confirm. Confirm who's asking before you do: the reset lets the user sign in with only +their Google account until they set up a new method. It needs MFA from you on the request. + +The reset removes their MFA methods, backup codes and trusted devices and signs them out everywhere. Their API tokens +are kept, so CI keeps working. They're emailed, and they set up a new method when they next sign in. The audit log +records it as `user.mfa_reset`. + +You can't reset your own MFA: manage your methods under **Account Security**. A reset doesn't unlock an account; if +failed MFA attempts locked it, unlock it as well. + ## Deleting Users (Admin) diff --git a/internal/gateway/audit/constants.go b/internal/gateway/audit/constants.go index b0598173..2fd56d12 100644 --- a/internal/gateway/audit/constants.go +++ b/internal/gateway/audit/constants.go @@ -54,6 +54,7 @@ const ( ActionVerbExport = "export" ActionVerbFailed = "failed" ActionVerbLock = "lock" + ActionVerbMFAReset = "mfa_reset" ActionVerbOAuthFailed = "oauth_failed" ActionVerbParamsSet = "params.set" ActionVerbReject = "reject" diff --git a/internal/gateway/db/deploy_approval_scan.go b/internal/gateway/db/deploy_approval_scan.go index accd67ab..1d959e2e 100644 --- a/internal/gateway/db/deploy_approval_scan.go +++ b/internal/gateway/db/deploy_approval_scan.go @@ -24,6 +24,7 @@ SELECT dr.target_api_token_id, target_token.public_id, target_token.name, + target_token.user_id, dr.approved_by_user_id, approved_user.email, approved_user.name, @@ -61,6 +62,7 @@ type deployApprovalNullables struct { createdTokenName sql.NullString targetTokenPublic sql.NullString targetTokenName sql.NullString + targetUserID sql.NullInt64 approvedByUserID sql.NullInt64 approvedByEmail sql.NullString approvedByName sql.NullString @@ -90,7 +92,7 @@ func scanDeployApprovalRequest(scanner rowScanner) (*DeployApprovalRequest, erro &dr.ID, &dr.PublicID, &dr.Message, &dr.Status, &dr.CreatedAt, &dr.UpdatedAt, &n.createdByUserID, &n.createdByEmail, &n.createdByName, &n.createdByTokenID, &n.createdTokenPublic, &n.createdTokenName, - &dr.TargetAPITokenID, &n.targetTokenPublic, &n.targetTokenName, + &dr.TargetAPITokenID, &n.targetTokenPublic, &n.targetTokenName, &n.targetUserID, &n.approvedByUserID, &n.approvedByEmail, &n.approvedByName, &n.approvedAt, &n.approvalExpiresAt, &n.rejectedByUserID, &n.rejectedByEmail, &n.rejectedByName, &n.rejectedAt, @@ -140,6 +142,9 @@ func applyCreatorFields(dr *DeployApprovalRequest, n *deployApprovalNullables) { if n.targetTokenName.Valid { dr.TargetAPITokenName = n.targetTokenName.String } + if n.targetUserID.Valid { + dr.TargetUserID = &n.targetUserID.Int64 + } } func applyApprovalFields(dr *DeployApprovalRequest, n *deployApprovalNullables) { diff --git a/internal/gateway/db/mfa.go b/internal/gateway/db/mfa.go index 575c8f9d..1628dc5c 100644 --- a/internal/gateway/db/mfa.go +++ b/internal/gateway/db/mfa.go @@ -66,9 +66,10 @@ func (d *Database) SetUserMFAEnrolled(userID int64, enrolled bool) error { return nil } -// ResetUserMFA completely removes all MFA configuration for a user. -// This includes methods, backup codes, trusted devices, and active sessions. -func (d *Database) ResetUserMFA(userID int64) error { +// ResetUserMFA removes a user's MFA for an admin (resetBy): their methods, backup codes and trusted devices, and +// it revokes their sessions, all in one transaction. Their API tokens are kept. They enroll a new method when they +// next sign in. +func (d *Database) ResetUserMFA(userID, resetBy int64) error { tx, err := d.db.Begin() if err != nil { return fmt.Errorf("failed to begin MFA reset: %w", err) @@ -84,7 +85,7 @@ func (d *Database) ResetUserMFA(userID int64) error { revokeDevicesQuery := ` UPDATE trusted_devices - SET revoked_at = NOW(), revoked_reason = 'reset', updated_at = NOW() + SET revoked_at = NOW(), revoked_reason = 'admin_mfa_reset', updated_at = NOW() WHERE user_id = ? AND revoked_at IS NULL ` if _, err := d.execTx(tx, revokeDevicesQuery, userID); err != nil { @@ -93,14 +94,15 @@ func (d *Database) ResetUserMFA(userID int64) error { revokeSessionsQuery := ` UPDATE user_sessions - SET revoked_at = NOW(), updated_at = NOW() + SET revoked_at = NOW(), revoked_by_user_id = ?, updated_at = NOW() WHERE user_id = ? AND revoked_at IS NULL ` - if _, err := d.execTx(tx, revokeSessionsQuery, userID); err != nil { + if _, err := d.execTx(tx, revokeSessionsQuery, resetBy, userID); err != nil { return fmt.Errorf("failed to revoke active sessions: %w", err) } - if _, err := d.execTx(tx, "UPDATE users SET mfa_enrolled = FALSE WHERE id = ?", userID); err != nil { + clearEnrolled := "UPDATE users SET mfa_enrolled = FALSE, updated_at = NOW() WHERE id = ?" + if _, err := d.execTx(tx, clearEnrolled, userID); err != nil { return fmt.Errorf("failed to update user MFA flag: %w", err) } diff --git a/internal/gateway/handlers/admin_users_lock.go b/internal/gateway/handlers/admin_users_lock.go index 01b6ac3b..f6c41bc3 100644 --- a/internal/gateway/handlers/admin_users_lock.go +++ b/internal/gateway/handlers/admin_users_lock.go @@ -64,7 +64,7 @@ func (h *AdminHandler) LockUser(c *gin.Context) { return } - user, adminUser, ok := h.validateLockUnlockUsers(c, userEmail, start, audit.ActionVerbLock) + user, adminUser, ok := h.loadUserAndAdmin(c, userEmail, start, audit.ActionVerbLock) if !ok { return } @@ -134,7 +134,7 @@ func (h *AdminHandler) UnlockUser(c *gin.Context) { return } - user, adminUser, ok := h.validateLockUnlockUsers(c, userEmail, start, audit.ActionVerbUnlock) + user, adminUser, ok := h.loadUserAndAdmin(c, userEmail, start, audit.ActionVerbUnlock) if !ok { return } @@ -169,7 +169,7 @@ func (h *AdminHandler) UnlockUser(c *gin.Context) { ) } -func (h *AdminHandler) validateLockUnlockUsers( +func (h *AdminHandler) loadUserAndAdmin( c *gin.Context, userEmail string, start time.Time, diff --git a/internal/gateway/handlers/admin_users_mfa_reset.go b/internal/gateway/handlers/admin_users_mfa_reset.go new file mode 100644 index 00000000..7a776422 --- /dev/null +++ b/internal/gateway/handlers/admin_users_mfa_reset.go @@ -0,0 +1,81 @@ +package handlers + +import ( + "net/http" + "strings" + "time" + + "github.com/gin-gonic/gin" + + "github.com/DocSpring/rack-gateway/internal/gateway/audit" + gtwlog "github.com/DocSpring/rack-gateway/internal/gateway/logging" + "github.com/DocSpring/rack-gateway/internal/gateway/rbac" +) + +// ResetUserMFA godoc +// @Summary Reset a user's MFA +// @Description Removes a user's MFA methods, backup codes and trusted devices and signs them out, so they can enroll +// @Description a new method at their next sign-in. Their API tokens are kept. An admin can't reset their own MFA. +// @Tags admin +// @Produce json +// @Param email path string true "User email" +// @Success 200 {object} StatusResponse +// @Failure 400 {object} ErrorResponse +// @Failure 404 {object} ErrorResponse +// @Failure 500 {object} ErrorResponse +// @Security SessionCookie +// @Security CSRFToken +// @Router /users/{email}/mfa/reset [post] +func (h *AdminHandler) ResetUserMFA(c *gin.Context) { + start := time.Now() + action := audit.BuildAction(rbac.ResourceUser.String(), audit.ActionVerbMFAReset) + userEmail := strings.TrimSpace(c.Param("email")) + if userEmail == "" { + h.respondAuditError(c, http.StatusBadRequest, action, userEmail, "email is required", start, nil) + return + } + + user, adminUser, ok := h.loadUserAndAdmin(c, userEmail, start, audit.ActionVerbMFAReset) + if !ok { + return + } + if user.ID == adminUser.ID { + h.respondAuditError(c, http.StatusBadRequest, action, userEmail, + "you can't reset your own MFA; manage your methods on the Account Security page", start, nil) + return + } + + if err := h.database.ResetUserMFA(user.ID, adminUser.ID); err != nil { + gtwlog.Errorf("users: MFA reset for %s failed: %v", user.Email, err) + h.respondAuditError(c, http.StatusInternalServerError, action, userEmail, "failed to reset MFA", start, nil) + return + } + + h.sendMFAResetEmail(user.Email) + + h.respondAuditSuccess( + c, + http.StatusOK, + StatusResponse{Status: "reset"}, + action, + userEmail, + start, + map[string]interface{}{"reset_by": adminUser.Email, "target_user": user.Email}, + ) +} + +func (h *AdminHandler) sendMFAResetEmail(userEmail string) { + if h.emailSender == nil { + return + } + subject := "Multi-factor authentication reset" + textBody := "An administrator reset your multi-factor authentication. Your MFA methods, backup codes and " + + "trusted devices were removed, and you were signed out.\n\n" + + "Sign in again to set up a new method. If you didn't ask for this, contact your administrator." + htmlBody := "

An administrator reset your multi-factor authentication. Your MFA methods, backup codes and " + + "trusted devices were removed, and you were signed out.

" + + "

Sign in again to set up a new method. If you didn't ask for this, contact your administrator.

" + if err := h.emailSender.Send(userEmail, subject, textBody, htmlBody); err != nil { + gtwlog.Errorf("users: MFA reset email to %s failed: %v", userEmail, err) + } +} diff --git a/internal/gateway/handlers/admin_users_mfa_reset_test.go b/internal/gateway/handlers/admin_users_mfa_reset_test.go new file mode 100644 index 00000000..06c25852 --- /dev/null +++ b/internal/gateway/handlers/admin_users_mfa_reset_test.go @@ -0,0 +1,118 @@ +package handlers + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/require" + + "github.com/DocSpring/rack-gateway/internal/gateway/audit" + "github.com/DocSpring/rack-gateway/internal/gateway/auth" + "github.com/DocSpring/rack-gateway/internal/gateway/config" + "github.com/DocSpring/rack-gateway/internal/gateway/db" + "github.com/DocSpring/rack-gateway/internal/gateway/testutil/dbtest" +) + +type sentEmail struct{ to, subject string } + +type recordingEmailSender struct{ sent []sentEmail } + +func (r *recordingEmailSender) Send(to, subject, _, _ string) error { + r.sent = append(r.sent, sentEmail{to: to, subject: subject}) + return nil +} + +func (r *recordingEmailSender) SendMany(to []string, subject, textBody, htmlBody string) error { + for _, recipient := range to { + _ = r.Send(recipient, subject, textBody, htmlBody) + } + return nil +} + +func resetUserMFA(t *testing.T, h *AdminHandler, admin *db.User, email string) *httptest.ResponseRecorder { + t.Helper() + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + req := httptest.NewRequest(http.MethodPost, "/api/v1/users/"+email+"/mfa/reset", nil) + c.Request = req.WithContext(context.WithValue(req.Context(), auth.UserContextKey, &auth.User{ + Email: admin.Email, Name: admin.Name, Roles: admin.Roles, DBUser: admin, + })) + c.Params = gin.Params{{Key: "email", Value: email}} + c.Set("user_email", admin.Email) + h.ResetUserMFA(c) + return w +} + +// An admin's Reset MFA removes everything the user signs in with but keeps their API tokens. +func TestResetUserMFA(t *testing.T) { + database := dbtest.NewDatabase(t) + admin, err := database.CreateUser("admin@example.com", "Admin", []string{"admin"}) + require.NoError(t, err) + user, err := database.CreateUser("dev@example.com", "Dev", []string{"deployer"}) + require.NoError(t, err) + + method, err := database.CreateMFAMethod(user.ID, "totp", "Phone", "JBSWY3DPEHPK3PXP", nil, nil, nil, nil) + require.NoError(t, err) + require.NoError(t, database.ConfirmMFAMethod(method.ID, time.Now())) + require.NoError(t, database.SetUserMFAEnrolled(user.ID, true)) + require.NoError(t, database.ReplaceBackupCodes(user.ID, []string{strings.Repeat("1", 64)})) + deviceID := "3f2b9c1e-8d4a-4b6e-9f1a-2c7d5e8a1b0f" + _, err = database.CreateTrustedDevice(user.ID, deviceID, strings.Repeat("d", 64), time.Now().Add(time.Hour), + "127.0.0.1", "ua", nil) + require.NoError(t, err) + _, err = database.CreateUserSession(user.ID, strings.Repeat("e", 64), time.Now().Add(time.Hour), "web", + deviceID, "Laptop", "127.0.0.1", "ua", nil, nil) + require.NoError(t, err) + _, err = database.CreateAPIToken(strings.Repeat("f", 64), "dev-ci", user.ID, []string{"deployer"}, nil, nil) + require.NoError(t, err) + + emails := &recordingEmailSender{} + h := &AdminHandler{ + database: database, + auditLogger: audit.NewLogger(database), + config: &config.Config{}, + emailSender: emails, + } + + w := resetUserMFA(t, h, admin, admin.Email) + require.Equal(t, http.StatusBadRequest, w.Code) + require.Contains(t, w.Body.String(), "you can't reset your own MFA") + + w = resetUserMFA(t, h, admin, user.Email) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + + methods, err := database.ListAllMFAMethods(user.ID) + require.NoError(t, err) + require.Empty(t, methods) + codes, err := database.ListBackupCodes(user.ID) + require.NoError(t, err) + require.Empty(t, codes) + devices, err := database.ListTrustedDevices(user.ID) + require.NoError(t, err) + for _, device := range devices { + require.NotNil(t, device.RevokedAt) + require.Equal(t, "admin_mfa_reset", device.RevokedReason) + } + sessions, err := database.ListActiveSessionsByUser(user.ID) + require.NoError(t, err) + require.Empty(t, sessions) + reloaded, err := database.GetUserByID(user.ID) + require.NoError(t, err) + require.False(t, reloaded.MFAEnrolled) + tokens, err := database.ListAPITokensByUser(user.ID) + require.NoError(t, err) + require.Len(t, tokens, 1) + + require.Equal(t, []sentEmail{{to: user.Email, subject: "Multi-factor authentication reset"}}, emails.sent) + logs, err := database.GetAuditLogs(admin.Email, time.Time{}, 10) + require.NoError(t, err) + require.NotEmpty(t, logs) + require.Equal(t, "user.mfa_reset", logs[0].Action) + require.Equal(t, audit.StatusSuccess, logs[0].Status) +} diff --git a/internal/gateway/handlers/deploy_approval_admin.go b/internal/gateway/handlers/deploy_approval_admin.go index da06ca84..ef04c6ac 100644 --- a/internal/gateway/handlers/deploy_approval_admin.go +++ b/internal/gateway/handlers/deploy_approval_admin.go @@ -125,21 +125,21 @@ func (h *AdminHandler) getDeployApprovalExpiry() time.Time { // @Param id path string true "Deploy approval request public ID" // @Param body body UpdateDeployApprovalRequestStatusRequest false "Approval notes" // @Success 200 {object} DeployApprovalRequestResponse +// @Failure 403 {object} ErrorResponse // @Failure 404 {object} ErrorResponse // @Failure 500 {object} ErrorResponse // @Security SessionCookie // @Router /deploy-approval-requests/{id}/approve [post] func (h *AdminHandler) ApproveDeployApprovalRequest(c *gin.Context) { - input, ok := h.parseDeployApprovalStatusUpdateRequest(c) + input, ok := h.parseApprovalWindowRequest(c) if !ok { return } - expiresAt := h.getDeployApprovalExpiry() record, err := h.database.ApproveDeployApprovalRequestByPublicID( input.publicID, input.approver.ID, - expiresAt, + input.expiresAt, input.notes, ) if err != nil { @@ -152,13 +152,7 @@ func (h *AdminHandler) ApproveDeployApprovalRequest(c *gin.Context) { } h.triggerCircleCIApprovalIfEnabled(c, record) - h.finishApprovalAction( - c, - record, - input, - rbac.ActionApprove.String(), - map[string]string{"expires_at": expiresAt.UTC().Format(time.RFC3339)}, - ) + h.finishApprovalAction(c, record, input.deployApprovalStatusInput, rbac.ActionApprove.String(), input.auditDetails) } // RejectDeployApprovalRequest godoc @@ -208,19 +202,19 @@ func (h *AdminHandler) RejectDeployApprovalRequest(c *gin.Context) { // @Param id path string true "Deploy approval request public ID" // @Param body body UpdateDeployApprovalRequestStatusRequest false "Extension notes" // @Success 200 {object} DeployApprovalRequestResponse +// @Failure 403 {object} ErrorResponse // @Failure 404 {object} ErrorResponse // @Failure 409 {object} ErrorResponse // @Failure 500 {object} ErrorResponse // @Security SessionCookie // @Router /deploy-approval-requests/{id}/extend [post] func (h *AdminHandler) ExtendDeployApprovalRequest(c *gin.Context) { - input, ok := h.parseDeployApprovalStatusUpdateRequest(c) + input, ok := h.parseApprovalWindowRequest(c) if !ok { return } - expiresAt := h.getDeployApprovalExpiry() - record, err := h.database.ExtendDeployApprovalRequestExpiry(input.publicID, expiresAt) + record, err := h.database.ExtendDeployApprovalRequestExpiry(input.publicID, input.expiresAt) if err != nil { if errors.Is(err, db.ErrDeployApprovalRequestNotFound) { c.JSON(http.StatusNotFound, gin.H{"error": "deploy approval request not found or not approved"}) @@ -236,13 +230,7 @@ func (h *AdminHandler) ExtendDeployApprovalRequest(c *gin.Context) { return } - h.finishApprovalAction( - c, - record, - input, - "extend", - map[string]string{"expires_at": expiresAt.UTC().Format(time.RFC3339)}, - ) + h.finishApprovalAction(c, record, input.deployApprovalStatusInput, "extend", input.auditDetails) } func (h *AdminHandler) triggerCircleCIApprovalIfEnabled(c *gin.Context, record *db.DeployApprovalRequest) { diff --git a/internal/gateway/handlers/deploy_approval_approver.go b/internal/gateway/handlers/deploy_approval_approver.go new file mode 100644 index 00000000..ea2b030e --- /dev/null +++ b/internal/gateway/handlers/deploy_approval_approver.go @@ -0,0 +1,82 @@ +package handlers + +import ( + "net/http" + "time" + + "github.com/gin-gonic/gin" + + "github.com/DocSpring/rack-gateway/internal/gateway/db" + gtwlog "github.com/DocSpring/rack-gateway/internal/gateway/logging" + "github.com/DocSpring/rack-gateway/internal/gateway/settings" +) + +// approvalWindowRequest is an approve or extend request: either opens an approval window, which runs to expiresAt. +type approvalWindowRequest struct { + *deployApprovalStatusInput + expiresAt time.Time + auditDetails map[string]string +} + +// parseApprovalWindowRequest parses an approve or extend request and checks its approver may open the window. +func (h *AdminHandler) parseApprovalWindowRequest(c *gin.Context) (*approvalWindowRequest, bool) { + input, ok := h.parseDeployApprovalStatusUpdateRequest(c) + if !ok { + return nil, false + } + differentApproverChecked, ok := h.requireDifferentApprover(c, input) + if !ok { + return nil, false + } + expiresAt := h.getDeployApprovalExpiry() + details := map[string]string{"expires_at": expiresAt.UTC().Format(time.RFC3339)} + if differentApproverChecked { + details[differentApproverAuditKey] = "true" + } + return &approvalWindowRequest{deployApprovalStatusInput: input, expiresAt: expiresAt, auditDetails: details}, true +} + +// differentApproverAuditKey marks an approval or extension that deploy_approval_require_different_approver checked. +const differentApproverAuditKey = "different_approver_required" + +// requireDifferentApprover refuses an approval or extension by the person who asked for the deploy, when +// deploy_approval_require_different_approver is on: the user who created the request, or the owner of the API token +// it's for. A setting it can't read refuses the request. checked says the setting applied, for the audit entry. +func (h *AdminHandler) requireDifferentApprover(c *gin.Context, input *deployApprovalStatusInput) (checked, ok bool) { + if h.settingsService == nil { + return false, true + } + required, err := h.settingsService.GetDeployApprovalRequireDifferentApprover() + if err != nil { + gtwlog.Errorf("deploy approvals: failed to read %s: %v", + settings.KeyDeployApprovalRequireDifferentApprover, err) + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to read deploy approval settings"}) + return false, false + } + if !required { + return false, true + } + record, ok := loadDeployApprovalRequest(c, h.database, input.publicID) + if !ok { + return false, false + } + if reason := ownDeployApprovalReason(record, input.approver.ID); reason != "" { + c.JSON(http.StatusForbidden, gin.H{ + "error": "this gateway requires a different person to approve a deploy: " + reason, + }) + return false, false + } + return true, true +} + +// ownDeployApprovalReason says why a user counts as the requester of a deploy approval request, or "" if they don't. +func ownDeployApprovalReason(record *db.DeployApprovalRequest, userID int64) string { + switch { + case sameID(record.CreatedByUserID, userID): + return "you created this request" + case sameID(record.TargetUserID, userID): + return "you own the API token this request is for" + default: + return "" + } +} diff --git a/internal/gateway/handlers/deploy_approval_approver_test.go b/internal/gateway/handlers/deploy_approval_approver_test.go new file mode 100644 index 00000000..79c141d5 --- /dev/null +++ b/internal/gateway/handlers/deploy_approval_approver_test.go @@ -0,0 +1,136 @@ +package handlers + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/require" + + "github.com/DocSpring/rack-gateway/internal/gateway/audit" + "github.com/DocSpring/rack-gateway/internal/gateway/auth" + "github.com/DocSpring/rack-gateway/internal/gateway/config" + "github.com/DocSpring/rack-gateway/internal/gateway/db" + "github.com/DocSpring/rack-gateway/internal/gateway/settings" + "github.com/DocSpring/rack-gateway/internal/gateway/testutil/dbtest" +) + +type differentApproverFixture struct { + database *db.Database + handler *AdminHandler + alice *db.User // an admin who owns token + bob *db.User // another admin + token *db.APIToken + commits []string +} + +func newDifferentApproverFixture(t *testing.T, required bool) *differentApproverFixture { + t.Helper() + gin.SetMode(gin.TestMode) + database := dbtest.NewDatabase(t) + alice, err := database.CreateUser("alice@example.com", "Alice", []string{"admin"}) + require.NoError(t, err) + bob, err := database.CreateUser("bob@example.com", "Bob", []string{"admin"}) + require.NoError(t, err) + token, err := database.CreateAPIToken(strings.Repeat("c", 64), "alice-ci", alice.ID, []string{"deployer"}, nil, nil) + require.NoError(t, err) + require.NoError(t, database.UpsertSetting(nil, settings.KeyDeployApprovalRequireDifferentApprover, required, nil)) + + return &differentApproverFixture{ + database: database, + alice: alice, + bob: bob, + token: token, + handler: &AdminHandler{ + rbac: newAllowAllRBAC(alice, bob), + database: database, + auditLogger: audit.NewLogger(database), + config: &config.Config{}, + settingsService: settings.NewService(database), + }, + } +} + +// request creates a pending request for alice's token, made by creator. +func (f *differentApproverFixture) request(t *testing.T, creator *db.User) *db.DeployApprovalRequest { + t.Helper() + commit := strings.Repeat(fmt.Sprintf("%02x", len(f.commits)), 20) + f.commits = append(f.commits, commit) + req, err := f.database.CreateDeployApprovalRequest( + "Deploy", "myapp", commit, "main", "", []byte("{}"), creator.ID, nil, f.token.ID, + ) + require.NoError(t, err) + return req +} + +func callDeployApprovalAction( + t *testing.T, + handle gin.HandlerFunc, + approver *db.User, + req *db.DeployApprovalRequest, +) (int, string) { + t.Helper() + httpReq := httptest.NewRequest(http.MethodPost, "/deploy-approval-requests/"+req.PublicID+"/approve", nil) + httpReq = httpReq.WithContext(context.WithValue(httpReq.Context(), auth.UserContextKey, &auth.User{ + Email: approver.Email, Name: approver.Name, Roles: approver.Roles, + })) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httpReq + c.Params = gin.Params{{Key: "id", Value: req.PublicID}} + c.Set("user_email", approver.Email) + handle(c) + var body struct { + Error string `json:"error"` + } + _ = json.Unmarshal(w.Body.Bytes(), &body) + return w.Code, body.Error +} + +func TestApproveOwnDeployRequestAllowedByDefault(t *testing.T) { + f := newDifferentApproverFixture(t, false) + status, msg := callDeployApprovalAction(t, f.handler.ApproveDeployApprovalRequest, f.alice, f.request(t, f.alice)) + require.Equal(t, http.StatusOK, status, msg) +} + +func TestDifferentApproverRequired(t *testing.T) { + f := newDifferentApproverFixture(t, true) + + status, msg := callDeployApprovalAction(t, f.handler.ApproveDeployApprovalRequest, f.alice, f.request(t, f.alice)) + require.Equal(t, http.StatusForbidden, status) + require.Equal(t, "this gateway requires a different person to approve a deploy: you created this request", msg) + + // Bob asks for a deploy with alice's CI token: alice can't approve it either. + status, msg = callDeployApprovalAction(t, f.handler.ApproveDeployApprovalRequest, f.alice, f.request(t, f.bob)) + require.Equal(t, http.StatusForbidden, status) + require.Contains(t, msg, "you own the API token this request is for") + + req := f.request(t, f.alice) + status, msg = callDeployApprovalAction(t, f.handler.ApproveDeployApprovalRequest, f.bob, req) + require.Equal(t, http.StatusOK, status, msg) + logs, err := f.database.GetAuditLogs(f.bob.Email, time.Time{}, 10) + require.NoError(t, err) + require.NotEmpty(t, logs) + require.Equal(t, "deploy_approval_request.approve", logs[0].Action) + require.Contains(t, logs[0].Details, `"different_approver_required":"true"`) +} + +func TestDifferentApproverRequiredToExtend(t *testing.T) { + f := newDifferentApproverFixture(t, true) + req := f.request(t, f.alice) + _, err := f.database.ApproveDeployApprovalRequest(req.ID, f.bob.ID, time.Now().Add(-time.Minute), "") + require.NoError(t, err) + + status, msg := callDeployApprovalAction(t, f.handler.ExtendDeployApprovalRequest, f.alice, req) + require.Equal(t, http.StatusForbidden, status) + require.Contains(t, msg, "you created this request") + + status, msg = callDeployApprovalAction(t, f.handler.ExtendDeployApprovalRequest, f.bob, req) + require.Equal(t, http.StatusOK, status, msg) +} diff --git a/internal/gateway/openapi/generated/swagger.json b/internal/gateway/openapi/generated/swagger.json index d4c0bbb9..f860ca33 100644 --- a/internal/gateway/openapi/generated/swagger.json +++ b/internal/gateway/openapi/generated/swagger.json @@ -2828,6 +2828,12 @@ "$ref": "#/definitions/handlers.DeployApprovalRequestResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/handlers.ErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -2937,6 +2943,12 @@ "$ref": "#/definitions/handlers.DeployApprovalRequestResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/handlers.ErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -4045,6 +4057,61 @@ } } }, + "/users/{email}/mfa/reset": { + "post": { + "security": [ + { + "SessionCookie": [] + }, + { + "CSRFToken": [] + } + ], + "description": "Removes a user's MFA methods, backup codes and trusted devices and signs them out, so they can enroll\na new method at their next sign-in. Their API tokens are kept. An admin can't reset their own MFA.", + "produces": [ + "application/json" + ], + "tags": [ + "admin" + ], + "summary": "Reset a user's MFA", + "parameters": [ + { + "type": "string", + "description": "User email", + "name": "email", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/handlers.StatusResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/handlers.ErrorResponse" + } + }, + "404": { + "description": "Not Found", + "schema": { + "$ref": "#/definitions/handlers.ErrorResponse" + } + }, + "500": { + "description": "Internal Server Error", + "schema": { + "$ref": "#/definitions/handlers.ErrorResponse" + } + } + } + } + }, "/users/{email}/name": { "put": { "security": [ diff --git a/internal/gateway/rbac/audit_names.go b/internal/gateway/rbac/audit_names.go index 1f79e3bb..b6dda94c 100644 --- a/internal/gateway/rbac/audit_names.go +++ b/internal/gateway/rbac/audit_names.go @@ -25,6 +25,7 @@ type revealRoute struct { var auditActions = map[string]string{ "POST /api/v1/users/:email/lock": "user.lock", "POST /api/v1/users/:email/unlock": "user.unlock", + "POST /api/v1/users/:email/mfa/reset": "user.mfa_reset", "POST /api/v1/users/:email/sessions/:sessionID/revoke": "user.sessions.revoke", "POST /api/v1/users/:email/sessions/revoke_all": "user.sessions.revoke_all", "POST /api/v1/deploy-approval-requests/:id/reject": "deploy_approval_request.reject", diff --git a/internal/gateway/rbac/gateway_routes.go b/internal/gateway/rbac/gateway_routes.go index 54103f23..d435d3f5 100644 --- a/internal/gateway/rbac/gateway_routes.go +++ b/internal/gateway/rbac/gateway_routes.go @@ -161,6 +161,7 @@ var httpRouteSpecs = []RouteSpec{ newHTTPRoute("PUT", "/api/v1/users/:email/name", Gateway(ResourceUser, ActionUpdateName)), newHTTPRoute("POST", "/api/v1/users/:email/lock", Gateway(ResourceUser, ActionUpdate)), newHTTPRoute("POST", "/api/v1/users/:email/unlock", Gateway(ResourceUser, ActionUpdate)), + newHTTPRoute("POST", "/api/v1/users/:email/mfa/reset", Gateway(ResourceUser, ActionUpdate)), // Every user can see their own profile, sessions and audit trail, and sign out their own sessions. // Other users' records need the listed permission. Revoking keeps user:update's MFA level. newOwnUserRoute("GET", "/api/v1/users/:email", Gateway(ResourceUser, ActionRead)), diff --git a/internal/gateway/routes/route_registration.go b/internal/gateway/routes/route_registration.go index c8149f2b..12e04383 100644 --- a/internal/gateway/routes/route_registration.go +++ b/internal/gateway/routes/route_registration.go @@ -167,6 +167,7 @@ func registerUserManagementRoutes(authenticated *gin.RouterGroup, cfg *Config, h users.GET("/:email/audit-logs", h.admin.ListUserAuditLogs) users.POST("/:email/lock", h.admin.LockUser) users.POST("/:email/unlock", h.admin.UnlockUser) + users.POST("/:email/mfa/reset", h.admin.ResetUserMFA) auditLogs := authenticated.Group("/audit-logs") if cfg.SessionManager != nil { diff --git a/internal/gateway/settings/defaults.go b/internal/gateway/settings/defaults.go index 5ab51ba5..61097796 100644 --- a/internal/gateway/settings/defaults.go +++ b/internal/gateway/settings/defaults.go @@ -15,6 +15,7 @@ const ( GlobalSettingAllowDestructiveActions GlobalSettingKey = iota GlobalSettingDefaultCIProvider GlobalSettingDefaultVCSProvider + GlobalSettingDeployApprovalRequireDifferentApprover GlobalSettingDeployApprovalsEnabled GlobalSettingDeployApprovalWindowMinutes GlobalSettingMFARequireAllUsers @@ -26,20 +27,22 @@ const ( // Global setting key strings. GlobalSettingStepUpWindowMinutes and AppSettingSecretEnvVars have no Key constant: // gosec reads those names as hardcoded credentials. Use their String(). const ( - KeyAllowDestructiveActions = "allow_destructive_actions" - KeyDefaultCIProvider = "default_ci_provider" - KeyDefaultVCSProvider = "default_vcs_provider" - KeyDeployApprovalsEnabled = "deploy_approvals_enabled" - KeyDeployApprovalWindowMinutes = "deploy_approval_window_minutes" - KeyMFARequireAllUsers = "mfa_require_all_users" - KeySessionTimeoutMinutes = "session_timeout_minutes" - KeyTrustedDeviceTTLDays = "mfa_trusted_device_ttl_days" + KeyAllowDestructiveActions = "allow_destructive_actions" + KeyDefaultCIProvider = "default_ci_provider" + KeyDefaultVCSProvider = "default_vcs_provider" + KeyDeployApprovalRequireDifferentApprover = "deploy_approval_require_different_approver" + KeyDeployApprovalsEnabled = "deploy_approvals_enabled" + KeyDeployApprovalWindowMinutes = "deploy_approval_window_minutes" + KeyMFARequireAllUsers = "mfa_require_all_users" + KeySessionTimeoutMinutes = "session_timeout_minutes" + KeyTrustedDeviceTTLDays = "mfa_trusted_device_ttl_days" ) var globalSettingKeyToString = [...]string{ KeyAllowDestructiveActions, KeyDefaultCIProvider, KeyDefaultVCSProvider, + KeyDeployApprovalRequireDifferentApprover, KeyDeployApprovalsEnabled, KeyDeployApprovalWindowMinutes, KeyMFARequireAllUsers, @@ -176,6 +179,7 @@ var DefaultGlobalSettings = map[string]interface{}{ KeyAllowDestructiveActions: false, KeyDefaultCIProvider: "circleci", KeyDefaultVCSProvider: "github", + KeyDeployApprovalRequireDifferentApprover: false, KeyDeployApprovalsEnabled: true, KeyDeployApprovalWindowMinutes: DefaultDeployApprovalWindowMinutes, KeyMFARequireAllUsers: true, diff --git a/internal/gateway/settings/groups.go b/internal/gateway/settings/groups.go index 7ecde289..4e63ed16 100644 --- a/internal/gateway/settings/groups.go +++ b/internal/gateway/settings/groups.go @@ -30,6 +30,7 @@ var globalSettingGroupKeys = map[GlobalSettingGroup][]GlobalSettingKey{ GlobalSettingGroupDeployApprovals: { GlobalSettingDeployApprovalsEnabled, GlobalSettingDeployApprovalWindowMinutes, + GlobalSettingDeployApprovalRequireDifferentApprover, }, GlobalSettingGroupSessionConfiguration: { GlobalSettingSessionTimeoutMinutes, diff --git a/internal/gateway/settings/service_deploy.go b/internal/gateway/settings/service_deploy.go index f6ae180f..cc873106 100644 --- a/internal/gateway/settings/service_deploy.go +++ b/internal/gateway/settings/service_deploy.go @@ -1,31 +1,32 @@ package settings -// GetAllowDestructiveActions returns whether destructive actions are allowed. -func (s *Service) GetAllowDestructiveActions() (bool, error) { - setting, err := s.GetGlobalSetting(KeyAllowDestructiveActions, false) +// globalBool returns a boolean global setting, or defaultValue when it isn't a boolean. On error it returns +// defaultValue with the error, so the caller decides whether to fail closed. +func (s *Service) globalBool(key string, defaultValue bool) (bool, error) { + setting, err := s.GetGlobalSetting(key, defaultValue) if err != nil { - return false, err + return defaultValue, err } - if val, ok := setting.Value.(bool); ok { return val, nil } + return defaultValue, nil +} - return false, nil +// GetAllowDestructiveActions returns whether destructive actions are allowed. +func (s *Service) GetAllowDestructiveActions() (bool, error) { + return s.globalBool(KeyAllowDestructiveActions, false) } // GetDeployApprovalsEnabled returns whether deploy approvals are enabled. func (s *Service) GetDeployApprovalsEnabled() (bool, error) { - setting, err := s.GetGlobalSetting(KeyDeployApprovalsEnabled, true) - if err != nil { - return true, err - } - - if val, ok := setting.Value.(bool); ok { - return val, nil - } + return s.globalBool(KeyDeployApprovalsEnabled, true) +} - return true, nil +// GetDeployApprovalRequireDifferentApprover returns whether a deploy approval must be approved (and extended) by +// someone other than the user who requested it or owns its API token. +func (s *Service) GetDeployApprovalRequireDifferentApprover() (bool, error) { + return s.globalBool(KeyDeployApprovalRequireDifferentApprover, false) } // GetDeployApprovalWindowMinutes returns the deploy approval window in minutes. diff --git a/web/src/api/generated.ts b/web/src/api/generated.ts index 9f0c9aa5..4ff9189b 100644 --- a/web/src/api/generated.ts +++ b/web/src/api/generated.ts @@ -1432,6 +1432,23 @@ export const getRackGatewayAPI = () => { ); }; + /** + * Removes a user's MFA methods, backup codes and trusted devices and signs them out, so they can enroll + * a new method at their next sign-in. Their API tokens are kept. An admin can't reset their own MFA. + * @summary Reset a user's MFA + */ + const postUsersEmailMfaReset = ( + email: string, + options?: SecondParameter< + typeof createGatewayClient + >, + ) => { + return createGatewayClient( + { url: `/users/${email}/mfa/reset`, method: "POST" }, + options, + ); + }; + /** * Updates only a user's display name. * @summary Update user name @@ -1595,6 +1612,7 @@ export const getRackGatewayAPI = () => { deleteUsersEmail, getUsersEmailAuditLogs, postUsersEmailLock, + postUsersEmailMfaReset, putUsersEmailName, getUsersEmailSessions, postUsersEmailSessionsRevokeAll, @@ -2001,6 +2019,11 @@ export type PostUsersEmailLockResult = NonNullable< ReturnType["postUsersEmailLock"]> > >; +export type PostUsersEmailMfaResetResult = NonNullable< + Awaited< + ReturnType["postUsersEmailMfaReset"]> + > +>; export type PutUsersEmailNameResult = NonNullable< Awaited["putUsersEmailName"]>> >; diff --git a/web/src/api/openapi.json b/web/src/api/openapi.json index dfaf8c94..a15428a2 100644 --- a/web/src/api/openapi.json +++ b/web/src/api/openapi.json @@ -3471,6 +3471,16 @@ } } }, + "403": { + "description": "Forbidden", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/handlers.ErrorResponse" + } + } + } + }, "404": { "description": "Not Found", "content": { @@ -3603,6 +3613,16 @@ } } }, + "403": { + "description": "Forbidden", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/handlers.ErrorResponse" + } + } + } + }, "404": { "description": "Not Found", "content": { @@ -4963,6 +4983,76 @@ } } }, + "/users/{email}/mfa/reset": { + "post": { + "security": [ + { + "SessionCookie": [] + }, + { + "CSRFToken": [] + } + ], + "description": "Removes a user's MFA methods, backup codes and trusted devices and signs them out, so they can enroll\na new method at their next sign-in. Their API tokens are kept. An admin can't reset their own MFA.", + "tags": [ + "admin" + ], + "summary": "Reset a user's MFA", + "parameters": [ + { + "description": "User email", + "name": "email", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/handlers.StatusResponse" + } + } + } + }, + "400": { + "description": "Bad Request", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/handlers.ErrorResponse" + } + } + } + }, + "404": { + "description": "Not Found", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/handlers.ErrorResponse" + } + } + } + }, + "500": { + "description": "Internal Server Error", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/handlers.ErrorResponse" + } + } + } + } + } + } + }, "/users/{email}/name": { "put": { "security": [ diff --git a/web/src/api/types.generated.ts b/web/src/api/types.generated.ts index 34fa9717..c34c68c1 100644 --- a/web/src/api/types.generated.ts +++ b/web/src/api/types.generated.ts @@ -3288,6 +3288,15 @@ export interface paths { "application/json": components["schemas"]["handlers.DeployApprovalRequestResponse"]; }; }; + /** @description Forbidden */ + 403: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["handlers.ErrorResponse"]; + }; + }; /** @description Not Found */ 404: { headers: { @@ -3416,6 +3425,15 @@ export interface paths { "application/json": components["schemas"]["handlers.DeployApprovalRequestResponse"]; }; }; + /** @description Forbidden */ + 403: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["handlers.ErrorResponse"]; + }; + }; /** @description Not Found */ 404: { headers: { @@ -4726,6 +4744,76 @@ export interface paths { patch?: never; trace?: never; }; + "/users/{email}/mfa/reset": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** + * Reset a user's MFA + * @description Removes a user's MFA methods, backup codes and trusted devices and signs them out, so they can enroll + * a new method at their next sign-in. Their API tokens are kept. An admin can't reset their own MFA. + */ + post: { + parameters: { + query?: never; + header?: never; + path: { + /** @description User email */ + email: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["handlers.StatusResponse"]; + }; + }; + /** @description Bad Request */ + 400: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["handlers.ErrorResponse"]; + }; + }; + /** @description Not Found */ + 404: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["handlers.ErrorResponse"]; + }; + }; + /** @description Internal Server Error */ + 500: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["handlers.ErrorResponse"]; + }; + }; + }; + }; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/users/{email}/name": { parameters: { query?: never; diff --git a/web/src/components/user-mfa-reset-dialog.test.tsx b/web/src/components/user-mfa-reset-dialog.test.tsx new file mode 100644 index 00000000..ceaf24d8 --- /dev/null +++ b/web/src/components/user-mfa-reset-dialog.test.tsx @@ -0,0 +1,57 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { api } from '@/lib/api' +import { UserMFAResetDialog } from './user-mfa-reset-dialog' + +vi.mock('@/lib/api', () => ({ + api: { + resetUserMFA: vi.fn(), + }, +})) + +const RESET_BUTTON = /^reset mfa$/i + +function renderDialog(onOpenChange = vi.fn()) { + const queryClient = new QueryClient({ defaultOptions: { mutations: { retry: false } } }) + render( + + + + ) + return onOpenChange +} + +describe('UserMFAResetDialog', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('says what a reset removes and keeps', () => { + renderDialog() + expect(screen.getByText(/backup codes and trusted devices/)).toBeInTheDocument() + expect(screen.getByText(/API tokens are kept/)).toBeInTheDocument() + }) + + it('resets the user and closes', async () => { + vi.mocked(api.resetUserMFA).mockResolvedValue({ status: 'reset' }) + const onOpenChange = renderDialog() + + fireEvent.click(screen.getByRole('button', { name: RESET_BUTTON })) + + await waitFor(() => { + expect(api.resetUserMFA).toHaveBeenCalledWith('dev@example.com') + expect(onOpenChange).toHaveBeenCalledWith(false) + }) + }) + + it('stays open when the reset fails', async () => { + vi.mocked(api.resetUserMFA).mockRejectedValue(new Error('failed to reset MFA')) + const onOpenChange = renderDialog() + + fireEvent.click(screen.getByRole('button', { name: RESET_BUTTON })) + + await waitFor(() => expect(api.resetUserMFA).toHaveBeenCalled()) + expect(onOpenChange).not.toHaveBeenCalled() + }) +}) diff --git a/web/src/components/user-mfa-reset-dialog.tsx b/web/src/components/user-mfa-reset-dialog.tsx new file mode 100644 index 00000000..292c238e --- /dev/null +++ b/web/src/components/user-mfa-reset-dialog.tsx @@ -0,0 +1,71 @@ +import { useQueryClient } from '@tanstack/react-query' +import { toast } from '@/components/ui/use-toast' +import { useMutation } from '@/hooks/use-mutation' +import { api } from '@/lib/api' +import { QUERY_KEYS } from '@/lib/query-keys' +import { Button } from './ui/button' +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from './ui/dialog' + +type UserMFAResetDialogProps = { + userEmail: string + open: boolean + onOpenChange: (open: boolean) => void +} + +// Resets a user's MFA, for a user who has lost every method and their backup codes. Their API tokens are kept. +export function UserMFAResetDialog({ userEmail, open, onOpenChange }: UserMFAResetDialogProps) { + const queryClient = useQueryClient() + + const resetMutation = useMutation({ + mutationFn: (email: string) => api.resetUserMFA(email), + onSuccess: () => { + toast.success('MFA reset. The user sets up a new method when they next sign in.') + queryClient.invalidateQueries({ queryKey: QUERY_KEYS.USERS }) + queryClient.invalidateQueries({ queryKey: [...QUERY_KEYS.USER, userEmail] }) + queryClient.invalidateQueries({ queryKey: [...QUERY_KEYS.USER_SESSIONS, userEmail] }) + onOpenChange(false) + }, + }) + + const confirmReset = async () => { + try { + await resetMutation.mutateAsync(userEmail) + } catch { + // The mutation shows the error + } + } + + return ( + + + + Reset MFA + + This removes the MFA methods, backup codes and trusted devices of "{userEmail}" and + signs them out everywhere. Their API tokens are kept. They set up a new method when they + next sign in, and they're emailed about the reset. + + + + + + + + + ) +} diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 30270a7d..aec7d37b 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -173,6 +173,9 @@ export const lockUser = (email: string, reason: string): Promise export const unlockUser = (email: string): Promise => unwrap(gateway.postUsersEmailUnlock(pathSegment(email))) as Promise +export const resetUserMFA = (email: string): Promise => + unwrap(gateway.postUsersEmailMfaReset(pathSegment(email))) as Promise + export const listDeployApprovalRequests = ( params?: GetDeployApprovalRequestsParams ): Promise => unwrap(gateway.getDeployApprovalRequests(params)) @@ -459,6 +462,7 @@ export const api = { revokeAllUserSessions, lockUser, unlockUser, + resetUserMFA, listAuditLogs, listUserAuditLogs, exportAuditLogs, diff --git a/web/src/lib/generated/mfa-requirements.ts b/web/src/lib/generated/mfa-requirements.ts index d0bd871e..99b292db 100644 --- a/web/src/lib/generated/mfa-requirements.ts +++ b/web/src/lib/generated/mfa-requirements.ts @@ -432,6 +432,12 @@ export const HTTP_ROUTE_MFA_REQUIREMENTS: HttpRouteMfaRequirement[] = [ permissions: ["gateway:user:update"], mfaLevel: "always", }, + { + method: "POST", + pattern: "/api/v1/users/:email/mfa/reset", + permissions: ["gateway:user:update"], + mfaLevel: "always", + }, { method: "POST", pattern: "/api/v1/users/:email/sessions/:sessionID/revoke", diff --git a/web/src/pages/settings-page.test.tsx b/web/src/pages/settings-page.test.tsx index 87e0a214..e58131f0 100644 --- a/web/src/pages/settings-page.test.tsx +++ b/web/src/pages/settings-page.test.tsx @@ -277,6 +277,31 @@ describe('SettingsPage', () => { }) }) + it('saves the different approver requirement for deploy approvals', async () => { + vi.mocked(api.get).mockResolvedValue({ + deploy_approval_require_different_approver: { + key: 'deploy_approval_require_different_approver', + value: false, + source: 'default', + env_var: 'RGW_SETTING_DEPLOY_APPROVAL_REQUIRE_DIFFERENT_APPROVER', + } as SettingsSetting, + }) + vi.mocked(api.put).mockResolvedValue({}) + + renderSettingsPage() + + const checkbox = await screen.findByLabelText(/require a different approver/i) + await waitFor(() => expect(checkbox).not.toBeChecked()) + fireEvent.click(checkbox) + fireEvent.click(await screen.findByRole('button', { name: /save/i })) + + await waitFor(() => { + expect(api.put).toHaveBeenCalledWith('/api/v1/settings/deploy-approvals', { + deploy_approval_require_different_approver: true, + }) + }) + }) + it('discards changes when Cancel is clicked', async () => { vi.mocked(api.get).mockResolvedValue({ mfa_require_all_users: { diff --git a/web/src/pages/settings/deploy-approvals-card.tsx b/web/src/pages/settings/deploy-approvals-card.tsx index 585e5efb..28bc8f21 100644 --- a/web/src/pages/settings/deploy-approvals-card.tsx +++ b/web/src/pages/settings/deploy-approvals-card.tsx @@ -23,40 +23,48 @@ type DeployApprovalsCardProps = { disabled: boolean } +type DeployApprovalValues = { + deploy_approvals_enabled: boolean + deploy_approval_window_minutes: number + deploy_approval_require_different_approver: boolean +} + +const DEFAULTS: DeployApprovalValues = { + deploy_approvals_enabled: true, + deploy_approval_window_minutes: 15, + deploy_approval_require_different_approver: false, +} + +const KEYS = Object.keys(DEFAULTS) as (keyof DeployApprovalValues)[] + export function DeployApprovalsCard({ settings, disabled }: DeployApprovalsCardProps) { const qc = useQueryClient() const { handleStepUpError } = useStepUp() - const [enabled, setEnabled] = useState(null) - const [windowMinutes, setWindowMinutes] = useState(null) + const [pending, setPending] = useState>({}) - const currentEnabled = getSettingValue(settings?.deploy_approvals_enabled, true) - const currentWindowMinutes = getSettingValue(settings?.deploy_approval_window_minutes, 15) + const display = (key: K): DeployApprovalValues[K] => + pending[key] ?? getSettingValue(settings?.[key], DEFAULTS[key]) + const change = (key: K, value: DeployApprovalValues[K]) => + setPending((current) => ({ ...current, [key]: value })) + const isLocked = (key: keyof DeployApprovalValues) => disabled || isSetByEnv(settings?.[key]) + const hasChanges = Object.keys(pending).length > 0 + const storedKeys = KEYS.filter((key) => hasStoredValue(settings?.[key])) - const displayEnabled = enabled ?? currentEnabled - const displayWindowMinutes = windowMinutes ?? currentWindowMinutes - const hasChanges = enabled !== null || windowMinutes !== null - - const updateMutation = useMutation({ - mutationFn: async () => { - const updates: Record = {} - if (enabled !== null) { - updates.deploy_approvals_enabled = enabled - } - if (windowMinutes !== null) { - updates.deploy_approval_window_minutes = windowMinutes - } - return await api.put>( - '/api/v1/settings/deploy-approvals', - updates - ) - }, - onSuccess: (updatedSettings) => { + const applyUpdatedSettings = (updatedSettings: Record | undefined) => { + if (updatedSettings) { qc.setQueryData(['globalSettings'], (old: GlobalSettingsResponse | undefined) => ({ ...old, ...updatedSettings, })) - setEnabled(null) - setWindowMinutes(null) + } + setPending({}) + } + + const updateMutation = useMutation({ + mutationFn: async () => + await api.put>('/api/v1/settings/deploy-approvals', pending), + onSuccess: (updatedSettings) => { + applyUpdatedSettings(updatedSettings) toast.success('Deploy approval settings updated') }, onError: (error: unknown) => { @@ -66,29 +74,15 @@ export function DeployApprovalsCard({ settings, disabled }: DeployApprovalsCardP const clearMutation = useMutation({ mutationFn: async () => { - const keys: string[] = [] - if (hasStoredValue(settings?.deploy_approvals_enabled)) { - keys.push('deploy_approvals_enabled') - } - if (hasStoredValue(settings?.deploy_approval_window_minutes)) { - keys.push('deploy_approval_window_minutes') - } - if (keys.length > 0) { - const params = keys.map((key) => `key=${key}`).join('&') + if (storedKeys.length > 0) { + const params = storedKeys.map((key) => `key=${key}`).join('&') return await api.delete>( `/api/v1/settings/deploy-approvals?${params}` ) } }, onSuccess: (updatedSettings) => { - if (updatedSettings) { - qc.setQueryData(['globalSettings'], (old: GlobalSettingsResponse | undefined) => ({ - ...old, - ...updatedSettings, - })) - } - setEnabled(null) - setWindowMinutes(null) + applyUpdatedSettings(updatedSettings) toast.success('Deploy approval settings cleared') }, onError: (error: unknown) => { @@ -96,18 +90,11 @@ export function DeployApprovalsCard({ settings, disabled }: DeployApprovalsCardP }, }) - const handleCancel = () => { - setEnabled(null) - setWindowMinutes(null) - } - const handleSave = async () => { try { await updateMutation.mutateAsync() } catch (error) { - if (handleStepUpError(error, () => updateMutation.mutateAsync())) { - return - } + handleStepUpError(error, () => updateMutation.mutateAsync()) } } @@ -115,16 +102,10 @@ export function DeployApprovalsCard({ settings, disabled }: DeployApprovalsCardP try { await clearMutation.mutateAsync() } catch (error) { - if (handleStepUpError(error, () => clearMutation.mutateAsync())) { - return - } + handleStepUpError(error, () => clearMutation.mutateAsync()) } } - const hasDbSettings = - hasStoredValue(settings?.deploy_approvals_enabled) || - hasStoredValue(settings?.deploy_approval_window_minutes) - return ( @@ -138,9 +119,9 @@ export function DeployApprovalsCard({ settings, disabled }: DeployApprovalsCardP