Simplify RedirectBackComponent into PreviousUrlComponent

Closes #12

src/Application.ts

@@ -121,7 +121,7 @@ export default abstract class Application implements Extendable<ApplicationCompo
                     },
                     html: () => {
                         req.flash('validation', bag.getMessages());
-                        res.redirectBack();
+                        res.redirect(req.getPreviousUrl() || Controller.route('home'));
                     },
                 });
                 return;

src/TestApp.ts

@@ -9,7 +9,6 @@ import MailComponent from "../src/components/MailComponent";
 import SessionComponent from "../src/components/SessionComponent";
 import AuthComponent from "../src/auth/AuthComponent";
 import FormHelperComponent from "../src/components/FormHelperComponent";
-import RedirectBackComponent from "../src/components/RedirectBackComponent";
 import ServeStaticDirectoryComponent from "../src/components/ServeStaticDirectoryComponent";
 import {Express} from "express";
 import MagicLinkAuthMethod from "../src/auth/magic_link/MagicLinkAuthMethod";
@@ -31,6 +30,7 @@ import AccountController from "./auth/AccountController";
 import MakeMagicLinksSessionNotUniqueMigration from "./auth/magic_link/MakeMagicLinksSessionNotUniqueMigration";
 import AddUsedToMagicLinksMigration from "./auth/magic_link/AddUsedToMagicLinksMigration";
 import packageJson = require('../package.json');
