Allow overriding OIDC logout endpoint (for IdP with non-standard logout endpoint)

[jyio]

Thank you for adding OIDC support! It works very well.

Now, in this episode, I'm using Grist with Authelia as OIDC IdP. Authelia does not currently support RP-initiated logout, so Grist complains The Identity provider does not propose end_session_endpoint. If that is expected, please set GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT=true. Of course, setting GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT=true makes the error go away, but logging out of Grist does not log out of the IdP. Although Authelia does not fully support RP-initiated logout, it does have a logout endpoint that RPs could redirect to.

Wouldn't it be great if there were a way to specify a non-standard logout endpoint, similar to GRIST_SAML_IDP_LOGOUT or GRIST_FORWARD_AUTH_LOGOUT_PATH? I get that this is the IdP's shortcoming, so you'd be justified in declaring this issue out of your scope. But this could be an opportunity to improve compatibility, since RP-initiated logout is not part of the core OIDC specification. It might be a good idea to investigate other IdPs with non-standard logout endpoints, to determine a general approach that would work with most.

---

That said... let me share a workaround for Authelia 😉
Simply configure your reverse proxy to redirect /o/*/signed-out to https://authelia.example.com/logout or https://authelia.example.com/logout?rd=https://grist.example.com. Here's my Caddy configuration:

grist.example.com {
	redir /o/*/signed-out https://authelia.example.com/logout?rd={scheme}://{host}
	reverse_proxy grist.lan:8484
}

That's not perfect, either, since Authelia might not actually redirect back, but this would have to do for now.

[fflorent]

I would say this is something reasonable to add. @dsagal do you agree?

Would you be keen to add this feature? I feel like it is not hard to implement.

[dsagal]

Yes, I think so. I saw a similar issue earlier at IdentityModel/oidc-client-js#1067.

[fflorent]

Would you be keen to add this feature? I feel like it is not hard to implement.

My above comment was not clear, I wanted to know whether you, @jyio, would like to implement?

I would be glad to help if so.

[jyio]

I'd be happy to, and actually got it working just by modifying app/server/lib/OIDCConfig.ts (not hard to implement, as you hinted). I'd like to run a couple of things by you.

1. What should the environment variable be called? I made it GRIST_OIDC_IDP_END_SESSION_ENDPOINT in reference to the end_session_endpoint parameter, but it could just as easily be GRIST_OIDC_IDP_LOGOUT or something else.
2. What should the config.json key be called? I made it endSessionEndpointOverride because endSessionEndpoint was already used.
3. Which variable takes precedence? In my opinion, it's most logical to have GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT > GRIST_OIDC_IDP_END_SESSION_ENDPOINT > end_session_endpoint (since GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT currently overrides end_session_endpoint).
4. I didn't add any unit tests, since it was not obvious to me how one would test such a thing without actually running a test server and trying to login/logout against an IdP...
5. Documentation would be a separate update to gristlabs/grist-help, right?

Thank you :)

[fflorent]

Thanks @jyio! ❤️

1. What should the environment variable be called? I made it GRIST_OIDC_IDP_END_SESSION_ENDPOINT in reference to the end_session_endpoint parameter, but it could just as easily be GRIST_OIDC_IDP_LOGOUT or something else.

IMHO GRIST_OIDC_IDP_END_SESSION_ENDPOINT, as it would be consistent with the GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT variable and we would be consistent with the OIDC wording.

2. What should the config.json key be called? I made it endSessionEndpointOverride because endSessionEndpoint was already used.

I don't understand this question (especially the config.json part), but probably I need to read anew the issue and maybe the code to understand. I'll come back later on this one!

3. Which variable takes precedence? In my opinion, it's most logical to have GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT > GRIST_OIDC_IDP_END_SESSION_ENDPOINT > end_session_endpoint (since GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT currently overrides end_session_endpoint).

I agree with your proposal

