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

OIDConfig.getLogoutRedirectUrl: use redirectURL when possible #1385

Merged

Conversation

jonathanperret
Copy link
Collaborator

Context

In #1276 we fixed the issue that OIDC logouts failed because of Grist using a variable post-logout URI.

Looking at the code again, we realized that when the user had disabled OIDC logouts by setting GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT to true, Grist could actually use the requested post-logout URI.

Proposed solution

When GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT is set to true, use the provided URL as the post-logout URI.

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

Copy link
Collaborator

@fflorent fflorent left a comment

Choose a reason for hiding this comment

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

LGTM. Worth noting: I was with Jonathan when he suggested this change.

When GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT is true, we can
actually use the requested destination URL since there is no
constraint imposed by the OIDC provider.

Co-authored-by: fflorent <[email protected]>
@jonathanperret jonathanperret force-pushed the oidc-use-redirecturl-if-possible branch from 5350e6f to 7ef389b Compare January 16, 2025 16:33
@georgegevoian georgegevoian self-requested a review January 21, 2025 07:10
Copy link
Contributor

@georgegevoian georgegevoian left a comment

Choose a reason for hiding this comment

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

Thanks @jonathanperret.

@georgegevoian georgegevoian merged commit 93cce85 into gristlabs:main Jan 21, 2025
12 checks passed
@jonathanperret jonathanperret deleted the oidc-use-redirecturl-if-possible branch January 21, 2025 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants