1063 967 oauth2 improvements (#1102)

kanidm_book/src/integrations/oauth2.md

@@ -83,19 +83,29 @@ a user wants to login, it may only request "access" as a scope from Kanidm.
 As each resource server may have its own scopes and understanding of these, Kanidm isolates
 scopes to each resource server connected to Kanidm. Kanidm has two methods of granting scopes to accounts (users).
 
-The first are implicit scopes. These are scopes granted to all accounts that Kanidm holds.
+The first is scope mappings. These provide a set of scopes if a user is a member of a specific
-
-The second is scope mappings. These provide a set of scopes if a user is a member of a specific
 group within Kanidm. This allows you to create a relationship between the scopes of a resource
 server, and the groups/roles in Kanidm which can be specific to that resource server.
 
 For an authorisation to proceed, all scopes requested must be available in the final scope set
-that is granted to the account. This final scope set can be built from implicit and mapped
+that is granted to the account.
-scopes.
 
-This use of scopes is the primary means to control who can access what resources. For example, if
+The second is supplemental scope mappings. These function the same as scope maps where membership
-you have a resource server that will always request a scope of "read", then you can limit the
+of a group provides a set of scopes to the account, however these scopes are NOT consulted during
-"read" scope to a single group of users by a scope map so that only they may access that resource.
+authorisation decisions made by Kanidm. These scopes exists to allow optional properties to be
+provided (such as personal information about a subset of accounts to be revealed) or so that the resource server
+may make it's own authorisation decisions based on the provided scopes.
+
+This use of scopes is the primary means to control who can access what resources. These access decisions
+can take place either on Kanidm or the resource server.
+
+For example, if you have a resource server that always requests a scope of "read", then users
+with scope maps that supply the read scope will be allowed by Kanidm to proceed to the resource server.
+Kanidm can then provide the supplementary scopes into provided tokens, so that the resource server
+can use these to choose if it wishes to display UI elements. If a user has a supplemental "admin"
+scope, then that user may be able to access an administration panel of the resource server. In this
+way Kanidm is still providing the authorisation information, but the control is then exercised by
+the resource server.
 
 ## Configuration
 
@@ -110,19 +120,14 @@ You can create a new resource server with:
     kanidm system oauth2 create <name> <displayname> <origin>
     kanidm system oauth2 create nextcloud "Nextcloud Production" https://nextcloud.example.com
 
-If you wish to create implicit scopes you can set these with:
-
-    kanidm system oauth2 set_implicit_scopes <name> [scopes]...
-    kanidm system oauth2 set_implicit_scopes nextcloud login read_user
-
 You can create a scope map with:
 
-    kanidm system oauth2 create_scope_map <name> <kanidm_group_name> [scopes]...
+    kanidm system oauth2 update_scope_map <name> <kanidm_group_name> [scopes]...
-    kanidm system oauth2 create_scope_map nextcloud nextcloud_admins admin
+    kanidm system oauth2 update_scope_map nextcloud nextcloud_admins admin
 
 > **WARNING**
 > If you are creating an OpenID Connect (OIDC) resource server you *MUST* provide a
-> scope map OR implicit scope named 'openid'. Without this, OpenID clients *WILL NOT WORK*
+> scope map named 'openid'. Without this, OpenID clients *WILL NOT WORK*
 
 > **HINT**
 > OpenID connect allows a number of scopes that affect the content of the resulting
@@ -136,6 +141,11 @@ You can create a scope map with:
 > * phone - (phone\_number, phone\_number\_verified)
 >
 
+You can create a supplemental scope map with:
+
+    kanidm system oauth2 update_sup_scope_map <name> <kanidm_group_name> [scopes]...
+    kanidm system oauth2 update_sup_scope_map nextcloud nextcloud_admins admin
+
 Once created you can view the details of the resource server.
 
     kanidm system oauth2 get nextcloud
@@ -274,6 +284,13 @@ these to a group with a scope map due to Velociraptors high impact.
 
 ### Vouch Proxy
 
+> **WARNING**
+> Vouch proxy requires a unique identifier but does not use the proper scope, "sub". It uses the fields
+> "username" or "email" as primary identifiers instead. As a result, this can cause user or deployment issues, at
+> worst security bypasses. You should avoid Vouch Proxy if possible due to these issues.
+> * https://github.com/vouch/vouch-proxy/issues/309
+> * https://github.com/vouch/vouch-proxy/issues/310
+
 _You need to run at least the version 0.37.0_.
 
 Vouch Proxy supports multiple OAuth and OIDC login providers.
@@ -288,7 +305,7 @@ oauth:
   code_challenge_method: S256
   provider: oidc
   scopes:
-    - email # Important, vouch proxy requiers a username (but does not use the proper scope, sub) or an email see https://github.com/vouch/vouch-proxy/issues/309, 310
+    - email # Required due to vouch proxy reliance on mail as a primary identifier
   token_url: https://idm.wherekanidmruns.com/oauth2/token
   user_info_url: https://idm.wherekanidmruns.com/oauth2/openid/<oauth2_rs_name>/userinfo
 ```

kanidm_client/src/lib.rs

@@ -1616,6 +1616,14 @@ impl KanidmClient {
             .await
     }
 
+    pub async fn idm_oauth2_rs_get_basic_secret(
+        &self,
+        id: &str,
+    ) -> Result<Option<String>, ClientError> {
+        self.perform_get_request(format!("/v1/oauth2/{}/_basic_secret", id).as_str())
+            .await
+    }
+
     #[allow(clippy::too_many_arguments)]
     pub async fn idm_oauth2_rs_update(
         &self,
@@ -1623,7 +1631,6 @@ impl KanidmClient {
         name: Option<&str>,
         displayname: Option<&str>,
         origin: Option<&str>,
-        scopes: Option<Vec<&str>>,
         reset_secret: bool,
         reset_token_key: bool,
         reset_sign_key: bool,
@@ -1647,12 +1654,6 @@ impl KanidmClient {
                 .attrs
                 .insert("oauth2_rs_origin".to_string(), vec![neworigin.to_string()]);
         }
-        if let Some(newscopes) = scopes {
-            update_oauth2_rs.attrs.insert(
-                "oauth2_rs_implicit_scopes".to_string(),
-                newscopes.into_iter().map(str::to_string).collect(),
-            );
-        }
         if reset_secret {
             update_oauth2_rs
                 .attrs
@@ -1675,7 +1676,7 @@ impl KanidmClient {
             .await
     }
 
-    pub async fn idm_oauth2_rs_create_scope_map(
+    pub async fn idm_oauth2_rs_update_scope_map(
         &self,
         id: &str,
         group: &str,
@@ -1698,6 +1699,29 @@ impl KanidmClient {
             .await
     }
 
+    pub async fn idm_oauth2_rs_update_sup_scope_map(
+        &self,
+        id: &str,
+        group: &str,
+        scopes: Vec<&str>,
+    ) -> Result<(), ClientError> {
+        let scopes: Vec<String> = scopes.into_iter().map(str::to_string).collect();
+        self.perform_post_request(
+            format!("/v1/oauth2/{}/_sup_scopemap/{}", id, group).as_str(),
+            scopes,
+        )
+        .await
+    }
+
+    pub async fn idm_oauth2_rs_delete_sup_scope_map(
+        &self,
+        id: &str,
+        group: &str,
+    ) -> Result<(), ClientError> {
+        self.perform_delete_request(format!("/v1/oauth2/{}/_sup_scopemap/{}", id, group).as_str())
+            .await
+    }
+
     pub async fn idm_oauth2_rs_delete(&self, id: &str) -> Result<(), ClientError> {
         self.perform_delete_request(["/v1/oauth2/", id].concat().as_str())
             .await

kanidm_tools/src/cli/oauth2.rs

@@ -6,10 +6,12 @@ impl Oauth2Opt {
             Oauth2Opt::List(copt) => copt.debug,
             Oauth2Opt::Get(nopt) => nopt.copt.debug,
             Oauth2Opt::CreateBasic(cbopt) => cbopt.nopt.copt.debug,
-            Oauth2Opt::SetImplictScopes(cbopt) => cbopt.nopt.copt.debug,
+            Oauth2Opt::UpdateScopeMap(cbopt) => cbopt.nopt.copt.debug,
-            Oauth2Opt::CreateScopeMap(cbopt) => cbopt.nopt.copt.debug,
             Oauth2Opt::DeleteScopeMap(cbopt) => cbopt.nopt.copt.debug,
+            Oauth2Opt::UpdateSupScopeMap(cbopt) => cbopt.nopt.copt.debug,
+            Oauth2Opt::DeleteSupScopeMap(cbopt) => cbopt.nopt.copt.debug,
             Oauth2Opt::ResetSecrets(cbopt) => cbopt.copt.debug,
+            Oauth2Opt::ShowBasicSecret(nopt) => nopt.copt.debug,
             Oauth2Opt::Delete(nopt) => nopt.copt.debug,
             Oauth2Opt::SetDisplayname(cbopt) => cbopt.nopt.copt.debug,
             Oauth2Opt::EnablePkce(nopt) => nopt.copt.debug,
@@ -52,29 +54,10 @@ impl Oauth2Opt {
                     Err(e) => error!("Error -> {:?}", e),
                 }
             }
-            Oauth2Opt::SetImplictScopes(cbopt) => {
+            Oauth2Opt::UpdateScopeMap(cbopt) => {
                 let client = cbopt.nopt.copt.to_client().await;
                 match client
-                    .idm_oauth2_rs_update(
+                    .idm_oauth2_rs_update_scope_map(
-                        cbopt.nopt.name.as_str(),
-                        None,
-                        None,
-                        None,
-                        Some(cbopt.scopes.iter().map(|s| s.as_str()).collect()),
-                        false,
-                        false,
-                        false,
-                    )
-                    .await
-                {
-                    Ok(_) => println!("Success"),
-                    Err(e) => error!("Error -> {:?}", e),
-                }
-            }
-            Oauth2Opt::CreateScopeMap(cbopt) => {
-                let client = cbopt.nopt.copt.to_client().await;
-                match client
-                    .idm_oauth2_rs_create_scope_map(
                         cbopt.nopt.name.as_str(),
                         cbopt.group.as_str(),
                         cbopt.scopes.iter().map(|s| s.as_str()).collect(),
@@ -95,18 +78,13 @@ impl Oauth2Opt {
                     Err(e) => error!("Error -> {:?}", e),
                 }
             }
-            Oauth2Opt::ResetSecrets(cbopt) => {
+            Oauth2Opt::UpdateSupScopeMap(cbopt) => {
-                let client = cbopt.copt.to_client().await;
+                let client = cbopt.nopt.copt.to_client().await;
                 match client
-                    .idm_oauth2_rs_update(
+                    .idm_oauth2_rs_update_sup_scope_map(
-                        cbopt.name.as_str(),
+                        cbopt.nopt.name.as_str(),
-                        None,
+                        cbopt.group.as_str(),
-                        None,
+                        cbopt.scopes.iter().map(|s| s.as_str()).collect(),
-                        None,
-                        None,
-                        true,
-                        true,
-                        true,
                     )
                     .await
                 {
@@ -114,6 +92,45 @@ impl Oauth2Opt {
                     Err(e) => error!("Error -> {:?}", e),
                 }
             }
+            Oauth2Opt::DeleteSupScopeMap(cbopt) => {
+                let client = cbopt.nopt.copt.to_client().await;
+                match client
+                    .idm_oauth2_rs_delete_sup_scope_map(
+                        cbopt.nopt.name.as_str(),
+                        cbopt.group.as_str(),
+                    )
+                    .await
+                {
+                    Ok(_) => println!("Success"),
+                    Err(e) => error!("Error -> {:?}", e),
+                }
+            }
+            Oauth2Opt::ResetSecrets(cbopt) => {
+                let client = cbopt.copt.to_client().await;
+                match client
+                    .idm_oauth2_rs_update(cbopt.name.as_str(), None, None, None, true, true, true)
+                    .await
+                {
+                    Ok(_) => println!("Success"),
+                    Err(e) => error!("Error -> {:?}", e),
+                }
+            }
+            Oauth2Opt::ShowBasicSecret(nopt) => {
+                let client = nopt.copt.to_client().await;
+                match client
+                    .idm_oauth2_rs_get_basic_secret(nopt.name.as_str())
+                    .await
+                {
+                    Ok(Some(secret)) => {
+                        println!("{secret}");
+                        eprintln!("Success");
+                    }
+                    Ok(None) => {
+                        eprintln!("No secret configured");
+                    }
+                    Err(e) => error!("Error -> {:?}", e),
+                }
+            }
             Oauth2Opt::Delete(nopt) => {
                 let client = nopt.copt.to_client().await;
                 match client.idm_oauth2_rs_delete(nopt.name.as_str()).await {
@@ -129,7 +146,6 @@ impl Oauth2Opt {
                         None,
                         Some(cbopt.displayname.as_str()),
                         None,
-                        None,
                         false,
                         false,
                         false,

kanidm_tools/src/opt/kanidm.rs

@@ -594,18 +594,26 @@ pub enum Oauth2Opt {
     #[clap(name = "create")]
     /// Create a new oauth2 resource server
     CreateBasic(Oauth2BasicCreateOpt),
-    #[clap(name = "set_implicit_scopes")]
+    #[clap(name = "update_scope_map", visible_aliases=&["create_scope_map"])]
-    /// Set the list of scopes that are granted to all valid accounts.
+    /// Update or add a new mapping from a group to scopes that it provides to members
-    SetImplictScopes(Oauth2SetImplicitScopes),
+    UpdateScopeMap(Oauth2CreateScopeMapOpt),
-    #[clap(name = "create_scope_map")]
-    /// Add a new mapping from a group to what scopes it provides
-    CreateScopeMap(Oauth2CreateScopeMapOpt),
     #[clap(name = "delete_scope_map")]
     /// Remove a mapping from groups to scopes
     DeleteScopeMap(Oauth2DeleteScopeMapOpt),
+
+    #[clap(name = "update_sup_scope_map", visible_aliases=&["create_sup_scope_map"])]
+    /// Update or add a new mapping from a group to scopes that it provides to members
+    UpdateSupScopeMap(Oauth2CreateScopeMapOpt),
+    #[clap(name = "delete_sup_scope_map")]
+    /// Remove a mapping from groups to scopes
+    DeleteSupScopeMap(Oauth2DeleteScopeMapOpt),
+
     #[clap(name = "reset_secrets")]
     /// Reset the secrets associated to this resource server
     ResetSecrets(Named),
+    #[clap(name = "show_basic_secret")]
+    /// Show the associated basic secret for this resource server
+    ShowBasicSecret(Named),
     #[clap(name = "delete")]
     /// Delete a oauth2 resource server
     Delete(Named),

kanidmd/core/src/actors/v1_read.rs

@@ -1067,6 +1067,56 @@ impl QueryServerReadV1 {
         .map(|sta| sta.into())
     }
 
+    #[instrument(
+        level = "info",
+        skip_all,
+        fields(uuid = ?eventid)
+    )]
+    pub async fn handle_oauth2_basic_secret_read(
+        &self,
+        uat: Option<String>,
+        filter: Filter<FilterInvalid>,
+        eventid: Uuid,
+    ) -> Result<Option<String>, OperationError> {
+        let ct = duration_from_epoch_now();
+        let idms_prox_read = self.idms.proxy_read_async().await;
+        let ident = idms_prox_read
+            .validate_and_parse_token_to_ident(uat.as_deref(), ct)
+            .map_err(|e| {
+                admin_error!("Invalid identity: {:?}", e);
+                e
+            })?;
+
+        // Make an event from the request
+        let srch =
+            match SearchEvent::from_internal_message(ident, &filter, None, &idms_prox_read.qs_read)
+            {
+                Ok(s) => s,
+                Err(e) => {
+                    admin_error!("Failed to begin oauth2 basic secret read: {:?}", e);
+                    return Err(e);
+                }
+            };
+
+        trace!(?srch, "Begin event");
+
+        // We have to use search_ext to guarantee acs was applied.
+        match idms_prox_read.qs_read.search_ext(&srch) {
+            Ok(mut entries) => {
+                let r = entries
+                    .pop()
+                    // From the entry, turn it into the value
+                    .and_then(|entry| {
+                        entry
+                            .get_ava_single("oauth2_rs_basic_secret")
+                            .and_then(|v| v.get_secret_str().map(str::to_string))
+                    });
+                Ok(r)
+            }
+            Err(e) => Err(e),
+        }
+    }
+
     #[instrument(
         level = "info",
         skip_all,

kanidmd/core/src/actors/v1_write.rs

@@ -1081,7 +1081,7 @@ impl QueryServerWriteV1 {
         skip_all,
         fields(uuid = ?eventid)
     )]
-    pub async fn handle_oauth2_scopemap_create(
+    pub async fn handle_oauth2_scopemap_update(
         &self,
         uat: Option<String>,
         group: String,
@@ -1190,6 +1190,120 @@ impl QueryServerWriteV1 {
             .and_then(|_| idms_prox_write.commit().map(|_| ()))
     }
 
+    #[instrument(
+        level = "info",
+        skip_all,
+        fields(uuid = ?eventid)
+    )]
+    pub async fn handle_oauth2_sup_scopemap_update(
+        &self,
+        uat: Option<String>,
+        group: String,
+        scopes: Vec<String>,
+        filter: Filter<FilterInvalid>,
+        eventid: Uuid,
+    ) -> Result<(), OperationError> {
+        // Because this is from internal, we can generate a real modlist, rather
+        // than relying on the proto ones.
+        let idms_prox_write = self.idms.proxy_write_async(duration_from_epoch_now()).await;
+        let ct = duration_from_epoch_now();
+
+        let ident = idms_prox_write
+            .validate_and_parse_token_to_ident(uat.as_deref(), ct)
+            .map_err(|e| {
+                admin_error!(err = ?e, "Invalid identity");
+                e
+            })?;
+
+        let group_uuid = idms_prox_write
+            .qs_write
+            .name_to_uuid(group.as_str())
+            .map_err(|e| {
+                admin_error!(err = ?e, "Error resolving group name to target");
+                e
+            })?;
+
+        let ml = ModifyList::new_append(
+            "oauth2_rs_sup_scope_map",
+            Value::new_oauthscopemap(group_uuid, scopes.into_iter().collect()).ok_or_else(
+                || OperationError::InvalidAttribute("Invalid Oauth Scope Map syntax".to_string()),
+            )?,
+        );
+
+        let mdf = match ModifyEvent::from_internal_parts(
+            ident,
+            &ml,
+            &filter,
+            &idms_prox_write.qs_write,
+        ) {
+            Ok(m) => m,
+            Err(e) => {
+                admin_error!(err = ?e, "Failed to begin modify");
+                return Err(e);
+            }
+        };
+
+        trace!(?mdf, "Begin modify event");
+
+        idms_prox_write
+            .qs_write
+            .modify(&mdf)
+            .and_then(|_| idms_prox_write.commit().map(|_| ()))
+    }
+
+    #[instrument(
+        level = "info",
+        skip_all,
+        fields(uuid = ?eventid)
+    )]
+    pub async fn handle_oauth2_sup_scopemap_delete(
+        &self,
+        uat: Option<String>,
+        group: String,
+        filter: Filter<FilterInvalid>,
+        eventid: Uuid,
+    ) -> Result<(), OperationError> {
+        let idms_prox_write = self.idms.proxy_write_async(duration_from_epoch_now()).await;
+        let ct = duration_from_epoch_now();
+
+        let ident = idms_prox_write
+            .validate_and_parse_token_to_ident(uat.as_deref(), ct)
+            .map_err(|e| {
+                admin_error!(err = ?e, "Invalid identity");
+                e
+            })?;
+
+        let group_uuid = idms_prox_write
+            .qs_write
+            .name_to_uuid(group.as_str())
+            .map_err(|e| {
+                admin_error!(err = ?e, "Error resolving group name to target");
+                e
+            })?;
+
+        let ml = ModifyList::new_remove("oauth2_rs_sup_scope_map", PartialValue::Refer(group_uuid));
+
+        let mdf = match ModifyEvent::from_internal_parts(
+            ident,
+            &ml,
+            &filter,
+            &idms_prox_write.qs_write,
+        ) {
+            Ok(m) => m,
+            Err(e) => {
+                admin_error!(err = ?e, "Failed to begin modify");
+                return Err(e);
+            }
+        };
+
+        trace!(?mdf, "Begin modify event");
+
+        idms_prox_write
+            .qs_write
+            .modify(&mdf)
+            .and_then(|_| idms_prox_write.commit().map(|_| ()))
+    }
+
     // ===== These below are internal only event types. =====
     #[instrument(
         level = "info",

kanidmd/core/src/https/mod.rs

@@ -569,11 +569,20 @@ pub fn create_https_server(
         .mapped_patch(&mut routemap, oauth2_id_patch)
         .mapped_delete(&mut routemap, oauth2_id_delete);
 
+    oauth2_route
+        .at("/:rs_name/_basic_secret")
+        .mapped_get(&mut routemap, oauth2_id_get_basic_secret);
+
     oauth2_route
         .at("/:id/_scopemap/:group")
         .mapped_post(&mut routemap, oauth2_id_scopemap_post)
         .mapped_delete(&mut routemap, oauth2_id_scopemap_delete);
 
+    oauth2_route
+        .at("/:id/_sup_scopemap/:group")
+        .mapped_post(&mut routemap, oauth2_id_sup_scopemap_post)
+        .mapped_delete(&mut routemap, oauth2_id_sup_scopemap_delete);
+
     let mut self_route = appserver.at("/v1/self");
     self_route.at("/").mapped_get(&mut routemap, whoami);
 

kanidmd/core/src/https/oauth2.rs

@@ -54,6 +54,23 @@ pub async fn oauth2_id_get(req: tide::Request<AppState>) -> tide::Result {
     to_tide_response(res, hvalue)
 }
 
+pub async fn oauth2_id_get_basic_secret(req: tide::Request<AppState>) -> tide::Result {
+    let uat = req.get_current_uat();
+
+    let id = req.get_url_param("rs_name")?;
+    let filter = oauth2_id(&id);
+
+    let (eventid, hvalue) = req.new_eventid();
+
+    let res = req
+        .state()
+        .qe_r_ref
+        .handle_oauth2_basic_secret_read(uat, filter, eventid)
+        .await;
+
+    to_tide_response(res, hvalue)
+}
+
 pub async fn oauth2_id_patch(mut req: tide::Request<AppState>) -> tide::Result {
     // Update a value / attrs
     let uat = req.get_current_uat();
@@ -86,7 +103,7 @@ pub async fn oauth2_id_scopemap_post(mut req: tide::Request<AppState>) -> tide::
     let res = req
         .state()
         .qe_w_ref
-        .handle_oauth2_scopemap_create(uat, group, scopes, filter, eventid)
+        .handle_oauth2_scopemap_update(uat, group, scopes, filter, eventid)
         .await;
     to_tide_response(res, hvalue)
 }
@@ -107,6 +124,40 @@ pub async fn oauth2_id_scopemap_delete(req: tide::Request<AppState>) -> tide::Re
     to_tide_response(res, hvalue)
 }
 
+pub async fn oauth2_id_sup_scopemap_post(mut req: tide::Request<AppState>) -> tide::Result {
+    let uat = req.get_current_uat();
+    let id = req.get_url_param("id")?;
+    let group = req.get_url_param("group")?;
+
+    let scopes: Vec<String> = req.body_json().await?;
+
+    let filter = oauth2_id(&id);
+
+    let (eventid, hvalue) = req.new_eventid();
+    let res = req
+        .state()
+        .qe_w_ref
+        .handle_oauth2_sup_scopemap_update(uat, group, scopes, filter, eventid)
+        .await;
+    to_tide_response(res, hvalue)
+}
+
+pub async fn oauth2_id_sup_scopemap_delete(req: tide::Request<AppState>) -> tide::Result {
+    let uat = req.get_current_uat();
+    let id = req.get_url_param("id")?;
+    let group = req.get_url_param("group")?;
+
+    let filter = oauth2_id(&id);
+
+    let (eventid, hvalue) = req.new_eventid();
+    let res = req
+        .state()
+        .qe_w_ref
+        .handle_oauth2_sup_scopemap_delete(uat, group, filter, eventid)
+        .await;
+    to_tide_response(res, hvalue)
+}
+
 pub async fn oauth2_id_delete(req: tide::Request<AppState>) -> tide::Result {
     // Delete this
     let uat = req.get_current_uat();

kanidmd/core/tests/oauth2_test.rs

@@ -78,31 +78,30 @@ async fn test_oauth2_openid_basic_flow() {
         .expect("Failed to configure account password");
 
     rsclient
-        .idm_oauth2_rs_update(
+        .idm_oauth2_rs_update("test_integration", None, None, None, true, true, true)
-            "test_integration",
-            None,
-            None,
-            None,
-            Some(vec!["read", "email", "openid"]),
-            false,
-            false,
-            false,
-        )
         .await
         .expect("Failed to update oauth2 config");
 
-    let oauth2_config = rsclient
+    rsclient
-        .idm_oauth2_rs_get("test_integration")
+        .idm_oauth2_rs_update_scope_map(
+            "test_integration",
+            "idm_all_accounts",
+            vec!["read", "email", "openid"],
+        )
+        .await
+        .expect("Failed to update oauth2 scopes");
+
+    rsclient
+        .idm_oauth2_rs_update_sup_scope_map("test_integration", "idm_all_accounts", vec!["admin"])
+        .await
+        .expect("Failed to update oauth2 scopes");
+
+    let client_secret = rsclient
+        .idm_oauth2_rs_get_basic_secret("test_integration")
         .await
         .ok()
         .flatten()
-        .expect("Failed to retrieve test_integration config");
+        .expect("Failed to retrieve test_integration basic secret");
-
-    let client_secret = oauth2_config
-        .attrs
-        .get("oauth2_rs_basic_secret")
-        .map(|s| s[0].to_string())
-        .expect("No basic secret present");
 
     // Get our admin's auth token for our new client.
     // We have to re-auth to update the mail field.
@@ -227,12 +226,18 @@ async fn test_oauth2_openid_basic_flow() {
         .await
         .expect("Failed to access response body");
 
-    let consent_token =
+    let consent_token = if let AuthorisationResponse::ConsentRequested {
-        if let AuthorisationResponse::ConsentRequested { consent_token, .. } = consent_req {
+        consent_token,
-            consent_token
+        scopes,
-        } else {
+        ..
-            unreachable!();
+    } = consent_req
-        };
+    {
+        // Note the supplemental scope here (admin)
+        assert!(scopes.contains(&"admin".to_string()));
+        consent_token
+    } else {
+        unreachable!();
+    };
 
     // Step 2 - we now send the consent get to the server which yields a redirect with a
     // state and code.

kanidmd/core/tests/proto_v1_test.rs

@@ -842,7 +842,6 @@ async fn test_server_rest_oauth2_basic_lifecycle() {
             None,
             Some("Test Integration"),
             Some("https://new_demo.example.com"),
-            Some(vec!["read", "email"]),
             true,
             true,
             true,
@@ -861,7 +860,7 @@ async fn test_server_rest_oauth2_basic_lifecycle() {
 
     // Check that we can add scope maps and delete them.
     rsclient
-        .idm_oauth2_rs_create_scope_map("test_integration", "system_admins", vec!["a", "b"])
+        .idm_oauth2_rs_update_scope_map("test_integration", "system_admins", vec!["a", "b"])
         .await
         .expect("Failed to create scope map");
 
@@ -874,10 +873,11 @@ async fn test_server_rest_oauth2_basic_lifecycle() {
 
     assert!(oauth2_config_updated != oauth2_config_updated2);
 
+    // Check we can update a scope map
     rsclient
-        .idm_oauth2_rs_delete_scope_map("test_integration", "system_admins")
+        .idm_oauth2_rs_update_scope_map("test_integration", "system_admins", vec!["a", "b", "c"])
         .await
-        .expect("Failed to delete scope map");
+        .expect("Failed to create scope map");
 
     let oauth2_config_updated3 = rsclient
         .idm_oauth2_rs_get("test_integration")
@@ -886,10 +886,26 @@ async fn test_server_rest_oauth2_basic_lifecycle() {
         .flatten()
         .expect("Failed to retrieve test_integration config");
 
-    eprintln!("{:?}", oauth2_config_updated);
+    assert!(oauth2_config_updated2 != oauth2_config_updated3);
-    eprintln!("{:?}", oauth2_config_updated3);
 
-    assert!(oauth2_config_updated == oauth2_config_updated3);
+    // Check we can delete a scope map.
+
+    rsclient
+        .idm_oauth2_rs_delete_scope_map("test_integration", "system_admins")
+        .await
+        .expect("Failed to delete scope map");
+
+    let oauth2_config_updated4 = rsclient
+        .idm_oauth2_rs_get("test_integration")
+        .await
+        .ok()
+        .flatten()
+        .expect("Failed to retrieve test_integration config");
+
+    eprintln!("{:?}", oauth2_config_updated);
+    eprintln!("{:?}", oauth2_config_updated4);
+
+    assert!(oauth2_config_updated == oauth2_config_updated4);
 
     // Delete the config
     rsclient

kanidmd/lib/src/constants/acp.rs

@@ -1180,7 +1180,7 @@ pub const JSON_IDM_HP_ACP_OAUTH2_MANAGE_PRIV_V1: &str = r#"{
             "oauth2_rs_name",
             "oauth2_rs_origin",
             "oauth2_rs_scope_map",
-            "oauth2_rs_implicit_scopes",
+            "oauth2_rs_sup_scope_map",
             "oauth2_rs_basic_secret",
             "oauth2_rs_token_key",
             "es256_private_key_der",
@@ -1195,7 +1195,7 @@ pub const JSON_IDM_HP_ACP_OAUTH2_MANAGE_PRIV_V1: &str = r#"{
             "oauth2_rs_name",
             "oauth2_rs_origin",
             "oauth2_rs_scope_map",
-            "oauth2_rs_implicit_scopes",
+            "oauth2_rs_sup_scope_map",
             "oauth2_rs_basic_secret",
             "oauth2_rs_token_key",
             "es256_private_key_der",
@@ -1209,8 +1209,8 @@ pub const JSON_IDM_HP_ACP_OAUTH2_MANAGE_PRIV_V1: &str = r#"{
             "displayname",
             "oauth2_rs_name",
             "oauth2_rs_origin",
+            "oauth2_rs_sup_scope_map",
             "oauth2_rs_scope_map",
-            "oauth2_rs_implicit_scopes",
             "oauth2_allow_insecure_client_disable_pkce",
             "oauth2_jwt_legacy_crypto_enable",
             "oauth2_prefer_short_username"
@@ -1222,8 +1222,8 @@ pub const JSON_IDM_HP_ACP_OAUTH2_MANAGE_PRIV_V1: &str = r#"{
             "displayname",
             "oauth2_rs_name",
             "oauth2_rs_origin",
+            "oauth2_rs_sup_scope_map",
             "oauth2_rs_scope_map",
-            "oauth2_rs_implicit_scopes",
             "oauth2_allow_insecure_client_disable_pkce",
             "oauth2_jwt_legacy_crypto_enable",
             "oauth2_prefer_short_username"

kanidmd/lib/src/constants/entries.rs

@@ -484,7 +484,7 @@ pub const JSON_SYSTEM_INFO_V1: &str = r#"{
         "class": ["object", "system_info", "system"],
         "uuid": ["00000000-0000-0000-0000-ffffff000001"],
         "description": ["System (local) info and metadata object."],
-        "version": ["8"]
+        "version": ["9"]
     }
 }"#;
 

kanidmd/lib/src/constants/schema.rs

@@ -663,6 +663,37 @@ pub const JSON_SCHEMA_ATTR_OAUTH2_RS_SCOPE_MAP: &str = r#"{
     }
 }"#;
 
+pub const JSON_SCHEMA_ATTR_OAUTH2_RS_SUP_SCOPE_MAP: &str = r#"{
+    "attrs": {
+      "class": [
+        "object",
+        "system",
+        "attributetype"
+      ],
+      "description": [
+        "A reference to a group mapped to scopes for the associated oauth2 resource server"
+      ],
+      "index": [
+        "EQUALITY"
+      ],
+      "unique": [
+        "false"
+      ],
+      "multivalue": [
+        "true"
+      ],
+      "attributename": [
+        "oauth2_rs_sup_scope_map"
+      ],
+      "syntax": [
+        "OAUTH_SCOPE_MAP"
+      ],
+      "uuid": [
+        "00000000-0000-0000-0000-ffff00000112"
+      ]
+    }
+}"#;
+
 pub const JSON_SCHEMA_ATTR_OAUTH2_RS_BASIC_SECRET: &str = r#"{
     "attrs": {
       "class": [
@@ -684,7 +715,7 @@ pub const JSON_SCHEMA_ATTR_OAUTH2_RS_BASIC_SECRET: &str = r#"{
         "oauth2_rs_basic_secret"
       ],
       "syntax": [
-        "UTF8STRING"
+        "SECRET_UTF8STRING"
       ],
       "uuid": [
         "00000000-0000-0000-0000-ffff00000083"
@@ -1434,7 +1465,7 @@ pub const JSON_SCHEMA_CLASS_OAUTH2_RS: &str = r#"
       "systemmay": [
         "description",
         "oauth2_rs_scope_map",
-        "oauth2_rs_implicit_scopes",
+        "oauth2_rs_sup_scope_map",
         "oauth2_allow_insecure_client_disable_pkce",
         "rs256_private_key_der",
         "oauth2_jwt_legacy_crypto_enable",

kanidmd/lib/src/constants/uuids.rs

@@ -189,6 +189,8 @@ pub const _UUID_SCHEMA_ATTR_OAUTH2_PREFERR_SHORT_USERNAME: Uuid =
 pub const _UUID_SCHEMA_ATTR_JWS_ES256_PRIVATE_KEY: Uuid =
     uuid!("00000000-0000-0000-0000-ffff00000110");
 pub const _UUID_SCHEMA_ATTR_API_TOKEN_SESSION: Uuid = uuid!("00000000-0000-0000-0000-ffff00000111");
+pub const _UUID_SCHEMA_ATTR_OAUTH2_RS_SUP_SCOPE_MAP: Uuid =
+    uuid!("00000000-0000-0000-0000-ffff00000112");
 
 // System and domain infos
 // I'd like to strongly criticise william of the past for making poor choices about these allocations.

kanidmd/lib/src/idm/oauth2.rs

@@ -174,9 +174,8 @@ pub struct Oauth2RS {
     displayname: String,
     uuid: Uuid,
     origin: Origin,
-    // Do we need optional maps?
     scope_maps: BTreeMap<Uuid, BTreeSet<String>>,
-    implicit_scopes: Vec<String>,
+    sup_scope_maps: BTreeMap<Uuid, BTreeSet<String>>,
     // Client Auth Type (basic is all we support for now.
     authz_secret: String,
     // Our internal exchange encryption material for this rs.
@@ -205,7 +204,7 @@ impl std::fmt::Debug for Oauth2RS {
             .field("uuid", &self.uuid)
             .field("origin", &self.origin)
             .field("scope_maps", &self.scope_maps)
-            .field("implicit_scopes", &self.implicit_scopes)
+            .field("sup_scope_maps", &self.sup_scope_maps)
             .finish()
     }
 }
@@ -297,7 +296,7 @@ impl<'a> Oauth2ResourceServersWriteTransaction<'a> {
                         .ok_or(OperationError::InvalidValueState)?;
                     trace!("authz_secret");
                     let authz_secret = ent
-                        .get_ava_single_utf8("oauth2_rs_basic_secret")
+                        .get_ava_single_secret("oauth2_rs_basic_secret")
                         .map(str::to_string)
                         .ok_or(OperationError::InvalidValueState)?;
                     trace!("token_key");
@@ -314,11 +313,11 @@ impl<'a> Oauth2ResourceServersWriteTransaction<'a> {
                         .cloned()
                         .unwrap_or_default();
 
-                    trace!("implicit_scopes");
+                    trace!("sup_scope_maps");
-                    let implicit_scopes = ent
+                    let sup_scope_maps = ent
-                        .get_ava_as_oauthscopes("oauth2_rs_implicit_scopes")
+                        .get_ava_as_oauthscopemaps("oauth2_rs_sup_scope_map")
-                        .map(|iter| iter.map(str::to_string).collect())
+                        .cloned()
-                        .unwrap_or_else(Vec::new);
+                        .unwrap_or_default();
 
                     trace!("oauth2_jwt_legacy_crypto_enable");
                     let jws_signer = if ent.get_ava_single_bool("oauth2_jwt_legacy_crypto_enable").unwrap_or(false) {
@@ -376,10 +375,18 @@ impl<'a> Oauth2ResourceServersWriteTransaction<'a> {
                     let mut iss = self.inner.origin.clone();
                     iss.set_path(&format!("/oauth2/openid/{}", name));
 
-                    let scopes_supported: BTreeSet<String> = implicit_scopes
+                    let scopes_supported: BTreeSet<String> =
-                        .iter()
+                    scope_maps
+                        .values()
+                        .flat_map(|bts| bts.iter())
+
+                        .chain(
+                            sup_scope_maps
+                                .values()
+                                .flat_map(|bts| bts.iter())
+                        )
+
                         .cloned()
-                        .chain(scope_maps.values().flat_map(|bts| bts.iter()).cloned())
                         .collect();
                     let scopes_supported: Vec<_> = scopes_supported.into_iter().collect();
 
@@ -390,7 +397,7 @@ impl<'a> Oauth2ResourceServersWriteTransaction<'a> {
                         uuid,
                         origin,
                         scope_maps,
-                        implicit_scopes,
+                        sup_scope_maps,
                         authz_secret,
                         token_fernet,
                         jws_signer,
@@ -517,7 +524,7 @@ impl Oauth2ResourceServersReadTransaction {
             return Err(Oauth2Error::AccessDenied);
         }
 
-        // scopes - you need to have every requested scope or this req is denied.
+        // scopes - you need to have every requested scope or this auth_req is denied.
         let req_scopes: BTreeSet<String> = auth_req
             .scope
             .split_ascii_whitespace()
@@ -537,20 +544,16 @@ impl Oauth2ResourceServersReadTransaction {
         }
 
         let uat_scopes: BTreeSet<String> = o2rs
-            .implicit_scopes
+            .scope_maps
             .iter()
-            .chain(
+            .filter_map(|(u, m)| {
-                o2rs.scope_maps
+                if ident.is_memberof(*u) {
-                    .iter()
+                    Some(m.iter())
-                    .filter_map(|(u, m)| {
+                } else {
-                        if ident.is_memberof(*u) {
+                    None
-                            Some(m.iter())
+                }
-                        } else {
+            })
-                            None
+            .flatten()
-                        }
-                    })
-                    .flatten(),
-            )
             .cloned()
             .collect();
 
@@ -560,18 +563,53 @@ impl Oauth2ResourceServersReadTransaction {
             .map(|s| s.to_string())
             .collect();
 
+        debug!(?o2rs.scope_maps);
+
+        // Due to the intersection above, this is correct because the equal len can only
+        // occur if all terms were satisfied.
         if avail_scopes.len() != req_scopes.len() {
             admin_warn!(
                 %ident,
-                %auth_req.scope,
+                requested_scopes = ?req_scopes,
+                available_scopes = ?uat_scopes,
                 "Identity does not have access to the requested scopes"
             );
             return Err(Oauth2Error::AccessDenied);
         }
 
+        drop(avail_scopes);
+
+        // ⚠️  At this point, per scopes we are *authorised*
+
+        // We now access the supplemental scopes that will be granted to this session. It is important
+        // we DO NOT do this prior to the requested scope check, just in case we accidentally
+        // confuse the two!
+
+        // The set of scopes that are being granted during this auth_request. This is a combination
+        // of the scopes that were requested, and the scopes we supplement.
+
+        // MICRO OPTIMISATION = flag if we have openid first, so we can into_iter here rather than
+        // cloning.
+        let openid_requested = req_scopes.contains("openid");
+
+        let granted_scopes: BTreeSet<String> = o2rs
+            .sup_scope_maps
+            .iter()
+            .filter_map(|(u, m)| {
+                if ident.is_memberof(*u) {
+                    Some(m.iter())
+                } else {
+                    None
+                }
+            })
+            .flatten()
+            .cloned()
+            .chain(req_scopes.into_iter())
+            .collect();
+
         let consent_previously_granted =
             if let Some(consent_scopes) = ident.get_oauth2_consent_scopes(o2rs.uuid) {
-                req_scopes.eq(consent_scopes)
+                granted_scopes.eq(consent_scopes)
             } else {
                 false
             };
@@ -579,7 +617,7 @@ impl Oauth2ResourceServersReadTransaction {
         if consent_previously_granted {
             admin_info!(
                 "User has previously consented, permitting. {:?}",
-                req_scopes
+                granted_scopes
             );
 
             // Setup for the permit success
@@ -587,7 +625,7 @@ impl Oauth2ResourceServersReadTransaction {
                 uat: uat.clone(),
                 code_challenge,
                 redirect_uri: auth_req.redirect_uri.clone(),
-                scopes: avail_scopes,
+                scopes: granted_scopes.into_iter().collect(),
                 nonce: auth_req.nonce.clone(),
             };
 
@@ -616,9 +654,11 @@ impl Oauth2ResourceServersReadTransaction {
             //
             // https://openid.net/specs/openid-connect-basic-1_0.html#StandardClaims
 
-            let pii_scopes = if req_scopes.contains("openid") {
+            // IMPORTANT DISTINCTION - Here req scopes must contain openid, but the PII can be supplemented
+            // be the servers scopes!
+            let pii_scopes = if openid_requested {
                 let mut pii_scopes = Vec::with_capacity(2);
-                if req_scopes.contains("email") {
+                if granted_scopes.contains("email") {
                     pii_scopes.push("email".to_string());
                     pii_scopes.push("email_verified".to_string());
                 }
@@ -639,7 +679,7 @@ impl Oauth2ResourceServersReadTransaction {
                 state: auth_req.state.clone(),
                 code_challenge,
                 redirect_uri: auth_req.redirect_uri.clone(),
-                scopes: avail_scopes.clone(),
+                scopes: granted_scopes.iter().cloned().collect(),
                 nonce: auth_req.nonce.clone(),
             };
 
@@ -655,7 +695,7 @@ impl Oauth2ResourceServersReadTransaction {
 
             Ok(AuthoriseResponse::ConsentRequested {
                 client_name: o2rs.displayname.clone(),
-                scopes: avail_scopes,
+                scopes: granted_scopes.into_iter().collect(),
                 pii_scopes,
                 consent_token,
             })
@@ -1432,16 +1472,25 @@ mod tests {
                 "oauth2_rs_origin",
                 Value::new_url_s("https://demo.example.com").unwrap()
             ),
-            (
-                "oauth2_rs_implicit_scopes",
-                Value::new_oauthscope("openid").expect("invalid oauthscope")
-            ),
             // System admins
             (
                 "oauth2_rs_scope_map",
                 Value::new_oauthscopemap(UUID_SYSTEM_ADMINS, btreeset!["read".to_string()])
                     .expect("invalid oauthscope")
             ),
+            (
+                "oauth2_rs_scope_map",
+                Value::new_oauthscopemap(UUID_IDM_ALL_ACCOUNTS, btreeset!["openid".to_string()])
+                    .expect("invalid oauthscope")
+            ),
+            (
+                "oauth2_rs_sup_scope_map",
+                Value::new_oauthscopemap(
+                    UUID_IDM_ALL_ACCOUNTS,
+                    btreeset!["supplement".to_string()]
+                )
+                .expect("invalid oauthscope")
+            ),
             (
                 "oauth2_allow_insecure_client_disable_pkce",
                 Value::new_bool(!enable_pkce)
@@ -1459,7 +1508,7 @@ mod tests {
             .internal_search_uuid(&uuid)
             .expect("Failed to retrieve oauth2 resource entry ");
         let secret = entry
-            .get_ava_single_utf8("oauth2_rs_basic_secret")
+            .get_ava_single_secret("oauth2_rs_basic_secret")
             .map(str::to_string)
             .expect("No oauth2_rs_basic_secret found");
 
@@ -2039,7 +2088,7 @@ mod tests {
 
                 eprintln!("👉  {:?}", intr_response);
                 assert!(intr_response.active);
-                assert!(intr_response.scope.as_deref() == Some("openid"));
+                assert!(intr_response.scope.as_deref() == Some("openid supplement"));
                 assert!(intr_response.client_id.as_deref() == Some("test_resource_server"));
                 assert!(intr_response.username.as_deref() == Some("admin@example.com"));
                 assert!(intr_response.token_type.as_deref() == Some("access_token"));
@@ -2244,7 +2293,12 @@ mod tests {
 
             eprintln!("{:?}", discovery.scopes_supported);
             assert!(
-                discovery.scopes_supported == Some(vec!["openid".to_string(), "read".to_string()])
+                discovery.scopes_supported
+                    == Some(vec![
+                        "openid".to_string(),
+                        "read".to_string(),
+                        "supplement".to_string(),
+                    ])
             );
 
             assert!(discovery.response_types_supported == vec![ResponseType::Code]);
@@ -2388,7 +2442,9 @@ mod tests {
                 assert!(oidc.jti.is_none());
                 assert!(oidc.s_claims.name == Some("System Administrator".to_string()));
                 assert!(oidc.s_claims.preferred_username == Some("admin@example.com".to_string()));
-                assert!(oidc.s_claims.scopes == vec!["openid".to_string()]);
+                assert!(
+                    oidc.s_claims.scopes == vec!["openid".to_string(), "supplement".to_string()]
+                );
                 assert!(oidc.claims.is_empty());
                 // Does our access token work with the userinfo endpoint?
                 // Do the id_token details line up to the userinfo?
@@ -2622,8 +2678,12 @@ mod tests {
                             PartialValue::new_iname("test_resource_server")
                         )),
                         ModifyList::new_list(vec![Modify::Present(
-                            AttrString::from("oauth2_rs_implicit_scopes"),
+                            AttrString::from("oauth2_rs_scope_map"),
-                            Value::new_oauthscope("email").expect("invalid oauthscope"),
+                            Value::new_oauthscopemap(
+                                UUID_IDM_ALL_ACCOUNTS,
+                                btreeset!["email".to_string(), "openid".to_string()],
+                            )
+                            .expect("invalid oauthscope"),
                         )]),
                     )
                 };
@@ -2671,7 +2731,76 @@ mod tests {
                         unreachable!();
                     };
 
+                drop(idms_prox_read);
+
                 // Success! We had to consent again due to the change :)
+
+                // Now change the supplemental scopes on the oauth2 instance, this revokes the permit.
+                let idms_prox_write = idms.proxy_write(ct);
+
+                let me_extend_scopes = unsafe {
+                    ModifyEvent::new_internal_invalid(
+                        filter!(f_eq(
+                            "oauth2_rs_name",
+                            PartialValue::new_iname("test_resource_server")
+                        )),
+                        ModifyList::new_list(vec![Modify::Present(
+                            AttrString::from("oauth2_rs_sup_scope_map"),
+                            Value::new_oauthscopemap(
+                                UUID_IDM_ALL_ACCOUNTS,
+                                btreeset!["newscope".to_string()],
+                            )
+                            .expect("invalid oauthscope"),
+                        )]),
+                    )
+                };
+
+                assert!(idms_prox_write.qs_write.modify(&me_extend_scopes).is_ok());
+                assert!(idms_prox_write.commit().is_ok());
+
+                // And do the workflow once more to see if we need to consent again.
+
+                let idms_prox_read = idms.proxy_read();
+
+                // We need to reload our identity
+                let ident = idms_prox_read
+                    .process_uat_to_identity(&uat, ct)
+                    .expect("Unable to process uat");
+
+                let (_code_verifier, code_challenge) = create_code_verifier!("Whar Garble");
+
+                let auth_req = AuthorisationRequest {
+                    response_type: "code".to_string(),
+                    client_id: "test_resource_server".to_string(),
+                    state: "123".to_string(),
+                    pkce_request: Some(PkceRequest {
+                        code_challenge: Base64UrlSafeData(code_challenge),
+                        code_challenge_method: CodeChallengeMethod::S256,
+                    }),
+                    redirect_uri: Url::parse("https://demo.example.com/oauth2/result").unwrap(),
+                    // Note the scope isn't requested here!
+                    scope: "openid email".to_string(),
+                    nonce: Some("abcdef".to_string()),
+                    oidc_ext: Default::default(),
+                    unknown_keys: Default::default(),
+                };
+
+                let consent_request = idms_prox_read
+                    .check_oauth2_authorisation(&ident, &uat, &auth_req, ct)
+                    .expect("Oauth2 authorisation failed");
+
+                // Should be present in the consent phase however!
+                let _consent_token = if let AuthoriseResponse::ConsentRequested {
+                    consent_token,
+                    scopes,
+                    ..
+                } = consent_request
+                {
+                    assert!(scopes.contains(&"newscope".to_string()));
+                    consent_token
+                } else {
+                    unreachable!();
+                };
             }
         )
     }
@@ -2727,7 +2856,7 @@ mod tests {
                 // Assert that the ident now has the consents.
                 assert!(
                     ident.get_oauth2_consent_scopes(o2rs_uuid)
-                        == Some(&btreeset!["openid".to_string()])
+                        == Some(&btreeset!["openid".to_string(), "supplement".to_string()])
                 );
 
                 // Now trigger the delete of the RS

kanidmd/lib/src/plugins/jwskeygen.rs

@@ -14,7 +14,7 @@ macro_rules! keygen_transform {
         if $e.attribute_equality("class", &PVCLASS_OAUTH2_BASIC) {
             if !$e.attribute_pres("oauth2_rs_basic_secret") {
                 security_info!("regenerating oauth2 basic secret");
-                let v = Value::new_utf8(password_from_random());
+                let v = Value::SecretValue(password_from_random());
                 $e.add_ava("oauth2_rs_basic_secret", v);
             }
             if !$e.attribute_pres("oauth2_rs_token_key") {
@@ -115,8 +115,9 @@ mod tests {
                 Value::new_url_s("https://demo.example.com").unwrap()
             ),
             (
-                "oauth2_rs_implicit_scopes",
+                "oauth2_rs_scope_map",
-                Value::new_oauthscope("read").expect("Invalid scope")
+                Value::new_oauthscopemap(UUID_IDM_ALL_ACCOUNTS, btreeset!["read".to_string()])
+                    .expect("invalid oauthscope")
             )
         );
 
@@ -153,10 +154,11 @@ mod tests {
                 Value::new_url_s("https://demo.example.com").unwrap()
             ),
             (
-                "oauth2_rs_implicit_scopes",
+                "oauth2_rs_scope_map",
-                Value::new_oauthscope("read").expect("Invalid scope")
+                Value::new_oauthscopemap(UUID_IDM_ALL_ACCOUNTS, btreeset!["read".to_string()])
+                    .expect("invalid oauthscope")
             ),
-            ("oauth2_rs_basic_secret", Value::new_utf8s("12345")),
+            ("oauth2_rs_basic_secret", Value::new_secret_str("12345")),
             ("oauth2_rs_token_key", Value::new_secret_str("12345"))
         );
 
@@ -179,7 +181,7 @@ mod tests {
                 assert!(e.attribute_pres("oauth2_rs_basic_secret"));
                 assert!(e.attribute_pres("oauth2_rs_token_key"));
                 // Check the values are different.
-                assert!(e.get_ava_single_utf8("oauth2_rs_basic_secret") != Some("12345"));
+                assert!(e.get_ava_single_secret("oauth2_rs_basic_secret") != Some("12345"));
                 assert!(e.get_ava_single_secret("oauth2_rs_token_key") != Some("12345"));
             }
         );

kanidmd/lib/src/plugins/refint.rs

@@ -734,10 +734,6 @@ mod tests {
                 "oauth2_rs_origin",
                 Value::new_url_s("https://demo.example.com").unwrap()
             ),
-            (
-                "oauth2_rs_implicit_scopes",
-                Value::new_oauthscope("test").expect("Invalid scope")
-            ),
             (
                 "oauth2_rs_scope_map",
                 Value::new_oauthscopemap(

kanidmd/lib/src/server.rs

@@ -1160,6 +1160,10 @@ impl QueryServer {
             migrate_txn.migrate_7_to_8()?;
         }
 
+        if system_info_version < 9 {
+            migrate_txn.migrate_8_to_9()?;
+        }
+
         migrate_txn.commit()?;
         // Migrations complete. Init idm will now set the version as needed.
 
@@ -2265,6 +2269,89 @@ impl<'a> QueryServerWriteTransaction<'a> {
         // Complete
     }
 
+    /// Migrate 8 to 9
+    ///
+    /// This migration updates properties of oauth2 relying server properties. First, it changes
+    /// the former basic value to a secret utf8string.
+    ///
+    /// The second change improves the current scope system to remove the implicit scope type.
+    #[instrument(level = "debug", skip_all)]
+    pub fn migrate_8_to_9(&self) -> Result<(), OperationError> {
+        admin_warn!("starting 8 to 9 migration.");
+        let filt = filter_all!(f_or!([
+            f_eq("class", PVCLASS_OAUTH2_RS.clone()),
+            f_eq("class", PVCLASS_OAUTH2_BASIC.clone()),
+        ]));
+
+        let pre_candidates = self.internal_search(filt).map_err(|e| {
+            admin_error!(err = ?e, "migrate_8_to_9 internal search failure");
+            e
+        })?;
+
+        // If there is nothing, we donn't need to do anything.
+        if pre_candidates.is_empty() {
+            admin_info!("migrate_8_to_9 no entries to migrate, complete");
+            return Ok(());
+        }
+
+        // Change the value type.
+        let mut candidates: Vec<Entry<EntryInvalid, EntryCommitted>> = pre_candidates
+            .iter()
+            .map(|er| er.as_ref().clone().invalidate(self.cid.clone()))
+            .collect();
+
+        candidates.iter_mut().try_for_each(|er| {
+            // Migrate basic secrets if they exist.
+            let nvs = er
+                .get_ava_set("oauth2_rs_basic_secret")
+                .and_then(|vs| vs.as_utf8_iter())
+                .and_then(|vs_iter| {
+                    ValueSetSecret::from_iter(vs_iter.map(|s: &str| s.to_string()))
+                });
+            if let Some(nvs) = nvs {
+                er.set_ava_set("oauth2_rs_basic_secret", nvs)
+            }
+
+            // Migrate implicit scopes if they exist.
+            let nv = if let Some(vs) = er.get_ava_set("oauth2_rs_implicit_scopes") {
+                vs.as_oauthscope_set()
+                    .map(|v| Value::OauthScopeMap(UUID_IDM_ALL_PERSONS, v.clone()))
+            } else {
+                None
+            };
+
+            if let Some(nv) = nv {
+                er.add_ava("oauth2_rs_scope_map", nv)
+            }
+            er.purge_ava("oauth2_rs_implicit_scopes");
+
+            Ok(())
+        })?;
+
+        // Schema check all.
+        let res: Result<Vec<Entry<EntrySealed, EntryCommitted>>, SchemaError> = candidates
+            .into_iter()
+            .map(|e| e.validate(&self.schema).map(|e| e.seal(&self.schema)))
+            .collect();
+
+        let norm_cand: Vec<Entry<_, _>> = match res {
+            Ok(v) => v,
+            Err(e) => {
+                admin_error!("migrate_8_to_9 schema error -> {:?}", e);
+                return Err(OperationError::SchemaViolation(e));
+            }
+        };
+
+        // Write them back.
+        self.be_txn
+            .modify(&self.cid, &pre_candidates, &norm_cand)
+            .map_err(|e| {
+                admin_error!("migrate_8_to_9 modification failure -> {:?}", e);
+                e
+            })
+        // Complete
+    }
+
     // These are where searches and other actions are actually implemented. This
     // is the "internal" version, where we define the event as being internal
     // only, allowing certain plugin by passes etc.
@@ -2567,6 +2654,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
             JSON_SCHEMA_ATTR_DYNGROUP_FILTER,
             JSON_SCHEMA_ATTR_JWS_ES256_PRIVATE_KEY,
             JSON_SCHEMA_ATTR_API_TOKEN_SESSION,
+            JSON_SCHEMA_ATTR_OAUTH2_RS_SUP_SCOPE_MAP,
             JSON_SCHEMA_CLASS_PERSON,
             JSON_SCHEMA_CLASS_ORGPERSON,
             JSON_SCHEMA_CLASS_GROUP,

kanidmd/lib/src/valueset/oauth.rs

@@ -193,11 +193,20 @@ impl ValueSetT for ValueSetOauthScopeMap {
     fn insert_checked(&mut self, value: Value) -> Result<bool, OperationError> {
         match value {
             Value::OauthScopeMap(u, m) => {
-                if let BTreeEntry::Vacant(e) = self.map.entry(u) {
+                match self.map.entry(u) {
-                    e.insert(m);
+                    BTreeEntry::Vacant(e) => {
-                    Ok(true)
+                        e.insert(m);
-                } else {
+                        Ok(true)
-                    Ok(false)
+                    }
+                    // In the case that the value already exists, we update it. This is a quirk
+                    // of the oauth2 scope map type where add_ava assumes that a value's entire state
+                    // will be reflected, but we were only checking the *uuid* existed, not it's
+                    // associated map state. So by always replacing on a present, we are true to
+                    // the intent of the api.
+                    BTreeEntry::Occupied(mut e) => {
+                        e.insert(m);
+                        Ok(true)
+                    }
                 }
             }
             _ => Err(OperationError::InvalidValueState),