From eb935bf52a51d7ede5d92d741340d4ca1cbaa27e Mon Sep 17 00:00:00 2001 From: Alice Gaudon Date: Wed, 15 Jul 2020 11:42:49 +0200 Subject: [PATCH] Fix auth redirect_uri chain --- package.json | 2 +- src/Controller.ts | 7 +++-- src/Mail.ts | 8 ++--- src/auth/AuthComponent.ts | 5 ++-- src/auth/AuthController.ts | 2 +- src/auth/MailController.ts | 2 +- .../magic_link/MagicLinkAuthController.ts | 29 ++++++++++++------- src/auth/magic_link/MagicLinkController.ts | 13 +++------ src/components/FormHelperComponent.ts | 2 -- src/components/NunjucksComponent.ts | 14 ++++++--- views/auth/auth.njk | 6 +++- 11 files changed, 50 insertions(+), 40 deletions(-) diff --git a/package.json b/package.json index bb60f55..097306f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "wms-core", - "version": "0.14.0", + "version": "0.15.0", "description": "Node web framework", "repository": "git@gitlab.com:ArisuOngaku/wms-core.git", "author": "Alice Gaudon ", diff --git a/src/Controller.ts b/src/Controller.ts index 32ef104..b13c9e4 100644 --- a/src/Controller.ts +++ b/src/Controller.ts @@ -4,11 +4,13 @@ import config from "config"; import Logger from "./Logger"; import Validator, {FileError, ValidationBag} from "./db/Validator"; import FileUploadMiddleware from "./FileUploadMiddleware"; +import * as querystring from "querystring"; +import {ParsedUrlQueryInput} from "querystring"; export default abstract class Controller { private static readonly routes: { [p: string]: string } = {}; - public static route(route: string, params: RouteParams = [], absolute: boolean = false): string { + public static route(route: string, params: RouteParams = [], query: ParsedUrlQueryInput = {}, absolute: boolean = false): string { let path = this.routes[route]; if (path === undefined) throw new Error(`Unknown route for name ${route}.`); @@ -31,7 +33,8 @@ export default abstract class Controller { } } - return `${absolute ? config.get('public_url') : ''}${path}`; + const queryStr = querystring.stringify(query); + return `${absolute ? config.get('public_url') : ''}${path}` + (queryStr.length > 0 ? '?' + queryStr : ''); } private readonly router: Router = express.Router(); diff --git a/src/Mail.ts b/src/Mail.ts index 267be88..377ad92 100644 --- a/src/Mail.ts +++ b/src/Mail.ts @@ -5,12 +5,8 @@ import nunjucks from 'nunjucks'; import * as util from "util"; import {WrappingError} from "./Utils"; import mjml2html from "mjml"; -import * as querystring from "querystring"; import Logger from "./Logger"; - -export function mailRoute(template: string): string { - return `/mail/${template}`; -} +import Controller from "./Controller"; export default class Mail { private static transporter: Transporter; @@ -101,7 +97,7 @@ export default class Mail { // Set data this.data.mail_subject = this.options.subject; this.data.mail_to = this.options.to; - this.data.mail_link = `${config.get('public_url')}${mailRoute(this.template.template)}?${querystring.stringify(this.data)}`; + this.data.mail_link = config.get('public_url') + Controller.route('mail', [this.template.template], this.data); this.data.app = config.get('app'); // Log diff --git a/src/auth/AuthComponent.ts b/src/auth/AuthComponent.ts index ea0e78b..0bacb83 100644 --- a/src/auth/AuthComponent.ts +++ b/src/auth/AuthComponent.ts @@ -3,7 +3,6 @@ import {NextFunction, Request, Response, Router} from "express"; import AuthGuard from "./AuthGuard"; import Controller from "../Controller"; import {ForbiddenHttpError} from "../HttpError"; -import * as querystring from "querystring"; export default class AuthComponent extends ApplicationComponent { private readonly authGuard: AuthGuard; @@ -25,7 +24,7 @@ export default class AuthComponent extends ApplicationComponent { export const REQUIRE_REQUEST_AUTH_MIDDLEWARE = async (req: Request, res: Response, next: NextFunction): Promise => { if (!await req.authGuard.isAuthenticatedViaRequest(req)) { req.flash('error', `You must be logged in to access ${req.url}.`); - res.redirect((Controller.route('auth') || '/') + '?' + querystring.stringify({ + res.redirect(Controller.route('auth', undefined, { redirect_uri: req.url, })); return; @@ -42,7 +41,7 @@ export const REQUIRE_AUTH_MIDDLEWARE = async (req: Request, res: Response, next: } else { if (!await req.authGuard.isAuthenticated(req.session!)) { req.flash('error', `You must be logged in to access ${req.url}.`); - res.redirect((Controller.route('auth') || '/') + '?' + querystring.stringify({ + res.redirect(Controller.route('auth', undefined, { redirect_uri: req.url, })); return; diff --git a/src/auth/AuthController.ts b/src/auth/AuthController.ts index e52e979..7bebc8e 100644 --- a/src/auth/AuthController.ts +++ b/src/auth/AuthController.ts @@ -28,7 +28,7 @@ export default abstract class AuthController extends Controller { protected async postLogout(req: Request, res: Response, next: NextFunction): Promise { await req.authGuard.logout(req.session!); req.flash('success', 'Successfully logged out.'); - res.redirectBack('/'); + res.redirect(req.query.redirect_uri?.toString() || '/'); } } \ No newline at end of file diff --git a/src/auth/MailController.ts b/src/auth/MailController.ts index 9a3c9ea..a76e521 100644 --- a/src/auth/MailController.ts +++ b/src/auth/MailController.ts @@ -4,7 +4,7 @@ import Mail from "../Mail"; export default class MailController extends Controller { routes(): void { - this.get("/mail/:template", this.getMail); + this.get("/mail/:template", this.getMail, 'mail'); } private async getMail(request: Request, response: Response) { diff --git a/src/auth/magic_link/MagicLinkAuthController.ts b/src/auth/magic_link/MagicLinkAuthController.ts index 3135126..3162f54 100644 --- a/src/auth/magic_link/MagicLinkAuthController.ts +++ b/src/auth/magic_link/MagicLinkAuthController.ts @@ -8,6 +8,7 @@ import {MailTemplate} from "../../Mail"; import {AuthError, PendingApprovalAuthError} from "../AuthGuard"; import geoip from "geoip-lite"; import AuthController from "../AuthController"; +import NunjucksComponent from "../../components/NunjucksComponent"; export default abstract class MagicLinkAuthController extends AuthController { @@ -30,7 +31,7 @@ export default abstract class MagicLinkAuthController extends AuthController { }, html: () => { req.flash('warning', `Your account is pending review. You'll receive an email once you're approved.`); - res.redirectBack('/'); + res.redirect('/'); } }); return; @@ -49,14 +50,16 @@ export default abstract class MagicLinkAuthController extends AuthController { this.magicLinkMailTemplate = magicLinkMailTemplate; } - protected async getAuth(request: Request, response: Response, next: NextFunction): Promise { - const link = await MagicLink.bySessionID(request.sessionID!, [this.loginMagicLinkActionType, this.registerMagicLinkActionType]); + protected async getAuth(req: Request, res: Response, next: NextFunction): Promise { + const link = await MagicLink.bySessionID(req.sessionID!, [this.loginMagicLinkActionType, this.registerMagicLinkActionType]); if (link && await link.isValid()) { - response.redirect(Controller.route('magic_link_lobby')); + res.redirect(Controller.route('magic_link_lobby', undefined, { + redirect_uri: req.query.redirect_uri?.toString() || undefined, + })); return; } - await super.getAuth(request, response, next); + await super.getAuth(req, res, next); } protected async postAuth(req: Request, res: Response, next: NextFunction): Promise { @@ -81,21 +84,27 @@ export default abstract class MagicLinkAuthController extends AuthController { await MagicLinkController.sendMagicLink( req.sessionID!, isRegistration ? this.registerMagicLinkActionType : this.loginMagicLinkActionType, - Controller.route('auth'), + Controller.route('auth', undefined, { + redirect_uri: req.query.redirect_uri?.toString() || undefined, + }), email, this.magicLinkMailTemplate, { type: isRegistration ? 'register' : 'login', ip: req.ip, geo: geo ? `${geo.city}, ${geo.country}` : 'Unknown location', - }, - req, - res + } ); + + res.redirect(Controller.route('magic_link_lobby', undefined, { + redirect_uri: req.query.redirect_uri?.toString() || NunjucksComponent.getPreviousURL(req), + })); } else { // Confirm registration req req.flash('register_confirm_email', email); - res.redirect(Controller.route('auth')); + res.redirect(Controller.route('auth', undefined, { + redirect_uri: req.query.redirect_uri?.toString() || undefined, + })); } } diff --git a/src/auth/magic_link/MagicLinkController.ts b/src/auth/magic_link/MagicLinkController.ts index fc601af..38a117e 100644 --- a/src/auth/magic_link/MagicLinkController.ts +++ b/src/auth/magic_link/MagicLinkController.ts @@ -2,38 +2,33 @@ import Controller from "../../Controller"; import {Request, Response} from "express"; import MagicLinkWebSocketListener from "./MagicLinkWebSocketListener"; import {BadRequestError, NotFoundHttpError} from "../../HttpError"; -import querystring from "querystring"; import Throttler from "../../Throttler"; import Mail, {MailTemplate} from "../../Mail"; import MagicLink from "../models/MagicLink"; import config from "config"; export default abstract class MagicLinkController extends Controller { - public static async sendMagicLink(sessionID: string, actionType: string, original_url: string, email: string, mailTemplate: MailTemplate, data: object, req: Request, res: Response): Promise { + public static async sendMagicLink(sessionID: string, actionType: string, original_url: string, email: string, mailTemplate: MailTemplate, data: object): Promise { Throttler.throttle('magic_link', 2, MagicLink.validityPeriod(), sessionID, 0, 0); Throttler.throttle('magic_link', 1, MagicLink.validityPeriod(), email, 0, 0); - let link = await MagicLink.bySessionID(sessionID, actionType); - if (!link) { - link = new MagicLink({ + const link = await MagicLink.bySessionID(sessionID, actionType) || + new MagicLink({ session_id: sessionID, action_type: actionType, original_url: original_url, }); - } const token = await link.generateToken(email); await link.save(); // Send email await new Mail(mailTemplate, Object.assign(data, { - link: `${config.get('base_url')}${Controller.route('magic_link')}?${querystring.stringify({ + link: `${config.get('base_url')}${Controller.route('magic_link', undefined, { id: link.id, token: token, })}`, })).send(email); - - res.redirect(Controller.route('magic_link_lobby')); } diff --git a/src/components/FormHelperComponent.ts b/src/components/FormHelperComponent.ts index 4937107..ab9f7f4 100644 --- a/src/components/FormHelperComponent.ts +++ b/src/components/FormHelperComponent.ts @@ -8,8 +8,6 @@ export default class FormHelperComponent extends ApplicationComponent { throw new Error('Session is unavailable.'); } - res.locals.query = req.query; - let _validation: any = null; res.locals.validation = () => { if (!_validation) { diff --git a/src/components/NunjucksComponent.ts b/src/components/NunjucksComponent.ts index 7db0d26..4ed10a7 100644 --- a/src/components/NunjucksComponent.ts +++ b/src/components/NunjucksComponent.ts @@ -5,10 +5,14 @@ import ApplicationComponent from "../ApplicationComponent"; import Controller from "../Controller"; import {ServerError} from "../HttpError"; import * as querystring from "querystring"; +import {ParsedUrlQueryInput} from "querystring"; export default class NunjucksComponent extends ApplicationComponent { public static getPreviousURL(req: Request, defaultUrl?: string): string { - return req.query.redirect_uri?.toString() || req.headers.referer?.[0] || defaultUrl || '/'; + let referrer = req.headers.referer?.toString() || req.headers.referrer?.toString() || defaultUrl || '/'; + if (referrer.startsWith('https://') || referrer.startsWith('http://')) return '/'; + else if (!referrer.startsWith('/')) referrer = req.url + (req.url.endsWith('/') ? '' : '/') + referrer; + return referrer; } private readonly viewsPath: string; @@ -40,8 +44,8 @@ export default class NunjucksComponent extends ApplicationComponent { noCache: !config.get('view.cache'), throwOnUndefined: true, }) - .addGlobal('route', (route: string, params: { [p: string]: string } | [] = [], absolute: boolean = false) => { - const path = Controller.route(route, params, absolute); + .addGlobal('route', (route: string, params?: { [p: string]: string } | [], query?: ParsedUrlQueryInput, absolute?: boolean) => { + const path = Controller.route(route, params, query, absolute); if (path === null) throw new ServerError(`Route ${route} not found.`); return path; }) @@ -58,7 +62,9 @@ export default class NunjucksComponent extends ApplicationComponent { router.use((req, res, next) => { req.env = this.env!; res.locals.url = req.url; - res.locals.params = () => req.params; + res.locals.params = req.params; + res.locals.query = req.query; + res.locals.body = req.body; res.locals.app = config.get('app'); diff --git a/views/auth/auth.njk b/views/auth/auth.njk index e7bf870..10fb3a4 100644 --- a/views/auth/auth.njk +++ b/views/auth/auth.njk @@ -8,7 +8,11 @@ {% block body %}
- {% set action = route('auth') + '?' + querystring.stringify({redirect_uri: req.url}) %} + {% set queryStr = '' %} + {% if query.redirect_uri | length %} + {% set queryStr = '?' + querystring.stringify({redirect_uri: query.redirect_uri}) %} + {% endif %} + {% set action = route('auth') + queryStr %} {% if register_confirm_email %}