From 1cbafc953a08f2673f857123cacf045144b754c5 Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Tue, 13 Oct 2026 15:48:01 +1300 Subject: [PATCH 1/2] fix: audit log types, anchor bucket retention check, dead code Fixes #139, fixes #140, fixes #141. Audit log types (#139): an admin action's entries take the action type of the route's permission scope (gateway or security) and the resource type of the action's prefix, instead of a guess from the action name. The audit page filters by convox/gateway/auth/security and lists the resource types the gateway writes. Anchor bucket retention (#140): the gateway reads the anchor bucket's Object Lock configuration at startup and refuses to start unless it locks objects in COMPLIANCE mode for at least AUDIT_ANCHOR_RETENTION_DAYS. audit verify runs the same check. The anchor writer no longer sets a per-object retention; the bucket default governs. The gateway's IAM role needs s3:GetBucketObjectLockConfiguration on the anchor bucket. Dead code (#141): removed users.suspended from the code (the column is dropped in a later release, #147), the plaintext column-encryption sweep (upgrade through v0.1.10 first), the unused suspicious-activity alert, the dev email outbox, the proxy's destructive-command switch, unused RBAC resources and actions, and Convox command catalog entries with no CLI command. Co-Authored-By: Claude Opus 5.5 --- cmd/gateway/main.go | 8 +- convox.yml | 2 +- .../content/docs/concepts/mfa-security.mdx | 5 +- .../configuration/environment-variables.mdx | 17 +- .../terraform/aws-infrastructure.mdx | 4 +- .../deployment/terraform/s3-worm-storage.mdx | 21 +- docs/src/content/docs/operations/upgrades.mdx | 23 +- .../security/authentication/api-tokens.mdx | 2 +- .../docs/security/authentication/index.mdx | 2 +- .../docs/security/authentication/sessions.mdx | 3 +- .../docs/security/compliance/audit-trail.mdx | 4 +- .../docs/security/rbac/permissions.mdx | 6 - docs/src/content/docs/security/rbac/roles.mdx | 2 +- .../docs/user-guide/web-ui/audit-logs.mdx | 11 +- internal/cli/gateway_test_auth.go | 5 +- internal/cli/mfa_verify.go | 9 - internal/convox/commands.go | 126 ----------- internal/gateway/CLAUDE.md | 4 +- internal/gateway/app/column_upgrade.go | 96 --------- internal/gateway/app/column_upgrade_test.go | 81 ------- internal/gateway/app/setup.go | 11 +- internal/gateway/audit/constants.go | 1 - internal/gateway/auditverify/anchor_checks.go | 5 + internal/gateway/auditverify/anchors_test.go | 22 +- .../gateway/auditverify/bucket_retention.go | 53 +++++ .../auditverify/bucket_retention_test.go | 70 +++++++ internal/gateway/auditverify/verify.go | 21 +- internal/gateway/auth/service.go | 8 +- internal/gateway/auth/session_manager.go | 4 - internal/gateway/colcrypt/colcrypt.go | 5 - internal/gateway/colcrypt/colcrypt_test.go | 2 +- internal/gateway/db/column_encryption.go | 188 +---------------- internal/gateway/db/column_encryption_test.go | 135 ------------ internal/gateway/db/sessions_lookup.go | 4 +- internal/gateway/db/types.go | 3 +- internal/gateway/db/users.go | 17 +- internal/gateway/db/users_scan.go | 5 +- internal/gateway/email/postmark.go | 51 +---- internal/gateway/handlers/admin_audit.go | 20 +- .../handlers/admin_audit_types_test.go | 36 ++++ internal/gateway/handlers/admin_users_list.go | 1 - internal/gateway/handlers/api_handler_env.go | 8 +- internal/gateway/handlers/auth_helpers.go | 4 +- internal/gateway/handlers/auth_mfa_audit.go | 2 +- .../handlers/deploy_approval_helpers.go | 2 +- .../gateway/jobs/audit/anchor_latest_test.go | 2 +- .../gateway/jobs/audit/anchor_window_test.go | 4 +- internal/gateway/jobs/audit/anchor_writer.go | 48 ++--- .../gateway/jobs/audit/anchor_writer_test.go | 75 ++----- internal/gateway/jobs/audit_jobs.go | 46 ++-- internal/gateway/jobs/client.go | 2 - .../gateway/jobs/email/suspicious_activity.go | 197 ------------------ .../jobs/email/suspicious_activity_test.go | 91 -------- .../gateway/jobs/verifyaudit/anchor_cache.go | 9 + .../gateway/jobs/verifyaudit/helpers_test.go | 2 +- internal/gateway/middleware/authorize.go | 4 +- .../gateway/openapi/generated/swagger.json | 5 +- internal/gateway/proxy/audit.go | 4 +- internal/gateway/proxy/env.go | 4 +- internal/gateway/proxy/handler.go | 55 +++-- internal/gateway/proxy/rack_params.go | 2 +- internal/gateway/proxy/websocket.go | 3 +- internal/gateway/rbac/action_string.go | 49 ++--- internal/gateway/rbac/constants.go | 16 +- internal/gateway/rbac/mfa_coverage_test.go | 22 +- internal/gateway/rbac/mfa_requirements.go | 12 +- .../gateway/rbac/mfa_requirements_test.go | 1 - internal/gateway/rbac/rbac.go | 4 +- internal/gateway/rbac/rbac_test.go | 6 +- internal/gateway/rbac/resource_string.go | 51 +++-- internal/gateway/security/notifier.go | 102 +-------- internal/gateway/security/notifier_test.go | 3 +- internal/gateway/testutil/fakes3/fakes3.go | 30 ++- web/src/api/openapi.json | 5 +- web/src/api/schemas/dbAuditLog.ts | 2 +- web/src/api/schemas/dbUser.ts | 1 - web/src/api/types.generated.ts | 3 +- web/src/components/audit-logs/badges.tsx | 6 +- web/src/components/audit-logs/utils.ts | 5 +- web/src/pages/audit-page.test.tsx | 20 +- web/src/pages/audit/utils.ts | 17 +- web/src/pages/rack-page.tsx | 45 ---- web/src/pages/users-page.test.tsx | 4 - web/src/pages/users-page.tsx | 1 - web/src/pages/users/user-utils.ts | 1 - 85 files changed, 548 insertions(+), 1520 deletions(-) delete mode 100644 internal/gateway/app/column_upgrade.go delete mode 100644 internal/gateway/app/column_upgrade_test.go create mode 100644 internal/gateway/auditverify/bucket_retention.go create mode 100644 internal/gateway/auditverify/bucket_retention_test.go create mode 100644 internal/gateway/handlers/admin_audit_types_test.go delete mode 100644 internal/gateway/jobs/email/suspicious_activity.go delete mode 100644 internal/gateway/jobs/email/suspicious_activity_test.go diff --git a/cmd/gateway/main.go b/cmd/gateway/main.go index 8a618627..ea74ecc2 100644 --- a/cmd/gateway/main.go +++ b/cmd/gateway/main.go @@ -223,13 +223,7 @@ func writeAuditAnchor() error { } defer closeDatabase(database) - worker := jobaudit.NewAnchorWriterWorker( - database, - anchorConfig.S3Client, - anchorConfig.Bucket, - anchorConfig.ChainID, - anchorConfig.RetentionDays, - ) + worker := jobaudit.NewAnchorWriterWorker(database, anchorConfig.S3Client, anchorConfig.Bucket, anchorConfig.ChainID) // Write this hour's anchor directly (synchronously), for the hour by the database's clock. It takes the same // lock as the periodic job, and fails rather than waits while another anchor write holds it. diff --git a/convox.yml b/convox.yml index dab7d382..69dc4b32 100644 --- a/convox.yml +++ b/convox.yml @@ -23,7 +23,7 @@ environment: - AUDIT_HMAC_SECRET # Secret key used for HMAC-SHA256 cryptographic chain of audit logs - AUDIT_ANCHOR_BUCKET # S3 bucket name for WORM audit anchors - AUDIT_ANCHOR_CHAIN_ID # Chain identifier (typically rack alias, e.g., staging, eu, us) - - AUDIT_ANCHOR_RETENTION_DAYS # Object Lock retention in days (default: 400) + - AUDIT_ANCHOR_RETENTION_DAYS # Least default COMPLIANCE retention the bucket must have, in days (default: 400) services: gateway: diff --git a/docs/src/content/docs/concepts/mfa-security.mdx b/docs/src/content/docs/concepts/mfa-security.mdx index 08ceb3d9..2512fb11 100644 --- a/docs/src/content/docs/concepts/mfa-security.mdx +++ b/docs/src/content/docs/concepts/mfa-security.mdx @@ -267,10 +267,7 @@ flowchart LR - **TOTP seeds and YubiKey public IDs** are encrypted with AES-256-GCM under `COLUMN_ENCRYPTION_KEY` and bound to their user, so a copy of the database alone doesn't reveal them, and a value copied to another - user's row is rejected. A plaintext seed is never used as it is, but while the upgrade sweep for column - encryption exists (it will be removed in a later release), a plaintext seed written into the database is - encrypted by the next sweep, at startup or 10 minutes after it, and then works. Each value the sweep encrypts - is logged as a warning, and the sweep 10 minutes after startup reports any to Sentry. See + user's row is rejected. A plaintext seed written into the database is never used. See [Column Encryption](/configuration/environment-variables/#column-encryption). - **Backup codes** are stored only as keyed hashes. - **WebAuthn** stores only each credential's public key. diff --git a/docs/src/content/docs/configuration/environment-variables.mdx b/docs/src/content/docs/configuration/environment-variables.mdx index 257bd585..8fb3ab6a 100644 --- a/docs/src/content/docs/configuration/environment-variables.mdx +++ b/docs/src/content/docs/configuration/environment-variables.mdx @@ -55,8 +55,8 @@ against a database, it marks the database as `production`, or `development` in a `COLUMN_ENCRYPTION_KEY` encrypts the secrets the gateway stores in its database: TOTP seeds and YubiKey public IDs (`mfa_methods.secret`) and the Slack bot token. Values are encrypted with AES-256-GCM and bound to where they belong (the user and MFA method type, or the Slack workspace), so a value copied to another -user's row, or to a method of another type, can't be decrypted there. A plaintext value is never used as it is, -but the upgrade sweep encrypts any it finds (see **Existing data is upgraded at startup** below). +user's row, or to a method of another type, can't be decrypted there. A plaintext value is never used: reading +one is an error, as for a value that doesn't decrypt. - **Generate it once** with `openssl rand -base64 32` and store it with your other secrets. Use a different key per environment. It is separate from `APP_SECRET_KEY`, so rotating the session secret @@ -73,14 +73,9 @@ but the upgrade sweep encrypts any it finds (see **Existing data is upgraded at automatic account lock. Slack must be reconnected. The gateway logs (and reports to Sentry, if configured) every value it fails to decrypt when it tries to use it. -- **Existing data is upgraded at startup.** Secrets stored by releases before column encryption (before - v0.1.7: plaintext TOTP seeds, base64 Slack tokens) are encrypted when the server starts, before it serves - requests, and once more 10 minutes later, in case the previous release stored more during the rollout. The - sweep runs on every start and is safe to repeat. It logs a warning naming each row it encrypts (the user - and MFA method type, or the Slack workspace, never the value). It will be removed in a later release; until then it encrypts any - plaintext value it finds, so someone who can write to the database could plant a TOTP seed they know and - have it encrypted. After the upgrade nothing else stores plaintext, so the sweep 10 minutes after startup - encrypting anything is reported to Sentry, if configured, and logged as an error. +- **Upgrading from before v0.1.7** (plaintext TOTP seeds, base64 Slack tokens): deploy a release from v0.1.7 to + v0.1.10 first. Those releases encrypt the existing secrets when they start; later releases don't, and can't + read secrets stored before column encryption. - **Rotation isn't supported yet.** Every encrypted value records which key encrypted it, so rotation can be added later without changing the stored format. @@ -225,7 +220,7 @@ See [Deploy Approvals](/integrations/deploy-approvals/) for common keys. | `AUDIT_HMAC_SECRET` | (required in prod) | HMAC chain secret: at least 32 bytes (`openssl rand -base64 32`). Without it (`DEV_MODE` only) the gateway doesn't verify its audit log. | | `AUDIT_ANCHOR_BUCKET` | | S3 bucket for anchors. Unset, no anchors are written | | `AUDIT_ANCHOR_CHAIN_ID` | | Chain identifier, the anchors' key prefix. Required with `AUDIT_ANCHOR_BUCKET` | -| `AUDIT_ANCHOR_RETENTION_DAYS` | `400` | Not used for retention: anchors are locked by the bucket's default Object Lock rule, so set the retention there. The gateway only shows this value in anchor write errors | +| `AUDIT_ANCHOR_RETENTION_DAYS` | `400` | The least the anchor bucket's default Object Lock retention may be. The bucket's rule locks the anchors; the gateway checks it at startup and in every verification, and refuses to start (verification fails) unless the bucket locks new objects in COMPLIANCE mode for at least this many days. Set it to the bucket's retention. Needs `s3:GetBucketObjectLockConfiguration` | | `AUDIT_ANCHOR_INTERVAL_MINUTES` | `60` | Anchor interval | | `AUDIT_HASH_V2_FROM` | | Chain index where hash v2 starts, recorded after upgrading, for the hourly verification and `verify-audit` (see [Verification](/deployment/terraform/s3-worm-storage/#pinning-where-hash-v2-starts)). The gateway refuses to start when it isn't a chain index. | | `AWS_ENDPOINT_URL_S3` | | S3 endpoint override, e.g. MinIO (`testbypass` builds only) | diff --git a/docs/src/content/docs/deployment/terraform/aws-infrastructure.mdx b/docs/src/content/docs/deployment/terraform/aws-infrastructure.mdx index 981f7cb9..1d663bdc 100644 --- a/docs/src/content/docs/deployment/terraform/aws-infrastructure.mdx +++ b/docs/src/content/docs/deployment/terraform/aws-infrastructure.mdx @@ -287,7 +287,9 @@ resource "aws_iam_role_policy" "gateway_s3" { "s3:ListBucket", # verify-audit reads every version of each anchor "s3:GetObjectVersion", - "s3:ListBucketVersions" + "s3:ListBucketVersions", + # The gateway checks the bucket's default retention at startup and in every verification + "s3:GetBucketObjectLockConfiguration" ] Resource = [ aws_s3_bucket.audit_anchors.arn, diff --git a/docs/src/content/docs/deployment/terraform/s3-worm-storage.mdx b/docs/src/content/docs/deployment/terraform/s3-worm-storage.mdx index b498ad61..ac08d1ba 100644 --- a/docs/src/content/docs/deployment/terraform/s3-worm-storage.mdx +++ b/docs/src/content/docs/deployment/terraform/s3-worm-storage.mdx @@ -288,7 +288,9 @@ resource "aws_iam_role_policy" "anchor_writer" { "s3:GetObject", "s3:GetObjectVersion", "s3:ListBucket", - "s3:ListBucketVersions" + "s3:ListBucketVersions", + # The gateway checks the bucket's default retention at startup and in every verification + "s3:GetBucketObjectLockConfiguration" ] Resource = [ aws_s3_bucket.audit_anchors.arn, @@ -347,7 +349,8 @@ resource "aws_iam_role_policy" "anchor_verifier" { "s3:GetObjectVersion", "s3:ListBucket", "s3:ListBucketVersions", - "s3:GetObjectRetention" + "s3:GetObjectRetention", + "s3:GetBucketObjectLockConfiguration" ] Resource = [ aws_s3_bucket.audit_anchors.arn, @@ -409,8 +412,10 @@ It prints a summary and exits non-zero if it finds any integrity failure. Warnin - `RGW_DATABASE_URL`, `GATEWAY_DATABASE_URL` or `DATABASE_URL`, for a role with `SELECT` on `audit.audit_event` (the `audit_reader` role is enough). - Unless `--no-anchors` is given: `AUDIT_ANCHOR_BUCKET`, `AUDIT_ANCHOR_CHAIN_ID` and AWS credentials allowed - `s3:ListBucket`, `s3:ListBucketVersions`, `s3:GetObject` and `s3:GetObjectVersion` on the bucket, and - `kms:Decrypt` on its key (the verifier role above). + `s3:ListBucket`, `s3:ListBucketVersions`, `s3:GetObject`, `s3:GetObjectVersion` and + `s3:GetBucketObjectLockConfiguration` on the bucket, and `kms:Decrypt` on its key (the verifier role above). + It fails unless the bucket's default retention is COMPLIANCE mode for at least `AUDIT_ANCHOR_RETENTION_DAYS` + (default 400). - On a gateway that was upgraded from a version before hash v2: `AUDIT_HASH_V2_FROM` (or `--hash-v2-from`), see below. @@ -497,9 +502,11 @@ version before hash v2 has no legacy events and needs no pin. ### Hourly Verification in the Gateway The gateway runs these checks itself at half past every hour (UTC), with the configuration `verify-audit` reads: -`AUDIT_HMAC_SECRET`, `AUDIT_HASH_V2_FROM` and the anchor settings. Its role needs `s3:ListBucketVersions` and -`s3:GetObjectVersion` on the bucket ([IAM Role for the Gateway](#iam-role-for-the-gateway)); without them every run -fails and alerts. +`AUDIT_HMAC_SECRET`, `AUDIT_HASH_V2_FROM` and the anchor settings. Its role needs `s3:ListBucketVersions`, +`s3:GetObjectVersion` and `s3:GetBucketObjectLockConfiguration` on the bucket +([IAM Role for the Gateway](#iam-role-for-the-gateway)); without them every run fails and alerts. The gateway also +reads the bucket's Object Lock configuration before it starts, and refuses to start unless the default retention is +COMPLIANCE mode for at least `AUDIT_ANCHOR_RETENTION_DAYS`. - **What a run checks.** The first run after the gateway starts, every run while verification is failing, and one run a day check every event. The hourly runs in between check the events written since the last passing run, diff --git a/docs/src/content/docs/operations/upgrades.mdx b/docs/src/content/docs/operations/upgrades.mdx index 0728572e..6a353fdc 100644 --- a/docs/src/content/docs/operations/upgrades.mdx +++ b/docs/src/content/docs/operations/upgrades.mdx @@ -207,6 +207,18 @@ A restore to before the upgrade loses the audit events written since; see ## Release Notes That Need Action +### Anchor bucket retention checked at startup + +`AUDIT_ANCHOR_RETENTION_DAYS` is now checked: the gateway reads the anchor bucket's Object Lock configuration at +startup and in every audit verification, and refuses to start (verification fails and alerts) unless the bucket's +default retention is COMPLIANCE mode for at least that many days. + +1. **Give the gateway's role `s3:GetBucketObjectLockConfiguration` on the anchor bucket before deploying** (see + [IAM Role for the Gateway](/deployment/terraform/s3-worm-storage/#iam-role-for-the-gateway)), and the role + `verify-audit` runs with elsewhere. Without it the new release doesn't start. +2. **Check `AUDIT_ANCHOR_RETENTION_DAYS` matches the bucket's retention** (`worm_retention_days`). It defaults to + 400. + ### Audit hash v2 and `verify-audit` This release changes how audit events are hashed (hash v2: every stored column, recomputable from the row) and @@ -260,7 +272,7 @@ itself every hour and alerts when they fail. run, and the hourly verification keeps failing (with a daily reminder), until the anchors' retention ends and they're deleted. -### Column encryption key required +### Column encryption key required (v0.1.7) TOTP seeds, YubiKey public IDs and the Slack bot token are now stored encrypted with a dedicated key. See [Column Encryption](/configuration/environment-variables/#column-encryption). @@ -268,12 +280,9 @@ TOTP seeds, YubiKey public IDs and the Slack bot token are now stored encrypted 1. **Set `COLUMN_ENCRYPTION_KEY` before deploying** (`openssl rand -base64 32`) and back it up. Every command that opens the database, including `migrate`, refuses to run without it, so a deploy without the key fails before the new release is promoted. -2. **Existing secrets are encrypted when the new release starts**, before it serves requests. The log shows - a `Column encryption:` line with counts and a warning for each row encrypted. This happens at startup rather - than in `migrate` because the previous release keeps serving while `migrate` runs. It runs again 10 minutes - after startup, for anything the previous release stored during the rollout (a TOTP enrollment or a Slack - reconnect); if it encrypts anything then, it reports it to Sentry. During this upgrade that can be a real - enrollment from the rollout; after it, it means a secret was written straight into the database. +2. **Existing secrets are encrypted when the new release starts** (v0.1.7 to v0.1.10 only), before it serves + requests. Releases after v0.1.10 don't encrypt existing secrets, so upgrade a gateway from before v0.1.7 to + v0.1.10 first. 3. **Expect a short gap for TOTP and YubiKey OTP.** Once the new release has upgraded the data, any process still running the previous release can't read the encrypted values, so its TOTP and YubiKey OTP checks and Slack notifications fail until it is replaced. WebAuthn and backup codes keep working. diff --git a/docs/src/content/docs/security/authentication/api-tokens.mdx b/docs/src/content/docs/security/authentication/api-tokens.mdx index a2898a15..bc449514 100644 --- a/docs/src/content/docs/security/authentication/api-tokens.mdx +++ b/docs/src/content/docs/security/authentication/api-tokens.mdx @@ -71,7 +71,7 @@ See [Roles](/security/rbac/roles/) for detailed permission lists. A token is always limited by the person who owns it: - **Capped by the owner's current role.** Every request needs the permission in the token's own list *and* in the owner's current role. Demote the owner and their tokens lose the extra access on the next request. -- **Stops working with its owner.** Tokens of a locked, suspended or deleted user are rejected. +- **Stops working with its owner.** Tokens of a locked or deleted user are rejected. - **Created within the owner's role.** When a token is created or edited, each permission must be one the owner's role grants. Deployers can create tokens for themselves; only admins can create tokens for other users. - **Rack access only, plus a few gateway endpoints.** Tokens can use the rack proxy (`/api/v1/rack-proxy/*`) and only these gateway endpoints: `GET /api/v1/info`, `GET /api/v1/rack`, `POST /api/v1/deploy-approval-requests` and `GET /api/v1/deploy-approval-requests/:id`. User, token, settings, audit-log and approval endpoints refuse tokens, even admin-owned ones. - **No privileged runs.** Tokens can never start a process with a custom image, host volumes, privileged mode or node placement options. diff --git a/docs/src/content/docs/security/authentication/index.mdx b/docs/src/content/docs/security/authentication/index.mdx index 511ae1a4..9dd55caa 100644 --- a/docs/src/content/docs/security/authentication/index.mdx +++ b/docs/src/content/docs/security/authentication/index.mdx @@ -118,7 +118,7 @@ Users from other domains receive an error during OAuth callback. The gateway verifies that: - Email is verified in Google's ID token - Email domain matches the allowed domain -- User account exists and is not locked/suspended +- User account exists and is not locked ### Token Hashing diff --git a/docs/src/content/docs/security/authentication/sessions.mdx b/docs/src/content/docs/security/authentication/sessions.mdx index 3cd04475..f2e975c3 100644 --- a/docs/src/content/docs/security/authentication/sessions.mdx +++ b/docs/src/content/docs/security/authentication/sessions.mdx @@ -116,7 +116,6 @@ sequenceDiagram | Not idle | `last_seen_at` + TTL > now | 401 + auto-revoke | | User exists | Associated user found | 401 + auto-revoke | | User not locked | `locked_at` is null | 401 + auto-revoke | -| User not suspended | `suspended` is false | 401 + auto-revoke | ## Session Expiration @@ -169,7 +168,7 @@ sessions: **Revoke** for one, or **Revoke All**. Users can revoke their own; rev Sessions stop working when: - The session expires (idle timeout) -- The user is locked (locking revokes every session) or suspended +- The user is locked (locking revokes every session) - The user is deleted ## Session Metadata diff --git a/docs/src/content/docs/security/compliance/audit-trail.mdx b/docs/src/content/docs/security/compliance/audit-trail.mdx index c8736a50..3da54244 100644 --- a/docs/src/content/docs/security/compliance/audit-trail.mdx +++ b/docs/src/content/docs/security/compliance/audit-trail.mdx @@ -73,7 +73,7 @@ it to stdout as one JSON line for CloudWatch: | `user_email` | User's email address (a token's owner for API tokens) | `alice@example.com` | | `user_name` | User's display name | `Alice Developer` | | `api_token_id`, `api_token_name` | The API token, when one made the request | `ci-deploy` | -| `action_type` | The scope of the action: `convox`, `gateway`, `auth` or `security`. Results that admin handlers record use `users`, `tokens` or `admin` | `convox` | +| `action_type` | The scope of the permission the action needs: `convox`, `gateway`, `auth` or `security`. A request's attempt and result entries have the same type | `convox` | | `action` | `.`, named after the route's permission | `release.promote`, `user.create`, `login.oauth_failed` | | `resource` | Target resource | `myapp` | | `resource_type` | Resource category | `app`, `build`, `release` | @@ -335,7 +335,7 @@ ORDER BY day DESC; | **Requests** | Every gateway API and rack request, reads included, with its RBAC decision | | **Deployments** | Build, release, promote, deploy approvals | | **Environment** | Read, set, unset (values redacted), and secret reveals | -| **User Management** | Create, update, delete, lock | +| **User Management** | Create, update, delete, lock, MFA reset | | **Token Management** | Create, delete tokens | | **Settings** | Configuration changes | diff --git a/docs/src/content/docs/security/rbac/permissions.mdx b/docs/src/content/docs/security/rbac/permissions.mdx index fda3d2c1..852f6745 100644 --- a/docs/src/content/docs/security/rbac/permissions.mdx +++ b/docs/src/content/docs/security/rbac/permissions.mdx @@ -42,7 +42,6 @@ Resources that map to Convox API operations: |----------|-------------| | `app` | Applications | | `build` | Docker image builds | -| `cert` | SSL/TLS certificates | | `deploy` | Deployment operations | | `env` | Environment variables | | `instance` | EC2/container instances | @@ -50,9 +49,7 @@ Resources that map to Convox API operations: | `object` | Build artifacts and objects | | `process` | Running processes/containers | | `rack` | Rack configuration | -| `registry` | Docker registries | | `release` | Application releases | -| `resource` | Convox resources (databases, etc.) | | `secret` | Secret env var values (`convox:secret:read` reveals them) | The gateway forwards only the rack API routes it has a permission for (see @@ -98,7 +95,6 @@ Authentication and MFA resources: | Action | Description | |--------|-------------| -| `add` | Add item to a collection | | `approve` | Approve a request | | `create` | Create a new resource | | `delete` | Delete a resource | @@ -106,12 +102,10 @@ Authentication and MFA resources: | `exec` | Execute commands | | `generate` | Generate credentials/codes | | `import` | Import external resources | -| `keyroll` | Rotate keys/credentials | | `list` | List resources | | `manage` | General management operations | | `promote` | Promote a release | | `read` | Read/view a resource | -| `remove` | Remove item from collection | | `restart` | Restart a service | | `run_privileged` | Start a process with privileged options | | `set` | Set a value | diff --git a/docs/src/content/docs/security/rbac/roles.mdx b/docs/src/content/docs/security/rbac/roles.mdx index de7b598d..7917bc38 100644 --- a/docs/src/content/docs/security/rbac/roles.mdx +++ b/docs/src/content/docs/security/rbac/roles.mdx @@ -155,7 +155,7 @@ Beyond all Ops permissions: | `gateway:api_token:delete` | Delete their own API tokens | | `convox:deploy:deploy_with_approval` | Lets their CI tokens deploy with approval | -Deployers can issue CI/CD tokens for their own pipelines. A token can't hold more than the deployer's role, and stops working if the deployer is demoted, locked, or suspended. +Deployers can issue CI/CD tokens for their own pipelines. A token can't hold more than the deployer's role, and stops working if the deployer is demoted or locked. ### Use Cases diff --git a/docs/src/content/docs/user-guide/web-ui/audit-logs.mdx b/docs/src/content/docs/user-guide/web-ui/audit-logs.mdx index 71a2702f..aa6ad100 100644 --- a/docs/src/content/docs/user-guide/web-ui/audit-logs.mdx +++ b/docs/src/content/docs/user-guide/web-ui/audit-logs.mdx @@ -36,10 +36,13 @@ Use the filter panel to narrow results: Filter by user, resource, or action text. ### Action Type -- Convox API -- Authentication -- User Management (results of user changes) -- Token Management (results of API token changes) + +The scope of the permission the action needs. A request's attempt and result entries have the same type. + +- Convox API (`convox`): requests to the rack +- Gateway (`gateway`): users, API tokens, deploy approvals, settings and the gateway's other features +- Authentication (`auth`): login, logout and MFA +- Security (`security`): secrets, rate limits and audit log verification ### Status - Success diff --git a/internal/cli/gateway_test_auth.go b/internal/cli/gateway_test_auth.go index 2833c408..9c6d4ad8 100644 --- a/internal/cli/gateway_test_auth.go +++ b/internal/cli/gateway_test_auth.go @@ -6,6 +6,8 @@ import ( "strings" "github.com/spf13/cobra" + + "github.com/DocSpring/rack-gateway/internal/cli/webauthn" ) // TestAuthCommand returns the cobra command for testing authentication and MFA flows. @@ -235,8 +237,7 @@ func testOTPAuth(cmd *cobra.Command, baseURL, bearer string, status *MFAStatusRe } func testWebAuthnAuth(baseURL, bearer string, status *MFAStatusResponse) error { - // Check WebAuthn availability - if !checkWebAuthnAvailability() { + if !webauthn.CheckAvailability() { return fmt.Errorf("no WebAuthn device detected") } diff --git a/internal/cli/mfa_verify.go b/internal/cli/mfa_verify.go index cfdc41c9..7b99c65d 100644 --- a/internal/cli/mfa_verify.go +++ b/internal/cli/mfa_verify.go @@ -328,15 +328,6 @@ func submitMFAVerification(baseURL, sessionToken, code string) error { return fmt.Errorf("gateway error: %s", RenderGatewayError(bodyBytes)) } -func checkWebAuthnAvailability() bool { - defer func() { - if r := recover(); r != nil { - _ = r - } - }() - return webauthn.CheckAvailability() -} - func hasMethodType(methods []MFAMethodResponse, methodType string) bool { for _, m := range methods { if m.Type == methodType && !m.IsEnrolling { diff --git a/internal/convox/commands.go b/internal/convox/commands.go index e55dd25f..44207ea6 100644 --- a/internal/convox/commands.go +++ b/internal/convox/commands.go @@ -50,12 +50,6 @@ var Commands = []Command{ AllowedFlags: []string{}, Description: "Get information about an app", }, - { - Command: "apps cancel", - Permissions: []string{rbac.Convox(rbac.ResourceApp, rbac.ActionUpdate)}, - AllowedFlags: []string{}, - Description: "Cancel an app update", - }, { Command: "apps lock", Permissions: []string{rbac.Convox(rbac.ResourceApp, rbac.ActionUpdate)}, @@ -146,12 +140,6 @@ var Commands = []Command{ AllowedFlags: []string{}, Description: "Get information about a release", }, - { - Command: "releases manifest", - Permissions: []string{rbac.Convox(rbac.ResourceRelease, rbac.ActionRead)}, - AllowedFlags: []string{}, - Description: "Get manifest for a release", - }, { Command: "releases promote", Permissions: []string{rbac.Convox(rbac.ResourceRelease, rbac.ActionPromote)}, @@ -262,19 +250,6 @@ var Commands = []Command{ AllowedFlags: []string{"count", "cpu", "memory"}, Description: "Scale a service", }, - // Services - { - Command: "services", - Permissions: []string{rbac.Convox(rbac.ResourceApp, rbac.ActionRead)}, - AllowedFlags: []string{}, - Description: "List services for an app", - }, - { - Command: "services restart", - Permissions: []string{rbac.Convox(rbac.ResourceProcess, rbac.ActionStart)}, - AllowedFlags: []string{}, - Description: "Restart a service", - }, // Instances { Command: "instances", @@ -282,24 +257,6 @@ var Commands = []Command{ AllowedFlags: []string{}, Description: "List instances", }, - { - Command: "instances ssh", - Permissions: []string{rbac.Convox(rbac.ResourceInstance, rbac.ActionRead)}, - AllowedFlags: []string{}, - Description: "Run a shell on an instance", - }, - { - Command: "instances terminate", - Permissions: []string{rbac.Convox(rbac.ResourceInstance, rbac.ActionTerminate)}, - AllowedFlags: []string{}, - Description: "Terminate an instance", - }, - { - Command: "instances keyroll", - Permissions: []string{rbac.Convox(rbac.ResourceInstance, rbac.ActionKeyroll)}, - AllowedFlags: []string{}, - Description: "Roll SSH key on instances", - }, // Rack { Command: "rack", @@ -337,89 +294,6 @@ var Commands = []Command{ AllowedFlags: []string{"version"}, Description: "Update the rack", }, - // Resources - { - Command: "resources", - Permissions: []string{rbac.Convox(rbac.ResourceResource, rbac.ActionList)}, - AllowedFlags: []string{}, - Description: "List resources", - }, - { - Command: "resources info", - Permissions: []string{rbac.Convox(rbac.ResourceResource, rbac.ActionRead)}, - AllowedFlags: []string{}, - Description: "Get information about a resource", - }, - { - Command: "resources url", - Permissions: []string{rbac.Convox(rbac.ResourceResource, rbac.ActionRead)}, - AllowedFlags: []string{}, - Description: "Get URL for a resource", - }, - // Balancers - { - Command: "balancers", - Permissions: []string{rbac.Convox(rbac.ResourceApp, rbac.ActionRead)}, - AllowedFlags: []string{}, - Description: "List balancers for an app", - }, - // SSL - { - Command: "ssl", - Permissions: []string{rbac.Convox(rbac.ResourceCert, rbac.ActionList)}, - AllowedFlags: []string{}, - Description: "List certificate associates for an app", - }, - { - Command: "ssl update", - Permissions: []string{rbac.Convox(rbac.ResourceCert, rbac.ActionUpdate)}, - AllowedFlags: []string{}, - Description: "Update certificate for an app", - }, - // Certs - { - Command: "certs", - Permissions: []string{rbac.Convox(rbac.ResourceCert, rbac.ActionList)}, - AllowedFlags: []string{}, - Description: "List certificates", - }, - { - Command: "certs delete", - Permissions: []string{rbac.Convox(rbac.ResourceCert, rbac.ActionDelete)}, - AllowedFlags: []string{}, - Description: "Delete a certificate", - }, - { - Command: "certs generate", - Permissions: []string{rbac.Convox(rbac.ResourceCert, rbac.ActionGenerate)}, - AllowedFlags: []string{"domains"}, - Description: "Generate a certificate", - }, - { - Command: "certs import", - Permissions: []string{rbac.Convox(rbac.ResourceCert, rbac.ActionImport)}, - AllowedFlags: []string{"chain", "public", "private"}, - Description: "Import a certificate", - }, - // Registries - { - Command: "registries", - Permissions: []string{rbac.Convox(rbac.ResourceRegistry, rbac.ActionList)}, - AllowedFlags: []string{}, - Description: "List private registries", - }, - { - Command: "registries add", - Permissions: []string{rbac.Convox(rbac.ResourceRegistry, rbac.ActionAdd)}, - AllowedFlags: []string{"username", "password"}, - Description: "Add a private registry", - }, - { - Command: "registries remove", - Permissions: []string{rbac.Convox(rbac.ResourceRegistry, rbac.ActionRemove)}, - AllowedFlags: []string{}, - Description: "Remove private registry", - }, } // commandMap indexes commands for fast lookup diff --git a/internal/gateway/CLAUDE.md b/internal/gateway/CLAUDE.md index 7fd08000..e0454404 100644 --- a/internal/gateway/CLAUDE.md +++ b/internal/gateway/CLAUDE.md @@ -153,7 +153,9 @@ Key files: - `db/audit_append.go`, `db/audit_hash.go`, `db/audit_normalize.go` - Hash-chained append (hash v2). The row is normalized to its stored form, hashed, then stored, so the stored row always re-hashes to `event_hash`. Never change the v2 encoding (existing rows would stop verifying); a new audit column needs a new hash version -- `jobs/audit/anchor_writer.go` - Hourly S3 Object Lock anchors. Every write (job or `write-anchor`) holds the +- `jobs/audit/anchor_writer.go` - Hourly S3 Object Lock anchors. The writer sets no retention: the bucket's default + COMPLIANCE rule locks them, and the gateway refuses to start (verification fails) unless that rule keeps them for + `AUDIT_ANCHOR_RETENTION_DAYS` (`auditverify.CheckBucketRetention`). Every write (job or `write-anchor`) holds the audit anchor advisory lock (`db.WithAuditAnchorLock`) from the existence check through the Put: anchors are permanent, so an overlapping write would be a permanent verification failure. `write-anchor` uses `WriteCurrentAnchor` (the database's clock, never the host's). The Put is sent once (`putAnchorOnce`: an SDK diff --git a/internal/gateway/app/column_upgrade.go b/internal/gateway/app/column_upgrade.go deleted file mode 100644 index bb6d9c5b..00000000 --- a/internal/gateway/app/column_upgrade.go +++ /dev/null @@ -1,96 +0,0 @@ -package app - -import ( - "context" - "fmt" - "sort" - "strings" - "time" - - sentry "github.com/getsentry/sentry-go" - - "github.com/DocSpring/rack-gateway/internal/gateway/db" - gtwlog "github.com/DocSpring/rack-gateway/internal/gateway/logging" -) - -// columnUpgradeRepeatDelay is when the column encryption upgrade runs again after startup: after a rolling -// deploy has replaced the previous release, which may have written plaintext secrets meanwhile. -const columnUpgradeRepeatDelay = 10 * time.Minute - -// upgradePlaintextColumns encrypts secrets stored before column encryption (v0.1.7), before anything reads -// them. During a rolling deploy the previous release can still write plaintext after this, so the sweep runs -// once more later. Both runs go away in a later release. -func (a *App) upgradePlaintextColumns() error { - upgraded, err := a.Database.UpgradePlaintextColumns(context.Background()) - if err != nil { - return err - } - reportPlaintextInUpgradedColumns(upgraded, captureColumnUpgradeAlert) - time.AfterFunc(columnUpgradeRepeatDelay, func() { - repeatColumnUpgrade(context.Background(), a.Database.UpgradePlaintextColumns, captureColumnUpgradeAlert) - }) - return nil -} - -// repeatColumnUpgrade runs the sweep again and reports it encrypting anything. By now this release, which never -// stores plaintext, has replaced the previous one, so a plaintext secret was written straight into the database -// (a known TOTP seed planted for a user, say) unless this is the rollout to column encryption and the previous -// release stored it meanwhile. The sweep has logged each row it encrypted. -func repeatColumnUpgrade( - ctx context.Context, - upgrade func(context.Context) (map[string]db.ColumnUpgradeCounts, error), - report func(message string), -) { - upgraded, err := upgrade(ctx) - if err != nil { - gtwlog.Errorf("Column encryption: repeat upgrade failed: %v", err) - return - } - encrypted := encryptedColumns(upgraded, func(db.ColumnUpgradeCounts) bool { return true }) - if encrypted == "" { - return - } - message := fmt.Sprintf("Column encryption: the sweep %s after startup encrypted plaintext secrets (%s). "+ - "Unless the previous release stored them during the upgrade to column encryption, someone wrote them "+ - "straight into the database; the warnings logged for each row name them", - columnUpgradeRepeatDelay, encrypted) - gtwlog.Errorf("%s", message) - report(message) -} - -// reportPlaintextInUpgradedColumns reports the startup sweep encrypting plaintext in a column that already held -// values encrypted under this key. That column was upgraded before, and this release and the one before it never -// store plaintext, so someone wrote it straight into the database. On the first upgrade every value is plaintext -// and nothing is reported. -func reportPlaintextInUpgradedColumns(upgraded map[string]db.ColumnUpgradeCounts, report func(message string)) { - encrypted := encryptedColumns(upgraded, func(counts db.ColumnUpgradeCounts) bool { return counts.CurrentKey > 0 }) - if encrypted == "" { - return - } - message := fmt.Sprintf("Column encryption: the startup sweep encrypted plaintext secrets in columns that "+ - "were already encrypted (%s), so someone wrote them straight into the database; the warnings logged for "+ - "each row name them", encrypted) - gtwlog.Errorf("%s", message) - report(message) -} - -// encryptedColumns lists the columns include accepts where the sweep encrypted values, with how many, sorted and -// comma-separated, or "" for none. -func encryptedColumns(upgraded map[string]db.ColumnUpgradeCounts, include func(db.ColumnUpgradeCounts) bool) string { - encrypted := make([]string, 0, len(upgraded)) - for column, counts := range upgraded { - if counts.Encrypted > 0 && include(counts) { - encrypted = append(encrypted, fmt.Sprintf("%s: %d", column, counts.Encrypted)) - } - } - sort.Strings(encrypted) - return strings.Join(encrypted, ", ") -} - -func captureColumnUpgradeAlert(message string) { - sentry.WithScope(func(scope *sentry.Scope) { - scope.SetLevel(sentry.LevelError) - scope.SetTag("component", "column_encryption") - sentry.CaptureMessage(message) - }) -} diff --git a/internal/gateway/app/column_upgrade_test.go b/internal/gateway/app/column_upgrade_test.go deleted file mode 100644 index a9394ada..00000000 --- a/internal/gateway/app/column_upgrade_test.go +++ /dev/null @@ -1,81 +0,0 @@ -package app - -import ( - "bytes" - "context" - "errors" - "log" - "os" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/DocSpring/rack-gateway/internal/gateway/db" -) - -func sweepReturning( - upgraded map[string]db.ColumnUpgradeCounts, err error, -) func(context.Context) (map[string]db.ColumnUpgradeCounts, error) { - return func(context.Context) (map[string]db.ColumnUpgradeCounts, error) { return upgraded, err } -} - -// After the rollout nothing stores plaintext secrets, so the repeat sweep encrypting one means someone wrote it -// straight into the database (a TOTP seed they know, say). That goes to Sentry, not just the log. -func TestRepeatColumnUpgradeReportsEncryptedSecrets(t *testing.T) { - var logs bytes.Buffer - log.SetOutput(&logs) - t.Cleanup(func() { log.SetOutput(os.Stderr) }) - var reported []string - report := func(message string) { reported = append(reported, message) } - - repeatColumnUpgrade(context.Background(), sweepReturning(map[string]db.ColumnUpgradeCounts{ - "slack_integration.bot_token_encrypted": {}, - "mfa_methods.secret": {Encrypted: 1, OtherKey: 2}, - }, nil), report) - - require.Len(t, reported, 1) - assert.Contains(t, reported[0], "encrypted plaintext secrets (mfa_methods.secret: 1)") - assert.Contains(t, logs.String(), "[ERROR] "+reported[0]) -} - -func TestRepeatColumnUpgradeIsQuietWhenNothingWasEncrypted(t *testing.T) { - var logs bytes.Buffer - log.SetOutput(&logs) - t.Cleanup(func() { log.SetOutput(os.Stderr) }) - reported := 0 - report := func(string) { reported++ } - - repeatColumnUpgrade(context.Background(), sweepReturning(map[string]db.ColumnUpgradeCounts{ - "mfa_methods.secret": {OtherKey: 1, Unreadable: 1}, - }, nil), report) - assert.Zero(t, reported, "values left alone aren't new plaintext") - - repeatColumnUpgrade(context.Background(), sweepReturning(nil, errors.New("connection refused")), report) - assert.Zero(t, reported) - assert.Contains(t, logs.String(), "[ERROR] Column encryption: repeat upgrade failed: connection refused") -} - -// The startup sweep reports plaintext only in a column that already held values encrypted under this key: that -// column was upgraded before, so the plaintext was written since. On the first upgrade everything is plaintext. -func TestStartupColumnUpgradeReportsPlaintextInUpgradedColumns(t *testing.T) { - var logs bytes.Buffer - log.SetOutput(&logs) - t.Cleanup(func() { log.SetOutput(os.Stderr) }) - var reported []string - report := func(message string) { reported = append(reported, message) } - - reportPlaintextInUpgradedColumns(map[string]db.ColumnUpgradeCounts{ - "mfa_methods.secret": {Encrypted: 12}, - "slack_integration.bot_token_encrypted": {Encrypted: 1}, - }, report) - assert.Empty(t, reported, "the first upgrade encrypts everything") - - reportPlaintextInUpgradedColumns(map[string]db.ColumnUpgradeCounts{ - "mfa_methods.secret": {Encrypted: 1, CurrentKey: 12}, - "slack_integration.bot_token_encrypted": {Encrypted: 1}, - }, report) - require.Len(t, reported, 1) - assert.Contains(t, reported[0], "columns that were already encrypted (mfa_methods.secret: 1)") - assert.Contains(t, logs.String(), "[ERROR] "+reported[0]) -} diff --git a/internal/gateway/app/setup.go b/internal/gateway/app/setup.go index f91a719e..efb7c15b 100644 --- a/internal/gateway/app/setup.go +++ b/internal/gateway/app/setup.go @@ -40,9 +40,6 @@ func (a *App) initializeServices() error { if err := a.ensureAuditSecret(); err != nil { return err } - if err := a.upgradePlaintextColumns(); err != nil { - return err - } if err := a.seedInitialUsers(); err != nil { return err } @@ -283,6 +280,12 @@ func (a *App) initJobsClient(sender email.Sender, notifier *slackpkg.Notifier, a if err != nil { return fmt.Errorf("failed to load audit anchor config: %w", err) } + // Anchors are only as permanent as the bucket's Object Lock rule: refuse to start without one that keeps them. + if auditAnchorConfig != nil { + if err := auditAnchorConfig.CheckBucketRetention(context.Background()); err != nil { + return fmt.Errorf("audit anchor bucket: %w", err) + } + } client, err := jobs.NewClient(a.Database.Pool(), &jobs.Dependencies{ Database: a.Database, @@ -335,7 +338,7 @@ func (a *App) startJobsWorker() error { } func (a *App) initSecurityNotifier(logger *audit.Logger) { - a.SecurityNotifier = security.NewNotifier(a.EmailSender, logger, a.Database, a.JobsClient) + a.SecurityNotifier = security.NewNotifier(logger, a.Database, a.JobsClient) } func (a *App) deriveRackIdentity() (string, string) { diff --git a/internal/gateway/audit/constants.go b/internal/gateway/audit/constants.go index 2fd56d12..2e7fbd90 100644 --- a/internal/gateway/audit/constants.go +++ b/internal/gateway/audit/constants.go @@ -25,7 +25,6 @@ var ( ActionScopeMFAPreferences = rbac.ResourceMFAPreferences.String() ActionScopeRateLimit = "rate_limit" ActionScopeRelease = rbac.ResourceRelease.String() - ActionScopeSuspiciousActivity = "suspicious_activity" ActionScopeTrustedDevice = rbac.ResourceTrustedDevice.String() ) diff --git a/internal/gateway/auditverify/anchor_checks.go b/internal/gateway/auditverify/anchor_checks.go index 6cf01e7f..44dceedb 100644 --- a/internal/gateway/auditverify/anchor_checks.go +++ b/internal/gateway/auditverify/anchor_checks.go @@ -27,6 +27,11 @@ func verifyAnchors( now time.Time, report *Report, ) error { + if opts.MinRetentionDays > 0 { + if err := CheckBucketRetention(ctx, opts.Store, opts.Bucket, opts.MinRetentionDays); err != nil { + report.fail("%v", err) + } + } records, err := loadAnchors(ctx, opts, report) if err != nil { return err diff --git a/internal/gateway/auditverify/anchors_test.go b/internal/gateway/auditverify/anchors_test.go index 39f26b18..20cb39fc 100644 --- a/internal/gateway/auditverify/anchors_test.go +++ b/internal/gateway/auditverify/anchors_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/riverqueue/river" "github.com/riverqueue/river/rivertype" @@ -41,7 +42,7 @@ func writeAnchor(t *testing.T, database *gwdb.Database, bucket *fakes3.Bucket, a func writeAnchorStoredAt(t *testing.T, database *gwdb.Database, bucket *fakes3.Bucket, at, stored time.Time) { t.Helper() bucket.AdvanceTo(stored) - worker := jobaudit.NewAnchorWriterWorker(database, bucket, testBucket, testChainID, 7) + worker := jobaudit.NewAnchorWriterWorker(database, bucket, testBucket, testChainID) job := &river.Job[jobaudit.AnchorWriterArgs]{JobRow: &rivertype.JobRow{CreatedAt: at}} require.NoError(t, worker.Work(context.Background(), job)) } @@ -261,3 +262,22 @@ func TestVerifyReadsAnchorsConcurrently(t *testing.T) { requireOK(t, report) assert.Equal(t, anchors, report.AnchorsChecked) } + +// Anchors are only kept as long as the bucket's default Object Lock rule says, so verification fails when that's +// shorter than AUDIT_ANCHOR_RETENTION_DAYS. +func TestVerifyFailsWhenTheBucketKeepsAnchorsTooBriefly(t *testing.T) { + database := newAuditDatabase(t) + bucket := fakes3.New(firstHour) + writeEvents(t, database, 1) + writeAnchor(t, database, bucket, firstHour) + bucket.LockConfiguration.Rule.DefaultRetention.Days = aws.Int32(30) + + report := verify(t, database, auditverify.Options{ + Anchors: &auditverify.AnchorOptions{ + Store: bucket, Bucket: testBucket, ChainID: testChainID, MinRetentionDays: 400, + }, + Now: firstHour.Add(time.Hour), + }) + require.Equal(t, 1, report.FailureCount, "failures: %q", report.Failures) + assert.Contains(t, report.Failures[0], "locks anchors for 30 days, less than AUDIT_ANCHOR_RETENTION_DAYS (400)") +} diff --git a/internal/gateway/auditverify/bucket_retention.go b/internal/gateway/auditverify/bucket_retention.go new file mode 100644 index 00000000..aa3ffaff --- /dev/null +++ b/internal/gateway/auditverify/bucket_retention.go @@ -0,0 +1,53 @@ +package auditverify + +import ( + "context" + "fmt" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/aws/aws-sdk-go-v2/service/s3/types" +) + +// daysPerYear is how S3 counts a default retention given in years. +const daysPerYear = 365 + +// BucketLockReader reads a bucket's Object Lock configuration (s3:GetBucketObjectLockConfiguration). *s3.Client +// implements it. +type BucketLockReader interface { + GetObjectLockConfiguration( + ctx context.Context, + params *s3.GetObjectLockConfigurationInput, + optFns ...func(*s3.Options), + ) (*s3.GetObjectLockConfigurationOutput, error) +} + +// CheckBucketRetention returns an error unless the bucket locks every new object in COMPLIANCE mode for at least +// minDays days by default. The anchor writer sets no retention of its own, so the bucket's default rule is all that +// keeps anchors from being deleted or overwritten; GOVERNANCE mode can be bypassed. +func CheckBucketRetention(ctx context.Context, store BucketLockReader, bucket string, minDays int) error { + output, err := store.GetObjectLockConfiguration(ctx, &s3.GetObjectLockConfigurationInput{ + Bucket: aws.String(bucket), + }) + if err != nil { + return fmt.Errorf("couldn't read the Object Lock configuration of s3://%s: %w", bucket, err) + } + config := output.ObjectLockConfiguration + if config == nil || config.ObjectLockEnabled != types.ObjectLockEnabledEnabled { + return fmt.Errorf("s3://%s doesn't have Object Lock enabled", bucket) + } + if config.Rule == nil || config.Rule.DefaultRetention == nil { + return fmt.Errorf("s3://%s has no default Object Lock retention, so anchors aren't locked", bucket) + } + retention := config.Rule.DefaultRetention + if retention.Mode != types.ObjectLockRetentionModeCompliance { + return fmt.Errorf("s3://%s's default Object Lock retention is in %s mode; anchors need COMPLIANCE mode", + bucket, retention.Mode) + } + days := int(aws.ToInt32(retention.Days)) + int(aws.ToInt32(retention.Years))*daysPerYear + if days < minDays { + return fmt.Errorf("s3://%s locks anchors for %d days, less than AUDIT_ANCHOR_RETENTION_DAYS (%d)", + bucket, days, minDays) + } + return nil +} diff --git a/internal/gateway/auditverify/bucket_retention_test.go b/internal/gateway/auditverify/bucket_retention_test.go new file mode 100644 index 00000000..79588aeb --- /dev/null +++ b/internal/gateway/auditverify/bucket_retention_test.go @@ -0,0 +1,70 @@ +package auditverify + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/stretchr/testify/require" + + "github.com/DocSpring/rack-gateway/internal/gateway/testutil/fakes3" +) + +func lockConfig(mode types.ObjectLockRetentionMode, days, years int32) *types.ObjectLockConfiguration { + retention := &types.DefaultRetention{Mode: mode} + if days > 0 { + retention.Days = aws.Int32(days) + } + if years > 0 { + retention.Years = aws.Int32(years) + } + return &types.ObjectLockConfiguration{ + ObjectLockEnabled: types.ObjectLockEnabledEnabled, + Rule: &types.ObjectLockRule{DefaultRetention: retention}, + } +} + +type deniedLockReader struct{} + +func (deniedLockReader) GetObjectLockConfiguration( + context.Context, + *s3.GetObjectLockConfigurationInput, + ...func(*s3.Options), +) (*s3.GetObjectLockConfigurationOutput, error) { + return nil, errors.New("AccessDenied") +} + +func TestCheckBucketRetention(t *testing.T) { + compliance, governance := types.ObjectLockRetentionModeCompliance, types.ObjectLockRetentionModeGovernance + noRule := &types.ObjectLockConfiguration{ObjectLockEnabled: types.ObjectLockEnabledEnabled} + cases := []struct { + name string + config *types.ObjectLockConfiguration + want string + }{ + {"compliance for long enough", lockConfig(compliance, 400, 0), ""}, + {"years count as 365 days", lockConfig(compliance, 0, 2), ""}, + {"too short", lockConfig(compliance, 30, 0), "for 30 days, less than AUDIT_ANCHOR_RETENTION_DAYS (400)"}, + {"governance mode", lockConfig(governance, 400, 0), "anchors need COMPLIANCE mode"}, + {"no default rule", noRule, "has no default Object Lock retention"}, + {"no Object Lock", &types.ObjectLockConfiguration{}, "doesn't have Object Lock enabled"}, + {"no configuration", nil, "doesn't have Object Lock enabled"}, + } + for _, tc := range cases { + bucket := fakes3.New(time.Now()) + bucket.LockConfiguration = tc.config + err := CheckBucketRetention(context.Background(), bucket, "anchors", 400) + if tc.want == "" { + require.NoError(t, err, tc.name) + } else { + require.ErrorContains(t, err, tc.want, tc.name) + } + } + + err := CheckBucketRetention(context.Background(), deniedLockReader{}, "anchors", 400) + require.ErrorContains(t, err, "couldn't read the Object Lock configuration of s3://anchors: AccessDenied") +} diff --git a/internal/gateway/auditverify/verify.go b/internal/gateway/auditverify/verify.go index 750a506f..0923054a 100644 --- a/internal/gateway/auditverify/verify.go +++ b/internal/gateway/auditverify/verify.go @@ -31,9 +31,10 @@ type ChainSource interface { AuditChainStats(ctx context.Context, toIndex int64) (db.AuditChainStats, error) } -// AnchorStore reads anchors and their versions. *s3.Client implements it. GetObject is called from several -// goroutines at once. +// AnchorStore reads anchors, their versions and the bucket's Object Lock configuration. *s3.Client implements it. +// GetObject is called from several goroutines at once. type AnchorStore interface { + BucketLockReader ListObjectVersions( ctx context.Context, params *s3.ListObjectVersionsInput, @@ -52,6 +53,9 @@ type AnchorOptions struct { // MaxAge is how old the latest anchor may be (default DefaultMaxAnchorAge). An older one means anchors have // stopped being written, which leaves every new event unprotected. MaxAge time.Duration + // MinRetentionDays is AUDIT_ANCHOR_RETENTION_DAYS: the bucket must lock new objects in COMPLIANCE mode for at + // least this long by default (CheckBucketRetention). Zero skips the check. + MinRetentionDays int } // Options configures a verification run. @@ -147,11 +151,18 @@ func (o *Options) validate() error { if o.Anchors == nil { return nil } - if o.Anchors.Store == nil || o.Anchors.Bucket == "" || o.Anchors.ChainID == "" { + return o.Anchors.validate() +} + +func (a *AnchorOptions) validate() error { + if a.Store == nil || a.Bucket == "" || a.ChainID == "" { return errors.New("anchor verification needs an S3 client, a bucket and a chain ID") } - if o.Anchors.MaxAge < 0 { - return fmt.Errorf("the maximum anchor age must not be negative (got %s)", o.Anchors.MaxAge) + if a.MaxAge < 0 { + return fmt.Errorf("the maximum anchor age must not be negative (got %s)", a.MaxAge) + } + if a.MinRetentionDays < 0 { + return fmt.Errorf("the minimum anchor retention must not be negative (got %d)", a.MinRetentionDays) } return nil } diff --git a/internal/gateway/auth/service.go b/internal/gateway/auth/service.go index b4e41bc7..3942d1ad 100644 --- a/internal/gateway/auth/service.go +++ b/internal/gateway/auth/service.go @@ -42,7 +42,6 @@ type User struct { Session *db.UserSession `json:"-"` MFAType string `json:"-"` // "totp" or "webauthn" - inline MFA provided with request MFAValue string `json:"-"` // The MFA verification data (TOTP code or WebAuthn assertion) - Suspended bool `json:"-"` MFAEnrolled bool `json:"-"` PreferredMFAMethod *string `json:"-"` LockedAt *time.Time `json:"-"` @@ -235,7 +234,7 @@ func (a *Service) validateAPIToken(tokenString string, r *http.Request) (*User, _ = audit.LogDB(a.database, &db.AuditLog{ UserEmail: "", UserName: "", - ActionType: "auth", + ActionType: audit.ActionTypeAuth, Action: "token.validate", ResourceType: "auth", Resource: "api_token", @@ -257,9 +256,6 @@ func (a *Service) validateAPIToken(tokenString string, r *http.Request) (*User, if user == nil { return nil, fmt.Errorf("token owner not found") } - if user.Suspended { - return nil, fmt.Errorf("token owner is suspended") - } if user.LockedAt != nil { return nil, fmt.Errorf("token owner is locked") } @@ -271,7 +267,6 @@ func (a *Service) validateAPIToken(tokenString string, r *http.Request) (*User, IsAPIToken: true, TokenID: &apiToken.ID, TokenName: apiToken.Name, - Suspended: user.Suspended, MFAEnrolled: user.MFAEnrolled, PreferredMFAMethod: user.PreferredMFAMethod, LockedAt: user.LockedAt, @@ -384,7 +379,6 @@ func newAuthUserFromSessionResult(result *SessionValidationResult) *User { Roles: append([]string(nil), result.User.Roles...), IsAPIToken: false, Session: result.Session, - Suspended: result.User.Suspended, MFAEnrolled: result.User.MFAEnrolled, PreferredMFAMethod: result.User.PreferredMFAMethod, LockedAt: result.User.LockedAt, diff --git a/internal/gateway/auth/session_manager.go b/internal/gateway/auth/session_manager.go index 03a2827b..7cfba6d1 100644 --- a/internal/gateway/auth/session_manager.go +++ b/internal/gateway/auth/session_manager.go @@ -234,10 +234,6 @@ func (m *SessionManager) validateUserState(sessionID int64, user *db.User) error _, _ = m.db.RevokeUserSession(sessionID, nil) return fmt.Errorf("session user missing") } - if user.Suspended { - _, _ = m.db.RevokeUserSession(sessionID, nil) - return fmt.Errorf("user suspended") - } if user.LockedAt != nil { _, _ = m.db.RevokeUserSession(sessionID, nil) return fmt.Errorf("user locked") diff --git a/internal/gateway/colcrypt/colcrypt.go b/internal/gateway/colcrypt/colcrypt.go index d10ff206..1c8c6446 100644 --- a/internal/gateway/colcrypt/colcrypt.go +++ b/internal/gateway/colcrypt/colcrypt.go @@ -144,11 +144,6 @@ func keyID(key []byte) string { return hex.EncodeToString(mac.Sum(nil)[:kidBytes]) } -// IsEncrypted reports whether value carries the envelope prefix (under any key or version). -func IsEncrypted(value string) bool { - return strings.HasPrefix(value, Prefix) -} - // EnvelopePrefix returns the start of every value this Cipher writes: "rgwenc:v1::". func (c *Cipher) EnvelopePrefix() string { return Prefix + version + ":" + c.kid + ":" diff --git a/internal/gateway/colcrypt/colcrypt_test.go b/internal/gateway/colcrypt/colcrypt_test.go index c92411cb..67e5a210 100644 --- a/internal/gateway/colcrypt/colcrypt_test.go +++ b/internal/gateway/colcrypt/colcrypt_test.go @@ -59,7 +59,7 @@ func TestRoundTrip(t *testing.T) { c := mustCipher(t, DevKey) for _, plaintext := range []string{"", "JBSWY3DPEHPK3PXP", "xoxb-1234-abcd", "with:colons|and|pipes"} { envelope := mustEncrypt(t, c, plaintext, MFASecretScope(7, "totp")) - if !strings.HasPrefix(envelope, c.EnvelopePrefix()) || !IsEncrypted(envelope) { + if !strings.HasPrefix(envelope, c.EnvelopePrefix()) { t.Fatalf("envelope %q lacks prefix %q", envelope, c.EnvelopePrefix()) } if plaintext != "" && strings.Contains(envelope, plaintext) { diff --git a/internal/gateway/db/column_encryption.go b/internal/gateway/db/column_encryption.go index afd32ce5..5e34fc70 100644 --- a/internal/gateway/db/column_encryption.go +++ b/internal/gateway/db/column_encryption.go @@ -1,15 +1,10 @@ package db import ( - "context" "database/sql" - "encoding/base64" "fmt" - "strconv" - "strings" "github.com/DocSpring/rack-gateway/internal/gateway/colcrypt" - gtwlog "github.com/DocSpring/rack-gateway/internal/gateway/logging" ) // Two columns hold secrets and are encrypted with COLUMN_ENCRYPTION_KEY (see package colcrypt): @@ -17,11 +12,7 @@ import ( // - mfa_methods.secret (TOTP seeds, YubiKey public IDs, WebAuthn enrollment placeholders), bound to user_id // - slack_integration.bot_token_encrypted, bound to workspace_id // -// Reads are strict: a stored value without the envelope prefix is an error, never a usable secret. But -// UpgradePlaintextColumns, which the server runs at startup and again 10 minutes later for the upgrade to column -// encryption (v0.1.7), encrypts every plaintext value it finds. Until it is removed, someone who can write to -// the database can plant a seed they know and have it encrypted, so each value it encrypts is logged (row IDs, -// never values), and the server reports the repeat run encrypting anything to Sentry. +// Reads are strict: a stored value without the envelope prefix is an error, never a usable secret. // decryptMFASecret fills method.Secret, or method.SecretErr when the stored value can't be decrypted. A // failure makes only this method unusable; it never turns into an empty secret silently. NULL means the @@ -37,180 +28,3 @@ func (d *Database) decryptMFASecret(method *MFAMethod, stored sql.NullString) { } method.Secret = plaintext } - -// ColumnUpgradeCounts counts what UpgradePlaintextColumns did with one column. -type ColumnUpgradeCounts struct { - Encrypted int // values written before encryption, now encrypted - OtherKey int // values encrypted under another key (or envelope version), left alone - Unreadable int // values written before encryption that couldn't be decoded, left alone - // CurrentKey is how many values were already encrypted under this key when the sweep started: when there are - // some, the column has been upgraded before, so a plaintext value found now was written since. - CurrentKey int -} - -// legacyColumn describes how to find and upgrade one encrypted column's values from before encryption. -type legacyColumn struct { - name string - // selectSQL returns (id, binding, value) for rows whose value doesn't start with the LIKE pattern $1. - selectSQL string - // countSQL counts the rows whose value starts with the LIKE pattern $1. - countSQL string - // updateSQL sets $1 on row $2 if it still holds $3. - updateSQL string - plaintext func(stored string) (string, error) - scope func(binding string) (colcrypt.Scope, error) - // owner names whose value a row holds, for the log (never the value). - owner func(binding string) string -} - -var legacyColumns = []legacyColumn{ - { - name: "mfa_methods.secret", - selectSQL: `SELECT id, user_id::text || ':' || type, secret FROM mfa_methods - WHERE secret IS NOT NULL AND secret NOT LIKE $1`, - countSQL: `SELECT count(*) FROM mfa_methods WHERE secret LIKE $1`, - updateSQL: `UPDATE mfa_methods SET secret = $1 WHERE id = $2 AND secret = $3`, - plaintext: func(stored string) (string, error) { return stored, nil }, - scope: func(binding string) (colcrypt.Scope, error) { - user, methodType, _ := strings.Cut(binding, ":") - userID, err := strconv.ParseInt(user, 10, 64) - return colcrypt.MFASecretScope(userID, methodType), err - }, - owner: func(binding string) string { - user, methodType, _ := strings.Cut(binding, ":") - return fmt.Sprintf("user %s, %s method", user, methodType) - }, - }, - { - name: "slack_integration.bot_token_encrypted", - selectSQL: `SELECT id, workspace_id, bot_token_encrypted FROM slack_integration - WHERE bot_token_encrypted NOT LIKE $1`, - countSQL: `SELECT count(*) FROM slack_integration WHERE bot_token_encrypted LIKE $1`, - updateSQL: `UPDATE slack_integration SET bot_token_encrypted = $1 WHERE id = $2 AND bot_token_encrypted = $3`, - // Tokens used to be stored base64-encoded. - plaintext: func(stored string) (string, error) { - token, err := base64.StdEncoding.DecodeString(stored) - return string(token), err - }, - scope: func(binding string) (colcrypt.Scope, error) { - return colcrypt.SlackBotTokenScope(binding), nil - }, - owner: func(binding string) string { return "Slack workspace " + binding }, - }, -} - -type legacyRow struct { - id int64 - binding string // "user_id:type" or workspace_id - value string -} - -// UpgradePlaintextColumns encrypts secrets written before column encryption existed: plaintext -// mfa_methods.secret values and base64-encoded Slack bot tokens. The server runs it at startup, after -// migrations and before serving. It is not a migration because the previous release keeps serving while a -// deploy runs `migrate`, and that release can't read encrypted values. -// -// It is safe to run repeatedly and from several instances at once: each update only applies if the row -// still holds the value that was read. Values encrypted under another key are counted and left alone. -// It encrypts any plaintext value, including one written straight into the database after the upgrade, so it -// logs each row it encrypts at warn level, with the row's IDs and never its value. Remove it in a later -// release, once deployments have upgraded past v0.1.7. -func (d *Database) UpgradePlaintextColumns(ctx context.Context) (map[string]ColumnUpgradeCounts, error) { - upgraded := make(map[string]ColumnUpgradeCounts, len(legacyColumns)) - for _, column := range legacyColumns { - counts, err := d.upgradeColumn(ctx, column) - if err != nil { - return nil, fmt.Errorf("encrypt existing %s values: %w", column.name, err) - } - if counts.Encrypted+counts.OtherKey+counts.Unreadable > 0 { - gtwlog.Infof("Column encryption: %s: encrypted %d existing values, %d under another key, %d unreadable", - column.name, counts.Encrypted, counts.OtherKey, counts.Unreadable) - } - upgraded[column.name] = counts - } - return upgraded, nil -} - -func (d *Database) upgradeColumn(ctx context.Context, column legacyColumn) (ColumnUpgradeCounts, error) { - var counts ColumnUpgradeCounts - if err := d.db.QueryRowContext(ctx, column.countSQL, d.cipher.EnvelopePrefix()+"%").Scan( - &counts.CurrentKey, - ); err != nil { - return counts, err - } - rows, err := d.legacyRows(ctx, column.selectSQL) - if err != nil { - return counts, err - } - for _, row := range rows { - if err := d.upgradeRow(ctx, column, row, &counts); err != nil { - return counts, fmt.Errorf("row %d: %w", row.id, err) - } - } - return counts, nil -} - -// legacyRows returns every row selectSQL finds without this key's envelope prefix. They are all read -// before any is updated. -func (d *Database) legacyRows(ctx context.Context, selectSQL string) ([]legacyRow, error) { - // The prefix is "rgwenc:v1::", which has no LIKE wildcards. - rows, err := d.db.QueryContext(ctx, selectSQL, d.cipher.EnvelopePrefix()+"%") - if err != nil { - return nil, err - } - defer func() { _ = rows.Close() }() - - var found []legacyRow - for rows.Next() { - var row legacyRow - if err := rows.Scan(&row.id, &row.binding, &row.value); err != nil { - return nil, err - } - found = append(found, row) - } - return found, rows.Err() -} - -// upgradeRow encrypts one row's value in place. The update goes straight to d.db rather than d.exec so the -// old value never reaches the SQL debug log. -func (d *Database) upgradeRow( - ctx context.Context, - column legacyColumn, - row legacyRow, - counts *ColumnUpgradeCounts, -) error { - if colcrypt.IsEncrypted(row.value) { - gtwlog.Warnf("Column encryption: %s row %d is encrypted with another column key or format; leaving it", - column.name, row.id) - counts.OtherKey++ - return nil - } - plaintext, err := column.plaintext(row.value) - if err != nil { - gtwlog.Warnf("Column encryption: %s row %d can't be decoded; leaving it", column.name, row.id) - counts.Unreadable++ - return nil - } - scope, err := column.scope(row.binding) - if err != nil { - return err - } - encrypted, err := d.cipher.Encrypt(plaintext, scope) - if err != nil { - return err - } - result, err := d.db.ExecContext(ctx, column.updateSQL, encrypted, row.id, row.value) - if err != nil { - return err - } - affected, err := result.RowsAffected() - if err != nil { - return err - } - if affected > 0 { // 0 when another instance upgraded the row first - gtwlog.Warnf("Column encryption: encrypted a plaintext %s value: row %d (%s)", - column.name, row.id, column.owner(row.binding)) - counts.Encrypted++ - } - return nil -} diff --git a/internal/gateway/db/column_encryption_test.go b/internal/gateway/db/column_encryption_test.go index 79a5933d..b7a3014a 100644 --- a/internal/gateway/db/column_encryption_test.go +++ b/internal/gateway/db/column_encryption_test.go @@ -1,13 +1,8 @@ package db_test import ( - "bytes" - "context" "database/sql" "encoding/base64" - "fmt" - "log" - "os" "testing" "time" @@ -21,8 +16,6 @@ import ( const ( testSeed = "JBSWY3DPEHPK3PXP" fakeSlackBot = "xoxb-test-token" - mfaColumn = "mfa_methods.secret" - slackColumn = "slack_integration.bot_token_encrypted" ) func storedMFASecret(t *testing.T, database *gwdb.Database, methodID int64) string { @@ -45,30 +38,6 @@ func setStoredMFASecret(t *testing.T, database *gwdb.Database, methodID int64, v require.NoError(t, err) } -func insertRawMFAMethod(t *testing.T, database *gwdb.Database, userID int64, methodType string, secret any) int64 { - t.Helper() - var id int64 - err := database.DB().QueryRow( - `INSERT INTO mfa_methods (user_id, type, label, secret, confirmed_at) - VALUES ($1, $2, 'Legacy', $3, NOW()) RETURNING id`, - userID, methodType, secret, - ).Scan(&id) - require.NoError(t, err) - return id -} - -// insertRawSlackIntegration stores a token the way an older release (or someone with database access) would. -func insertRawSlackIntegration(t *testing.T, database *gwdb.Database, storedToken string) { - t.Helper() - _, err := database.DB().Exec( - `INSERT INTO slack_integration ( - workspace_id, workspace_name, bot_token_encrypted, alert_deploy_approvals_channel_id, bot_user_id, scope - ) VALUES ('T123', 'Test', $1, '', 'U123', 'chat:write')`, - storedToken, - ) - require.NoError(t, err) -} - func createSlackIntegration(t *testing.T, database *gwdb.Database) { t.Helper() _, err := database.CreateSlackIntegration( @@ -195,110 +164,6 @@ func TestSlackBotTokenStoredEncryptedAndBoundToWorkspace(t *testing.T) { require.Empty(t, integration.BotToken) } -func TestUpgradePlaintextColumns(t *testing.T) { - 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{"viewer"}) - require.NoError(t, err) - - // Rows as an older release wrote them. - legacy := map[int64]string{ - insertRawMFAMethod(t, database, alice.ID, "totp", testSeed): testSeed, - insertRawMFAMethod(t, database, alice.ID, "yubiotp", "cccccccccccc"): "cccccccccccc", - insertRawMFAMethod(t, database, bob.ID, "webauthn", "webauthn_enroll_2_1700000"): "webauthn_enroll_2_1700000", - } - noSecret := insertRawMFAMethod(t, database, bob.ID, "webauthn", nil) - current, err := database.CreateMFAMethod(bob.ID, "totp", "App", "KB6VQXGZLMN4Y3DC", nil, nil, nil, nil) - require.NoError(t, err) - currentStored := storedMFASecret(t, database, current.ID) - - otherCipher, err := colcrypt.New(base64.StdEncoding.EncodeToString(make([]byte, 32))) - require.NoError(t, err) - otherKeyValue, err := otherCipher.Encrypt(testSeed, colcrypt.MFASecretScope(bob.ID, "totp")) - require.NoError(t, err) - otherKey := insertRawMFAMethod(t, database, bob.ID, "totp", otherKeyValue) - - insertRawSlackIntegration(t, database, base64.StdEncoding.EncodeToString([]byte(fakeSlackBot))) - - upgraded, err := database.UpgradePlaintextColumns(context.Background()) - require.NoError(t, err) - require.Equal(t, gwdb.ColumnUpgradeCounts{Encrypted: 3, OtherKey: 1, CurrentKey: 1}, upgraded[mfaColumn]) - require.Equal(t, gwdb.ColumnUpgradeCounts{Encrypted: 1}, upgraded[slackColumn]) - - for id, plaintext := range legacy { - stored := storedMFASecret(t, database, id) - require.True(t, colcrypt.IsEncrypted(stored)) - require.NotContains(t, stored, plaintext) - method, err := database.GetMFAMethodByID(id) - require.NoError(t, err) - require.NoError(t, method.SecretErr) - require.Equal(t, plaintext, method.Secret) - } - - unchanged, err := database.GetMFAMethodByID(noSecret) - require.NoError(t, err) - require.NoError(t, unchanged.SecretErr) - require.Empty(t, unchanged.Secret) - require.Equal(t, currentStored, storedMFASecret(t, database, current.ID)) - require.Equal(t, otherKeyValue, storedMFASecret(t, database, otherKey)) - other, err := database.GetMFAMethodByID(otherKey) - require.NoError(t, err) - require.ErrorIs(t, other.SecretErr, colcrypt.ErrWrongKey) - - integration, err := database.GetSlackIntegration() - require.NoError(t, err) - require.NoError(t, integration.BotTokenErr) - require.Equal(t, fakeSlackBot, integration.BotToken) - slackStored := storedBotToken(t, database) - - // A second run finds nothing left to do. - again, err := database.UpgradePlaintextColumns(context.Background()) - require.NoError(t, err) - require.Equal(t, gwdb.ColumnUpgradeCounts{OtherKey: 1, CurrentKey: 4}, again[mfaColumn]) - require.Equal(t, gwdb.ColumnUpgradeCounts{CurrentKey: 1}, again[slackColumn]) - require.Equal(t, slackStored, storedBotToken(t, database)) -} - -// The sweep encrypts any plaintext it finds, including a seed someone planted in the database after the upgrade, -// so it logs whose value each row it encrypts holds (never the value). -func TestUpgradePlaintextColumnsLogsEachEncryptedRow(t *testing.T) { - database := dbtest.NewDatabase(t) - alice, err := database.CreateUser("alice@example.com", "Alice", []string{"admin"}) - require.NoError(t, err) - planted := insertRawMFAMethod(t, database, alice.ID, "totp", testSeed) - insertRawSlackIntegration(t, database, base64.StdEncoding.EncodeToString([]byte(fakeSlackBot))) - var logs bytes.Buffer - log.SetOutput(&logs) - t.Cleanup(func() { log.SetOutput(os.Stderr) }) - - _, err = database.UpgradePlaintextColumns(context.Background()) - require.NoError(t, err) - - out := logs.String() - require.Contains(t, out, fmt.Sprintf( - "[WARN] Column encryption: encrypted a plaintext %s value: row %d (user %d, totp method)", - mfaColumn, planted, alice.ID)) - require.Contains(t, out, "[WARN] Column encryption: encrypted a plaintext "+slackColumn+" value: row ") - require.Contains(t, out, "(Slack workspace T123)") - require.NotContains(t, out, testSeed) - require.NotContains(t, out, fakeSlackBot) -} - -func TestUpgradePlaintextColumnsLeavesUndecodableSlackToken(t *testing.T) { - database := dbtest.NewDatabase(t) - insertRawSlackIntegration(t, database, "not base64!") - - upgraded, err := database.UpgradePlaintextColumns(context.Background()) - require.NoError(t, err) - require.Equal(t, gwdb.ColumnUpgradeCounts{Unreadable: 1}, upgraded[slackColumn]) - require.Equal(t, "not base64!", storedBotToken(t, database)) - - integration, err := database.GetSlackIntegration() - require.NoError(t, err) - require.ErrorIs(t, integration.BotTokenErr, colcrypt.ErrNotEncrypted) -} - func TestNewDatabaseRequiresColumnEncryptionKey(t *testing.T) { for value, want := range map[string]error{ "": colcrypt.ErrKeyMissing, diff --git a/internal/gateway/db/sessions_lookup.go b/internal/gateway/db/sessions_lookup.go index 62ade7d5..68cd0bde 100644 --- a/internal/gateway/db/sessions_lookup.go +++ b/internal/gateway/db/sessions_lookup.go @@ -36,7 +36,7 @@ func (d *Database) getUserSessionWithUser( us.expires_at, us.channel, us.device_id, us.device_name, us.mfa_verified_at, us.recent_step_up_at, us.trusted_device_id, us.ip_address, us.user_agent, us.revoked_at, us.revoked_by_user_id, us.metadata, us.device_metadata, - u.id, u.email, u.name, u.roles, u.created_at, u.updated_at, u.suspended, + u.id, u.email, u.name, u.roles, u.created_at, u.updated_at, u.mfa_enrolled, u.mfa_enforced_at, u.preferred_mfa_method, u.locked_at, u.locked_reason, u.locked_by_user_id, lbu.email, lbu.name FROM user_sessions us @@ -65,7 +65,7 @@ func (d *Database) getUserSessionWithUser( &session.ID, &session.UserID, &session.TokenHash, &session.CreatedAt, &session.UpdatedAt, &session.LastSeenAt, &session.ExpiresAt, &session.Channel, &s.deviceID, &s.deviceName, &s.mfaVerified, &s.recentStep, &s.trustedID, &s.ip, &s.ua, &s.revoked, &s.revoker, &s.meta, &s.deviceMeta, - &user.ID, &user.Email, &user.Name, &rolesJSON, &user.CreatedAt, &user.UpdatedAt, &user.Suspended, + &user.ID, &user.Email, &user.Name, &rolesJSON, &user.CreatedAt, &user.UpdatedAt, &user.MFAEnrolled, &mfaEnforced, &preferred, &lockedAt, &lockedReason, &lockedBy, &lockedByEmail, &lockedByName, ); err != nil { diff --git a/internal/gateway/db/types.go b/internal/gateway/db/types.go index d91712c8..4f3db0cb 100644 --- a/internal/gateway/db/types.go +++ b/internal/gateway/db/types.go @@ -13,7 +13,6 @@ type User struct { Roles []string `json:"roles"` CreatedAt time.Time `json:"created_at"` UpdatedAt time.Time `json:"updated_at"` - Suspended bool `json:"suspended"` MFAEnrolled bool `json:"mfa_enrolled"` MFAEnforcedAt *time.Time `json:"mfa_enforced_at,omitempty"` PreferredMFAMethod *string `json:"preferred_mfa_method,omitempty"` @@ -86,7 +85,7 @@ type AuditLog struct { UserName string `json:"user_name,omitempty"` APITokenID *int64 `json:"api_token_id,omitempty"` APITokenName string `json:"api_token_name,omitempty"` - ActionType string `json:"action_type"` // "convox", "users", "auth" + ActionType string `json:"action_type"` // permission scope: auth, convox, gateway, security Action string `json:"action"` // e.g., "env.read", "user.create", "login.oauth_failed" Command string `json:"command,omitempty"` Resource string `json:"resource,omitempty"` diff --git a/internal/gateway/db/users.go b/internal/gateway/db/users.go index 198f3f7c..ab504cd2 100644 --- a/internal/gateway/db/users.go +++ b/internal/gateway/db/users.go @@ -11,7 +11,7 @@ import ( // GetUserByID retrieves a user by ID func (d *Database) GetUserByID(id int64) (*User, error) { row := d.queryRow( - `SELECT id, email, name, roles, created_at, updated_at, suspended, mfa_enrolled, + `SELECT id, email, name, roles, created_at, updated_at, mfa_enrolled, mfa_enforced_at, preferred_mfa_method, locked_at, locked_reason, locked_by_user_id FROM users WHERE id = ?`, id, @@ -35,7 +35,7 @@ func (d *Database) GetUserByID(id int64) (*User, error) { // GetUser retrieves a user by email func (d *Database) GetUser(email string) (*User, error) { row := d.queryRow( - `SELECT u.id, u.email, u.name, u.roles, u.created_at, u.updated_at, u.suspended, + `SELECT u.id, u.email, u.name, u.roles, u.created_at, u.updated_at, u.mfa_enrolled, u.mfa_enforced_at, u.preferred_mfa_method, u.locked_at, u.locked_reason, u.locked_by_user_id, lbu.email, lbu.name @@ -96,13 +96,6 @@ func (d *Database) UpdateUserRoles(email string, roles []string) error { return nil } -// deleteUserAuditLogs is a no-op that preserves audit logs for compliance. -// Audit logs are immutable and should never be deleted. -// The audit.audit_event table has triggers that block DELETE operations. -func (_ *Database) deleteUserAuditLogs(_ string) error { - return nil -} - // DeleteUser removes a user from the database and revokes all their sessions. func (d *Database) DeleteUser(email string) error { user, err := d.GetUser(email) @@ -110,10 +103,6 @@ func (d *Database) DeleteUser(email string) error { return err } - if err := d.deleteUserAuditLogs(email); err != nil { - return err - } - if user != nil { count, err := d.RevokeAllUserSessions(user.ID, nil) if err != nil { @@ -132,7 +121,7 @@ func (d *Database) DeleteUser(email string) error { // ListUsers returns all users func (d *Database) ListUsers() ([]*User, error) { rows, err := d.query( - `SELECT u.id, u.email, u.name, u.roles, u.created_at, u.updated_at, u.suspended, + `SELECT u.id, u.email, u.name, u.roles, u.created_at, u.updated_at, u.locked_at, u.locked_reason, u.locked_by_user_id, cu.id, cu.email, cu.name FROM users u diff --git a/internal/gateway/db/users_scan.go b/internal/gateway/db/users_scan.go index 3886e0c5..b038f894 100644 --- a/internal/gateway/db/users_scan.go +++ b/internal/gateway/db/users_scan.go @@ -13,7 +13,7 @@ type userScanner interface { } // scanUserBasicFields scans the basic user fields that are present in all user queries. -// This includes ID, email, name, roles, timestamps, suspended state, and MFA fields. +// This includes ID, email, name, roles, timestamps, and MFA and lock fields. // Returns the user struct and any nullable fields that need additional processing. func scanUserBasicFields(scanner userScanner) (*User, *userNullableFields, error) { var user User @@ -26,7 +26,6 @@ func scanUserBasicFields(scanner userScanner) (*User, *userNullableFields, error &fields.rolesJSON, &user.CreatedAt, &user.UpdatedAt, - &user.Suspended, &user.MFAEnrolled, &user.MFAEnforcedAt, &user.PreferredMFAMethod, @@ -55,7 +54,6 @@ func scanUserWithLockedBy(scanner userScanner) (*User, error) { &rolesJSON, &user.CreatedAt, &user.UpdatedAt, - &user.Suspended, &user.MFAEnrolled, &user.MFAEnforcedAt, &user.PreferredMFAMethod, @@ -99,7 +97,6 @@ func scanUserWithCreator(scanner userScanner) (*User, error) { &rolesJSON, &user.CreatedAt, &user.UpdatedAt, - &user.Suspended, &user.LockedAt, &lockedReason, &user.LockedByUserID, diff --git a/internal/gateway/email/postmark.go b/internal/gateway/email/postmark.go index e2395ba0..df0cc940 100644 --- a/internal/gateway/email/postmark.go +++ b/internal/gateway/email/postmark.go @@ -6,7 +6,6 @@ import ( "fmt" "net/http" "os" - "sync" "time" gtwlog "github.com/DocSpring/rack-gateway/internal/gateway/logging" @@ -135,14 +134,10 @@ type LoggerSender struct{ From string } // Send logs an email to stdout instead of sending it. func (_ *LoggerSender) Send(to, subject, textBody, htmlBody string) error { + gtwlog.DebugTopicf(gtwlog.TopicEmailSummary, "to=%s subject=%q", to, subject) if htmlBody != "" { - gtwlog.DebugTopicf(gtwlog.TopicEmailSummary, "to=%s subject=%q", to, subject) gtwlog.DebugTopicf(gtwlog.TopicEmailBody, "text=%s\n\nhtml=%s", textBody, htmlBody) - appendDevEmail([]string{to}, subject, textBody, htmlBody) - return nil } - gtwlog.DebugTopicf(gtwlog.TopicEmailSummary, "to=%s subject=%q", to, subject) - appendDevEmail([]string{to}, subject, textBody, htmlBody) return nil } @@ -157,51 +152,9 @@ func (_ *LoggerSender) SendMany(to []string, subject, textBody, htmlBody string) bcc = append(bcc, to[1:]...) } b, _ := json.Marshal(bcc) + gtwlog.DebugTopicf(gtwlog.TopicEmailSummary, "to=%s bcc=%s subject=%q", primary, string(b), subject) if htmlBody != "" { - gtwlog.DebugTopicf(gtwlog.TopicEmailSummary, "to=%s bcc=%s subject=%q", primary, string(b), subject) gtwlog.DebugTopicf(gtwlog.TopicEmailBody, "text=%s html=%s", textBody, htmlBody) - appendDevEmail(to, subject, textBody, htmlBody) - return nil } - gtwlog.DebugTopicf(gtwlog.TopicEmailSummary, "to=%s bcc=%s subject=%q", primary, string(b), subject) - appendDevEmail(to, subject, textBody, htmlBody) return nil } - -// DevEmail represents an in-memory email for E2E tests and local development. -type DevEmail struct { - To []string `json:"to"` - Subject string `json:"subject"` - Text string `json:"text"` - HTML string `json:"html"` - TS time.Time `json:"ts"` -} - -var ( - devMu sync.Mutex - devOutbox []DevEmail - devMax = 100 -) - -func appendDevEmail(to []string, subject, text, html string) { - devMu.Lock() - defer devMu.Unlock() - devOutbox = append(devOutbox, DevEmail{To: to, Subject: subject, Text: text, HTML: html, TS: time.Now().UTC()}) - if len(devOutbox) > devMax { - devOutbox = devOutbox[len(devOutbox)-devMax:] - } -} - -// GetDevOutbox returns up to 'limit' most recent dev emails (newest last) -func GetDevOutbox(limit int) []DevEmail { - devMu.Lock() - defer devMu.Unlock() - n := len(devOutbox) - if limit <= 0 || limit > n { - limit = n - } - start := n - limit - out := make([]DevEmail, limit) - copy(out, devOutbox[start:]) - return out -} diff --git a/internal/gateway/handlers/admin_audit.go b/internal/gateway/handlers/admin_audit.go index 98961f2f..298f3106 100644 --- a/internal/gateway/handlers/admin_audit.go +++ b/internal/gateway/handlers/admin_audit.go @@ -12,8 +12,10 @@ import ( "github.com/gin-gonic/gin" + "github.com/DocSpring/rack-gateway/internal/gateway/audit" "github.com/DocSpring/rack-gateway/internal/gateway/auth" "github.com/DocSpring/rack-gateway/internal/gateway/db" + "github.com/DocSpring/rack-gateway/internal/gateway/rbac" ) // AdminHandler is defined in admin.go @@ -279,7 +281,7 @@ func (h *AdminHandler) auditAdminAction( email, name := extractAuditUserInfo(c) detailsJSON := marshalAuditDetails(details) - actionType, resourceType := classifyAuditAction(action) + actionType, resourceType := auditEntryTypes(c, action) entry := &db.AuditLog{ UserEmail: email, @@ -329,15 +331,15 @@ func marshalAuditDetails(details map[string]interface{}) string { return "" } -func classifyAuditAction(action string) (actionType, resourceType string) { - switch { - case strings.HasPrefix(action, "api_token."): - return "tokens", "api_token" - case strings.HasPrefix(action, "user."): - return "users", "user" - default: - return "admin", "admin" +// auditEntryTypes returns the action type and resource type of an admin handler's audit entry, as its request's +// attempt entry has them: the scope of the route's permission, and the action's resource. +func auditEntryTypes(c *gin.Context, action string) (actionType, resourceType string) { + actionType = audit.ActionTypeGateway + if spec, ok := rbac.LookupHTTPRoute(c.Request.Method, c.FullPath()); ok { + actionType = spec.AuditName(c.Request.URL.Query()).ActionType } + resourceType, _, _ = strings.Cut(action, ".") + return actionType, resourceType } func mapStatusToRBAC(status string) string { diff --git a/internal/gateway/handlers/admin_audit_types_test.go b/internal/gateway/handlers/admin_audit_types_test.go new file mode 100644 index 00000000..53fb6923 --- /dev/null +++ b/internal/gateway/handlers/admin_audit_types_test.go @@ -0,0 +1,36 @@ +package handlers + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/require" +) + +// An admin handler's entries have the action type of their request's attempt entry: its route's permission scope. +func TestAuditEntryTypesFollowTheRoute(t *testing.T) { + gin.SetMode(gin.TestMode) + const tlsRefresh = "/api/v1/settings/rack-tls-cert/refresh" + cases := []struct { + method, pattern, path, action string + actionType, resourceType string + }{ + {http.MethodPost, "/api/v1/users/:email/lock", "/api/v1/users/a@b.co/lock", "user.lock", "gateway", "user"}, + {http.MethodPost, "/api/v1/api-tokens", "/api/v1/api-tokens", "api_token.create", "gateway", "api_token"}, + {http.MethodPost, tlsRefresh, tlsRefresh, "rack_tls_cert.refresh", "security", "rack_tls_cert"}, + // A request outside the route table (handlers called directly) is a gateway entry. + {http.MethodGet, "/unrouted", "/unrouted", "audit.export", "gateway", "audit"}, + } + for _, tc := range cases { + router := gin.New() + var actionType, resourceType string + router.Handle(tc.method, tc.pattern, func(c *gin.Context) { + actionType, resourceType = auditEntryTypes(c, tc.action) + }) + router.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(tc.method, tc.path, nil)) + require.Equal(t, tc.actionType, actionType, tc.action) + require.Equal(t, tc.resourceType, resourceType, tc.action) + } +} diff --git a/internal/gateway/handlers/admin_users_list.go b/internal/gateway/handlers/admin_users_list.go index edbcd33b..d24c2d4e 100644 --- a/internal/gateway/handlers/admin_users_list.go +++ b/internal/gateway/handlers/admin_users_list.go @@ -51,7 +51,6 @@ func directoryEntry(user *db.User) *db.User { Roles: user.Roles, CreatedAt: user.CreatedAt, UpdatedAt: user.UpdatedAt, - Suspended: user.Suspended, MFAEnrolled: user.MFAEnrolled, LockedAt: user.LockedAt, CreatedByUserID: user.CreatedByUserID, diff --git a/internal/gateway/handlers/api_handler_env.go b/internal/gateway/handlers/api_handler_env.go index 86b68631..cb7b6f7d 100644 --- a/internal/gateway/handlers/api_handler_env.go +++ b/internal/gateway/handlers/api_handler_env.go @@ -55,7 +55,7 @@ func (h *APIHandler) logEnvUpdateDiffs( _ = h.auditLogger.LogRequestEntry(c.Request.Context(), &db.AuditLog{ UserEmail: email, UserName: name, - ActionType: "convox", + ActionType: audit.ActionTypeConvox, Action: action, ResourceType: resourceType, Resource: fmt.Sprintf("%s/%s", app, diff.Key), @@ -172,7 +172,7 @@ func (h *APIHandler) logSecretAccessDenied( _ = h.auditLogger.LogRequestEntry(c.Request.Context(), &db.AuditLog{ UserEmail: email, UserName: name, - ActionType: "convox", + ActionType: audit.ActionTypeConvox, Action: audit.BuildAction(rbac.ResourceSecret.String(), rbac.ActionRead.String()), ResourceType: "secret", Resource: res, @@ -224,7 +224,7 @@ func (h *APIHandler) logEnvRead( _ = h.auditLogger.LogRequestEntry(c.Request.Context(), &db.AuditLog{ UserEmail: email, UserName: name, - ActionType: "convox", + ActionType: audit.ActionTypeConvox, Action: action, ResourceType: resourceType, Resource: resource, @@ -403,7 +403,7 @@ func (h *APIHandler) respondNoEnvChanges(c *gin.Context, ctx *envUpdateContext, _ = h.auditLogger.LogRequestEntry(c.Request.Context(), &db.AuditLog{ UserEmail: ctx.email, UserName: ctx.name, - ActionType: "convox", + ActionType: audit.ActionTypeConvox, Action: audit.BuildAction(rbac.ResourceEnv.String(), rbac.ActionUpdate.String()), ResourceType: "env", Resource: ctx.app, diff --git a/internal/gateway/handlers/auth_helpers.go b/internal/gateway/handlers/auth_helpers.go index 9905be30..6dd1a894 100644 --- a/internal/gateway/handlers/auth_helpers.go +++ b/internal/gateway/handlers/auth_helpers.go @@ -108,7 +108,7 @@ func (h *AuthHandler) auditLogin(c *gin.Context, resource, status string) bool { return true } err := h.auditLogger.LogDBEntry(&db.AuditLog{ - ActionType: "auth", + ActionType: audit.ActionTypeAuth, Action: audit.BuildAction(audit.ActionScopeLogin, rbac.ActionStart.String()), ResourceType: "auth", Resource: resource, @@ -426,7 +426,7 @@ func (h *AuthHandler) auditMFAUpdate(c *gin.Context, user *db.User, methodID int if err := h.auditLogger.LogDBEntry(&db.AuditLog{ UserEmail: user.Email, UserName: user.Name, - ActionType: "auth", + ActionType: audit.ActionTypeAuth, Action: audit.BuildAction(audit.ActionScopeMFAPreferences, rbac.ActionUpdate.String()), ResourceType: "mfa_method", Resource: fmt.Sprintf("%d", methodID), diff --git a/internal/gateway/handlers/auth_mfa_audit.go b/internal/gateway/handlers/auth_mfa_audit.go index 69ece3f1..2cd0b4ba 100644 --- a/internal/gateway/handlers/auth_mfa_audit.go +++ b/internal/gateway/handlers/auth_mfa_audit.go @@ -29,7 +29,7 @@ func (h *AuthHandler) auditMFAEvent(c *gin.Context, user *db.User, event mfaAudi entry := &db.AuditLog{ UserEmail: user.Email, UserName: user.Name, - ActionType: "auth", + ActionType: audit.ActionTypeAuth, Action: action, ResourceType: event.resourceType, Resource: event.resource, diff --git a/internal/gateway/handlers/deploy_approval_helpers.go b/internal/gateway/handlers/deploy_approval_helpers.go index 508f6a0f..794866e3 100644 --- a/internal/gateway/handlers/deploy_approval_helpers.go +++ b/internal/gateway/handlers/deploy_approval_helpers.go @@ -326,7 +326,7 @@ func logDeployApprovalAudit( _ = logger.LogRequestEntry(ctx, &db.AuditLog{ UserEmail: userEmail, UserName: userName, - ActionType: "gateway", + ActionType: audit.ActionTypeGateway, Action: action, ResourceType: "deploy-approval-request", Resource: resourceID, diff --git a/internal/gateway/jobs/audit/anchor_latest_test.go b/internal/gateway/jobs/audit/anchor_latest_test.go index 6fff3b57..4ccfe00c 100644 --- a/internal/gateway/jobs/audit/anchor_latest_test.go +++ b/internal/gateway/jobs/audit/anchor_latest_test.go @@ -43,7 +43,7 @@ func sha256Hex(data []byte) string { } func newTestWorker(database *db.Database, s3Client S3Client) *AnchorWriterWorker { - return NewAnchorWriterWorker(database, s3Client, "test-bucket", testChainID, 7) + return NewAnchorWriterWorker(database, s3Client, "test-bucket", testChainID) } func anchorJob(at time.Time) *river.Job[AnchorWriterArgs] { diff --git a/internal/gateway/jobs/audit/anchor_window_test.go b/internal/gateway/jobs/audit/anchor_window_test.go index efb2eff6..79826dad 100644 --- a/internal/gateway/jobs/audit/anchor_window_test.go +++ b/internal/gateway/jobs/audit/anchor_window_test.go @@ -52,8 +52,8 @@ func TestAnchorPutIsSentOnce(t *testing.T) { Credentials: aws.AnonymousCredentials{}, }) - err := newTestWorker(nil, client).putObjectWithRetention( - context.Background(), AnchorKey(testChainID, firstAnchorHour), []byte(`{}`), firstAnchorHour, nil, + err := newTestWorker(nil, client).putAnchorObject( + context.Background(), AnchorKey(testChainID, firstAnchorHour), []byte(`{}`), nil, ) require.Error(t, err) assert.Equal(t, int32(1), puts.Load(), "the SDK must not resend the anchor; the job's retry checks for it first") diff --git a/internal/gateway/jobs/audit/anchor_writer.go b/internal/gateway/jobs/audit/anchor_writer.go index 1791a565..0a6c5291 100644 --- a/internal/gateway/jobs/audit/anchor_writer.go +++ b/internal/gateway/jobs/audit/anchor_writer.go @@ -27,30 +27,19 @@ type AnchorWriterArgs struct{} // Kind returns the unique identifier for this job type func (AnchorWriterArgs) Kind() string { return "audit:anchor_writer" } -// AnchorWriterWorker writes audit log chain anchors to S3 WORM bucket +// AnchorWriterWorker writes audit log chain anchors to the S3 Object Lock bucket. The bucket's default COMPLIANCE +// retention locks each anchor (the gateway checks it at startup and in every verification); the writer sets none. type AnchorWriterWorker struct { river.WorkerDefaults[AnchorWriterArgs] - db *db.Database - s3Client S3Client - bucket string - chainID string - retentionDays int + db *db.Database + s3Client S3Client + bucket string + chainID string } // NewAnchorWriterWorker creates a new anchor writer worker -func NewAnchorWriterWorker( - database *db.Database, - s3Client S3Client, - bucket, chainID string, - retentionDays int, -) *AnchorWriterWorker { - return &AnchorWriterWorker{ - db: database, - s3Client: s3Client, - bucket: bucket, - chainID: chainID, - retentionDays: retentionDays, - } +func NewAnchorWriterWorker(database *db.Database, s3Client S3Client, bucket, chainID string) *AnchorWriterWorker { + return &AnchorWriterWorker{db: database, s3Client: s3Client, bucket: bucket, chainID: chainID} } // AnchorFormat is the format of the anchors this version writes. A format 2 anchor's prev_anchor_hash always @@ -181,12 +170,9 @@ func (w *AnchorWriterWorker) writeAnchorLocked(ctx context.Context, lock *db.Aud hash := sha256.Sum256(canonicalJSON) hashBase64 := base64.StdEncoding.EncodeToString(hash[:]) - // Calculate retention date - retainUntil := timestamp.Add(time.Duration(w.retentionDays) * 24 * time.Hour) - - // Write JSON to S3 with Object Lock and checksum - // The SHA256 checksum is stored in S3 metadata, so no separate .sha256 file is needed - if err := w.putObjectWithRetention(ctx, key, canonicalJSON, retainUntil, &hashBase64); err != nil { + // Write JSON to S3 with its checksum. The SHA256 checksum is stored in S3 metadata, so no separate .sha256 file + // is needed. + if err := w.putAnchorObject(ctx, key, canonicalJSON, &hashBase64); err != nil { return fmt.Errorf("failed to write anchor JSON: %w", err) } @@ -212,14 +198,12 @@ func newAnchorPayload(chainID string, ts time.Time, head db.AuditChainHead, prev return anchor } -// putObjectWithRetention writes an object to S3 with appropriate retention and encryption settings. -// For AWS S3: uses KMS encryption. Object Lock retention is applied via bucket default retention. -// For MinIO: relies on bucket-level default retention. -func (w *AnchorWriterWorker) putObjectWithRetention( +// putAnchorObject writes an anchor to S3. On AWS it uses KMS encryption. Both AWS and MinIO lock it with the +// bucket's default Object Lock retention. +func (w *AnchorWriterWorker) putAnchorObject( ctx context.Context, key string, data []byte, - retainUntil time.Time, checksumSHA256 *string, ) error { putInput := &s3.PutObjectInput{ @@ -244,8 +228,8 @@ func (w *AnchorWriterWorker) putObjectWithRetention( if err != nil { // Log detailed error information for debugging 400 errors return fmt.Errorf( - "S3 PutObject failed (bucket=%s, key=%s, size=%d, useAWS=%t, hasChecksum=%t, retainUntil=%s): %w", - w.bucket, key, len(data), useAWS, checksumSHA256 != nil, retainUntil.Format(time.RFC3339), err, + "S3 PutObject failed (bucket=%s, key=%s, size=%d, useAWS=%t, hasChecksum=%t): %w", + w.bucket, key, len(data), useAWS, checksumSHA256 != nil, err, ) } return nil diff --git a/internal/gateway/jobs/audit/anchor_writer_test.go b/internal/gateway/jobs/audit/anchor_writer_test.go index 1a190692..87f3020d 100644 --- a/internal/gateway/jobs/audit/anchor_writer_test.go +++ b/internal/gateway/jobs/audit/anchor_writer_test.go @@ -98,14 +98,12 @@ func TestNewAnchorWriterWorker(t *testing.T) { s3Client := &mockS3Client{} bucket := "test-bucket" chainID := "staging" - retentionDays := 400 - worker := NewAnchorWriterWorker(nil, s3Client, bucket, chainID, retentionDays) + worker := NewAnchorWriterWorker(nil, s3Client, bucket, chainID) require.NotNil(t, worker) assert.Equal(t, bucket, worker.bucket) assert.Equal(t, chainID, worker.chainID) - assert.Equal(t, retentionDays, worker.retentionDays) } func TestAnchorWriterWorker_Work_BothFilesMissing_EmptyChain(t *testing.T) { @@ -156,11 +154,10 @@ func TestAnchorWriterWorker_Work_BothFilesMissing_EmptyChain(t *testing.T) { } worker := &AnchorWriterWorker{ - db: database, - s3Client: s3Client, - bucket: "test-bucket", - chainID: "staging", - retentionDays: 7, + db: database, + s3Client: s3Client, + bucket: "test-bucket", + chainID: "staging", } job := &river.Job[AnchorWriterArgs]{ @@ -224,11 +221,10 @@ func TestAnchorWriterWorker_Work_PutJSONFails(t *testing.T) { } worker := &AnchorWriterWorker{ - db: database, - s3Client: s3Client, - bucket: "test-bucket", - chainID: "staging", - retentionDays: 7, + db: database, + s3Client: s3Client, + bucket: "test-bucket", + chainID: "staging", } job := &river.Job[AnchorWriterArgs]{ @@ -453,41 +449,6 @@ func TestAnchorWriterWorker_getLatestAnchor_GetObjectError(t *testing.T) { assert.Equal(t, "", hash) } -func TestAnchorWriterWorker_RetentionDateCalculation(t *testing.T) { - testCases := []struct { - name string - retentionDays int - timestamp time.Time - expected time.Time - }{ - { - name: "7 days retention", - retentionDays: 7, - timestamp: time.Date(2025, 11, 1, 0, 0, 0, 0, time.UTC), - expected: time.Date(2025, 11, 8, 0, 0, 0, 0, time.UTC), - }, - { - name: "400 days retention", - retentionDays: 400, - timestamp: time.Date(2025, 11, 1, 0, 0, 0, 0, time.UTC), - expected: time.Date(2026, 12, 6, 0, 0, 0, 0, time.UTC), - }, - { - name: "1 day retention", - retentionDays: 1, - timestamp: time.Date(2025, 11, 1, 12, 0, 0, 0, time.UTC), - expected: time.Date(2025, 11, 2, 12, 0, 0, 0, time.UTC), - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - retainUntil := tc.timestamp.Add(time.Duration(tc.retentionDays) * 24 * time.Hour) - assert.Equal(t, tc.expected, retainUntil) - }) - } -} - func TestAnchorWriterWorker_S3KeyPrefix(t *testing.T) { testCases := []struct { name string @@ -583,11 +544,10 @@ func TestAnchorWriterWorker_ObjectLockParameters(t *testing.T) { } worker := &AnchorWriterWorker{ - db: database, - s3Client: s3Client, - bucket: "test-bucket", - chainID: "staging", - retentionDays: 7, + db: database, + s3Client: s3Client, + bucket: "test-bucket", + chainID: "staging", } job := &river.Job[AnchorWriterArgs]{ @@ -659,11 +619,10 @@ func TestAnchorWriterWorker_NoIfNoneMatch(t *testing.T) { } worker := &AnchorWriterWorker{ - db: database, - s3Client: s3Client, - bucket: "test-bucket", - chainID: "staging", - retentionDays: 7, + db: database, + s3Client: s3Client, + bucket: "test-bucket", + chainID: "staging", } job := &river.Job[AnchorWriterArgs]{ diff --git a/internal/gateway/jobs/audit_jobs.go b/internal/gateway/jobs/audit_jobs.go index ff7e6a40..8570438c 100644 --- a/internal/gateway/jobs/audit_jobs.go +++ b/internal/gateway/jobs/audit_jobs.go @@ -24,11 +24,19 @@ import ( // defaultAnchorInterval is how often the anchor writer runs unless AUDIT_ANCHOR_INTERVAL_MINUTES says otherwise. const defaultAnchorInterval = time.Hour +// defaultAnchorRetentionDays is AUDIT_ANCHOR_RETENTION_DAYS when it isn't set. +const defaultAnchorRetentionDays = 400 + +// bucketCheckTimeout bounds the startup check of the anchor bucket's Object Lock configuration. +const bucketCheckTimeout = 30 * time.Second + // AuditAnchorConfig says where audit anchors are written: an S3 bucket with Object Lock. type AuditAnchorConfig struct { - S3Client *s3.Client - Bucket string - ChainID string + S3Client *s3.Client + Bucket string + ChainID string + // RetentionDays is the least the bucket's default COMPLIANCE retention may be (AUDIT_ANCHOR_RETENTION_DAYS). + // The bucket's rule locks the anchors; the gateway only checks it. RetentionDays int // Interval is how often the anchor writer runs (AUDIT_ANCHOR_INTERVAL_MINUTES). Anchors are keyed by hour, so // it writes at most one an hour. @@ -47,11 +55,11 @@ func NewAuditAnchorConfigFromEnv() (*AuditAnchorConfig, error) { return nil, fmt.Errorf("AUDIT_ANCHOR_BUCKET is set but AUDIT_ANCHOR_CHAIN_ID is missing") } - retentionDays := 400 + retentionDays := defaultAnchorRetentionDays if value := os.Getenv("AUDIT_ANCHOR_RETENTION_DAYS"); value != "" { parsed, err := strconv.Atoi(value) - if err != nil { - return nil, fmt.Errorf("invalid AUDIT_ANCHOR_RETENTION_DAYS: %w", err) + if err != nil || parsed <= 0 { + return nil, fmt.Errorf("AUDIT_ANCHOR_RETENTION_DAYS must be a positive number of days (got %q)", value) } retentionDays = parsed } @@ -76,6 +84,14 @@ func NewAuditAnchorConfigFromEnv() (*AuditAnchorConfig, error) { }, nil } +// CheckBucketRetention refuses an anchor bucket that doesn't lock anchors in COMPLIANCE mode for at least +// RetentionDays (auditverify.CheckBucketRetention). The gateway checks it before it starts. +func (c *AuditAnchorConfig) CheckBucketRetention(ctx context.Context) error { + ctx, cancel := context.WithTimeout(ctx, bucketCheckTimeout) + defer cancel() + return auditverify.CheckBucketRetention(ctx, c.S3Client, c.Bucket, c.RetentionDays) +} + // anchorIntervalFromEnv returns AUDIT_ANCHOR_INTERVAL_MINUTES, or an hour when it isn't a positive number. func anchorIntervalFromEnv() time.Duration { if minutes, err := strconv.Atoi(os.Getenv("AUDIT_ANCHOR_INTERVAL_MINUTES")); err == nil && minutes > 0 { @@ -99,10 +115,11 @@ func AuditVerifyOptions(anchors *AuditAnchorConfig) (auditverify.Options, error) opts := auditverify.Options{Secret: secret, HashV2From: pinned} if anchors != nil { opts.Anchors = &auditverify.AnchorOptions{ - Store: anchors.S3Client, - Bucket: anchors.Bucket, - ChainID: anchors.ChainID, - MaxAge: maxAnchorAge(anchors.Interval), + Store: anchors.S3Client, + Bucket: anchors.Bucket, + ChainID: anchors.ChainID, + MaxAge: maxAnchorAge(anchors.Interval), + MinRetentionDays: anchors.RetentionDays, } } return opts, nil @@ -116,13 +133,8 @@ func maxAnchorAge(interval time.Duration) time.Duration { // anchorWriterJob registers the anchor writer, which runs when the gateway starts and every anchor interval. func anchorWriterJob(workers *river.Workers, deps *Dependencies, anchors *AuditAnchorConfig) *river.PeriodicJob { - river.AddWorker(workers, jobaudit.NewAnchorWriterWorker( - deps.Database, - anchors.S3Client, - anchors.Bucket, - anchors.ChainID, - anchors.RetentionDays, - )) + worker := jobaudit.NewAnchorWriterWorker(deps.Database, anchors.S3Client, anchors.Bucket, anchors.ChainID) + river.AddWorker(workers, worker) return river.NewPeriodicJob( river.PeriodicInterval(anchors.Interval), func() (river.JobArgs, *river.InsertOpts) { return jobaudit.AnchorWriterArgs{}, nil }, diff --git a/internal/gateway/jobs/client.go b/internal/gateway/jobs/client.go index 98a3c892..99cf322c 100644 --- a/internal/gateway/jobs/client.go +++ b/internal/gateway/jobs/client.go @@ -57,8 +57,6 @@ func NewClient(pool *pgxpool.Pool, deps *Dependencies, auditAnchorConfig *AuditA river.AddWorker(workers, jobemail.NewNewCLISessionWorker(deps.EmailSender)) river.AddWorker(workers, jobemail.NewRateLimitUserWorker(deps.EmailSender)) river.AddWorker(workers, jobemail.NewRateLimitAdminWorker(deps.EmailSender)) - river.AddWorker(workers, jobemail.NewSuspiciousActivityUserWorker(deps.EmailSender)) - river.AddWorker(workers, jobemail.NewSuspiciousActivityAdminWorker(deps.EmailSender)) // Email workers - user management river.AddWorker(workers, jobemail.NewWelcomeWorker(deps.EmailSender)) diff --git a/internal/gateway/jobs/email/suspicious_activity.go b/internal/gateway/jobs/email/suspicious_activity.go deleted file mode 100644 index b34a5ea2..00000000 --- a/internal/gateway/jobs/email/suspicious_activity.go +++ /dev/null @@ -1,197 +0,0 @@ -package email - -import ( - "context" - "fmt" - "html" - "strings" - - "github.com/riverqueue/river" - - "github.com/DocSpring/rack-gateway/internal/gateway/email" -) - -// SuspiciousActivityUserArgs contains parameters for suspicious activity (user) email notification -type SuspiciousActivityUserArgs struct { - UserEmail string `json:"user_email"` - UserName string `json:"user_name"` - Reason string `json:"reason"` - IPAddress string `json:"ip_address"` - UserAgent string `json:"user_agent"` - Details map[string]string `json:"details"` -} - -// Kind returns the unique identifier for this job type -func (SuspiciousActivityUserArgs) Kind() string { return "email:security:suspicious_activity_user" } - -// SuspiciousActivityUserWorker sends suspicious activity (user) email notifications -type SuspiciousActivityUserWorker struct { - river.WorkerDefaults[SuspiciousActivityUserArgs] - emailSender email.Sender -} - -// NewSuspiciousActivityUserWorker creates a new suspicious activity user email worker -func NewSuspiciousActivityUserWorker(emailSender email.Sender) *SuspiciousActivityUserWorker { - return &SuspiciousActivityUserWorker{emailSender: emailSender} -} - -// Work sends the suspicious activity email to the user -func (w *SuspiciousActivityUserWorker) Work(_ context.Context, job *river.Job[SuspiciousActivityUserArgs]) error { - args := job.Args - - subject := "Suspicious Activity Detected" - - detailsText := formatDetailsAsText(args.Details) - text := fmt.Sprintf(`Hello %s, - -We detected suspicious activity on your account. - -Reason: %s - -Details: -- Time: %s -- IP Address: %s -- User Agent: %s -%s -If this wasn't you, please contact your administrator immediately and consider changing your password. - -This is an automated security notification from Rack Gateway.`, - args.UserName, - args.Reason, - job.CreatedAt.Format("2006-01-02 15:04:05 MST"), - args.IPAddress, - args.UserAgent, - detailsText, - ) - - detailsHTML := formatDetailsAsHTML(args.Details) - htmlBody := fmt.Sprintf(`

Hello %s,

-

We detected suspicious activity on your account.

-

Reason: %s

-

Details:

-
    -
  • Time: %s
    • -
  • IP Address: %s
    • -
  • User Agent: %s
    • -%s -
-

If this wasn't you, please contact your administrator immediately and consider changing your password.

-

This is an automated security notification from Rack Gateway.

`, - html.EscapeString(args.UserName), - html.EscapeString(args.Reason), - html.EscapeString(job.CreatedAt.Format("2006-01-02 15:04:05 MST")), - html.EscapeString(args.IPAddress), - html.EscapeString(args.UserAgent), - detailsHTML, - ) - - if err := w.emailSender.Send(args.UserEmail, subject, text, htmlBody); err != nil { - return fmt.Errorf("failed to send suspicious activity user email: %w", err) - } - - return nil -} - -// SuspiciousActivityAdminArgs contains parameters for suspicious activity (admin alert) email notification -type SuspiciousActivityAdminArgs struct { - AdminEmails []string `json:"admin_emails"` - UserEmail string `json:"user_email"` - UserName string `json:"user_name"` - Reason string `json:"reason"` - IPAddress string `json:"ip_address"` - UserAgent string `json:"user_agent"` - Details map[string]string `json:"details"` -} - -// Kind returns the unique identifier for this job type -func (SuspiciousActivityAdminArgs) Kind() string { return "email:security:suspicious_activity_admin" } - -// SuspiciousActivityAdminWorker sends suspicious activity (admin alert) email notifications -type SuspiciousActivityAdminWorker struct { - river.WorkerDefaults[SuspiciousActivityAdminArgs] - emailSender email.Sender -} - -// NewSuspiciousActivityAdminWorker creates a new suspicious activity admin email worker -func NewSuspiciousActivityAdminWorker(emailSender email.Sender) *SuspiciousActivityAdminWorker { - return &SuspiciousActivityAdminWorker{emailSender: emailSender} -} - -// Work sends the suspicious activity alert to admins -func (w *SuspiciousActivityAdminWorker) Work(_ context.Context, job *river.Job[SuspiciousActivityAdminArgs]) error { - args := job.Args - - subject := fmt.Sprintf("Suspicious Activity - User %s", args.UserEmail) - - detailsText := formatDetailsAsText(args.Details) - text := fmt.Sprintf(`Admin Alert: Suspicious activity detected. - -User: %s (%s) -Reason: %s - -Details: -- Time: %s -- IP Address: %s -- User Agent: %s -%s -This is an automated security notification from Rack Gateway.`, - args.UserEmail, - args.UserName, - args.Reason, - job.CreatedAt.Format("2006-01-02 15:04:05 MST"), - args.IPAddress, - args.UserAgent, - detailsText, - ) - - detailsHTML := formatDetailsAsHTML(args.Details) - htmlBody := fmt.Sprintf(`

Admin Alert: Suspicious activity detected.

-

User: %s (%s)

-

Reason: %s

-

Details:

-
    -
  • Time: %s
    • -
  • IP Address: %s
    • -
  • User Agent: %s
    • -%s -
-

This is an automated security notification from Rack Gateway.

`, - html.EscapeString(args.UserEmail), - html.EscapeString(args.UserName), - html.EscapeString(args.Reason), - html.EscapeString(job.CreatedAt.Format("2006-01-02 15:04:05 MST")), - html.EscapeString(args.IPAddress), - html.EscapeString(args.UserAgent), - detailsHTML, - ) - - if err := w.emailSender.SendMany(args.AdminEmails, subject, text, htmlBody); err != nil { - return fmt.Errorf("failed to send suspicious activity admin emails: %w", err) - } - - return nil -} - -// formatDetailsAsText formats additional details map as plain text list items -func formatDetailsAsText(details map[string]string) string { - if len(details) == 0 { - return "" - } - parts := make([]string, 0, len(details)) - for k, v := range details { - parts = append(parts, fmt.Sprintf("- %s: %s", k, v)) - } - return "\n" + strings.Join(parts, "\n") -} - -// formatDetailsAsHTML formats additional details map as HTML list items -func formatDetailsAsHTML(details map[string]string) string { - if len(details) == 0 { - return "" - } - parts := make([]string, 0, len(details)) - for k, v := range details { - parts = append(parts, fmt.Sprintf("
  • %s: %s
    • ", html.EscapeString(k), html.EscapeString(v))) - } - return strings.Join(parts, "\n") -} diff --git a/internal/gateway/jobs/email/suspicious_activity_test.go b/internal/gateway/jobs/email/suspicious_activity_test.go deleted file mode 100644 index dd478b7f..00000000 --- a/internal/gateway/jobs/email/suspicious_activity_test.go +++ /dev/null @@ -1,91 +0,0 @@ -package email - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// Test SuspiciousActivityUserArgs.Kind -func TestSuspiciousActivityUserArgs_Kind(t *testing.T) { - args := SuspiciousActivityUserArgs{ - UserEmail: "test@example.com", - UserName: "Test User", - Reason: "Multiple failed login attempts", - IPAddress: "192.168.1.1", - UserAgent: "Mozilla/5.0", - Details: map[string]string{"attempts": "5"}, - } - assert.Equal(t, "email:security:suspicious_activity_user", args.Kind()) -} - -// Test NewSuspiciousActivityUserWorker -func TestNewSuspiciousActivityUserWorker(t *testing.T) { - worker := NewSuspiciousActivityUserWorker(nil) - require.NotNil(t, worker) -} - -// Test SuspiciousActivityAdminArgs.Kind -func TestSuspiciousActivityAdminArgs_Kind(t *testing.T) { - args := SuspiciousActivityAdminArgs{ - AdminEmails: []string{"admin@example.com"}, - UserEmail: "test@example.com", - UserName: "Test User", - Reason: "Multiple failed login attempts", - IPAddress: "192.168.1.1", - UserAgent: "Mozilla/5.0", - Details: map[string]string{"attempts": "5"}, - } - assert.Equal(t, "email:security:suspicious_activity_admin", args.Kind()) -} - -// Test NewSuspiciousActivityAdminWorker -func TestNewSuspiciousActivityAdminWorker(t *testing.T) { - worker := NewSuspiciousActivityAdminWorker(nil) - require.NotNil(t, worker) -} - -// Test formatDetailsAsText - Empty -func TestFormatDetailsAsText_Empty(t *testing.T) { - result := formatDetailsAsText(nil) - assert.Equal(t, "", result) - - result = formatDetailsAsText(map[string]string{}) - assert.Equal(t, "", result) -} - -// Test formatDetailsAsText - With Data -func TestFormatDetailsAsText_WithData(t *testing.T) { - details := map[string]string{ - "attempts": "5", - "window": "10m", - } - result := formatDetailsAsText(details) - - assert.NotEmpty(t, result) - assert.Contains(t, result, "attempts: 5") - assert.Contains(t, result, "window: 10m") -} - -// Test formatDetailsAsHTML - Empty -func TestFormatDetailsAsHTML_Empty(t *testing.T) { - result := formatDetailsAsHTML(nil) - assert.Equal(t, "", result) - - result = formatDetailsAsHTML(map[string]string{}) - assert.Equal(t, "", result) -} - -// Test formatDetailsAsHTML - With Data -func TestFormatDetailsAsHTML_WithData(t *testing.T) { - details := map[string]string{ - "attempts": "5", - "window": "10m", - } - result := formatDetailsAsHTML(details) - - assert.NotEmpty(t, result) - assert.Contains(t, result, "
  • attempts: 5
    • ") - assert.Contains(t, result, "
  • window: 10m
    • ") -} diff --git a/internal/gateway/jobs/verifyaudit/anchor_cache.go b/internal/gateway/jobs/verifyaudit/anchor_cache.go index 2079433b..ece44011 100644 --- a/internal/gateway/jobs/verifyaudit/anchor_cache.go +++ b/internal/gateway/jobs/verifyaudit/anchor_cache.go @@ -36,6 +36,15 @@ func newAnchorCache(store auditverify.AnchorStore) *anchorCache { return &anchorCache{store: store, bodies: map[anchorVersion][]byte{}} } +// GetObjectLockConfiguration always reads the bucket's configuration. +func (c *anchorCache) GetObjectLockConfiguration( + ctx context.Context, + params *s3.GetObjectLockConfigurationInput, + optFns ...func(*s3.Options), +) (*s3.GetObjectLockConfigurationOutput, error) { + return c.store.GetObjectLockConfiguration(ctx, params, optFns...) +} + // ListObjectVersions always lists the bucket. func (c *anchorCache) ListObjectVersions( ctx context.Context, diff --git a/internal/gateway/jobs/verifyaudit/helpers_test.go b/internal/gateway/jobs/verifyaudit/helpers_test.go index f7b2aa79..f99f3378 100644 --- a/internal/gateway/jobs/verifyaudit/helpers_test.go +++ b/internal/gateway/jobs/verifyaudit/helpers_test.go @@ -129,7 +129,7 @@ func (h *harness) head() int64 { func (h *harness) writeAnchor() { h.t.Helper() h.bucket.AdvanceTo(h.now) - writer := jobaudit.NewAnchorWriterWorker(h.database, h.bucket, testBucket, testChainID, 7) + writer := jobaudit.NewAnchorWriterWorker(h.database, h.bucket, testBucket, testChainID) require.NoError(h.t, writer.WriteAnchor(context.Background(), h.now)) } diff --git a/internal/gateway/middleware/authorize.go b/internal/gateway/middleware/authorize.go index d3bbd70b..93a581dc 100644 --- a/internal/gateway/middleware/authorize.go +++ b/internal/gateway/middleware/authorize.go @@ -84,13 +84,13 @@ func routeAllowed(manager rbac.Manager, spec rbac.RouteSpec, principal rbac.Prin // isOwnAccount reports whether a human caller is using a self-service route for their own account. // The path parameter must match the caller's email exactly (handlers look users up by exact email). -// API tokens, and suspended or locked users (whom RBAC also denies), never qualify. +// API tokens, and locked users (whom RBAC also denies), never qualify. func isOwnAccount(c *gin.Context, spec rbac.RouteSpec, authUser *auth.User) bool { if spec.SelfParam == "" || authUser.IsAPIToken { return false } user := authUser.DBUser - if user == nil || user.Suspended || user.LockedAt != nil { + if user == nil || user.LockedAt != nil { return false } target := strings.TrimSpace(c.Param(spec.SelfParam)) diff --git a/internal/gateway/openapi/generated/swagger.json b/internal/gateway/openapi/generated/swagger.json index f860ca33..499498b6 100644 --- a/internal/gateway/openapi/generated/swagger.json +++ b/internal/gateway/openapi/generated/swagger.json @@ -4460,7 +4460,7 @@ "type": "string" }, "action_type": { - "description": "\"convox\", \"users\", \"auth\"", + "description": "permission scope: auth, convox, gateway, security", "type": "string" }, "api_token_id": { @@ -4814,9 +4814,6 @@ "type": "string" } }, - "suspended": { - "type": "boolean" - }, "updated_at": { "type": "string" } diff --git a/internal/gateway/proxy/audit.go b/internal/gateway/proxy/audit.go index c1453ef5..8d32c77f 100644 --- a/internal/gateway/proxy/audit.go +++ b/internal/gateway/proxy/audit.go @@ -72,7 +72,7 @@ func (h *Handler) auditReleaseCreations( UserName: r.Header.Get("X-User-Name"), APITokenID: headerTokenID(r), APITokenName: strings.TrimSpace(r.Header.Get("X-API-Token-Name")), - ActionType: "convox", + ActionType: audit.ActionTypeConvox, Action: audit.BuildAction(rbac.ResourceRelease.String(), rbac.ActionCreate.String()), ResourceType: "release", Resource: rel, @@ -179,7 +179,7 @@ func (h *Handler) requestAuditEntry(r *http.Request, authUser *auth.User, path s UserName: r.Header.Get("X-User-Name"), APITokenID: headerTokenID(r), APITokenName: strings.TrimSpace(r.Header.Get("X-API-Token-Name")), - ActionType: "convox", + ActionType: audit.ActionTypeConvox, Action: action, Resource: resource, ResourceType: h.auditLogger.InferResourceType(r.URL.Path, action), diff --git a/internal/gateway/proxy/env.go b/internal/gateway/proxy/env.go index e2dbff1d..40fb6de0 100644 --- a/internal/gateway/proxy/env.go +++ b/internal/gateway/proxy/env.go @@ -30,7 +30,7 @@ func (h *Handler) logDeniedRBACAction( _ = h.logAudit(r, &db.AuditLog{ UserEmail: email, UserName: r.Header.Get("X-User-Name"), - ActionType: "convox", + ActionType: audit.ActionTypeConvox, Action: audit.BuildAction(resource.String(), action.String()), ResourceType: resource.String(), Resource: resourceName, @@ -337,7 +337,7 @@ func (h *Handler) logEnvDiffs(r *http.Request, email, _ string, diffs []envutil. _ = h.logAudit(r, &db.AuditLog{ UserEmail: email, UserName: userName, - ActionType: "convox", + ActionType: audit.ActionTypeConvox, Action: action, ResourceType: rtype, Resource: fmt.Sprintf("%s/%s", app, d.Key), diff --git a/internal/gateway/proxy/handler.go b/internal/gateway/proxy/handler.go index 5aa21f04..dfd332d3 100644 --- a/internal/gateway/proxy/handler.go +++ b/internal/gateway/proxy/handler.go @@ -28,20 +28,19 @@ import ( // Handler proxies requests through the gateway while enforcing RBAC, MFA, and // audit logging requirements. type Handler struct { - config *config.Config - rbacManager rbac.Manager - auditLogger *audit.Logger - secretNames map[string]struct{} - secretNamesErr error // CONVOX_SECRET_ENV_VARS can't be read: every env var counts as secret - database *db.Database - settingsService *settings.Service - allowDestructive bool - emailer email.Sender - rackName string - rackAlias string - rackCertManager *rackcert.Manager - mfaService *mfa.Service - sessionManager *auth.SessionManager + config *config.Config + rbacManager rbac.Manager + auditLogger *audit.Logger + secretNames map[string]struct{} + secretNamesErr error // CONVOX_SECRET_ENV_VARS can't be read: every env var counts as secret + database *db.Database + settingsService *settings.Service + emailer email.Sender + rackName string + rackAlias string + rackCertManager *rackcert.Manager + mfaService *mfa.Service + sessionManager *auth.SessionManager } // no async jobs client here; emailer may be an async sender @@ -62,19 +61,18 @@ func NewHandler( sessionManager *auth.SessionManager, ) *Handler { h := &Handler{ - config: cfg, - rbacManager: rbacManager, - auditLogger: auditLogger, - secretNames: make(map[string]struct{}), - database: database, - settingsService: settingsService, - allowDestructive: false, - emailer: mailer, - rackName: rackName, - rackAlias: strings.TrimSpace(rackAlias), - rackCertManager: rackCertManager, - mfaService: mfaService, - sessionManager: sessionManager, + config: cfg, + rbacManager: rbacManager, + auditLogger: auditLogger, + secretNames: make(map[string]struct{}), + database: database, + settingsService: settingsService, + emailer: mailer, + rackName: rackName, + rackAlias: strings.TrimSpace(rackAlias), + rackCertManager: rackCertManager, + mfaService: mfaService, + sessionManager: sessionManager, } h.secretNamesErr = h.loadSecretNames() // Settings are loaded on-demand from the settings service (no initialization needed) @@ -116,9 +114,6 @@ func (h *Handler) rackDisplay() string { return h.rackName } -// SetAllowDestructive toggles whether destructive Convox operations are allowed. -func (h *Handler) SetAllowDestructive(v bool) { h.allowDestructive = v } - // ProxyToRack handles proxying requests to the Convox rack with RBAC, MFA, and audit logging. func (h *Handler) ProxyToRack(w http.ResponseWriter, r *http.Request) { start := time.Now() diff --git a/internal/gateway/proxy/rack_params.go b/internal/gateway/proxy/rack_params.go index 7d3b8b93..c94f3e1c 100644 --- a/internal/gateway/proxy/rack_params.go +++ b/internal/gateway/proxy/rack_params.go @@ -121,7 +121,7 @@ func (h *Handler) auditRackParamsChanged(r *http.Request, actor string, keys []s _ = h.logAudit(r, &db.AuditLog{ UserEmail: actor, UserName: r.Header.Get("X-User-Name"), - ActionType: "convox", + ActionType: audit.ActionTypeConvox, Action: audit.BuildAction(rbac.ResourceRack.String(), audit.ActionVerbParamsSet), ResourceType: "rack", Resource: h.rackName, diff --git a/internal/gateway/proxy/websocket.go b/internal/gateway/proxy/websocket.go index 8ed754e4..b1fbc050 100644 --- a/internal/gateway/proxy/websocket.go +++ b/internal/gateway/proxy/websocket.go @@ -13,6 +13,7 @@ import ( "github.com/gorilla/websocket" + "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" @@ -395,7 +396,7 @@ func (h *Handler) logExecStart(r *http.Request, authUser *auth.User, path string UserName: r.Header.Get("X-User-Name"), APITokenID: tokenID, APITokenName: strings.TrimSpace(r.Header.Get("X-API-Token-Name")), - ActionType: "convox", + ActionType: audit.ActionTypeConvox, Action: "process.exec.start", Resource: resource, ResourceType: resourceType, diff --git a/internal/gateway/rbac/action_string.go b/internal/gateway/rbac/action_string.go index ebe40ff4..043fcd6c 100644 --- a/internal/gateway/rbac/action_string.go +++ b/internal/gateway/rbac/action_string.go @@ -8,35 +8,32 @@ func _() { // An "invalid array index" compiler error signifies that the constant values have changed. // Re-run the stringer command to generate them again. var x [1]struct{} - _ = x[ActionAdd-0] - _ = x[ActionApprove-1] - _ = x[ActionCreate-2] - _ = x[ActionDelete-3] - _ = x[ActionDeployWithApproval-4] - _ = x[ActionExec-5] - _ = x[ActionGenerate-6] - _ = x[ActionImport-7] - _ = x[ActionKeyroll-8] - _ = x[ActionList-9] - _ = x[ActionManage-10] - _ = x[ActionPromote-11] - _ = x[ActionRead-12] - _ = x[ActionRemove-13] - _ = x[ActionRestart-14] - _ = x[ActionSet-15] - _ = x[ActionStart-16] - _ = x[ActionStop-17] - _ = x[ActionTerminate-18] - _ = x[ActionUnset-19] - _ = x[ActionUpdate-20] - _ = x[ActionUpdateName-21] - _ = x[ActionRunPrivileged-22] - _ = x[ActionSetProtected-23] + _ = x[ActionApprove-0] + _ = x[ActionCreate-1] + _ = x[ActionDelete-2] + _ = x[ActionDeployWithApproval-3] + _ = x[ActionExec-4] + _ = x[ActionGenerate-5] + _ = x[ActionImport-6] + _ = x[ActionList-7] + _ = x[ActionManage-8] + _ = x[ActionPromote-9] + _ = x[ActionRead-10] + _ = x[ActionRestart-11] + _ = x[ActionSet-12] + _ = x[ActionStart-13] + _ = x[ActionStop-14] + _ = x[ActionTerminate-15] + _ = x[ActionUnset-16] + _ = x[ActionUpdate-17] + _ = x[ActionUpdateName-18] + _ = x[ActionRunPrivileged-19] + _ = x[ActionSetProtected-20] } -const _Action_name = "addapprovecreatedeletedeploy_with_approvalexecgenerateimportkeyrolllistmanagepromotereadremoverestartsetstartstopterminateunsetupdateupdate_namerun_privilegedset_protected" +const _Action_name = "approvecreatedeletedeploy_with_approvalexecgenerateimportlistmanagepromotereadrestartsetstartstopterminateunsetupdateupdate_namerun_privilegedset_protected" -var _Action_index = [...]uint8{0, 3, 10, 16, 22, 42, 46, 54, 60, 67, 71, 77, 84, 88, 94, 101, 104, 109, 113, 122, 127, 133, 144, 158, 171} +var _Action_index = [...]uint8{0, 7, 13, 19, 39, 43, 51, 57, 61, 67, 74, 78, 85, 88, 93, 97, 106, 111, 117, 128, 142, 155} func (i Action) String() string { idx := int(i) - 0 diff --git a/internal/gateway/rbac/constants.go b/internal/gateway/rbac/constants.go index 227361c2..f294b1b1 100644 --- a/internal/gateway/rbac/constants.go +++ b/internal/gateway/rbac/constants.go @@ -93,8 +93,6 @@ const ( ResourceApp Resource = iota // app // ResourceBuild identifies a Convox build resource. ResourceBuild // build - // ResourceCert identifies a Convox certificate resource. - ResourceCert // cert // ResourceDeploy identifies a Convox deploy resource. ResourceDeploy // deploy // ResourceEnv identifies a Convox environment resource. @@ -109,12 +107,8 @@ const ( ResourceProcess // process // ResourceRack identifies a Convox rack resource. ResourceRack // rack - // ResourceRegistry identifies a Convox registry resource. - ResourceRegistry // registry // ResourceRelease identifies a Convox release resource. ResourceRelease // release - // ResourceResource identifies a generic Convox resource. - ResourceResource // resource // ResourceAPIToken identifies a gateway API token resource. ResourceAPIToken // api_token // ResourceDeployApprovalRequest identifies a deploy approval request. @@ -201,10 +195,8 @@ func (r *Resource) UnmarshalJSON(data []byte) error { type Action uint8 const ( - // ActionAdd represents an add operation on a resource. - ActionAdd Action = iota // add // ActionApprove represents an approval operation. - ActionApprove // approve + ActionApprove Action = iota // approve // ActionCreate represents a creation operation. ActionCreate // create // ActionDelete represents a deletion operation. @@ -217,8 +209,6 @@ const ( ActionGenerate // generate // ActionImport represents an import operation. ActionImport // import - // ActionKeyroll represents a key rotation operation. - ActionKeyroll // keyroll // ActionList represents listing resources. ActionList // list // ActionManage represents management operations. @@ -227,8 +217,6 @@ const ( ActionPromote // promote // ActionRead represents read-only access. ActionRead // read - // ActionRemove represents removing associations. - ActionRemove // remove // ActionRestart signifies restarting a resource. ActionRestart // restart // ActionSet represents setting a value. @@ -259,7 +247,7 @@ func (a Action) IsValid() bool { return a <= ActionSetProtected } // ParseAction converts a string name into an Action value. func ParseAction(v string) (Action, error) { // Try each known value - for a := ActionAdd; a <= ActionSetProtected; a++ { + for a := ActionApprove; a <= ActionSetProtected; a++ { if a.String() == v { return a, nil } diff --git a/internal/gateway/rbac/mfa_coverage_test.go b/internal/gateway/rbac/mfa_coverage_test.go index 189f1fac..f544138b 100644 --- a/internal/gateway/rbac/mfa_coverage_test.go +++ b/internal/gateway/rbac/mfa_coverage_test.go @@ -41,23 +41,15 @@ func TestEveryWritePermissionHasAnMFALevel(t *testing.T) { } } -// Rack operations that replace credentials or certificates, or skip the checks a build gets, need MFA on the request. -// The other rack writes need it within the step-up window. +// Rack operations that delete an app or skip the checks a build gets need MFA on the request. The other rack writes +// need it within the step-up window. func TestRackWriteMFALevels(t *testing.T) { cases := map[string]rbac.MFALevel{ - rbac.Convox(rbac.ResourceBuild, rbac.ActionImport): rbac.MFAAlways, - rbac.Convox(rbac.ResourceInstance, rbac.ActionKeyroll): rbac.MFAAlways, - rbac.Convox(rbac.ResourceCert, rbac.ActionDelete): rbac.MFAAlways, - rbac.Convox(rbac.ResourceBuild, rbac.ActionUpdate): rbac.MFAStepUp, - rbac.Convox(rbac.ResourceCert, rbac.ActionUpdate): rbac.MFAStepUp, - rbac.Convox(rbac.ResourceCert, rbac.ActionGenerate): rbac.MFAStepUp, - rbac.Convox(rbac.ResourceCert, rbac.ActionImport): rbac.MFAStepUp, - rbac.Convox(rbac.ResourceRegistry, rbac.ActionAdd): rbac.MFAStepUp, - rbac.Convox(rbac.ResourceRegistry, rbac.ActionRemove): rbac.MFAStepUp, - rbac.Convox(rbac.ResourceBuild, rbac.ActionCreate): rbac.MFAStepUp, - rbac.Convox(rbac.ResourceObject, rbac.ActionCreate): rbac.MFANone, - rbac.Convox(rbac.ResourceApp, rbac.ActionDelete): rbac.MFAAlways, - rbac.Convox(rbac.ResourceInstance, rbac.ActionTerminate): rbac.MFAAlways, + rbac.Convox(rbac.ResourceBuild, rbac.ActionImport): rbac.MFAAlways, + rbac.Convox(rbac.ResourceBuild, rbac.ActionUpdate): rbac.MFAStepUp, + rbac.Convox(rbac.ResourceBuild, rbac.ActionCreate): rbac.MFAStepUp, + rbac.Convox(rbac.ResourceObject, rbac.ActionCreate): rbac.MFANone, + rbac.Convox(rbac.ResourceApp, rbac.ActionDelete): rbac.MFAAlways, } for perm, want := range cases { if got := rbac.MFALevelForPermission(perm); got != want { diff --git a/internal/gateway/rbac/mfa_requirements.go b/internal/gateway/rbac/mfa_requirements.go index 1d60008f..3454dcb8 100644 --- a/internal/gateway/rbac/mfa_requirements.go +++ b/internal/gateway/rbac/mfa_requirements.go @@ -31,10 +31,7 @@ var MFARequirements = map[string]MFALevel{ // ======================================================================== // DESTRUCTIVE OPERATIONS - Always require fresh MFA, no time window - Convox(ResourceApp, ActionDelete): MFAAlways, - Convox(ResourceInstance, ActionTerminate): MFAAlways, - Convox(ResourceInstance, ActionKeyroll): MFAAlways, // Replaces every instance's SSH key pair - Convox(ResourceCert, ActionDelete): MFAAlways, + Convox(ResourceApp, ActionDelete): MFAAlways, // Importing a build skips the manifest and image checks a build gets: its manifest and images come straight // from the archive. Admins only (convox:*:*), and rare. @@ -66,13 +63,6 @@ var MFARequirements = map[string]MFALevel{ // Rack operations Convox(ResourceRack, ActionUpdate): MFAStepUp, - // TLS certificates and private registries (what serves the apps' traffic, and where their images come from) - Convox(ResourceCert, ActionUpdate): MFAStepUp, - Convox(ResourceCert, ActionGenerate): MFAStepUp, - Convox(ResourceCert, ActionImport): MFAStepUp, - Convox(ResourceRegistry, ActionAdd): MFAStepUp, - Convox(ResourceRegistry, ActionRemove): MFAStepUp, - // Object uploads for builds (write operation but low risk) Convox(ResourceObject, ActionCreate): MFANone, diff --git a/internal/gateway/rbac/mfa_requirements_test.go b/internal/gateway/rbac/mfa_requirements_test.go index d395521b..b3602196 100644 --- a/internal/gateway/rbac/mfa_requirements_test.go +++ b/internal/gateway/rbac/mfa_requirements_test.go @@ -46,7 +46,6 @@ func TestMFARequirements_ConvoxPermissions(t *testing.T) { cases := []singlePermissionCase{ // Destructive operations - ALWAYS {Convox(ResourceApp, ActionDelete), MFAAlways}, - {Convox(ResourceInstance, ActionTerminate), MFAAlways}, // Sensitive operations - Step-up {Convox(ResourceApp, ActionCreate), MFAStepUp}, diff --git a/internal/gateway/rbac/rbac.go b/internal/gateway/rbac/rbac.go index 2ef84326..4217dda1 100644 --- a/internal/gateway/rbac/rbac.go +++ b/internal/gateway/rbac/rbac.go @@ -78,10 +78,10 @@ func NewDBManager(database *db.Database, domain string) (*DBManager, error) { // // Human users are checked against the roles on their current database record, so role changes take // effect on the next request. API tokens must hold the permission themselves AND their owner's -// current roles must allow it. Suspended or locked users (and tokens they own) are always denied. +// current roles must allow it. Locked users (and tokens they own) are always denied. func (m *DBManager) Authorize(p Principal, permission string) (bool, error) { user := p.User - if user == nil || user.Suspended || user.LockedAt != nil { + if user == nil || user.LockedAt != nil { return false, nil } if p.APIToken && !matchesAnyPermission(p.TokenPermissions, permission) { diff --git a/internal/gateway/rbac/rbac_test.go b/internal/gateway/rbac/rbac_test.go index c9d7bcc5..f061d371 100644 --- a/internal/gateway/rbac/rbac_test.go +++ b/internal/gateway/rbac/rbac_test.go @@ -104,16 +104,14 @@ func TestAuthorizeUsesCurrentRoles(t *testing.T) { require.False(t, userCan(t, mgr, database, "user@test.com", deleteApps), "re-created user must not keep old roles") } -// TestAuthorizeDeniesSuspendedAndLockedUsers verifies blocked users get nothing. -func TestAuthorizeDeniesSuspendedAndLockedUsers(t *testing.T) { +// TestAuthorizeDeniesLockedUsers verifies locked users, and their tokens, get nothing. +func TestAuthorizeDeniesLockedUsers(t *testing.T) { mgr, _ := newTestManager(t) now := time.Now() locked := &db.User{Email: "locked@test.com", Roles: []string{"admin"}, LockedAt: &now} - suspended := &db.User{Email: "suspended@test.com", Roles: []string{"admin"}, Suspended: true} for _, p := range []Principal{ UserPrincipal(locked), - UserPrincipal(suspended), TokenPrincipal(locked, []string{"convox:*:*"}), UserPrincipal(nil), } { diff --git a/internal/gateway/rbac/resource_string.go b/internal/gateway/rbac/resource_string.go index 3a93ed78..82469b26 100644 --- a/internal/gateway/rbac/resource_string.go +++ b/internal/gateway/rbac/resource_string.go @@ -10,36 +10,33 @@ func _() { var x [1]struct{} _ = x[ResourceApp-0] _ = x[ResourceBuild-1] - _ = x[ResourceCert-2] - _ = x[ResourceDeploy-3] - _ = x[ResourceEnv-4] - _ = x[ResourceInstance-5] - _ = x[ResourceLog-6] - _ = x[ResourceObject-7] - _ = x[ResourceProcess-8] - _ = x[ResourceRack-9] - _ = x[ResourceRegistry-10] - _ = x[ResourceRelease-11] - _ = x[ResourceResource-12] - _ = x[ResourceAPIToken-13] - _ = x[ResourceDeployApprovalRequest-14] - _ = x[ResourceIntegration-15] - _ = x[ResourceJob-16] - _ = x[ResourceSecret-17] - _ = x[ResourceSetting-18] - _ = x[ResourceUser-19] - _ = x[ResourceAuth-20] - _ = x[ResourceMFABackupCodes-21] - _ = x[ResourceMFAMethod-22] - _ = x[ResourceMFAPreferences-23] - _ = x[ResourceMFAVerification-24] - _ = x[ResourceTrustedDevice-25] - _ = x[ResourceAuditLog-26] + _ = x[ResourceDeploy-2] + _ = x[ResourceEnv-3] + _ = x[ResourceInstance-4] + _ = x[ResourceLog-5] + _ = x[ResourceObject-6] + _ = x[ResourceProcess-7] + _ = x[ResourceRack-8] + _ = x[ResourceRelease-9] + _ = x[ResourceAPIToken-10] + _ = x[ResourceDeployApprovalRequest-11] + _ = x[ResourceIntegration-12] + _ = x[ResourceJob-13] + _ = x[ResourceSecret-14] + _ = x[ResourceSetting-15] + _ = x[ResourceUser-16] + _ = x[ResourceAuth-17] + _ = x[ResourceMFABackupCodes-18] + _ = x[ResourceMFAMethod-19] + _ = x[ResourceMFAPreferences-20] + _ = x[ResourceMFAVerification-21] + _ = x[ResourceTrustedDevice-22] + _ = x[ResourceAuditLog-23] } -const _Resource_name = "appbuildcertdeployenvinstancelogobjectprocessrackregistryreleaseresourceapi_tokendeploy_approval_requestintegrationjobsecretsettinguserauthmfa_backup_codesmfa_methodmfa_preferencesmfa_verificationtrusted_deviceaudit_log" +const _Resource_name = "appbuilddeployenvinstancelogobjectprocessrackreleaseapi_tokendeploy_approval_requestintegrationjobsecretsettinguserauthmfa_backup_codesmfa_methodmfa_preferencesmfa_verificationtrusted_deviceaudit_log" -var _Resource_index = [...]uint8{0, 3, 8, 12, 18, 21, 29, 32, 38, 45, 49, 57, 64, 72, 81, 104, 115, 118, 124, 131, 135, 139, 155, 165, 180, 196, 210, 219} +var _Resource_index = [...]uint8{0, 3, 8, 14, 17, 25, 28, 34, 41, 45, 52, 61, 84, 95, 98, 104, 111, 115, 119, 135, 145, 160, 176, 190, 199} func (i Resource) String() string { idx := int(i) - 0 diff --git a/internal/gateway/security/notifier.go b/internal/gateway/security/notifier.go index af5fed9a..16694caa 100644 --- a/internal/gateway/security/notifier.go +++ b/internal/gateway/security/notifier.go @@ -13,7 +13,6 @@ import ( "github.com/DocSpring/rack-gateway/internal/gateway/audit" "github.com/DocSpring/rack-gateway/internal/gateway/db" - "github.com/DocSpring/rack-gateway/internal/gateway/email" "github.com/DocSpring/rack-gateway/internal/gateway/jobs" jobemail "github.com/DocSpring/rack-gateway/internal/gateway/jobs/email" "github.com/DocSpring/rack-gateway/internal/gateway/rbac" @@ -21,11 +20,9 @@ import ( // Notifier handles security event notifications via audit logs and email type Notifier struct { - emailSender email.Sender - auditLogger *audit.Logger - database *db.Database - emailsEnabled bool - jobsClient *jobs.Client + auditLogger *audit.Logger + database *db.Database + jobsClient *jobs.Client // Email rate limiting and deduplication mu sync.Mutex @@ -35,27 +32,11 @@ type Notifier struct { stopCleanup chan struct{} } -// NewNotifier creates a new security notifier -func NewNotifier( - emailSender email.Sender, - auditLogger *audit.Logger, - database *db.Database, - jobsClient *jobs.Client, -) *Notifier { - // Check if email sender is actually configured (not noop) - emailsEnabled := false - if emailSender != nil { - // NoopSender is not useful for notifications - if _, isNoop := emailSender.(email.NoopSender); !isNoop { - emailsEnabled = true - } - } - +// NewNotifier creates a new security notifier. Its emails are sent by jobs on jobsClient. +func NewNotifier(auditLogger *audit.Logger, database *db.Database, jobsClient *jobs.Client) *Notifier { n := &Notifier{ - emailSender: emailSender, auditLogger: auditLogger, database: database, - emailsEnabled: emailsEnabled, jobsClient: jobsClient, recentEmails: make(map[string]time.Time), cleanupInterval: 5 * time.Minute, @@ -97,7 +78,6 @@ func (n *Notifier) cleanupRecentEmails() { } // shouldSendEmail checks if we should send this email based on deduplication window -// enqueueSecurityNotification enqueues a security notification if deduplication allows // admins returns the admins to alert, looked up now so admins added or removed since startup are respected. func (n *Notifier) admins() []string { if n.database == nil { @@ -111,6 +91,7 @@ func (n *Notifier) admins() []string { return emails } +// enqueueSecurityNotification enqueues a security notification if deduplication allows func (n *Notifier) enqueueSecurityNotification(recipient, subject, eventType string, args river.JobArgs) { if recipient == "" && !strings.HasPrefix(eventType, "admin") { return @@ -138,10 +119,6 @@ func (n *Notifier) enqueueSecurityNotification(recipient, subject, eventType str // Returns true if email should be sent, false if it was recently sent func (n *Notifier) shouldSendEmail(recipient, subject, eventType string) bool { - if !n.emailsEnabled { - return false - } - // Create a hash of recipient + subject + event type for deduplication hash := fmt.Sprintf("%x", sha256.Sum256([]byte(recipient+subject+eventType))) @@ -180,7 +157,7 @@ func (n *Notifier) FailedMFAAttempt(userEmail, userName, ipAddress, userAgent st if err := n.auditLogger.LogDBEntry(&db.AuditLog{ UserEmail: userEmail, UserName: userName, - ActionType: "auth", + ActionType: audit.ActionTypeAuth, Action: audit.ActionMFAVerifyFailed, ResourceType: "auth", Resource: "mfa", @@ -265,7 +242,7 @@ func (n *Notifier) RateLimitExceeded(userEmail, userName, path, ipAddress, userA if err := n.auditLogger.LogDBEntry(&db.AuditLog{ UserEmail: userEmail, UserName: userName, - ActionType: "security", + ActionType: audit.ActionTypeSecurity, Action: audit.BuildAction(audit.ActionScopeRateLimit, audit.ActionVerbExceeded), ResourceType: "security", Resource: path, @@ -307,66 +284,3 @@ func (n *Notifier) RateLimitExceeded(userEmail, userName, path, ipAddress, userA UserAgent: userAgent, }) } - -// SuspiciousActivity logs and notifies about suspicious login patterns -func (n *Notifier) SuspiciousActivity( - userEmail, userName, reason, ipAddress, userAgent string, - details map[string]string, -) { - // Audit log - if n.database != nil { - if err := n.auditLogger.LogDBEntry(&db.AuditLog{ - UserEmail: userEmail, - UserName: userName, - ActionType: audit.ActionTypeSecurity, - Action: audit.ActionScopeSuspiciousActivity, - ResourceType: audit.ActionScopeRateLimit, - Resource: reason, - Status: audit.StatusAlert, - IPAddress: ipAddress, - UserAgent: userAgent, - }); err != nil { - log.Printf( - `{"level":"error","event":"audit_log_failed","action":%q,"error":%q}`, - audit.ActionScopeSuspiciousActivity, - err, - ) - } - } - - // Email notification to user (with rate limiting) - n.enqueueSecurityNotification( - userEmail, - "Suspicious Activity", - "suspicious_activity", - jobemail.SuspiciousActivityUserArgs{ - UserEmail: userEmail, - UserName: userName, - Reason: reason, - IPAddress: ipAddress, - UserAgent: userAgent, - Details: details, - }, - ) - - // Notify admins about suspicious activity (with deduplication per user+reason) - admins := n.admins() - if len(admins) == 0 { - return - } - adminKey := fmt.Sprintf("admins-%s-%s", userEmail, reason) - n.enqueueSecurityNotification( - adminKey, - "Suspicious Activity", - "suspicious_activity_admin", - jobemail.SuspiciousActivityAdminArgs{ - AdminEmails: admins, - UserEmail: userEmail, - UserName: userName, - Reason: reason, - IPAddress: ipAddress, - UserAgent: userAgent, - Details: details, - }, - ) -} diff --git a/internal/gateway/security/notifier_test.go b/internal/gateway/security/notifier_test.go index 78b0a3f7..93f9f64f 100644 --- a/internal/gateway/security/notifier_test.go +++ b/internal/gateway/security/notifier_test.go @@ -5,14 +5,13 @@ import ( "github.com/stretchr/testify/require" - "github.com/DocSpring/rack-gateway/internal/gateway/email" "github.com/DocSpring/rack-gateway/internal/gateway/testutil/dbtest" ) // Alerts go to the admins at the time of the alert: one added after startup gets them, and a locked one doesn't. func TestNotifierLooksUpAdminsWhenAlerting(t *testing.T) { database := dbtest.NewDatabase(t) - n := NewNotifier(email.NoopSender{}, nil, database, nil) + n := NewNotifier(nil, database, nil) t.Cleanup(n.Stop) require.Empty(t, n.admins()) diff --git a/internal/gateway/testutil/fakes3/fakes3.go b/internal/gateway/testutil/fakes3/fakes3.go index febf9621..78a4c4d0 100644 --- a/internal/gateway/testutil/fakes3/fakes3.go +++ b/internal/gateway/testutil/fakes3/fakes3.go @@ -29,10 +29,16 @@ type entry struct { deleteMarker bool } +// DefaultRetentionDays is the default COMPLIANCE retention of a new fake bucket. +const DefaultRetentionDays = 400 + // Bucket is a fake versioned bucket. The zero value isn't usable; call New. type Bucket struct { // PageSize is the number of keys (or versions) per listing page. PageSize int + // LockConfiguration is what GetObjectLockConfiguration returns: by default Object Lock with a COMPLIANCE + // retention of DefaultRetentionDays. + LockConfiguration *types.ObjectLockConfiguration mu sync.Mutex entries map[string][]*entry // oldest first @@ -43,7 +49,29 @@ type Bucket struct { // New returns an empty bucket whose clock starts at start. Each write without an explicit time is stamped one // second after the latest write so far. func New(start time.Time) *Bucket { - return &Bucket{PageSize: DefaultPageSize, entries: map[string][]*entry{}, clock: start} + return &Bucket{ + PageSize: DefaultPageSize, + LockConfiguration: &types.ObjectLockConfiguration{ + ObjectLockEnabled: types.ObjectLockEnabledEnabled, + Rule: &types.ObjectLockRule{DefaultRetention: &types.DefaultRetention{ + Mode: types.ObjectLockRetentionModeCompliance, + Days: aws.Int32(DefaultRetentionDays), + }}, + }, + entries: map[string][]*entry{}, + clock: start, + } +} + +// GetObjectLockConfiguration returns the bucket's LockConfiguration. +func (b *Bucket) GetObjectLockConfiguration( + _ context.Context, + _ *s3.GetObjectLockConfigurationInput, + _ ...func(*s3.Options), +) (*s3.GetObjectLockConfigurationOutput, error) { + b.mu.Lock() + defer b.mu.Unlock() + return &s3.GetObjectLockConfigurationOutput{ObjectLockConfiguration: b.LockConfiguration}, nil } // PutAt stores a new version of key written at modified. diff --git a/web/src/api/openapi.json b/web/src/api/openapi.json index a15428a2..404ad93d 100644 --- a/web/src/api/openapi.json +++ b/web/src/api/openapi.json @@ -5508,7 +5508,7 @@ "type": "string" }, "action_type": { - "description": "\"convox\", \"users\", \"auth\"", + "description": "permission scope: auth, convox, gateway, security", "type": "string" }, "api_token_id": { @@ -5862,9 +5862,6 @@ "type": "string" } }, - "suspended": { - "type": "boolean" - }, "updated_at": { "type": "string" } diff --git a/web/src/api/schemas/dbAuditLog.ts b/web/src/api/schemas/dbAuditLog.ts index 61de9d96..c93793a8 100644 --- a/web/src/api/schemas/dbAuditLog.ts +++ b/web/src/api/schemas/dbAuditLog.ts @@ -9,7 +9,7 @@ export interface DbAuditLog { /** e.g., "env.read", "user.create", "login.oauth_failed" */ action?: string; - /** "convox", "users", "auth" */ + /** permission scope: auth, convox, gateway, security */ action_type?: string; api_token_id?: number; api_token_name?: string; diff --git a/web/src/api/schemas/dbUser.ts b/web/src/api/schemas/dbUser.ts index b84213a4..1775e5a7 100644 --- a/web/src/api/schemas/dbUser.ts +++ b/web/src/api/schemas/dbUser.ts @@ -23,6 +23,5 @@ export interface DbUser { name?: string; preferred_mfa_method?: string; roles?: string[]; - suspended?: boolean; updated_at?: string; } diff --git a/web/src/api/types.generated.ts b/web/src/api/types.generated.ts index c34c68c1..08f5606a 100644 --- a/web/src/api/types.generated.ts +++ b/web/src/api/types.generated.ts @@ -5186,7 +5186,7 @@ export interface components { "db.AuditLog": { /** @description e.g., "env.read", "user.create", "login.oauth_failed" */ action?: string; - /** @description "convox", "users", "auth" */ + /** @description permission scope: auth, convox, gateway, security */ action_type?: string; api_token_id?: number; api_token_name?: string; @@ -5309,7 +5309,6 @@ export interface components { name?: string; preferred_mfa_method?: string; roles?: string[]; - suspended?: boolean; updated_at?: string; }; "handlers.AuditLogsResponse": { diff --git a/web/src/components/audit-logs/badges.tsx b/web/src/components/audit-logs/badges.tsx index c71635d7..7f998fae 100644 --- a/web/src/components/audit-logs/badges.tsx +++ b/web/src/components/audit-logs/badges.tsx @@ -58,15 +58,15 @@ export function getActionTypeBadgeAppearance(type?: string): AuditLogBadgeAppear variant: 'outline', className: 'bg-blue-600 text-white border border-border', } - case 'users': + case 'gateway': return { variant: 'outline', className: 'bg-purple-600 text-white border border-border', } - case 'tokens': + case 'security': return { variant: 'outline', - className: 'bg-amber-600 text-white border border-border', + className: 'bg-red-700 text-white border border-border', } case 'convox': return { diff --git a/web/src/components/audit-logs/utils.ts b/web/src/components/audit-logs/utils.ts index 9165f1ca..f9be7428 100644 --- a/web/src/components/audit-logs/utils.ts +++ b/web/src/components/audit-logs/utils.ts @@ -36,11 +36,10 @@ export function getAPITokenInfo(log: AuditLogRecord): { export function resourceLabelForLog(log: AuditLogRecord): string { const details = safeParseDetails(log.details) let label = '' - const actionType = log.action_type ?? '' const actionName = log.action ?? '' - if (actionType === 'users' || actionName.startsWith('user.')) { + if (actionName.startsWith('user.')) { label = (details.email as string) || '' - } else if (actionType === 'tokens' || actionName.startsWith('api_token.')) { + } else if (actionName.startsWith('api_token.')) { label = (details.name as string) || '' } if (!label) { diff --git a/web/src/pages/audit-page.test.tsx b/web/src/pages/audit-page.test.tsx index 938a43d3..7b8318d6 100644 --- a/web/src/pages/audit-page.test.tsx +++ b/web/src/pages/audit-page.test.tsx @@ -66,7 +66,7 @@ const mockLogs = [ timestamp: '2024-01-15T10:30:00Z', user_email: 'admin@example.com', user_name: 'Admin User', - action_type: 'users', + action_type: 'gateway', action: 'user.create', resource: 'newuser@example.com', resource_type: 'user', @@ -407,6 +407,24 @@ describe('AuditPage', () => { }) }) + it('offers one action type per permission scope', async () => { + vi.mocked(api.get).mockResolvedValue(makeResponse(mockLogs)) + + const Wrapper = createWrapper() + render(, { wrapper: Wrapper }) + + await waitFor(() => { + expect(screen.getByText('admin@example.com')).toBeInTheDocument() + }) + + const actionTypeSelect = document.getElementById('action-type') as HTMLSelectElement | null + if (!actionTypeSelect) { + throw new Error('Action type select not found') + } + const values = Array.from(actionTypeSelect.options, (option) => option.value) + expect(values).toEqual(['all', 'convox', 'gateway', 'auth', 'security']) + }) + it('filters by resource type', async () => { // Mock initial load and filtered load vi.mocked(api.get) diff --git a/web/src/pages/audit/utils.ts b/web/src/pages/audit/utils.ts index 453dbc4b..7fa101fa 100644 --- a/web/src/pages/audit/utils.ts +++ b/web/src/pages/audit/utils.ts @@ -1,25 +1,32 @@ import { DEFAULT_PER_PAGE } from '@/lib/constants' +// An entry's resource type is its action's resource (user for user.lock). export const RESOURCE_TYPES = { all: 'All Resources', app: 'App', - rack: 'Rack', + build: 'Build', + release: 'Release', + process: 'Process', env: 'Env', secret: 'Secret', - process: 'Process', + rack: 'Rack', system: 'System', + deploy_approval_request: 'Deploy Approval', api_token: 'API Token', user: 'User', + setting: 'Setting', + integration: 'Integration', auth: 'Auth', - admin: 'Admin', + mfa_method: 'MFA Method', } as const +// An entry's action type is the scope of the permission its action needs. export const ACTION_TYPES = { all: 'All Actions', convox: 'Convox API', + gateway: 'Gateway', auth: 'Authentication', - users: 'User Management', - tokens: 'Token Management', + security: 'Security', } as const export const STATUS_TYPES = { diff --git a/web/src/pages/rack-page.tsx b/web/src/pages/rack-page.tsx index 34aca30e..8eb203d0 100644 --- a/web/src/pages/rack-page.tsx +++ b/web/src/pages/rack-page.tsx @@ -1,15 +1,6 @@ import { useQuery } from '@tanstack/react-query' import { CardGrid } from '../components/card-grid' import { PageLayout } from '../components/page-layout' -// import { TablePane } from '../components/table-pane' -// import { -// Table, -// TableBody, -// TableCell, -// TableHead, -// TableHeader, -// TableRow, -// } from '../components/ui/table' import { api } from '../lib/api' type RackInfo = { @@ -59,44 +50,8 @@ export function RackPage() { { label: 'Status', value: data.status }, ]} /> - - {/* Update - Racks don't actually return parameters, those are in Terraform. */} - {/* {(() => { - const p = data.parameters || {} - return p && Object.keys(p).length > 0 ? ( - - - - ) : null - })()} */} - - {/* Outputs intentionally hidden */} )} ) } - -// function KvTable({ obj }: { obj: Record }) { -// const entries = Object.entries(obj).sort((a, b) => a[0].localeCompare(b[0])) -// return ( -//
    -// -// -// -// Key -// Value -// -// -// -// {entries.map(([k, v]) => ( -// -// {k} -// {v} -// -// ))} -// -//
    -//
    -// ) -// } diff --git a/web/src/pages/users-page.test.tsx b/web/src/pages/users-page.test.tsx index 2d7bdd75..eab0af79 100644 --- a/web/src/pages/users-page.test.tsx +++ b/web/src/pages/users-page.test.tsx @@ -61,7 +61,6 @@ const mockUsers = [ roles: ['admin'], created_at: '2024-01-01T00:00:00Z', updated_at: '2024-01-01T00:00:00Z', - suspended: false, }, { email: 'viewer@example.com', @@ -69,7 +68,6 @@ const mockUsers = [ roles: ['viewer'], created_at: '2024-01-01T00:00:00Z', updated_at: '2024-01-01T00:00:00Z', - suspended: false, }, ] @@ -327,7 +325,6 @@ describe('UsersPage', () => { // created_at and updated_at intentionally empty to simulate missing created_at: '', updated_at: '', - suspended: false, }, ] vi.mocked(api.get).mockResolvedValue(users) @@ -351,7 +348,6 @@ describe('UsersPage', () => { roles: ['viewer'], created_at: '2024-01-01T00:00:00Z', updated_at: '2024-01-01T00:00:00Z', - suspended: false, }, ] vi.mocked(api.get).mockResolvedValue(users) diff --git a/web/src/pages/users-page.tsx b/web/src/pages/users-page.tsx index 7d3c7e10..bdd0ebb8 100644 --- a/web/src/pages/users-page.tsx +++ b/web/src/pages/users-page.tsx @@ -177,7 +177,6 @@ export function UsersPage() { roles: [role], created_at: new Date().toISOString(), updated_at: new Date().toISOString(), - suspended: false, created_by_email: currentUser?.email, created_by_name: currentUser?.name, } as User) diff --git a/web/src/pages/users/user-utils.ts b/web/src/pages/users/user-utils.ts index efc0bcb6..908a7348 100644 --- a/web/src/pages/users/user-utils.ts +++ b/web/src/pages/users/user-utils.ts @@ -7,7 +7,6 @@ export type User = { roles: string[] created_at: string updated_at: string - suspended: boolean created_by_email?: string created_by_name?: string locked_at?: string From 48f1807c2ede615d5bb2561870c2e9de4c7501e8 Mon Sep 17 00:00:00 2001 From: Nathan Broadbent Date: Tue, 13 Oct 2026 15:58:25 +1300 Subject: [PATCH 2/2] test: filter the audit log E2E by the gateway type and api_token resource Co-Authored-By: Claude Opus 5.5 --- web/e2e/manage.spec.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/web/e2e/manage.spec.ts b/web/e2e/manage.spec.ts index 8fb896b9..e6d37c40 100644 --- a/web/e2e/manage.spec.ts +++ b/web/e2e/manage.spec.ts @@ -368,8 +368,9 @@ test('audit logs: view and filter', async ({ page }) => { const table = page.getByRole('table') await expect(table).toBeVisible() - // Filter by Action Type: Token Management - await page.selectOption('#action-type', 'tokens') + // Token changes are gateway actions on the api_token resource + await page.selectOption('#action-type', 'gateway') + await page.selectOption('#resource-type', 'api_token') // Filter by Status: Success await page.selectOption('#status', 'success')