Please create a new Github issue referencing the Jira issue so this feature can be discussed.

This is to close KEYCLOAK-19956

Created #20191

@pedroigor @mposolda could you take a look at this please?

Had a quick look at this and couple notes from me:

• General approach looks sane to me, but not sure about the names/descriptions of the fields. Filter by claim is not directly clear to me that it means require claim to be present. Perhaps it could be renamed to something like Require claim to authenticate and Required claim name / Required claim value?
• We need some tests for this, not sure how we can do that, but I imagine we could extend on testing with IdP brokering with another realm as that would be easier to configure/setup

This looks somewhat related to https://openid.net/specs/openid-connect-core-1_0.html#ClaimsParameter.

However, the claims parameter is OPTIONAL and I'm not sure if OPs are supposed to not authenticate a user if a requested claim (and optionally its value) is not available for a given user.

I think it makes sense to at least make it more aligned with the idea of the claims parameter from the specification. Something like:

* Verify essential claim (ON/OFF. If enabled, ID tokens issued by the OP must have a specific claim. Otherwise, the user can not authenticate through this broker.)

* Essential claim (name)

* Essential claim value (value)

Even though not strictly related to claims support, using a similar terminology should help to understand what it does.

@pedroigor Agree that there is some relation to claims parameter. So if I understand correctly, in this PR you propose to pretty much keep it as it is, but rather "rename" the configuration options and possibly slightly update their tooltips. Is it correct?

@pedroigor Agree that there is some relation to claims parameter. So if I understand correctly, in this PR you propose to pretty much keep it as it is, but rather "rename" the configuration options and possibly slightly update their tooltips. Is it correct?
Glad to align the terminology with these standards, if we find an agreement between the options above:
Require claim to authenticate/Verify essential claim
Required claim name/Essential claim
Required claim value/Essential claim value

I am marking this as "missing tests". We have identity brokering tests in the testsuite in the package org.keycloak.testsuite.broker . Hope you can use some existing tests for the inspiration.

@mposolda while writing the test code I verified that UserRepresentation.setAttributes method has a Map<String, List<String>>, as if for each user attribute we could define multiple values, which I think it's not doable from the Keycloak Admin console. As a result, the OIDC provider receives an ID token claim that is a JSON array and cannot easily compare it with the filter configured in the OIDC configuration.
I can change the provider to look at the actual type of claim value, in this case a java.util.List, and apply the match on any of the contained values, but I'd also like to understand why I saw this behavior only with the test code while the debugging on the development server only returned claim values of String type.

@jonkoops @dmartionol Thanks for the updates, but I am not sure if test failures are just "instability" or if it's regression of this PR? Seems to me that issues in Admin UI E2E (3, Chrome) are regression of this PR (Seeing 7 failures in identity_providers_test.spec.ts in this group). There is also something in Admin UI E2E (1, Chrome), but looks only as single test failure, so might be instability? Could you confirm?

@jonkoops @dmartionol Thanks for the updates, but I am not sure if test failures are just "instability" or if it's regression of this PR? Seems to me that issues in Admin UI E2E (3, Chrome) are regression of this PR (Seeing 7 failures in identity_providers_test.spec.ts in this group). There is also something in Admin UI E2E (1, Chrome), but looks only as single test failure, so might be instability? Could you confirm?

Now users_test.spec.ts is failiing, and I don't think this is affected by the PR, is it? Anyway, I also see random failures on my local env, so I'm not sure what's going on.

@dmartinol I think that those test failures are not caused by your PR, but rather some general instability as I see them also in some other PR #20621 (looks like exactly same test failures)... @jonkoops Could you please confirm?

BTV. It can be helpful if you stop merging this PR with latest Keycloak main (without any changes in your PR) as it just re-triggers the build when the links to the old build would be lost. We can eventually re-trigger your jobs when needed and/or will ask you for rebase/merge.

Unreported flaky test detected

If the below flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.forms.ResetPasswordTest#resetPasswordLink

Keycloak CI - Forms IT (firefox)

java.lang.AssertionError: Expected LoginPage but was Keycloak Account Management (https://localhost:8543/auth/realms/test/account/?session_state=0ea1fa25-e995-42ae-be88-3541f712f207&code=0b3d4ded-0cb7-428d-b58c-f49b7642b48d.0ea1fa25-e995-42ae-be88-3541f712f207.b0fd96e0-d4b5-4610-b98d-14967dfd8bf7)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
	at org.keycloak.testsuite.pages.LoginPage.open(LoginPage.java:236)
...

java.lang.AssertionError: Expected LoginPage but was Keycloak Account Management (https://localhost:8543/auth/realms/test/account/?session_state=59fa5142-1494-4a3a-a92c-9b071739fd92&code=2c140f34-e554-408c-91a6-14d96b5a4254.59fa5142-1494-4a3a-a92c-9b071739fd92.a8b3ed3c-984e-43e8-b323-b4b410dd3436)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
	at org.keycloak.testsuite.pages.LoginPage.open(LoginPage.java:236)
...

Report flaky test

@dmartinol I've updated the branch to latest Keycloak main. That should probably help to fix test failures (They were fixed in the meantime in Keycloak main by PR #21312 ). Hopefully we're finally very close to merge now :-)