UDS Package CR with URI for the SSO ClientID Fails to Create

[anthonywendt]

Environment

Device and OS: NUC Ubuntu
App version: 0.22.0
Kubernetes distro being used: k3d
Other:

Steps to reproduce

1. Create a Package CR with an sso spec that defines a URI for the clientId like the example below.

spec:
  sso:
    - name: Nexus Login
      clientId: https://nexus
      redirectUris:
        - "https://nexus.{{ .Values.domain }}/saml"
      protocol: saml
      defaultClientScopes:
        - "mapper-saml-email-email"
        - "mapper-saml-username-login"
        - "mapper-saml-firstname-first_name"
        - "mapper-saml-lastname-last_name"
        - "mapper-saml-grouplist-groups"

      attributes:
        saml.client.signature: "false"

Expected result

Client gets created and the package CR gets created and has a Ready status.

Actual Result

Client gets created, but keycloak ends up in a forever retry loop erroring with a jdbc unique index or primary key violation.
The Package CR continually goes from Pending to Failed.

Visual Proof (screenshots, videos, text, etc)

keycloak.log

2024-05-30 23:52:09,869 DEBUG [org.hibernate.engine.jdbc.spi.SqlExceptionHelper] (executor-thread-1) could not execute statement [insert into CLIENT (ALWAYS_DISPLAY_IN_CONSOLE,BASE_URL,BEARER_ONLY,CLIENT_AUTHENTICATOR_TYPE,CLIENT_ID,CONSENT_REQUIRED,DESCRIPTION,DIRECT_ACCESS_GRANTS_ENABLED,ENABLED,FRONTCHANNEL_LOGOUT,FULL_SCOPE_ALLOWED,IMPLICIT_FLOW_ENABLED,MANAGEMENT_URL,NAME,NODE_REREG_TIMEOUT,NOT_BEFORE,PROTOCOL,PUBLIC_CLIENT,REALM_ID,REGISTRATION_TOKEN,ROOT_URL,SECRET,SERVICE_ACCOUNTS_ENABLED,STANDARD_FLOW_ENABLED,SURROGATE_AUTH_REQUIRED,ID) values (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)]: org.h2.jdbc.JdbcSQLIntegrityConstraintViolationException: Unique index or primary key violation: "PUBLIC.UK_B71CJLBENV945RB6GCON438AT_INDEX_3 ON PUBLIC.CLIENT(REALM_ID NULLS FIRST, CLIENT_ID NULLS FIRST) VALUES ( /* key:14 */ 'uds', 'https://nexus')"; SQL statement:
insert into CLIENT (ALWAYS_DISPLAY_IN_CONSOLE,BASE_URL,BEARER_ONLY,CLIENT_AUTHENTICATOR_TYPE,CLIENT_ID,CONSENT_REQUIRED,DESCRIPTION,DIRECT_ACCESS_GRANTS_ENABLED,ENABLED,FRONTCHANNEL_LOGOUT,FULL_SCOPE_ALLOWED,IMPLICIT_FLOW_ENABLED,MANAGEMENT_URL,NAME,NODE_REREG_TIMEOUT,NOT_BEFORE,PROTOCOL,PUBLIC_CLIENT,REALM_ID,REGISTRATION_TOKEN,ROOT_URL,SECRET,SERVICE_ACCOUNTS_ENABLED,STANDARD_FLOW_ENABLED,SURROGATE_AUTH_REQUIRED,ID) values (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?) [23505-224]
	at org.h2.message.DbException.getJdbcSQLException(DbException.java:520)
	at org.h2.message.DbException.getJdbcSQLException(DbException.java:489)
	at org.h2.message.DbException.get(DbException.java:223)
	...

