Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 12 additions & 16 deletions docs/features/org-onboarding/plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,35 +132,31 @@ No code change expected. Add a route-level or middleware-level test proving the

## Ticket API-2: Prerequisites for fluent-web #489 (Org Manager self-service on the Users page)

**Implemented on `feat/org-manager-self-service`.** Product decisions (2026-09-16): **D1 = Option (b)** — org-level roles only on the Users page (`Org Manager` to promote, `Org Member` to demote); no org-level PM, so Task 3 was dropped. **D2** — self-change block is the last-OM guard; DELETE also rejects self-removal. **D3** — role display precedence is web-side.

Full task detail, including tests, lives in `fluent-web/docs/features/org-manager-users-page/plan.md` Phase A. Summary:

### Task 1: Org Manager may assign Org Manager

- [ ] `rbac.ts`: add `{ roleName: ROLES.ORG_MANAGER, permissionName: PERMISSIONS.ROLE_ASSIGN_ORG_MANAGER }`.
- [ ] `authorize.test.ts`: org-scoped Org Manager can `canAssignRole(…, ORG_MANAGER, ORG, null)`; still cannot assign `SuperAdmin`.
- [ ] Update the comment in `requireSuperAdmin` (`role-auth.ts:121-123`): the permission is no longer SuperAdmin-exclusive, but the check still holds because it also requires a _global_ grant. Add a test that an Org Manager with the new permission is still rejected by `requireSuperAdmin`.
- [x] `rbac.ts`: added `{ roleName: ROLES.ORG_MANAGER, permissionName: PERMISSIONS.ROLE_ASSIGN_ORG_MANAGER }`.
- [x] `authorize.test.ts`: org-scoped OM can assign OM (not SuperAdmin, not cross-org); project-pinned `role:assign:org_manager` can't satisfy org scope.
- [x] `requireSuperAdmin` comment updated; new `role-auth.test.ts` proves an OM holding the org-scoped permission is still rejected.

### Task 2: Org-level role change

```
PATCH /organizations/{orgId}/users/{userId} body { roleName: 'Org Manager' | 'Project Manager'* }
200 userResponseSchema | 400 role not allowed at org level | 403 self-change or !canAssignRole | 404 not a member
PATCH /organizations/{orgId}/users/{userId} body { roleName: 'Org Manager' | 'Org Member' }
200 userResponseSchema | 400 not a member | 403 self-change or !canAssignRole
```

\* `Project Manager` only if decision D1 in the #489 plan lands on "org-level PM".

- [ ] Service `updateOrgUserRole(callerId, orgId, userId, roleName)` in `src/domains/organizations/users/org-users.service.ts`: forbid `callerId === userId`; in a transaction delete `user_roles` rows `(userId, orgId, projectId IS NULL, role ≠ Org Member)` then insert the new grant; return `usersService.getUserById(userId)`.
- [ ] Route in `org-users.route.ts`, mirroring `PATCH /projects/{projectId}/users/{userId}`.
- [ ] Tests: self-change → 403; non-member → 404; anchor + project-scoped grants preserved; idempotent when unchanged.

### Task 3 (conditional on D1): Org-level Project Manager grant
- [x] `org-users.service.ts` + `repo.updateOrgUserRole`: forbid `callerId === userId`; require org membership (`findUserIdsInOrg`); transaction replaces the non-anchor org-level grant (or deletes it on demote), keeping the `Org Member` anchor and all project-scoped grants; idempotent when unchanged.
- [x] Route in `org-users.route.ts` with `requirePermission(USER_UPDATE, org-scope)` + `canAssignRole` in the handler. DELETE org-user now also blocks `caller.id === userId` (self-removal → 403).

- [ ] `canAssignRole` branch 4: `PROJECT_MANAGER` with `projectId === null` requires `ROLE_ASSIGN_PROJECT` at org scope. Translator/Observer remain project-only.
- [ ] Revisit the TEMP bypass in `projects.route.ts:90-104`.
### Task 3: ~~Org-level Project Manager grant~~ — dropped per D1 (b)

### Task 4: Optional cleanup
### Task 4: Cleanup

- [ ] `PATCH /users/:id` handler (`users.route.ts:465-475`) strips `role` after the zod schema has already dropped it — dead code. Remove or add `role` to the schema deliberately (recommend remove; role changes go through the org/project endpoints).
- [x] Removed the dead `updates.role` strip (and now-unused `authorize` import) from `PATCH /users/:id`.

