Merge pull request #2352 from everettraven/bugfix/oidc-username-claim-required

CNTRLPLANE-337: authentication: oidc: make username claim mapping required

Author: openshift-merge-bot[bot]

config/v1/tests/authentications.config.openshift.io/ExternalOIDC.yaml

@@ -2,7 +2,7 @@ apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if w
 name: "Authentication"
 crdName: authentications.config.openshift.io
 featureGates:
-- ExternalOIDC
+  - ExternalOIDC
 tests:
   onCreate:
     - name: Should be able to create a minimal Authentication
@@ -121,6 +121,64 @@ tests:
               username:
                 claim: "preferred_username"
                 prefixPolicy: NoPrefix
+    - name: Cannot set OIDC providers with no claim mappings
+      initial: |
+        apiVersion: config.openshift.io/v1 
+        kind: Authentication
+        spec:
+          type: OIDC
+          oidcProviders:
+          - name: myoidc
+            issuer:
+              issuerURL: https://meh.tld
+              audiences: ['openshift-aud']
+      expectedError: "Required value"
+    - name: Cannot set OIDC providers with no username claim mapping
+      initial: |
+        apiVersion: config.openshift.io/v1
+        kind: Authentication
+        spec:
+          type: OIDC
+          oidcProviders:
+          - name: myoidc
+            issuer:
+              issuerURL: https://meh.tld
+              audiences: ['openshift-aud']
+            claimMappings:
+              groups:
+                claim: roles
+      expectedError: "Required value"
+    - name: Can set OIDC providers with empty group claim mapping
+      initial: |
+        apiVersion: config.openshift.io/v1
+        kind: Authentication
+        spec:
+          type: OIDC
+          oidcProviders:
+          - name: myoidc
+            issuer:
+              issuerURL: https://meh.tld
+              audiences: ['openshift-aud']
+            claimMappings:
+              username:
+                claim: "username"
+              groups:
+                claim: ""
+      expected: |
+        apiVersion: config.openshift.io/v1
+        kind: Authentication
+        spec:
+          type: OIDC
+          oidcProviders:
+          - name: myoidc
+            issuer:
+              issuerURL: https://meh.tld
+              audiences: ['openshift-aud']
+            claimMappings:
+              username:
+                claim: "username"
+              groups:
+                claim: ""
   onUpdate:
     - name: Updating OIDC provider with a client that's not in the status
       initial: &initConfig |
@@ -133,6 +191,9 @@ tests:
             issuer:
               issuerURL: https://meh.tld
               audiences: ['openshift-aud']
+            claimMappings:
+              username:
+                claim: username
             oidcClients:
               - componentNamespace: namespace
                 componentName: preexisting
@@ -158,6 +219,9 @@ tests:
             issuer:
               issuerURL: https://meh.tld
               audiences: ['openshift-aud']
+            claimMappings:
+              username:
+                claim: username
             oidcClients:
             - componentNamespace: namespace
               componentName: preexisting
@@ -189,6 +253,9 @@ tests:
             issuer:
               issuerURL: https://meh.tld
               audiences: ['openshift-aud']
+            claimMappings:
+              username:
+                claim: username
             oidcClients:
             - componentNamespace: dif-namespace
               componentName: tehName
@@ -214,6 +281,9 @@ tests:
             issuer:
               issuerURL: https://meh.tld
               audiences: ['openshift-aud']
+            claimMappings:
+              username:
+                claim: username
             oidcClients:
               - componentNamespace: namespace
                 componentName: preexisting
@@ -239,6 +309,9 @@ tests:
             issuer:
               issuerURL: https://meh.tld
               audiences: ['openshift-aud']
+            claimMappings:
+              username:
+                claim: username
             oidcClients:
               - componentNamespace: namespace
                 componentName: preexisting
@@ -265,6 +338,9 @@ tests:
             issuer:
               issuerURL: https://meh.tld
               audiences: ['openshift-aud']
+            claimMappings:
+              username:
+                claim: username
             oidcClients:
               - componentNamespace: namespace
                 componentName: preexisting
@@ -298,3 +374,89 @@ tests:
           - componentNamespace: namespace2
             componentName: name3
       expected: *removeFromStatus
+    - name: Should allow updating other fields if existing username claim mapping is empty string
+      initialCRDPatches:
+        - op: remove
+          path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/oidcProviders/items/properties/claimMappings/properties/username/properties/claim/minLength
+      initial: |
+        apiVersion: config.openshift.io/v1
+        kind: Authentication
+        spec:
+          type: OIDC
+          oidcProviders:
+          - name: myoidc
+            issuer:
+              issuerURL: https://meh.tld
+              audiences: ['openshift-aud']
+            claimMappings:
+              username:
+                claim: ""
+      updated: |
+        apiVersion: config.openshift.io/v1
+        kind: Authentication
+        spec:
+          type: OIDC
+          oidcProviders:
+          - name: myoidc
+            issuer:
+              issuerURL: https://huh.tld
+              audiences: ['openshift-aud']
+            claimMappings:
+              username:
+                claim: ""
+      expected: |
+        apiVersion: config.openshift.io/v1
+        kind: Authentication
+        spec:
+          type: OIDC
+          oidcProviders:
+          - name: myoidc
+            issuer:
+              issuerURL: https://huh.tld
+              audiences: ['openshift-aud']
+            claimMappings:
+              username:
+                claim: ""
+    - name: Should allow updating other fields if existing username claim mapping is longer than 256 characters
+      initialCRDPatches:
+        - op: remove
+          path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/oidcProviders/items/properties/claimMappings/properties/username/properties/claim/maxLength
+      initial: |
+        apiVersion: config.openshift.io/v1
+        kind: Authentication
+        spec:
+          type: OIDC
+          oidcProviders:
+          - name: myoidc
+            issuer:
+              issuerURL: https://meh.tld
+              audiences: ['openshift-aud']
+            claimMappings:
+              username:
+                claim: "thisisanincrediblylongclaimnamethatwhileacceptableinjwtsisgenerallyadvisedagainstbecauseitisextremelylongandnoteasilyusablebutmaybethereisausecaseouttherethathasdecidedthattheyneedtousethisextremelylongclaimnameforsomereasoneventhoughtheyreallyshouldreconsiderthis"
+      updated: |
+        apiVersion: config.openshift.io/v1
+        kind: Authentication
+        spec:
+          type: OIDC
+          oidcProviders:
+          - name: myoidc
+            issuer:
+              issuerURL: https://huh.tld
+              audiences: ['openshift-aud']
+            claimMappings:
+              username:
+                claim: "thisisanincrediblylongclaimnamethatwhileacceptableinjwtsisgenerallyadvisedagainstbecauseitisextremelylongandnoteasilyusablebutmaybethereisausecaseouttherethathasdecidedthattheyneedtousethisextremelylongclaimnameforsomereasoneventhoughtheyreallyshouldreconsiderthis"
+      expected: |
+        apiVersion: config.openshift.io/v1
+        kind: Authentication
+        spec:
+          type: OIDC
+          oidcProviders:
+          - name: myoidc
+            issuer:
+              issuerURL: https://huh.tld
+              audiences: ['openshift-aud']
+            claimMappings:
+              username:
+                claim: "thisisanincrediblylongclaimnamethatwhileacceptableinjwtsisgenerallyadvisedagainstbecauseitisextremelylongandnoteasilyusablebutmaybethereisausecaseouttherethathasdecidedthattheyneedtousethisextremelylongclaimnameforsomereasoneventhoughtheyreallyshouldreconsiderthis"