grist-core icon indicating copy to clipboard operation
grist-core copied to clipboard

Support for HTTP Basic Auth

Open MHOOO opened this issue 2 years ago • 29 comments

Hello grist-core team!

I've built a small internal app using grist-core and I really love what you've done with this software. At the moment I am talking with our IT department, and they are not very fond of the idea of running an extra SAML service. Presently all authentication runs through a web server which uses an LDAP backend. Now, I'm no web technology expert, but I believe that is essentially HTTP basic auth. I would like to know which steps would be required to patch grist-core to support this. Any ideas?

MHOOO avatar Mar 02 '22 08:03 MHOOO

Related: https://github.com/gristlabs/grist-core/issues/44

dumblob avatar Mar 02 '22 09:03 dumblob

@MHOOO there are a few possible directions to go.

One way is to patch this method: https://github.com/gristlabs/grist-core/blob/99ee495b4be0af7408c662671acb540988e3b624/app/server/lib/Authorizer.ts#L93

It is a piece of middleware wrapped around every request. It has access to the request headers and a database of users, and it has the power to declare that a particular request is from a particular user. This makes sense for auth approaches that "wrap" Grist and set headers in the request.

Another way to go is to implement the GristLoginSystem interface. This makes sense for auth approaches with a separate SSO. There's a trivial example of implementing it here: https://github.com/gristlabs/grist-core/blob/8853e095bb09afdfaa984de271736b6edd36114c/app/server/lib/MinimalLogin.ts#L8 and here it is for SAML: https://github.com/gristlabs/grist-core/blob/8853e095bb09afdfaa984de271736b6edd36114c/app/server/lib/SamlConfig.ts#L243

A different possibility, not quite what you have in mind, would be to bundle a pre-configured SSO supporting SAML (such as Authentik) in the Grist image.

paulfitz avatar Mar 02 '22 14:03 paulfitz

Thanks, I think patching the Authorizer might be the way to go:

  • Look for a particular header (e.g. X-Remote-User or similar)
  • If set, use it to lookup an existing or new user

I'll see if I can get this to work and come back to you with my results

MHOOO avatar Mar 03 '22 10:03 MHOOO

So I've successfully set up apache to provide a x-remote-user header with the users Name. However, I'm sort of stuck with creating a profile. My current approach is very simple. I've basically appended the following:

  if (!mreq.userId) {
    if (mreq.headers && mreq.headers["x-remote-user"]) {
      const remoteUser = mreq.headers["x-remote-user"].toString();
      log.info("Authorized user found");
      profile = {
              "email": remoteUser,
              "name": remoteUser
      };
      const user = await dbManager.getUserByLoginWithRetry(remoteUser, profile);
      log.info(`user: ${user}`);
      if(user) {
        mreq.user = user;
        mreq.userId = user.id;
        mreq.userIsAuthorized = true;
      }
    }
  }

However, I am getting the following error:

2022-03-08 00:09:51.307 - warn: client error stack=Error: Request to http://192.168.1.111/o/docs/api/session/access/all failed with status 401: Unauthorized (user profile not found)
    at throwApiError (http://192.168.1.111/v/unknown/main.bundle.js:54926:11)
    at UserAPIImpl.<anonymous> (http://192.168.1.111/v/unknown/main.bundle.js:54876:21)
    at Generator.next (<anonymous>)
    at fulfilled (http://192.168.1.111/v/unknown/main.bundle.js:54777:58), message=Request to http://192.168.1.111/o/docs/api/session/access/all failed with status 401: Unauthorized (user profile not found), status=401, userError=user profile not found, page=http://192.168.1.111/3nLZ5C3U7q5g/Untitled-document, language=de-DE, platform=Linux x86_64, userAgent=Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/98.0.4758.80 Safari/537.36, org=docs, email=tkarolski, userId=6

It appears I need to create a user profile first. So my question would be:

  1. How do I find out, if a profile already exists?
  2. If it does not, how can I create one?
  3. What exactly is a profile to begin with?

MHOOO avatar Mar 08 '22 00:03 MHOOO

@MHOOO that looks very close. The error you are seeing is coming from getUserProfiles, which is expecting to find a list of potential users in mreq.users. To explain that: Grist has a notion of "team sites", and a single session can be associated with multiple sites, and potentially distinct user ids + names on each site. Most endpoints work with a single user, but a few select endpoints need to be able to look up information for all these users in order e.g. to offer a drop-down to choose between them, or pick the best default one for a site.

For your scenario, this multi-user stuff isn't important at all, so when you set mreq.user, mreq.userId, and mreq.userIsAuthorized, you could also set mreq.users to an array with a single element. The type of that element is UserProfile rather than User. The main difference between the two is that a Profile doesn't need to have a user id, but that's irrelevant to you, you have an id.

Eyeballing the code, I think mreq.users = [profile] should work. There's a later line that'd you'd need to tweak - this one:

mreq.users = getSessionProfiles(session);

You'd need to tweak that to something like this so it doesn't overwrite users if you've already dealt with it:

mreq.users = mreq.users || getSessionProfiles(session);

If you push a fork somewhere I'd be happy to help.

paulfitz avatar Mar 08 '22 04:03 paulfitz

Thanks so far. It definitely works better already, I'll push the changes onto a fork later today. However I appear to be having issues to use grist with a reverse proxy to begin with (without modifications). I'm getting the following error:

2022-03-08 09:50:53.426 - debug: DocManager.openDoc Authorizer key { urlId: 'v7BzXb7mLT8F5muWSxMqyk', userId: 5, org: 'docs' }
2022-03-08 09:50:53.440 - debug: DocManager.fetchDoc v7BzXb7mLT8F5muWSxMqyk
2022-03-08 09:50:53.441 - debug: DocClients now 1 clients; new client is 41d6101bbb7c4669 (fd 0) access=owners, userId=5, [email protected], age=0, org=docs, clientId=41d6101bbb7c4669, counter=161, docId=v7BzXb7mLT8F5muWSxMqyk
2022-03-08 09:50:53.441 - info: ActiveDoc will stay open access=owners, userId=5, [email protected], age=0, org=docs, clientId=41d6101bbb7c4669, counter=161, docId=v7BzXb7mLT8F5muWSxMqyk
2022-03-08 09:50:53.441 - info: ActiveDoc fetchMetaTables access=owners, userId=5, [email protected], age=0, org=docs, clientId=41d6101bbb7c4669, counter=161, docId=v7BzXb7mLT8F5muWSxMqyk
2022-03-08 09:50:53.442 - debug: Time taken in getRecentMinimalActionGroups: 0 ms
2022-03-08 09:50:53.477 - info: Client onMessage '{"reqId":1,"method":"fetchTable","args":[0,"Table1"]}' [email protected], userId=5, age=0, org=docs, clientId=41d6101bbb7c4669, counter=161
2022-03-08 09:50:53.477 - debug: activeDocMethod access=owners, userId=5, [email protected], age=0, org=docs, clientId=41d6101bbb7c4669, counter=161, docId=v7BzXb7mLT8F5muWSxMqyk, docMethod=fetchTable
2022-03-08 09:50:53.477 - info: ActiveDoc fetchQuery {"tableId":"Table1","filters":{}} (regular) access=owners, userId=5, [email protected], age=0, org=docs, clientId=41d6101bbb7c4669, counter=161, docId=v7BzXb7mLT8F5muWSxMqyk
2022-03-08 09:50:53.478 - warn: Client Error SandboxError: [Sandbox] PipeToSandbox is closed
    at NSandbox._sendData (/home/tkarolski/sources/grist-core/_build/app/server/lib/NSandbox.js:175:19)
    at NSandbox.pyCall (/home/tkarolski/sources/grist-core/_build/app/server/lib/NSandbox.js:128:14)
    at ActiveDoc.<anonymous> (/home/tkarolski/sources/grist-core/_build/app/server/lib/ActiveDoc.js:1608:35)
    at Generator.next (<anonymous>)
    at fulfilled (/home/tkarolski/sources/grist-core/_build/app/server/lib/ActiveDoc.js:19:58)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)  [email protected], userId=5, age=0, org=docs, clientId=41d6101bbb7c4669, counter=161
2022-03-08 09:50:53.479 - warn: Client responding to #1 ERROR [Sandbox] PipeToSandbox is closed [email protected], userId=5, age=0, org=docs, clientId=41d6101bbb7c4669, counter=161
2022-03-08 09:50:53.565 - info: mreq.users: [object Object]
2022-03-08 09:50:53.565 - info: profile: [object Object]
2022-03-08 09:50:53.568 - debug: Auth[POST]: id 5 email [email protected] host 192.168.1.111:8485 path /log org docs
2022-03-08 09:50:53.569 - warn: client error stack=Error: [Sandbox] PipeToSandbox is closed
    at Comm._onServerMessage (http://192.168.1.111/v/unknown/main.bundle.js:5364:37)
    at triggerEvents (http://192.168.1.111/v/unknown/main.bundle.js:69345:57)
    at triggerApi (http://192.168.1.111/v/unknown/main.bundle.js:69332:19)
    at eventsApi (http://192.168.1.111/v/unknown/main.bundle.js:69131:16)
    at GristWSConnection.Events.trigger (http://192.168.1.111/v/unknown/main.bundle.js:69322:5)
    at GristWSConnection.onmessage (http://192.168.1.111/v/unknown/main.bundle.js:9623:14), message=[Sandbox] PipeToSandbox is closed, page=http://192.168.1.111/v7BzXb7mLT8F/Untitled-document, language=de-DE, platform=Linux x86_64, userAgent=Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/98.0.4758.80 Safari/537.36, org=docs, [email protected], userId=5
2022-03-08 09:50:53.590 - debug: Comm Client 41d6101bbb7c4669 #161: clientConnect sent successfully

I'm fairly sure I've configured the reverse proxy & websockets correctly. Excerpt:

# ... snip ...
RewriteEngine On

# Pass x-remote-user header
RewriteCond %{REMOTE_USER} ^(.*)$
RewriteRule ^(.*)$ - [E=R_U:%1] [L]
RequestHeader set X-Remote-User %{R_U}e

</Location>

RewriteEngine On
# proxy support for websockets
RewriteCond %{HTTP:Connection} upgrade [NC]
RewriteCond %{HTTP:Upgrade} websocket [NC]
RewriteRule ^/(.*) "ws://192.168.1.111:8485/$1" [P,L]
ProxyRequests off

# Reverse Proxy support
ProxyPass / http://192.168.1.111:8485/
ProxyPassReverse / http://192.168.1.111:8485/
# ... snip ...

Any Idea what might cause above error? Again, no modifications. Just plain grist-core/master trying to edit a newly created document with the default user ([email protected])

MHOOO avatar Mar 08 '22 10:03 MHOOO

Hmm not sure what that error is, are there any earlier errors? The error is showing a problem starting python for formula evaluation, but the error shown in the logs is recording an error shown to user in the browser - I'd have expected earlier error messages (perhaps with more detail) directly from the server. Maybe restart the server to be sure you are capturing the earliest error?

If you rebase against latest main, you'll get a bunch of improvements specifically to streamline reverse proxy use - but none of the issues fixed resonate with what you are seeing. You've definitely done yarn run install:python? That sets up a python virtual environment with everything formula evaluation needs.

paulfitz avatar Mar 08 '22 14:03 paulfitz

Ah, that must be it. buildtools/prepare_python.sh invokes /bin/bash and I'm on nixos, which does not have /bin/bash. I've ignored the error. Sorry about that. I'll patch the uses of /bin/bash in my fork to make use of /usr/bin/env bash and try again later today

MHOOO avatar Mar 08 '22 15:03 MHOOO

Great, look forward to seeing what you come up with!

paulfitz avatar Mar 08 '22 18:03 paulfitz

I've come further. The header is being interpreted correctly, and I can see myself logged in with the email that was passed via x-remote-user. However, when creating a new document (and switching to it), the browser goes into an infinite error loop (apparently it refreshes upon receiving an exception). Server logs (starting with the first error that occurred):

2022-03-08 19:31:45.695 - info: Comm: Got WebSocket connection: /dw/self/v/unknown/o/docs?clientId=0&counter=196&newClient=1&browserSettings=%7B%22timezone%22%3A%22Europe%2FBerlin%22%7D&user=me%40example.com
2022-03-08 19:31:45.698 - info: Comm Client 67825ce347a9bdd1 #196: new client
2022-03-08 19:31:45.698 - info: Comm Client 67825ce347a9bdd1 #196: using session g-2pcwsBt3BwcGgmWXLFsTMw
2022-03-08 19:31:45.700 - debug: Comm Client 67825ce347a9bdd1 #196: sending clientConnect with 0 missed messages
2022-03-08 19:31:45.717 - info: Client onMessage '{"reqId":0,"method":"openDoc","args":["vgvTorZaD3qUnXk7pTH85S","default",null]}' org=docs, clientId=67825ce347a9bdd1, counter=196
2022-03-08 19:31:45.720 - debug: DocManager.openDoc Authorizer key { urlId: 'vgvTorZaD3qUnXk7pTH85S', userId: 1, org: 'docs' }
2022-03-08 19:31:45.733 - warn: Client Error ErrorWithCode: No view access
    at Object.assertAccess (/home/tkarolski/sources/grist-core/_build/app/server/lib/Authorizer.js:425:19)
    at DocManager.<anonymous> (/home/tkarolski/sources/grist-core/_build/app/server/lib/DocManager.js:260:30)
    at Generator.next (<anonymous>)
    at fulfilled (/home/tkarolski/sources/grist-core/_build/app/server/lib/DocManager.js:5:58) {
  code: 'AUTH_NO_VIEW',
  details: [Object]
} AUTH_NO_VIEW userId=1, org=docs, clientId=67825ce347a9bdd1, counter=196
2022-03-08 19:31:45.733 - warn: Client responding to #0 ERROR No view access userId=1, org=docs, clientId=67825ce347a9bdd1, counter=196
2022-03-08 19:31:45.743 - info: Comm cid 67825ce347a9bdd1: onClose
2022-03-08 19:31:45.744 - warn: Comm cid 67825ce347a9bdd1: will discard client in 300 sec
2022-03-08 19:31:45.757 - debug: Authorized user based on 'x-remote-user' header found.
2022-03-08 19:31:45.757 - debug: Authorized user based on 'x-remote-user' header found.
2022-03-08 19:31:45.758 - debug: Authorized user based on 'x-remote-user' header found.
2022-03-08 19:31:45.759 - debug: Auth[GET]: id 7 email [email protected] host 192.168.1.111:8485 path /vgvTorZaD3qU/Untitled-document org docs
2022-03-08 19:31:45.769 - debug: Auth[POST]: id 7 email [email protected] host 192.168.1.111:8485 path /log org docs
2022-03-08 19:31:45.770 - warn: client error stack=Error: No view access
    at Comm._onServerMessage (http://192.168.1.111/v/unknown/main.bundle.js:5364:37)
    at triggerEvents (http://192.168.1.111/v/unknown/main.bundle.js:69345:57)
    at triggerApi (http://192.168.1.111/v/unknown/main.bundle.js:69332:19)
    at eventsApi (http://192.168.1.111/v/unknown/main.bundle.js:69131:16)
    at GristWSConnection.Events.trigger (http://192.168.1.111/v/unknown/main.bundle.js:69322:5)
    at GristWSConnection.onmessage (http://192.168.1.111/v/unknown/main.bundle.js:9623:14), message=No view access, code=AUTH_NO_VIEW, status=403, accessMode=default, page=http://192.168.1.111/vgvTorZaD3qU/Untitled-document, language=de-DE, platform=Linux x86_64, userAgent=Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/99.0.4844.51 Safari/537.36, org=docs, [email protected], userId=7
2022-03-08 19:31:45.777 - debug: Auth[POST]: id 7 email [email protected] host 192.168.1.111:8485 path /log org docs
2022-03-08 19:31:45.778 - warn: client error stack=TypeError: Cannot read properties of null (reading 'delete')
    at Comm._onServerMessage (http://192.168.1.111/v/unknown/main.bundle.js:5384:42)
    at triggerEvents (http://192.168.1.111/v/unknown/main.bundle.js:69345:57)
    at triggerApi (http://192.168.1.111/v/unknown/main.bundle.js:69332:19)
    at eventsApi (http://192.168.1.111/v/unknown/main.bundle.js:69131:16)
    at GristWSConnection.Events.trigger (http://192.168.1.111/v/unknown/main.bundle.js:69322:5)
    at GristWSConnection.onmessage (http://192.168.1.111/v/unknown/main.bundle.js:9623:14), message=Cannot read properties of null (reading 'delete'), page=http://192.168.1.111/vgvTorZaD3qU/Untitled-document, language=de-DE, platform=Linux x86_64, userAgent=Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/99.0.4844.51 Safari/537.36, org=docs, [email protected], userId=7

I'm thinking this might be because the profile that I've passed is really only a dummy struct. Do I maybe need to save it somehow? In any case you can find the meager current state at: https://github.com/gristlabs/grist-core/pull/165/files

MHOOO avatar Mar 08 '22 19:03 MHOOO

I can change the access rights on the document to public, at which point I can view it. However, when attempting to add a row, I get:

2022-03-08 20:03:38.635 - info: Client onMessage '{"reqId":2,"method":"applyUserActions","args":[0,[["AddRecord","Table1",null,{"A... (93 length)' userId=1, org=docs, clientId=c4c9ea17d80ddaa2, counter=231
2022-03-08 20:03:38.636 - warn: Client Error ErrorWithCode: No write access
    at assertAccess (/home/tkarolski/sources/grist-core/_build/app/server/lib/Authorizer.js:454:19)
    at DocAuthorizer.<anonymous> (/home/tkarolski/sources/grist-core/_build/app/server/lib/Authorizer.js:396:13)
    at Generator.next (<anonymous>)
    at fulfilled (/home/tkarolski/sources/grist-core/_build/app/server/lib/Authorizer.js:5:58)
    at processTicksAndRejections (internal/process/task_queues.js:95:5) {
  code: 'AUTH_NO_EDIT',
  details: [Object]
} AUTH_NO_EDIT userId=1, org=docs, clientId=c4c9ea17d80ddaa2, counter=231

MHOOO avatar Mar 08 '22 20:03 MHOOO

Ah, I think there's a wrinkle with the websocket used for documents. The websocket code kind of does its own thing for authentication, the express middleware isn't easy to apply. Sorry about that, I'll take a look.

paulfitz avatar Mar 08 '22 21:03 paulfitz

@MHOOO there is one more place that needs updating, related to websockets, in Comm.js. I made this experiment, hardcoding a [email protected] user just for simplicity:

diff --git a/app/server/lib/Authorizer.ts b/app/server/lib/Authorizer.ts
index 64f2059..10e672a 100644
--- a/app/server/lib/Authorizer.ts
+++ b/app/server/lib/Authorizer.ts
@@ -249,6 +249,7 @@ export async function addRequestUser(dbManager: HomeDBManager, permitStore: IPer
   // TODO: The header should probably be set via an environment variable and if it is not set,
   //       this code path should be disabled altogether.
   if (!mreq.userId) {
+    mreq.headers["x-remote-user"] = "[email protected]";
     if (mreq.headers && mreq.headers["x-remote-user"]) {
       const remoteUser = mreq.headers["x-remote-user"].toString();
       log.debug("Authorized user based on 'x-remote-user' header found.");
diff --git a/app/server/lib/Comm.js b/app/server/lib/Comm.js
index b841ad9..f380aff 100644
--- a/app/server/lib/Comm.js
+++ b/app/server/lib/Comm.js
@@ -150,6 +150,14 @@ Comm.prototype._broadcastMessage = function(type, data, clients) {
   clients.forEach(client => client.sendMessage({type, data}));
 };
 
+Comm.prototype._getSessionProfile = function(scopedSession, req) {
+  return Promise.resolve({
+    "email": "[email protected]",
+    "name": "[email protected]"
+  });
+  return scopedSession.getSessionProfile();
+}
+
 /**
  * Sends a per-doc message to the given client.
  * @param {Object} client - The client object, as passed to all per-doc methods.
@@ -236,7 +244,7 @@ Comm.prototype._onWebSocketConnection = async function(websocket, req) {
   // Delegate message handling to the client
   websocket.on('message', client.onMessage.bind(client));
 
-  scopedSession.getSessionProfile()
+  this._getSessionProfile()
   .then((profile) => {
     log.debug(`Comm ${client}: sending clientConnect with ` +
       `${client._missedMessages.length} missed messages`);

With this change, I was able to open and edit a document. You could try adapting the getSessionProfile to return a good profile when the appropriate header is set. It is awkward that this has to be done in two places...

(For your public rights experiment: I think the websocket is just seeing you as anonymous, so it makes sense that public access gets you into the doc. If you gave the public write access, then you should be able to edit the doc).

paulfitz avatar Mar 08 '22 21:03 paulfitz

Thanks, this worked! Would you want this in your upstream branch? If yes, I would consider refactoring this, though I'm really not sure how to best go about that. Maybe a single object, that would be responsible for:

  • checking if a particular header (i.e. x-remote-user or similar) is set
  • returning a profile based on that header
  • returning a user instance

What do you think?

MHOOO avatar Mar 09 '22 10:03 MHOOO

Glad that works @MHOOO! Yes, we'd be glad to incorporate this, as a form of authentication that can be configured - wouldn't want it on by default since x-remote-user could be spoofed in the general case.

For a refactor, perhaps it would suffice to add a method that scans headers and returns a (partial?) profile or empty based on those headers. I think Grist itself is due a refactor to reconcile auth code for the websocket and the rest of the app, so if I were you I wouldn't wrestle with it too much if it proves awkward - an alternative would be to just add sufficient documentation at each point touched.

paulfitz avatar Mar 09 '22 14:03 paulfitz

Alright, I'll try that and also add an environment variable to configure the feature. Any preferences on where such a method should live?

MHOOO avatar Mar 10 '22 08:03 MHOOO

Alright, I'll try that and also add an environment variable to configure the feature. Any preferences on where such a method should live?

Perhaps in Authorizer.ts? No strong preference.

paulfitz avatar Mar 10 '22 19:03 paulfitz

Alright, I've updated my pull request. Before this, I've never written a single line of typescript, so I hope I did not do anything too obviously stupid (had to read up on the language first).

I've called the environment variable GRIST_PROXY_AUTH_HEADER. Maybe REVERSE_PROXY or RPROXY instead of just PROXY would be more applicable, but I'm not too knowledgeable in the field, so I have no issues if you'd like to change the name. Moreover, not sure if this should be in the GRIST_ namespace.

Are there any additional steps required to get the environment variable into the docker image?

MHOOO avatar Mar 12 '22 19:03 MHOOO

I saw some commits regarding this feature, a changed README and a current Docker image from today, but it looks like the GRIST_PROXY_AUTH_HEADER variable isn't used in the Docker image for now.

helmut72 avatar Mar 16 '22 10:03 helmut72

I'm a complete TS noob (and developer). I've added two lines to get some output:

function getRequestProfile(req) {
    const header = process.env.GRIST_PROXY_AUTH_HEADER;
    let profile;
==>    log.error(`show header: ${header}`);
==>    log.error(`show reqheader: ${req.headers}`);
    if (header && req.headers && req.headers[header]) {
        const headerContent = req.headers[header];

Don't know if syntax and variables are ok. I get this output:

2022-03-16 13:59:45.908 - show header: X-Token-User-Email
2022-03-16 13:59:45.908 - show reqheader: undefined

Looks like GRIST_PROXY_AUTH_HEADER is used, but the headers does have no content. If req.headers[header] stores the content.

helmut72 avatar Mar 16 '22 13:03 helmut72

Is the header being set by your reverse proxy?

MHOOO avatar Mar 16 '22 13:03 MHOOO

Hmm, after pulling the latest docker image I ran:

docker run --env GRIST_PROXY_AUTH_HEADER=trust-me --env DEBUG=1  -p8484:8484 -it gristlabs/grist

and then:

$ curl -H "trust-me: paul@me" http://localhost:8484/api/session/access/active
{"user":{"id":6,"email":"paul@me","name":"paul","picture":null},"org":null}

So it looks to me like if the header is being set, it should be honored.

paulfitz avatar Mar 16 '22 14:03 paulfitz

@helmut72 I think it may be a header name normalization issue. Would it work with all lowercase?

paulfitz avatar Mar 16 '22 17:03 paulfitz

@paulfitz Thanks! Yes, this is the solution.

helmut72 avatar Mar 16 '22 18:03 helmut72

Great - I'm adding tests, and will fix the case issue.

paulfitz avatar Mar 16 '22 18:03 paulfitz

Have played with it a bit. FYI:

  1. open grist.example.com
  2. need to login to my reverse proxy, using "user1"
  3. still need to press "sign in" in Grist
  4. "user1" is logged on in Grist
  5. press "sign out" and "sign in again" in Grist
  6. "[email protected]" is logged on in Grist?
  7. quit the browser with "[email protected]" logged on in Grist
  8. doing steps 1+2
  9. "[email protected]" is logged on in Grist?
  10. press "sign out" in Grist
  11. Grist provides "sign in again", but shows "user1" on right top?
  12. press "sign out" in Grist
  13. quit again the Browser, doing steps 1+2
  14. "user1" is logged on in Grist without need to press "sign in" like in step 3?

helmut72 avatar Mar 16 '22 19:03 helmut72

@helmut72: @MHOOO mentioned that logging out doesn't work https://github.com/gristlabs/grist-core/pull/165#issuecomment-1067078415. I think the next step for someone polishing this would be to change cookie handling when this kind of auth is in use. It might suffice to simply ignore cookies for user authentication. This is handled in the same area as the changes @MHOOO has already made.

paulfitz avatar Mar 19 '22 02:03 paulfitz

Intuitively, sign in and sign out are not needed and do not work as expected because they're outsourced to the reverse proxy (which effectively becomes the IdP). It makes sense to ignore cookies if GRIST_PROXY_AUTH_HEADER is set, and create the profile from headers.

So Grist should probably cooperate with the proxy:

  • If the auth headers were available, they should be used to create/retrieve the profile, and the sign out link should be presented in the upper right
    • The sign out link should point to the proxy's sign out page
  • If the auth headers were not available (the proxy allows anonymous "guest" access), the sign in link should be presented in the upper right
    • The sign in link should point to the proxy's sign in page

Perhaps these could be specified by additional variables, such as:

GRIST_PROXY_AUTH_LOGIN=https://auth.example.com/login?return=https://grist.example.com
GRIST_PROXY_AUTH_LOGOUT=https://auth.example.com/logout?return=https://grist.example.com

Most of this is already in @MHOOO's enhancement, except for the UI integration 🙂

jyio avatar Mar 19 '22 20:03 jyio

A recent change affected this issue, and fixed some wrong behavior reported above.

  • GRIST_PROXY_AUTH_HEADER is now a synonym to GRIST_FORWARD_AUTH_HEADER; the latter is preferred.
  • GRIST_IGNORE_SESSION=true is needed for a setup that puts Grist behind HTTP Basic Auth.

See here for more details: https://github.com/gristlabs/grist-core#forward-authentication-variables.

Grist behind HTTP Basic Auth is not a well-tested setup, but for those trying it, here are the pieces of configuration that I found necessary to get it working (websockets were tricky; other suggested methods didn't work).

  • In Apache virtual-host configuration:
    ProxyPreserveHost On
    
    # Enable rewrite engine
    RewriteEngine On
    
    # Check for the WebSocket upgrade headers
    RewriteCond %{HTTP:Upgrade} =websocket [NC]
    RewriteRule /(.*)           ws://grist-service:8080/$1 [P,L]
    
    RewriteCond %{HTTP:Upgrade} !=websocket [NC]
    RewriteRule /(.*)           http://grist-service:8080/$1 [P,L]
    
    ProxyPassReverse "/" "http://grist-service:8080/"
    ProxyPassReverse "/" "ws://grist-service:8080/"
    
    <Location "/">
        AuthType Basic
        AuthName "Restricted Content"
        AuthUserFile "/usr/local/apache2/.htpasswd"
        Require valid-user
    
        # Headers for WebSocket Upgrade
        RequestHeader set X-Real-IP %{REMOTE_ADDR}s
        RequestHeader set X-Forwarded-For %{REMOTE_ADDR}s
        RequestHeader set X-Forwarded-Proto http
        RequestHeader set X-Forwarded-Port 80
    
        # Add the X-Remote-User header
        RewriteCond %{REMOTE_USER} (.+)
        RewriteRule .* - [E=RU:%1]
        RequestHeader set X-Remote-User %{RU}e env=RU
    </Location>
    
  • In Grist configuration:
    PORT: 8080
    GRIST_FORWARD_AUTH_HEADER: X-Remote-User
    GRIST_IGNORE_SESSION: true
    GRIST_SANDBOX_FLAVOR: gvisor
    

The last setting (GRIST_SANDBOX_FLAVOR) is not strictly related to HTTP Basic Auth, but important for security of any setup where Grist documents will be shared with others (see https://support.getgrist.com/self-managed/#how-do-i-sandbox-documents).

dsagal avatar Nov 23 '23 06:11 dsagal