4. I didn't add any unit tests, since it was not obvious to me how one would test such a thing without actually running a test server and trying to login/logout against an IdP...

There is currently no unit test for this module. The more I think of this, the more I feel like it lacks some.
I consider adding some, I'll keep you informed if/when I add them. Until then, you can go on as if there is none if you're OK.

5. Documentation would be a separate update to gristlabs/grist-help, right?

Exactly ✔️ !

[jyio]

Thanks for your input! PR is forthcoming. Documentation would come after.

Aha, I wasn't sure about the config.json either and might've confabulated it. This is what I mean:

    this._skipEndSessionEndpoint = section.flag('endSessionEndpoint').readBool({
      envVar: 'GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT',
      defaultValue: false,
    })!;

I see that readX({ envVar }) gets the environment variable, but flag(Y) seems to get-or-create a section in some other configuration source. Is that how it works? The question is what to substitute for Y 😃

[fflorent]

Huh, I wrote this, my bad 😶!

Here:

grist-core/app/server/lib/OIDCConfig.ts

Lines 97 to 100 in a3161f6

	this._skipEndSessionEndpoint = section.flag('endSessionEndpoint').readBool({
	envVar: 'GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT',
	defaultValue: false,
	})!;

I'd suggest to change flag('endSessionEndpoint') to flag('skipEndSessionEndpoint'), my guess is that it won't have any side-effect (I think that currently the flags can have free forms).

Once this fix done, just call section.flag('endSessionEndpoint') as I think you intended.

[jyio]

Now I'm adding to the help file, but did not touch the Auth0 example because I'm unsure about something and don't have an Auth0 instance to check for myself --

Does Auth0 conform with RP-Initiated Logout or not? Their documentation says yes, but I could not reproduce their example

curl -X GET https://acme.eu.auth0.com/.well-known/openid-configuration
{
  "issuer": "https://acme.eu.auth0.com/",
  "authorization_endpoint": "https://acme.eu.auth0.com/authorize",
  ...
  "end_session_endpoint": "https://acme.eu.auth0.com/oidc/logout"
}

If Auth0 did actually conform with RP-Initiated Logout, then there should be no need to change the example. If not, then I'd expect the example config to elicit a complaint from Grist about end_session_endpoint being missing.

[dsagal]

I don't either, but the documentation is clear that this is how it should work. Too bad that their own example isn't working, but reading the note "Once you have contacted Auth0 Support to enable endpoint discovery..." it's probably the case that it needs to be enabled, at least on existing Auth0 accounts.

In short, it seems to me OK to leave the example as is. For new Auth0 accounts it should work fine; for those without the endpoint enabled, people can either contact Auth0 support to enable it, or use the new env var, and both ways can be found by Googling.

Thanks for looking into it!

[fflorent]

That's indeed what I observed. end_session_endpoint is not provided in their /openid-configuration endpoint.

AFAICT, we have 3 possibilities to logout with Auth0:

• set GRIST_OIDC_IDP_SKIP_END_SESSION_ENDPOINT=true, which is a shame (while having logged out on Grist, the user will remain authenticated on Auth0...);
• contact Auth0 support as mentionned @dsagal;
• set GRIST_OIDC_IDP_END_SESSION_ENDPOINT="https://{subdomain}.auth0.com/oidc/logout?post_logout_redirect_uri={encodedLogoutUrl}&client_id={clientId}" (replacing {subdomain} and {clientId} by the relevant values and {encodedLogoutUrl} by the url-encoded value for the logout URL), and configure the post_logout_redirect_uri in Auth0 as explained in the second point here: https://auth0.com/docs/authenticate/login/logout/log-users-out-of-auth0#redirect-users-after-logout

[jyio]

Thank you for the sanity check. Alright, I have faith in the Auth0 administrator's ability to figure this out. So it seems this little project is complete. Thanks for your guidance @dsagal and @fflorent 😉

[fflorent]

Thank you for implementing this feature and for the documentation @jyio!