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

Use getClientId instead of getName for getting clients' name #136

Merged
merged 2 commits into from
Jan 25, 2023

Conversation

danifr
Copy link
Contributor

@danifr danifr commented Jan 24, 2023

Hi!

When looking at the logs messages:

Jan 24 09:53:41 auth.bbp.epfl.ch keycloak[2359967]: 2023-01-24 09:53:41,858 WARN [de.sventorben.keycloak.authorization.client.access.role.ClientRoleBasedAccessProvider] (executor-thread-8) Access for user 'danielfr' to client '' in realm 'BBP' is denied. User does not have client role 'restricted-access' on client with id 'b5c85eb7-9a94-4058-b483-a2e7f29f995e'.

The client name seems to be empty.

The current version of the authenticator uses getName() to get the clients' name, but this attribute is optional in Keycloak.
I'd suggest changing it by getClientId() (https://github.com/keycloak/keycloak/blob/main/server-spi/src/main/java/org/keycloak/models/ClientModel.java#L112) that returns the mandatory "client ID" attribute as defined by the user.

@sventorben sventorben self-assigned this Jan 24, 2023
@sventorben sventorben added the enhancement New feature or request label Jan 24, 2023
@sventorben
Copy link
Owner

Thanks @danifr
Happy to merge once the checks are green.

'Name' field is optional whereas 'client ID' is mandatory
@sventorben sventorben merged commit 8cf3879 into sventorben:main Jan 25, 2023
@sventorben
Copy link
Owner

Thank you!

@danifr
Copy link
Contributor Author

danifr commented Feb 6, 2023

Hi Sven-Torben,

do you think you can release a new version of this package (v.20.0.X or the version number you consider) including this patch and the other changes that landed in main?

Thank you very much!

@sventorben
Copy link
Owner

Sure! It's 20.0.1
https://github.com/sventorben/keycloak-restrict-client-auth/releases/tag/v20.0.1

@sventorben sventorben added the ignore-for-release Not included in release notes label Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ignore-for-release Not included in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants