Add a basic behat test

.travis.yml

@@ -1,35 +1,55 @@
 language: php
 
+before_install:
+  - sudo apt-get update
+  - sudo apt-get install chromium-chromedriver
+
 dist: trusty
 
 env:
   global:
     - TRAVIS_NODE_VERSION="10"
+    - DISPLAY=":99"
+    - XVFBARGS=":99 -ac -screen 0 1024x768x16"
+    - SS_BASE_URL="http://localhost:8080/"
+    - SS_ENVIRONMENT_TYPE="dev"
 
 matrix:
   include:
-    - php: 7.1
+    - php: '7.1'
       env: DB=MYSQL RECIPE_VERSION=4.2.x-dev PHPUNIT_TEST=1 PHPCS_TEST=1
-    - php: 7.1
+    - php: '7.1'
       env: DB=PGSQL RECIPE_VERSION=4.3.x-dev PHPUNIT_COVERAGE_TEST=1
-    - php: 7.2
+    - php: '7.2'
       env: DB=MYSQL RECIPE_VERSION=4.4.x-dev PHPUNIT_TEST=1
-    - php: 7.3
+    - php: '7.3'
+      env: DB=MYSQL RECIPE_VERSION=4.4.x-dev BEHAT_TEST=1
+    - php: '7.3'
       env: DB=MYSQL RECIPE_VERSION=4.3.x-dev NPM_TEST=1
-    - php: 7.3
+    - php: '7.3'
       env: DB=MYSQL RECIPE_VERSION=4.x-dev PHPUNIT_TEST=1
 
 before_script:
+  # Extra $PATH
+  - export PATH=/usr/lib/chromium-browser/:$PATH
+
   # Init PHP
   - phpenv rehash
   - phpenv config-rm xdebug.ini || true
 
   # Install composer dependencies
   - composer validate
   - if [[ $DB == PGSQL ]]; then composer require silverstripe/postgresql:2.1.x-dev --no-update; fi
-  - composer require silverstripe/recipe-cms "$RECIPE_VERSION" --no-update
+  - composer require --no-update silverstripe/recipe-cms:"$RECIPE_VERSION" silverstripe/recipe-testing:^1
   - composer install --prefer-dist --no-interaction --no-progress --no-suggest --optimize-autoloader --verbose --profile
 
+  # Behat bootstrapping
+  - if [[ $BEHAT_TEST ]]; then mkdir artifacts; fi
+  - if [[ $BEHAT_TEST ]]; then cp composer.lock artifacts/; fi
+  - if [[ $BEHAT_TEST ]]; then sh -e /etc/init.d/xvfb start; sleep 3; fi
+  - if [[ $BEHAT_TEST ]]; then (chromedriver > artifacts/chromedriver.log 2>&1 &); fi
+  - if [[ $BEHAT_TEST ]]; then (vendor/bin/serve --bootstrap-file vendor/silverstripe/cms/tests/behat/serve-bootstrap.php &> artifacts/serve.log &); fi
+
   # Install NPM dependencies
   - if [[ $NPM_TEST ]]; then nvm install $TRAVIS_NODE_VERSION && nvm use $TRAVIS_NODE_VERSION && npm install -g yarn && yarn install --network-concurrency 1 && yarn run build; fi
 
@@ -41,7 +61,11 @@ script:
   - if [[ $NPM_TEST ]]; then git diff --name-status --relative=client; fi
   - if [[ $NPM_TEST ]]; then yarn run coverage; fi
   - if [[ $NPM_TEST ]]; then yarn run lint; fi
