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

Bugfix: Broken re-login after logout fails #8694

Merged
merged 1 commit into from
Mar 24, 2023

Conversation

fschade
Copy link
Contributor

@fschade fschade commented Mar 23, 2023

Description

After a user logged out, it was no longer possible to re-login without reloading the ocis root domain,
this has now been fixed and only whitelisted query-params are taken into account in the logout url-replace.

How Has This Been Tested?

  • unit test
  • manual testing

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised

@fschade fschade added Type:Bug Something isn't working Status:Needs-Review Needs review from a maintainer Category:Defect Existing functionality is not working as expected labels Mar 23, 2023
@fschade fschade self-assigned this Mar 23, 2023
@fschade fschade requested a review from JammingBen March 23, 2023 16:49
@fschade fschade marked this pull request as draft March 23, 2023 16:58
@fschade
Copy link
Contributor Author

fschade commented Mar 23, 2023

i found another case where it happens, need to fix that too.

@fschade fschade marked this pull request as ready for review March 23, 2023 18:05
…after logout or in the buildUrl helper, allowed params have to be whitelisted now.
@fschade fschade force-pushed the bugfix-oidc-logout-query-param-replication branch from 7f35d0d to 2098a5d Compare March 23, 2023 18:09
@fschade fschade requested a review from michaelstingl March 23, 2023 18:11
@sonarcloud
Copy link

sonarcloud bot commented Mar 23, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@JammingBen JammingBen left a comment

Choose a reason for hiding this comment

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

LGTM and works just fine. Thanks for fixing! 🤗

@kulmann
Copy link
Member

kulmann commented Mar 24, 2023

Out of curiosity, @fschade was it one specific query param that was breaking the login? Because apparently the details query param doesn't do any harm.

@fschade
Copy link
Contributor Author

fschade commented Mar 24, 2023

Out of curiosity, @fschade was it one specific query param that was breaking the login? Because apparently the details query param doesn't do any harm.

yes, the state (oidc), we had this multiple times in the query-param ... but there multiple potential other params which shouldnt be there (scope , session_state, code_challenge, code_challenge_method, flow, ....)

@kulmann kulmann merged commit fc59006 into master Mar 24, 2023
@delete-merged-branch delete-merged-branch bot deleted the bugfix-oidc-logout-query-param-replication branch March 24, 2023 13:17
@micbar micbar mentioned this pull request May 3, 2023
89 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category:Defect Existing functionality is not working as expected Status:Needs-Review Needs review from a maintainer Type:Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants