From b8665972a08415bcdd8bb158d146b5b7cfea1c81 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 21:00:40 +0000 Subject: [PATCH] Reader role migration: give administrators the reader role too Migration 0013 gave the built-in `reader` role only to users holding neither `admin` nor `writer`. An identity provider or upstream that manages an account's admin rights (OIDC `admin_groups`/`admin_claim`, a credential-forwarding `admin_group`) demotes it with `sync_admin_role`, which takes `admin` away and nothing else. An account provisioned on 2.2 holds the realm's `new_account_access` role next to `admin` and stays a reader; an administrator from before the upgrade held `admin` alone and was left Roles only, reading nothing. 0013 now gives `reader` to every user that is not a writer, administrators included. While they are administrators it adds nothing (the access level shown is the highest built-in role). 0013 has not shipped, so it changes in place. The migration test checks that an administrator holds both roles after the upgrade and a reader remains once the identity provider takes `admin` away; 733.upgrade.md says so. Fixes #792 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012e6DJc1H3748D8sheqhKTV --- crates/db/tests/migrations.rs | 15 +++++++++++---- docs/changes/2.2.0/733.upgrade.md | 6 ++++-- migrations/0013_reader_role.sql | 14 ++++++++++---- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/crates/db/tests/migrations.rs b/crates/db/tests/migrations.rs index 6c41369e..862f4481 100644 --- a/crates/db/tests/migrations.rs +++ b/crates/db/tests/migrations.rs @@ -1052,9 +1052,11 @@ async fn versions_unlisted_before_the_upgrade_count_from_their_last_change() { } /// The built-in `reader` role (0013): every user that was a reader (neither -/// an administrator nor a writer) holds it after the upgrade; administrators, -/// writers and service accounts do not, and a role an administrator had named -/// `reader` keeps its grants and members under another name. +/// an administrator nor a writer) holds it after the upgrade, and so does an +/// administrator, who stays a reader when an identity provider later takes +/// `admin` away (#792); writers and service accounts do not, and a role an +/// administrator had named `reader` keeps its grants and members under +/// another name. #[tokio::test] async fn readers_before_the_upgrade_get_the_reader_role() { let (server, _container) = server().await; @@ -1183,7 +1185,12 @@ async fn readers_before_the_upgrade_get_the_reader_role() { }; assert_eq!(roles(reader).await, ["reader"]); assert_eq!(roles(writer).await, ["writer"]); - assert_eq!(roles(admin).await, ["admin"]); + assert_eq!(roles(admin).await, ["admin", "reader"]); + // An identity provider's demotion takes `admin` alone: a reader remains. + assert!(forge_db::authz_dao::sync_admin_role(&pool, admin, false) + .await + .expect("demote the administrator")); + assert_eq!(roles(admin).await, ["reader"]); let renamed = format!("reader-{custom}"); assert_eq!( roles(service).await, diff --git a/docs/changes/2.2.0/733.upgrade.md b/docs/changes/2.2.0/733.upgrade.md index ab79ccd3..1b790e02 100644 --- a/docs/changes/2.2.0/733.upgrade.md +++ b/docs/changes/2.2.0/733.upgrade.md @@ -1,5 +1,7 @@ -The upgrade gives the built-in `reader` role to every user who held neither -`admin` nor `writer`: they read every repository from then on. Service +The upgrade gives the built-in `reader` role to every user who did not hold +`writer`: they read every repository from then on. Administrators get it +too, so one whose identity provider or upstream later takes `admin` away +stays a reader, as an account created after the upgrade does. Service accounts keep only their own roles (Roles only). A role an administrator had named `reader` is renamed `reader-`, its grants and members kept. `POST /admin/users` without a `role` now makes a user a reader and a diff --git a/migrations/0013_reader_role.sql b/migrations/0013_reader_role.sql index e570c9af..ec6e28b2 100644 --- a/migrations/0013_reader_role.sql +++ b/migrations/0013_reader_role.sql @@ -19,9 +19,15 @@ -- upgrade notes ask administrators to check their rules first. -- -- Every user that is neither an administrator nor a writer was a reader and --- gets the role, so they read every repository from now on. Service accounts --- do not: each stands for one pipeline or network, whose roles were chosen --- for it, and they become Roles only. +-- gets the role, so they read every repository from now on. Administrators +-- get it too (#792): an identity provider or upstream that manages their +-- admin rights (OIDC `admin_groups`, a credential-forwarding `admin_group`) +-- takes only `admin` away when it demotes them, and they are then readers, +-- as an account created after the upgrade is, instead of Roles only. While +-- they are administrators it adds nothing. Writers do not need it, since +-- `writer` reads every repository. Service accounts do not get it: each +-- stands for one pipeline or network, whose roles were chosen for it, and +-- they become Roles only. -- --------------------------------------------------------------------------- UPDATE auth_realm realm SET config = jsonb_set( @@ -64,5 +70,5 @@ WHERE r.name = 'reader' SELECT 1 FROM user_role held JOIN role held_role ON held_role.id = held.role_id - WHERE held.user_id = u.id AND held_role.name IN ('admin', 'writer') + WHERE held.user_id = u.id AND held_role.name = 'writer' );