+  - if [[ $BEHAT_TEST ]]; then vendor/bin/behat @mfa; fi
 
 after_success:
   - if [[ $PHPUNIT_COVERAGE_TEST ]]; then bash <(curl -s https://codecov.io/bash) -f coverage.xml -F php; fi
   - if [[ $NPM_TEST ]]; then bash <(curl -s https://codecov.io/bash) -F js; fi
+
+after_failure:
+  - if [[ $BEHAT_TEST ]]; then php ./vendor/silverstripe/framework/tests/behat/travis-upload-artifacts.php --if-env BEHAT_TEST,ARTIFACTS_BUCKET,ARTIFACTS_KEY,ARTIFACTS_SECRET --target-path $TRAVIS_REPO_SLUG/$TRAVIS_BUILD_ID/$TRAVIS_JOB_ID --artifacts-base-url https://s3.amazonaws.com/$ARTIFACTS_BUCKET/ --artifacts-path ./artifacts/; fi

behat.yml

@@ -0,0 +1,31 @@
+# Run mfa behat tests with this command
+# Note that mfa behat tests require CMS module
+# ========================================================================= #
+# chromedriver
+# vendor/bin/behat @mfa
+# ========================================================================= #
+default:
+  suites:
+    mfa:
+      paths:
+        - %paths.modules.mfa%/tests/Behat/features
+      contexts:
+        - SilverStripe\Framework\Tests\Behaviour\FeatureContext
+        - SilverStripe\Framework\Tests\Behaviour\CmsFormsContext
+        - SilverStripe\Framework\Tests\Behaviour\CmsUiContext
+        - SilverStripe\BehatExtension\Context\BasicContext
+        - SilverStripe\BehatExtension\Context\EmailContext
+        - SilverStripe\MFA\Tests\Behat\Context\LoginContext
+        - SilverStripe\CMS\Tests\Behaviour\ThemeContext
+  extensions:
+    SilverStripe\BehatExtension\MinkExtension:
+      default_session: facebook_web_driver
+      javascript_session: facebook_web_driver
+      facebook_web_driver:
+        browser: chrome
+        wd_host: "http://127.0.0.1:9515" #chromedriver port
+      browser_name: chrome
+    SilverStripe\BehatExtension\Extension:
+      bootstrap_file: vendor/silverstripe/cms/tests/behat/serve-bootstrap.php
+      screenshot_path: %paths.base%/artifacts/screenshots
+      retry_seconds: 4 # default is 2

client/src/bundles/bundle-cms.scss

@@ -1,5 +1,5 @@
 // Core variables
-@import "variables";
+@import 'variables';
 
 @import '../styles/mfa-variables';
 @import '../styles/mfa';

client/src/components/BackupCodes/Verify.js

@@ -63,7 +63,7 @@ class Verify extends Component {
       <ul className="mfa-action-list">
         <li className="mfa-action-list__item">
           <button
-            className="btn btn-success"
+            className="btn btn-primary"
             disabled={this.state.value.length === 0}
             onClick={this.handleCompleteVerification}
           >

client/src/components/LoadingError.js

@@ -0,0 +1,33 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import CircleWarning from 'components/Icons/CircleWarning';
+
+/**
+ * Renders an error screen, used for when a schema fetch fails in the
+ * MFA app, or if a method is unavailable to be used but is already
+ * selected in the verification process.
+ */
+const LoadingError = ({ title, message, controls }) => (
+  <div className="mfa-method mfa-method--unavailable">
+    <div className="mfa-method-icon mfa-method-icon--unavailable">
+      <CircleWarning size="80px" />
+    </div>
+
+    <h2 className="mfa-method-title mfa-method-title--unavailable">
+      {title}
+    </h2>
+    {message && <p>{message}</p>}
+
+    <div className="mfa-method-options">
+      {controls}
+    </div>
+  </div>
+);
+
+LoadingError.propTypes = {
+  title: PropTypes.string.isRequired,
+  message: PropTypes.string,
+  controls: PropTypes.oneOfType([PropTypes.node, PropTypes.func]),
+};
+
+export default LoadingError;

client/src/components/Register/SelectMethod.js

@@ -82,7 +82,7 @@ class SelectMethod extends Component {
       <ul className="mfa-action-list">
         <li className="mfa-action-list__item">
           <button
-            className="btn btn-success"
+            className="btn btn-primary"
             disabled={highlightedMethod === null}
             onClick={this.handleGoToNext}
           >

client/src/components/Verify.js

@@ -8,7 +8,7 @@ import registeredMethodType from 'types/registeredMethod';
 import LoadingIndicator from 'components/LoadingIndicator';
 import SelectMethod from 'components/Verify/SelectMethod';
 import withMethodAvailability from 'state/methodAvailability/withMethodAvailability';
-import CircleWarning from 'components/Icons/CircleWarning';
+import LoadingError from 'components/LoadingError';
 
 class Verify extends Component {
   constructor(props) {
@@ -291,20 +291,16 @@ class Verify extends Component {
     if (isAvailable && !isAvailable(selectedMethod)) {
       const unavailableMessage = getUnavailableMessage(selectedMethod);
       return (
-        <div className="mfa-method mfa-method--unavailable">
-          <div className="mfa-method-icon mfa-method-icon--unavailable">
-            <CircleWarning size="80px" />
-          </div>
-
-          <h2 className="mfa-method-title mfa-method-title--unavailable">
-            { i18n._t('MFAVerify.METHOD_UNAVAILABLE', 'This authentication method is unavailable') }
-          </h2>
-          {unavailableMessage && <p>{unavailableMessage}</p>}
-
-          <div className="mfa-method-options">
-            {this.renderOtherMethodsControl('btn-outline-secondary')}
-          </div>
-        </div>
+        <LoadingError
+          title={
+            i18n._t(
+              'MFAVerify.METHOD_UNAVAILABLE',
+              'This authentication method is unavailable'
+            )
+          }
+          message={unavailableMessage}
+          controls={this.renderOtherMethodsControl('btn-outline-secondary')}
+        />
       );
     }
 

client/src/components/tests/Verify-test.js

@@ -9,6 +9,7 @@ import Enzyme, { shallow } from 'enzyme';
 import Adapter from 'enzyme-adapter-react-16';
 import { Component as Verify } from '../Verify';
 import SelectMethod from '../Verify/SelectMethod';
+import LoadingError from 'components/LoadingError';
 import { loadComponent } from 'lib/Injector'; // eslint-disable-line
 
 Enzyme.configure({ adapter: new Adapter() });
@@ -329,8 +330,7 @@ describe('Verify', () => {
           selectedMethod: mockRegisteredMethods[0],
         });
 
-        expect(wrapper.find('.mfa-method--unavailable')).toHaveLength(1);
-        expect(wrapper.text()).toContain('There is no spoon');
+        expect(wrapper.find(LoadingError)).toHaveLength(1);
         done();
       });
     });

client/src/containers/Login.js

@@ -5,6 +5,7 @@ import PropTypes from 'prop-types';
 import Verify from 'components/Verify';
 import Register from 'components/Register';
 import LoadingIndicator from 'components/LoadingIndicator';
+import LoadingError from 'components/LoadingError';
 import { chooseMethod, setAvailableMethods } from 'state/mfaRegister/actions';
 import { setAllMethods } from 'state/mfaVerify/actions';
 import { connect } from 'react-redux';
@@ -37,13 +38,22 @@ class Login extends Component {
     const { schemaURL, onSetAllMethods } = this.props;
 
     return fetch(schemaURL)
-      .then(response => response.json())
+      .then(response => {
+        if (response.status !== 200) {
+          this.setState({
+            schemaLoaded: true, // Triggers an error state - see render()
+          });
+          return Promise.reject();
+        }
+        return response.json();
+      })
       .then(schemaData => {
         this.setState({
           schema: schemaData
         });
         onSetAllMethods(schemaData.allMethods);
-      });
+      })
+      .catch(() => {}); // noop
   }
 
   componentDidUpdate(prevProps, prevState) {
@@ -158,10 +168,24 @@ class Login extends Component {
 
   render() {
     const { schema, schemaLoaded, loading } = this.state;
+    const { ss: { i18n } } = window;
 
     if (!schema || loading) {
       if (!schema && schemaLoaded) {
-        throw new Error('Could not read configuration schema to load MFA interface');
+        return (
+          <LoadingError
+            title={i18n._t('MFALogin.SOMETHING_WENT_WRONG', 'Something went wrong!')}
+            controls={
+              <button
+                type="button"
+                onClick={() => window.location.reload()}
+                className="btn btn-outline-secondary"
+              >
+                {i18n._t('MFALogin.TRY_AGAIN', 'Try again')}
+              </button>
+            }
+          />
+        );
       }
 
       return <LoadingIndicator block />;
@@ -177,7 +201,7 @@ class Login extends Component {
 }
 
 Login.propTypes = {
-  schemaURL: PropTypes.string
+  schemaURL: PropTypes.string.isRequired,
 };
 
 const mapDispatchToProps = dispatch => ({

client/src/containers/tests/Login-test.js

@@ -0,0 +1,80 @@
+/* global jest, describe, it, expect */
+
+jest.mock('lib/Injector');
+
+// eslint-disable-next-line no-unused-vars
+import fetch from 'isomorphic-fetch';
+import React from 'react';
+import Enzyme, { shallow } from 'enzyme';
+import Adapter from 'enzyme-adapter-react-16';
+import LoadingError from 'components/LoadingError';
+import { Component as Login } from '../Login';
+
+Enzyme.configure({ adapter: new Adapter() });
+
+window.ss = {
+  i18n: { _t: (key, string) => string },
+};
+
+const fetchMock = jest.spyOn(global, 'fetch');
+
+describe('Login', () => {
+  beforeEach(() => {
+    fetchMock.mockClear();
+  });
+
+  describe('componentDidMount()', () => {
+    it('handles schema fetch errors', done => {
+      fetchMock.mockImplementation(() => Promise.resolve({
+        status: 500,
+      }));
+
+      const wrapper = shallow(
+        <Login schemaURL="/foo" />
+      );
+
+      setTimeout(() => {
+        expect(wrapper.instance().state.schemaLoaded).toBe(true);
+        done();
+      });
+    });
+
+    it('handles successful schema fetch', done => {
+      fetchMock.mockImplementation(() => Promise.resolve({
+        status: 200,
+        json: () => Promise.resolve({
+          schemaData: { allMethods: [] },
+        }),
+      }));
+
+      const wrapper = shallow(
+        <Login schemaURL="/foo" />
+      );
+
+      setTimeout(() => {
+        expect(wrapper.instance().state.schema).toEqual({
+          schemaData: { allMethods: [] },
+        });
+        done();
+      });
+    });
+  });
+
+  describe('render()', () => {
+    it('renders an error screen', done => {
+      const wrapper = shallow(
+        <Login schemaURL="/foo" />,
+        { disableLifecycleMethods: true }
+      );
+
+      wrapper.instance().setState({
+        loading: true,
+        schema: null,
+        schemaLoaded: true,
+      }, () => {
+        expect(wrapper.find(LoadingError)).toHaveLength(1);
+        done();
+      });
+    });
+  });
+});

composer.json

@@ -48,7 +48,8 @@
     "autoload": {
         "psr-4": {
             "SilverStripe\\MFA\\": "src/",
-            "SilverStripe\\MFA\\Tests\\": "tests/php/"
+            "SilverStripe\\MFA\\Tests\\": "tests/php/",
+            "SilverStripe\\MFA\\Tests\\Behat\\": "tests/Behat/"
         }
     },
     "support": {

src/Service/EnforcementManager.php

@@ -100,7 +100,10 @@ public function shouldRedirectToMFA(Member $member): bool
             return false;
         }
 
-        if (empty(MethodRegistry::singleton()->getMethods())) {
+        $methodRegistry = MethodRegistry::singleton();
+        $methods = $methodRegistry->getMethods();
+        // If there are no methods available excluding backup codes, do not redirect
+        if (!count($methods) || (count($methods) === 1 && $methodRegistry->getBackupMethod() !== null)) {
             return false;
         }