+import PreviousUrlComponent from "./components/PreviousUrlComponent";
 
 export const MIGRATIONS = [
     CreateMigrationsTable,
@@ -72,7 +72,7 @@ export default class TestApp extends Application {
 
         // Dynamic views and routes
         this.use(new NunjucksComponent(['test/views', 'views']));
-        this.use(new RedirectBackComponent());
+        this.use(new PreviousUrlComponent());
 
         // Services
         this.use(new MysqlComponent());

src/auth/AccountController.ts

@@ -58,7 +58,7 @@ export default class AccountController extends Controller {
         const passwordComponent = user.as(UserPasswordComponent);
         if (passwordComponent.hasPassword() && !await passwordComponent.verifyPassword(req.body.current_password)) {
             req.flash('error', 'Invalid current password.');
-            res.redirectBack(Controller.route('account'));
+            res.redirect(Controller.route('account'));
             return;
         }
 
@@ -66,7 +66,7 @@ export default class AccountController extends Controller {
         await user.save();
 
         req.flash('success', 'Password changed successfully.');
-        res.redirectBack(Controller.route('account'));
+        res.redirect(Controller.route('account'));
     }
 
 
@@ -120,7 +120,7 @@ export default class AccountController extends Controller {
         await user.save();
 
         req.flash('success', 'This email was successfully set as your main address.');
-        res.redirectBack();
+        res.redirect(Controller.route('account'));
     }
 
     protected async postRemoveEmail(req: Request, res: Response): Promise<void> {
@@ -140,6 +140,6 @@ export default class AccountController extends Controller {
         await userEmail.delete();
 
         req.flash('success', 'This email was successfully removed from your account.');
-        res.redirectBack();
+        res.redirect(Controller.route('account'));
     }
 }

src/auth/AuthComponent.ts

@@ -116,7 +116,7 @@ export class RequireGuestMiddleware extends Middleware {
     protected async handle(req: Request, res: Response, next: NextFunction): Promise<void> {
         const proofs = await req.as(AuthMiddleware).getAuthGuard().getProofsForSession(req.getSession());
         if (proofs.length > 0) {
-            res.redirectBack();
+            res.redirect(Controller.route('home'));
             return;
         }
 

src/auth/AuthController.ts

@@ -117,7 +117,7 @@ export default class AuthController extends Controller {
         }
 
         req.flash('success', 'Successfully logged out.');
-        res.redirect(req.query.redirect_uri?.toString() || '/');
+        res.redirect(req.getIntendedUrl() || '/');
     }
 
     protected async redirectToRegistration(req: Request, res: Response, identifier: string): Promise<void> {

src/auth/magic_link/MagicLinkAuthMethod.ts

@@ -7,7 +7,6 @@ import {WhereTest} from "../../db/ModelQuery";
 import Controller from "../../Controller";
 import geoip from "geoip-lite";
 import MagicLinkController from "./MagicLinkController";
-import RedirectBackComponent from "../../components/RedirectBackComponent";
 import Application from "../../Application";
 import {MailTemplate} from "../../mail/Mail";
 import AuthMagicLinkActionType from "./AuthMagicLinkActionType";
@@ -55,7 +54,7 @@ export default class MagicLinkAuthMethod implements AuthMethod<MagicLink> {
         if (pendingLink) {
             if (await pendingLink.isValid()) {
                 res.redirect(Controller.route('magic_link_lobby', undefined, {
-                    redirect_uri: req.query.redirect_uri?.toString() || pendingLink.original_url || undefined,
+                    redirect_uri: req.getIntendedUrl() || pendingLink.original_url || undefined,
                 }));
                 return true;
             } else {
@@ -105,7 +104,7 @@ export default class MagicLinkAuthMethod implements AuthMethod<MagicLink> {
             req.getSession().id,
             actionType,
             Controller.route('auth', undefined, {
-                redirect_uri: req.query.redirect_uri?.toString() || undefined,
+                redirect_uri: req.getIntendedUrl() || undefined,
             }),
             email,
             this.magicLinkMailTemplate,
@@ -120,7 +119,7 @@ export default class MagicLinkAuthMethod implements AuthMethod<MagicLink> {
         );
 
         res.redirect(Controller.route('magic_link_lobby', undefined, {
-            redirect_uri: req.query.redirect_uri?.toString() || RedirectBackComponent.getPreviousURL(req),
+            redirect_uri: req.getIntendedUrl(),
         }));
     }
 }

src/auth/magic_link/MagicLinkController.ts

@@ -192,7 +192,7 @@ export default class MagicLinkController<A extends Application> extends Controll
                 if (!res.headersSent && user) {
                     // Auth success
                     req.flash('success', `Authentication success. Welcome, ${user.name}!`);
-                    res.redirect(req.query.redirect_uri?.toString() || Controller.route('home'));
+                    res.redirect(req.getIntendedUrl() || Controller.route('home'));
                 }
                 break;
             }

src/auth/password/PasswordAuthMethod.ts

@@ -72,7 +72,7 @@ export default class PasswordAuthMethod implements AuthMethod<PasswordAuthProof>
 
                 if (e instanceof PendingApprovalAuthError) {
                     req.flash('error', 'Your account is still being reviewed.');
-                    res.redirectBack();
+                    res.redirect(Controller.route('auth'));
                     return;
                 } else {
                     const err = new InvalidFormatValidationError('Invalid password.');
@@ -85,7 +85,7 @@ export default class PasswordAuthMethod implements AuthMethod<PasswordAuthProof>
         }
 
         req.flash('success', `Welcome, ${user.name}.`);
-        res.redirect(Controller.route('home'));
+        res.redirect(req.getIntendedUrl() || Controller.route('home'));
     }
 
     public async attemptRegister(req: Request, res: Response, identifier: string): Promise<void> {
@@ -123,7 +123,7 @@ export default class PasswordAuthMethod implements AuthMethod<PasswordAuthProof>
         } catch (e) {
             if (e instanceof PendingApprovalAuthError) {
                 req.flash('info', `Your account was successfully created and is pending review from an administrator.`);
-                res.redirect(Controller.route('home'));
+                res.redirect(Controller.route('auth'));
                 return;
             } else {
                 throw e;
@@ -133,7 +133,7 @@ export default class PasswordAuthMethod implements AuthMethod<PasswordAuthProof>
         const user = await passwordAuthProof.getResource();
 
         req.flash('success', `Your account was successfully created! Welcome, ${user?.as(UserNameComponent).name}.`);
-        res.redirect(Controller.route('home'));
+        res.redirect(req.getIntendedUrl() || Controller.route('home'));
     }
 
 }

src/components/PreviousUrlComponent.ts

@@ -0,0 +1,54 @@
+import ApplicationComponent from "../ApplicationComponent";
+import {Router} from "express";
+import onFinished from "on-finished";
+import {logger} from "../Logger";
+import SessionComponent from "./SessionComponent";
+
+export default class PreviousUrlComponent extends ApplicationComponent {
+
+    public async handle(router: Router): Promise<void> {
+        router.use((req, res, next) => {
+            req.getPreviousUrl = () => {
+                let url = req.header('referer');
+                if (url) {
+                    if (url.indexOf('://') >= 0) url = '/' + url.split('/').slice(3).join('/');
+                    if (url !== req.originalUrl) return url;
+                }
+
+                if (this.getApp().asOptional(SessionComponent)) {
+                    const session = req.getSessionOptional();
+                    url = session?.previousUrl;
+                    if (url && url !== req.originalUrl) return url;
+                }
+
+                return null;
+            };
+            res.locals.getPreviousUrl = req.getPreviousUrl;
+
+            req.getIntendedUrl = () => {
+                return req.query.redirect_uri?.toString() || null;
+            };
+
+            if (this.getApp().asOptional(SessionComponent)) {
+                const session = req.getSessionOptional();
+                if (session && req.method === 'GET') {
+                    onFinished(res, (err) => {
+                        if (err) return;
+
+                        const contentType = res.getHeader('content-type');
+                        if (res.statusCode === 200 &&
+                            contentType && typeof contentType !== 'number' && contentType.indexOf('text/html') >= 0) {
+                            session.previousUrl = req.originalUrl;
+
+                            session.save((err) => {
+                                if (err) logger.error(err, 'Error while saving session');
+                                else logger.debug('Prev url set to', session.previousUrl);
+                            });
+                        }
+                    });
+                }
+            }
+            next();
+        });
+    }
+}

src/components/RedirectBackComponent.ts

@@ -1,45 +0,0 @@
-import ApplicationComponent from "../ApplicationComponent";
-import {Request, Router} from "express";
-import {ServerError} from "../HttpError";
-import onFinished from "on-finished";
-import {logger} from "../Logger";
-
-export default class RedirectBackComponent extends ApplicationComponent {
-    public static getPreviousURL(req: Request, defaultUrl?: string): string | undefined {
-        return req.getSessionOptional()?.previousUrl || defaultUrl;
-    }
-
-    public async handle(router: Router): Promise<void> {
-        router.use((req, res, next) => {
-            res.redirectBack = (defaultUrl?: string): void => {
-                const previousUrl = RedirectBackComponent.getPreviousURL(req, defaultUrl);
-                if (!previousUrl) throw new ServerError(`Couldn't redirect you back.`);
-                res.redirect(previousUrl);
-            };
-
-            res.locals.getPreviousURL = (defaultUrl?: string) => {
-                return RedirectBackComponent.getPreviousURL(req, defaultUrl);
-            };
-
-            onFinished(res, (err) => {
-                const session = req.getSessionOptional();
-                if (session) {
-                    const contentType = res.getHeader('content-type');
-                    if (!err && res.statusCode === 200 && (
-                        contentType && typeof contentType !== 'number' && contentType.indexOf('text/html') >= 0
-                    )) {
-                        session.previousUrl = req.originalUrl;
-                        logger.debug('Prev url set to', session.previousUrl);
-                        session.save((err) => {
-                            if (err) {
-                                logger.error(err, 'Error while saving session');
-                            }
-                        });
-                    }
-                }
-            });
-
-            next();
-        });
-    }
-}

src/helpers/BackendController.ts

@@ -82,7 +82,7 @@ export default class BackendController extends Controller {
         }
 
         req.flash('success', `Account successfully approved.`);
-        res.redirectBack(Controller.route('accounts-approval'));
+        res.redirect(Controller.route('accounts-approval'));
     }
 
     protected async postRejectAccount(req: Request, res: Response): Promise<void> {
@@ -97,7 +97,7 @@ export default class BackendController extends Controller {
         }
 
         req.flash('success', `Account successfully deleted.`);
-        res.redirectBack(Controller.route('accounts-approval'));
+        res.redirect(Controller.route('accounts-approval'));
     }
 
     protected async accountRequest(req: Request): Promise<{

src/types/Express.d.ts

@@ -26,10 +26,11 @@ declare global {
             flash(message: string): unknown[];
 
             flash(event: string, message: unknown): void;
-        }
 
-        export interface Response {
+
-            redirectBack(defaultUrl?: string): void;
+            getPreviousUrl(): string | null;
+
+            getIntendedUrl(): string | null;
         }
     }
 }

test/Authentication.test.ts

@@ -10,6 +10,7 @@ import {popEmail} from "./_mail_server";
 import AuthComponent from "../src/auth/AuthComponent";
 import {followMagicLinkFromMail, testLogout} from "./_authentication_common";
 import UserEmail from "../src/auth/models/UserEmail";
+import * as querystring from "querystring";
 
 let app: TestApp;
 useApp(async (addr, port) => {
@@ -87,7 +88,7 @@ describe('Register with username and password (password)', () => {
                 terms: 'on',
             })
             .expect(302)
-            .expect('Location', '/csrf');
+            .expect('Location', '/');
 
         const user2 = await User.select()
             .where('name', 'entrapta2')
@@ -158,7 +159,7 @@ describe('Register with email (magic_link)', () => {
         const cookies = res.get('Set-Cookie');
         const csrf = res.text;
 
-        await agent.post('/auth/register')
+        await agent.post('/auth/register?' + querystring.stringify({redirect_uri: '/redirect-uri'}))
             .set('Cookie', cookies)
             .send({
                 csrf: csrf,
@@ -167,7 +168,7 @@ describe('Register with email (magic_link)', () => {
                 name: 'glimmer',
             })
             .expect(302)
-            .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf');
+            .expect('Location', '/magic/lobby?redirect_uri=%2Fredirect-uri');
 
         await followMagicLinkFromMail(agent, cookies);
 
@@ -221,7 +222,7 @@ describe('Register with email (magic_link)', () => {
                 name: 'angella',
             })
             .expect(302)
-            .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf');
+            .expect('Location', '/magic/lobby?redirect_uri=');
 
         await followMagicLinkFromMail(agent, cookies);
 
@@ -263,7 +264,7 @@ describe('Register with email (magic_link)', () => {
                 name: 'bow',
             })
             .expect(302)
-            .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf');
+            .expect('Location', '/magic/lobby?redirect_uri=');
 
         await followMagicLinkFromMail(agent, cookies);
 
@@ -314,7 +315,7 @@ describe('Authenticate with username and password (password)', () => {
         expect(res.body.messages?.password?.name).toStrictEqual('InvalidFormatValidationError');
 
         // Authenticate
-        await agent.post('/auth/login')
+        await agent.post('/auth/login?' + querystring.stringify({redirect_uri: '/redirect-uri'}))
             .set('Cookie', cookies)
             .send({
                 csrf: csrf,
@@ -323,7 +324,7 @@ describe('Authenticate with username and password (password)', () => {
                 auth_method: 'password',
             })
             .expect(302)
-            .expect('Location', '/');
+            .expect('Location', '/redirect-uri');
 
         await testLogout(agent, cookies, csrf);
     });
@@ -458,7 +459,7 @@ describe('Authenticate with email (magic_link)', () => {
         await agent.get('/is-auth').set('Cookie', cookies).expect(401);
 
         // Authenticate
-        await agent.post('/auth/login')
+        await agent.post('/auth/login?' + querystring.stringify({redirect_uri: '/redirect-uri'}))
             .set('Cookie', cookies)
             .send({
                 csrf: csrf,
@@ -466,7 +467,7 @@ describe('Authenticate with email (magic_link)', () => {
                 auth_method: 'magic_link',
             })
             .expect(302)
-            .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf');
+            .expect('Location', '/magic/lobby?redirect_uri=%2Fredirect-uri');
 
         await followMagicLinkFromMail(agent, cookies);
 
@@ -489,7 +490,7 @@ describe('Authenticate with email (magic_link)', () => {
                 identifier: 'angella@example.org',
             })
             .expect(302)
-            .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf');
+            .expect('Location', '/magic/lobby?redirect_uri=');
 
         await followMagicLinkFromMail(agent, cookies);
 
@@ -553,7 +554,7 @@ describe('Authenticate with email and password (password)', () => {
                 name: 'double-trouble',
             })
             .expect(302)
-            .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf');
+            .expect('Location', '/magic/lobby?redirect_uri=');
 
         await followMagicLinkFromMail(agent, cookies);
 
@@ -664,7 +665,7 @@ describe('Change password', () => {
                 name: 'aang',
             })
             .expect(302)
-            .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf');
+            .expect('Location', '/magic/lobby?redirect_uri=');
 
         await followMagicLinkFromMail(agent, cookies);
     });
@@ -699,7 +700,7 @@ describe('Change password', () => {
                 'new_password_confirmation': 'a_very_strong_password',
             })
             .expect(302)
-            .expect('Location', '/csrf'); // TODO: because of buggy RedirectBackComponent, change to /account once fixed.
+            .expect('Location', '/account/');
 
         const user = await User.select()
             .where('name', 'aang')
@@ -741,7 +742,7 @@ describe('Change password', () => {
                 'new_password_confirmation': 'a_very_strong_password_but_different',
             })
             .expect(302)
-            .expect('Location', '/csrf'); // TODO: because of buggy RedirectBackComponent, change to /account once fixed.
+            .expect('Location', '/account/');
 
         const user = await User.select()
             .where('name', 'aang')
@@ -839,7 +840,7 @@ describe('Manage email addresses', () => {
                 name: 'katara',
             })
             .expect(302)
-            .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf');
+            .expect('Location', '/magic/lobby?redirect_uri=');
 
         await followMagicLinkFromMail(agent, cookies);
 
@@ -933,7 +934,7 @@ describe('Manage email addresses', () => {
                     id: beforeSecondaryEmail?.id,
                 })
                 .expect(302)
-                .expect('Location', '/csrf'); // TODO: because of buggy RedirectBackComponent, change to /account once fixed.
+                .expect('Location', '/account/');
 
             await testMainSecondaryState('katara3@example.org', 'katara@example.org');
         });
@@ -981,7 +982,7 @@ describe('Manage email addresses', () => {
                     id: (await UserEmail.select().where('email', 'katara2@example.org').first())?.id,
                 })
                 .expect(302)
-                .expect('Location', '/csrf'); // TODO: because of buggy RedirectBackComponent, change to /account once fixed.
+                .expect('Location', '/account/');
 
             expect(await UserEmail.select().where('email', 'katara2@example.org').count()).toBe(0);
         });
@@ -1046,7 +1047,7 @@ describe('Session persistence', () => {
                 name: 'zuko',
             })
             .expect(302)
-            .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf');
+            .expect('Location', '/magic/lobby?redirect_uri=');
 
         await followMagicLinkFromMail(agent, cookies);
 
@@ -1075,7 +1076,7 @@ describe('Session persistence', () => {
                 persist_session: 'on',
             })
             .expect(302)
-            .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf');
+            .expect('Location', '/magic/lobby?redirect_uri=');
 
         await followMagicLinkFromMail(agent, cookies);
 
@@ -1102,7 +1103,7 @@ describe('Session persistence', () => {
                 persist_session: undefined,
             })
             .expect(302)
-            .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf');
+            .expect('Location', '/magic/lobby?redirect_uri=');
 
         await followMagicLinkFromMail(agent, cookies);
 

test/AuthenticationNoUsername.test.ts

@@ -81,7 +81,7 @@ describe('Register with email (magic_link)', () => {
                 identifier: 'glimmer@example.org',
             })
             .expect(302)
-            .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf');
+            .expect('Location', '/magic/lobby?redirect_uri=');
 
         await followMagicLinkFromMail(agent, cookies);
 
@@ -116,7 +116,7 @@ describe('Register with email (magic_link)', () => {
                 name: 'bow',
             })
             .expect(302)
-            .expect('Location', '/magic/lobby?redirect_uri=%2Fcsrf');
+            .expect('Location', '/magic/lobby?redirect_uri=');
 
         await followMagicLinkFromMail(agent, cookies);
 

views/auth/auth.njk

@@ -8,8 +8,11 @@
 {% block body %}
     <div class="container">
         {% set queryStr = '' %}
+        {% set previousUrl = getPreviousUrl() %}
         {% if query.redirect_uri | length %}
             {% set queryStr = '?' + querystring.stringify({redirect_uri: query.redirect_uri}) %}
+        {% elif previousUrl | length %}
+            {% set queryStr = '?' + querystring.stringify({redirect_uri: previousUrl}) %}
         {% endif %}
 
         <section class="panel">

views/errors/error.njk

@@ -19,9 +19,9 @@
         <div class="error-instructions">{{ error_instructions|safe }}</div>
 
         <nav>
-            {% set previousURL = getPreviousURL() %}
+            {% set previousUrl = getPreviousUrl() %}
-            {% if previousURL and previousURL != '/' and previousURL != url %}
+            {% if previousUrl and previousUrl != '/' and previousUrl != url %}
-                <a href="{{ previousURL }}" class="button"><i data-feather="arrow-left"></i> Go back</a>
+                <a href="{{ previousUrl }}" class="button"><i data-feather="arrow-left"></i> Go back</a>
             {% endif %}
 
             <a href="/" class="button"><i data-feather="home"></i> Go to homepage</a>