THREESCALE-15279: Add an "audience" protocol mapper to Keycloak integration#597
THREESCALE-15279: Add an "audience" protocol mapper to Keycloak integration#597jlledom wants to merge 3 commits into
Conversation
Compatibility & Correctness ReviewThe code change itself is clean and correct for the target use case (RHBK 26.6.2+). The However, there are two operational risks worth discussing before merge: 1. "Allowed Protocol Mapper Types" policy will reject clients with 403By default, Keycloak's Client Registration Policies do not whitelist The PR description documents this as verification step 1, but it's actually a hard prerequisite — not a verification step. This should be surfaced prominently in release notes or a migration guide. Ideally, Zync would detect the 403 and log a clear message pointing at the policy configuration. 2. Potential overwrite of existing protocol mappersPreviously, Worth verifying this behavior against the target Keycloak version, or consider reading the existing client's mappers and merging. Test coverage gaps
SuggestionConsider making the mapper configurable via |
|
Hi bot that has nothing to do with @akostadinov.
This is true. But the whole Jira issue is about adding this audience mapping. Right now, preparing an RHBK or Keycloak instance to be integrated with zync is a more complicated process, it requires creating a client there with the proper scopes and permissions to manage other clients in the realm. The users can do this because it's probably documented in some of our guides, the only thing we need is to tell Docs people to add a new step in that manual to add the audience mapper to the whitelist. And of course write some release notes.
This is also true, but it's true also in
I added such test: ddf7f91
I don't think we need a test for this because we don't do anything special in this scenario. |
I think that the bot was referring to the fact that if one upgrades 3scale but does not update RHBK, they will need to execute a manual step or the integration will break. And these manual upgrade steps are often missed. The concern is not new installations. So is your suggestion to rely on release/upgrade notes?
Have you verified that? Because the poor bot thought it didn't override default mappers on PUT previously.
I can't follow how overriding default mappers relates to touching clients manually. |
Yeah that's true.
Do you have any other idea? either for new installations or for upgrades, they have guides and manuals, right?
Yes
Because a mapper is attached to a client, and clients are managed entirely by Zync. Zync doesn't add any mapper before this PR, so the only way for a client to have a mapper is if somebody actually went to the RHBK admin console, looked for the client and added the mapper to it manually. In that case, the next push from zync would remove it, also in current master |
|
Sounds good then. We just need to be very loud in release notes about upgrade manual steps. |
akostadinov
left a comment
There was a problem hiding this comment.
Looks good. I would consider the ability to turn this off though for customers on older versions and semi-compatible versions. Just to be sure. But only consider, leaving it on your judgement whether this will provide possible value or will only be unnecessary cruft.
|
@akostadinov I tried to satisfy you and your bot: 1dca454 Now you can opt-out by setting |
|
If we upgrade SaaS without ability to turn this on/off for individual accounts, then many SaaS customers will break. And it turned out very hard to get hold on customers to update their configs. So I think as it presently is implemented, it appears to me that we will have troubles with SaaS. |
Add the ZYNC_CLIENT_AUDIENCE_MAPPER_DISABLED environment variable and document the RHBK admin prerequisite: the oidc-audience-mapper type must be added to Allowed Protocol Mapper Types in Client Registration Policies before enabling the mapper. The kill switch allows operators to disable the mapper instance-wide if needed. Assisted-by: Claude Code
Add an oidc-audience-mapper protocol mapper to the Keycloak client registration payload when audience_mapper_client_id is provided. RHBK 26.6.2+ rejects token introspection when the calling client is not listed in the token's aud claim. Porta will send the introspection- calling client's ID as audience_mapper_client_id; Zync sets that as included.client.audience in the mapper. A global kill switch (client_audience_mapper_disabled) allows disabling it instance-wide. Assisted-by: Claude Code
Update service and integration tests to verify the audience mapper appears in Keycloak API requests when audience_mapper_client_id is present in the entry data. Adds assert_requested assertions that were missing, and updates Keycloak request body stubs to include the protocolMappers field. Assisted-by: Claude Code
13e7bcc to
0764f7a
Compare
Jira Issue:
https://redhat.atlassian.net/browse/THREESCALE-15279
Description:
As the issue describes, after a breaking change in a recent RHBK release, the token introspection feature stopped working. In order to fix it, we need to ensure the access token received from keycloak includes the client id in the
audfield.Keycloak provides a way to write on this field, which is installing a protocol mapper type
oidc-audiende-mapperin all clients.This PR modifies the Keycloak integration in Zync to add such mapper in the requests to the client registration service in Keycloak.
Verification steps:
oidc-audience-mapperfor client registrationoidc-audience-mapperto theAllowed Protocol Mapper Typespolicy for bothAnonymousandAuthenticatedclients{"active": false}then it's not working.