---

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Org Manager self-service prerequisites for fluent-web #489

> **Status: NOT STARTED** — awaiting go-ahead to implement, and Product decision D1 (see below).
> **Status: IMPLEMENTED (local)** — on `feat/org-manager-self-service`, awaiting review before push.
> GitHub: [fluent-api#337](https://github.com/eten-tech-foundation/fluent-api/issues/337)

**Parent feature:** [`org-onboarding`](../plan.md) — Ticket API-2. Task-level detail with tests also in `fluent-web/docs/features/org-manager-users-page/plan.md` Phase A.
Expand All @@ -16,58 +16,45 @@ fluent-web#489 lets an Org Manager add/edit Org Managers and Project Managers fr
2. `PATCH /users/:id` ignores `role` — not in `updateUserRequestSchema`, and stripped again in `users.repository.update()`. The Edit User dialog "saves" a role that never changes. No org-level role-change endpoint exists; only `PATCH /projects/{projectId}/users/{userId}`.
3. `Project Manager` exists only as a project-pinned grant; `canAssignRole` returns `false` for PM with `projectId === null` (`authorize.ts:82-90`). "Org-scoped Project Manager" as written in #489 is not in the model.

## Product decision D1 (open)
## Product decisions (resolved 2026-09-16)

How should "Project Manager" behave on the org-level Users page?

- **(a) Org-level PM grant** — allow `Project Manager` with `projectId = null`. Matches #489's wording. `grant-utils.isProjectManager()` on the web already treats a null-project manager grant as managing every project in the org. New RBAC concept; touches `canAssignRole` and the TEMP bypass in `projects.route.ts:90-104`.
- **(b) Org Manager only** — the Users page offers only `Org Manager`; PMs stay per-project via Add Project User. Deviates from #489.

Task 3 below exists only under (a).
- **D1 → Option (b):** Project roles stay project-scoped. The Users page manages org-level roles only — `Org Manager` (promote) and `Org Member` (demote, removes the org-level role while keeping the anchor + project grants). There is no org-level Project Manager; Task 3 is dropped.
- **D2 → Self-change block is the guard:** an Org Manager cannot change their own org-level role; they can only be demoted by a different Org Manager, which keeps the org with ≥1 OM. Self-removal via DELETE is blocked for the same reason.
- **D3 → display order** is a web concern (member → org-level role → project role priority PM > Translator > Observer); no API change.
- **Removal notice:** `DELETE /organizations/{orgId}/users/{userId}` already clears chapter assignments + all org grants; the "user has assignments" warning is rendered web-side via `GET /users/{userId}/chapter-assignments`.

## Tasks

### 1. Org Manager may assign the Org Manager role

Files: `src/db/seeds/rbac.ts`, `src/lib/services/permissions/authorize.test.ts`, `src/middlewares/role-auth.ts` (+ test)

- [ ] Add `{ roleName: ROLES.ORG_MANAGER, permissionName: PERMISSIONS.ROLE_ASSIGN_ORG_MANAGER }` to the Org Manager block.
- [ ] Tests: org-scoped Org Manager → `canAssignRole(…, ORG_MANAGER, ORG, null) === true`; same caller cannot assign `SuperAdmin`.
- [ ] `requireSuperAdmin` (`role-auth.ts:121-123`) comments call this permission SuperAdmin-exclusive; it no longer is. The check still holds because it also requires a _global_ grant. Fix the comment and add a test that an Org Manager holding the new permission is still rejected.
- [x] Add `{ roleName: ROLES.ORG_MANAGER, permissionName: PERMISSIONS.ROLE_ASSIGN_ORG_MANAGER }` to the Org Manager block.
- [x] Tests: org-scoped Org Manager → `canAssignRole(…, ORG_MANAGER, ORG, null) === true`; same caller cannot assign `SuperAdmin`; project-pinned `role:assign:org_manager` cannot satisfy org scope.
- [x] `requireSuperAdmin` comment updated (permission no longer exclusive; global-grant requirement keeps the check). New `src/middlewares/role-auth.test.ts` proves an org-scoped OM holding the permission is still rejected.

### 2. Org-level role change endpoint

Create: `src/domains/organizations/users/org-users.service.ts` (+ `.test.ts`), `org-users.types.ts`. Modify: `org-users.route.ts`.
Created: `org-users.service.ts`, `org-users.service.test.ts`, `org-users.types.ts`. Modified: `org-users.route.ts`, `org-users.repository.ts`.

```
PATCH /organizations/{orgId}/users/{userId} body { roleName: 'Org Manager' | 'Project Manager'* }
middleware: authenticateUser, requireUserAccess(USER_ACTIONS.UPDATE, 'userId')
200 userResponseSchema (orgGrants refreshed)
400 roleName not allowed at org level
403 caller === target (self-change blocked, same as project route) OR !canAssignRole(caller, roleName, orgId, null)
404 target is not a member of orgId
PATCH /organizations/{orgId}/users/{userId} body { roleName: 'Org Manager' | 'Org Member' }
middleware: authenticateUser, requirePermission(USER_UPDATE, orgId-from-param)
handler: canAssignRole(caller, roleName, orgId, null) → 403
200 userResponseSchema (grants refreshed)
400 target is not a member of orgId (USER_NOT_IN_ORGANIZATION, per getHttpStatus)
403 caller === target (self-change, D2) OR !canAssignRole
```

\* only under D1 (a).

- [ ] Failing service tests: self-change → `FORBIDDEN`; non-member → `USER_NOT_FOUND`; replaces the existing org-level non-anchor grant, keeps the `Org Member` anchor and all project-scoped grants; idempotent when unchanged.
- [ ] Service `updateOrgUserRole(callerId, orgId, userId, roleName)`: `getRoleId`; in one transaction delete `user_roles` where `(userId, orgId, projectId IS NULL, role ≠ Org Member)`, insert the new grant; return `usersService.getUserById(userId)`.
- [ ] Route mirrors `updateProjectUserRoleRoute` in `project-users.route.ts:185-240`; body schema `z.object({ roleName: z.enum([...allowedOrgRoles]) })`.
- [ ] Unit tests for `canAssignRole` at scope `{ orgId, projectId: null }` for each allowed role.

### 3. Org-level Project Manager grant — only under D1 (a)

Files: `src/lib/services/permissions/authorize.ts` (+ test), `src/domains/projects/projects.route.ts`

- [ ] `canAssignRole` branch 4: `PROJECT_MANAGER && projectId === null` → require `ROLE_ASSIGN_PROJECT` at `{ orgId, projectId: null }`. Translator/Observer remain project-only.
- [ ] Tests: Org Manager can assign org-level PM; project-pinned PM cannot (grant not applicable at org scope); Translator/Observer at org scope still `false`.
- [ ] Revisit the TEMP bypass comment in `projects.route.ts:90-104` — an org-level PM satisfies the normal `authorize()` path. Keep the bypass for legacy project-pinned PMs or remove in a follow-up; document the choice.
- [x] Service tests: self-change → `FORBIDDEN` (repo untouched); non-member → `USER_NOT_IN_ORGANIZATION`; success returns refreshed user; repo failure propagates.
- [x] `repo.updateOrgUserRole` in one transaction: `roleId === orgMemberRoleId` → delete non-anchor org-level rows (demote); otherwise replace the non-anchor org-level row set with the new grant (idempotent when unchanged). Anchor + project-scoped grants always preserved.
- [x] DELETE org-user now also rejects `caller.id === userId` (self-removal) → 403, consistent with D2.

Under D1 (b): skip, and restrict Task 2's enum to `Org Manager`.
### 3. ~~Org-level Project Manager grant~~ — dropped per D1 (b)

### 4. Optional cleanup
### 4. Cleanup

- [ ] `PATCH /users/:id` handler (`users.route.ts:465-475`) deletes `updates.role` after zod already dropped it — dead code. Remove it; role changes go through org/project endpoints.
- [x] Removed dead `updates.role` strip + unused `authorize`/`hasGrantManagement` block from `PATCH /users/:id` (`users.route.ts`). Role changes go through the org/project endpoints only.

## Verification

Expand Down
1 change: 1 addition & 0 deletions src/db/seeds/rbac.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const ROLE_PERMISSION_MAP = [
{ roleName: ROLES.ORG_MANAGER, permissionName: PERMISSIONS.CONTENT_UPDATE },
{ roleName: ROLES.ORG_MANAGER, permissionName: PERMISSIONS.MEMBERSHIP_REVOKE },
{ roleName: ROLES.ORG_MANAGER, permissionName: PERMISSIONS.ROLE_ASSIGN_PROJECT },
{ roleName: ROLES.ORG_MANAGER, permissionName: PERMISSIONS.ROLE_ASSIGN_ORG_MANAGER },
{ roleName: ROLES.ORG_MANAGER, permissionName: PERMISSIONS.USER_VIEW },
{ roleName: ROLES.ORG_MANAGER, permissionName: PERMISSIONS.USER_CREATE },
{ roleName: ROLES.ORG_MANAGER, permissionName: PERMISSIONS.USER_UPDATE },
Expand Down
99 changes: 98 additions & 1 deletion src/domains/organizations/users/org-users.repository.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { and, eq, inArray, sql } from 'drizzle-orm';
import { and, eq, inArray, isNull, sql } from 'drizzle-orm';

import type { Result } from '@/lib/types';

import { db } from '@/db';
import { chapter_assignments, project_units, projects, user_roles } from '@/db/schema';
import { getRoleId } from '@/domains/user-roles/user-roles.service';
import { logger } from '@/lib/logger';
import { ROLES } from '@/lib/roles';
import { err, ErrorCode, ok } from '@/lib/types';

/**
Expand Down Expand Up @@ -82,3 +84,98 @@ export async function removeOrgUser(orgId: number, userId: number): Promise<Resu
return err(ErrorCode.INTERNAL_ERROR);
}
}

/**
* Updates a user's org-level role within an org in a single transaction.
*
* Org-level roles live on grant rows with projectId IS NULL. The Org Member
* row is the membership anchor and is never touched here; every other
* org-level row is the functional org role (Org Manager today).
*
* - roleId === orgMemberRoleId → demotion: delete all non-anchor org-level rows.
* - otherwise → replace the non-anchor org-level row set with a single row for
* roleId (idempotent when the role is unchanged).
*
* Project-scoped grants (projectId IS NOT NULL) are never affected — a PM who
* is demoted from Org Manager keeps their project role, and vice versa.
*/
export async function updateOrgUserRole(
orgId: number,
userId: number,
roleId: number,
createdBy: number | null
): Promise<Result<void>> {
try {
const orgMemberRoleId = await getRoleId(ROLES.ORG_MEMBER);

return await db.transaction(async (tx) => {
// Lock the Org Member anchor first — it is the membership record, and
// the row lock serializes this update against a concurrent
// removeOrgUser: either we hold the anchor before removal deletes it,
// or removal already committed and we bail out as not-a-member.
const anchor = await tx
.select({ id: user_roles.id })
.from(user_roles)
.where(
and(
eq(user_roles.userId, userId),
eq(user_roles.orgId, orgId),
isNull(user_roles.projectId),
eq(user_roles.roleId, orgMemberRoleId)
)
)
.for('update');

if (anchor.length === 0) {
throw new UserNotInOrgException('User not in organization');
}

const nonAnchorOrgScope = and(
eq(user_roles.userId, userId),
eq(user_roles.orgId, orgId),
isNull(user_roles.projectId),
sql`${user_roles.roleId} != ${orgMemberRoleId}`
);

if (roleId === orgMemberRoleId) {
await tx.delete(user_roles).where(nonAnchorOrgScope);
return ok(undefined);
}

const existing = await tx
.select({ id: user_roles.id, roleId: user_roles.roleId })
.from(user_roles)
.where(nonAnchorOrgScope);

if (existing.length === 1 && existing[0].roleId === roleId) {
return ok(undefined);
}

if (existing.length > 0) {
await tx.delete(user_roles).where(
inArray(
user_roles.id,
existing.map((r) => r.id)
)
);
}

await tx
.insert(user_roles)
.values({ userId, orgId, projectId: null, roleId, createdBy })
.onConflictDoNothing();
Comment thread
coderabbitai[bot] marked this conversation as resolved.

return ok(undefined);
});
} catch (error) {
if (error instanceof UserNotInOrgException) {
return err(ErrorCode.USER_NOT_IN_ORGANIZATION);
}
logger.error({
cause: error,
message: 'Failed to update org-level role for user',
context: { orgId, userId, roleId },
});
return err(ErrorCode.INTERNAL_ERROR);
}
}
Loading