Merge 48f7324080 into 9bf17c4846

idm_admin and admin should be exempted from the denied names process,
as these values will already be denied due to attribute uniqueness.
Additionally improved the denied names check to only validate the
name during a change, not during a modifification. This way entries
that become denied can get themself out of the pickle.

CONTRIBUTORS.md

@@ -1,6 +1,6 @@
 ## Author
 
-- William Brown (Firstyear): william@blackhats.net.au
+- William Brown (Firstyear): <william@blackhats.net.au>
 
 ## Contributors
 
@@ -44,6 +44,7 @@
 - adamcstephens
 - Chris Olstrom (colstrom)
 - Christopher-Robin (cebbinghaus)
+- Krzysztof Dajka (alteriks)
 - Fabian Kammel (datosh)
 - Andris Raugulis (arthepsy)
 - Jason (argonaut0)

book/src/integrations/oauth2/examples.md

@@ -556,6 +556,65 @@ php occ config:app:set --value=0 user_oidc allow_multiple_user_backends
 You can login directly by appending `?direct=1` to your login page. You can re-enable other backends
 by setting the value to `1`
 
+## OAuth2 Proxy
+
+OAuth2 Proxy is a reverse proxy that provides authentication with OpenID Connect identity providers.
+It is typically used to secure web applications without native OpenID Connect support.
+
+Prepare the environment.
+Due to a [lack of public client support](https://github.com/oauth2-proxy/oauth2-proxy/issues/1714) we have to set it up as a basic client. 
+
+```bash
+kanidm system oauth2 create webapp 'webapp.example.com' 'https://webapp.example.com'
+kanidm system add-redirect-url webapp 'https://webapp.example.com/oauth2/callback'
+kanidm system oauth2 update-scope-map webapp email openid
+kanidm system oauth2 get webapp
+kanidm system oauth2 show-basic-secret webapp
+<SECRET>
+```
+
+Create a user group.
+
+```bash
+kanidm group create 'webapp_admin'
+```
+
+Setup the claim-map to add `webapp_group` to the userinfo claim.
+
+```bash
+kanidm system oauth2 update-claim-map-join 'webapp' 'webapp_group' array
+kanidm system oauth2 update-claim-map 'webapp' 'webapp_group' 'webapp_admin' 'webapp_admin'
+```
+
+Authorize users for the application.
+Additionally OAuth2 Proxy requires all users have an email, reference this issue for more details:
+
+- <https://github.com/oauth2-proxy/oauth2-proxy/issues/2667>
+
+```bash
+kanidm person update '<user>' --legalname 'Personal Name' --mail 'user@example.com'
+kanidm group add-members 'webapp_admin' '<user>'
+```
+
+And add the following to your OAuth2 Proxy config.
+
+```toml
+provider = "oidc"
+scope = "openid email"
+# change to match your kanidm domain and client id
+oidc_issuer_url = "https://idm.example.com/oauth2/openid/webapp"
+# client ID from `kanidm system oauth2 create`
+client_id = "webapp"
+# redirect URL from `kanidm system add-redirect-url webapp`
+redirect_url = "https://webapp.example.com/oauth2/callback"
+# claim name from `kanidm system oauth2 update-claim-map-join`
+oidc_groups_claim = "webapp_group"
+# user group from `kanidm group create`
+allowed_groups = ["webapp_admin"]
+# secret from `kanidm system oauth2 show-basic-secret webapp`
+client_secret = "<SECRET>"
+```
+
 ## Outline
 
 > These instructions were tested with self-hosted Outline 0.80.2.

examples/proxmox.md

@@ -0,0 +1,91 @@
+# Proxmox PVE/PBS
+
+## Helpful links
+
+- <https://pve.proxmox.com/wiki/User_Management>
+- <https://pve.proxmox.com/pve-docs/pve-admin-guide.html#pveum_openid>
+
+## Proxmox OIDC limitation
+
+As of December 2024, the OIDC implementation in Proxmox supports only authentication.
+Authorization has to be done manually.
+Mapping user to specific groups won't work yet (steps 2,3,4).
+
+Patch for this feature exists, but it hasn't been tested extensively:
+<https://lore.proxmox.com/pve-devel/20240901165512.687801-1-thomas@atskinner.net/>
+See also:
+<https://forum.proxmox.com/threads/openid-connect-default-group.103394/>
+
+## On Kanidm
+
+### 1. Create the proxmox resource server and configure the redirect URL
+
+```bash
+kanidm system oauth2 create proxmox "proxmox" https://yourproxmox.example.com
+kanidm system oauth2 add-redirect-url "proxmox" https://yourproxmox.example.com
+```
+
+### 2. Create the appropriate group(s)
+
+```bash
+kanidm group create proxmox_users --name idm_admin
+kanidm group create proxmox_admins --name idm_admin
+```
+
+### 3. Add the appropriate users to the group
+
+```bash
+kanidm group add-members proxmox_users user.name
+kanidm group add-members proxmox_admins user.name
+```
+
+### 4. scope map
+
+```bash
+kanidm system oauth2 update-claim-map-join 'proxmox' 'proxmox_role' array
+kanidm system oauth2 update-claim-map 'proxmox' 'proxmox_role' 'proxmox_admins' 'admin'
+kanidm system oauth2 update-claim-map 'proxmox' 'proxmox_role' 'proxmox_users' 'user'
+```
+
+### 5. Add the scopes
+
+```bash
+kanidm system oauth2 update-scope-map proxmox proxmox_users email profile openid
+```
+
+### 6. Get the client secret
+
+```bash
+kanidm system oauth2 show-basic-secret proxmox
+```
+
+Copy the value that is returned.
+
+## On proxmox server
+
+### Using WebGUI
+
+Go to <https://yourproxmox.example.com>
+Select Datacenter->Realms->Add->OpenID Connect Server
+![](media/kanidm_proxmox.png)
+Issuer URL:
+
+- <https://idm.example.com:8443/oauth2/openid/proxmox>
+When kanidm is behind reverse proxy or when using docker port mapping:
+- <https://idm.example.com/oauth2/openid/proxmox>
+
+Realm: give some proper name or anything that's meaningful
+
+Client ID: name given in step 1 (resource server)
+
+Client Key: secret from step 6
+
+Autocreate Users: Automatically create users if they do not exist. Users are stored in Proxmox Cluster File System (pmxcfs) - /etc/pve/user.cfg
+
+### Using CLI
+
+Login to proxmox node and execute:
+
+```bash
+pveum realm add kanidm --type openid --issuer-url https://idm.example.com/oauth2/openid/proxmox --client-id proxmox --client-key="secret from step 6" --username-claim username --scopes="email profile openid" --autocreate
+```

server/lib/src/constants/schema.rs

@@ -208,6 +208,16 @@ pub static ref SCHEMA_ATTR_DENIED_NAME: SchemaAttribute = SchemaAttribute {
     ..Default::default()
 };
 
+pub static ref SCHEMA_ATTR_DENIED_NAME_DL10: SchemaAttribute = SchemaAttribute {
+    uuid: UUID_SCHEMA_ATTR_DENIED_NAME,
+    name: Attribute::DeniedName,
+    description: "Iname values that are not allowed to be used in 'name'.".to_string(),
+
+    syntax: SyntaxType::Utf8StringIname,
+    multivalue: true,
+    ..Default::default()
+};
+
 pub static ref SCHEMA_ATTR_DOMAIN_TOKEN_KEY: SchemaAttribute = SchemaAttribute {
     uuid: UUID_SCHEMA_ATTR_DOMAIN_TOKEN_KEY,
     name: Attribute::DomainTokenKey,

server/lib/src/plugins/valuedeny.rs

@@ -10,7 +10,7 @@ impl Plugin for ValueDeny {
         "plugin_value_deny"
     }
 
-    #[instrument(level = "debug", name = "base_pre_create_transform", skip_all)]
+    #[instrument(level = "debug", name = "denied_names_pre_create_transform", skip_all)]
     #[allow(clippy::cognitive_complexity)]
     fn pre_create_transform(
         qs: &mut QueryServerWriteTransaction,
@@ -19,9 +19,25 @@ impl Plugin for ValueDeny {
     ) -> Result<(), OperationError> {
         let denied_names = qs.denied_names();
 
+        if denied_names.is_empty() {
+            // Nothing to check.
+            return Ok(());
+        }
+
         let mut pass = true;
 
         for entry in cand {
+            // If the entry doesn't have a uuid, it's invalid anyway and will fail schema.
+            if let Some(e_uuid) = entry.get_uuid() {
+                // SAFETY - Thanks to JpWarren blowing his nipper clean off, we need to
+                // assert that the break glass and system accounts are NOT subject to
+                // this process.
+                if e_uuid < DYNAMIC_RANGE_MINIMUM_UUID {
+                    // These entries are exempt
+                    continue;
+                }
+            }
+
             if let Some(name) = entry.get_ava_single_iname(Attribute::Name) {
                 if denied_names.contains(name) {
                     pass = false;
@@ -37,27 +53,24 @@ impl Plugin for ValueDeny {
         }
     }
 
-    #[instrument(level = "debug", name = "base_pre_modify", skip_all)]
     fn pre_modify(
         qs: &mut QueryServerWriteTransaction,
-        _pre_cand: &[Arc<EntrySealedCommitted>],
+        pre_cand: &[Arc<EntrySealedCommitted>],
         cand: &mut Vec<Entry<EntryInvalid, EntryCommitted>>,
         _me: &ModifyEvent,
     ) -> Result<(), OperationError> {
-        Self::modify(qs, cand)
+        Self::modify(qs, pre_cand, cand)
     }
 
-    #[instrument(level = "debug", name = "base_pre_modify", skip_all)]
     fn pre_batch_modify(
         qs: &mut QueryServerWriteTransaction,
-        _pre_cand: &[Arc<EntrySealedCommitted>],
+        pre_cand: &[Arc<EntrySealedCommitted>],
         cand: &mut Vec<Entry<EntryInvalid, EntryCommitted>>,
         _me: &BatchModifyEvent,
     ) -> Result<(), OperationError> {
-        Self::modify(qs, cand)
+        Self::modify(qs, pre_cand, cand)
     }
 
-    #[instrument(level = "debug", name = "base::verify", skip_all)]
     fn verify(qs: &mut QueryServerReadTransaction) -> Vec<Result<(), ConsistencyError>> {
         let denied_names = qs.denied_names().clone();
 
@@ -68,7 +81,15 @@ impl Plugin for ValueDeny {
             match qs.internal_search(filt) {
                 Ok(entries) => {
                     for entry in entries {
-                        results.push(Err(ConsistencyError::DeniedName(entry.get_uuid())));
+                        let e_uuid = entry.get_uuid();
+                        // SAFETY - Thanks to JpWarren blowing his nipper clean off, we need to
+                        // assert that the break glass accounts are NOT subject to this process.
+                        if e_uuid < DYNAMIC_RANGE_MINIMUM_UUID {
+                            // These entries are exempt
+                            continue;
+                        }
+
+                        results.push(Err(ConsistencyError::DeniedName(e_uuid)));
                     }
                 }
                 Err(err) => {
@@ -83,17 +104,37 @@ impl Plugin for ValueDeny {
 }
 
 impl ValueDeny {
+    #[instrument(level = "debug", name = "denied_names_modify", skip_all)]
     fn modify(
         qs: &mut QueryServerWriteTransaction,
-        cand: &mut Vec<Entry<EntryInvalid, EntryCommitted>>,
+        pre_cand: &[Arc<EntrySealedCommitted>],
+        cand: &mut [EntryInvalidCommitted],
     ) -> Result<(), OperationError> {
         let denied_names = qs.denied_names();
 
+        if denied_names.is_empty() {
+            // Nothing to check.
+            return Ok(());
+        }
+
         let mut pass = true;
 
-        for entry in cand {
-            if let Some(name) = entry.get_ava_single_iname(Attribute::Name) {
-                if denied_names.contains(name) {
+        for (pre_entry, post_entry) in pre_cand.iter().zip(cand.iter()) {
+            // If the entry doesn't have a uuid, it's invalid anyway and will fail schema.
+            let e_uuid = pre_entry.get_uuid();
+            // SAFETY - Thanks to JpWarren blowing his nipper clean off, we need to
+            // assert that the break glass accounts are NOT subject to this process.
+            if e_uuid < DYNAMIC_RANGE_MINIMUM_UUID {
+                // These entries are exempt
+                continue;
+            }
+
+            let pre_name = pre_entry.get_ava_single_iname(Attribute::Name);
+            let post_name = post_entry.get_ava_single_iname(Attribute::Name);
+
+            if let Some(name) = post_name {
+                // Only if the name is changing, and is denied.
+                if pre_name != post_name && denied_names.contains(name) {
                     pass = false;
                     error!(?name, "name denied by system configuration");
                 }
@@ -117,10 +158,10 @@ mod tests {
 
         let me_inv_m = ModifyEvent::new_internal_invalid(
             filter!(f_eq(Attribute::Uuid, PVUUID_SYSTEM_CONFIG.clone())),
-            ModifyList::new_list(vec![Modify::Present(
-                Attribute::DeniedName,
-                Value::new_iname("tobias"),
-            )]),
+            ModifyList::new_list(vec![
+                Modify::Present(Attribute::DeniedName, Value::new_iname("tobias")),
+                Modify::Present(Attribute::DeniedName, Value::new_iname("ellie")),
+            ]),
         );
         assert!(server_txn.modify(&me_inv_m).is_ok());
 
@@ -148,30 +189,103 @@ mod tests {
 
     #[qs_test]
     async fn test_valuedeny_modify(server: &QueryServer) {
-        setup_name_deny(server).await;
-
+        // Create an entry that has a name which will become denied to test how it
+        // interacts.
         let mut server_txn = server.write(duration_from_epoch_now()).await.unwrap();
-        let t_uuid = Uuid::new_v4();
+        let e_uuid = Uuid::new_v4();
         assert!(server_txn
             .internal_create(vec![entry_init!(
                 (Attribute::Class, EntryClass::Object.to_value()),
                 (Attribute::Class, EntryClass::Account.to_value()),
                 (Attribute::Class, EntryClass::Person.to_value()),
-                (Attribute::Name, Value::new_iname("newname")),
-                (Attribute::Uuid, Value::Uuid(t_uuid)),
-                (Attribute::Description, Value::new_utf8s("Tobias")),
-                (Attribute::DisplayName, Value::new_utf8s("Tobias"))
+                (Attribute::Name, Value::new_iname("ellie")),
+                (Attribute::Uuid, Value::Uuid(e_uuid)),
+                (Attribute::Description, Value::new_utf8s("Ellie Meow")),
+                (Attribute::DisplayName, Value::new_utf8s("Ellie Meow"))
             ),])
             .is_ok());
 
-        // Now mod it
+        assert!(server_txn.commit().is_ok());
 
+        setup_name_deny(server).await;
+
+        let mut server_txn = server.write(duration_from_epoch_now()).await.unwrap();
+
+        // Attempt to mod ellie.
+
+        // Can mod a different attribute
         assert!(server_txn
             .internal_modify_uuid(
-                t_uuid,
+                e_uuid,
+                &ModifyList::new_purge_and_set(Attribute::DisplayName, Value::new_utf8s("tobias"))
+            )
+            .is_ok());
+
+        // Can't mod to another invalid name.
+        assert!(server_txn
+            .internal_modify_uuid(
+                e_uuid,
                 &ModifyList::new_purge_and_set(Attribute::Name, Value::new_iname("tobias"))
             )
             .is_err());
+
+        // Can mod to a valid name.
+        assert!(server_txn
+            .internal_modify_uuid(
+                e_uuid,
+                &ModifyList::new_purge_and_set(
+                    Attribute::Name,
+                    Value::new_iname("miss_meowington")
+                )
+            )
+            .is_ok());
+
+        // Now mod from the valid name to an invalid one.
+        assert!(server_txn
+            .internal_modify_uuid(
+                e_uuid,
+                &ModifyList::new_purge_and_set(Attribute::Name, Value::new_iname("tobias"))
+            )
+            .is_err());
+
+        assert!(server_txn.commit().is_ok());
+    }
+
+    #[qs_test]
+    async fn test_valuedeny_jpwarren_special(server: &QueryServer) {
+        // Assert that our break glass accounts are exempt from this processing.
+        let mut server_txn = server.write(duration_from_epoch_now()).await.unwrap();
+
+        let me_inv_m = ModifyEvent::new_internal_invalid(
+            filter!(f_eq(Attribute::Uuid, PVUUID_SYSTEM_CONFIG.clone())),
+            ModifyList::new_list(vec![
+                Modify::Present(Attribute::DeniedName, Value::new_iname("admin")),
+                Modify::Present(Attribute::DeniedName, Value::new_iname("idm_admin")),
+            ]),
+        );
+        assert!(server_txn.modify(&me_inv_m).is_ok());
+        assert!(server_txn.commit().is_ok());
+
+        let mut server_txn = server.write(duration_from_epoch_now()).await.unwrap();
+
+        assert!(server_txn
+            .internal_modify_uuid(
+                UUID_IDM_ADMIN,
+                &ModifyList::new_purge_and_set(
+                    Attribute::DisplayName,
+                    Value::new_utf8s("Idm Admin")
+                )
+            )
+            .is_ok());
+
+        assert!(server_txn
+            .internal_modify_uuid(
+                UUID_ADMIN,
+                &ModifyList::new_purge_and_set(Attribute::DisplayName, Value::new_utf8s("Admin"))
+            )
+            .is_ok());
+
+        assert!(server_txn.commit().is_ok());
     }
 
     #[qs_test]

server/lib/src/server/migrations.rs

@@ -427,7 +427,10 @@ impl QueryServerWriteTransaction<'_> {
         // =========== Apply changes ==============
 
         // Now update schema
-        let idm_schema_changes = [SCHEMA_CLASS_DOMAIN_INFO_DL10.clone().into()];
+        let idm_schema_changes = [
+            SCHEMA_ATTR_DENIED_NAME_DL10.clone().into(),
+            SCHEMA_CLASS_DOMAIN_INFO_DL10.clone().into(),
+        ];
 
         idm_schema_changes
             .into_iter()