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

Grist doesn't respect changes in FORWARD_AUTH_HEADER when doing SSO #207

Open
helmut72 opened this issue May 27, 2022 · 9 comments
Open
Labels
bug Something isn't working

Comments

@helmut72
Copy link

helmut72 commented May 27, 2022

Based on following how-tos:
https://community.getgrist.com/t/a-template-for-self-hosting-grist-with-traefik-and-docker-compose/856
https://community.getgrist.com/t/grist-authelia-custom-logout-path/967

... I have setup Grist with Caddy as Reverse Proxy and Authelia for authentication. But I think there is an error on Grist when it comes to single sign on.

For showing the problem I have installed 2 different whoami, whoami.example.com and whoami2.example.com. Both secured with Authelia. Also Grist is secured.

Logout will be catched on all 3 apps by Caddy when calling /signed-out:

1. open in a tab whoami.example.com, login with Authelia
   Result: Remote-Email: myuser@example.com, Cookie: 123

2. open in a tab whoami2.example.com
   Result: Remote-Email: myuser@example.com, Cookie: 123

3. open in a tab grist.example.com, press sign-in
   Result: signed in automatically, Auth[GET]: id 6 email myuser@example.com, Cookie: 123

4. open whoami.example.com/signed-out
   Result: Authelia login page returns to both whoami and whoami2
           on grist Auth[GET]: id 6 email myuser@example.com, Cookie: 123, stil logged in!!

5. login to whoami.example.com with anotheruser
   Result: whoami and whoami2 are logged in with Remote-Email: anotheruser@example.com, Cookie: 789
           on grist still Auth[GET]: id 6 email myuser@example.com, but Cookie is: 789

Everything works fine as long as I only use Grist for Login/Logout. When I login on another Webapp first, Grist doesn't respect the new login from another user.

@paulfitz paulfitz added the bug Something isn't working label Jun 1, 2022
@paulfitz
Copy link
Member

paulfitz commented Jun 1, 2022

Thanks for spelling this out @helmut72. It makes sense, the ForwardAuthLogin mechanism as implemented doesn't play well with peer sites. I think the fix could be fairly straightforward.

@helmut72
Copy link
Author

helmut72 commented Jun 2, 2022

Thank you. That would be great!

@paulfitz
Copy link
Member

Hi @helmut72, a recent commit cleaned up behavior and gave some more options 561d969. Should be included in the latest grist-core docker image now. I'd be interested if it helps with your situation.

@helmut72
Copy link
Author

My intention no to use authentication on all paths was the feature to share some tables with anonymous users. Using authentication on all paths even works great with 0.7.9. Now with the latest docker image and to have shared links, I need to set GRIST_IGNORE_SESSION=false, right?

When it's set to false, nothing really changes. The old user is still logged in. I need to logout from Grist after Step 5. In the meantime another user is logged in on whoami.example.com and whoami2.example.com.

One other question. What do you mean with GRIST_PROXY_AUTH_HEADER? Do I also need to set this variable or should I leave it empty?

Thank you!

@paulfitz
Copy link
Member

GRIST_IGNORE_SESSION will default to false, no need to set it.

That's too bad, in that case the updates won't help you. Yes, if you do need to set a session on Grist, then you are going to have a user on Grist that could be inconsistent with the user elsewhere. There's no signal I know of that would let Grist know to remove/update it (since in your situation you're specifically omitting to pass on the auth header). Is it possible to ask Caddy to hit an endpoint to let Grist know it should remove the user?

I wonder if it would be useful to have a distinct url available for sharing with anonymous users. Something with a common prefix that can be easily excluded from auth by reverse proxies.

The GRIST_PROXY_AUTH_HEADER environment variable is something that already existed, from a user contributed feature https://github.com/gristlabs/grist-core/pull/165/files. You don't need it when using forward auth.

@helmut72
Copy link
Author

I wonder if it would be useful to have a distinct url available for sharing with anonymous users. Something with a common prefix that can be easily excluded from auth by reverse proxies.

An own prefix is common for API access or shared links (for example Nextcloud). This would solve the problem.

Grist works perfect when the reverse proxy handles session management if authentication is on all paths, but then I lost sharing with others.

@helmut72
Copy link
Author

helmut72 commented Jul 23, 2022

Any update?

My intention is dropping SAML/Keycloak for Authelia and using sharing also with header auth, because it would be cool to integrate Grist tables to Outline like it's possible with Airtable:
https://www.getoutline.com

Tables are better placed into something like Grist and Text is better placed into something like Outline.

Thank you.

@paulfitz
Copy link
Member

No update, sorry. One thing I did want to mention is a currently undocumented feature where you can assign a custom id to a document, like https://templates.getgrist.com/doc/afterschool-program - see how the url is /doc/ instead of /GeNeRatED-Id/? You can set this kind of id via the api, using https://support.getgrist.com/api/#tag/docs/paths/~1docs~1{docId}/patch and supplying a urlId value. You could choose to do this with docs you want to share, and leave the /doc prefix open for anonymous access.

Otherwise, I think implementing the feature you're looking for would mostly involve adding a new endpoint family here https://github.com/gristlabs/grist-core/blob/main/app/server/lib/AppEndpoint.ts#L301-L303 and tweaking docHandler to treat docs that are shared with the public as having a distinct canonical url. There might need to be some refactoring throughout the app to make sure that any code issuing urls for a doc is aware of its current sharing status.

@helmut72
Copy link
Author

helmut72 commented Jul 30, 2022

One thing I did want to mention is a currently undocumented feature

Thanks! This is enough for personal use, because I'm being able to include a tables/URLs in Markdown documents and no one need an account on my Grist installation to view/open the table.

There might need to be some refactoring throughout the app

I understand that this takes a longer time, also needs testing. Will ask in some months again. ;-) Thanks for sharing this undocumented feature. Unfortunately I'm busy these days and need to test it, but this should help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants