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 query-safe url fragment when returning an error #1733

Merged
merged 3 commits into from
Jan 24, 2025
Merged

Conversation

bhazen
Copy link
Contributor

@bhazen bhazen commented Jan 24, 2025

This is a PR based off the work done in PR 1670, but with test coverage added.

From the original author of the original PR:
This PR adjusts the fragment that is added to error redirect responses, which is supposed to prevent browsers from reattaching existing ones.

The previous fragment #_=_ is an invalid query selector and can not be used as is on the receiving end, hence it has been adjusted to the recommended #_.

Additionally, I adjusted the link to the respective latest active internet draft.

This looks like a minor thing, but one can run into issues caused by the browsers behavior of carrying along fragments.

@bhazen bhazen requested a review from josephdecock January 24, 2025 21:17
Copy link
Member

@josephdecock josephdecock left a comment

Choose a reason for hiding this comment

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

Nice work!

@bhazen bhazen merged commit 283aeae into main Jan 24, 2025
6 checks passed
@bhazen bhazen deleted the beh/update-fragment branch January 24, 2025 21:30
@josephdecock josephdecock added the area/identity-server Related to Identity Server label Jan 28, 2025
@josephdecock josephdecock added this to the is-7.2.0 milestone Jan 28, 2025
@josephdecock josephdecock changed the title fix(error-redirect): use query-safe fragment as recommended in the latest OAuth 2 security best practices Use query-safe url fragment when returning an error Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/identity-server Related to Identity Server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants