diff --git a/.dockerignore b/.dockerignore index 93e14b7..cc30164 100644 --- a/.dockerignore +++ b/.dockerignore @@ -3,4 +3,5 @@ bin coverage.out google_secrets.json +data diff --git a/Makefile b/Makefile index a0ace0d..7b36763 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,7 @@ SHELL := /bin/sh GO_LOOSE_ADDR ?= :8081 +GO_LOOSE_PORT ?= 8081 GO_LOOSE_BASE_URL ?= http://localhost:$(GO_LOOSE_PORT) GO_LOOSE_DATABASE_URL ?= postgres://goloose:goloose@localhost:5433/goloose?sslmode=disable GO_LOOSE_SESSION_SECRET ?= local-development-secret-change-me-now @@ -29,8 +30,12 @@ integration-test: run: go run ./cmd/go-loose +# The variables above are exported for `make run`, but compose reads +# GO_LOOSE_BASE_URL too and would inherit the local http://localhost:8081 value, +# which then overrides the https:// defaults in docker-compose.yaml. Clearing it +# for the compose targets lets compose apply its own defaults. compose-up: - docker compose up --build + GO_LOOSE_BASE_URL= docker compose up --build compose-down: docker compose down diff --git a/docs/roles.md b/docs/roles.md index 366f9b1..3016b7a 100644 --- a/docs/roles.md +++ b/docs/roles.md @@ -1,36 +1,71 @@ # Roles and capabilities -Every tenant role maps to a fixed set of capabilities in `internal/role`. The console reads the capability list from the server instead of hardcoding role names, and store queries pass `role.Grants(...)` into SQL so authorization and rendering cannot drift apart. +A role is a named bundle of permissions. Roles are rows: six built-in ones seeded by migration `0006_database_roles.sql`, plus whatever each tenant defines for itself. `internal/role` no longer holds a matrix — it holds the closed vocabulary of permissions a role may be granted, and the database resolves what a membership actually grants. -## Capability matrix +``` +roles one row per role. tenant_id NULL means built-in. +role_permissions (role_id, permission) — what the role grants. +memberships.role_id → roles.id, NOT NULL, ON DELETE RESTRICT +``` -| Capability | owner | admin | developer | operator | viewer | user | +## Built-in capability matrix + +This is what the migration seeds. It is data, not code: change it with a new migration, not by editing Go. + +| Permission | owner | admin | developer | operator | viewer | user | | --- | --- | --- | --- | --- | --- | --- | -| `console.read` | yes | yes | yes | yes | yes | no | +| `tenants.manage` | yes | no | no | no | no | no | +| `roles.manage` | yes | yes | no | no | no | no | +| `users.manage` | yes | yes | yes | yes | no | no | | `applications.manage` | yes | yes | yes | no | no | no | | `keys.manage` | yes | yes | yes | yes | no | no | | `contracts.manage` | yes | yes | yes | yes | no | no | -| `users.manage` | yes | yes | no | no | no | no | +| `users.read` | yes | yes | yes | yes | yes | no | +| `console.read` | yes | yes | yes | yes | yes | no | | `profile.edit` | yes | yes | yes | yes | yes | yes | Intended use: -- **owner** — full control of the tenant, including role assignment. -- **admin** — manages applications, keys, contracts, and members. -- **developer** — ships service contracts and API keys, but cannot change membership. -- **operator** — runs existing applications: rotates keys and imports contracts. +- **owner** — full control of the tenant, including defining roles and handing them out. +- **admin** — manages applications, keys, contracts, members, and roles. +- **developer** — ships applications, keys, and contracts, and maintains the tenant's members. +- **operator** — runs existing applications and maintains the tenant's members. - **viewer** — read-only console access. - **user** — self-service only. No console access; the session lands on `/profile`. +`developer` and `operator` hold `users.manage` so that anyone who runs the console can maintain their own tenant's members. `roles.manage` deliberately stays narrow: defining roles is what defines privilege, so it is not something a member administrator can grant themselves. + `user` members without any tenant membership still see the waiting room. Those with a membership are redirected from `/` to `/profile`, because the dashboard requires `console.read`. +## Tenant-defined roles + +`POST /api/v1/roles` creates a role for one tenant. It is visible only in that tenant, and the `/roles` screen lists it beside the built-ins with its permission chips. + +Three rules keep this from becoming a privilege-escalation hole: + +1. **Roles are scoped.** `roles.tenant_id` is set, and every query that resolves a role filters on the caller's own tenant, so a role id cannot be probed across tenants. +2. **You cannot mint what you do not hold.** Creating or editing a role requires `roles.manage` *and* requires every permission in the role to be one the caller already has. Assigning a role enforces the same subset rule. +3. **A role in use cannot be deleted.** `memberships.role_id` is `ON DELETE RESTRICT`, and `DELETE /api/v1/roles/{id}` reports 409 until the members are moved. Built-in roles are immutable and report 404. + +Built-in roles are read only: the store refuses to edit or delete a row with `is_system` set. + ## Where the checks live -Store methods enforce capabilities inside SQL (`role = ANY($n)` with a text array) or inside the same transaction as the write. The server additionally filters and labels UI controls with `role.Allows`. Both layers read the same matrix, so adding a capability or a role is a one-line change in `internal/role/role.go`. +Tenant-scoped statements resolve the grant through `role_permissions` using one shared predicate, `permissionPredicate` in `internal/store/roles.go`: -`role.Role` values are also validated by a database check constraint on `memberships.role`, so an unknown role cannot be stored even if a caller bypasses the Go layer. +```sql +EXISTS ( + SELECT 1 + FROM memberships scoped + JOIN role_permissions granted ON granted.role_id = scoped.role_id + WHERE scoped.tenant_id = $1 AND scoped.user_id = $2 + AND granted.permission = $3 +) +``` -New roles must be added to the check constraint in a new migration as well as to the matrix; existing deployments keep the constraint in `internal/database/migrations/0005_roles_and_profile.sql` for historical reasons. +Because the check asks for a permission rather than a role name, a tenant's own role works everywhere a built-in one does without any Go change. The server never decides authorization: `handleStoreError` maps `store.ErrNotFound` to a 404 so a denied write and a missing row are indistinguishable. + +`role.Permission` names are also constrained by a check constraint on `role_permissions.permission`, so an unknown capability cannot be stored even if a caller bypasses the Go layer. **Adding a permission therefore needs three edits**: the constant in `internal/role/role.go`, the check constraint and seed in a new migration, and the expected grants in `internal/store/roles_integration_test.go`. ## Display names and profile pictures @@ -50,4 +85,4 @@ Accepted formats are JPEG, PNG, and GIF up to 2 MiB and 4096 by 4096 pixels. Rep `/connect/token` and `/connect/userinfo` return the caller's `role`, `permissions`, and absolute `avatar_url`, resolved against the tenant that owns the application. `client.User.Can` exposes the same checks to Go code, so a client can adapt its UI without duplicating the matrix. -A user with no membership in the application's tenant still authenticates, but the role falls back to `user` with only `profile.edit`. \ No newline at end of file +A user with no membership in the application's tenant still authenticates, but the role falls back to the built-in `user` role with only `profile.edit`. \ No newline at end of file diff --git a/internal/database/migrations/0006_database_roles.sql b/internal/database/migrations/0006_database_roles.sql new file mode 100644 index 0000000..d8bd84d --- /dev/null +++ b/internal/database/migrations/0006_database_roles.sql @@ -0,0 +1,106 @@ +-- +goose Up +-- Roles become rows so a tenant can define its own. A NULL tenant_id marks a +-- built-in role: every tenant sees it and nobody may edit or delete it. +CREATE TABLE roles ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + tenant_id uuid REFERENCES tenants(id) ON DELETE CASCADE, + slug text NOT NULL CHECK (slug ~ '^[a-z0-9][a-z0-9-]{0,62}$'), + name text NOT NULL CHECK (length(btrim(name)) BETWEEN 1 AND 100), + description text NOT NULL DEFAULT '', + is_system boolean NOT NULL DEFAULT false, + created_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now(), + -- Custom roles belong to exactly one tenant; built-in roles to none. + CONSTRAINT roles_scope CHECK ( + (is_system AND tenant_id IS NULL) OR (NOT is_system AND tenant_id IS NOT NULL) + ) +); + +CREATE UNIQUE INDEX roles_system_slug_unique ON roles (slug) WHERE tenant_id IS NULL; +CREATE UNIQUE INDEX roles_tenant_slug_unique ON roles (tenant_id, slug) WHERE tenant_id IS NOT NULL; + +-- The permission vocabulary is closed, so keep this list in step with the +-- constants in internal/role. +CREATE TABLE role_permissions ( + role_id uuid NOT NULL REFERENCES roles(id) ON DELETE CASCADE, + permission text NOT NULL CHECK (permission IN ( + 'console.read', 'users.read', 'applications.manage', 'keys.manage', + 'contracts.manage', 'users.manage', 'roles.manage', 'tenants.manage', + 'profile.edit')), + created_at timestamptz NOT NULL DEFAULT now(), + PRIMARY KEY (role_id, permission) +); + +INSERT INTO roles (slug, name, description, is_system) VALUES + ('owner', 'Owner', 'Full control of the tenant, including roles, members, and tenant settings.', true), + ('admin', 'Administrator', 'Manages applications, credentials, contracts, members, and roles.', true), + ('developer', 'Developer', 'Manages applications, credentials, contracts, and members.', true), + ('operator', 'Operator', 'Issues and revokes credentials, imports contracts, and manages members.', true), + ('viewer', 'Viewer', 'Reads the console without changing anything.', true), + ('user', 'User', 'Self-service member with no console access.', true); + +-- Seed what used to be the Go matrix. developer and operator join owner and +-- admin in holding users.manage so that any tenant member who runs the console +-- can maintain their own tenant's members. roles.manage stays with owner and +-- admin: minting roles is what defines privilege in the first place. +INSERT INTO role_permissions (role_id, permission) +SELECT r.id, granted.permission +FROM (VALUES + ('owner', ARRAY[ + 'console.read', 'users.read', 'applications.manage', 'keys.manage', + 'contracts.manage', 'users.manage', 'roles.manage', 'tenants.manage', + 'profile.edit']), + ('admin', ARRAY[ + 'console.read', 'users.read', 'applications.manage', 'keys.manage', + 'contracts.manage', 'users.manage', 'roles.manage', 'profile.edit']), + ('developer', ARRAY[ + 'console.read', 'users.read', 'applications.manage', 'keys.manage', + 'contracts.manage', 'users.manage', 'profile.edit']), + ('operator', ARRAY[ + 'console.read', 'users.read', 'keys.manage', 'contracts.manage', + 'users.manage', 'profile.edit']), + ('viewer', ARRAY['console.read', 'users.read', 'profile.edit']), + ('user', ARRAY['profile.edit']) +) AS seed(slug, permissions) +JOIN roles r ON r.slug = seed.slug AND r.tenant_id IS NULL +CROSS JOIN LATERAL unnest(seed.permissions) AS granted(permission); + +-- Point every membership at a role row. RESTRICT rather than CASCADE so that a +-- role still held by somebody cannot be deleted out from under them. +ALTER TABLE memberships ADD COLUMN role_id uuid REFERENCES roles(id) ON DELETE RESTRICT; + +UPDATE memberships m SET role_id = r.id +FROM roles r +WHERE r.tenant_id IS NULL AND r.slug = m.role; + +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM memberships WHERE role_id IS NULL) THEN + RAISE EXCEPTION 'cannot migrate memberships: a membership names a role that was never seeded'; + END IF; +END +$$; + +ALTER TABLE memberships ALTER COLUMN role_id SET NOT NULL; +ALTER TABLE memberships DROP CONSTRAINT memberships_role_check; +ALTER TABLE memberships DROP COLUMN role; + +-- +goose Down +ALTER TABLE memberships ADD COLUMN role text; + +UPDATE memberships m SET role = r.slug +FROM roles r +WHERE r.id = m.role_id AND r.is_system; + +-- Custom roles have no equivalent in the old fixed vocabulary. +UPDATE memberships SET role = 'viewer' WHERE role IS NULL; + +ALTER TABLE memberships DROP CONSTRAINT memberships_role_check; +ALTER TABLE memberships + ADD CONSTRAINT memberships_role_check + CHECK (role IN ('owner', 'admin', 'developer', 'operator', 'viewer', 'user')); +ALTER TABLE memberships ALTER COLUMN role SET NOT NULL; +ALTER TABLE memberships DROP COLUMN role_id; + +DROP TABLE role_permissions; +DROP TABLE roles; diff --git a/internal/role/role.go b/internal/role/role.go index c4f3bda..bbaa361 100644 --- a/internal/role/role.go +++ b/internal/role/role.go @@ -1,175 +1,132 @@ -// Package role defines the tenant membership roles and the single permission -// matrix that authorizes console and management behavior. +// Package role defines the permission vocabulary that authorizes console and +// management behavior. // -// Roles are stored in memberships.role. Authorization decisions must derive -// from Grants or Allows so that SQL role checks and Go checks cannot drift -// apart. +// Roles are rows in the roles table and their permissions live in +// role_permissions, so a tenant can define its own and authorization is +// resolved by the database rather than from a matrix held here. What belongs in +// this package is the closed set of permissions a role may be granted, plus the +// labels the console renders. +// +// Keep the permission list in step with the role_permissions_permission_check +// constraint in migration 0006: a name that is not in both places cannot be +// stored. package role -// Role is a tenant membership role. -type Role string - -const ( - // Owner has full control of a tenant, including tenant-level settings. - Owner Role = "owner" - // Admin manages applications, credentials, contracts, and members. - Admin Role = "admin" - // Developer manages applications, credentials, and contracts. - Developer Role = "developer" - // Operator issues and revokes credentials and imports contracts. - Operator Role = "operator" - // Viewer reads the console without changing anything. - Viewer Role = "viewer" - // User is a self-service member. It carries no console access and exists so - // that end users of client applications still hold a tenant role that - // applications can read through /connect/userinfo. - User Role = "user" -) - // Permission is a capability granted by a role. type Permission string const ( - ReadConsole Permission = "console.read" - ReadUsers Permission = "users.read" + // ReadConsole opens the management console. Self-service members lack it so + // that they are routed to their profile instead. + ReadConsole Permission = "console.read" + // ReadUsers lists a tenant's members and their roles. + ReadUsers Permission = "users.read" + // ManageApplications registers applications and configures their client + // credentials. ManageApplications Permission = "applications.manage" - ManageKeys Permission = "keys.manage" - ManageContracts Permission = "contracts.manage" - ManageUsers Permission = "users.manage" - ManageTenants Permission = "tenants.manage" - EditProfile Permission = "profile.edit" + // ManageKeys issues and revokes API keys. + ManageKeys Permission = "keys.manage" + // ManageContracts imports OpenAPI contracts. + ManageContracts Permission = "contracts.manage" + // ManageUsers invites members and changes the roles and application grants + // of existing ones. + ManageUsers Permission = "users.manage" + // ManageRoles creates and edits the tenant's own roles. Kept separate from + // ManageUsers because defining roles is what defines privilege. + ManageRoles Permission = "roles.manage" + // ManageTenants creates tenants and administers the installation. + ManageTenants Permission = "tenants.manage" + // EditProfile changes one's own display name and picture. + EditProfile Permission = "profile.edit" ) -// catalog lists every role in descending order of privilege. -var catalog = []Role{Owner, Admin, Developer, Operator, Viewer, User} - -// grants is the permission matrix. ReadConsole is deliberately absent for User -// so that self-service members are routed to the profile screen instead of the -// management console. -var grants = map[Role]map[Permission]bool{ - Owner: { - ReadConsole: true, ReadUsers: true, ManageApplications: true, ManageKeys: true, - ManageContracts: true, ManageUsers: true, ManageTenants: true, EditProfile: true, - }, - Admin: { - ReadConsole: true, ReadUsers: true, ManageApplications: true, ManageKeys: true, - ManageContracts: true, ManageUsers: true, EditProfile: true, - }, - Developer: { - ReadConsole: true, ReadUsers: true, ManageApplications: true, ManageKeys: true, - ManageContracts: true, EditProfile: true, - }, - Operator: { - ReadConsole: true, ReadUsers: true, ManageKeys: true, ManageContracts: true, EditProfile: true, - }, - Viewer: { - ReadConsole: true, ReadUsers: true, EditProfile: true, - }, - User: { - EditProfile: true, - }, -} - +// ordered lists every permission from most to least sensitive. The order is the +// one the console renders checkboxes in. var ordered = []Permission{ - ReadConsole, ReadUsers, ManageApplications, ManageKeys, - ManageContracts, ManageUsers, ManageTenants, EditProfile, + ManageTenants, + ManageRoles, + ManageUsers, + ManageApplications, + ManageKeys, + ManageContracts, + ReadUsers, + ReadConsole, + EditProfile, } -// Entry describes a role for the console and for API clients. +var known = map[Permission]bool{ + ReadConsole: true, + ReadUsers: true, + ManageApplications: true, + ManageKeys: true, + ManageContracts: true, + ManageUsers: true, + ManageRoles: true, + ManageTenants: true, + EditProfile: true, +} + +// Entry describes a permission for the console and for API clients. type Entry struct { - Value string `json:"value"` - Label string `json:"label"` - Permissions []string `json:"permissions"` + Value string `json:"value"` + Label string `json:"label"` + Description string `json:"description"` } -var labels = map[Role]string{ - Owner: "Owner", - Admin: "Administrator", - Developer: "Developer", - Operator: "Operator", - Viewer: "Viewer", - User: "User", +var details = map[Permission]struct{ label, description string }{ + ReadConsole: {"Read console", "Open the management console and pick tenants."}, + ReadUsers: {"Read members", "List the members of a tenant and the roles they hold."}, + ManageApplications: {"Manage applications", "Register applications and configure client credentials."}, + ManageKeys: {"Manage API keys", "Issue and revoke API keys."}, + ManageContracts: {"Manage contracts", "Import OpenAPI contracts and their endpoints."}, + ManageUsers: {"Manage members", "Invite members and change their roles and application grants."}, + ManageRoles: {"Manage roles", "Create and edit the roles of this tenant."}, + ManageTenants: {"Manage tenants", "Create tenants and administer the installation."}, + EditProfile: {"Edit own profile", "Change one's own display name and picture."}, } -// All returns every role ordered from most to least privileged. -func All() []Role { - result := make([]Role, len(catalog)) - copy(result, catalog) +// Permissions returns every permission in display order. +func Permissions() []Permission { + result := make([]Permission, len(ordered)) + copy(result, ordered) return result } -// Catalog returns every role with its label and permissions so that the console -// and API clients never hardcode the role list. -func Catalog() []Entry { - result := make([]Entry, 0, len(catalog)) - for _, item := range catalog { - result = append(result, Entry{Value: string(item), Label: labels[item], Permissions: item.Permissions()}) - } - return result -} - -// Parse converts a stored or submitted value into a Role. -func Parse(value string) (Role, bool) { - candidate := Role(value) - if _, known := grants[candidate]; !known { +// Parse validates a submitted permission name. Unknown values are rejected so +// that a crafted payload cannot invent capabilities. +func Parse(value string) (Permission, bool) { + candidate := Permission(value) + if !known[candidate] { return "", false } return candidate, true } -// Valid reports whether the value names a role. -func (r Role) Valid() bool { - _, known := grants[r] - return known +// Valid reports whether the value names a permission. +func (p Permission) Valid() bool { + return known[p] } -// Allows reports whether the role grants the permission. -func (r Role) Allows(permission Permission) bool { - return grants[r][permission] +// Catalog returns every permission with a label and description so that the +// console and API clients never hardcode the list. +func Catalog() []Entry { + result := make([]Entry, 0, len(ordered)) + for _, permission := range ordered { + detail := details[permission] + result = append(result, Entry{ + Value: string(permission), + Label: detail.label, + Description: detail.description, + }) + } + return result } -// Permissions returns the role's permissions in a stable order. -func (r Role) Permissions() []string { +// Values returns every permission as strings, for use as a SQL array parameter. +func Values() []string { result := make([]string, 0, len(ordered)) for _, permission := range ordered { - if r.Allows(permission) { - result = append(result, string(permission)) - } - } - return result -} - -// Grants returns the roles that grant a permission, for use as a SQL parameter -// so that role checks in queries stay aligned with this matrix. -func Grants(permission Permission) []string { - result := make([]string, 0, len(catalog)) - for _, item := range catalog { - if item.Allows(permission) { - result = append(result, string(item)) - } - } - return result -} - -// Grantable returns the roles a tenant administrator may hand to a new member. -// Ownership is not grantable during an invitation because it is established by -// creating or claiming a tenant. -func Grantable() []Role { - result := make([]Role, 0, len(catalog)-1) - for _, item := range catalog { - if item != Owner { - result = append(result, item) - } - } - return result -} - -// GrantableValues returns Grantable as strings for a request payload. -func GrantableValues() []string { - values := Grantable() - result := make([]string, len(values)) - for index, item := range values { - result[index] = string(item) + result = append(result, string(permission)) } return result } diff --git a/internal/role/role_test.go b/internal/role/role_test.go index d22a107..749b3a5 100644 --- a/internal/role/role_test.go +++ b/internal/role/role_test.go @@ -6,130 +6,112 @@ import ( "testing" ) -func TestParseRejectsUnknownRoles(t *testing.T) { - tests := []struct { - value string - want bool - }{ - {value: "owner", want: true}, - {value: "admin", want: true}, - {value: "developer", want: true}, - {value: "operator", want: true}, - {value: "viewer", want: true}, - {value: "user", want: true}, - {value: "Owner", want: false}, - {value: "", want: false}, - {value: "superuser", want: false}, - {value: "owner; DROP TABLE memberships", want: false}, +func TestParseRejectsUnknownPermissions(t *testing.T) { + valid := []string{ + "console.read", "users.read", "applications.manage", "keys.manage", + "contracts.manage", "users.manage", "roles.manage", "tenants.manage", + "profile.edit", } - for _, test := range tests { - t.Run(test.value, func(t *testing.T) { - _, ok := Parse(test.value) - if ok != test.want { - t.Fatalf("Parse(%q) ok = %v, want %v", test.value, ok, test.want) - } - }) + for _, value := range valid { + if _, ok := Parse(value); !ok { + t.Errorf("Parse(%q) = not ok, want a permission", value) + } } -} - -func TestPermissionMatrix(t *testing.T) { - tests := []struct { - role Role - permission Permission - want bool - }{ - {role: Owner, permission: ManageTenants, want: true}, - {role: Owner, permission: ManageUsers, want: true}, - {role: Admin, permission: ManageTenants, want: false}, - {role: Admin, permission: ManageUsers, want: true}, - {role: Admin, permission: ManageApplications, want: true}, - {role: Developer, permission: ManageUsers, want: false}, - {role: Developer, permission: ManageApplications, want: true}, - {role: Developer, permission: ManageKeys, want: true}, - {role: Developer, permission: ManageContracts, want: true}, - {role: Operator, permission: ManageApplications, want: false}, - {role: Operator, permission: ManageKeys, want: true}, - {role: Operator, permission: ManageContracts, want: true}, - {role: Viewer, permission: ManageKeys, want: false}, - {role: Viewer, permission: ReadConsole, want: true}, - {role: User, permission: ReadConsole, want: false}, - {role: User, permission: ReadUsers, want: false}, - {role: User, permission: EditProfile, want: true}, - } - for _, test := range tests { - t.Run(string(test.role)+"/"+string(test.permission), func(t *testing.T) { - if got := test.role.Allows(test.permission); got != test.want { - t.Fatalf("Allows(%q) = %v, want %v", test.permission, got, test.want) - } - }) - } -} - -func TestEveryRoleCanEditItsOwnProfile(t *testing.T) { - for _, item := range All() { - if !item.Allows(EditProfile) { - t.Fatalf("role %q cannot edit its own profile", item) + for _, value := range []string{"", "Console.read", "console", "superuser", "users.manage; DROP TABLE roles"} { + if parsed, ok := Parse(value); ok { + t.Errorf("Parse(%q) = %q, want rejection", value, parsed) } } } -func TestGrantsMatchAllows(t *testing.T) { - for _, permission := range ordered { - granted := Grants(permission) - for _, item := range All() { - if slices.Contains(granted, string(item)) != item.Allows(permission) { - t.Fatalf("Grants(%q) and Allows disagree for role %q", permission, item) +func TestValidMatchesParse(t *testing.T) { + for _, permission := range Permissions() { + if !permission.Valid() { + t.Errorf("%q reports itself invalid", permission) + } + if parsed, ok := Parse(string(permission)); !ok || parsed != permission { + t.Errorf("Parse(%q) did not round-trip", permission) + } + } + if Permission("nope").Valid() { + t.Error("an unknown permission reports itself valid") + } +} + +func TestPermissionsAreDottedNames(t *testing.T) { + for _, permission := range Permissions() { + name := string(permission) + if len(name) < 3 || strings.HasPrefix(name, ".") || strings.HasSuffix(name, ".") { + t.Fatalf("%q is not a plausible permission name", name) + } + dot := false + for _, character := range name { + switch { + case character == '.': + dot = true + case character >= 'a' && character <= 'z', character >= '0' && character <= '9', character == '-': + default: + t.Fatalf("%q contains %q; permissions use lowercase words", name, character) } } - } -} - -func TestGrantsListsRolesInPrivilegeOrder(t *testing.T) { - if got := Grants(ManageKeys); !slices.Equal(got, []string{"owner", "admin", "developer", "operator"}) { - t.Fatalf("Grants(ManageKeys) = %v", got) - } - if got := Grants(ManageUsers); !slices.Equal(got, []string{"owner", "admin"}) { - t.Fatalf("Grants(ManageUsers) = %v", got) - } - if got := Grants(ReadConsole); !slices.Contains(got, string(Owner)) || slices.Contains(got, string(User)) { - t.Fatalf("Grants(ReadConsole) = %v", got) - } -} - -func TestCatalogCoversEveryRoleWithALabel(t *testing.T) { - entries := Catalog() - if len(entries) != len(catalog) { - t.Fatalf("Catalog() length = %d, want %d", len(entries), len(catalog)) - } - for _, entry := range entries { - if entry.Label == "" { - t.Fatalf("role %q has no label", entry.Value) - } - parsed, ok := Parse(entry.Value) - if !ok { - t.Fatalf("catalog entry %q is not a known role", entry.Value) - } - if len(entry.Permissions) != len(parsed.Permissions()) { - t.Fatalf("role %q catalog permissions drifted from the matrix", entry.Value) + if !dot { + t.Errorf("%q should be a namespaced name", name) } } } -func TestGrantableExcludesOwner(t *testing.T) { - for _, item := range Grantable() { - if item == Owner { - t.Fatal("owner must not be grantable through an invitation") - } +func TestCatalogDescribesEveryPermission(t *testing.T) { + catalog := Catalog() + if len(catalog) != len(Permissions()) { + t.Fatalf("catalog has %d entries, want %d", len(catalog), len(Permissions())) } - if len(GrantableValues()) != len(catalog)-1 { - t.Fatalf("GrantableValues() length = %d", len(GrantableValues())) + seen := make(map[string]bool, len(catalog)) + for _, entry := range catalog { + if entry.Value == "" || entry.Label == "" || entry.Description == "" { + t.Errorf("incomplete catalog entry %+v", entry) + } + if seen[entry.Value] { + t.Errorf("permission %q appears twice in the catalog", entry.Value) + } + seen[entry.Value] = true } } -func TestPermissionsUseDottedNames(t *testing.T) { - for _, permission := range All()[0].Permissions() { - if !strings.Contains(permission, ".") { - t.Fatalf("permission %q should be a dotted capability name", permission) +// TestCatalogFollowsDisplayOrder guards the order the console renders the +// permission checkboxes in: the most sensitive capability first. +func TestCatalogFollowsDisplayOrder(t *testing.T) { + catalog := Catalog() + want := []string{"tenants.manage", "roles.manage", "users.manage"} + for index, value := range want { + if catalog[index].Value != value { + t.Errorf("catalog[%d] = %q, want %q", index, catalog[index].Value, value) } } } + +func TestPermissionsReturnsACopy(t *testing.T) { + first := Permissions() + if len(first) == 0 { + t.Fatal("no permissions") + } + first[0] = "mutated" + if Permissions()[0] == "mutated" { + t.Error("Permissions exposed its backing array") + } +} + +func TestValuesMatchesPermissions(t *testing.T) { + values := Values() + permissions := Permissions() + if len(values) != len(permissions) { + t.Fatalf("Values has %d entries, want %d", len(values), len(permissions)) + } + for index, permission := range permissions { + if values[index] != string(permission) { + t.Errorf("Values()[%d] = %q, want %q", index, values[index], permission) + } + } + if !slices.Contains(values, string(ManageUsers)) { + t.Error("users.manage is missing from Values") + } +} diff --git a/internal/server/clientauth.go b/internal/server/clientauth.go index f7e83cf..1b3ffaf 100644 --- a/internal/server/clientauth.go +++ b/internal/server/clientauth.go @@ -8,7 +8,6 @@ import ( "time" "github.com/danyel/go-loose/internal/key" - "github.com/danyel/go-loose/internal/role" "github.com/danyel/go-loose/internal/store" ) @@ -33,7 +32,7 @@ func (s *Server) clientUser(user store.ClientUser) clientUserResponse { ID: user.ID, Email: user.Email, DisplayName: user.DisplayName, AvatarURL: s.avatarURL(user.AvatarKey), TenantID: user.TenantID, TenantSlug: user.TenantSlug, ApplicationID: user.ApplicationID, Application: user.Application, - Role: user.Role, Permissions: role.Role(user.Role).Permissions(), + Role: user.Role, Permissions: user.Permissions, } } diff --git a/internal/server/origin_test.go b/internal/server/origin_test.go index 25266a5..26f21bd 100644 --- a/internal/server/origin_test.go +++ b/internal/server/origin_test.go @@ -39,3 +39,100 @@ func TestRequireSameOriginRejectsDifferentTenantHost(t *testing.T) { t.Fatalf("status = %d, want %d", response.Code, http.StatusForbidden) } } + +// TestRequireSameOriginSurvivesStaleBaseURL covers the deployment that made +// profile saves fail with "same-origin request required": the container was +// started with a base URL of "http://localhost:" while users browsed to +// https://auth.urpi.be. The configured base URL must not be the only accepted +// origin, or a stale value breaks every state-changing route at once. +func TestRequireSameOriginSurvivesStaleBaseURL(t *testing.T) { + for _, baseURL := range []string{"http://localhost:", "", "http://localhost:8080", "not a url"} { + server := &Server{cfg: config.Config{BaseURL: baseURL}} + handler := server.requireSameOrigin(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + })) + request := httptest.NewRequest(http.MethodPut, "https://auth.urpi.be/api/v1/profile", nil) + request.Header.Set("Origin", "https://auth.urpi.be") + response := httptest.NewRecorder() + + handler.ServeHTTP(response, request) + + if response.Code != http.StatusNoContent { + t.Errorf("base URL %q: status = %d, want %d", baseURL, response.Code, http.StatusNoContent) + } + } +} + +// TestRequireSameOriginAcceptsForwardedHTTPS covers a reverse proxy that +// terminates TLS: the server sees a plain HTTP connection, so the scheme has to +// come from X-Forwarded-Proto rather than from r.TLS. +func TestRequireSameOriginAcceptsForwardedHTTPS(t *testing.T) { + server := &Server{cfg: config.Config{BaseURL: "http://go-loose:8080"}} + handler := server.requireSameOrigin(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + })) + request := httptest.NewRequest(http.MethodPut, "http://auth.urpi.be/api/v1/profile", nil) + request.Host = "auth.urpi.be" + request.Header.Set("Origin", "https://auth.urpi.be") + request.Header.Set("X-Forwarded-Proto", "https") + response := httptest.NewRecorder() + + handler.ServeHTTP(response, request) + + if response.Code != http.StatusNoContent { + t.Fatalf("status = %d, want %d", response.Code, http.StatusNoContent) + } +} + +func TestRequireSameOriginFallsBackToReferer(t *testing.T) { + server := &Server{cfg: config.Config{BaseURL: "https://auth.dev"}} + handler := server.requireSameOrigin(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + })) + request := httptest.NewRequest(http.MethodPost, "https://auth.dev/api/v1/tenants", nil) + request.Header.Set("Referer", "https://auth.dev/console") + response := httptest.NewRecorder() + + handler.ServeHTTP(response, request) + + if response.Code != http.StatusNoContent { + t.Fatalf("status = %d, want %d", response.Code, http.StatusNoContent) + } +} + +func TestRequireSameOriginRejectsMissingAndForeignSources(t *testing.T) { + cases := []struct { + name string + origin string + referer string + }{ + {name: "no origin or referer"}, + {name: "foreign origin", origin: "https://evil.example"}, + {name: "null origin", origin: "null"}, + {name: "same host different scheme", origin: "http://auth.dev"}, + {name: "origin is a prefix of the host", origin: "https://auth.dev.evil.example"}, + {name: "referer from another tenant", referer: "https://ypto.auth.dev/profile"}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + server := &Server{cfg: config.Config{BaseURL: "https://auth.dev"}} + handler := server.requireSameOrigin(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + t.Error("handler reached for a request that was not same-origin") + })) + request := httptest.NewRequest(http.MethodPost, "https://auth.dev/api/v1/tenants", nil) + if testCase.origin != "" { + request.Header.Set("Origin", testCase.origin) + } + if testCase.referer != "" { + request.Header.Set("Referer", testCase.referer) + } + response := httptest.NewRecorder() + + handler.ServeHTTP(response, request) + + if response.Code != http.StatusForbidden { + t.Fatalf("status = %d, want %d", response.Code, http.StatusForbidden) + } + }) + } +} diff --git a/internal/server/profile_test.go b/internal/server/profile_test.go index 9668698..87e2869 100644 --- a/internal/server/profile_test.go +++ b/internal/server/profile_test.go @@ -52,12 +52,17 @@ func TestAvatarURLIsAbsoluteAndOmittedWithoutKey(t *testing.T) { } } +// TestClientUserExposesRoleCapabilitiesAndPicture checks that the identity +// handed to a client application carries the capabilities the store resolved for +// the membership's role rather than anything the server infers. func TestClientUserExposesRoleCapabilitiesAndPicture(t *testing.T) { server := profileTestServer() key := "avatar-key" payload := server.clientUser(store.ClientUser{ ID: "user-1", Email: "operator@example.test", DisplayName: "Operator", - TenantSlug: "nmbs", Application: "guess", Role: string(role.Operator), AvatarKey: &key, + TenantSlug: "nmbs", Application: "guess", Role: "operator", + Permissions: []string{string(role.ManageKeys), string(role.EditProfile)}, + AvatarKey: &key, }) if payload.Role != "operator" { t.Fatalf("role = %q", payload.Role) @@ -65,17 +70,21 @@ func TestClientUserExposesRoleCapabilitiesAndPicture(t *testing.T) { if !slices.Contains(payload.Permissions, string(role.ManageKeys)) { t.Fatalf("permissions = %v, want keys.manage", payload.Permissions) } - if slices.Contains(payload.Permissions, string(role.ManageUsers)) { - t.Fatalf("permissions = %v, must not grant users.manage to an operator", payload.Permissions) + if slices.Contains(payload.Permissions, string(role.ManageTenants)) { + t.Fatalf("permissions = %v, must not grant tenants.manage", payload.Permissions) } if !strings.HasSuffix(payload.AvatarURL, "/api/v1/avatars/avatar-key") { t.Fatalf("avatar url = %q", payload.AvatarURL) } } +// TestClientUserWithoutMembershipIsLeastPrivileged documents the pass-through: a +// user with no membership is reported with the permissions of the built-in User +// role, which the store resolves, so an empty capability set reaches clients +// rather than an error. func TestClientUserWithoutMembershipIsLeastPrivileged(t *testing.T) { server := profileTestServer() - payload := server.clientUser(store.ClientUser{Role: string(role.User)}) + payload := server.clientUser(store.ClientUser{Role: "user", Permissions: []string{string(role.EditProfile)}}) if payload.AvatarURL != "" { t.Fatalf("avatar url = %q, want empty", payload.AvatarURL) } @@ -138,38 +147,6 @@ func TestUpdateProfileRejectsOversizedBody(t *testing.T) { } } -func TestInvitableRoleExcludesOwner(t *testing.T) { - if _, ok := invitableRole(string(role.Owner)); ok { - t.Fatal("an invitation must not be able to grant ownership") - } - for _, item := range role.Grantable() { - if _, ok := invitableRole(string(item)); !ok { - t.Fatalf("invitableRole(%q) = false", item) - } - } - for _, item := range []string{"", "superuser", "Owner", "user'", "owner"} { - if item == string(role.Owner) { - continue - } - if _, ok := invitableRole(item); ok { - t.Fatalf("invitableRole(%q) = true", item) - } - } -} - -func TestAssignableRoleAcceptsEveryKnownRole(t *testing.T) { - for _, item := range role.All() { - if got, ok := assignableRole(string(item)); !ok || got != string(item) { - t.Fatalf("assignableRole(%q) = %q, %v", item, got, ok) - } - } - for _, item := range []string{"", "superuser", "Owner", "admin,owner"} { - if _, ok := assignableRole(item); ok { - t.Fatalf("assignableRole(%q) = true", item) - } - } -} - func TestProfilePageIsEmbedded(t *testing.T) { content, err := web.Files.ReadFile("profile.html") if err != nil { diff --git a/internal/server/roles.go b/internal/server/roles.go new file mode 100644 index 0000000..1bf5cbb --- /dev/null +++ b/internal/server/roles.go @@ -0,0 +1,151 @@ +package server + +import ( + "errors" + "net/http" + "strings" + + "github.com/danyel/go-loose/internal/store" +) + +const ( + maxRoleName = 100 + maxRoleDescription = 500 +) + +// rolesPage serves the role management screen. +func (s *Server) rolesPage(w http.ResponseWriter, _ *http.Request) { + s.serveEmbedded(w, "roles.html") +} + +// listRoles serves the role management screen's data: every built-in role plus +// the tenant's own, each annotated with whether the caller may assign it. +func (s *Server) listRoles(w http.ResponseWriter, r *http.Request) { + tenantID, ok := s.requestTenant(w, r) + if !ok { + return + } + roles, err := s.store.ListRoles(r.Context(), claimsFrom(r).UserID, tenantID) + if err != nil { + s.handleStoreError(w, "list roles", err) + return + } + writeJSON(w, http.StatusOK, map[string]any{"roles": roles}) +} + +// createRole adds a role to the tenant. The store refuses a role that grants +// more than the caller holds, so the endpoint does not have to restate that rule. +func (s *Server) createRole(w http.ResponseWriter, r *http.Request) { + var request struct { + TenantID string `json:"tenant_id"` + Slug string `json:"slug"` + Name string `json:"name"` + Description string `json:"description"` + Permissions []string `json:"permissions"` + } + if !decodeJSON(w, r, &request) { + return + } + slug := strings.TrimSpace(request.Slug) + name := strings.TrimSpace(request.Name) + description := strings.TrimSpace(request.Description) + if !validSlug(slug) { + writeError(w, http.StatusBadRequest, "slug must be lowercase letters, digits, and dashes") + return + } + if name == "" || len(name) > maxRoleName { + writeError(w, http.StatusBadRequest, "name must be between 1 and 100 characters") + return + } + if len(description) > maxRoleDescription { + writeError(w, http.StatusBadRequest, "description must be 500 characters or fewer") + return + } + created, err := s.store.CreateRole( + r.Context(), claimsFrom(r).UserID, request.TenantID, slug, name, description, request.Permissions) + if err != nil { + if errors.Is(err, store.ErrRoleSlugTaken) { + writeError(w, http.StatusConflict, "a role with this slug already exists in this tenant") + return + } + if errors.Is(err, store.ErrUnknownPermission) { + writeError(w, http.StatusBadRequest, err.Error()) + return + } + s.handleStoreError(w, "create role", err) + return + } + writeJSON(w, http.StatusCreated, created) +} + +// updateRole renames a role and replaces the permissions it grants. +func (s *Server) updateRole(w http.ResponseWriter, r *http.Request) { + var request struct { + Name string `json:"name"` + Description string `json:"description"` + Permissions []string `json:"permissions"` + } + if !decodeJSON(w, r, &request) { + return + } + name := strings.TrimSpace(request.Name) + description := strings.TrimSpace(request.Description) + if name == "" || len(name) > maxRoleName { + writeError(w, http.StatusBadRequest, "name must be between 1 and 100 characters") + return + } + if len(description) > maxRoleDescription { + writeError(w, http.StatusBadRequest, "description must be 500 characters or fewer") + return + } + updated, err := s.store.UpdateRole( + r.Context(), claimsFrom(r).UserID, r.PathValue("id"), name, description, request.Permissions) + if err != nil { + if errors.Is(err, store.ErrUnknownPermission) { + writeError(w, http.StatusBadRequest, err.Error()) + return + } + s.handleStoreError(w, "update role", err) + return + } + writeJSON(w, http.StatusOK, updated) +} + +// deleteRole removes one of the tenant's own roles. +func (s *Server) deleteRole(w http.ResponseWriter, r *http.Request) { + if err := s.store.DeleteRole(r.Context(), claimsFrom(r).UserID, r.PathValue("id")); err != nil { + if errors.Is(err, store.ErrRoleInUse) { + writeError(w, http.StatusConflict, "reassign the members of this role before deleting it") + return + } + s.handleStoreError(w, "delete role", err) + return + } + w.WriteHeader(http.StatusNoContent) +} + +// requestTenant reads the tenant a request targets, preferring an explicit +// tenant_id and falling back to the tenant named by the host so that the role +// screen works on a tenant subdomain. +func (s *Server) requestTenant(w http.ResponseWriter, r *http.Request) (string, bool) { + if tenantID := strings.TrimSpace(r.URL.Query().Get("tenant_id")); tenantID != "" { + return tenantID, true + } + slug := s.requestTenantSlug(r.Host) + if slug == "" { + writeError(w, http.StatusBadRequest, "tenant_id is required") + return "", false + } + tenants, err := s.store.ListTenants(r.Context(), claimsFrom(r).UserID) + if err != nil { + s.handleStoreError(w, "resolve tenant", err) + return "", false + } + for _, tenant := range tenants { + if tenant.Slug == slug { + return tenant.ID, true + } + } + writeError(w, http.StatusNotFound, "resource not found or permission denied") + return "", false +} diff --git a/internal/server/roles_test.go b/internal/server/roles_test.go new file mode 100644 index 0000000..fc8eb7d --- /dev/null +++ b/internal/server/roles_test.go @@ -0,0 +1,177 @@ +package server + +import ( + "bytes" + "context" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "path" + "strings" + "testing" + + "github.com/danyel/go-loose/internal/config" + "github.com/danyel/go-loose/internal/session" + "github.com/danyel/go-loose/web" +) + +// withClaims injects a management session the way requireSession would, so a +// handler can be called directly without standing up a session manager. +func withClaims(request *http.Request, claims session.Claims) context.Context { + return context.WithValue(request.Context(), claimsKey, claims) +} + +func TestRolesPageIsEmbedded(t *testing.T) { + content, err := web.Files.ReadFile("roles.html") + if err != nil { + t.Fatalf("roles.html must be embedded: %v", err) + } + for _, marker := range []string{ + "role-list", "role-form", "permission-list", "tenant-context", "/assets/roles.js", + } { + if !bytes.Contains(content, []byte(marker)) { + t.Errorf("roles.html is missing %q", marker) + } + } +} + +// TestRolesScriptIsServed guards the asset allowlist: roles.js is a new file, and +// the handler serves assets by exact name rather than by pattern. +func TestRolesScriptIsServed(t *testing.T) { + server := profileTestServer() + request := httptest.NewRequest(http.MethodGet, "https://auth.example.test/assets/roles.js", nil) + request.SetPathValue("name", "roles.js") + response := httptest.NewRecorder() + + server.asset(response, request) + + if response.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", response.Code, http.StatusOK) + } + if !strings.Contains(response.Body.String(), "/api/v1/roles") { + t.Error("roles.js does not call the role endpoints") + } +} + +func TestAssetAllowlistRejectsUnknownFiles(t *testing.T) { + server := profileTestServer() + for _, name := range []string{"admin.js", "roles.html", "app.js.map", "secret.env"} { + request := httptest.NewRequest(http.MethodGet, "https://auth.example.test/assets/"+name, nil) + request.SetPathValue("name", name) + response := httptest.NewRecorder() + + server.asset(response, request) + + if response.Code != http.StatusNotFound { + t.Errorf("asset(%q) status = %d, want %d", name, response.Code, http.StatusNotFound) + } + } +} + +// TestAssetTraversalCollapsesToTheAllowlistedName documents that a traversal +// attempt cannot escape the allowlist: path.Base reduces it to the bare file +// name, which is then matched against the list like any other request. +func TestAssetTraversalCollapsesToTheAllowlistedName(t *testing.T) { + server := profileTestServer() + for _, name := range []string{"../roles.js", "../../etc/passwd", "..%2froles.js"} { + request := httptest.NewRequest(http.MethodGet, "https://auth.example.test/assets/"+name, nil) + request.SetPathValue("name", name) + response := httptest.NewRecorder() + + server.asset(response, request) + + served := path.Base(name) + if served == "roles.js" { + if response.Code != http.StatusOK { + t.Errorf("asset(%q) status = %d, want the allowlisted file to serve", name, response.Code) + } + continue + } + if response.Code != http.StatusNotFound { + t.Errorf("asset(%q) status = %d, want %d", name, response.Code, http.StatusNotFound) + } + } +} + +func TestCreateRoleRejectsMalformedInputBeforeTouchingTheStore(t *testing.T) { + cases := []struct { + name string + body string + }{ + {"missing slug", `{"name":"Role","permissions":[]}`}, + {"uppercase slug", `{"slug":"Release Manager","name":"Role","permissions":[]}`}, + {"slug starting with a dash", `{"slug":"-role","name":"Role","permissions":[]}`}, + {"empty name", `{"slug":"role","name":" ","permissions":[]}`}, + {"unknown permission", `{"slug":"role","name":"Role","permissions":["superpower"]}`}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + server := profileTestServer() + request := httptest.NewRequest(http.MethodPost, "https://auth.example.test/api/v1/roles", + strings.NewReader(testCase.body)) + request.SetPathValue("id", "") + request = request.WithContext(withClaims(request, session.Claims{UserID: "user-1", Management: true})) + response := httptest.NewRecorder() + + server.createRole(response, request) + + // The store would need a database, so a 400 proves validation ran first. + if response.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want %d (body %s)", response.Code, http.StatusBadRequest, response.Body) + } + }) + } +} + +func TestListRolesRequiresATenant(t *testing.T) { + server := profileTestServer() + request := httptest.NewRequest(http.MethodGet, "https://auth.example.test/api/v1/roles", nil) + request = request.WithContext(withClaims(request, session.Claims{UserID: "user-1", Management: true})) + response := httptest.NewRecorder() + + server.listRoles(response, request) + + if response.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want %d", response.Code, http.StatusBadRequest) + } + if !strings.Contains(response.Body.String(), "tenant_id") { + t.Errorf("body = %s, want it to name the missing parameter", response.Body) + } +} + +// TestRoleRoutesAreRegistered documents the two wrappers the role routes rely +// on. The server under test is not installed, so requireSession redirects every +// session-guarded route to the installer; an unregistered route would answer 404 +// instead, which is what makes this a registration check. +func TestRoleRoutesAreRegistered(t *testing.T) { + // Handler() wraps every route in the request logger, so this server needs a + // logger where the profile tests, which call handlers directly, do not. + server := &Server{ + cfg: config.Config{BaseURL: "https://auth.example.test"}, + logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + } + handler := server.Handler() + cases := []struct { + method, path string + }{ + {http.MethodGet, "/api/v1/roles?tenant_id=t"}, + {http.MethodPost, "/api/v1/roles"}, + {http.MethodPut, "/api/v1/roles/abc"}, + {http.MethodDelete, "/api/v1/roles/abc"}, + {http.MethodGet, "/roles"}, + } + for _, testCase := range cases { + t.Run(testCase.method+" "+testCase.path, func(t *testing.T) { + request := httptest.NewRequest(testCase.method, "https://auth.example.test"+testCase.path, nil) + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + if response.Code != http.StatusSeeOther { + t.Fatalf("status = %d, want %d from requireSession", response.Code, http.StatusSeeOther) + } + if location := response.Header().Get("Location"); location != "/install" { + t.Fatalf("location = %q, want /install", location) + } + }) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 7f9e775..b18be28 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -120,6 +120,7 @@ func (s *Server) Handler() http.Handler { // browser favicon request would redirect to /install and rotate its CSRF cookie. mux.Handle("GET /{$}", s.requireSession(http.HandlerFunc(s.index))) mux.Handle("GET /profile", s.requireSession(http.HandlerFunc(s.profilePage))) + mux.Handle("GET /roles", s.requireSession(http.HandlerFunc(s.rolesPage))) mux.Handle("GET /api/v1/profile", s.requireSession(http.HandlerFunc(s.profile))) mux.Handle("PUT /api/v1/profile", s.requireSession(s.requireSameOrigin(http.HandlerFunc(s.updateProfile)))) mux.Handle("GET /docs", s.requireSession(http.HandlerFunc(s.docs))) @@ -134,6 +135,10 @@ func (s *Server) Handler() http.Handler { mux.Handle("POST /api/v1/users", s.requireSession(s.requireSameOrigin(http.HandlerFunc(s.inviteUser)))) mux.Handle("PUT /api/v1/users/{id}/access", s.requireSession(s.requireSameOrigin(http.HandlerFunc(s.setUserAccess)))) mux.Handle("POST /api/v1/users/{id}/approve", s.requireSession(s.requireSameOrigin(http.HandlerFunc(s.approveUser)))) + mux.Handle("GET /api/v1/roles", s.requireSession(http.HandlerFunc(s.listRoles))) + mux.Handle("POST /api/v1/roles", s.requireSession(s.requireSameOrigin(http.HandlerFunc(s.createRole)))) + mux.Handle("PUT /api/v1/roles/{id}", s.requireSession(s.requireSameOrigin(http.HandlerFunc(s.updateRole)))) + mux.Handle("DELETE /api/v1/roles/{id}", s.requireSession(s.requireSameOrigin(http.HandlerFunc(s.deleteRole)))) return s.recover(s.logRequests(s.securityHeaders(mux))) } @@ -192,7 +197,7 @@ func (s *Server) openapi(w http.ResponseWriter, _ *http.Request) { func (s *Server) asset(w http.ResponseWriter, r *http.Request) { name := path.Base(r.PathValue("name")) - if name != "style.css" && name != "app.js" && name != "profile.js" { + if name != "style.css" && name != "app.js" && name != "profile.js" && name != "roles.js" { http.NotFound(w, r) return } @@ -400,7 +405,7 @@ func (s *Server) dashboard(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, map[string]any{ "user": claims, "profile": s.profileResponse(profile), "tenants": tenants, "applications": applications, "keys": keys, "contracts": contracts, - "users": users, "pending_users": pendingUsers, "roles": role.Catalog(), + "users": users, "pending_users": pendingUsers, "permissions": role.Catalog(), "is_system_administrator": isSystemAdministrator, }) } @@ -623,16 +628,24 @@ func (s *Server) requireSession(next http.Handler) http.Handler { }) } +// requireSameOrigin rejects state-changing requests that did not come from the +// page the session cookie belongs to. +// +// The comparison is made against the origin the request actually arrived on +// rather than against cfg.BaseURL alone. A deployment whose configured base URL +// disagrees with the address users browse to -- a reverse proxy terminating TLS, +// a tenant subdomain, or simply a stale GO_LOOSE_BASE_URL -- would otherwise +// reject every legitimate write. Deriving the scheme from the request keeps the +// check anchored to r.Host, which is the part an attacker cannot influence: the +// browser sets Origin itself and it always names the page's own host. func (s *Server) requireSameOrigin(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { source := r.Header.Get("Origin") if source == "" { source = r.Header.Get("Referer") } - sourceURL, sourceErr := url.Parse(source) - baseURL, baseErr := url.Parse(s.cfg.BaseURL) - if sourceErr != nil || baseErr != nil || sourceURL.Scheme != baseURL.Scheme || - !strings.EqualFold(sourceURL.Host, r.Host) { + sourceURL, err := url.Parse(source) + if err != nil || !s.originAllowed(sourceURL, r) { writeError(w, http.StatusForbidden, "same-origin request required") return } @@ -640,6 +653,56 @@ func (s *Server) requireSameOrigin(next http.Handler) http.Handler { }) } +// originAllowed reports whether source names the same origin as the request. +// The configured base URL is accepted as well so that installations which +// advertise a canonical address keep working when a request reaches the server +// through a different but still legitimate hostname. +func (s *Server) originAllowed(source *url.URL, r *http.Request) bool { + if source.Host == "" { + return false + } + baseScheme, baseHost := baseOrigin(s.cfg.BaseURL) + for _, candidate := range [][2]string{ + {s.requestScheme(r), r.Host}, + {baseScheme, baseHost}, + } { + scheme, host := candidate[0], candidate[1] + if host == "" { + continue + } + if strings.EqualFold(source.Host, host) && strings.EqualFold(source.Scheme, scheme) { + return true + } + } + return false +} + +// requestScheme reports the scheme the client used to reach this server. A +// reverse proxy terminates TLS before the request arrives, so the forwarded +// protocol takes precedence over the local connection state. +func (s *Server) requestScheme(r *http.Request) string { + if forwarded := r.Header.Get("X-Forwarded-Proto"); forwarded != "" { + if scheme, _, found := strings.Cut(forwarded, ","); found { + return strings.TrimSpace(scheme) + } + return strings.TrimSpace(forwarded) + } + if r.TLS != nil { + return "https" + } + return "http" +} + +// baseOrigin splits a configured base URL into its scheme and host, returning +// empty strings when the value is missing or unparseable. +func baseOrigin(baseURL string) (scheme, host string) { + parsed, err := url.Parse(baseURL) + if err != nil { + return "", "" + } + return parsed.Scheme, parsed.Host +} + func (s *Server) securityHeaders(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("X-Content-Type-Options", "nosniff") diff --git a/internal/server/useradmin.go b/internal/server/useradmin.go index c1b0f64..2e6bf5a 100644 --- a/internal/server/useradmin.go +++ b/internal/server/useradmin.go @@ -8,28 +8,8 @@ import ( "github.com/danyel/go-loose/internal/key" "github.com/danyel/go-loose/internal/password" - "github.com/danyel/go-loose/internal/role" ) -// assignableRole validates a role that an administrator may hand to an existing -// member. Any known role qualifies, including owner, so that ownership can move -// between accounts deliberately. -func assignableRole(value string) (string, bool) { - parsed, ok := role.Parse(value) - return string(parsed), ok -} - -// invitableRole validates a role for a new invitation. Ownership is established -// by creating or claiming a tenant, so it is never granted here. -func invitableRole(value string) (string, bool) { - for _, item := range role.Grantable() { - if string(item) == value { - return value, true - } - } - return "", false -} - func (s *Server) configureClient(w http.ResponseWriter, r *http.Request) { var request struct { RedirectURIs []string `json:"redirect_uris"` @@ -79,7 +59,7 @@ func (s *Server) inviteUser(w http.ResponseWriter, r *http.Request) { TenantID string `json:"tenant_id"` Email string `json:"email"` DisplayName string `json:"display_name"` - Role string `json:"role"` + RoleID string `json:"role_id"` Password string `json:"password"` } if !decodeJSON(w, r, &request) { @@ -95,9 +75,8 @@ func (s *Server) inviteUser(w http.ResponseWriter, r *http.Request) { if request.DisplayName == "" { request.DisplayName = request.Email } - invitedRole, ok := invitableRole(request.Role) - if !ok { - writeError(w, http.StatusBadRequest, "role must be one of "+strings.Join(role.GrantableValues(), ", ")) + if strings.TrimSpace(request.RoleID) == "" { + writeError(w, http.StatusBadRequest, "role_id is required") return } passwordHash, err := password.Hash(request.Password) @@ -105,7 +84,7 @@ func (s *Server) inviteUser(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, err.Error()) return } - user, err := s.store.InviteUser(r.Context(), claimsFrom(r).UserID, request.TenantID, request.Email, request.DisplayName, invitedRole, passwordHash) + user, err := s.store.InviteUser(r.Context(), claimsFrom(r).UserID, request.TenantID, request.Email, request.DisplayName, request.RoleID, passwordHash) if err != nil { s.handleStoreError(w, "invite user", err) return @@ -117,18 +96,17 @@ func (s *Server) inviteUser(w http.ResponseWriter, r *http.Request) { func (s *Server) setUserAccess(w http.ResponseWriter, r *http.Request) { var request struct { TenantID string `json:"tenant_id"` - Role string `json:"role"` + RoleID string `json:"role_id"` ApplicationIDs []string `json:"application_ids"` } if !decodeJSON(w, r, &request) { return } - membershipRole, ok := assignableRole(request.Role) - if !ok { - writeError(w, http.StatusBadRequest, "invalid role") + if strings.TrimSpace(request.RoleID) == "" { + writeError(w, http.StatusBadRequest, "role_id is required") return } - if err := s.store.SetUserAccess(r.Context(), claimsFrom(r).UserID, request.TenantID, r.PathValue("id"), membershipRole, request.ApplicationIDs); err != nil { + if err := s.store.SetUserAccess(r.Context(), claimsFrom(r).UserID, request.TenantID, r.PathValue("id"), request.RoleID, request.ApplicationIDs); err != nil { s.handleStoreError(w, "update user access", err) return } @@ -138,20 +116,19 @@ func (s *Server) setUserAccess(w http.ResponseWriter, r *http.Request) { func (s *Server) approveUser(w http.ResponseWriter, r *http.Request) { var request struct { TenantID string `json:"tenant_id"` - Role string `json:"role"` + RoleID string `json:"role_id"` ApplicationIDs []string `json:"application_ids"` } if !decodeJSON(w, r, &request) { return } - membershipRole, ok := assignableRole(request.Role) - if !ok { - writeError(w, http.StatusBadRequest, "invalid role") + if strings.TrimSpace(request.RoleID) == "" { + writeError(w, http.StatusBadRequest, "role_id is required") return } if err := s.store.ApprovePendingUser( r.Context(), claimsFrom(r).UserID, request.TenantID, r.PathValue("id"), - membershipRole, request.ApplicationIDs, + request.RoleID, request.ApplicationIDs, ); err != nil { s.handleStoreError(w, "approve pending user", err) return diff --git a/internal/store/clientauth.go b/internal/store/clientauth.go index eaf513c..60d00ee 100644 --- a/internal/store/clientauth.go +++ b/internal/store/clientauth.go @@ -16,6 +16,7 @@ type ManagedUser struct { Email string `json:"email"` DisplayName string `json:"display_name"` Role string `json:"role"` + RoleID string `json:"role_id"` Permissions []string `json:"permissions"` Status string `json:"status"` ApplicationIDs []string `json:"application_ids"` @@ -46,7 +47,9 @@ type ClientUser struct { // Role is the membership role in the tenant owning the application. A user // without a membership is reported as User so clients always see the least // privileged role. - Role string `json:"role"` + Role string `json:"role"` + // Permissions is read from the role's own rows. A user without a membership + // gets the permissions of the built-in User role. Permissions []string `json:"permissions"` // AvatarKey is the stored locator; the server expands it into AvatarURL. AvatarKey *string `json:"-"` @@ -55,9 +58,13 @@ type ClientUser struct { func (s *Store) ListManagedUsers(ctx context.Context, administratorID string) ([]ManagedUser, error) { rows, err := s.db.QueryContext(ctx, ` - SELECT u.id, m.tenant_id, u.email, u.display_name, m.role, + SELECT u.id, m.tenant_id, u.email, u.display_name, r.slug, m.role_id, CASE WHEN u.subject LIKE 'invited:%' THEN 'invited' ELSE 'active' END, u.avatar_key, + COALESCE(( + SELECT json_agg(rp.permission ORDER BY rp.permission) + FROM role_permissions rp WHERE rp.role_id = m.role_id + ), '[]'::json), COALESCE(( SELECT json_agg(ua.application_id ORDER BY ua.application_id) FROM user_application_access ua @@ -66,12 +73,9 @@ func (s *Store) ListManagedUsers(ctx context.Context, administratorID string) ([ ), '[]'::json) FROM users u JOIN memberships m ON m.user_id = u.id - WHERE EXISTS ( - SELECT 1 FROM memberships own - WHERE own.tenant_id = m.tenant_id AND own.user_id = $1 - AND own.role = ANY($2) - ) - ORDER BY u.email, m.tenant_id`, administratorID, role.Grants(role.ManageUsers)) + JOIN roles r ON r.id = m.role_id + WHERE `+permissionPredicate("m.tenant_id", "$1", "$2")+` + ORDER BY u.email, m.tenant_id`, administratorID, string(role.ManageUsers)) if err != nil { return nil, err } @@ -79,37 +83,42 @@ func (s *Store) ListManagedUsers(ctx context.Context, administratorID string) ([ result := make([]ManagedUser, 0) for rows.Next() { var user ManagedUser - var applicationIDs []byte + var permissions, applicationIDs []byte if err := rows.Scan(&user.ID, &user.TenantID, &user.Email, &user.DisplayName, &user.Role, - &user.Status, &user.AvatarKey, &applicationIDs); err != nil { + &user.RoleID, &user.Status, &user.AvatarKey, &permissions, &applicationIDs); err != nil { + return nil, err + } + if err := json.Unmarshal(permissions, &user.Permissions); err != nil { return nil, err } if err := json.Unmarshal(applicationIDs, &user.ApplicationIDs); err != nil { return nil, err } - user.Permissions = role.Role(user.Role).Permissions() result = append(result, user) } return result, rows.Err() } -func (s *Store) InviteUser(ctx context.Context, administratorID, tenantID, email, displayName, membershipRole, passwordHash string) (ManagedUser, error) { +// InviteUser adds a member to a tenant or, when the email already exists, +// re-invites them. The caller must hold users.manage in that tenant and may only +// hand out a role whose permissions they hold themselves. +func (s *Store) InviteUser(ctx context.Context, administratorID, tenantID, email, displayName, roleID string, passwordHash string) (ManagedUser, error) { tx, err := s.db.BeginTx(ctx, nil) if err != nil { return ManagedUser{}, err } defer tx.Rollback() - var permitted bool - if err := tx.QueryRowContext(ctx, ` - SELECT EXISTS( - SELECT 1 FROM memberships - WHERE tenant_id = $1 AND user_id = $2 AND role = ANY($3) - )`, tenantID, administratorID, role.Grants(role.ManageUsers)).Scan(&permitted); err != nil { + held, err := heldPermissions(ctx, tx, administratorID, tenantID) + if err != nil { return ManagedUser{}, err } - if !permitted { + if !within([]string{string(role.ManageUsers)}, held) { return ManagedUser{}, ErrNotFound } + resolved, err := resolveRoleID(ctx, tx, tenantID, roleID, held) + if err != nil { + return ManagedUser{}, err + } var user ManagedUser err = tx.QueryRowContext(ctx, ` INSERT INTO users(subject, email, display_name, password_hash, password_changed_at) @@ -125,37 +134,65 @@ func (s *Store) InviteUser(ctx context.Context, administratorID, tenantID, email return ManagedUser{}, err } if _, err := tx.ExecContext(ctx, ` - INSERT INTO memberships(tenant_id, user_id, role) VALUES ($1, $2, $3) - ON CONFLICT (tenant_id, user_id) DO UPDATE SET role = EXCLUDED.role`, - tenantID, user.ID, membershipRole); err != nil { + INSERT INTO memberships(tenant_id, user_id, role_id) VALUES ($1, $2, $3) + ON CONFLICT (tenant_id, user_id) DO UPDATE SET role_id = EXCLUDED.role_id`, + tenantID, user.ID, resolved); err != nil { return ManagedUser{}, err } if err := tx.Commit(); err != nil { return ManagedUser{}, err } - user.TenantID, user.Role, user.ApplicationIDs = tenantID, membershipRole, []string{} - user.Permissions = role.Role(membershipRole).Permissions() + granted, err := roleGrants(ctx, s.db, resolved) + if err != nil { + return ManagedUser{}, err + } + user.TenantID, user.RoleID, user.ApplicationIDs = tenantID, resolved, []string{} + if err := s.db.QueryRowContext(ctx, `SELECT slug FROM roles WHERE id = $1`, resolved).Scan(&user.Role); err != nil { + return ManagedUser{}, err + } + user.Permissions = granted return user, nil } -func (s *Store) SetUserAccess(ctx context.Context, administratorID, tenantID, userID, membershipRole string, applicationIDs []string) error { +// SetUserAccess changes a member's role and replaces their application grants. +func (s *Store) SetUserAccess(ctx context.Context, administratorID, tenantID, userID, roleID string, applicationIDs []string) error { tx, err := s.db.BeginTx(ctx, nil) if err != nil { return err } defer tx.Rollback() + held, err := heldPermissions(ctx, tx, administratorID, tenantID) + if err != nil { + return err + } + if !within([]string{string(role.ManageUsers)}, held) { + return ErrNotFound + } + resolved, err := resolveRoleID(ctx, tx, tenantID, roleID, held) + if err != nil { + return err + } + if err := ensureTenantKeepsAnOwner(ctx, tx, tenantID, userID, resolved); err != nil { + return err + } result, err := tx.ExecContext(ctx, ` - UPDATE memberships target SET role = $4 - FROM memberships administrator - WHERE target.tenant_id = $1 AND target.user_id = $2 - AND administrator.tenant_id = target.tenant_id - AND administrator.user_id = $3 AND administrator.role = ANY($5)`, - tenantID, userID, administratorID, membershipRole, role.Grants(role.ManageUsers)) + UPDATE memberships SET role_id = $3 + WHERE tenant_id = $1 AND user_id = $2 AND role_id IS DISTINCT FROM $3`, + tenantID, userID, resolved) if err != nil { return err } if count, _ := result.RowsAffected(); count == 0 { - return ErrNotFound + // Either the membership does not exist or the role is unchanged. + var exists bool + if err := tx.QueryRowContext(ctx, ` + SELECT EXISTS(SELECT 1 FROM memberships WHERE tenant_id = $1 AND user_id = $2)`, + tenantID, userID).Scan(&exists); err != nil { + return err + } + if !exists { + return ErrNotFound + } } if _, err := tx.ExecContext(ctx, ` DELETE FROM user_application_access ua @@ -179,17 +216,49 @@ func (s *Store) SetUserAccess(ctx context.Context, administratorID, tenantID, us return tx.Commit() } +// ErrLastOwner is returned when a change would leave a tenant without an owner. +var ErrLastOwner = errors.New("a tenant must keep at least one owner") + +// ensureTenantKeepsAnOwner blocks demoting or moving away the only membership +// that still holds the owner role. Without this a tenant administrator could +// lock every administrator out of their own tenant with a single edit. +func ensureTenantKeepsAnOwner(ctx context.Context, tx *sql.Tx, tenantID, userID, newRoleID string) error { + var losesOwner bool + if err := tx.QueryRowContext(ctx, ` + SELECT EXISTS( + SELECT 1 FROM memberships m + JOIN roles r ON r.id = m.role_id + WHERE m.tenant_id = $1 AND m.user_id = $2 AND r.slug = 'owner' AND m.role_id <> $3 + )`, tenantID, userID, newRoleID).Scan(&losesOwner); err != nil { + return err + } + if !losesOwner { + return nil + } + var remaining int + if err := tx.QueryRowContext(ctx, ` + SELECT count(*) + FROM memberships m + JOIN roles r ON r.id = m.role_id + WHERE m.tenant_id = $1 AND r.slug = 'owner' AND m.user_id <> $2`, + tenantID, userID).Scan(&remaining); err != nil { + return err + } + if remaining == 0 { + return ErrLastOwner + } + return nil +} + func (s *Store) ConfigureClient(ctx context.Context, administratorID, applicationID string, redirectURIs []string, secretHash []byte) (Application, error) { var app Application var encoded []byte err := s.db.QueryRowContext(ctx, ` UPDATE applications a SET redirect_uris = $4, client_secret_hash = $5, updated_at = now() - FROM memberships m - WHERE a.id = $1 AND m.tenant_id = a.tenant_id AND m.user_id = $2 - AND m.role = ANY($3) + WHERE a.id = $1 AND `+permissionPredicate("a.tenant_id", "$2", "$3")+` RETURNING a.id, a.client_id, to_json(a.redirect_uris), true`, - applicationID, administratorID, role.Grants(role.ManageApplications), redirectURIs, secretHash). + applicationID, administratorID, string(role.ManageApplications), redirectURIs, secretHash). Scan(&app.ID, &app.ClientID, &encoded, &app.ClientReady) if errors.Is(err, sql.ErrNoRows) { return Application{}, ErrNotFound @@ -263,6 +332,7 @@ func (s *Store) ExchangeAuthorizationCode(ctx context.Context, clientID, redirec } defer tx.Rollback() var user ClientUser + var permissions []byte err = tx.QueryRowContext(ctx, ` UPDATE client_authorization_codes c SET used_at = now() FROM applications a @@ -270,21 +340,25 @@ func (s *Store) ExchangeAuthorizationCode(ctx context.Context, clientID, redirec JOIN user_application_access ua ON ua.application_id = a.id JOIN users u ON u.id = ua.user_id LEFT JOIN memberships m ON m.tenant_id = t.id AND m.user_id = u.id + LEFT JOIN roles r ON r.id = m.role_id WHERE c.code_hash = $1 AND c.used_at IS NULL AND c.expires_at > now() AND c.redirect_uri = $2 AND a.id = c.application_id AND ua.user_id = c.user_id AND ua.application_id = a.id AND a.client_id = $3 AND a.client_secret_hash = $4 RETURNING u.id, u.email, u.display_name, t.id, t.slug, a.id, a.slug, - COALESCE(m.role, $5), u.avatar_key`, - codeHash, redirectURI, clientID, secretHash, role.User). + COALESCE(r.slug, 'user'), `+clientPermissions+`, u.avatar_key`, + codeHash, redirectURI, clientID, secretHash). Scan(&user.ID, &user.Email, &user.DisplayName, &user.TenantID, &user.TenantSlug, - &user.ApplicationID, &user.Application, &user.Role, &user.AvatarKey) + &user.ApplicationID, &user.Application, &user.Role, &permissions, &user.AvatarKey) if errors.Is(err, sql.ErrNoRows) { return ClientUser{}, time.Time{}, ErrNotFound } if err != nil { return ClientUser{}, time.Time{}, err } + if err := json.Unmarshal(permissions, &user.Permissions); err != nil { + return ClientUser{}, time.Time{}, err + } expiresAt := time.Now().UTC().Add(12 * time.Hour) if _, err := tx.ExecContext(ctx, ` INSERT INTO client_user_sessions(token_hash, application_id, user_id, expires_at) @@ -296,6 +370,7 @@ func (s *Store) ExchangeAuthorizationCode(ctx context.Context, clientID, redirec func (s *Store) ClientUserByToken(ctx context.Context, tokenHash []byte) (ClientUser, error) { var user ClientUser + var permissions []byte err := s.db.QueryRowContext(ctx, ` UPDATE client_user_sessions s SET last_used_at = now() FROM applications a @@ -303,17 +378,24 @@ func (s *Store) ClientUserByToken(ctx context.Context, tokenHash []byte) (Client JOIN user_application_access ua ON ua.application_id = a.id JOIN users u ON u.id = ua.user_id LEFT JOIN memberships m ON m.tenant_id = t.id AND m.user_id = u.id + LEFT JOIN roles r ON r.id = m.role_id WHERE s.token_hash = $1 AND s.revoked_at IS NULL AND s.expires_at > now() AND a.id = s.application_id AND ua.user_id = s.user_id RETURNING u.id, u.email, u.display_name, t.id, t.slug, a.id, a.slug, - COALESCE(m.role, $2), u.avatar_key`, - tokenHash, role.User). + COALESCE(r.slug, 'user'), `+clientPermissions+`, u.avatar_key`, + tokenHash). Scan(&user.ID, &user.Email, &user.DisplayName, &user.TenantID, &user.TenantSlug, - &user.ApplicationID, &user.Application, &user.Role, &user.AvatarKey) + &user.ApplicationID, &user.Application, &user.Role, &permissions, &user.AvatarKey) if errors.Is(err, sql.ErrNoRows) { return ClientUser{}, ErrNotFound } - return user, err + if err != nil { + return ClientUser{}, err + } + if err := json.Unmarshal(permissions, &user.Permissions); err != nil { + return ClientUser{}, err + } + return user, nil } func (s *Store) RevokeClientSession(ctx context.Context, tokenHash []byte) error { diff --git a/internal/store/installation.go b/internal/store/installation.go index 10b9917..87b693c 100644 --- a/internal/store/installation.go +++ b/internal/store/installation.go @@ -80,9 +80,11 @@ func (s *Store) Install(ctx context.Context, installation Installation, demoUser return err } if _, err := tx.ExecContext(ctx, ` - INSERT INTO memberships(tenant_id, user_id, role) VALUES ($1, $2, $3) - ON CONFLICT (tenant_id, user_id) DO UPDATE SET role = EXCLUDED.role`, - tenantID, userID, role.Admin); err != nil { + INSERT INTO memberships(tenant_id, user_id, role_id) + SELECT $1, $2, r.id FROM roles r + WHERE r.slug = $3 AND r.tenant_id IS NULL + ON CONFLICT (tenant_id, user_id) DO UPDATE SET role_id = EXCLUDED.role_id`, + tenantID, userID, "admin"); err != nil { return err } if _, err := tx.ExecContext(ctx, ` @@ -115,8 +117,10 @@ func (s *Store) ClaimFirstSystemAdministrator(ctx context.Context, userID string return false, err } if _, err := tx.ExecContext(ctx, ` - INSERT INTO memberships(tenant_id, user_id, role) - SELECT id, $1, $2 FROM tenants ON CONFLICT DO NOTHING`, userID, role.Owner); err != nil { + INSERT INTO memberships(tenant_id, user_id, role_id) + SELECT id, $1, r.id FROM tenants, roles r + WHERE r.slug = 'owner' AND r.tenant_id IS NULL + ON CONFLICT DO NOTHING`, userID); err != nil { return false, err } if _, err := tx.ExecContext(ctx, ` @@ -128,14 +132,14 @@ func (s *Store) ClaimFirstSystemAdministrator(ctx context.Context, userID string } // HasConsoleAccess reports whether the user may read the management console. -// System administrators always qualify; otherwise the membership role must +// System administrators always qualify; otherwise one of their memberships must // grant console.read, which excludes self-service User members. func (s *Store) HasConsoleAccess(ctx context.Context, userID string) (bool, error) { var allowed bool err := s.db.QueryRowContext(ctx, ` SELECT EXISTS(SELECT 1 FROM system_administrators WHERE user_id = $1) - OR EXISTS(SELECT 1 FROM memberships WHERE user_id = $1 AND role = ANY($2))`, - userID, role.Grants(role.ReadConsole)).Scan(&allowed) + OR `+permissionPredicate("scoped.tenant_id", "$1", "$2"), + userID, string(role.ReadConsole)).Scan(&allowed) return allowed, err } @@ -183,7 +187,11 @@ func (s *Store) ListPendingUsers(ctx context.Context, administratorID string) ([ return result, rows.Err() } -func (s *Store) ApprovePendingUser(ctx context.Context, administratorID, tenantID, userID, role string, applicationIDs []string) error { +// ApprovePendingUser gives a waiting-room visitor their first membership. +// Only a system administrator may do this, so the escalation guard that +// normally limits role assignment does not apply: the administrator is acting +// across tenants rather than inside one. +func (s *Store) ApprovePendingUser(ctx context.Context, administratorID, tenantID, userID, roleID string, applicationIDs []string) error { isAdministrator, err := s.IsSystemAdministrator(ctx, administratorID) if err != nil { return err @@ -196,11 +204,17 @@ func (s *Store) ApprovePendingUser(ctx context.Context, administratorID, tenantI return err } defer tx.Rollback() + if _, err := resolveRoleID(ctx, tx, tenantID, roleID, role.Values()); err != nil { + return err + } result, err := tx.ExecContext(ctx, ` - INSERT INTO memberships(tenant_id, user_id, role) + INSERT INTO memberships(tenant_id, user_id, role_id) SELECT $1, id, $3 FROM users WHERE id = $2 - ON CONFLICT (tenant_id, user_id) DO UPDATE SET role = EXCLUDED.role`, - tenantID, userID, role) + ON CONFLICT (tenant_id, user_id) DO UPDATE SET role_id = EXCLUDED.role_id`, + tenantID, userID, roleID) + if err != nil { + return err + } if err != nil { return err } diff --git a/internal/store/integration_test.go b/internal/store/integration_test.go index 692ea66..ce80ece 100644 --- a/internal/store/integration_test.go +++ b/internal/store/integration_test.go @@ -107,10 +107,14 @@ func newTenant(t *testing.T, s *store.Store, ctx context.Context, owner, slug st return tenant } -// addMember invites a member into the tenant with the given role. +// addMember invites a member into the tenant with the given built-in role. func addMember(t *testing.T, s *store.Store, ctx context.Context, owner string, tenantID, email, membershipRole string) string { t.Helper() - member, err := s.InviteUser(ctx, owner, tenantID, email, email, membershipRole, "not-a-real-hash") + roleID, err := s.SystemRoleID(ctx, membershipRole) + if err != nil { + t.Fatalf("look up role %q: %v", membershipRole, err) + } + member, err := s.InviteUser(ctx, owner, tenantID, email, email, roleID, "not-a-real-hash") if err != nil { t.Fatalf("invite %s as %s: %v", email, membershipRole, err) } @@ -124,21 +128,21 @@ func TestMigrationsApplyAndRolesAreAccepted(t *testing.T) { s, _, ctx := integrationStore(t) owner := newUser(t, s, ctx, "owner@example.test") tenant := newTenant(t, s, ctx, owner, "nmbs") - for _, item := range []role.Role{role.Developer, role.Operator, role.User} { - addMember(t, s, ctx, owner, tenant.ID, string(item)+"@example.test", string(item)) + for _, item := range []string{"developer", "operator", "user"} { + addMember(t, s, ctx, owner, tenant.ID, item+"@example.test", item) } if _, err := s.ListTenants(ctx, owner); err != nil { t.Fatalf("list tenants: %v", err) } } -func TestMembershipRoleConstraintRejectsUnknownRole(t *testing.T) { +func TestUnknownRoleIDIsRefused(t *testing.T) { s, _, ctx := integrationStore(t) owner := newUser(t, s, ctx, "owner@example.test") tenant := newTenant(t, s, ctx, owner, "nmbs") - member := addMember(t, s, ctx, owner, tenant.ID, "member@example.test", string(role.Viewer)) - if err := s.SetUserAccess(ctx, owner, tenant.ID, member, "superuser", nil); err == nil { - t.Fatal("the database must reject a role outside the permission matrix") + member := addMember(t, s, ctx, owner, tenant.ID, "member@example.test", "viewer") + if err := s.SetUserAccess(ctx, owner, tenant.ID, member, "00000000-0000-0000-0000-000000000000", nil); err == nil { + t.Fatal("assigning a role that does not exist must be refused") } } @@ -153,24 +157,26 @@ func TestPermissionMatrixIsEnforcedInQueries(t *testing.T) { t.Fatalf("owner cannot create an application: %v", err) } tests := []struct { - role role.Role + role string canCreateApp bool canIssueKey bool canSaveContract bool canListManaged bool canConfigureClien bool }{ - {role: role.Owner, canCreateApp: true, canIssueKey: true, canSaveContract: true, canListManaged: true, canConfigureClien: true}, - {role: role.Admin, canCreateApp: true, canIssueKey: true, canSaveContract: true, canListManaged: true, canConfigureClien: true}, - {role: role.Developer, canCreateApp: true, canIssueKey: true, canSaveContract: true, canListManaged: false, canConfigureClien: true}, - {role: role.Operator, canCreateApp: false, canIssueKey: true, canSaveContract: true, canListManaged: false, canConfigureClien: false}, - {role: role.Viewer, canCreateApp: false, canIssueKey: false, canSaveContract: false, canListManaged: false, canConfigureClien: false}, - {role: role.User, canCreateApp: false, canIssueKey: false, canSaveContract: false, canListManaged: false, canConfigureClien: false}, + // owner and admin administer members; developer and operator may also + // maintain their own tenant's members, which is why they can list them. + {role: "owner", canCreateApp: true, canIssueKey: true, canSaveContract: true, canListManaged: true, canConfigureClien: true}, + {role: "admin", canCreateApp: true, canIssueKey: true, canSaveContract: true, canListManaged: true, canConfigureClien: true}, + {role: "developer", canCreateApp: true, canIssueKey: true, canSaveContract: true, canListManaged: true, canConfigureClien: true}, + {role: "operator", canCreateApp: false, canIssueKey: true, canSaveContract: true, canListManaged: true, canConfigureClien: false}, + {role: "viewer", canCreateApp: false, canIssueKey: false, canSaveContract: false, canListManaged: false, canConfigureClien: false}, + {role: "user", canCreateApp: false, canIssueKey: false, canSaveContract: false, canListManaged: false, canConfigureClien: false}, } for _, test := range tests { - t.Run(string(test.role), func(t *testing.T) { - member := addMember(t, s, ctx, owner, tenant.ID, string(test.role)+"@example.test", string(test.role)) - slug := strings.ReplaceAll(string(test.role), "owner", "own") + t.Run(test.role, func(t *testing.T) { + member := addMember(t, s, ctx, owner, tenant.ID, test.role+"@example.test", test.role) + slug := strings.ReplaceAll(test.role, "owner", "own") _, err := s.CreateApplication(ctx, member, store.Application{ TenantID: tenant.ID, Slug: slug, Name: slug, AllowedHosts: []string{}, @@ -201,8 +207,8 @@ func TestConsoleAccessExcludesSelfServiceMembers(t *testing.T) { s, _, ctx := integrationStore(t) owner := newUser(t, s, ctx, "owner@example.test") tenant := newTenant(t, s, ctx, owner, "nmbs") - selfService := addMember(t, s, ctx, owner, tenant.ID, "enduser@example.test", string(role.User)) - operator := addMember(t, s, ctx, owner, tenant.ID, "operator@example.test", string(role.Operator)) + selfService := addMember(t, s, ctx, owner, tenant.ID, "enduser@example.test", "user") + operator := addMember(t, s, ctx, owner, tenant.ID, "operator@example.test", "operator") waiting := newUser(t, s, ctx, "waiting@example.test") for _, test := range []struct { @@ -239,20 +245,25 @@ func TestTenantPermissionsComeFromTheRoleMatrix(t *testing.T) { s, _, ctx := integrationStore(t) owner := newUser(t, s, ctx, "owner@example.test") tenant := newTenant(t, s, ctx, owner, "nmbs") - developer := addMember(t, s, ctx, owner, tenant.ID, "dev@example.test", string(role.Developer)) + developer := addMember(t, s, ctx, owner, tenant.ID, "dev@example.test", "developer") tenants, err := s.ListTenants(ctx, developer) if err != nil { t.Fatalf("list tenants: %v", err) } - if len(tenants) != 1 || tenants[0].Role != string(role.Developer) { + if len(tenants) != 1 || tenants[0].Role != "developer" { t.Fatalf("tenants = %+v", tenants) } if !contains(tenants[0].Permissions, string(role.ManageApplications)) { t.Fatalf("developer permissions = %v", tenants[0].Permissions) } - if contains(tenants[0].Permissions, string(role.ManageUsers)) { - t.Fatalf("developer permissions = %v, must not manage users", tenants[0].Permissions) + // developer administers members but must not mint roles, which is what + // defines privilege in the first place. + if !contains(tenants[0].Permissions, string(role.ManageUsers)) { + t.Fatalf("developer permissions = %v, want users.manage", tenants[0].Permissions) + } + if contains(tenants[0].Permissions, string(role.ManageRoles)) { + t.Fatalf("developer permissions = %v, must not manage roles", tenants[0].Permissions) } } @@ -260,7 +271,7 @@ func TestProfileRoundTrip(t *testing.T) { s, _, ctx := integrationStore(t) owner := newUser(t, s, ctx, "owner@example.test") tenant := newTenant(t, s, ctx, owner, "nmbs") - member := addMember(t, s, ctx, owner, tenant.ID, "member@example.test", string(role.Operator)) + member := addMember(t, s, ctx, owner, tenant.ID, "member@example.test", "operator") before, err := s.Profile(ctx, member) if err != nil { @@ -269,7 +280,7 @@ func TestProfileRoundTrip(t *testing.T) { if before.AvatarKey != nil || before.NameCustomized { t.Fatalf("fresh profile = %+v", before) } - if len(before.Memberships) != 1 || before.Memberships[0].Role != string(role.Operator) { + if len(before.Memberships) != 1 || before.Memberships[0].Role != "operator" { t.Fatalf("memberships = %+v", before.Memberships) } if !contains(before.Memberships[0].Permissions, string(role.ManageKeys)) { @@ -329,7 +340,7 @@ func TestPictureIsStoredAndReadableByKey(t *testing.T) { s, _, ctx := integrationStore(t) owner := newUser(t, s, ctx, "owner@example.test") tenant := newTenant(t, s, ctx, owner, "nmbs") - member := addMember(t, s, ctx, owner, tenant.ID, "member@example.test", string(role.Viewer)) + member := addMember(t, s, ctx, owner, tenant.ID, "member@example.test", "viewer") key := "Zq1Yb3l0Rk9pT2h3ZjQ4eXh3YjFkNGQ3OTFhMmNkYThl" payload := []byte{0x89, 'P', 'N', 'G', 1, 2, 3, 4} @@ -425,7 +436,7 @@ func TestClientSessionReportsMembershipRoleAndPicture(t *testing.T) { if err != nil { t.Fatalf("create application: %v", err) } - member := addMember(t, s, ctx, owner, tenant.ID, "member@example.test", string(role.Operator)) + member := addMember(t, s, ctx, owner, tenant.ID, "member@example.test", "operator") if _, err := s.ConfigureClient(ctx, owner, application.ID, []string{"https://app.example.test/callback"}, []byte("hash")); err != nil { t.Fatalf("configure client: %v", err) } @@ -443,7 +454,7 @@ func TestClientSessionReportsMembershipRoleAndPicture(t *testing.T) { if err != nil { t.Fatalf("exchange authorization code: %v", err) } - if user.Role != string(role.Operator) { + if user.Role != "operator" { t.Fatalf("role = %q, want operator", user.Role) } if user.AvatarKey == nil { @@ -453,7 +464,7 @@ func TestClientSessionReportsMembershipRoleAndPicture(t *testing.T) { if err != nil { t.Fatalf("client user by token: %v", err) } - if byToken.Role != string(role.Operator) || byToken.AvatarKey == nil { + if byToken.Role != "operator" || byToken.AvatarKey == nil { t.Fatalf("client user = %+v", byToken) } } @@ -480,8 +491,8 @@ func TestClientSessionWithoutMembershipFallsBackToUserRole(t *testing.T) { if err != nil { t.Fatalf("exchange authorization code: %v", err) } - if user.Role != string(role.User) { - t.Fatalf("role = %q, want the least privileged fallback %q", user.Role, role.User) + if user.Role != "user" { + t.Fatalf("role = %q, want the least privileged fallback %q", user.Role, "user") } } diff --git a/internal/store/profile.go b/internal/store/profile.go index 4296f4e..7f4e769 100644 --- a/internal/store/profile.go +++ b/internal/store/profile.go @@ -3,10 +3,9 @@ package store import ( "context" "database/sql" + "encoding/json" "errors" "time" - - "github.com/danyel/go-loose/internal/role" ) // Membership is one tenant role held by a user. @@ -15,6 +14,7 @@ type Membership struct { TenantSlug string `json:"tenant_slug"` TenantName string `json:"tenant_name"` Role string `json:"role"` + RoleID string `json:"role_id"` Permissions []string `json:"permissions"` } @@ -77,9 +77,14 @@ func (s *Store) Profile(ctx context.Context, userID string) (Profile, error) { } profile.Memberships = []Membership{} rows, err := s.db.QueryContext(ctx, ` - SELECT m.tenant_id, t.slug, t.name, m.role + SELECT m.tenant_id, t.slug, t.name, r.slug, m.role_id, + COALESCE(( + SELECT json_agg(rp.permission ORDER BY rp.permission) + FROM role_permissions rp WHERE rp.role_id = m.role_id + ), '[]'::json) FROM memberships m JOIN tenants t ON t.id = m.tenant_id + JOIN roles r ON r.id = m.role_id WHERE m.user_id = $1 ORDER BY t.name`, userID) if err != nil { @@ -88,10 +93,14 @@ func (s *Store) Profile(ctx context.Context, userID string) (Profile, error) { defer rows.Close() for rows.Next() { var item Membership - if err := rows.Scan(&item.TenantID, &item.TenantSlug, &item.TenantName, &item.Role); err != nil { + var permissions []byte + if err := rows.Scan(&item.TenantID, &item.TenantSlug, &item.TenantName, &item.Role, + &item.RoleID, &permissions); err != nil { + return Profile{}, err + } + if err := json.Unmarshal(permissions, &item.Permissions); err != nil { return Profile{}, err } - item.Permissions = role.Role(item.Role).Permissions() profile.Memberships = append(profile.Memberships, item) } return profile, rows.Err() diff --git a/internal/store/roles.go b/internal/store/roles.go new file mode 100644 index 0000000..d317f5b --- /dev/null +++ b/internal/store/roles.go @@ -0,0 +1,455 @@ +package store + +import ( + "context" + "database/sql" + "errors" + "fmt" + "strings" + + "github.com/danyel/go-loose/internal/role" +) + +// Role is a named bundle of permissions. A role with no TenantID is built in: +// every tenant sees it and it cannot be edited or deleted. A role with a +// TenantID belongs to that tenant alone. +type Role struct { + ID string `json:"id"` + TenantID *string `json:"tenant_id,omitempty"` + Slug string `json:"slug"` + Name string `json:"name"` + Description string `json:"description"` + System bool `json:"system"` + // Permissions is sorted so that the console can compare two roles without + // worrying about the order they were stored in. + Permissions []string `json:"permissions"` + MemberCount int `json:"member_count"` + // Assignable reports whether the requesting administrator may hand this role + // to somebody. A role is assignable only when every permission it grants is + // one the administrator already holds, so managing members can never be used + // to grant more authority than the administrator has. + Assignable bool `json:"assignable"` +} + +var ( + // ErrRoleInUse is returned when a role that still has members is deleted. + ErrRoleInUse = errors.New("role is still assigned to members") + // ErrRoleSlugTaken is returned when a tenant already uses the requested slug. + ErrRoleSlugTaken = errors.New("role slug is already in use") + // ErrUnknownPermission is returned when a role is defined with a permission + // outside the vocabulary. It is a caller mistake rather than a server fault, + // so callers report it as a bad request. + ErrUnknownPermission = errors.New("unknown permission") +) + +// querier is satisfied by both *sql.DB and *sql.Tx so that the authorization +// helpers can run inside or outside a transaction. +type querier interface { + ExecContext(ctx context.Context, query string, args ...any) (sql.Result, error) + QueryContext(ctx context.Context, query string, args ...any) (*sql.Rows, error) + QueryRowContext(ctx context.Context, query string, args ...any) *sql.Row +} + +// scanner is satisfied by both *sql.Row and *sql.Rows. +type scanner interface { + Scan(dest ...any) error +} + +// permissionPredicate renders the one authorization check used by every +// tenant-scoped statement: it holds when the user holds permission in the +// tenant. The arguments are SQL expressions rather than placeholders because the +// queries that need it have already joined memberships under a name of their +// own. Resolving the grant through role_permissions is what lets a tenant define +// its own roles without any Go code having to know they exist. +func permissionPredicate(tenantExpr, userExpr, permissionExpr string) string { + return fmt.Sprintf(`EXISTS ( + SELECT 1 + FROM memberships scoped + JOIN role_permissions granted ON granted.role_id = scoped.role_id + WHERE scoped.tenant_id = %s AND scoped.user_id = %s + AND granted.permission = %s + )`, tenantExpr, userExpr, permissionExpr) +} + +// SystemRoleID returns the id of a built-in role. Callers that seed memberships +// outside a tenant context -- the installer, the bootstrapper -- need it because +// a membership is a role id rather than a name. +func (s *Store) SystemRoleID(ctx context.Context, slug string) (string, error) { + var id string + err := s.db.QueryRowContext(ctx, `SELECT id FROM roles WHERE slug = $1 AND tenant_id IS NULL`, slug).Scan(&id) + if errors.Is(err, sql.ErrNoRows) { + return "", ErrNotFound + } + return id, err +} + +// clientPermissions is the permission aggregate reported to client applications +// through /connect/userinfo. A user with no membership in the owning tenant is +// described by the built-in User role, which is the least privileged one. +const clientPermissions = `COALESCE( + (SELECT json_agg(rp.permission ORDER BY rp.permission) + FROM role_permissions rp WHERE rp.role_id = m.role_id), + (SELECT json_agg(rp.permission ORDER BY rp.permission) + FROM role_permissions rp + JOIN roles fallback ON fallback.id = rp.role_id + WHERE fallback.slug = 'user' AND fallback.tenant_id IS NULL), + '[]'::json)` + +// heldPermissions returns every permission the actor holds in a tenant, sorted. +// It is the basis for both the per-action checks and the escalation guard: an +// actor may never hand out a capability they do not have themselves. +func heldPermissions(ctx context.Context, q querier, actorID, tenantID string) ([]string, error) { + rows, err := q.QueryContext(ctx, ` + SELECT DISTINCT granted.permission + FROM memberships scoped + JOIN role_permissions granted ON granted.role_id = scoped.role_id + WHERE scoped.tenant_id = $1 AND scoped.user_id = $2 + ORDER BY granted.permission`, tenantID, actorID) + if err != nil { + return nil, err + } + defer rows.Close() + held := make([]string, 0) + for rows.Next() { + var permission string + if err := rows.Scan(&permission); err != nil { + return nil, err + } + held = append(held, permission) + } + return held, rows.Err() +} + +// hasPermission reports whether the actor holds one permission in a tenant. +func hasPermission(ctx context.Context, q querier, actorID, tenantID string, permission role.Permission) (bool, error) { + var permitted bool + err := q.QueryRowContext(ctx, + `SELECT `+permissionPredicate("$2", "$1", "$3"), + actorID, tenantID, string(permission)).Scan(&permitted) + return permitted, err +} + +// within reports whether every permission in granted also appears in held. +func within(granted, held []string) bool { + available := make(map[string]bool, len(held)) + for _, permission := range held { + available[permission] = true + } + for _, permission := range granted { + if !available[permission] { + return false + } + } + return true +} + +// normalizePermissions validates submitted permission names and returns them +// deduplicated in vocabulary order, so a role cannot be stored with an invented +// capability or an unstable order. +func normalizePermissions(permissions []string) ([]string, error) { + requested := make(map[string]bool, len(permissions)) + for _, value := range permissions { + parsed, ok := role.Parse(value) + if !ok { + return nil, fmt.Errorf("%w: %q", ErrUnknownPermission, value) + } + requested[string(parsed)] = true + } + result := make([]string, 0, len(requested)) + for _, permission := range role.Permissions() { + if requested[string(permission)] { + result = append(result, string(permission)) + } + } + return result, nil +} + +// roleGrants reads a role's permissions. +func roleGrants(ctx context.Context, q querier, roleID string) ([]string, error) { + rows, err := q.QueryContext(ctx, ` + SELECT permission FROM role_permissions WHERE role_id = $1 ORDER BY permission`, roleID) + if err != nil { + return nil, err + } + defer rows.Close() + result := make([]string, 0) + for rows.Next() { + var permission string + if err := rows.Scan(&permission); err != nil { + return nil, err + } + result = append(result, permission) + } + return result, rows.Err() +} + +func scanRole(row scanner) (Role, error) { + var item Role + var tenantID sql.NullString + if err := row.Scan(&item.ID, &tenantID, &item.Slug, &item.Name, &item.Description, + &item.System, &item.MemberCount); err != nil { + return Role{}, err + } + if tenantID.Valid { + item.TenantID = &tenantID.String + } + return item, nil +} + +const roleSelect = ` + SELECT r.id, r.tenant_id, r.slug, r.name, r.description, r.is_system, + (SELECT count(*) FROM memberships m WHERE m.role_id = r.id) + FROM roles r ` + +// ListRoles returns the roles available in a tenant: every built-in role plus +// that tenant's own. Reading the catalog needs users.read so the role screen can +// describe a role before anybody is assigned it. +func (s *Store) ListRoles(ctx context.Context, actorID, tenantID string) ([]Role, error) { + permitted, err := hasPermission(ctx, s.db, actorID, tenantID, role.ReadUsers) + if err != nil { + return nil, err + } + if !permitted { + return nil, ErrNotFound + } + roles, err := s.rolesIn(ctx, s.db, actorID, tenantID) + if err != nil { + return nil, err + } + return roles, nil +} + +// AssignableRoles returns only the roles the administrator may hand to a member. +func (s *Store) AssignableRoles(ctx context.Context, actorID, tenantID string) ([]Role, error) { + roles, err := s.ListRoles(ctx, actorID, tenantID) + if err != nil { + return nil, err + } + result := make([]Role, 0, len(roles)) + for _, item := range roles { + if item.Assignable { + result = append(result, item) + } + } + return result, nil +} + +// rolesIn loads the tenant's catalog and marks what the actor may assign. +func (s *Store) rolesIn(ctx context.Context, q querier, actorID, tenantID string) ([]Role, error) { + rows, err := q.QueryContext(ctx, roleSelect+` + WHERE r.tenant_id IS NULL OR r.tenant_id = $1 + ORDER BY r.is_system DESC, + array_position(ARRAY['owner','admin','developer','operator','viewer','user'], r.slug) NULLS LAST, + r.name`, tenantID) + if err != nil { + return nil, err + } + defer rows.Close() + held, err := heldPermissions(ctx, q, actorID, tenantID) + if err != nil { + return nil, err + } + result := make([]Role, 0) + for rows.Next() { + item, err := scanRole(rows) + if err != nil { + return nil, err + } + permissions, err := roleGrants(ctx, q, item.ID) + if err != nil { + return nil, err + } + item.Permissions = permissions + item.Assignable = within(permissions, held) + result = append(result, item) + } + return result, rows.Err() +} + +func roleByID(ctx context.Context, q querier, roleID string) (Role, error) { + item, err := scanRole(q.QueryRowContext(ctx, roleSelect+`WHERE r.id = $1`, roleID)) + if errors.Is(err, sql.ErrNoRows) { + return Role{}, ErrNotFound + } + if err != nil { + return Role{}, err + } + return item, nil +} + +// CreateRole adds a tenant's own role. The caller must hold roles.manage and may +// not mint a role more powerful than one they already hold. +func (s *Store) CreateRole(ctx context.Context, actorID, tenantID, slug, name, description string, permissions []string) (Role, error) { + granted, err := normalizePermissions(permissions) + if err != nil { + return Role{}, err + } + tx, err := s.db.BeginTx(ctx, nil) + if err != nil { + return Role{}, err + } + defer tx.Rollback() + + held, err := heldPermissions(ctx, tx, actorID, tenantID) + if err != nil { + return Role{}, err + } + if !within([]string{string(role.ManageRoles)}, held) || !within(granted, held) { + return Role{}, ErrNotFound + } + created, err := insertRole(ctx, tx, tenantID, slug, name, description) + if err != nil { + return Role{}, err + } + for _, permission := range granted { + if _, err := tx.ExecContext(ctx, ` + INSERT INTO role_permissions(role_id, permission) VALUES ($1, $2)`, + created.ID, permission); err != nil { + return Role{}, err + } + } + created.Permissions = granted + created.Assignable = true + return created, tx.Commit() +} + +// UpdateRole renames a role and replaces its permissions. Built-in roles are +// immutable and a tenant may only edit a role it owns. +func (s *Store) UpdateRole(ctx context.Context, actorID, roleID, name, description string, permissions []string) (Role, error) { + granted, err := normalizePermissions(permissions) + if err != nil { + return Role{}, err + } + tx, err := s.db.BeginTx(ctx, nil) + if err != nil { + return Role{}, err + } + defer tx.Rollback() + + existing, err := lockRole(ctx, tx, roleID) + if err != nil { + return Role{}, err + } + if existing.System || existing.TenantID == nil { + return Role{}, ErrNotFound + } + held, err := heldPermissions(ctx, tx, actorID, *existing.TenantID) + if err != nil { + return Role{}, err + } + if !within([]string{string(role.ManageRoles)}, held) || !within(granted, held) { + return Role{}, ErrNotFound + } + if _, err := tx.ExecContext(ctx, ` + UPDATE roles SET name = $2, description = $3, updated_at = now() WHERE id = $1`, + roleID, name, description); err != nil { + return Role{}, err + } + if _, err := tx.ExecContext(ctx, `DELETE FROM role_permissions WHERE role_id = $1`, roleID); err != nil { + return Role{}, err + } + for _, permission := range granted { + if _, err := tx.ExecContext(ctx, ` + INSERT INTO role_permissions(role_id, permission) VALUES ($1, $2)`, + roleID, permission); err != nil { + return Role{}, err + } + } + existing.Name, existing.Description, existing.Permissions = name, description, granted + return existing, tx.Commit() +} + +// DeleteRole removes a tenant's own role. Built-in roles and roles that still +// have members cannot be deleted, so nobody is left pointing at nothing. +func (s *Store) DeleteRole(ctx context.Context, actorID, roleID string) error { + tx, err := s.db.BeginTx(ctx, nil) + if err != nil { + return err + } + defer tx.Rollback() + + existing, err := lockRole(ctx, tx, roleID) + if err != nil { + return err + } + if existing.System || existing.TenantID == nil { + return ErrNotFound + } + permissions, err := roleGrants(ctx, tx, roleID) + if err != nil { + return err + } + held, err := heldPermissions(ctx, tx, actorID, *existing.TenantID) + if err != nil { + return err + } + if !within([]string{string(role.ManageRoles)}, held) || !within(permissions, held) { + return ErrNotFound + } + if existing.MemberCount > 0 { + return ErrRoleInUse + } + if _, err := tx.ExecContext(ctx, `DELETE FROM roles WHERE id = $1`, roleID); err != nil { + return err + } + return tx.Commit() +} + +// resolveRoleID validates that a role may be assigned to a member of the tenant +// and returns its id. A role belonging to another tenant is reported as not +// found so that role ids cannot be probed across tenants. +func resolveRoleID(ctx context.Context, tx *sql.Tx, tenantID, roleID string, held []string) (string, error) { + item, err := roleByID(ctx, tx, roleID) + if err != nil { + return "", err + } + if item.TenantID != nil && *item.TenantID != tenantID { + return "", ErrNotFound + } + granted, err := roleGrants(ctx, tx, roleID) + if err != nil { + return "", err + } + if !within(granted, held) { + return "", ErrNotFound + } + return roleID, nil +} + +// lockRole loads a role and holds the row for the rest of the transaction. +func lockRole(ctx context.Context, tx *sql.Tx, roleID string) (Role, error) { + item, err := scanRole(tx.QueryRowContext(ctx, roleSelect+`WHERE r.id = $1 FOR UPDATE`, roleID)) + if errors.Is(err, sql.ErrNoRows) { + return Role{}, ErrNotFound + } + if err != nil { + return Role{}, err + } + permissions, err := roleGrants(ctx, tx, roleID) + if err != nil { + return Role{}, err + } + item.Permissions = permissions + return item, nil +} + +// insertRole writes the role row, translating a slug collision into a stable +// error rather than a driver-specific constraint violation. +func insertRole(ctx context.Context, tx *sql.Tx, tenantID, slug, name, description string) (Role, error) { + item, err := scanRole(tx.QueryRowContext(ctx, ` + INSERT INTO roles(tenant_id, slug, name, description) + VALUES ($1, $2, $3, $4) + RETURNING id, tenant_id, slug, name, description, is_system, 0`, + tenantID, slug, name, description)) + if err != nil { + if isUniqueViolation(err) { + return Role{}, ErrRoleSlugTaken + } + return Role{}, err + } + return item, nil +} + +func isUniqueViolation(err error) bool { + return err != nil && strings.Contains(err.Error(), "duplicate key value") +} diff --git a/internal/store/roles_integration_test.go b/internal/store/roles_integration_test.go new file mode 100644 index 0000000..c4fc7ad --- /dev/null +++ b/internal/store/roles_integration_test.go @@ -0,0 +1,242 @@ +package store_test + +import ( + "context" + "errors" + "testing" + + "github.com/danyel/go-loose/internal/role" + "github.com/danyel/go-loose/internal/store" +) + +// rolesFixture creates a tenant owned by owner plus a developer member. The +// developer is the interesting actor: it administers members but does not hold +// roles.manage, so it is the boundary the escalation guards are tested against. +func rolesFixture(t *testing.T) (s *store.Store, ctx context.Context, owner, developer, tenantID string) { + t.Helper() + s, _, ctx = integrationStore(t) + owner = newUser(t, s, ctx, "owner@example.test") + tenantID = newTenant(t, s, ctx, owner, "nmbs").ID + developer = addMember(t, s, ctx, owner, tenantID, "developer@example.test", "developer") + return s, ctx, owner, developer, tenantID +} + +func systemRole(t *testing.T, s *store.Store, ctx context.Context, slug string) string { + t.Helper() + id, err := s.SystemRoleID(ctx, slug) + if err != nil { + t.Fatalf("look up built-in role %q: %v", slug, err) + } + return id +} + +func TestTenantCanDefineItsOwnRoleAndItIsEnforced(t *testing.T) { + s, ctx, owner, _, tenantID := rolesFixture(t) + application, err := s.CreateApplication(ctx, owner, store.Application{ + TenantID: tenantID, Slug: "guess", Name: "Guess", AllowedHosts: []string{}, + }) + if err != nil { + t.Fatalf("create application: %v", err) + } + + created, err := s.CreateRole(ctx, owner, tenantID, "release-manager", + "Release manager", "Prepares releases.", + []string{string(role.ReadConsole), string(role.ManageContracts), string(role.ReadUsers)}) + if err != nil { + t.Fatalf("create role: %v", err) + } + if created.System { + t.Error("a tenant role must not be marked as built in") + } + if created.TenantID == nil || *created.TenantID != tenantID { + t.Errorf("tenant id = %v, want %s", created.TenantID, tenantID) + } + if !contains(created.Permissions, string(role.ManageContracts)) { + t.Errorf("permissions = %v, want contracts.manage", created.Permissions) + } + + // The role grants exactly what it says and nothing more. + member := addMember(t, s, ctx, owner, tenantID, "member@example.test", "viewer") + if err := s.SetUserAccess(ctx, owner, tenantID, member, created.ID, nil); err != nil { + t.Fatalf("assign the custom role: %v", err) + } + _, err = s.CreateAPIKey(ctx, member, application.ID, "key", "gl_prefix", secretHash("custom-denied"), nil) + assertPermission(t, "custom role issuing a key", err, false) + _, err = s.SaveContract(ctx, member, application.ID, contractDocument(), nil) + assertPermission(t, "custom role importing a contract", err, true) +} + +func TestCustomRoleIsScopedToItsOwnTenant(t *testing.T) { + s, ctx, owner, _, tenantID := rolesFixture(t) + other := newUser(t, s, ctx, "other-owner@example.test") + otherTenant := newTenant(t, s, ctx, other, "ypto").ID + + created, err := s.CreateRole(ctx, owner, tenantID, "auditor", "Auditor", "", []string{string(role.ReadConsole)}) + if err != nil { + t.Fatalf("create role: %v", err) + } + + roles, err := s.ListRoles(ctx, other, otherTenant) + if err != nil { + t.Fatalf("list roles: %v", err) + } + for _, item := range roles { + if item.ID == created.ID { + t.Fatal("a role leaked across tenants") + } + } + if err := s.DeleteRole(ctx, other, created.ID); !errors.Is(err, store.ErrNotFound) { + t.Fatalf("cross-tenant delete = %v, want ErrNotFound", err) + } + if _, err := s.UpdateRole(ctx, other, created.ID, "Hijacked", "", nil); !errors.Is(err, store.ErrNotFound) { + t.Fatalf("cross-tenant update = %v, want ErrNotFound", err) + } +} + +func TestBuiltInRolesCannotBeEditedOrDeleted(t *testing.T) { + s, ctx, owner, _, _ := rolesFixture(t) + ownerRole := systemRole(t, s, ctx, "owner") + if _, err := s.UpdateRole(ctx, owner, ownerRole, "Supreme", "", nil); !errors.Is(err, store.ErrNotFound) { + t.Fatalf("editing a built-in role = %v, want ErrNotFound", err) + } + if err := s.DeleteRole(ctx, owner, ownerRole); !errors.Is(err, store.ErrNotFound) { + t.Fatalf("deleting a built-in role = %v, want ErrNotFound", err) + } +} + +func TestRoleInUseCannotBeDeleted(t *testing.T) { + s, ctx, owner, _, tenantID := rolesFixture(t) + created, err := s.CreateRole(ctx, owner, tenantID, "support", "Support", "", []string{string(role.ReadConsole)}) + if err != nil { + t.Fatalf("create role: %v", err) + } + member := addMember(t, s, ctx, owner, tenantID, "member@example.test", "viewer") + if err := s.SetUserAccess(ctx, owner, tenantID, member, created.ID, nil); err != nil { + t.Fatalf("assign role: %v", err) + } + if err := s.DeleteRole(ctx, owner, created.ID); !errors.Is(err, store.ErrRoleInUse) { + t.Fatalf("delete an assigned role = %v, want ErrRoleInUse", err) + } + // Once nobody holds it the delete goes through. + if err := s.SetUserAccess(ctx, owner, tenantID, member, systemRole(t, s, ctx, "viewer"), nil); err != nil { + t.Fatalf("move the member off the role: %v", err) + } + if err := s.DeleteRole(ctx, owner, created.ID); err != nil { + t.Fatalf("delete an unused role: %v", err) + } +} + +// TestRoleAssignmentCannotEscalate is the guard that matters most now that +// developer and operator may manage members: they must not be able to hand +// somebody a role more powerful than their own, which would otherwise be a +// straightforward route to owner. +func TestRoleAssignmentCannotEscalate(t *testing.T) { + s, ctx, owner, developer, tenantID := rolesFixture(t) + victim := addMember(t, s, ctx, owner, tenantID, "victim@example.test", "viewer") + ownerRole := systemRole(t, s, ctx, "owner") + + if err := s.SetUserAccess(ctx, developer, tenantID, victim, ownerRole, nil); !errors.Is(err, store.ErrNotFound) { + t.Fatalf("developer promoting a member to owner = %v, want ErrNotFound", err) + } + if _, err := s.InviteUser(ctx, developer, tenantID, "sneaky@example.test", "Sneaky", ownerRole, "hash"); !errors.Is(err, store.ErrNotFound) { + t.Fatalf("developer inviting as owner = %v, want ErrNotFound", err) + } + if _, err := s.CreateRole(ctx, developer, tenantID, "shadow-owner", "Shadow", "", + []string{string(role.ReadConsole), string(role.ManageTenants)}); !errors.Is(err, store.ErrNotFound) { + t.Fatalf("developer creating an owner-grade role = %v, want ErrNotFound", err) + } + // Nor may they define any role at all: roles.manage is not theirs to hold, + // however weak the role they were trying to mint. + if _, err := s.CreateRole(ctx, developer, tenantID, "teammate", "Teammate", "", + []string{string(role.ReadConsole), string(role.ReadUsers)}); !errors.Is(err, store.ErrNotFound) { + t.Fatalf("developer creating any role = %v, want ErrNotFound", err) + } + // The developer can still maintain members using the roles already offered. + if _, err := s.InviteUser(ctx, developer, tenantID, "teammate@example.test", "Teammate", + systemRole(t, s, ctx, "viewer"), "hash"); err != nil { + t.Fatalf("developer inviting a member: %v", err) + } +} + +func TestRolesManageIsRequiredToDefineRoles(t *testing.T) { + s, ctx, _, developer, tenantID := rolesFixture(t) + if _, err := s.ListRoles(ctx, developer, tenantID); err != nil { + t.Fatalf("a developer may read the role catalog: %v", err) + } + if _, err := s.CreateRole(ctx, developer, tenantID, "nope", "Nope", "", + []string{string(role.ReadConsole)}); !errors.Is(err, store.ErrNotFound) { + t.Fatalf("developer creating a role = %v, want ErrNotFound", err) + } +} + +func TestAssignableRolesExcludeAnythingAboveTheActor(t *testing.T) { + s, ctx, _, developer, tenantID := rolesFixture(t) + assignable, err := s.AssignableRoles(ctx, developer, tenantID) + if err != nil { + t.Fatalf("assignable roles: %v", err) + } + ownerRole := systemRole(t, s, ctx, "owner") + found := 0 + for _, item := range assignable { + if !item.Assignable { + t.Errorf("role %q was returned but is not marked assignable", item.Slug) + } + if contains(item.Permissions, string(role.ManageTenants)) { + t.Errorf("developer may assign %q, which grants tenants.manage", item.Slug) + } + if item.ID == ownerRole { + t.Error("a developer must not be offered the owner role") + } + found++ + } + if found == 0 { + t.Fatal("a developer should still be offered the roles within their authority") + } +} + +func TestUnknownPermissionIsRejectedWhenDefiningARole(t *testing.T) { + s, ctx, owner, _, tenantID := rolesFixture(t) + _, err := s.CreateRole(ctx, owner, tenantID, "sneaky", "Sneaky", "", + []string{string(role.ReadConsole), "superpower"}) + if err == nil { + t.Fatal("a role must not be creatable with an unknown permission") + } +} + +func TestDuplicateRoleSlugIsRejected(t *testing.T) { + s, ctx, owner, _, tenantID := rolesFixture(t) + if _, err := s.CreateRole(ctx, owner, tenantID, "auditor", "Auditor", "", + []string{string(role.ReadConsole)}); err != nil { + t.Fatalf("create role: %v", err) + } + _, err := s.CreateRole(ctx, owner, tenantID, "auditor", "Another", "", + []string{string(role.ReadConsole)}) + if !errors.Is(err, store.ErrRoleSlugTaken) { + t.Fatalf("duplicate slug = %v, want ErrRoleSlugTaken", err) + } +} + +// TestTenantKeepsAnOwner guards the demotion path: the last owner of a tenant +// must not be able to walk away from it and lock every administrator out. +func TestTenantKeepsAnOwner(t *testing.T) { + s, ctx, owner, _, tenantID := rolesFixture(t) + admin := addMember(t, s, ctx, owner, tenantID, "admin@example.test", "admin") + adminRole := systemRole(t, s, ctx, "admin") + ownerRole := systemRole(t, s, ctx, "owner") + + // The owner cannot simply step down while nobody else holds the role. + if err := s.SetUserAccess(ctx, owner, tenantID, owner, adminRole, nil); !errors.Is(err, store.ErrLastOwner) { + t.Fatalf("owner stepping down alone = %v, want ErrLastOwner", err) + } + // Handing the role over first makes the step down legal. + if err := s.SetUserAccess(ctx, owner, tenantID, admin, ownerRole, nil); err != nil { + t.Fatalf("promote the admin to owner: %v", err) + } + if err := s.SetUserAccess(ctx, admin, tenantID, owner, adminRole, nil); err != nil { + t.Fatalf("step down once a second owner exists: %v", err) + } + // The admin is now the only owner and must not be able to walk away from it. + if err := s.SetUserAccess(ctx, admin, tenantID, admin, adminRole, nil); !errors.Is(err, store.ErrLastOwner) { + t.Fatalf("demoting the last owner = %v, want ErrLastOwner", err) + } +} diff --git a/internal/store/store.go b/internal/store/store.go index 24f7f80..dd55116 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -30,8 +30,8 @@ type Tenant struct { Slug string `json:"slug"` Name string `json:"name"` Role string `json:"role"` - // Permissions is derived from Role so that the console and API clients read - // capabilities instead of re-implementing the role matrix. + // Permissions is read from the role's own rows rather than from a Go matrix, + // so a tenant that defines its own roles is described accurately. Permissions []string `json:"permissions"` } @@ -130,11 +130,13 @@ func (s *Store) Bootstrap(ctx context.Context, userID, tenantSlug, appSlug strin return err } if _, err = tx.ExecContext(ctx, ` - INSERT INTO memberships(tenant_id, user_id, role) - SELECT $1, $2, $3 - WHERE NOT EXISTS (SELECT 1 FROM memberships WHERE tenant_id = $1) - OR EXISTS (SELECT 1 FROM memberships WHERE tenant_id = $1 AND user_id = $2) - ON CONFLICT DO NOTHING`, tenantID, userID, role.Owner); err != nil { + INSERT INTO memberships(tenant_id, user_id, role_id) + SELECT $1, $2, r.id + FROM roles r + WHERE r.slug = $3 AND r.tenant_id IS NULL + AND (NOT EXISTS (SELECT 1 FROM memberships WHERE tenant_id = $1) + OR EXISTS (SELECT 1 FROM memberships WHERE tenant_id = $1 AND user_id = $2)) + ON CONFLICT DO NOTHING`, tenantID, userID, "owner"); err != nil { return err } if _, err = tx.ExecContext(ctx, ` @@ -157,9 +159,15 @@ func (s *Store) Bootstrap(ctx context.Context, userID, tenantSlug, appSlug strin func (s *Store) ListTenants(ctx context.Context, userID string) ([]Tenant, error) { rows, err := s.db.QueryContext(ctx, ` - SELECT t.id, t.slug, t.name, m.role + SELECT t.id, t.slug, t.name, r.slug, + COALESCE(( + SELECT json_agg(rp.permission ORDER BY rp.permission) + FROM role_permissions rp + WHERE rp.role_id = m.role_id + ), '[]'::json) FROM tenants t JOIN memberships m ON m.tenant_id = t.id + JOIN roles r ON r.id = m.role_id WHERE m.user_id = $1 ORDER BY t.name`, userID) if err != nil { @@ -169,10 +177,13 @@ func (s *Store) ListTenants(ctx context.Context, userID string) ([]Tenant, error result := make([]Tenant, 0) for rows.Next() { var item Tenant - if err := rows.Scan(&item.ID, &item.Slug, &item.Name, &item.Role); err != nil { + var permissions []byte + if err := rows.Scan(&item.ID, &item.Slug, &item.Name, &item.Role, &permissions); err != nil { + return nil, err + } + if err := json.Unmarshal(permissions, &item.Permissions); err != nil { return nil, err } - item.Permissions = role.Role(item.Role).Permissions() result = append(result, item) } return result, rows.Err() @@ -190,11 +201,26 @@ func (s *Store) CreateTenant(ctx context.Context, userID, slug, name string) (Te if err != nil { return Tenant{}, err } - if _, err := tx.ExecContext(ctx, `INSERT INTO memberships(tenant_id, user_id, role) VALUES ($1, $2, 'owner')`, tenant.ID, userID); err != nil { + // Whoever creates a tenant owns it. + var permissions []byte + err = tx.QueryRowContext(ctx, ` + INSERT INTO memberships(tenant_id, user_id, role_id) + SELECT $1, $2, r.id FROM roles r + WHERE r.slug = 'owner' AND r.tenant_id IS NULL + RETURNING (SELECT slug FROM roles WHERE slug = 'owner' AND tenant_id IS NULL), + COALESCE(( + SELECT json_agg(rp.permission ORDER BY rp.permission) + FROM role_permissions rp + JOIN roles granted ON granted.id = rp.role_id + WHERE granted.slug = 'owner' AND granted.tenant_id IS NULL + ), '[]'::json)`, + tenant.ID, userID).Scan(&tenant.Role, &permissions) + if err != nil { + return Tenant{}, err + } + if err := json.Unmarshal(permissions, &tenant.Permissions); err != nil { return Tenant{}, err } - tenant.Role = string(role.Owner) - tenant.Permissions = role.Owner.Permissions() if err := tx.Commit(); err != nil { return Tenant{}, err } @@ -237,14 +263,11 @@ func (s *Store) CreateApplication(ctx context.Context, userID string, app Applic row := s.db.QueryRowContext(ctx, ` INSERT INTO applications(tenant_id, slug, name, description, allowed_hosts) SELECT $1, $2, $3, $4, $5 - WHERE EXISTS ( - SELECT 1 FROM memberships - WHERE tenant_id = $1 AND user_id = $6 AND role = ANY($7) - ) + WHERE `+permissionPredicate("$1", "$6", "$7")+` RETURNING id, tenant_id, slug, name, description, to_json(allowed_hosts), client_id, to_json(redirect_uris), false`, app.TenantID, app.Slug, app.Name, app.Description, app.AllowedHosts, userID, - role.Grants(role.ManageApplications)) + string(role.ManageApplications)) var allowedHosts, redirectURIs []byte err := row.Scan(&app.ID, &app.TenantID, &app.Slug, &app.Name, &app.Description, &allowedHosts, &app.ClientID, &redirectURIs, &app.ClientReady) if errors.Is(err, sql.ErrNoRows) { @@ -287,10 +310,9 @@ func (s *Store) CreateAPIKey(ctx context.Context, userID, applicationID, name, p INSERT INTO api_keys(tenant_id, application_id, name, prefix, secret_hash, expires_at, created_by) SELECT a.tenant_id, a.id, $2, $3, $4, $5, $6 FROM applications a - JOIN memberships m ON m.tenant_id = a.tenant_id - WHERE a.id = $1 AND m.user_id = $6 AND m.role = ANY($7) + WHERE a.id = $1 AND `+permissionPredicate("a.tenant_id", "$6", "$7")+` RETURNING id, application_id, name, prefix, status, expires_at, last_used_at, created_at`, - applicationID, name, prefix, hash, expiresAt, userID, role.Grants(role.ManageKeys)). + applicationID, name, prefix, hash, expiresAt, userID, string(role.ManageKeys)). Scan(&item.ID, &item.ApplicationID, &item.Name, &item.Prefix, &item.Status, &item.ExpiresAt, &item.LastUsedAt, &item.CreatedAt) if errors.Is(err, sql.ErrNoRows) { return APIKey{}, ErrNotFound @@ -301,9 +323,8 @@ func (s *Store) CreateAPIKey(ctx context.Context, userID, applicationID, name, p func (s *Store) RevokeAPIKey(ctx context.Context, userID, keyID string) error { result, err := s.db.ExecContext(ctx, ` UPDATE api_keys k SET status = 'revoked', revoked_at = now() - FROM memberships m - WHERE k.id = $1 AND m.tenant_id = k.tenant_id AND m.user_id = $2 AND m.role = ANY($3)`, - keyID, userID, role.Grants(role.ManageKeys)) + WHERE k.id = $1 AND `+permissionPredicate("k.tenant_id", "$2", "$3"), + keyID, userID, string(role.ManageKeys)) if err != nil { return err } @@ -363,11 +384,10 @@ func (s *Store) SaveContract(ctx context.Context, userID, applicationID string, INSERT INTO openapi_contracts(application_id, version, source_url, document, created_by) SELECT a.id, $2, $3, $4, $5 FROM applications a - JOIN memberships m ON m.tenant_id = a.tenant_id - WHERE a.id = $1 AND m.user_id = $5 AND m.role = ANY($6) + WHERE a.id = $1 AND `+permissionPredicate("a.tenant_id", "$5", "$6")+` RETURNING id, application_id, version, source_url, created_at`, applicationID, document.Version, sourceURL, document.JSON, userID, - role.Grants(role.ManageContracts)). + string(role.ManageContracts)). Scan(&result.ID, &result.ApplicationID, &result.Version, &result.SourceURL, &result.CreatedAt) if errors.Is(err, sql.ErrNoRows) { return Contract{}, ErrNotFound diff --git a/web/app.js b/web/app.js index 6f5535a..462ba3e 100644 --- a/web/app.js +++ b/web/app.js @@ -1,15 +1,12 @@ const state = { tenants: [], applications: [], keys: [], contracts: [], users: [], pending_users: [], - roles: [], profile: null, selectedTenant: '' + roles: [], permissions: [], profile: null, selectedTenant: '' }; // Capability names mirror internal/role. The server sends the grants for the // selected tenant so the console never re-implements the matrix. const grants = (tenant, permission) => Boolean(tenant?.permissions?.includes(permission)); const $ = selector => document.querySelector(selector); -const roleOptions = roles => roles - .map(item => ``) - .join(''); const $$ = selector => [...document.querySelectorAll(selector)]; const escapeHTML = value => String(value ?? '').replace(/[&<>"']/g, c => ({ '&': '&', @@ -48,11 +45,28 @@ async function load() { const data = await api('/api/v1/dashboard'); Object.assign(state, data); if (!state.tenants.some(tenant => tenant.id === state.selectedTenant)) state.selectedTenant = state.tenants[0]?.id || ''; + await loadRoles(); renderIdentity(); - renderRoleOptions(); render(); } +// loadRoles fetches the role catalog for the selected tenant. Roles are rows now +// rather than a fixed list, so they have to be asked for per tenant, and the +// server marks which ones the signed-in user is allowed to hand out. +async function loadRoles() { + if (!state.selectedTenant) { + state.roles = []; + return + } + try { + state.roles = (await api(`/api/v1/roles?tenant_id=${encodeURIComponent(state.selectedTenant)}`)).roles + } catch (error) { + // A user who cannot read the role catalog still gets the rest of the + // console; the invite and access dialogs simply offer nothing. + state.roles = []; + } +} + function renderIdentity() { const profile = state.profile || {}; $('#identity-name').textContent = profile.display_name || profile.email || ''; @@ -66,11 +80,15 @@ function renderIdentity() { } } -function renderRoleOptions() { - // Inviting an owner is rejected by the server, so leave it out of the list. - const invitable = state.roles.filter(item => item.value !== 'owner'); - $('#invite-role').innerHTML = roleOptions(invitable); - $('#access-role').innerHTML = roleOptions(state.roles); +// assignableRoles is what the invite and access dialogs may offer: the server +// refuses to hand out a role more powerful than the caller's own, so offering +// anything else would only produce an error. +function assignableRoles() { + return state.roles.filter(item => item.assignable) +} + +function roleLabel(roleID) { + return state.roles.find(item => item.id === roleID)?.name || roleID } function render() { @@ -98,7 +116,8 @@ function render() { $('#key-list').innerHTML = tenantKeys.map(item => `${escapeHTML(item.name)}${escapeHTML(apps.get(item.application_id)?.name || '—')}${escapeHTML(item.prefix)}…${escapeHTML(item.status)}${date(item.last_used_at)}${canManageKeys && item.status === 'active' ? `` : ''}`).join('') || rowEmpty(6, 'No keys issued'); $('#contract-list').innerHTML = tenantContracts.map(item => `${escapeHTML(apps.get(item.application_id)?.name || '—')}${escapeHTML(item.version)}${item.endpoint_count}${item.source_url ? `source` : 'Pasted'}${date(item.created_at)}`).join('') || rowEmpty(5, 'No contracts imported'); const pending = state.is_system_administrator ? state.pending_users.map(item => `${escapeHTML(item.display_name)}
${escapeHTML(item.email)}Waiting room—pending0`).join('') : ''; - $('#user-list').innerHTML = tenantUsers.map(item => `${avatarCell(item)}${escapeHTML(item.display_name)}
${escapeHTML(item.email)}${escapeHTML(state.tenants.find(t => t.id === item.tenant_id)?.name || '—')}${escapeHTML(item.role)}${escapeHTML(item.status)}${item.application_ids.length}${canManageUsers ? `` : ''}`).join('') + pending || rowEmpty(6, 'No users'); + $('#user-list').innerHTML = tenantUsers.map(item => `${avatarCell(item)}${escapeHTML(item.display_name)}
${escapeHTML(item.email)}${escapeHTML(state.tenants.find(t => t.id === item.tenant_id)?.name || '—')}${escapeHTML(roleLabel(item.role_id) || item.role)}${escapeHTML(item.status)}${item.application_ids.length}${canManageUsers ? `` : ''}`).join('') + pending || rowEmpty(6, 'No users'); + renderRoleOptions(); $$('[data-tenant-application-action]').forEach(element => element.hidden = !canManageApplications); $$('[data-tenant-key-action]').forEach(element => element.hidden = !canManageKeys); $$('[data-tenant-contract-action]').forEach(element => element.hidden = !canManageContracts); @@ -110,6 +129,20 @@ function render() { $$('.app-select').forEach(select => select.innerHTML = tenantApps.map(item => ``).join('')); } +// renderRoleOptions fills both role pickers. The current value is preserved so +// that re-rendering the table does not silently reset a pending edit. +function renderRoleOptions() { + const options = assignableRoles(); + const html = options.map(item => ``).join(''); + for (const id of ['#invite-role', '#access-role']) { + const select = $(id); + if (!select) continue; + const current = select.value; + select.innerHTML = html; + if (current && options.some(item => item.id === current)) select.value = current + } +} + const empty = text => `

