Added explicit 2fa required error detection

no issue

- previously we determined any 403 response was an indication that we should switch to the 2fa input screen during sign-in
- added a custom error that explicitly looks for an error with our `2FA_TOKEN_REQUIRED` code so we don't have any confusion when a non-2fa 403 is received for any reason and to have the option of moving away from the 403 if needed without breaking the client
- test to ensure our error 2fa-required error detection works correctly
- extracted duplicate steps in the authentication tests into a helper function
- fixed authentication tests so they better represent our API output of `errors` being an array

ghost/admin/app/controllers/signin.js

@@ -4,7 +4,7 @@ import {action} from '@ember/object';
 import {htmlSafe} from '@ember/template';
 import {inject} from 'ghost-admin/decorators/inject';
 import {isArray as isEmberArray} from '@ember/array';
-import {isForbiddenError} from 'ember-ajax/errors';
+import {isTwoFactorTokenRequiredError} from '../services/ajax';
 import {isVersionMismatchError} from 'ghost-admin/services/ajax';
 import {inject as service} from '@ember/service';
 import {task} from 'ember-concurrency';
@@ -57,10 +57,10 @@ export default class SigninController extends Controller.extend(ValidationEngine
             yield this.session.authenticate(authStrategy, {identification, password});
             return SUCCESS;
         } catch (error) {
-            if (isForbiddenError(error)) {
+            if (isTwoFactorTokenRequiredError(error)) {
                 // login was successful, but 2FA verification is required
                 this.router.transitionTo('signin-verify');
-                return true;
+                return SUCCESS;
             }
 
             if (isVersionMismatchError(error)) {

ghost/admin/app/services/ajax.js

@@ -178,6 +178,33 @@ export function isEmailError(errorOrStatus, payload) {
     }
 }
 
+/* 2FA required error */
+export class TwoFactorTokenRequiredError extends AjaxError {
+    constructor(payload) {
+        super(payload, '2nd factor verification is required to sign in.');
+    }
+}
+
+export function isTwoFactorTokenRequiredError(errorOrStatus, payload) {
+    const tokenRequiredCode = '2FA_TOKEN_REQUIRED';
+
+    // ember-simple-auth prevents ember-ajax parsing response as JSON but
+    // we need a JSON object to test against
+    if (typeof payload === 'string') {
+        try {
+            payload = JSON.parse(payload);
+        } catch (e) {
+            // do nothing
+        }
+    }
+
+    if (isAjaxError(errorOrStatus)) {
+        return errorOrStatus instanceof TwoFactorTokenRequiredError || getErrorCode(errorOrStatus) === tokenRequiredCode;
+    } else {
+        return get(payload || {}, 'errors.firstObject.code') === tokenRequiredCode;
+    }
+}
+
 /* end: custom error types */
 
 export class AcceptedResponse {
@@ -318,7 +345,9 @@ class ajaxService extends AjaxService {
             }
         }
 
-        if (this.isVersionMismatchError(status, headers, payload)) {
+        if (this.isTwoFactorTokenRequiredError(status, headers, payload)) {
+            return new TwoFactorTokenRequiredError(payload);
+        } else if (this.isVersionMismatchError(status, headers, payload)) {
             return new VersionMismatchError(payload);
         } else if (this.isServerUnreachableError(status, headers, payload)) {
             return new ServerUnreachableError(payload);
@@ -378,6 +407,10 @@ class ajaxService extends AjaxService {
         return super.normalizeErrorResponse(status, headers, payload);
     }
 
+    isTwoFactorTokenRequiredError(status, headers, payload) {
+        return isTwoFactorTokenRequiredError(status, payload);
+    }
+
     isVersionMismatchError(status, headers, payload) {
         return isVersionMismatchError(status, payload);
     }

ghost/admin/tests/acceptance/authentication-test.js

@@ -36,6 +36,14 @@ describe('Acceptance: Authentication', function () {
     describe('general page', function () {
         let newLocation;
 
+        async function completeSignIn() {
+            await invalidateSession();
+            await visit('/signin');
+            await fillIn('[data-test-input="email"]', 'my@email.com');
+            await fillIn('[data-test-input="password"]', 'password');
+            await click('[data-test-button="sign-in"]');
+        }
+
         beforeEach(function () {
             originalReplaceLocation = windowProxy.replaceLocation;
             windowProxy.replaceLocation = function (url) {
@@ -121,9 +129,9 @@ describe('Acceptance: Authentication', function () {
         it('has 2fa code happy path', async function () {
             this.server.post('/session', function () {
                 return new Response(403, {}, {
-                    errors: {
+                    errors: [{
                         code: '2FA_TOKEN_REQUIRED'
-                    }
+                    }]
                 });
             });
 
@@ -131,13 +139,9 @@ describe('Acceptance: Authentication', function () {
                 return new Response(201);
             });
 
-            await invalidateSession();
-            await visit('/signin');
-            await fillIn('[data-test-input="email"]', 'my@email.com');
-            await fillIn('[data-test-input="password"]', 'password');
-            await click('[data-test-button="sign-in"]');
+            await completeSignIn();
 
-            expect(currentURL(), 'url after u+p submit').to.equal('/signin/verify');
+            expect(currentURL(), 'url after email+password submit').to.equal('/signin/verify');
 
             await fillIn('[data-test-input="token"]', 123456);
             await click('[data-test-button="verify"]');
@@ -148,31 +152,56 @@ describe('Acceptance: Authentication', function () {
         it('handles 2fa code verification errors', async function () {
             this.server.post('/session', function () {
                 return new Response(403, {}, {
-                    errors: {
+                    errors: [{
                         code: '2FA_TOKEN_REQUIRED'
-                    }
+                    }]
                 });
             });
 
             this.server.put('/session/verify', function () {
                 return new Response(401, {}, {
-                    errors: {
+                    errors: [{
                         message: 'Invalid or expired token'
-                    }
+                    }]
                 });
             });
 
-            await invalidateSession();
-            await visit('/signin');
-            await fillIn('[data-test-input="email"]', 'my@email.com');
-            await fillIn('[data-test-input="password"]', 'password');
-            await click('[data-test-button="sign-in"]');
+            await completeSignIn();
 
             await fillIn('[data-test-input="token"]', 123456);
             await click('[data-test-button="verify"]');
 
             expect(find('[data-test-flow-notification]')).to.have.trimmed.text('Invalid or expired token');
         });
+
+        it('handles 2fa-required on a 2xx response', async function () {
+            this.server.post('/session', function () {
+                return new Response(200, {}, {
+                    errors: [{
+                        code: '2FA_TOKEN_REQUIRED'
+                    }]
+                });
+            });
+
+            await completeSignIn();
+
+            expect(currentURL(), 'url after email+password submit').to.equal('/signin/verify');
+        });
+
+        it('handles non-2fa 403 response', async function () {
+            this.server.post('/session', function () {
+                return new Response(403, {}, {
+                    errors: [{
+                        message: 'Insufficient permissions'
+                    }]
+                });
+            });
+
+            await completeSignIn();
+
+            expect(currentURL(), 'url after email+password submit').to.equal('/signin');
+            expect(find('[data-test-flow-notification]')).to.have.trimmed.text('Insufficient permissions');
+        });
     });
 
     describe('editor', function () {