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

OIDC: Support overriding end_session_endpoint using environment variable GRIST_OIDC_IDP_END_SESSION_ENDPOINT #802

Merged

Conversation

jyio
Copy link
Contributor

@jyio jyio commented Dec 23, 2023

This adds support for overriding end_session_endpoint using environment variable GRIST_OIDC_IDP_END_SESSION_ENDPOINT, enabling single logout using OIDC IdPs with non-standard logout endpoints, resolving #782.

```
/home/runner/work/grist-core/grist-core/app/server/lib/OIDCConfig.ts
Warning:   124:1  warning  This line has a length of 136. Maximum allowed is 120  max-len
```
@dsagal
Copy link
Member

dsagal commented Dec 27, 2023

@fflorent, I can review, and would you be interested in reviewing too?

@fflorent fflorent self-requested a review December 27, 2023 09:41
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.

@jyio Looks good to me, thanks! And works well with Auth0 using GRIST_OIDC_IDP_END_SESSION_ENDPOINT="https://{subdomain}.auth0.com/oidc/logout"

@dsagal Sure! If I remember correctly, PRs for what relates to login methods are reviewed by two persons?

@jyio
Copy link
Contributor Author

jyio commented Jan 2, 2024

Hey, thank you for helping me make this little enhancement a reality. Good to know it works for Auth0 also; we could add that example to the docs later.

Happy new year 🎉

@fflorent
Copy link
Collaborator

fflorent commented Jan 2, 2024

Happy new year to you too @jyio! 🎉

we could add that example to the docs later.

I could just manage to make it work correctly with the redirect_uri in the query params, like: GRIST_OIDC_IDP_END_SESSION_ENDPOINT="https://{subdomain}.auth0.com/oidc/logout?post_logout_redirect_uri={encodedLogoutUrl}&client_id={clientId}".
The documentation should probably mention this paragraph: https://auth0.com/docs/authenticate/login/logout/log-users-out-of-auth0#redirect-users-after-logout

Copy link
Member

@dsagal dsagal left a comment

Choose a reason for hiding this comment

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

Thank you @jyio !

@fflorent
Copy link
Collaborator

fflorent commented Jan 2, 2024

@dsagal I don't understand why, my review is not counted. I also approved this PR 👍.

@paulfitz paulfitz merged commit ba14a1b into gristlabs:main Jan 3, 2024
13 checks passed
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.

4 participants