${text}

`; const initials = name => String(name || '?').split(/[\s@._-]+/).filter(Boolean).slice(0, 2).map(part => part[0].toUpperCase()).join('') || '?'; const avatarCell = item => item.avatar_url @@ -230,7 +263,10 @@ document.addEventListener('click', async event => { form.tenant_id.value = user.tenant_id; form.approval_mode.value = 'false'; $('#approval-tenant-label').hidden = true; - form.role.value = user.role; + // A role the caller may not hand out is not in the picker, so fall back + // to the first one they can assign rather than submitting the old value. + const current = assignableRoles().some(item => item.id === user.role_id); + form.role_id.value = current ? user.role_id : assignableRoles()[0]?.id || ''; renderAccessApps(user.tenant_id, user.application_ids); $('#access-dialog').showModal(); return @@ -242,7 +278,7 @@ document.addEventListener('click', async event => { form.approval_mode.value = 'true'; $('#approval-tenant-label').hidden = false; form.approval_tenant_id.value = state.selectedTenant; - form.role.value = 'viewer'; + form.role_id.value = assignableRoles()[0]?.id || ''; renderAccessApps(state.selectedTenant, []); $('#access-dialog').showModal() } @@ -269,8 +305,9 @@ mode.addEventListener('click', () => { document.documentElement.dataset.mode = next; localStorage.setItem('gl-mode', next) }); -$('#tenant-context').addEventListener('change', event => { +$('#tenant-context').addEventListener('change', async event => { state.selectedTenant = event.target.value; + await loadRoles(); render() }); $('#access-form').approval_tenant_id.addEventListener('change', event => renderAccessApps(event.target.value, [])); diff --git a/web/index.html b/web/index.html index 8dc2a9b..6edc2ef 100644 --- a/web/index.html +++ b/web/index.html @@ -13,6 +13,7 @@ GLGo Loose