Add tests for implicit flow in Token Exchange and delegation parameterized type - #51222
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds missing test coverage for implicit flow behavior around token exchange delegation and parameterized (delegation) scopes.
Changes:
- Adds implicit-flow delegation tests for token exchange and parameterized scopes.
- Extends test-framework builders to support protocol mappers on client scopes and enabling implicit flow on clients.
- Resets OAuth response type after tests to avoid cross-test leakage.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/base/src/test/java/org/keycloak/tests/oauth/tokenexchange/TokenExchangeDelegationTest.java | Adds a new implicit-flow delegation test and resets OAuth response type in teardown. |
| tests/base/src/test/java/org/keycloak/tests/oauth/ParameterizedScopesOAuthGrantTest.java | Adds implicit-flow delegation scope test and a second test user used as delegation target. |
| tests/base/src/test/java/org/keycloak/tests/oauth/ParameterizedScopeBuilder.java | Adds convenience API to attach protocol mappers when building parameterized scopes. |
| test-framework/builders/src/main/java/org/keycloak/testframework/realm/ProtocolMapperBuilder.java | Introduces a builder for protocol mapper representations used in tests. |
| test-framework/builders/src/main/java/org/keycloak/testframework/realm/ClientScopeBuilder.java | Adds support for attaching protocol mappers to client scopes. |
| test-framework/builders/src/main/java/org/keycloak/testframework/realm/ClientBuilder.java | Adds builder support for enabling implicit flow on clients. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
tests/base/src/test/java/org/keycloak/tests/oauth/tokenexchange/TokenExchangeDelegationTest.java:252
- This method is missing
@Test, so JUnit will never discover it and the token-exchange implicit-flow coverage described by this PR is not actually added.
public void delegationImplicit() {
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
tests/base/src/test/java/org/keycloak/tests/oauth/tokenexchange/TokenExchangeDelegationTest.java:252
delegationImplicitis missing@Test, so JUnit will never execute the new implicit-flow coverage added forTokenExchangeDelegationTest. Annotate the method as a test.
public void delegationImplicit() {
…rized type Closes keycloak#51152 Signed-off-by: rmartinc <rmartinc@redhat.com>
| * | ||
| * @author rmartinc | ||
| */ | ||
| public class ProtocolMapperBuilder extends Builder<ProtocolMapperRepresentation> { |
There was a problem hiding this comment.
JFYI - the instantiation of the protocol mappers is quite fragile even in our core logic, and this PR should resolve it and might be used in the testsuite as well if we decide to go that way: #50974
ahus1
left a comment
There was a problem hiding this comment.
Approving based on previous reviews
Closes #51152
Just adding implicit flow tests for TE Delegation and Parameterized scopes. We had no coverage for implicit in the Test Suite.