From 4624c926fd1afc528be6e1b1cdabb6b0c6ff2f78 Mon Sep 17 00:00:00 2001 From: sslypushenko Date: Mon, 31 Aug 2015 20:14:04 +0300 Subject: [PATCH] Improve authentication alerts If OpenStackID returns failure user will be shown failure message. Steps to check: Setup RefStack as usual but point REFSTACK_HOST=127.0.0.1. It will cause OpenStackID auth failure. So try to auth and you should get error message in popup (instead of plain json document as it was before). Change-Id: Icda5b219429047e89b393245a3a6eeadf171bd90 --- refstack-ui/app/app.js | 9 +++- .../auth-failure/authFailureController.js | 17 +++++++ .../components/results/resultsController.js | 2 +- refstack-ui/app/index.html | 1 + refstack/api/controllers/auth.py | 10 ++-- refstack/tests/unit/test_api.py | 48 +++++++++++-------- 6 files changed, 61 insertions(+), 26 deletions(-) create mode 100644 refstack-ui/app/components/auth-failure/authFailureController.js diff --git a/refstack-ui/app/app.js b/refstack-ui/app/app.js index 524deef0..6479400b 100644 --- a/refstack-ui/app/app.js +++ b/refstack-ui/app/app.js @@ -26,12 +26,12 @@ refstackApp.config([ templateUrl: '/components/capabilities/capabilities.html', controller: 'capabilitiesController' }). - state('community_results', { + state('communityResults', { url: '/community_results', templateUrl: '/components/results/results.html', controller: 'resultsController' }). - state('user_results', { + state('userResults', { url: '/user_results', templateUrl: '/components/results/results.html', controller: 'resultsController' @@ -45,6 +45,11 @@ refstackApp.config([ url: '/profile', templateUrl: '/components/profile/profile.html', controller: 'profileController' + }). + state('authFailure', { + url: '/auth_failure/:message', + templateUrl: '/components/home/home.html', + controller: 'authFailureController' }); } ]); diff --git a/refstack-ui/app/components/auth-failure/authFailureController.js b/refstack-ui/app/components/auth-failure/authFailureController.js new file mode 100644 index 00000000..597f1578 --- /dev/null +++ b/refstack-ui/app/components/auth-failure/authFailureController.js @@ -0,0 +1,17 @@ +/** + * Refstack Auth Failure Controller + * This controller handles messages from Refstack API if user auth fails. + */ + +var refstackApp = angular.module('refstackApp'); + +refstackApp.controller('authFailureController', + [ + '$stateParams', '$state', 'raiseAlert', + function($stateParams, $state, raiseAlert) { + 'use strict'; + raiseAlert('danger', 'Authentication Failure:', + $stateParams.message); + $state.go('home'); + } + ]); diff --git a/refstack-ui/app/components/results/resultsController.js b/refstack-ui/app/components/results/resultsController.js index 40a08207..cbe148fb 100644 --- a/refstack-ui/app/components/results/resultsController.js +++ b/refstack-ui/app/components/results/resultsController.js @@ -33,7 +33,7 @@ refstackApp.controller('resultsController', /** The upload date upper limit to be used in filtering results. */ $scope.endDate = ''; - $scope.isUserResults = $state.current.name === 'user_results'; + $scope.isUserResults = $state.current.name === 'userResults'; $scope.pageHeader = $scope.isUserResults ? 'Private test results' : 'Community test results'; /** diff --git a/refstack-ui/app/index.html b/refstack-ui/app/index.html index 5fed5cd7..80a75058 100644 --- a/refstack-ui/app/index.html +++ b/refstack-ui/app/index.html @@ -44,6 +44,7 @@ + diff --git a/refstack/api/controllers/auth.py b/refstack/api/controllers/auth.py index 2949017d..7894cae6 100644 --- a/refstack/api/controllers/auth.py +++ b/refstack/api/controllers/auth.py @@ -101,6 +101,10 @@ class AuthController(rest.RestController): "signout": ["GET"] } + def _auth_failure(self, message): + pecan.redirect(parse.urljoin(CONF.ui_url, + '/#/auth_failure/%s') % message) + @pecan.expose() def signin(self): """Handle signin request.""" @@ -138,17 +142,17 @@ class AuthController(rest.RestController): session = api_utils.get_user_session() if pecan.request.GET.get(const.OPENID_ERROR): api_utils.delete_params_from_user_session([const.CSRF_TOKEN]) - pecan.abort(401, pecan.request.GET.get(const.OPENID_ERROR)) + self._auth_failure(pecan.request.GET.get(const.OPENID_ERROR)) if pecan.request.GET.get(const.OPENID_MODE) == 'cancel': api_utils.delete_params_from_user_session([const.CSRF_TOKEN]) - pecan.abort(401, 'Authentication canceled.') + self._auth_failure('Authentication canceled.') session_token = session.get(const.CSRF_TOKEN) request_token = pecan.request.GET.get(const.CSRF_TOKEN) if request_token != session_token: api_utils.delete_params_from_user_session([const.CSRF_TOKEN]) - pecan.abort(401, 'Authentication is failed. Try again.') + self._auth_failure('Authentication failed. Please try again.') api_utils.verify_openid_request(pecan.request) user_info = { diff --git a/refstack/tests/unit/test_api.py b/refstack/tests/unit/test_api.py index c3be57af..0239ac97 100644 --- a/refstack/tests/unit/test_api.py +++ b/refstack/tests/unit/test_api.py @@ -412,7 +412,7 @@ class AuthControllerTestCase(BaseControllerTestCase): self.config_fixture = config_fixture.Config() self.CONF = self.useFixture(self.config_fixture).conf self.CONF.set_override('app_dev_mode', True, 'api') - self.CONF.set_override('ui_url', '127.0.0.1') + self.CONF.set_override('ui_url', 'http://127.0.0.1') @mock.patch('refstack.api.utils.get_user_session') @mock.patch('pecan.redirect', side_effect=webob.exc.HTTPRedirection) @@ -420,7 +420,7 @@ class AuthControllerTestCase(BaseControllerTestCase): mock_session = mock.MagicMock(**{const.USER_OPENID: 'foo@bar.org'}) mock_get_user_session.return_value = mock_session self.assertRaises(webob.exc.HTTPRedirection, self.controller.signin) - mock_redirect.assert_called_with('127.0.0.1') + mock_redirect.assert_called_with('http://127.0.0.1') @mock.patch('refstack.api.utils.get_user_session') @mock.patch('pecan.redirect', side_effect=webob.exc.HTTPRedirection) @@ -434,8 +434,10 @@ class AuthControllerTestCase(BaseControllerTestCase): @mock.patch('socket.gethostbyname', return_value='1.1.1.1') @mock.patch('refstack.api.utils.get_user_session') - def test_signin_return_failed(self, mock_get_user_session, mock_socket): - self.mock_abort.side_effect = webob.exc.HTTPError + @mock.patch('pecan.redirect', side_effect=webob.exc.HTTPRedirection) + def test_signin_return_failed(self, mock_redirect, + mock_get_user_session, + mock_socket): mock_session = mock.MagicMock(**{const.USER_OPENID: 'foo@bar.org', const.CSRF_TOKEN: '42'}) mock_get_user_session.return_value = mock_session @@ -447,45 +449,51 @@ class AuthControllerTestCase(BaseControllerTestCase): self.mock_request.environ['beaker.session'] = { const.CSRF_TOKEN: 42 } - self.assertRaises(webob.exc.HTTPError, self.controller.signin_return) - self.mock_abort.assert_called_once_with( - 401, self.mock_request.GET[const.OPENID_ERROR]) + self.assertRaises(webob.exc.HTTPRedirection, + self.controller.signin_return) + mock_redirect.assert_called_once_with( + 'http://127.0.0.1/#/auth_failure/foo is not bar!!!') self.assertNotIn(const.CSRF_TOKEN, self.mock_request.environ['beaker.session']) - self.mock_abort.reset_mock() + mock_redirect.reset_mock() self.mock_request.environ['beaker.session'] = { const.CSRF_TOKEN: 42 } self.mock_request.GET = { const.OPENID_MODE: 'cancel' } - self.assertRaises(webob.exc.HTTPError, self.controller.signin_return) - self.mock_abort.assert_called_once_with( - 401, 'Authentication canceled.') + self.assertRaises(webob.exc.HTTPRedirection, + self.controller.signin_return) + mock_redirect.assert_called_once_with( + 'http://127.0.0.1/#/auth_failure/Authentication canceled.') self.assertNotIn(const.CSRF_TOKEN, self.mock_request.environ['beaker.session']) - self.mock_abort.reset_mock() + mock_redirect.reset_mock() self.mock_request.environ['beaker.session'] = { const.CSRF_TOKEN: 42 } self.mock_request.GET = {} - self.assertRaises(webob.exc.HTTPError, self.controller.signin_return) - self.mock_abort.assert_called_once_with( - 401, 'Authentication is failed. Try again.') + self.assertRaises(webob.exc.HTTPRedirection, + self.controller.signin_return) + mock_redirect.assert_called_once_with( + 'http://127.0.0.1/#/auth_failure/' + 'Authentication failed. Please try again.') self.assertNotIn(const.CSRF_TOKEN, self.mock_request.environ['beaker.session']) - self.mock_abort.reset_mock() + mock_redirect.reset_mock() self.mock_request.environ['beaker.session'] = { const.CSRF_TOKEN: 42 } self.mock_request.GET = {const.CSRF_TOKEN: '24'} self.mock_request.remote_addr = '1.1.1.1' - self.assertRaises(webob.exc.HTTPError, self.controller.signin_return) - self.mock_abort.assert_called_once_with( - 401, 'Authentication is failed. Try again.') + self.assertRaises(webob.exc.HTTPRedirection, + self.controller.signin_return) + mock_redirect.assert_called_once_with( + 'http://127.0.0.1/#/auth_failure/' + 'Authentication failed. Please try again.') self.assertNotIn(const.CSRF_TOKEN, self.mock_request.environ['beaker.session']) @@ -519,7 +527,7 @@ class AuthControllerTestCase(BaseControllerTestCase): const.CSRF_TOKEN: 42 } self.assertRaises(webob.exc.HTTPRedirection, self.controller.signout) - mock_redirect.assert_called_with('127.0.0.1') + mock_redirect.assert_called_with('http://127.0.0.1') self.assertNotIn(const.CSRF_TOKEN, mock_request.environ['beaker.session'])