From 82a7f0a796fdf722a217180e0c81bb8f2ab6f57f Mon Sep 17 00:00:00 2001 From: Thomas Karolski Date: Tue, 8 Mar 2022 19:24:11 +0000 Subject: [PATCH 1/9] Implement support for webserver header based auth --- app/server/lib/Authorizer.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/app/server/lib/Authorizer.ts b/app/server/lib/Authorizer.ts index f62eb874..1dbfbeda 100644 --- a/app/server/lib/Authorizer.ts +++ b/app/server/lib/Authorizer.ts @@ -192,6 +192,7 @@ export async function addRequestUser(dbManager: HomeDBManager, permitStore: IPer } mreq.users = getSessionProfiles(session); + log.info(`mreq.users: ${mreq.users}`); // If we haven't set a maxAge yet, set it now. if (session && session.cookie && !session.cookie.maxAge) { @@ -232,6 +233,7 @@ export async function addRequestUser(dbManager: HomeDBManager, permitStore: IPer } profile = sessionUser && sessionUser.profile || undefined; + log.info(`profile: ${profile}`); // If we haven't computed a userId yet, check for one using an email address in the profile. // A user record will be created automatically for emails we've never seen before. @@ -245,6 +247,28 @@ export async function addRequestUser(dbManager: HomeDBManager, permitStore: IPer } } + // Try to determine user based on x-remote-user header + if (!mreq.userId) { + // mreg.headers["x-remote-user"]; + // log.info(`mreg.headers: ${JSON.stringify(mreq.headers, null, 4)}`); + 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); + if(user) { + mreq.user = user; + mreq.users = [profile]; + mreq.userId = user.id; + mreq.userIsAuthorized = true; + } + } + } + + // If no userId has been found yet, fall back on anonymous. if (!mreq.userId) { const anon = dbManager.getAnonymousUser(); From 953ac7c6891cb142ef75b8fe33429df70398f691 Mon Sep 17 00:00:00 2001 From: Thomas Karolski Date: Tue, 8 Mar 2022 19:24:36 +0000 Subject: [PATCH 2/9] Use /usr/bin/env instead of /bin/bash --- buildtools/prepare_python.sh | 2 +- buildtools/prepare_python2.sh | 2 +- buildtools/prepare_python3.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/buildtools/prepare_python.sh b/buildtools/prepare_python.sh index 05b793d2..b3be736e 100755 --- a/buildtools/prepare_python.sh +++ b/buildtools/prepare_python.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash set -e diff --git a/buildtools/prepare_python2.sh b/buildtools/prepare_python2.sh index 97e6f7a8..59e3acfd 100755 --- a/buildtools/prepare_python2.sh +++ b/buildtools/prepare_python2.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash set -e diff --git a/buildtools/prepare_python3.sh b/buildtools/prepare_python3.sh index 8df3a3a9..a6a2c1c8 100755 --- a/buildtools/prepare_python3.sh +++ b/buildtools/prepare_python3.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash set -e From 116295e42f226afa4aa6034f8e3b4a45c9ccdbc0 Mon Sep 17 00:00:00 2001 From: Thomas Karolski Date: Tue, 8 Mar 2022 19:40:25 +0000 Subject: [PATCH 3/9] Minor refactor & comments --- app/server/lib/Authorizer.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/app/server/lib/Authorizer.ts b/app/server/lib/Authorizer.ts index 1dbfbeda..64f20592 100644 --- a/app/server/lib/Authorizer.ts +++ b/app/server/lib/Authorizer.ts @@ -192,7 +192,6 @@ export async function addRequestUser(dbManager: HomeDBManager, permitStore: IPer } mreq.users = getSessionProfiles(session); - log.info(`mreq.users: ${mreq.users}`); // If we haven't set a maxAge yet, set it now. if (session && session.cookie && !session.cookie.maxAge) { @@ -233,7 +232,6 @@ export async function addRequestUser(dbManager: HomeDBManager, permitStore: IPer } profile = sessionUser && sessionUser.profile || undefined; - log.info(`profile: ${profile}`); // If we haven't computed a userId yet, check for one using an email address in the profile. // A user record will be created automatically for emails we've never seen before. @@ -247,21 +245,21 @@ export async function addRequestUser(dbManager: HomeDBManager, permitStore: IPer } } - // Try to determine user based on x-remote-user header + // Try to determine user based on 'x-remote-user' header passed via a webserver rewrite rule. + // 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) { - // mreg.headers["x-remote-user"]; - // log.info(`mreg.headers: ${JSON.stringify(mreq.headers, null, 4)}`); if (mreq.headers && mreq.headers["x-remote-user"]) { const remoteUser = mreq.headers["x-remote-user"].toString(); - log.info("Authorized user found"); + log.debug("Authorized user based on 'x-remote-user' header found."); profile = { - "email": remoteUser, - "name": remoteUser + "email": remoteUser, + "name": remoteUser }; const user = await dbManager.getUserByLoginWithRetry(remoteUser, profile); if(user) { mreq.user = user; - mreq.users = [profile]; + mreq.users = [profile]; mreq.userId = user.id; mreq.userIsAuthorized = true; } From a584bc3a19a430216ed730295564520fcdfb6384 Mon Sep 17 00:00:00 2001 From: Thomas Karolski Date: Wed, 9 Mar 2022 10:00:03 +0000 Subject: [PATCH 4/9] [Comm.js] Return a session profile based on the x-remote-user header if set --- app/server/lib/Comm.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/app/server/lib/Comm.js b/app/server/lib/Comm.js index b841ad9d..d00d1a99 100644 --- a/app/server/lib/Comm.js +++ b/app/server/lib/Comm.js @@ -150,6 +150,21 @@ Comm.prototype._broadcastMessage = function(type, data, clients) { clients.forEach(client => client.sendMessage({type, data})); }; + +Comm.prototype._getSessionProfile = function(scopedSession, req) { + // apply x-remote-user header as a profile if the header was set + if (req.headers && req.headers['x-remote-user']) { + const userName = req.headers['x-remote-user'].toString(); + return Promise.resolve({ + "email": userName, + "name": userName, + }); + } + 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 +251,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(scopedSession, req) .then((profile) => { log.debug(`Comm ${client}: sending clientConnect with ` + `${client._missedMessages.length} missed messages`); From c459037b0413b5ca54cfaa32bea96b685c7ed69c Mon Sep 17 00:00:00 2001 From: Thomas Karolski Date: Sat, 12 Mar 2022 20:01:03 +0100 Subject: [PATCH 5/9] [authorizer] Move code for extracting auth header into a function --- app/server/lib/Authorizer.ts | 44 +++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/app/server/lib/Authorizer.ts b/app/server/lib/Authorizer.ts index 64f20592..e2cf376b 100644 --- a/app/server/lib/Authorizer.ts +++ b/app/server/lib/Authorizer.ts @@ -89,6 +89,36 @@ export function isSingleUserMode(): boolean { return process.env.GRIST_SINGLE_USER === '1'; } + +/** + * Returns a profile if it can be deduced from the request. This requires a + * header to specify the users' email address. + */ +export function getRequestProfile(req: Request): UserProfile|undefined { + // Try to determine user based on 'x-remote-user' header passed via a webserver rewrite rule. + // TODO: The header should probably be set via an environment variable and if it is not set, + // this code path should be disabled altogether. + let header:string = "x-remote-user"; + let profile: UserProfile|undefined; + + if (req.headers && req.headers[header]) { + let headerContent = req.headers[header]; + if (headerContent) { + const userEmail = headerContent.toString(); + const [userName] = userEmail.split("@", 1); + if (userEmail && userName) { + profile = { + "email": userEmail, + "name": userName + }; + } + } + } + + return profile; +} + + /** * Returns the express request object with user information added, if it can be * found based on passed in headers or the session. Specifically, sets: @@ -245,18 +275,10 @@ export async function addRequestUser(dbManager: HomeDBManager, permitStore: IPer } } - // Try to determine user based on 'x-remote-user' header passed via a webserver rewrite rule. - // 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) { - 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."); - profile = { - "email": remoteUser, - "name": remoteUser - }; - const user = await dbManager.getUserByLoginWithRetry(remoteUser, profile); + profile = getRequestProfile(mreq); + if (profile) { + const user = await dbManager.getUserByLoginWithRetry(profile.email, profile); if(user) { mreq.user = user; mreq.users = [profile]; From 9f3ed989c4c5edc5a999bbd3162079c2cb620c3d Mon Sep 17 00:00:00 2001 From: Thomas Karolski Date: Sat, 12 Mar 2022 20:31:17 +0100 Subject: [PATCH 6/9] [authorizer] Determine auth header to use via an environment variable --- app/server/lib/Authorizer.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/app/server/lib/Authorizer.ts b/app/server/lib/Authorizer.ts index e2cf376b..ef8f6312 100644 --- a/app/server/lib/Authorizer.ts +++ b/app/server/lib/Authorizer.ts @@ -92,16 +92,14 @@ export function isSingleUserMode(): boolean { /** * Returns a profile if it can be deduced from the request. This requires a - * header to specify the users' email address. + * header to specify the users' email address. The header to set comes from the + * environment variable GRIST_PROXY_AUTH_HEADER. */ export function getRequestProfile(req: Request): UserProfile|undefined { - // Try to determine user based on 'x-remote-user' header passed via a webserver rewrite rule. - // TODO: The header should probably be set via an environment variable and if it is not set, - // this code path should be disabled altogether. - let header:string = "x-remote-user"; + const header = process.env.GRIST_PROXY_AUTH_HEADER; let profile: UserProfile|undefined; - if (req.headers && req.headers[header]) { + if (header && req.headers && req.headers[header]) { let headerContent = req.headers[header]; if (headerContent) { const userEmail = headerContent.toString(); From e5dc2d198f4ed81f1bdd610997b2a320475607fe Mon Sep 17 00:00:00 2001 From: Thomas Karolski Date: Sat, 12 Mar 2022 20:31:53 +0100 Subject: [PATCH 7/9] [comm] Use getRequestProfile from Authorizer --- app/server/lib/Comm.js | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/app/server/lib/Comm.js b/app/server/lib/Comm.js index d00d1a99..9e7df52c 100644 --- a/app/server/lib/Comm.js +++ b/app/server/lib/Comm.js @@ -50,6 +50,7 @@ const {parseFirstUrlPart} = require('app/common/gristUrls'); const version = require('app/common/version'); const {Client} = require('./Client'); const {localeFromRequest} = require('app/server/lib/ServerLocale'); +const {getRequestProfile} = require('app/server/lib/Authorizer'); // Bluebird promisification, to be able to use e.g. websocket.sendAsync method. Promise.promisifyAll(ws.prototype); @@ -151,18 +152,16 @@ Comm.prototype._broadcastMessage = function(type, data, clients) { }; +/** + * Returns a profile based on the request or session. + */ Comm.prototype._getSessionProfile = function(scopedSession, req) { - // apply x-remote-user header as a profile if the header was set - if (req.headers && req.headers['x-remote-user']) { - const userName = req.headers['x-remote-user'].toString(); - return Promise.resolve({ - "email": userName, - "name": userName, - }); - } - return scopedSession.getSessionProfile(); -} - + const profile = getRequestProfile(req); + if (profile) + return Promise.resolve(profile); + else + return scopedSession.getSessionProfile(); +}; /** From 87af7a36cdb8e8cfb597c796b1e835b7b4371517 Mon Sep 17 00:00:00 2001 From: Thomas Karolski Date: Sat, 12 Mar 2022 20:40:40 +0100 Subject: [PATCH 8/9] [README] Update with doc on header based auth --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 5f92ea22..0d481acd 100644 --- a/README.md +++ b/README.md @@ -188,6 +188,7 @@ GRIST_MANAGED_WORKERS | if set, Grist can assume that if a url targeted at a doc GRIST_MAX_UPLOAD_ATTACHMENT_MB | max allowed size for attachments (0 or empty for unlimited). GRIST_MAX_UPLOAD_IMPORT_MB | max allowed size for imports (except .grist files) (0 or empty for unlimited). GRIST_ORG_IN_PATH | if true, encode org in path rather than domain +GRIST_PROXY_AUTH_HEADER | header which will be set by a (reverse) proxy webserver with an authorized users' email. This can be used as an alternative to a SAML service. GRIST_ROUTER_URL | optional url for an api that allows servers to be (un)registered with a load balancer GRIST_SERVE_SAME_ORIGIN | set to "true" to access home server and doc workers on the same protocol-host-port as the top-level page, same as for custom domains (careful, host header should be trustworthy) GRIST_SESSION_COOKIE | if set, overrides the name of Grist's cookie From ccdd551b4d0a6879c743684667fbc95a43275ac2 Mon Sep 17 00:00:00 2001 From: Thomas Karolski Date: Mon, 14 Mar 2022 17:51:10 +0100 Subject: [PATCH 9/9] style fixes --- app/server/lib/Authorizer.ts | 2 +- app/server/lib/Comm.js | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/server/lib/Authorizer.ts b/app/server/lib/Authorizer.ts index ef8f6312..63f1a906 100644 --- a/app/server/lib/Authorizer.ts +++ b/app/server/lib/Authorizer.ts @@ -100,7 +100,7 @@ export function getRequestProfile(req: Request): UserProfile|undefined { let profile: UserProfile|undefined; if (header && req.headers && req.headers[header]) { - let headerContent = req.headers[header]; + const headerContent = req.headers[header]; if (headerContent) { const userEmail = headerContent.toString(); const [userName] = userEmail.split("@", 1); diff --git a/app/server/lib/Comm.js b/app/server/lib/Comm.js index 9e7df52c..6e7eaca4 100644 --- a/app/server/lib/Comm.js +++ b/app/server/lib/Comm.js @@ -157,10 +157,11 @@ Comm.prototype._broadcastMessage = function(type, data, clients) { */ Comm.prototype._getSessionProfile = function(scopedSession, req) { const profile = getRequestProfile(req); - if (profile) + if (profile) { return Promise.resolve(profile); - else + } else { return scopedSession.getSessionProfile(); + } };