2024-05-30 23:52:09,869 WARN  [org.hibernate.engine.jdbc.spi.SqlExceptionHelper] (executor-thread-1) SQL Error: 23505, SQLState: 23505
2024-05-30 23:52:09,869 ERROR [org.hibernate.engine.jdbc.spi.SqlExceptionHelper] (executor-thread-1) Unique index or primary key violation: "PUBLIC.UK_B71CJLBENV945RB6GCON438AT_INDEX_3 ON PUBLIC.CLIENT(REALM_ID NULLS FIRST, CLIENT_ID NULLS FIRST) VALUES ( /* key:14 */ 'uds', 'https://nexus')"; SQL statement:
insert into CLIENT (ALWAYS_DISPLAY_IN_CONSOLE,BASE_URL,BEARER_ONLY,CLIENT_AUTHENTICATOR_TYPE,CLIENT_ID,CONSENT_REQUIRED,DESCRIPTION,DIRECT_ACCESS_GRANTS_ENABLED,ENABLED,FRONTCHANNEL_LOGOUT,FULL_SCOPE_ALLOWED,IMPLICIT_FLOW_ENABLED,MANAGEMENT_URL,NAME,NODE_REREG_TIMEOUT,NOT_BEFORE,PROTOCOL,PUBLIC_CLIENT,REALM_ID,REGISTRATION_TOKEN,ROOT_URL,SECRET,SERVICE_ACCOUNTS_ENABLED,STANDARD_FLOW_ENABLED,SURROGATE_AUTH_REQUIRED,ID) values (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?) [23505-224]
2024-05-30 23:52:09,869 DEBUG [org.hibernate.engine.transaction.internal.TransactionImpl] (executor-thread-1) On TransactionImpl creation, JpaCompliance#isJpaTransactionComplianceEnabled == false
2024-05-30 23:52:09,869 DEBUG [WebApplicationException] (executor-thread-1) Restarting handler chain for exception exception: org.keycloak.services.ErrorResponseException: HTTP 500 Internal Server Error
	at org.keycloak.services.clientregistration.AbstractClientRegistrationProvider.create(AbstractClientRegistrationProvider.java:119)
	at org.keycloak.services.clientregistration.DefaultClientRegistrationProvider.createDefault(DefaultClientRegistrationProvider.java:52)
        ...

[mjnagel]

Just wanted to update this issue as we identified the root cause here but haven't found a happy path forward.

Currently we use the clientId in constructing a few resources:

• The actual keycloak client: This supports most any name in testing
• The pepr store key where we store the token: This supports most names but / does cause an issue (see pepr issue)
• The secret name for the secret generated with the client details (by default): This is the most restrictive, has character limits + only supports a-z + 0-9 + -?

In all these places we currently just use the client ID without any sanitizing/validating that it is a valid name for those use-cases. We started to go down the route of sanitizing names before using them but determined that for the pepr store case it would be helpful to have this resolved/handled in pepr itself, rather than having to build sanitization, mapping, migrations, etc on our side. Currently waiting on an implementation there - we likely need to discuss with the pepr team what will make the most sense for rolling that out.

[anthonywendt]

Doc for context on Nexus requiring a URI for the entity id. Entity ID == Client ID
https://help.sonatype.com/en/saml-integration.html#:~:text=It%20must%20be%20a%20valid%20URI

[blancharda]

@mjnagel -- thoughts on an (optional) refName (or something) field in the package definition for the client as an offramp for apps with wierd requirements for the clientID name like nexus^?

[mjnagel]

@blancharda there are plans to sanitize these keys on the pepr side so we had held off on implementing anything in our own custom code to account for this scenario. We could consider adding something like this for a separate ref, it just means we would have to track throughout all places we use the token (some of which don't have the full Package spec). Just something for us to consider if we did want to implement this on our own.

[anthonywendt]

Posting another update! After some experimenting and testing it appears that using something like uds:nexus as the clientID and entityID as well as setting the secretName to something else make both UDS operator and Nexus happy without having to do anything else special. Pepr store entry for that does look a little weird but seems to work.

data:
    __pepr_do_not_delete__: k-thx-bye
    uds-core-operator-sso-client-uds:nexus: eyJhbGciOiJIUzUxMi ...

[cmwylie19]

I have the implementation, fuzzing, property-based and unit tests ready to go for this on the Pepr side, need to write a journey test and this one is ready for review. Should be able to get it reviewed on Monday.

[mjnagel]

Moving back into planned work - Pepr update is the primary piece here and then ensuring that our code handles this case well.

[mjnagel]

Blocked on defenseunicorns/pepr#1047