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

Avoid using oidc callback URL as post-login URL #1384

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jonathanperret
Copy link
Collaborator

Context

Fixes #1383.

Proposed solution

This PR fixes the referenced issue by adding the OIDC callback route to the list of routes that should not be redirected to after sign-in.

We also changed the names of the related constant and function to make their role hopefully clearer.

Has this been tested?

  • 👍 yes, I added tests to the test suite
  • 💭 no, because this PR is a draft and still needs work
  • 🙅 no, because this is not relevant here
  • 🙋 no, because I need help

Screenshots / Screencasts

grist-oidc-stuck-on-sign-in-fixed.mp4

@@ -97,14 +97,15 @@ export function getWelcomeHomeUrl() {
return _buildUrl('welcome/home').href;
}

const FINAL_PATHS = ['/signed-out', '/account-deleted'];
const PATHS_TO_EXCLUDE_FROM_NEXT = ['/signed-out', '/account-deleted', '/oauth2/callback'];
Copy link
Member

Choose a reason for hiding this comment

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

Putting OIDC-specific logic in the front-end makes me feel a little itchy. Is there any way to quarantine the OIDC behavior to https://OIDCConfig.ts? Via a change to the CALLBACK_URL handler there perhaps?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a very good point. I'll try to see if there's a way to delegate this to login middleware such as OIDC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OIDC: retrying a sign-in from the error page returns to error page even on success
3 participants