From fb01d33cad66c2bb99130bf96673de5ca27e363d Mon Sep 17 00:00:00 2001 From: osama1998H Date: Sat, 28 Mar 2026 01:04:22 +0300 Subject: [PATCH 1/2] Enforce RBAC permissions on privileged routes --- docs/docs.go | 160 ++++++- docs/security-report.md | 19 +- docs/swagger.json | 160 ++++++- docs/swagger.yaml | 121 +++++- internal/api/handlers/apikeys.go | 9 +- internal/api/handlers/audit.go | 3 +- internal/api/handlers/organizations.go | 6 +- internal/api/handlers/roles.go | 21 +- internal/api/handlers/users.go | 9 +- internal/api/handlers/webhooks.go | 12 +- internal/api/middleware/authorization.go | 59 +++ internal/api/router.go | 41 +- internal/api/router_authorization_test.go | 390 ++++++++++++++++++ internal/domain/role.go | 18 + internal/repository/postgres/roles.go | 19 + .../repository/postgres/tenant_scope_test.go | 54 +++ internal/service/rbac.go | 34 +- internal/service/rbac_scope_test.go | 46 +++ .../000003_organization_permissions.down.sql | 2 + .../000003_organization_permissions.up.sql | 4 + sql/schema.sql | 4 +- 21 files changed, 1077 insertions(+), 114 deletions(-) create mode 100644 internal/api/middleware/authorization.go create mode 100644 internal/api/router_authorization_test.go create mode 100644 migrations/000003_organization_permissions.down.sql create mode 100644 migrations/000003_organization_permissions.up.sql diff --git a/docs/docs.go b/docs/docs.go index 5a0780f..2f8cea2 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -30,7 +30,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Returns all API keys for the authenticated user's organization. Key prefixes are shown; full keys are never returned after creation.", + "description": "Returns all API keys for the authenticated user's organization. Key prefixes are shown; full keys are never returned after creation. Requires the ` + "`" + `apikeys:read` + "`" + ` permission.", "produces": [ "application/json" ], @@ -51,6 +51,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -65,7 +71,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Creates a new API key. The full plaintext key is returned only in this response — store it securely.", + "description": "Creates a new API key. The full plaintext key is returned only in this response — store it securely. Requires the ` + "`" + `apikeys:write` + "`" + ` permission.", "consumes": [ "application/json" ], @@ -106,6 +112,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -122,7 +134,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Immediately revokes the specified API key. All subsequent requests using this key will be rejected.", + "description": "Immediately revokes the specified API key. All subsequent requests using this key will be rejected. Requires the ` + "`" + `apikeys:delete` + "`" + ` permission.", "produces": [ "application/json" ], @@ -158,6 +170,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -180,7 +198,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Returns audit log entries for the authenticated user's organization, with optional filters.", + "description": "Returns audit log entries for the authenticated user's organization, with optional filters. Requires the ` + "`" + `audit:read` + "`" + ` permission.", "produces": [ "application/json" ], @@ -239,6 +257,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -660,7 +684,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Returns the organization associated with the authenticated user's JWT.", + "description": "Returns the organization associated with the authenticated user's JWT. Requires the ` + "`" + `organizations:read` + "`" + ` permission.", "produces": [ "application/json" ], @@ -681,6 +705,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -701,7 +731,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Updates the name of the authenticated user's organization.", + "description": "Updates the name of the authenticated user's organization. Requires the ` + "`" + `organizations:write` + "`" + ` permission.", "consumes": [ "application/json" ], @@ -742,6 +772,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -758,7 +794,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Returns all roles defined within the authenticated user's organization.", + "description": "Returns all roles defined within the authenticated user's organization. Requires the ` + "`" + `roles:read` + "`" + ` permission.", "produces": [ "application/json" ], @@ -779,6 +815,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -793,7 +835,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Creates a new RBAC role within the authenticated user's organization.", + "description": "Creates a new RBAC role within the authenticated user's organization. Requires the ` + "`" + `roles:write` + "`" + ` permission.", "consumes": [ "application/json" ], @@ -834,6 +876,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "409": { "description": "Role name already exists", "schema": { @@ -856,7 +904,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Returns all system-level permissions that can be assigned to roles.", + "description": "Returns all system-level permissions that can be assigned to roles. Requires the ` + "`" + `roles:read` + "`" + ` permission.", "produces": [ "application/json" ], @@ -877,6 +925,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -893,7 +947,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Updates the name and/or description of an existing role.", + "description": "Updates the name and/or description of an existing role. Requires the ` + "`" + `roles:write` + "`" + ` permission.", "consumes": [ "application/json" ], @@ -941,6 +995,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -961,7 +1021,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Permanently deletes a role from the organization.", + "description": "Permanently deletes a role from the organization. Requires the ` + "`" + `roles:delete` + "`" + ` permission.", "produces": [ "application/json" ], @@ -997,6 +1057,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -1019,7 +1085,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Replaces the permission set of the specified role with the provided list.", + "description": "Replaces the permission set of the specified role with the provided list. Requires the ` + "`" + `roles:write` + "`" + ` permission.", "consumes": [ "application/json" ], @@ -1067,6 +1133,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -1089,7 +1161,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Returns a paginated list of all users in the authenticated user's organization.", + "description": "Returns a paginated list of all users in the authenticated user's organization. Requires the ` + "`" + `users:read` + "`" + ` permission.", "produces": [ "application/json" ], @@ -1124,6 +1196,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -1238,7 +1316,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Returns the profile of a specific user within the organization.", + "description": "Returns the profile of a specific user within the organization. Requires the ` + "`" + `users:read` + "`" + ` permission.", "produces": [ "application/json" ], @@ -1274,6 +1352,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -1294,7 +1378,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Marks the specified user as inactive. Deactivated users cannot log in.", + "description": "Marks the specified user as inactive. Deactivated users cannot log in. Requires the ` + "`" + `users:delete` + "`" + ` permission.", "produces": [ "application/json" ], @@ -1330,6 +1414,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -1352,7 +1442,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Grants the specified role to the target user within the organization.", + "description": "Grants the specified role to the target user within the organization. Requires the ` + "`" + `roles:write` + "`" + ` permission.", "consumes": [ "application/json" ], @@ -1400,6 +1490,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -1422,7 +1518,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Returns all webhook endpoints configured for the authenticated user's organization.", + "description": "Returns all webhook endpoints configured for the authenticated user's organization. Requires the ` + "`" + `webhooks:read` + "`" + ` permission.", "produces": [ "application/json" ], @@ -1443,6 +1539,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -1457,7 +1559,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Registers a new webhook endpoint. Webhook URLs must be direct public HTTPS endpoints; localhost, private, link-local, and metadata-style targets are rejected. Redirects are not followed. The HMAC signing secret is returned only in this response — store it securely.", + "description": "Registers a new webhook endpoint. Webhook URLs must be direct public HTTPS endpoints; localhost, private, link-local, and metadata-style targets are rejected. Redirects are not followed. The HMAC signing secret is returned only in this response — store it securely. Requires the ` + "`" + `webhooks:write` + "`" + ` permission.", "consumes": [ "application/json" ], @@ -1498,6 +1600,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -1514,7 +1622,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Updates the URL, event subscriptions, and/or active status of a webhook. If a URL is provided, it must be a direct public HTTPS endpoint and redirects will not be followed during delivery.", + "description": "Updates the URL, event subscriptions, and/or active status of a webhook. If a URL is provided, it must be a direct public HTTPS endpoint and redirects will not be followed during delivery. Requires the ` + "`" + `webhooks:write` + "`" + ` permission.", "consumes": [ "application/json" ], @@ -1562,6 +1670,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -1582,7 +1696,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Permanently removes the specified webhook endpoint.", + "description": "Permanently removes the specified webhook endpoint. Requires the ` + "`" + `webhooks:delete` + "`" + ` permission.", "produces": [ "application/json" ], @@ -1618,6 +1732,12 @@ const docTemplate = `{ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { diff --git a/docs/security-report.md b/docs/security-report.md index 417fe31..e20a1d3 100644 --- a/docs/security-report.md +++ b/docs/security-report.md @@ -1,9 +1,9 @@ **1. Executive Summary** -This repo’s remaining main security problems are real and concentrated in authorization and operational hardening. The highest-risk remaining issue is that most protected routes check only “has a valid JWT,” not “has the right permission.” The next tier is that Redis fail-open behavior and weak JWT secret acceptance still weaken abuse protection and revocation guarantees. +This repo’s remaining main security problems are real and concentrated in operational hardening. The highest-risk remaining issue is now that Redis fail-open behavior can weaken revocation and throttling during outages, with weak JWT secret acceptance as the next tier of residual risk. -Update (2026-03-28): findings 1, 3, 4, 5, 6, and 7 have been fixed in the repo. The remaining findings below are still outstanding unless explicitly marked otherwise. +Update (2026-03-28): findings 1, 2, 3, 4, 5, 6, and 7 have been fixed in the repo. The remaining findings below are still outstanding unless explicitly marked otherwise. -This report began as a read-only review of the codebase across auth/session logic, authorization/tenant boundaries, and config/deployment. Findings 1, 3, 4, 5, 6, and 7 have since been remediated in the repo. `go test ./...` passed locally; `govulncheck` was not installed, so dependency-CVE verification remains open. +This report began as a read-only review of the codebase across auth/session logic, authorization/tenant boundaries, and config/deployment. Findings 1, 2, 3, 4, 5, 6, and 7 have since been remediated in the repo. `go test ./...` passed locally; `govulncheck` was not installed, so dependency-CVE verification remains open. **2. Architecture and Attack Surface** - Entry points: public auth routes, `/health`, `/ready`, `/swagger/*`, and JWT-protected `/api/v1/*` in [router.go](/Users/osamamuhammed/uniauth/internal/api/router.go#L76). @@ -24,6 +24,8 @@ This report began as a read-only review of the codebase across auth/session logi Remediation implemented: access and refresh JWTs now carry an explicit token-purpose claim, bearer middleware accepts access tokens only, and refresh-only consumers use a dedicated verifier with a legacy bridge limited to active-session-backed refresh flows. Regression tests were added in [jwt_test.go](/Users/osamamuhammed/uniauth/pkg/token/jwt_test.go#L1), [auth_test.go](/Users/osamamuhammed/uniauth/internal/api/middleware/auth_test.go#L1), and [auth_test.go](/Users/osamamuhammed/uniauth/internal/service/auth_test.go#L1). 2. **Protected admin routes have authentication but almost no authorization** — High, High confidence. Affected: [router.go](/Users/osamamuhammed/uniauth/internal/api/router.go#L95), [rbac.go](/Users/osamamuhammed/uniauth/internal/service/rbac.go#L103), [users handler](/Users/osamamuhammed/uniauth/internal/api/handlers/users.go#L104), [roles handler](/Users/osamamuhammed/uniauth/internal/api/handlers/roles.go#L56), [audit handler](/Users/osamamuhammed/uniauth/internal/api/handlers/audit.go#L41), [apikey handler](/Users/osamamuhammed/uniauth/internal/api/handlers/apikeys.go#L34), [webhook handler](/Users/osamamuhammed/uniauth/internal/api/handlers/webhooks.go#L35). Evidence: every protected route is gated only by `JWTAuth`; `HasPermission` exists but is never called. Risk: any authenticated user can list/deactivate users, manage roles, create/revoke API keys, read audit logs, manage webhooks, and update the org. Preconditions: attacker has any valid JWT in the tenant. Fix: add explicit permission checks per route/action and define a permission matrix. Verify with negative tests for every admin endpoint using a valid but low-privilege JWT. OWASP: A01 Broken Access Control / privilege escalation. + Status: Fixed. + Remediation implemented: privileged JWT routes now enforce a router-level permission matrix via reusable middleware, RBAC authorization is evaluated request-time against a DB-backed `Authorize` path with `is_superuser` bootstrap bypass support, and the permission seed set now includes `organizations:read` and `organizations:write` for org-profile endpoints. Regression coverage was added in [router_authorization_test.go](/Users/osamamuhammed/uniauth/internal/api/router_authorization_test.go#L1), [tenant_scope_test.go](/Users/osamamuhammed/uniauth/internal/repository/postgres/tenant_scope_test.go#L1), and [rbac_scope_test.go](/Users/osamamuhammed/uniauth/internal/service/rbac_scope_test.go#L1). 3. **User and role object access is not consistently scoped by `org_id`** — High, High confidence. Affected: [users handler](/Users/osamamuhammed/uniauth/internal/api/handlers/users.go#L143), [user service](/Users/osamamuhammed/uniauth/internal/service/user.go#L24), [users repo](/Users/osamamuhammed/uniauth/internal/repository/postgres/users.go#L24), [roles handler](/Users/osamamuhammed/uniauth/internal/api/handlers/roles.go#L129), [rbac service](/Users/osamamuhammed/uniauth/internal/service/rbac.go#L45), [roles repo](/Users/osamamuhammed/uniauth/internal/repository/postgres/roles.go#L24). Evidence: `GetUserByID`, `UpdateUser`, `DeactivateUser`, `GetRoleByID`, `UpdateRole`, and `DeleteRole` operate on raw UUIDs without caller-org filtering. Risk: if a foreign UUID is learned from logs, support data, exports, or future endpoints, a user in one org can read or mutate another org’s user/role. Preconditions: attacker knows a target UUID. Fix: pass `org_id` into service/repo methods and enforce it on all lookups/mutations, returning `404` for out-of-org objects. Verify with cross-org tests for every `{id}` route. OWASP: A01 Broken Access Control / multi-tenant isolation failure. Status: Fixed. @@ -60,23 +62,23 @@ This report began as a read-only review of the codebase across auth/session logi **5. Remediation Roadmap** - Immediate hotfix 1: separate access and refresh token semantics, enforce access-only in `JWTAuth`, and harden invalidation. Complexity: Medium-High. Regression risk: Medium-High. Order: 1. -- Immediate hotfix 2: add authorization checks to all privileged routes. Complexity: Medium. Regression risk: Medium. Order: 2. +- Completed hotfix 2: add authorization checks to privileged routes with a reusable permission middleware and request-time RBAC enforcement. Complexity: Medium. Regression risk: Medium. Order: 2. - Completed hotfix 3: make all user/role object lookups and mutations tenant-scoped. Complexity: Medium. Regression risk: Medium. Order: 3. - Completed hotfix 4: stop logging reset tokens and fail closed when reset-email delivery is unavailable. Complexity: Small. Regression risk: Low. Order: 4. - Completed hardening 1: add webhook URL validation and outbound SSRF guards. Complexity: Medium. Regression risk: Medium. Order: 5. - Completed hardening 2: replace naive forwarded-header trust with trusted-proxy-aware IP extraction and explicit proxy CIDR configuration. Complexity: Small-Medium. Regression risk: Medium. Order: 6. - Short-term hardening 3: enforce JWT secret quality, sanitize `/ready`, and define Redis degraded-mode behavior. Complexity: Small. Regression risk: Low-Medium. Order: 7. -- Structural improvement 1: introduce reusable authorization middleware/policy mapping instead of ad hoc handler checks. Complexity: High. Regression risk: Medium. +- Completed structural improvement 1: introduce reusable authorization middleware/policy mapping instead of ad hoc handler checks. Complexity: High. Regression risk: Medium. - Completed structural improvement 2: add DB-level same-org enforcement for role assignments and auto-remove legacy cross-tenant links during migration. Complexity: Medium. Regression risk: Medium. - Structural improvement 3: pin CI actions/artifacts/images and add `govulncheck` to CI. Complexity: Small. Regression risk: Low. **6. Verification Checklist** - Add a test proving a refresh token gets `401` on a normal protected route like `/api/v1/users/me`. - Add tests proving logout, logout-all, password change, and password reset invalidate both access and refresh credentials in practice. -- Add negative authz tests for every privileged endpoint using a valid but low-privilege JWT. +- Implemented: router-level authorization tests now verify low-privilege JWTs receive `403` on every permission-gated endpoint, self-service user routes remain accessible without extra RBAC grants, explicit permission grants unlock representative endpoints, and superusers bypass route-level checks in [router_authorization_test.go](/Users/osamamuhammed/uniauth/internal/api/router_authorization_test.go#L1). - Implemented: cross-org repository tests now cover user/role lookup and mutation scoping in [tenant_scope_test.go](/Users/osamamuhammed/uniauth/internal/repository/postgres/tenant_scope_test.go#L1). - Implemented: user-role integrity tests now cover same-org assignment success, repository rejection of foreign-org pairs, and raw SQL schema-level enforcement of the same-org invariant in [tenant_scope_test.go](/Users/osamamuhammed/uniauth/internal/repository/postgres/tenant_scope_test.go#L1). -- Implemented: RBAC service tests now reject foreign-org role assignment and permission assignment in [rbac_scope_test.go](/Users/osamamuhammed/uniauth/internal/service/rbac_scope_test.go#L1). +- Implemented: RBAC service tests now reject foreign-org role assignment and permission assignment, and verify `Authorize` returns `ErrForbidden` for missing grants while allowing superuser bypass in [rbac_scope_test.go](/Users/osamamuhammed/uniauth/internal/service/rbac_scope_test.go#L1). - Implemented: trusted-proxy-aware IP resolution tests now confirm untrusted peers cannot spoof forwarded headers, trusted proxy chains resolve the correct client IP, and rate limiting buckets remain stable under header spoofing unless the peer is explicitly trusted in [ratelimit_test.go](/Users/osamamuhammed/uniauth/internal/api/middleware/ratelimit_test.go#L1). - Implemented: webhook validation and delivery tests now reject private/link-local/metadata webhook targets and ensure blocked destinations are not dialed in [webhook_test.go](/Users/osamamuhammed/uniauth/internal/service/webhook_test.go#L1) and [webhooks_test.go](/Users/osamamuhammed/uniauth/internal/api/handlers/webhooks_test.go#L1). - Add startup/config tests that weak or placeholder `JWT_SECRET` values fail fast. @@ -85,9 +87,8 @@ This report began as a read-only review of the codebase across auth/session logi - Add `govulncheck ./...` to CI and pin supply-chain artifacts. **7. Open Questions / Assumptions** -- Closing finding 1 safely may require invalidating all existing JWTs or accepting a short coexistence window; there is no reliable way to distinguish already-issued access vs refresh JWTs because they were minted with the same structure. - Deployments behind a reverse proxy must now set `TRUSTED_PROXY_CIDRS` explicitly; otherwise UniAuth will use the proxy peer IP for rate limiting and audit trails by design. - I did not verify pod/network egress policy. Existing egress controls remain useful defense-in-depth on top of the new webhook validation and delivery guards. - I could not verify dependency-level vulns because `govulncheck` is not installed in this workspace. -This report is now a current status snapshot of the repo; 5 issues remain open in the report: findings 2, 8, 9, plus the 2 lower-severity hardening items. +Quick note: 4 security issues remain open in the report: findings 8 and 9, plus the 2 lower-severity hardening items. diff --git a/docs/swagger.json b/docs/swagger.json index e7ed5f0..ad5ec15 100644 --- a/docs/swagger.json +++ b/docs/swagger.json @@ -24,7 +24,7 @@ "BearerAuth": [] } ], - "description": "Returns all API keys for the authenticated user's organization. Key prefixes are shown; full keys are never returned after creation.", + "description": "Returns all API keys for the authenticated user's organization. Key prefixes are shown; full keys are never returned after creation. Requires the `apikeys:read` permission.", "produces": [ "application/json" ], @@ -45,6 +45,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -59,7 +65,7 @@ "BearerAuth": [] } ], - "description": "Creates a new API key. The full plaintext key is returned only in this response — store it securely.", + "description": "Creates a new API key. The full plaintext key is returned only in this response — store it securely. Requires the `apikeys:write` permission.", "consumes": [ "application/json" ], @@ -100,6 +106,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -116,7 +128,7 @@ "BearerAuth": [] } ], - "description": "Immediately revokes the specified API key. All subsequent requests using this key will be rejected.", + "description": "Immediately revokes the specified API key. All subsequent requests using this key will be rejected. Requires the `apikeys:delete` permission.", "produces": [ "application/json" ], @@ -152,6 +164,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -174,7 +192,7 @@ "BearerAuth": [] } ], - "description": "Returns audit log entries for the authenticated user's organization, with optional filters.", + "description": "Returns audit log entries for the authenticated user's organization, with optional filters. Requires the `audit:read` permission.", "produces": [ "application/json" ], @@ -233,6 +251,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -654,7 +678,7 @@ "BearerAuth": [] } ], - "description": "Returns the organization associated with the authenticated user's JWT.", + "description": "Returns the organization associated with the authenticated user's JWT. Requires the `organizations:read` permission.", "produces": [ "application/json" ], @@ -675,6 +699,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -695,7 +725,7 @@ "BearerAuth": [] } ], - "description": "Updates the name of the authenticated user's organization.", + "description": "Updates the name of the authenticated user's organization. Requires the `organizations:write` permission.", "consumes": [ "application/json" ], @@ -736,6 +766,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -752,7 +788,7 @@ "BearerAuth": [] } ], - "description": "Returns all roles defined within the authenticated user's organization.", + "description": "Returns all roles defined within the authenticated user's organization. Requires the `roles:read` permission.", "produces": [ "application/json" ], @@ -773,6 +809,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -787,7 +829,7 @@ "BearerAuth": [] } ], - "description": "Creates a new RBAC role within the authenticated user's organization.", + "description": "Creates a new RBAC role within the authenticated user's organization. Requires the `roles:write` permission.", "consumes": [ "application/json" ], @@ -828,6 +870,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "409": { "description": "Role name already exists", "schema": { @@ -850,7 +898,7 @@ "BearerAuth": [] } ], - "description": "Returns all system-level permissions that can be assigned to roles.", + "description": "Returns all system-level permissions that can be assigned to roles. Requires the `roles:read` permission.", "produces": [ "application/json" ], @@ -871,6 +919,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -887,7 +941,7 @@ "BearerAuth": [] } ], - "description": "Updates the name and/or description of an existing role.", + "description": "Updates the name and/or description of an existing role. Requires the `roles:write` permission.", "consumes": [ "application/json" ], @@ -935,6 +989,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -955,7 +1015,7 @@ "BearerAuth": [] } ], - "description": "Permanently deletes a role from the organization.", + "description": "Permanently deletes a role from the organization. Requires the `roles:delete` permission.", "produces": [ "application/json" ], @@ -991,6 +1051,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -1013,7 +1079,7 @@ "BearerAuth": [] } ], - "description": "Replaces the permission set of the specified role with the provided list.", + "description": "Replaces the permission set of the specified role with the provided list. Requires the `roles:write` permission.", "consumes": [ "application/json" ], @@ -1061,6 +1127,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -1083,7 +1155,7 @@ "BearerAuth": [] } ], - "description": "Returns a paginated list of all users in the authenticated user's organization.", + "description": "Returns a paginated list of all users in the authenticated user's organization. Requires the `users:read` permission.", "produces": [ "application/json" ], @@ -1118,6 +1190,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -1232,7 +1310,7 @@ "BearerAuth": [] } ], - "description": "Returns the profile of a specific user within the organization.", + "description": "Returns the profile of a specific user within the organization. Requires the `users:read` permission.", "produces": [ "application/json" ], @@ -1268,6 +1346,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -1288,7 +1372,7 @@ "BearerAuth": [] } ], - "description": "Marks the specified user as inactive. Deactivated users cannot log in.", + "description": "Marks the specified user as inactive. Deactivated users cannot log in. Requires the `users:delete` permission.", "produces": [ "application/json" ], @@ -1324,6 +1408,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -1346,7 +1436,7 @@ "BearerAuth": [] } ], - "description": "Grants the specified role to the target user within the organization.", + "description": "Grants the specified role to the target user within the organization. Requires the `roles:write` permission.", "consumes": [ "application/json" ], @@ -1394,6 +1484,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -1416,7 +1512,7 @@ "BearerAuth": [] } ], - "description": "Returns all webhook endpoints configured for the authenticated user's organization.", + "description": "Returns all webhook endpoints configured for the authenticated user's organization. Requires the `webhooks:read` permission.", "produces": [ "application/json" ], @@ -1437,6 +1533,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -1451,7 +1553,7 @@ "BearerAuth": [] } ], - "description": "Registers a new webhook endpoint. Webhook URLs must be direct public HTTPS endpoints; localhost, private, link-local, and metadata-style targets are rejected. Redirects are not followed. The HMAC signing secret is returned only in this response — store it securely.", + "description": "Registers a new webhook endpoint. Webhook URLs must be direct public HTTPS endpoints; localhost, private, link-local, and metadata-style targets are rejected. Redirects are not followed. The HMAC signing secret is returned only in this response — store it securely. Requires the `webhooks:write` permission.", "consumes": [ "application/json" ], @@ -1492,6 +1594,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "500": { "description": "Internal Server Error", "schema": { @@ -1508,7 +1616,7 @@ "BearerAuth": [] } ], - "description": "Updates the URL, event subscriptions, and/or active status of a webhook. If a URL is provided, it must be a direct public HTTPS endpoint and redirects will not be followed during delivery.", + "description": "Updates the URL, event subscriptions, and/or active status of a webhook. If a URL is provided, it must be a direct public HTTPS endpoint and redirects will not be followed during delivery. Requires the `webhooks:write` permission.", "consumes": [ "application/json" ], @@ -1556,6 +1664,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { @@ -1576,7 +1690,7 @@ "BearerAuth": [] } ], - "description": "Permanently removes the specified webhook endpoint.", + "description": "Permanently removes the specified webhook endpoint. Requires the `webhooks:delete` permission.", "produces": [ "application/json" ], @@ -1612,6 +1726,12 @@ "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" } }, + "403": { + "description": "Forbidden", + "schema": { + "$ref": "#/definitions/internal_api_handlers.SwaggerErrorResponse" + } + }, "404": { "description": "Not Found", "schema": { diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 9ea147e..37c4a26 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -505,7 +505,8 @@ paths: /api/v1/api-keys: get: description: Returns all API keys for the authenticated user's organization. - Key prefixes are shown; full keys are never returned after creation. + Key prefixes are shown; full keys are never returned after creation. Requires + the `apikeys:read` permission. produces: - application/json responses: @@ -517,6 +518,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "500": description: Internal Server Error schema: @@ -530,7 +535,7 @@ paths: consumes: - application/json description: Creates a new API key. The full plaintext key is returned only - in this response — store it securely. + in this response — store it securely. Requires the `apikeys:write` permission. parameters: - description: API key configuration in: body @@ -553,6 +558,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "500": description: Internal Server Error schema: @@ -565,7 +574,7 @@ paths: /api/v1/api-keys/{id}: delete: description: Immediately revokes the specified API key. All subsequent requests - using this key will be rejected. + using this key will be rejected. Requires the `apikeys:delete` permission. parameters: - description: API Key UUID in: path @@ -587,6 +596,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "404": description: Not Found schema: @@ -603,7 +616,7 @@ paths: /api/v1/audit: get: description: Returns audit log entries for the authenticated user's organization, - with optional filters. + with optional filters. Requires the `audit:read` permission. parameters: - description: Maximum number of results in: query @@ -640,6 +653,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "500": description: Internal Server Error schema: @@ -921,7 +938,7 @@ paths: /api/v1/organizations/me: get: description: Returns the organization associated with the authenticated user's - JWT. + JWT. Requires the `organizations:read` permission. produces: - application/json responses: @@ -933,6 +950,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "404": description: Not Found schema: @@ -949,7 +970,8 @@ paths: put: consumes: - application/json - description: Updates the name of the authenticated user's organization. + description: Updates the name of the authenticated user's organization. Requires + the `organizations:write` permission. parameters: - description: New organization name in: body @@ -972,6 +994,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "500": description: Internal Server Error schema: @@ -984,6 +1010,7 @@ paths: /api/v1/roles: get: description: Returns all roles defined within the authenticated user's organization. + Requires the `roles:read` permission. produces: - application/json responses: @@ -995,6 +1022,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "500": description: Internal Server Error schema: @@ -1008,6 +1039,7 @@ paths: consumes: - application/json description: Creates a new RBAC role within the authenticated user's organization. + Requires the `roles:write` permission. parameters: - description: Role details in: body @@ -1030,6 +1062,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "409": description: Role name already exists schema: @@ -1045,7 +1081,8 @@ paths: - Roles /api/v1/roles/{id}: delete: - description: Permanently deletes a role from the organization. + description: Permanently deletes a role from the organization. Requires the + `roles:delete` permission. parameters: - description: Role UUID in: path @@ -1067,6 +1104,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "404": description: Not Found schema: @@ -1083,7 +1124,8 @@ paths: put: consumes: - application/json - description: Updates the name and/or description of an existing role. + description: Updates the name and/or description of an existing role. Requires + the `roles:write` permission. parameters: - description: Role UUID in: path @@ -1111,6 +1153,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "404": description: Not Found schema: @@ -1129,7 +1175,7 @@ paths: consumes: - application/json description: Replaces the permission set of the specified role with the provided - list. + list. Requires the `roles:write` permission. parameters: - description: Role UUID in: path @@ -1157,6 +1203,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "404": description: Not Found schema: @@ -1173,6 +1223,7 @@ paths: /api/v1/roles/permissions: get: description: Returns all system-level permissions that can be assigned to roles. + Requires the `roles:read` permission. produces: - application/json responses: @@ -1184,6 +1235,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "500": description: Internal Server Error schema: @@ -1196,7 +1251,7 @@ paths: /api/v1/users: get: description: Returns a paginated list of all users in the authenticated user's - organization. + organization. Requires the `users:read` permission. parameters: - description: Maximum number of results (default 50, max 100) in: query @@ -1217,6 +1272,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "500": description: Internal Server Error schema: @@ -1229,7 +1288,7 @@ paths: /api/v1/users/{id}: delete: description: Marks the specified user as inactive. Deactivated users cannot - log in. + log in. Requires the `users:delete` permission. parameters: - description: User UUID in: path @@ -1251,6 +1310,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "404": description: Not Found schema: @@ -1266,6 +1329,7 @@ paths: - Users get: description: Returns the profile of a specific user within the organization. + Requires the `users:read` permission. parameters: - description: User UUID in: path @@ -1287,6 +1351,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "404": description: Not Found schema: @@ -1305,6 +1373,7 @@ paths: consumes: - application/json description: Grants the specified role to the target user within the organization. + Requires the `roles:write` permission. parameters: - description: User UUID in: path @@ -1332,6 +1401,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "404": description: Not Found schema: @@ -1411,7 +1484,7 @@ paths: /api/v1/webhooks: get: description: Returns all webhook endpoints configured for the authenticated - user's organization. + user's organization. Requires the `webhooks:read` permission. produces: - application/json responses: @@ -1423,6 +1496,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "500": description: Internal Server Error schema: @@ -1438,7 +1515,7 @@ paths: description: Registers a new webhook endpoint. Webhook URLs must be direct public HTTPS endpoints; localhost, private, link-local, and metadata-style targets are rejected. Redirects are not followed. The HMAC signing secret is returned - only in this response — store it securely. + only in this response — store it securely. Requires the `webhooks:write` permission. parameters: - description: Webhook configuration in: body @@ -1461,6 +1538,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "500": description: Internal Server Error schema: @@ -1472,7 +1553,8 @@ paths: - Webhooks /api/v1/webhooks/{id}: delete: - description: Permanently removes the specified webhook endpoint. + description: Permanently removes the specified webhook endpoint. Requires the + `webhooks:delete` permission. parameters: - description: Webhook UUID in: path @@ -1494,6 +1576,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "404": description: Not Found schema: @@ -1512,7 +1598,8 @@ paths: - application/json description: Updates the URL, event subscriptions, and/or active status of a webhook. If a URL is provided, it must be a direct public HTTPS endpoint and - redirects will not be followed during delivery. + redirects will not be followed during delivery. Requires the `webhooks:write` + permission. parameters: - description: Webhook UUID in: path @@ -1540,6 +1627,10 @@ paths: description: Unauthorized schema: $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' + "403": + description: Forbidden + schema: + $ref: '#/definitions/internal_api_handlers.SwaggerErrorResponse' "404": description: Not Found schema: diff --git a/internal/api/handlers/apikeys.go b/internal/api/handlers/apikeys.go index e2c42e3..1bc0c9e 100644 --- a/internal/api/handlers/apikeys.go +++ b/internal/api/handlers/apikeys.go @@ -23,11 +23,12 @@ func NewAPIKeyHandler(apiKeySvc *service.APIKeyService) *APIKeyHandler { // ListAPIKeys godoc // @Summary List API keys -// @Description Returns all API keys for the authenticated user's organization. Key prefixes are shown; full keys are never returned after creation. +// @Description Returns all API keys for the authenticated user's organization. Key prefixes are shown; full keys are never returned after creation. Requires the `apikeys:read` permission. // @Tags API Keys // @Produce json // @Success 200 {object} APIKeyListResponse // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth // @Router /api/v1/api-keys [get] @@ -53,7 +54,7 @@ func (h *APIKeyHandler) ListAPIKeys(w http.ResponseWriter, r *http.Request) { // CreateAPIKey godoc // @Summary Create an API key -// @Description Creates a new API key. The full plaintext key is returned only in this response — store it securely. +// @Description Creates a new API key. The full plaintext key is returned only in this response — store it securely. Requires the `apikeys:write` permission. // @Tags API Keys // @Accept json // @Produce json @@ -61,6 +62,7 @@ func (h *APIKeyHandler) ListAPIKeys(w http.ResponseWriter, r *http.Request) { // @Success 201 {object} CreateAPIKeyResponse // @Failure 400 {object} SwaggerErrorResponse // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth // @Router /api/v1/api-keys [post] @@ -101,13 +103,14 @@ func (h *APIKeyHandler) CreateAPIKey(w http.ResponseWriter, r *http.Request) { // RevokeAPIKey godoc // @Summary Revoke an API key -// @Description Immediately revokes the specified API key. All subsequent requests using this key will be rejected. +// @Description Immediately revokes the specified API key. All subsequent requests using this key will be rejected. Requires the `apikeys:delete` permission. // @Tags API Keys // @Produce json // @Param id path string true "API Key UUID" // @Success 200 {object} SwaggerMessageResponse // @Failure 400 {object} SwaggerErrorResponse "Invalid UUID" // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 404 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth diff --git a/internal/api/handlers/audit.go b/internal/api/handlers/audit.go index 002e63b..168cba8 100644 --- a/internal/api/handlers/audit.go +++ b/internal/api/handlers/audit.go @@ -24,7 +24,7 @@ func NewAuditHandler(store *db.Store) *AuditHandler { // ListAuditLogs godoc // @Summary List audit logs -// @Description Returns audit log entries for the authenticated user's organization, with optional filters. +// @Description Returns audit log entries for the authenticated user's organization, with optional filters. Requires the `audit:read` permission. // @Tags Audit // @Produce json // @Param limit query int false "Maximum number of results" @@ -35,6 +35,7 @@ func NewAuditHandler(store *db.Store) *AuditHandler { // @Param until query string false "Filter entries before this timestamp (RFC3339)" // @Success 200 {object} AuditListResponse // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth // @Router /api/v1/audit [get] diff --git a/internal/api/handlers/organizations.go b/internal/api/handlers/organizations.go index a4dde39..9d723e2 100644 --- a/internal/api/handlers/organizations.go +++ b/internal/api/handlers/organizations.go @@ -19,11 +19,12 @@ func NewOrgHandler(orgSvc *service.OrgService) *OrgHandler { // GetMyOrg godoc // @Summary Get current organization -// @Description Returns the organization associated with the authenticated user's JWT. +// @Description Returns the organization associated with the authenticated user's JWT. Requires the `organizations:read` permission. // @Tags Organizations // @Produce json // @Success 200 {object} OrgView // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 404 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth @@ -46,7 +47,7 @@ func (h *OrgHandler) GetMyOrg(w http.ResponseWriter, r *http.Request) { // UpdateMyOrg godoc // @Summary Update current organization -// @Description Updates the name of the authenticated user's organization. +// @Description Updates the name of the authenticated user's organization. Requires the `organizations:write` permission. // @Tags Organizations // @Accept json // @Produce json @@ -54,6 +55,7 @@ func (h *OrgHandler) GetMyOrg(w http.ResponseWriter, r *http.Request) { // @Success 200 {object} OrgView // @Failure 400 {object} SwaggerErrorResponse // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth // @Router /api/v1/organizations/me [put] diff --git a/internal/api/handlers/roles.go b/internal/api/handlers/roles.go index 1055b04..6a0b302 100644 --- a/internal/api/handlers/roles.go +++ b/internal/api/handlers/roles.go @@ -22,11 +22,12 @@ func NewRoleHandler(rbacSvc *service.RBACService) *RoleHandler { // ListPermissions godoc // @Summary List available permissions -// @Description Returns all system-level permissions that can be assigned to roles. +// @Description Returns all system-level permissions that can be assigned to roles. Requires the `roles:read` permission. // @Tags Roles // @Produce json // @Success 200 {object} PermissionsListResponse // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth // @Router /api/v1/roles/permissions [get] @@ -45,11 +46,12 @@ func (h *RoleHandler) ListPermissions(w http.ResponseWriter, r *http.Request) { // ListRoles godoc // @Summary List roles in organization -// @Description Returns all roles defined within the authenticated user's organization. +// @Description Returns all roles defined within the authenticated user's organization. Requires the `roles:read` permission. // @Tags Roles // @Produce json // @Success 200 {object} RolesListResponse // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth // @Router /api/v1/roles [get] @@ -74,7 +76,7 @@ func (h *RoleHandler) ListRoles(w http.ResponseWriter, r *http.Request) { // CreateRole godoc // @Summary Create a new role -// @Description Creates a new RBAC role within the authenticated user's organization. +// @Description Creates a new RBAC role within the authenticated user's organization. Requires the `roles:write` permission. // @Tags Roles // @Accept json // @Produce json @@ -82,6 +84,7 @@ func (h *RoleHandler) ListRoles(w http.ResponseWriter, r *http.Request) { // @Success 201 {object} RoleView // @Failure 400 {object} SwaggerErrorResponse // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 409 {object} SwaggerErrorResponse "Role name already exists" // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth @@ -113,7 +116,7 @@ func (h *RoleHandler) CreateRole(w http.ResponseWriter, r *http.Request) { // UpdateRole godoc // @Summary Update a role -// @Description Updates the name and/or description of an existing role. +// @Description Updates the name and/or description of an existing role. Requires the `roles:write` permission. // @Tags Roles // @Accept json // @Produce json @@ -122,6 +125,7 @@ func (h *RoleHandler) CreateRole(w http.ResponseWriter, r *http.Request) { // @Success 200 {object} RoleView // @Failure 400 {object} SwaggerErrorResponse // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 404 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth @@ -157,13 +161,14 @@ func (h *RoleHandler) UpdateRole(w http.ResponseWriter, r *http.Request) { // DeleteRole godoc // @Summary Delete a role -// @Description Permanently deletes a role from the organization. +// @Description Permanently deletes a role from the organization. Requires the `roles:delete` permission. // @Tags Roles // @Produce json // @Param id path string true "Role UUID" // @Success 200 {object} SwaggerMessageResponse // @Failure 400 {object} SwaggerErrorResponse "Invalid UUID" // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 404 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth @@ -186,7 +191,7 @@ func (h *RoleHandler) DeleteRole(w http.ResponseWriter, r *http.Request) { // AssignPermissions godoc // @Summary Assign permissions to a role -// @Description Replaces the permission set of the specified role with the provided list. +// @Description Replaces the permission set of the specified role with the provided list. Requires the `roles:write` permission. // @Tags Roles // @Accept json // @Produce json @@ -195,6 +200,7 @@ func (h *RoleHandler) DeleteRole(w http.ResponseWriter, r *http.Request) { // @Success 200 {object} SwaggerMessageResponse // @Failure 400 {object} SwaggerErrorResponse // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 404 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth @@ -228,7 +234,7 @@ func (h *RoleHandler) AssignPermissions(w http.ResponseWriter, r *http.Request) // AssignRoleToUser godoc // @Summary Assign a role to a user -// @Description Grants the specified role to the target user within the organization. +// @Description Grants the specified role to the target user within the organization. Requires the `roles:write` permission. // @Tags Roles // @Accept json // @Produce json @@ -237,6 +243,7 @@ func (h *RoleHandler) AssignPermissions(w http.ResponseWriter, r *http.Request) // @Success 200 {object} SwaggerMessageResponse // @Failure 400 {object} SwaggerErrorResponse // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 404 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth diff --git a/internal/api/handlers/users.go b/internal/api/handlers/users.go index d1a5b01..7952283 100644 --- a/internal/api/handlers/users.go +++ b/internal/api/handlers/users.go @@ -101,13 +101,14 @@ func (h *UserHandler) UpdateMe(w http.ResponseWriter, r *http.Request) { // ListUsers godoc // @Summary List users in organization -// @Description Returns a paginated list of all users in the authenticated user's organization. +// @Description Returns a paginated list of all users in the authenticated user's organization. Requires the `users:read` permission. // @Tags Users // @Produce json // @Param limit query int false "Maximum number of results (default 50, max 100)" // @Param offset query int false "Number of results to skip" // @Success 200 {object} UserListResponse // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth // @Router /api/v1/users [get] @@ -139,13 +140,14 @@ func (h *UserHandler) ListUsers(w http.ResponseWriter, r *http.Request) { // GetUser godoc // @Summary Get user by ID -// @Description Returns the profile of a specific user within the organization. +// @Description Returns the profile of a specific user within the organization. Requires the `users:read` permission. // @Tags Users // @Produce json // @Param id path string true "User UUID" // @Success 200 {object} UserView // @Failure 400 {object} SwaggerErrorResponse "Invalid UUID" // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 404 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth @@ -173,13 +175,14 @@ func (h *UserHandler) GetUser(w http.ResponseWriter, r *http.Request) { // DeactivateUser godoc // @Summary Deactivate a user -// @Description Marks the specified user as inactive. Deactivated users cannot log in. +// @Description Marks the specified user as inactive. Deactivated users cannot log in. Requires the `users:delete` permission. // @Tags Users // @Produce json // @Param id path string true "User UUID" // @Success 200 {object} SwaggerMessageResponse // @Failure 400 {object} SwaggerErrorResponse "Invalid UUID" // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 404 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth diff --git a/internal/api/handlers/webhooks.go b/internal/api/handlers/webhooks.go index e6ca357..bf17397 100644 --- a/internal/api/handlers/webhooks.go +++ b/internal/api/handlers/webhooks.go @@ -23,11 +23,12 @@ func NewWebhookHandler(webhookSvc *service.WebhookService) *WebhookHandler { // ListWebhooks godoc // @Summary List webhooks -// @Description Returns all webhook endpoints configured for the authenticated user's organization. +// @Description Returns all webhook endpoints configured for the authenticated user's organization. Requires the `webhooks:read` permission. // @Tags Webhooks // @Produce json // @Success 200 {object} WebhookListResponse // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth // @Router /api/v1/webhooks [get] @@ -53,7 +54,7 @@ func (h *WebhookHandler) ListWebhooks(w http.ResponseWriter, r *http.Request) { // CreateWebhook godoc // @Summary Create a webhook -// @Description Registers a new webhook endpoint. Webhook URLs must be direct public HTTPS endpoints; localhost, private, link-local, and metadata-style targets are rejected. Redirects are not followed. The HMAC signing secret is returned only in this response — store it securely. +// @Description Registers a new webhook endpoint. Webhook URLs must be direct public HTTPS endpoints; localhost, private, link-local, and metadata-style targets are rejected. Redirects are not followed. The HMAC signing secret is returned only in this response — store it securely. Requires the `webhooks:write` permission. // @Tags Webhooks // @Accept json // @Produce json @@ -61,6 +62,7 @@ func (h *WebhookHandler) ListWebhooks(w http.ResponseWriter, r *http.Request) { // @Success 201 {object} CreateWebhookResponse // @Failure 400 {object} SwaggerErrorResponse "Invalid or unsafe webhook URL" // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth // @Router /api/v1/webhooks [post] @@ -93,7 +95,7 @@ func (h *WebhookHandler) CreateWebhook(w http.ResponseWriter, r *http.Request) { // UpdateWebhook godoc // @Summary Update a webhook -// @Description Updates the URL, event subscriptions, and/or active status of a webhook. If a URL is provided, it must be a direct public HTTPS endpoint and redirects will not be followed during delivery. +// @Description Updates the URL, event subscriptions, and/or active status of a webhook. If a URL is provided, it must be a direct public HTTPS endpoint and redirects will not be followed during delivery. Requires the `webhooks:write` permission. // @Tags Webhooks // @Accept json // @Produce json @@ -102,6 +104,7 @@ func (h *WebhookHandler) CreateWebhook(w http.ResponseWriter, r *http.Request) { // @Success 200 {object} WebhookView // @Failure 400 {object} SwaggerErrorResponse "Invalid or unsafe webhook URL" // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 404 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth @@ -134,13 +137,14 @@ func (h *WebhookHandler) UpdateWebhook(w http.ResponseWriter, r *http.Request) { // DeleteWebhook godoc // @Summary Delete a webhook -// @Description Permanently removes the specified webhook endpoint. +// @Description Permanently removes the specified webhook endpoint. Requires the `webhooks:delete` permission. // @Tags Webhooks // @Produce json // @Param id path string true "Webhook UUID" // @Success 200 {object} SwaggerMessageResponse // @Failure 400 {object} SwaggerErrorResponse "Invalid UUID" // @Failure 401 {object} SwaggerErrorResponse +// @Failure 403 {object} SwaggerErrorResponse // @Failure 404 {object} SwaggerErrorResponse // @Failure 500 {object} SwaggerErrorResponse // @Security BearerAuth diff --git a/internal/api/middleware/authorization.go b/internal/api/middleware/authorization.go new file mode 100644 index 0000000..e28b626 --- /dev/null +++ b/internal/api/middleware/authorization.go @@ -0,0 +1,59 @@ +package middleware + +import ( + "context" + "errors" + "net/http" + + "github.com/google/uuid" + + "github.com/osama1998h/uniauth/internal/domain" +) + +type permissionAuthorizer interface { + Authorize(ctx context.Context, orgID, userID uuid.UUID, permission string) error +} + +// RequirePermission ensures the authenticated user has the requested permission. +func RequirePermission(authorizer permissionAuthorizer, permission string) func(next http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + userID, ok := GetUserID(r.Context()) + if !ok { + writeUnauthorized(w, "unauthorized") + return + } + orgID, ok := GetOrgID(r.Context()) + if !ok { + writeUnauthorized(w, "unauthorized") + return + } + + if err := authorizer.Authorize(r.Context(), orgID, userID, permission); err != nil { + switch { + case errors.Is(err, domain.ErrForbidden): + writeForbidden(w, "forbidden") + case errors.Is(err, domain.ErrUnauthorized): + writeUnauthorized(w, "unauthorized") + default: + writeInternalError(w) + } + return + } + + next.ServeHTTP(w, r) + }) + } +} + +func writeForbidden(w http.ResponseWriter, msg string) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"error":"` + msg + `"}`)) +} + +func writeInternalError(w http.ResponseWriter) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":"internal server error"}`)) +} diff --git a/internal/api/router.go b/internal/api/router.go index fa0135c..f9dfbe3 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -13,6 +13,7 @@ import ( "github.com/osama1998h/uniauth/internal/api/handlers" "github.com/osama1998h/uniauth/internal/api/middleware" "github.com/osama1998h/uniauth/internal/config" + "github.com/osama1998h/uniauth/internal/domain" "github.com/osama1998h/uniauth/internal/repository/cache" db "github.com/osama1998h/uniauth/internal/repository/postgres" "github.com/osama1998h/uniauth/internal/service" @@ -101,44 +102,44 @@ func NewRouter( r.Route("/users", func(r chi.Router) { r.Get("/me", userH.GetMe) r.Put("/me", userH.UpdateMe) - r.Get("/", userH.ListUsers) - r.Get("/{id}", userH.GetUser) - r.Delete("/{id}", userH.DeactivateUser) - r.Post("/{id}/roles", roleH.AssignRoleToUser) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionUsersRead)).Get("/", userH.ListUsers) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionUsersRead)).Get("/{id}", userH.GetUser) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionUsersDelete)).Delete("/{id}", userH.DeactivateUser) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionRolesWrite)).Post("/{id}/roles", roleH.AssignRoleToUser) }) // Organizations r.Route("/organizations", func(r chi.Router) { - r.Get("/me", orgH.GetMyOrg) - r.Put("/me", orgH.UpdateMyOrg) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionOrganizationsRead)).Get("/me", orgH.GetMyOrg) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionOrganizationsWrite)).Put("/me", orgH.UpdateMyOrg) }) // Roles & Permissions r.Route("/roles", func(r chi.Router) { - r.Get("/permissions", roleH.ListPermissions) - r.Get("/", roleH.ListRoles) - r.Post("/", roleH.CreateRole) - r.Put("/{id}", roleH.UpdateRole) - r.Delete("/{id}", roleH.DeleteRole) - r.Post("/{id}/permissions", roleH.AssignPermissions) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionRolesRead)).Get("/permissions", roleH.ListPermissions) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionRolesRead)).Get("/", roleH.ListRoles) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionRolesWrite)).Post("/", roleH.CreateRole) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionRolesWrite)).Put("/{id}", roleH.UpdateRole) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionRolesDelete)).Delete("/{id}", roleH.DeleteRole) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionRolesWrite)).Post("/{id}/permissions", roleH.AssignPermissions) }) // API Keys r.Route("/api-keys", func(r chi.Router) { - r.Get("/", apiKeyH.ListAPIKeys) - r.Post("/", apiKeyH.CreateAPIKey) - r.Delete("/{id}", apiKeyH.RevokeAPIKey) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionAPIKeysRead)).Get("/", apiKeyH.ListAPIKeys) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionAPIKeysWrite)).Post("/", apiKeyH.CreateAPIKey) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionAPIKeysDelete)).Delete("/{id}", apiKeyH.RevokeAPIKey) }) // Audit Logs - r.Get("/audit", auditH.ListAuditLogs) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionAuditRead)).Get("/audit", auditH.ListAuditLogs) // Webhooks r.Route("/webhooks", func(r chi.Router) { - r.Get("/", webhookH.ListWebhooks) - r.Post("/", webhookH.CreateWebhook) - r.Put("/{id}", webhookH.UpdateWebhook) - r.Delete("/{id}", webhookH.DeleteWebhook) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionWebhooksRead)).Get("/", webhookH.ListWebhooks) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionWebhooksWrite)).Post("/", webhookH.CreateWebhook) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionWebhooksWrite)).Put("/{id}", webhookH.UpdateWebhook) + r.With(middleware.RequirePermission(rbacSvc, domain.PermissionWebhooksDelete)).Delete("/{id}", webhookH.DeleteWebhook) }) }) }) diff --git a/internal/api/router_authorization_test.go b/internal/api/router_authorization_test.go new file mode 100644 index 0000000..a20728d --- /dev/null +++ b/internal/api/router_authorization_test.go @@ -0,0 +1,390 @@ +package api_test + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/google/uuid" + + "github.com/osama1998h/uniauth/internal/api" + "github.com/osama1998h/uniauth/internal/config" + "github.com/osama1998h/uniauth/internal/domain" + db "github.com/osama1998h/uniauth/internal/repository/postgres" + "github.com/osama1998h/uniauth/internal/testutil" + "github.com/osama1998h/uniauth/pkg/token" +) + +const routerTestJWTSecret = "supersecretkey-at-least-32-chars!!" + +func TestRouterRequiresPermissionsOnPrivilegedRoutes(t *testing.T) { + store := testutil.RequireTestStore(t) + handler := newTestRouter(store) + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + org := testutil.CreateOrganization(t, store, "router-forbidden-org") + user := testutil.CreateUser(t, store, org.ID, "router-forbidden-user") + accessToken := issueAccessToken(t, user.ID, org.ID) + + tests := []struct { + name string + method string + path string + body string + }{ + {name: "list users", method: http.MethodGet, path: "/api/v1/users"}, + {name: "get user", method: http.MethodGet, path: "/api/v1/users/" + uuid.NewString()}, + {name: "deactivate user", method: http.MethodDelete, path: "/api/v1/users/" + uuid.NewString()}, + {name: "assign role to user", method: http.MethodPost, path: "/api/v1/users/" + uuid.NewString() + "/roles", body: `{"role_id":"` + uuid.NewString() + `"}`}, + {name: "get org", method: http.MethodGet, path: "/api/v1/organizations/me"}, + {name: "update org", method: http.MethodPut, path: "/api/v1/organizations/me", body: `{"name":"Updated Org"}`}, + {name: "list permissions", method: http.MethodGet, path: "/api/v1/roles/permissions"}, + {name: "list roles", method: http.MethodGet, path: "/api/v1/roles"}, + {name: "create role", method: http.MethodPost, path: "/api/v1/roles", body: `{"name":"support"}`}, + {name: "update role", method: http.MethodPut, path: "/api/v1/roles/" + uuid.NewString(), body: `{"name":"support"}`}, + {name: "delete role", method: http.MethodDelete, path: "/api/v1/roles/" + uuid.NewString()}, + {name: "assign permissions", method: http.MethodPost, path: "/api/v1/roles/" + uuid.NewString() + "/permissions", body: `{"permissions":["users:read"]}`}, + {name: "list api keys", method: http.MethodGet, path: "/api/v1/api-keys"}, + {name: "create api key", method: http.MethodPost, path: "/api/v1/api-keys", body: `{"name":"build-bot"}`}, + {name: "revoke api key", method: http.MethodDelete, path: "/api/v1/api-keys/" + uuid.NewString()}, + {name: "list audit logs", method: http.MethodGet, path: "/api/v1/audit"}, + {name: "list webhooks", method: http.MethodGet, path: "/api/v1/webhooks"}, + {name: "create webhook", method: http.MethodPost, path: "/api/v1/webhooks", body: `{"url":"https://example.com/hooks","events":["user.login"]}`}, + {name: "update webhook", method: http.MethodPut, path: "/api/v1/webhooks/" + uuid.NewString(), body: `{"url":"https://example.com/hooks"}`}, + {name: "delete webhook", method: http.MethodDelete, path: "/api/v1/webhooks/" + uuid.NewString()}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + rec := performAuthedRequest(handler, tc.method, tc.path, tc.body, accessToken) + if rec.Code != http.StatusForbidden { + t.Fatalf("%s %s status = %d, want %d, body=%s", tc.method, tc.path, rec.Code, http.StatusForbidden, rec.Body.String()) + } + }) + } + + _ = ctx +} + +func TestRouterAllowsSelfServiceEndpointsWithoutRBACPermissions(t *testing.T) { + store := testutil.RequireTestStore(t) + handler := newTestRouter(store) + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + org := testutil.CreateOrganization(t, store, "router-self-org") + user := testutil.CreateUser(t, store, org.ID, "router-self-user") + accessToken := issueAccessToken(t, user.ID, org.ID) + + getRec := performAuthedRequest(handler, http.MethodGet, "/api/v1/users/me", "", accessToken) + if getRec.Code != http.StatusOK { + t.Fatalf("GET /api/v1/users/me status = %d, want %d, body=%s", getRec.Code, http.StatusOK, getRec.Body.String()) + } + + updateRec := performAuthedRequest(handler, http.MethodPut, "/api/v1/users/me", `{"full_name":"Updated Name"}`, accessToken) + if updateRec.Code != http.StatusOK { + t.Fatalf("PUT /api/v1/users/me status = %d, want %d, body=%s", updateRec.Code, http.StatusOK, updateRec.Body.String()) + } + + updatedUser, err := store.GetUserByID(ctx, org.ID, user.ID) + if err != nil { + t.Fatalf("GetUserByID() error = %v", err) + } + if updatedUser.FullName == nil || *updatedUser.FullName != "Updated Name" { + t.Fatalf("updated full name = %v, want %q", updatedUser.FullName, "Updated Name") + } +} + +func TestRouterAllowsGrantedPermissions(t *testing.T) { + store := testutil.RequireTestStore(t) + handler := newTestRouter(store) + + tests := []struct { + name string + permission string + method string + path func(t *testing.T, ctx context.Context, store *db.Store, orgID uuid.UUID) string + body string + wantStatus int + }{ + { + name: "users read", + permission: domain.PermissionUsersRead, + method: http.MethodGet, + path: func(_ *testing.T, _ context.Context, _ *db.Store, _ uuid.UUID) string { return "/api/v1/users" }, + wantStatus: http.StatusOK, + }, + { + name: "users delete", + permission: domain.PermissionUsersDelete, + method: http.MethodDelete, + path: func(t *testing.T, _ context.Context, store *db.Store, orgID uuid.UUID) string { + target := testutil.CreateUser(t, store, orgID, "router-delete-user") + return "/api/v1/users/" + target.ID.String() + }, + wantStatus: http.StatusOK, + }, + { + name: "roles read", + permission: domain.PermissionRolesRead, + method: http.MethodGet, + path: func(_ *testing.T, _ context.Context, _ *db.Store, _ uuid.UUID) string { return "/api/v1/roles" }, + wantStatus: http.StatusOK, + }, + { + name: "roles write", + permission: domain.PermissionRolesWrite, + method: http.MethodPost, + path: func(_ *testing.T, _ context.Context, _ *db.Store, _ uuid.UUID) string { return "/api/v1/roles" }, + body: `{"name":"support"}`, + wantStatus: http.StatusCreated, + }, + { + name: "roles delete", + permission: domain.PermissionRolesDelete, + method: http.MethodDelete, + path: func(t *testing.T, _ context.Context, store *db.Store, orgID uuid.UUID) string { + role := testutil.CreateRole(t, store, orgID, "router-delete-role") + return "/api/v1/roles/" + role.ID.String() + }, + wantStatus: http.StatusOK, + }, + { + name: "api keys read", + permission: domain.PermissionAPIKeysRead, + method: http.MethodGet, + path: func(_ *testing.T, _ context.Context, _ *db.Store, _ uuid.UUID) string { return "/api/v1/api-keys" }, + wantStatus: http.StatusOK, + }, + { + name: "api keys write", + permission: domain.PermissionAPIKeysWrite, + method: http.MethodPost, + path: func(_ *testing.T, _ context.Context, _ *db.Store, _ uuid.UUID) string { return "/api/v1/api-keys" }, + body: `{"name":"build-bot"}`, + wantStatus: http.StatusCreated, + }, + { + name: "api keys delete", + permission: domain.PermissionAPIKeysDelete, + method: http.MethodDelete, + path: func(t *testing.T, ctx context.Context, store *db.Store, orgID uuid.UUID) string { + key, err := store.CreateAPIKey(ctx, orgID, "router-delete-key", "uni", uuid.NewString(), nil, nil) + if err != nil { + t.Fatalf("CreateAPIKey() error = %v", err) + } + return "/api/v1/api-keys/" + key.ID.String() + }, + wantStatus: http.StatusOK, + }, + { + name: "audit read", + permission: domain.PermissionAuditRead, + method: http.MethodGet, + path: func(_ *testing.T, _ context.Context, _ *db.Store, _ uuid.UUID) string { return "/api/v1/audit" }, + wantStatus: http.StatusOK, + }, + { + name: "webhooks read", + permission: domain.PermissionWebhooksRead, + method: http.MethodGet, + path: func(_ *testing.T, _ context.Context, _ *db.Store, _ uuid.UUID) string { return "/api/v1/webhooks" }, + wantStatus: http.StatusOK, + }, + { + name: "webhooks write", + permission: domain.PermissionWebhooksWrite, + method: http.MethodPost, + path: func(_ *testing.T, _ context.Context, _ *db.Store, _ uuid.UUID) string { return "/api/v1/webhooks" }, + body: `{"url":"https://example.com/hooks","events":["user.login"]}`, + wantStatus: http.StatusCreated, + }, + { + name: "webhooks delete", + permission: domain.PermissionWebhooksDelete, + method: http.MethodDelete, + path: func(t *testing.T, ctx context.Context, store *db.Store, orgID uuid.UUID) string { + webhook, err := store.CreateWebhook(ctx, orgID, "https://example.com/delete-hook", []string{"user.login"}, "secret") + if err != nil { + t.Fatalf("CreateWebhook() error = %v", err) + } + return "/api/v1/webhooks/" + webhook.ID.String() + }, + wantStatus: http.StatusOK, + }, + { + name: "organizations read", + permission: domain.PermissionOrganizationsRead, + method: http.MethodGet, + path: func(_ *testing.T, _ context.Context, _ *db.Store, _ uuid.UUID) string { + return "/api/v1/organizations/me" + }, + wantStatus: http.StatusOK, + }, + { + name: "organizations write", + permission: domain.PermissionOrganizationsWrite, + method: http.MethodPut, + path: func(_ *testing.T, _ context.Context, _ *db.Store, _ uuid.UUID) string { + return "/api/v1/organizations/me" + }, + body: `{"name":"Renamed Org"}`, + wantStatus: http.StatusOK, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + org := testutil.CreateOrganization(t, store, "router-allowed-org") + user := testutil.CreateUser(t, store, org.ID, "router-allowed-user") + grantPermissionToUser(t, ctx, store, org.ID, user.ID, tc.permission) + + path := tc.path(t, ctx, store, org.ID) + rec := performAuthedRequest(handler, tc.method, path, tc.body, issueAccessToken(t, user.ID, org.ID)) + if rec.Code != tc.wantStatus { + t.Fatalf("%s %s status = %d, want %d, body=%s", tc.method, path, rec.Code, tc.wantStatus, rec.Body.String()) + } + }) + } +} + +func TestRouterAllowsSuperuserBypass(t *testing.T) { + store := testutil.RequireTestStore(t) + handler := newTestRouter(store) + + tests := []struct { + name string + method string + path func(t *testing.T, ctx context.Context, store *db.Store, orgID uuid.UUID) string + body string + }{ + { + name: "users family", + method: http.MethodGet, + path: func(_ *testing.T, _ context.Context, _ *db.Store, _ uuid.UUID) string { return "/api/v1/users" }, + }, + { + name: "organizations family", + method: http.MethodGet, + path: func(_ *testing.T, _ context.Context, _ *db.Store, _ uuid.UUID) string { + return "/api/v1/organizations/me" + }, + }, + { + name: "roles family", + method: http.MethodGet, + path: func(_ *testing.T, _ context.Context, _ *db.Store, _ uuid.UUID) string { return "/api/v1/roles" }, + }, + { + name: "api keys family", + method: http.MethodGet, + path: func(_ *testing.T, _ context.Context, _ *db.Store, _ uuid.UUID) string { return "/api/v1/api-keys" }, + }, + { + name: "audit family", + method: http.MethodGet, + path: func(_ *testing.T, _ context.Context, _ *db.Store, _ uuid.UUID) string { return "/api/v1/audit" }, + }, + { + name: "webhooks family", + method: http.MethodGet, + path: func(_ *testing.T, _ context.Context, _ *db.Store, _ uuid.UUID) string { return "/api/v1/webhooks" }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + org := testutil.CreateOrganization(t, store, "router-super-org") + superuser := createSuperuser(t, ctx, store, org.ID, "router-super-user") + + path := tc.path(t, ctx, store, org.ID) + rec := performAuthedRequest(handler, tc.method, path, tc.body, issueAccessToken(t, superuser.ID, org.ID)) + if rec.Code != http.StatusOK { + t.Fatalf("%s %s status = %d, want %d, body=%s", tc.method, path, rec.Code, http.StatusOK, rec.Body.String()) + } + }) + } +} + +func newTestRouter(store *db.Store) http.Handler { + cfg := &config.Config{ + Server: config.ServerConfig{ + Environment: "development", + }, + Auth: config.AuthConfig{ + JWTSecret: routerTestJWTSecret, + AccessTokenDuration: 15 * time.Minute, + RefreshTokenDuration: 7 * 24 * time.Hour, + RateLimitPerMinute: 1000, + }, + Email: config.EmailConfig{ + BaseURL: "http://localhost:8080", + }, + } + + return api.NewRouter(cfg, store, nil, testutil.DiscardLogger()) +} + +func issueAccessToken(t *testing.T, userID, orgID uuid.UUID) string { + t.Helper() + + maker := token.NewMaker(routerTestJWTSecret, 15*time.Minute, 7*24*time.Hour) + tokenStr, _, err := maker.CreateAccessToken(userID, orgID) + if err != nil { + t.Fatalf("CreateAccessToken() error = %v", err) + } + return tokenStr +} + +func performAuthedRequest(handler http.Handler, method, path, body, accessToken string) *httptest.ResponseRecorder { + req := httptest.NewRequest(method, path, strings.NewReader(body)) + req.Header.Set("Authorization", "Bearer "+accessToken) + if body != "" { + req.Header.Set("Content-Type", "application/json") + } + + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + return rec +} + +func createSuperuser(t *testing.T, ctx context.Context, store *db.Store, orgID uuid.UUID, prefix string) *domain.User { + t.Helper() + + fullName := prefix + "-name" + email := fmt.Sprintf("%s-%s@example.com", prefix, strings.ToLower(uuid.NewString())) + user, err := store.CreateUser(ctx, orgID, email, "hashed-password", &fullName, true) + if err != nil { + t.Fatalf("CreateUser(superuser) error = %v", err) + } + return user +} + +func grantPermissionToUser(t *testing.T, ctx context.Context, store *db.Store, orgID, userID uuid.UUID, permission string) { + t.Helper() + + role := testutil.CreateRole(t, store, orgID, "router-permission-role") + p, err := store.GetPermissionByName(ctx, permission) + if err != nil { + t.Fatalf("GetPermissionByName(%q) error = %v", permission, err) + } + if err := store.AssignPermissionToRole(ctx, role.ID, p.ID); err != nil { + t.Fatalf("AssignPermissionToRole() error = %v", err) + } + if err := store.AssignRoleToUser(ctx, orgID, userID, role.ID); err != nil { + t.Fatalf("AssignRoleToUser() error = %v", err) + } +} diff --git a/internal/domain/role.go b/internal/domain/role.go index cdead60..372770e 100644 --- a/internal/domain/role.go +++ b/internal/domain/role.go @@ -20,3 +20,21 @@ type Permission struct { Name string // e.g. "users:read", "users:write" Description *string } + +const ( + PermissionUsersRead = "users:read" + PermissionUsersWrite = "users:write" + PermissionUsersDelete = "users:delete" + PermissionRolesRead = "roles:read" + PermissionRolesWrite = "roles:write" + PermissionRolesDelete = "roles:delete" + PermissionAPIKeysRead = "apikeys:read" + PermissionAPIKeysWrite = "apikeys:write" + PermissionAPIKeysDelete = "apikeys:delete" + PermissionAuditRead = "audit:read" + PermissionWebhooksRead = "webhooks:read" + PermissionWebhooksWrite = "webhooks:write" + PermissionWebhooksDelete = "webhooks:delete" + PermissionOrganizationsRead = "organizations:read" + PermissionOrganizationsWrite = "organizations:write" +) diff --git a/internal/repository/postgres/roles.go b/internal/repository/postgres/roles.go index 32fe896..c031d1e 100644 --- a/internal/repository/postgres/roles.go +++ b/internal/repository/postgres/roles.go @@ -173,6 +173,25 @@ func (s *Store) ListPermissionsByUser(ctx context.Context, userID uuid.UUID) ([] return collectPermissions(rows) } +func (s *Store) UserHasPermission(ctx context.Context, orgID, userID uuid.UUID, permission string) (bool, error) { + row := s.pool.QueryRow(ctx, + `SELECT EXISTS ( + SELECT 1 + FROM user_roles ur + JOIN role_permissions rp ON rp.role_id = ur.role_id + JOIN permissions p ON p.id = rp.permission_id + WHERE ur.org_id = $1 AND ur.user_id = $2 AND p.name = $3 + )`, + orgID, userID, permission, + ) + + var allowed bool + if err := row.Scan(&allowed); err != nil { + return false, fmt.Errorf("check user permission: %w", err) + } + return allowed, nil +} + func (s *Store) GetPermissionByName(ctx context.Context, name string) (*domain.Permission, error) { row := s.pool.QueryRow(ctx, `SELECT id, name, description FROM permissions WHERE name = $1`, name) p := &domain.Permission{} diff --git a/internal/repository/postgres/tenant_scope_test.go b/internal/repository/postgres/tenant_scope_test.go index a53d75c..e287719 100644 --- a/internal/repository/postgres/tenant_scope_test.go +++ b/internal/repository/postgres/tenant_scope_test.go @@ -223,3 +223,57 @@ func TestStoreUserRoleTenantIntegrity(t *testing.T) { } }) } + +func TestStoreUserHasPermissionHonorsOrgScope(t *testing.T) { + store := testutil.RequireTestStore(t) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + orgA := testutil.CreateOrganization(t, store, "user-perm-org-a") + orgB := testutil.CreateOrganization(t, store, "user-perm-org-b") + userA := testutil.CreateUser(t, store, orgA.ID, "user-perm-user-a") + userB := testutil.CreateUser(t, store, orgB.ID, "user-perm-user-b") + roleA := testutil.CreateRole(t, store, orgA.ID, "user-perm-role-a") + roleB := testutil.CreateRole(t, store, orgB.ID, "user-perm-role-b") + + perm, err := store.GetPermissionByName(ctx, domain.PermissionUsersRead) + if err != nil { + t.Fatalf("GetPermissionByName() error = %v", err) + } + if err := store.AssignPermissionToRole(ctx, roleA.ID, perm.ID); err != nil { + t.Fatalf("AssignPermissionToRole(roleA) error = %v", err) + } + if err := store.AssignPermissionToRole(ctx, roleB.ID, perm.ID); err != nil { + t.Fatalf("AssignPermissionToRole(roleB) error = %v", err) + } + if err := store.AssignRoleToUser(ctx, orgA.ID, userA.ID, roleA.ID); err != nil { + t.Fatalf("AssignRoleToUser() error = %v", err) + } + if err := store.AssignRoleToUser(ctx, orgB.ID, userB.ID, roleB.ID); err != nil { + t.Fatalf("AssignRoleToUser() error = %v", err) + } + + allowed, err := store.UserHasPermission(ctx, orgA.ID, userA.ID, domain.PermissionUsersRead) + if err != nil { + t.Fatalf("UserHasPermission(orgA, userA) error = %v", err) + } + if !allowed { + t.Fatal("expected userA to have users:read in orgA") + } + + allowed, err = store.UserHasPermission(ctx, orgA.ID, userB.ID, domain.PermissionUsersRead) + if err != nil { + t.Fatalf("UserHasPermission(orgA, userB) error = %v", err) + } + if allowed { + t.Fatal("expected foreign-org user to be denied") + } + + allowed, err = store.UserHasPermission(ctx, orgA.ID, userA.ID, domain.PermissionUsersDelete) + if err != nil { + t.Fatalf("UserHasPermission(orgA, userA, users:delete) error = %v", err) + } + if allowed { + t.Fatal("expected missing permission to be denied") + } +} diff --git a/internal/service/rbac.go b/internal/service/rbac.go index ab820c7..e216cdf 100644 --- a/internal/service/rbac.go +++ b/internal/service/rbac.go @@ -2,6 +2,7 @@ package service import ( "context" + "errors" "fmt" "github.com/google/uuid" @@ -109,16 +110,31 @@ func (s *RBACService) ListUserPermissions(ctx context.Context, userID uuid.UUID) return s.store.ListPermissionsByUser(ctx, userID) } -// HasPermission checks if a user has a specific permission. -func (s *RBACService) HasPermission(ctx context.Context, userID uuid.UUID, permission string) (bool, error) { - perms, err := s.store.ListPermissionsByUser(ctx, userID) +// HasPermission checks if a user has a specific permission within an organization. +func (s *RBACService) HasPermission(ctx context.Context, orgID, userID uuid.UUID, permission string) (bool, error) { + return s.store.UserHasPermission(ctx, orgID, userID, permission) +} + +// Authorize verifies that a user is allowed to perform an action in an organization. +func (s *RBACService) Authorize(ctx context.Context, orgID, userID uuid.UUID, permission string) error { + user, err := s.store.GetUserByID(ctx, orgID, userID) if err != nil { - return false, err - } - for _, p := range perms { - if p.Name == permission { - return true, nil + if errors.Is(err, domain.ErrNotFound) { + return domain.ErrUnauthorized } + return fmt.Errorf("get actor: %w", err) + } + + if user.IsSuperuser { + return nil } - return false, nil + + allowed, err := s.store.UserHasPermission(ctx, orgID, userID, permission) + if err != nil { + return fmt.Errorf("check permission: %w", err) + } + if !allowed { + return domain.ErrForbidden + } + return nil } diff --git a/internal/service/rbac_scope_test.go b/internal/service/rbac_scope_test.go index 7064906..dc19814 100644 --- a/internal/service/rbac_scope_test.go +++ b/internal/service/rbac_scope_test.go @@ -55,3 +55,49 @@ func TestRBACServiceRejectsCrossOrgPermissionAssignment(t *testing.T) { t.Fatalf("expected ErrNotFound for foreign-org permission assignment, got %v", err) } } + +func TestRBACServiceAuthorize(t *testing.T) { + store := testutil.RequireTestStore(t) + svc := NewRBACService(store, NewAuditService(store, testutil.DiscardLogger())) + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + org := testutil.CreateOrganization(t, store, "svc-authorize-org") + regularUser := testutil.CreateUser(t, store, org.ID, "svc-authorize-user") + superuser, err := store.CreateUser(ctx, org.ID, "svc-authorize-superuser@example.com", "hashed-password", nil, true) + if err != nil { + t.Fatalf("CreateUser(superuser) error = %v", err) + } + + t.Run("denies user without permission", func(t *testing.T) { + err := svc.Authorize(ctx, org.ID, regularUser.ID, domain.PermissionUsersRead) + if !errors.Is(err, domain.ErrForbidden) { + t.Fatalf("expected ErrForbidden, got %v", err) + } + }) + + t.Run("allows user with permission", func(t *testing.T) { + role := testutil.CreateRole(t, store, org.ID, "svc-authorize-role") + perm, err := store.GetPermissionByName(ctx, domain.PermissionUsersRead) + if err != nil { + t.Fatalf("GetPermissionByName() error = %v", err) + } + if err := store.AssignPermissionToRole(ctx, role.ID, perm.ID); err != nil { + t.Fatalf("AssignPermissionToRole() error = %v", err) + } + if err := store.AssignRoleToUser(ctx, org.ID, regularUser.ID, role.ID); err != nil { + t.Fatalf("AssignRoleToUser() error = %v", err) + } + + if err := svc.Authorize(ctx, org.ID, regularUser.ID, domain.PermissionUsersRead); err != nil { + t.Fatalf("Authorize() error = %v", err) + } + }) + + t.Run("allows superuser without role assignments", func(t *testing.T) { + if err := svc.Authorize(ctx, org.ID, superuser.ID, domain.PermissionUsersDelete); err != nil { + t.Fatalf("Authorize(superuser) error = %v", err) + } + }) +} diff --git a/migrations/000003_organization_permissions.down.sql b/migrations/000003_organization_permissions.down.sql new file mode 100644 index 0000000..2ca8c71 --- /dev/null +++ b/migrations/000003_organization_permissions.down.sql @@ -0,0 +1,2 @@ +DELETE FROM permissions +WHERE name IN ('organizations:read', 'organizations:write'); diff --git a/migrations/000003_organization_permissions.up.sql b/migrations/000003_organization_permissions.up.sql new file mode 100644 index 0000000..4fcc231 --- /dev/null +++ b/migrations/000003_organization_permissions.up.sql @@ -0,0 +1,4 @@ +INSERT INTO permissions (name, description) VALUES + ('organizations:read', 'Read organization profile data'), + ('organizations:write', 'Update organization profile data') +ON CONFLICT (name) DO NOTHING; diff --git a/sql/schema.sql b/sql/schema.sql index d3de222..2ffa45e 100644 --- a/sql/schema.sql +++ b/sql/schema.sql @@ -85,7 +85,9 @@ INSERT INTO permissions (name, description) VALUES ('audit:read', 'Read audit logs'), ('webhooks:read', 'List webhooks'), ('webhooks:write', 'Create and update webhooks'), - ('webhooks:delete', 'Delete webhooks'); + ('webhooks:delete', 'Delete webhooks'), + ('organizations:read', 'Read organization profile data'), + ('organizations:write','Update organization profile data'); -- Role <-> Permission CREATE TABLE role_permissions ( From 709891d34de57d409a045908706f37d64c64e5aa Mon Sep 17 00:00:00 2001 From: osama1998H Date: Sat, 28 Mar 2026 01:16:06 +0300 Subject: [PATCH 2/2] Fix router panics and nullable scan handling --- internal/api/middleware/ratelimit.go | 17 +++- internal/api/middleware/ratelimit_test.go | 15 +++ internal/api/router_authorization_test.go | 2 +- internal/repository/postgres/apikeys.go | 21 +++- .../repository/postgres/null_scans_test.go | 96 +++++++++++++++++++ internal/repository/postgres/users.go | 23 ++++- 6 files changed, 168 insertions(+), 6 deletions(-) create mode 100644 internal/repository/postgres/null_scans_test.go diff --git a/internal/api/middleware/ratelimit.go b/internal/api/middleware/ratelimit.go index b558911..c8a5f38 100644 --- a/internal/api/middleware/ratelimit.go +++ b/internal/api/middleware/ratelimit.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "reflect" "time" ) @@ -18,7 +19,7 @@ func RateLimit(redisCache rateLimitCounter, requestsPerMinute int) func(next htt ip := ClientIP(r) key := fmt.Sprintf("rl:%s", ip) - if redisCache == nil { + if isNilRateLimitCounter(redisCache) { next.ServeHTTP(w, r) return } @@ -42,3 +43,17 @@ func RateLimit(redisCache rateLimitCounter, requestsPerMinute int) func(next htt }) } } + +func isNilRateLimitCounter(counter rateLimitCounter) bool { + if counter == nil { + return true + } + + value := reflect.ValueOf(counter) + switch value.Kind() { + case reflect.Chan, reflect.Func, reflect.Interface, reflect.Map, reflect.Pointer, reflect.Slice: + return value.IsNil() + default: + return false + } +} diff --git a/internal/api/middleware/ratelimit_test.go b/internal/api/middleware/ratelimit_test.go index 19413e5..eaa6c1f 100644 --- a/internal/api/middleware/ratelimit_test.go +++ b/internal/api/middleware/ratelimit_test.go @@ -147,6 +147,21 @@ func TestRateLimitUsesResolvedClientIP(t *testing.T) { }) } +func TestRateLimitAllowsTypedNilCounter(t *testing.T) { + t.Parallel() + + var counter *fakeRateLimitCounter + handler := RateLimit(counter, 10)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNoContent) + })) + + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/", nil)) + if rec.Code != http.StatusNoContent { + t.Fatalf("expected 204, got %d", rec.Code) + } +} + type fakeRateLimitCounter struct { counts map[string]int64 keys []string diff --git a/internal/api/router_authorization_test.go b/internal/api/router_authorization_test.go index a20728d..8b8a5e6 100644 --- a/internal/api/router_authorization_test.go +++ b/internal/api/router_authorization_test.go @@ -176,7 +176,7 @@ func TestRouterAllowsGrantedPermissions(t *testing.T) { permission: domain.PermissionAPIKeysDelete, method: http.MethodDelete, path: func(t *testing.T, ctx context.Context, store *db.Store, orgID uuid.UUID) string { - key, err := store.CreateAPIKey(ctx, orgID, "router-delete-key", "uni", uuid.NewString(), nil, nil) + key, err := store.CreateAPIKey(ctx, orgID, "router-delete-key", "uni", uuid.NewString(), []string{}, nil) if err != nil { t.Fatalf("CreateAPIKey() error = %v", err) } diff --git a/internal/repository/postgres/apikeys.go b/internal/repository/postgres/apikeys.go index 0f15897..add4a7f 100644 --- a/internal/repository/postgres/apikeys.go +++ b/internal/repository/postgres/apikeys.go @@ -2,6 +2,7 @@ package db import ( "context" + "database/sql" "errors" "fmt" "time" @@ -13,6 +14,10 @@ import ( ) func (s *Store) CreateAPIKey(ctx context.Context, orgID uuid.UUID, name, keyPrefix, keyHash string, scopes []string, expiresAt *time.Time) (*domain.APIKey, error) { + if scopes == nil { + scopes = []string{} + } + row := s.pool.QueryRow(ctx, `INSERT INTO api_keys (org_id, name, key_prefix, key_hash, scopes, expires_at) VALUES ($1, $2, $3, $4, $5, $6) @@ -69,16 +74,28 @@ func (s *Store) UpdateAPIKeyLastUsed(ctx context.Context, id uuid.UUID) error { func scanAPIKey(row pgx.Row) (*domain.APIKey, error) { k := &domain.APIKey{} - if err := row.Scan(&k.ID, &k.OrgID, &k.Name, &k.KeyPrefix, &k.KeyHash, &k.Scopes, &k.ExpiresAt, &k.LastUsedAt, &k.RevokedAt, &k.CreatedAt); err != nil { + var expiresAt sql.NullTime + var lastUsedAt sql.NullTime + var revokedAt sql.NullTime + if err := row.Scan(&k.ID, &k.OrgID, &k.Name, &k.KeyPrefix, &k.KeyHash, &k.Scopes, &expiresAt, &lastUsedAt, &revokedAt, &k.CreatedAt); err != nil { return nil, fmt.Errorf("scan api key: %w", err) } + k.ExpiresAt = nullTimePtr(expiresAt) + k.LastUsedAt = nullTimePtr(lastUsedAt) + k.RevokedAt = nullTimePtr(revokedAt) return k, nil } func scanAPIKeyRow(rows pgx.Rows) (*domain.APIKey, error) { k := &domain.APIKey{} - if err := rows.Scan(&k.ID, &k.OrgID, &k.Name, &k.KeyPrefix, &k.KeyHash, &k.Scopes, &k.ExpiresAt, &k.LastUsedAt, &k.RevokedAt, &k.CreatedAt); err != nil { + var expiresAt sql.NullTime + var lastUsedAt sql.NullTime + var revokedAt sql.NullTime + if err := rows.Scan(&k.ID, &k.OrgID, &k.Name, &k.KeyPrefix, &k.KeyHash, &k.Scopes, &expiresAt, &lastUsedAt, &revokedAt, &k.CreatedAt); err != nil { return nil, fmt.Errorf("scan api key row: %w", err) } + k.ExpiresAt = nullTimePtr(expiresAt) + k.LastUsedAt = nullTimePtr(lastUsedAt) + k.RevokedAt = nullTimePtr(revokedAt) return k, nil } diff --git a/internal/repository/postgres/null_scans_test.go b/internal/repository/postgres/null_scans_test.go new file mode 100644 index 0000000..9b9346c --- /dev/null +++ b/internal/repository/postgres/null_scans_test.go @@ -0,0 +1,96 @@ +package db_test + +import ( + "context" + "testing" + "time" + + "github.com/osama1998h/uniauth/internal/testutil" + "github.com/osama1998h/uniauth/pkg/token" +) + +func TestStoreUserHandlesNullableTimestamps(t *testing.T) { + store := testutil.RequireTestStore(t) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + org := testutil.CreateOrganization(t, store, "nullable-user-org") + user := testutil.CreateUser(t, store, org.ID, "nullable-user") + + got, err := store.GetUserByID(ctx, org.ID, user.ID) + if err != nil { + t.Fatalf("GetUserByID() error = %v", err) + } + if got.EmailVerifiedAt != nil { + t.Fatalf("EmailVerifiedAt = %v, want nil", got.EmailVerifiedAt) + } + if got.LastLoginAt != nil { + t.Fatalf("LastLoginAt = %v, want nil", got.LastLoginAt) + } + + users, err := store.ListUsersByOrg(ctx, org.ID, 10, 0) + if err != nil { + t.Fatalf("ListUsersByOrg() error = %v", err) + } + if len(users) == 0 { + t.Fatal("expected at least one user in organization") + } + + updatedName := "Nullable User" + updatedEmail := "nullable-user-updated@example.com" + updated, err := store.UpdateUser(ctx, org.ID, user.ID, &updatedName, &updatedEmail) + if err != nil { + t.Fatalf("UpdateUser() error = %v", err) + } + if updated.EmailVerifiedAt != nil { + t.Fatalf("updated EmailVerifiedAt = %v, want nil", updated.EmailVerifiedAt) + } + if updated.LastLoginAt != nil { + t.Fatalf("updated LastLoginAt = %v, want nil", updated.LastLoginAt) + } +} + +func TestStoreAPIKeyHandlesNullableFieldsAndNilScopes(t *testing.T) { + store := testutil.RequireTestStore(t) + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + org := testutil.CreateOrganization(t, store, "nullable-api-key-org") + keyHash := token.HashAPIKey("router-nullable-api-key") + + key, err := store.CreateAPIKey(ctx, org.ID, "nullable-api-key", "uni", keyHash, nil, nil) + if err != nil { + t.Fatalf("CreateAPIKey() error = %v", err) + } + if len(key.Scopes) != 0 { + t.Fatalf("created scopes = %v, want empty", key.Scopes) + } + if key.ExpiresAt != nil || key.LastUsedAt != nil || key.RevokedAt != nil { + t.Fatalf("created nullable timestamps = (%v, %v, %v), want nil", key.ExpiresAt, key.LastUsedAt, key.RevokedAt) + } + + got, err := store.GetAPIKeyByHash(ctx, keyHash) + if err != nil { + t.Fatalf("GetAPIKeyByHash() error = %v", err) + } + if len(got.Scopes) != 0 { + t.Fatalf("loaded scopes = %v, want empty", got.Scopes) + } + if got.ExpiresAt != nil || got.LastUsedAt != nil || got.RevokedAt != nil { + t.Fatalf("loaded nullable timestamps = (%v, %v, %v), want nil", got.ExpiresAt, got.LastUsedAt, got.RevokedAt) + } + + keys, err := store.ListAPIKeysByOrg(ctx, org.ID) + if err != nil { + t.Fatalf("ListAPIKeysByOrg() error = %v", err) + } + if len(keys) != 1 { + t.Fatalf("expected 1 api key, got %d", len(keys)) + } + if len(keys[0].Scopes) != 0 { + t.Fatalf("listed scopes = %v, want empty", keys[0].Scopes) + } + if keys[0].ExpiresAt != nil || keys[0].LastUsedAt != nil || keys[0].RevokedAt != nil { + t.Fatalf("listed nullable timestamps = (%v, %v, %v), want nil", keys[0].ExpiresAt, keys[0].LastUsedAt, keys[0].RevokedAt) + } +} diff --git a/internal/repository/postgres/users.go b/internal/repository/postgres/users.go index aae0f92..0e07d3a 100644 --- a/internal/repository/postgres/users.go +++ b/internal/repository/postgres/users.go @@ -2,8 +2,10 @@ package db import ( "context" + "database/sql" "errors" "fmt" + "time" "github.com/google/uuid" "github.com/jackc/pgx/v5" @@ -106,14 +108,18 @@ func (s *Store) DeactivateUser(ctx context.Context, orgID, id uuid.UUID) error { func scanUser(row pgx.Row) (*domain.User, error) { u := &domain.User{} + var emailVerifiedAt sql.NullTime + var lastLoginAt sql.NullTime err := row.Scan( &u.ID, &u.OrgID, &u.Email, &u.HashedPassword, &u.FullName, - &u.IsActive, &u.IsSuperuser, &u.EmailVerifiedAt, &u.LastLoginAt, + &u.IsActive, &u.IsSuperuser, &emailVerifiedAt, &lastLoginAt, &u.CreatedAt, &u.UpdatedAt, ) if err != nil { return nil, fmt.Errorf("scan user: %w", err) } + u.EmailVerifiedAt = nullTimePtr(emailVerifiedAt) + u.LastLoginAt = nullTimePtr(lastLoginAt) return u, nil } @@ -121,14 +127,27 @@ func collectUsers(rows pgx.Rows) ([]*domain.User, error) { var users []*domain.User for rows.Next() { u := &domain.User{} + var emailVerifiedAt sql.NullTime + var lastLoginAt sql.NullTime if err := rows.Scan( &u.ID, &u.OrgID, &u.Email, &u.HashedPassword, &u.FullName, - &u.IsActive, &u.IsSuperuser, &u.EmailVerifiedAt, &u.LastLoginAt, + &u.IsActive, &u.IsSuperuser, &emailVerifiedAt, &lastLoginAt, &u.CreatedAt, &u.UpdatedAt, ); err != nil { return nil, fmt.Errorf("scan user row: %w", err) } + u.EmailVerifiedAt = nullTimePtr(emailVerifiedAt) + u.LastLoginAt = nullTimePtr(lastLoginAt) users = append(users, u) } return users, rows.Err() } + +func nullTimePtr(value sql.NullTime) *time.Time { + if !value.Valid { + return nil + } + + t := value.Time + return &t +}