Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "basic" to client scopes for auto generated client in keycloak devservice #46339

Merged
merged 1 commit into from
Feb 19, 2025

Conversation

stephan-strate
Copy link
Contributor

Following up on the discussion #44053, I have added the basic scope to the auto generated client.

I am unsure about testing, any suggestions?

Copy link

quarkus-bot bot commented Feb 18, 2025

Thanks for your pull request!

Your pull request does not follow our editorial rules. Could you have a look?

  • title should not end up with ellipsis (make sure the title is complete)

This message is automatically generated by a bot.

@stephan-strate stephan-strate changed the title Add "basic" to client scopes for auto generated client in keycloak de… Add "basic" to client scopes for auto generated client in keycloak devservice Feb 18, 2025
@sberyozkin
Copy link
Member

sberyozkin commented Feb 18, 2025

Thanks @stephan-strate, I recall I was trying to get one of the existing test failing, asserting the sub claim is not null, and it was there but it might be related to the actual token grant which is used.

Would it be possible for you to create a reproducer, simple HelloWorld returning a sub claim value and empty application.properties, and test asserting this subject is not null. Maybe the simplest is to copy and paste https://github.com/quarkusio/quarkus-quickstarts/blob/main/security-openid-connect-quickstart, remove everything from application.properties and have a basic test (the existing one already uses KeycloakTestClient) ?

I'd just like to have a clearer understanding under which conditions a sub claim is not included

@stephan-strate
Copy link
Contributor Author

Sure, there you go:
security-openid-connect-quickstart.zip

@sberyozkin sberyozkin self-requested a review February 19, 2025 17:18
Copy link
Member

@sberyozkin sberyozkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @stephan-strate for the reproducer, confirmed, sub is a pretty important claim for the tracing of the unique access token activity, ideally it would've been kept by default.

@sberyozkin
Copy link
Member

There a few tests which involve the client generated by default in the main branch but they are already rather complex. When the default is being generated, some properties likely to be missing that make Keycloak include this claim even without a directly enabled basic scope; adding it won't harm in any case

Copy link

quarkus-bot bot commented Feb 19, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 5e4fc32.

✅ 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 sberyozkin merged commit d213527 into quarkusio:main Feb 19, 2025
26 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.21 - main milestone Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants