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' );