Improve approval mode component security, reliability and usage

src/auth/AuthGuard.ts

@@ -53,7 +53,7 @@ export default abstract class AuthGuard<P extends AuthProof> {
             }
 
             if (user) {
-                if (User.isApprovalMode()) {
+                if (!user!.isApproved()) {
                     await new Mail(PENDING_ACCOUNT_REVIEW_MAIL_TEMPLATE, {
                         username: user!.name,
                         link: config.get<string>('base_url') + Controller.route('accounts-approval'),
@@ -66,7 +66,9 @@ export default abstract class AuthGuard<P extends AuthProof> {
             throw new UserAlreadyExistsAuthError(await proof.getEmail());
         }
 
-        if (!user.isApproved()) throw new PendingApprovalAuthError();
+        if (!user.isApproved()) {
+            throw new PendingApprovalAuthError();
+        }
 
         session.auth_id = user.id;
     }
@@ -131,4 +133,4 @@ export class PendingApprovalAuthError extends AuthError {
     constructor() {
         super(`User is not approved.`);
     }
-}
+}

src/helpers/BackendController.ts

@@ -3,9 +3,10 @@ import Controller from "../Controller";
 import {REQUIRE_ADMIN_MIDDLEWARE, REQUIRE_AUTH_MIDDLEWARE} from "../auth/AuthComponent";
 import User from "../auth/models/User";
 import {Request, Response} from "express";
-import {NotFoundHttpError} from "../HttpError";
+import {BadRequestError, NotFoundHttpError} from "../HttpError";
 import Mail from "../Mail";
 import {ACCOUNT_REVIEW_NOTICE_MAIL_TEMPLATE} from "../Mails";
+import UserEmail from "../auth/models/UserEmail";
 
 export default class BackendController extends Controller {
     getRoutesPrefix(): string {
@@ -16,8 +17,8 @@ export default class BackendController extends Controller {
         this.get('/', this.getIndex, 'backend', REQUIRE_AUTH_MIDDLEWARE, REQUIRE_ADMIN_MIDDLEWARE);
         if (User.isApprovalMode()) {
             this.get('/accounts-approval', this.getAccountApproval, 'accounts-approval', REQUIRE_AUTH_MIDDLEWARE, REQUIRE_ADMIN_MIDDLEWARE);
-            this.post('/accounts-approval/approve/:id', this.postApproveAccount, 'approve-account', REQUIRE_AUTH_MIDDLEWARE, REQUIRE_ADMIN_MIDDLEWARE);
+            this.post('/accounts-approval/approve', this.postApproveAccount, 'approve-account', REQUIRE_AUTH_MIDDLEWARE, REQUIRE_ADMIN_MIDDLEWARE);
-            this.post('/accounts-approval/reject/:id', this.postRejectAccount, 'reject-account', REQUIRE_AUTH_MIDDLEWARE, REQUIRE_ADMIN_MIDDLEWARE);
+            this.post('/accounts-approval/reject', this.postRejectAccount, 'reject-account', REQUIRE_AUTH_MIDDLEWARE, REQUIRE_ADMIN_MIDDLEWARE);
         }
     }
 
@@ -36,9 +37,7 @@ export default class BackendController extends Controller {
     }
 
     public async postApproveAccount(req: Request, res: Response): Promise<void> {
-        const account = await User.select().where('id', req.params.id).with('mainEmail').first();
+        const {account, email} = await this.accountRequest(req);
-        if (!account) throw new NotFoundHttpError('User', req.url);
-        const email = await account.mainEmail.get();
 
         account.approved = true;
         await account.save();
@@ -53,9 +52,7 @@ export default class BackendController extends Controller {
     }
 
     public async postRejectAccount(req: Request, res: Response): Promise<void> {
-        const account = await User.select().where('id', req.params.id).with('mainEmail').first();
+        const {account, email} = await this.accountRequest(req);
-        if (!account) throw new NotFoundHttpError('User', req.url);
-        const email = await account.mainEmail.get();
 
         await account.delete();
 
@@ -66,4 +63,19 @@ export default class BackendController extends Controller {
         req.flash('success', `Account successfully deleted.`);
         res.redirectBack(Controller.route('accounts-approval'));
     }
+
+    private async accountRequest(req: Request): Promise<{
+        account: User,
+        email: UserEmail,
+    }> {
+        if (!req.body.user_id) throw new BadRequestError('Missing user_id field', 'Check your form', req.url);
+        const account = await User.select().where('id', req.body.user_id).with('mainEmail').first();
+        if (!account) throw new NotFoundHttpError('User', req.url);
+        const email = await account.mainEmail.get();
+
+        return {
+            account: account,
+            email: email!,
+        };
+    }
 }

views/backend/accounts_approval.njk

@@ -25,12 +25,18 @@
                     <td>{{ user.created_at.toISOString() }}</td>
                     <td>
                         <div class="max-content">
-                            <a href="{{ route('approve-account', user.id) }}"
+                            <form action="{{ route('approve-account') }}" method="POST">
-                               class="button success"><i data-feather="check"></i> Approve</a>
+                                <input type="hidden" name="user_id" value="{{ user.id }}">
+                                <button class="success"><i data-feather="check"></i> Approve</button>
+                                {{ macros.csrf(getCSRFToken) }}
+                            </form>
 
-                            <a href="{{ route('reject-account', user.id) }}"
+                            <form action="{{ route('reject-account') }}" method="POST"
-                               onclick="return confirm(`This will irrevocably delete the ${user.main_email} account.`)"
+                                  data-confirm="This will irrevocably delete the {{ user.mainEmail.getOrFail().email }} account.">
-                               class="button danger"><i data-feather="x"></i> Reject</a>
+                                <input type="hidden" name="user_id" value="{{ user.id }}">
+                                <button class="danger"><i data-feather="check"></i> Reject</button>
+                                {{ macros.csrf(getCSRFToken) }}
+                            </form>
                         </div>
                     </td>
                 </tr>
@@ -38,4 +44,16 @@
             </tbody>
         </table>
     </div>
+
+    <script>
+        document.addEventListener('DOMContentLoaded', () => {
+            document.querySelectorAll('form[data-confirm]').forEach(el => {
+                el.addEventListener('submit', e => {
+                    if (!confirm(el.dataset['confirm'])) {
+                        e.preventDefault();
+                    }
+                });
+            });
+        });
+    </script>
 {% endblock %}

views/mails/pending_account_review.mjml.njk

@@ -8,11 +8,11 @@
             </mj-text>
             <mj-text>
                 A new user is pending review on {{ app.name }}.
-
+                <br>
                 Username: {{ username }}
             </mj-text>
 
-            <mj-text><a href="{{ link | safe }}">Finalize my account registration</a></mj-text>
+            <mj-text><a href="{{ link | safe }}">Go to account reviews</a></mj-text>
         </mj-column>
     </mj-section>
 {% endblock %}