Update OidcWireMock to include the client_id in the ID token audience dynamically

[douglas444]

As mentioned in this discussion, Quarkus now enforces that the ID token aud is set to the client id and because the OidcWiremock code was created earlier, the ID token granted sets the aud to https://id.server.example.com. Because of that, tests that use OidcWireMockTestResource will not work unless the aud is customized through the quarkus.oidc.token.audience property.

This PR updates the documentation to customize the audience used in the example for code-flow testing with OidcWireMock, making the example functional.

[github-actions]

🎊 PR Preview 744bc75 has been successfully built and deployed to https://quarkus-pr-main-43949-preview.surge.sh/version/main/guides/

• Images of blog posts older than 3 months are not available.
• Newsletters older than 3 months are not available.

[sberyozkin]

Thanks @douglas444, I was thinking about it, and I wonder if we can just fix OidcWiremock instead to follow the OIDC spec requirement for ID token's audience be set to the client id.

I can't find anywhere how to use a POST url form encoded parameter like client_id in the response.

This code must be updated, I've tried:

private void defineCodeFlowAuthorizationMockTokenStub() {
        server.stubFor(post("/auth/realms/quarkus/token")
                .withRequestBody(containing("authorization_code"))
                .willReturn(aResponse()
                        .withHeader("Content-Type", "application/json")
                        .withBody("{\n" +
                                "  \"access_token\": \""
                                + getAccessToken("alice", getAdminRoles()) + "\",\n" +
                                "  \"refresh_token\": \"07e08903-1263-4dd1-9fd1-4a59b0db5283\",\n" +
                                "  \"id_token\": \""
                                + getIdToken("{{request.body.client_id}}", getAdminRoles())
                                + "\"\n" +
                                "}")
                        .withTransformers("response-template")));
    }

replacing alice for a moment with "{{request.body.client_id}}", just to get integration-tests/oidc-wiremock fail with a value like quarkus-web-app returned, instead of alice, the tests are failing all right but "{{request.body.client_id}}" is returned...

Can you experiment please, and try to find the way to refer to the client_id request parameter ?

[douglas444]

@sberyozkin I guess wiremock can't replace the placeholder because when withBody is called, the {{request.body.client_id}} is inside the base64 encoded string. To access the client_id value in this way, we would need to call getIdToken "lazily" somehow, meaning after withBody is called, maybe with handlebars template helpers https://wiremock.org/docs/extensibility/adding-template-helpers/ ...

Anyway, yes, I will experiment and let you know when I find a solution! Thank you for the reply!

[sberyozkin]

Thanks @douglas444, yeah, I forgot that the client id is coming by default in the Basic scheme, while not everyone is using a client post form authentication. Indeed, the only option is likely to be a custom transformer, have a look please

[sberyozkin]

That said, if requiring users setting the client post authentication option to have something like {{request.body.client_id}} pick up the client_id, then this PR can recommend it as an alternative to customizing the audience.

[douglas444]

@sberyozkin With the last commit, OidcWiremock will pick up the client_id and use it as the id token audience if the new quarkus.test.oidc.idtoken.derive-audience property is set to true. For now, this is done only in the Basic scheme, by getting the client_id in the Authorization header through a handlebars template helper.

Does this approach attend? Or maybe you meant "the client post authentication option" to be some flag signalized in the request instead of a new system property?

The failed tests were an attempt to test without hardcoding the expected client_id.

[sberyozkin]

Hi @douglas444

Thanks a lot for figuring out how to get it done dynamically

With the last commit, OidcWiremock will pick up the client_id and use it as the id token audience if the new quarkus.test.oidc.idtoken.derive-audience property is set to true.

We can safely drop this condition (and the added test), because this is what OIDC providers must do anyway, set the aud to the client id when generating id tokens. IMHO supporting variations where non-compliant OIDC providers may set some other aud value for ID tokens is not necessary. Essentially, your update turns OIDC wiremock into a more compliant OIDC provider :-).

So please remove the added test and the new system property and it should all be good, you might need to tweak some oidc wiremock test properties which expect the custom audience, or you can set the token.audience for some tenants to be a list: https://id.server.example.com,quarkus-web-app as some of them may be validating both ID and access tokens

[sberyozkin]

Thanks @douglas444, that should be the best solution that you have come up with as none of the tests is broken

[douglas444]

@sberyozkin I was reading the oidc spec and it accepts other values to be included in the audience beyond the client_id. So there's no need to remove the https://id.server.example.com for now. Good that with this approach OidcWireMock will work with existing code that uses OidcWireMock with quarkus.oidc.token.audience=https://id.server.example.com or with a custom quarkus.test.oidc.idtoken.audience :D

[quarkus-bot]

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 829f5d4.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

[gsmet]

I will let @sberyozkin have a final look but I asked for a small change.

Thanks a lot for this contribution!

[sberyozkin]

Thanks @gsmet @douglas444

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 64446f7.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

[sberyozkin]

@douglas444 By the way, you can customize the authorization endpoint stub to have a better feel for your test code interacting with your provider

[douglas444]

@douglas444 By the way, you can customize the authorization endpoint stub to have a better feel for your test code interacting with your provider

Thanks @sberyozkin . I see. One thing that we can't customize though is the token generation. If the generateJwtToken weren't static, I could extend the class and override it I guess.

For instance I would like the id token to contain an email claim. How would you do this customization? Right now I think the only way is creating a stub that calls my own token generation methods.

Maybe in a future PR we can add a protected instance method, or maybe we could add a property like quarkus.test.oidc.idtoken.claim."claim_name"=${claim_value} , avoiding the necessity of inheritance. What do you think?

[sberyozkin]

@douglas444 Creating an out of the box OidcWiremock solution which works for all the variations and have everything parameterized is probably not possible, which is why users inject OidcWiremock directly into the test code (see CodeFlowAuthorizationTest which you also modified in this PR) and add specific stubs. You can override a token, user info, jwk, authorization and introspection endpoint addresses to point to the custom stub.

Additionally you can try @TestSecurity to run unit like tests: https://quarkus.io/guides/security-oidc-bearer-token-authentication#bearer-token-integration-testing-security-annotation (this is in the bearer token guide but applies to the code flow too)