Archives/gristlabs_grist-core
1
0
Fork 0
mirror of https://github.com/gristlabs/grist-core.git synced 2024-10-27 20:44:07 +00:00
Code Issues Projects Releases Wiki Activity

(core) make sharing with everyone@ on documents effective

Browse Source 
Summary:
Sharing a document with everyone@ was effective at the api level,
but had two flaws in the web client:

 * A logged in user with no access at the org level could not access
   a publically shared doc within that org.
 * Likewise, for the anonymous user (but for a different reason).

This diff tweaks the web client to permit accessing a doc when
org information is unavailable.

It also changes how redirects happen for the anonymous user when
accessing a doc.  They now only happen once it has been confirmed
that the user does not have access to the doc.

Test Plan: added tests

Reviewers: dsagal

Reviewed By: dsagal

Differential Revision: https://phab.getgrist.com/D2591
This commit is contained in:
Paul Fitzpatrick 2020-08-19 16:25:42 -04:00
parent 0a5afd1f98
commit 4a545c9f2a
3 changed files with 61 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
app/server/lib/AppEndpoint.ts
View File

@ -13,7 +13,8 @@ import {removeTrailingSlash} from 'app/common/gutil';
import {Document as APIDocument} from 'app/common/UserAPI';
import {Document} from "app/gen-server/entity/Document";
import {HomeDBManager} from 'app/gen-server/lib/HomeDBManager';
import {assertAccess, getTransitiveHeaders, getUserId, RequestWithLogin} from 'app/server/lib/Authorizer';
import {assertAccess, getTransitiveHeaders, getUserId, isAnonymousUser,
RequestWithLogin} from 'app/server/lib/Authorizer';
import {DocStatus, IDocWorkerMap} from 'app/server/lib/DocWorkerMap';
import {expressWrap} from 'app/server/lib/expressWrap';
import {getAssignmentId} from 'app/server/lib/idUtils';
@ -22,8 +23,10 @@ import {adaptServerUrl, pruneAPIResult, trustOrigin} from 'app/server/lib/reques
import {ISendAppPageOptions} from 'app/server/lib/sendAppPage';
export interface AttachOptions {
app: express.Application; // Express app to which to add endpoints
middleware: express.RequestHandler[]; // Middleware to apply for all endpoints
app: express.Application; // Express app to which to add endpoints
middleware: express.RequestHandler[]; // Middleware to apply for all endpoints except docs
docMiddleware: express.RequestHandler[]; // Middleware to apply for doc landing pages
forceLogin: express.RequestHandler|null; // Method to force user to login (if logins are possible)
docWorkerMap: IDocWorkerMap|null;
sendAppPage: (req: express.Request, resp: express.Response, options: ISendAppPageOptions) => Promise<void>;
dbManager: HomeDBManager;
@ -147,7 +150,7 @@ async function getWorker(docWorkerMap: IDocWorkerMap, assignmentId: string,
}
export function attachAppEndpoint(options: AttachOptions): void {
const {app, middleware, docWorkerMap, sendAppPage, dbManager} = options;
const {app, middleware, docMiddleware, docWorkerMap, forceLogin, sendAppPage, dbManager} = options;
// Per-workspace URLs open the same old Home page, and it's up to the client to notice and
// render the right workspace.
app.get(['/', '/ws/:wsId', '/p/:page'], ...middleware, expressWrap(async (req, res) =>
@ -219,6 +222,18 @@ export function attachAppEndpoint(options: AttachOptions): void {
throw new ApiError('Document not found.', 404);
} else if (err.status === 403) {
log.info("/:urlId/app.html denied access", mreq.userId, urlId, doc && doc.access, mreq.org);
// If the user does not have access to the document, and is anonymous, and we
// have a login system, we may wish to redirect them to login process.
if (isAnonymousUser(mreq) && forceLogin) {
// First check if anonymous user has access to this org. If so, we don't propose
// that they log in. This is the same check made in redirectToLogin() middleware.
const result = await dbManager.getOrg({userId: getUserId(mreq)}, mreq.org || null);
if (result.status !== 200) {
// Anonymous user does not have any access to this org, or to this doc.
// Redirect to log in.
return forceLogin(req, res, next);
}
}
throw new ApiError('You do not have access to this document.', 403);
}
throw err;
@ -248,7 +263,7 @@ export function attachAppEndpoint(options: AttachOptions): void {
});
// The * is a wildcard in express 4, rather than a regex symbol.
// See https://expressjs.com/en/guide/routing.html
app.get('/doc/:urlId([^/]+):remainder(*)', ...middleware, docHandler);
app.get('/doc/:urlId([^/]+):remainder(*)', ...docMiddleware, docHandler);
app.get('/:urlId([^/]{12,})/:slug([^/]+):remainder(*)',
...middleware, docHandler);
...docMiddleware, docHandler);
}

37
app/server/lib/Authorizer.ts
View File

@ -251,6 +251,28 @@ export async function addRequestUser(dbManager: HomeDBManager, permitStore: IPer
return next();
}
/**
* Returns a handler that redirects the user to a login or signup page.
*/
export function redirectToLoginUnconditionally(
getLoginRedirectUrl: (redirectUrl: URL) => Promise<string>,
getSignUpRedirectUrl: (redirectUrl: URL) => Promise<string>
) {
return async (req: Request, resp: Response, next: NextFunction) => {
const mreq = req as RequestWithLogin;
// Redirect to sign up if it doesn't look like the user has ever logged in (on
// this browser) After logging in, `users` will be set in the session. Even after
// logging out again, `users` will still be set.
const signUp: boolean = (mreq.session.users === undefined);
log.debug(`Authorizer: redirecting to ${signUp ? 'sign up' : 'log in'}`);
const redirectUrl = new URL(req.protocol + '://' + req.get('host') + req.originalUrl);
if (signUp) {
return resp.redirect(await getSignUpRedirectUrl(redirectUrl));
} else {
return resp.redirect(await getLoginRedirectUrl(redirectUrl));
}
};
}
/**
* Middleware to redirects user to a login page when the user is not
@ -264,6 +286,8 @@ export function redirectToLogin(
getSignUpRedirectUrl: (redirectUrl: URL) => Promise<string>,
dbManager: HomeDBManager
): RequestHandler {
const redirectUnconditionally = redirectToLoginUnconditionally(getLoginRedirectUrl,
getSignUpRedirectUrl);
return async (req: Request, resp: Response, next: NextFunction) => {
const mreq = req as RequestWithLogin;
mreq.session.alive = true; // This will ensure that express-session will set our cookie
@ -280,18 +304,7 @@ export function redirectToLogin(
}
// In all other cases (including unknown org), redirect user to login or sign up.
// Redirect to sign up if it doesn't look like the user has ever logged in (on
// this browser) After logging in, `users` will be set in the session. Even after
// logging out again, `users` will still be set.
const signUp: boolean = (mreq.session.users === undefined);
log.debug(`Authorizer: redirecting to ${signUp ? 'sign up' : 'log in'}`);
const redirectUrl = new URL(req.protocol + '://' + req.get('host') + req.originalUrl);
if (signUp) {
return resp.redirect(await getSignUpRedirectUrl(redirectUrl));
} else {
return resp.redirect(await getLoginRedirectUrl(redirectUrl));
}
return redirectUnconditionally(req, resp, next);
} catch (err) {
log.info("Authorizer failed to redirect", err.message);
return resp.status(401).send(err.message);

16
app/server/lib/FlexServer.ts
View File

@ -14,7 +14,8 @@ import {HomeDBManager} from 'app/gen-server/lib/HomeDBManager';
import {Housekeeper} from 'app/gen-server/lib/Housekeeper';
import {Usage} from 'app/gen-server/lib/Usage';
import {attachAppEndpoint} from 'app/server/lib/AppEndpoint';
import {addRequestUser, getUser, getUserId, isSingleUserMode} from 'app/server/lib/Authorizer';
import {addRequestUser, getUser, getUserId, isSingleUserMode,
redirectToLoginUnconditionally} from 'app/server/lib/Authorizer';
import {redirectToLogin, RequestWithLogin} from 'app/server/lib/Authorizer';
import {SessionUserObj} from 'app/server/lib/BrowserSession';
import * as Comm from 'app/server/lib/Comm';
@ -125,6 +126,8 @@ export class FlexServer implements GristServer {
// This unconditionally redirects to signin/signup for anon, for pages where anon access
// is never desired.
private _redirectToLoginWithoutExceptionsMiddleware: express.RequestHandler;
// This can be called to do a redirect to signin/signup in a nuanced situation.
private _redirectToLoginUnconditionally: express.RequestHandler | null;
private _redirectToOrgMiddleware: express.RequestHandler;
private _redirectToHostMiddleware: express.RequestHandler;
private _getLoginRedirectUrl: (target: URL) => Promise<string>;
@ -415,6 +418,8 @@ export class FlexServer implements GristServer {
this._getLoginRedirectUrl,
this._getSignUpRedirectUrl,
this.dbManager);
this._redirectToLoginUnconditionally = redirectToLoginUnconditionally(this._getLoginRedirectUrl,
this._getSignUpRedirectUrl);
this._redirectToOrgMiddleware = tbind(this._redirectToOrg, this);
} else {
const noop: express.RequestHandler = (req, res, next) => next();
@ -430,6 +435,7 @@ export class FlexServer implements GristServer {
};
this._redirectToLoginWithExceptionsMiddleware = noop;
this._redirectToLoginWithoutExceptionsMiddleware = noop;
this._redirectToLoginUnconditionally = null; // there is no way to log in.
this._redirectToOrgMiddleware = noop;
this._redirectToHostMiddleware = noop;
}
@ -639,6 +645,14 @@ export class FlexServer implements GristServer {
this._redirectToOrgMiddleware,
welcomeNewUser
],
docMiddleware: [
// Same as middleware, except without login redirect middleware.
this._redirectToHostMiddleware,
this._userIdMiddleware,
this._redirectToOrgMiddleware,
welcomeNewUser
],
forceLogin: this._redirectToLoginUnconditionally,
docWorkerMap: isSingleUserMode() ? null : this._docWorkerMap,
sendAppPage: this._sendAppPage,
dbManager: this.dbManager,