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

Encode realm name in console URIs #29102

Merged
merged 2 commits into from May 2, 2024
Merged

Encode realm name in console URIs #29102

merged 2 commits into from May 2, 2024

Conversation

rmartinc
Copy link
Contributor

Closes #25807

This probably also closes #26108. I'll take a look but I think that with this the realm is properly checked in name and id.

The PR encodes the URLs generated when a realm is created. It also decodes the issuer to obtain the realm that was previously encoded. Tests added.

Maybe there are more issues about spaces in realms but the idea is managing them one by one. With this PR at least the realm is created and it's functional with the admin console. I'm going to file a new bug for the account console. The v2 one worked OK but v3 is failing.

Before this fix console uris (including the client redirect uris) did not contain the url encoded realm name and therefore were invalid.

Fixes keycloak#25807

Signed-off-by: Philip Sanetra <code@psanetra.de>
@rmartinc rmartinc requested review from a team as code owners April 26, 2024 08:00
This was referenced Apr 26, 2024
@rmartinc
Copy link
Contributor Author

I have fixed also the account v3. It was just a decode in the path of the router. Please @keycloak/ui take a look just in case.

jonkoops
jonkoops previously approved these changes Apr 26, 2024
Copy link
Contributor

@jonkoops jonkoops left a comment

Choose a reason for hiding this comment

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

LGTM, just wondering if the URLs could be done with less repetition.

Closes keycloak#25807

Signed-off-by: rmartinc <rmartinc@redhat.com>
Copy link
Contributor

@jonkoops jonkoops left a comment

Choose a reason for hiding this comment

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

LGTM!

@mposolda mposolda self-assigned this Apr 26, 2024
Copy link
Contributor

@mposolda mposolda left a comment

Choose a reason for hiding this comment

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

@mposolda mposolda merged commit 65bdf1a into keycloak:main May 2, 2024
74 checks passed
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.

Realm improper input sanitization Space in realm name breaks initial console uris
4 participants