diff --git a/ckanext/oauth2/controller.py b/ckanext/oauth2/controller.py index 4c1fab8..238dff1 100644 --- a/ckanext/oauth2/controller.py +++ b/ckanext/oauth2/controller.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # Copyright (c) 2014 CoNWeT Lab., Universidad Politécnica de Madrid +# Copyright (c) 2018 Future Internet Consulting and Development Solutions S.L. # This file is part of OAuth2 CKAN Extension. @@ -25,7 +26,7 @@ import ckan.lib.base as base from ckan.common import session -from ckanext.oauth2.plugin import toolkit +from ckanext.oauth2.plugin import toolkit, _get_previous_page log = logging.getLogger(__name__) @@ -36,6 +37,19 @@ class OAuth2Controller(base.BaseController): def __init__(self): self.oauth2helper = oauth2.OAuth2Helper() + def login(self): + log.debug('login') + + # Log in attemps are fired when the user is not logged in and they click + # on the log in button + + # Get the page where the user was when the loggin attemp was fired + # When the user is not logged in, he/she should be redirected to the dashboard when + # the system cannot get the previous page + came_from_url = _get_previous_page(constants.INITIAL_PAGE) + + self.oauth2helper.challenge(came_from_url) + def callback(self): try: token = self.oauth2helper.get_token() diff --git a/ckanext/oauth2/oauth2.py b/ckanext/oauth2/oauth2.py index cf0bf91..60c5ae3 100644 --- a/ckanext/oauth2/oauth2.py +++ b/ckanext/oauth2/oauth2.py @@ -31,6 +31,7 @@ import os from base64 import b64encode, b64decode +from ckan.lib import helpers as h from ckan.plugins import toolkit from oauthlib.oauth2 import InsecureTransportError import requests @@ -85,9 +86,8 @@ def challenge(self, came_from_url): state = generate_state(came_from_url) oauth = OAuth2Session(self.client_id, redirect_uri=self.redirect_uri, scope=self.scope, state=state) auth_url, _ = oauth.authorization_url(self.authorization_endpoint) - toolkit.response.status = 302 - toolkit.response.location = auth_url log.debug('Challenge: Redirecting challenge to page {0}'.format(auth_url)) + return h.redirect_to(auth_url) def get_token(self): oauth = OAuth2Session(self.client_id, redirect_uri=self.redirect_uri, scope=self.scope) diff --git a/ckanext/oauth2/plugin.py b/ckanext/oauth2/plugin.py index 3c7fa32..37b17c5 100644 --- a/ckanext/oauth2/plugin.py +++ b/ckanext/oauth2/plugin.py @@ -27,6 +27,7 @@ from functools import partial from ckan import plugins +from ckan.common import g from ckan.plugins import toolkit from urlparse import urlparse @@ -62,6 +63,27 @@ def request_reset(context, data_dict): return _no_permissions(context, msg) +def _get_previous_page(default_page): + if 'came_from' not in toolkit.request.params: + came_from_url = toolkit.request.headers.get('Referer', default_page) + else: + came_from_url = toolkit.request.params.get('came_from', default_page) + + came_from_url_parsed = urlparse(came_from_url) + + # Avoid redirecting users to external hosts + if came_from_url_parsed.netloc != '' and came_from_url_parsed.netloc != toolkit.request.host: + came_from_url = default_page + + # When a user is being logged and REFERER == HOME or LOGOUT_PAGE + # he/she must be redirected to the dashboard + pages = ['/', '/user/logged_out_redirect'] + if came_from_url_parsed.path in pages: + came_from_url = default_page + + return came_from_url + + class OAuth2Plugin(plugins.SingletonPlugin): plugins.implements(plugins.IAuthenticator, inherit=True) @@ -78,6 +100,10 @@ def __init__(self, name=None): def before_map(self, m): log.debug('Setting up the redirections to the OAuth2 service') + m.connect('/user/login', + controller='ckanext.oauth2.controller:OAuth2Controller', + action='login') + # We need to handle petitions received to the Callback URL # since some error can arise and we need to process them m.connect('/oauth2/callback', @@ -125,45 +151,14 @@ def _refresh_and_save_token(user_name): # If we have been able to log in the user (via API or Session) if user_name: + g.user = user_name toolkit.c.user = user_name toolkit.c.usertoken = self.oauth2helper.get_stored_token(user_name) toolkit.c.usertoken_refresh = partial(_refresh_and_save_token, user_name) else: + g.user = None log.warn('The user is not currently logged...') - def _get_previous_page(self, default_page): - if 'came_from' not in toolkit.request.params: - came_from_url = toolkit.request.headers.get('Referer', default_page) - else: - came_from_url = toolkit.request.params.get('came_from', default_page) - - came_from_url_parsed = urlparse(came_from_url) - - # Avoid redirecting users to external hosts - if came_from_url_parsed.netloc != '' and came_from_url_parsed.netloc != toolkit.request.host: - came_from_url = default_page - - # When a user is being logged and REFERER == HOME or LOGOUT_PAGE - # he/she must be redirected to the dashboard - pages = ['/', '/user/logged_out_redirect'] - if came_from_url_parsed.path in pages: - came_from_url = default_page - - return came_from_url - - def login(self): - log.debug('login') - - # Log in attemps are fired when the user is not logged in and they click - # on the log in button - - # Get the page where the user was when the loggin attemp was fired - # When the user is not logged in, he/she should be redirected to the dashboard when - # the system cannot get the previous page - came_from_url = self._get_previous_page(constants.INITIAL_PAGE) - - self.oauth2helper.challenge(came_from_url) - def abort(self, status_code, detail, headers, comment): log.debug('abort') @@ -176,7 +171,7 @@ def abort(self, status_code, detail, headers, comment): if toolkit.c.user: # USER IS AUTHENTICATED # When the user is logged in, he/she should be redirected to the main page when # the system cannot get the previous page - came_from_url = self._get_previous_page('/') + came_from_url = _get_previous_page('/') # Init headers and set Location if headers is None: diff --git a/ckanext/oauth2/tests/test_controller.py b/ckanext/oauth2/tests/test_controller.py index 689268c..b4c7b8a 100644 --- a/ckanext/oauth2/tests/test_controller.py +++ b/ckanext/oauth2/tests/test_controller.py @@ -18,14 +18,16 @@ # You should have received a copy of the GNU Affero General Public License # along with OAuth2 CKAN Extension. If not, see . +from base64 import b64decode, b64encode import unittest -import ckanext.oauth2.controller as controller import json -from base64 import b64decode, b64encode from mock import MagicMock from parameterized import parameterized +from ckanext.oauth2 import controller, plugin + + RETURNED_STATUS = 302 EXAMPLE_FLASH = 'This is a test' EXCEPTION_MSG = 'Invalid' @@ -55,8 +57,9 @@ def setUp(self): self._oauth2 = controller.oauth2 controller.oauth2 = MagicMock() - self._toolkit = controller.toolkit - controller.toolkit = MagicMock() + self._toolkit_controller = controller.toolkit + self._toolkit_plugin = plugin.toolkit + plugin.toolkit = controller.toolkit = MagicMock() self.__session = controller.session controller.session = MagicMock() @@ -67,8 +70,9 @@ def tearDown(self): # Unmock the function controller.helpers = self._helpers controller.oauth2 = self._oauth2 - controller.toolkit = self._toolkit + controller.toolkit = self._toolkit_controller controller.session = self.__session + plugin.toolkit = self._toolkit_plugin def generate_state(self, url): return b64encode(bytes(json.dumps({CAME_FROM_FIELD: url}))) @@ -76,7 +80,7 @@ def generate_state(self, url): def get_came_from(self, state): return json.loads(b64decode(state)).get(CAME_FROM_FIELD, '/') - def test_controller_no_errors(self): + def test_callback_no_errors(self): oauth2Helper = controller.oauth2.OAuth2Helper.return_value token = 'TOKEN' @@ -104,7 +108,7 @@ def test_controller_no_errors(self): ('/', VoidException(), None, type(VoidException()).__name__), ('/about', Exception(EXCEPTION_MSG), EXAMPLE_FLASH, EXAMPLE_FLASH) ]) - def test_controller_errors(self, came_from=None, exception=Exception(EXCEPTION_MSG), + def test_callback_errors(self, came_from=None, exception=Exception(EXCEPTION_MSG), error_description=None, expected_flash=EXCEPTION_MSG): # Recover function @@ -127,3 +131,33 @@ def test_controller_errors(self, came_from=None, exception=Exception(EXCEPTION_M self.assertEquals(RETURNED_STATUS, controller.toolkit.response.status_int) self.assertEquals(came_from, controller.toolkit.response.location) controller.helpers.flash_error.assert_called_once_with(expected_flash) + + @parameterized.expand([ + (), + (None, None, '/dashboard'), + ('/about', None, '/about'), + ('/about', '/ckan-admin', '/ckan-admin'), + (None, '/ckan-admin', '/ckan-admin'), + ('/', None, '/dashboard'), + ('/user/logged_out_redirect', None, '/dashboard'), + ('/', '/ckan-admin', '/ckan-admin'), + ('/user/logged_out_redirect', '/ckan-admin', '/ckan-admin'), + ('http://google.es', None, '/dashboard'), + ('http://google.es', None, '/dashboard') + ]) + def test_login(self, referer=None, came_from=None, expected_referer='/dashboard'): + + # The login function will check these variables + controller.toolkit.request.headers = {} + controller.toolkit.request.params = {} + + if referer: + controller.toolkit.request.headers['Referer'] = referer + + if came_from: + controller.toolkit.request.params['came_from'] = came_from + + # Call the function + self.controller.login() + + self.controller.oauth2helper.challenge.assert_called_once_with(expected_referer) diff --git a/ckanext/oauth2/tests/test_plugin.py b/ckanext/oauth2/tests/test_plugin.py index fb4ab7b..04f3625 100644 --- a/ckanext/oauth2/tests/test_plugin.py +++ b/ckanext/oauth2/tests/test_plugin.py @@ -21,7 +21,7 @@ import unittest import ckanext.oauth2.plugin as plugin -from mock import MagicMock +from mock import MagicMock, patch from parameterized import parameterized AUTHORIZATION_HEADER = 'custom_header' @@ -110,7 +110,7 @@ def test_auth_functions(self): self.assertEquals(False, function_result['success']) @parameterized.expand([ - (), + ({}, None, None, None), ({}, None, 'test', 'test'), ({AUTHORIZATION_HEADER: 'api_key'}, 'test', None, 'test'), ({AUTHORIZATION_HEADER: 'api_key'}, 'test', 'test2', 'test'), @@ -119,7 +119,8 @@ def test_auth_functions(self): ({'invalid_header': 'api_key'}, 'test', None, None), ({'invalid_header': 'api_key'}, 'test', 'test2', 'test2'), ]) - def test_identify(self, headers={}, authenticate_result=None, identity=None, expected_user=None): + @patch("ckanext.oauth2.plugin.g") + def test_identify(self, headers, authenticate_result, identity, expected_user, g_mock): self._set_identity(identity) @@ -163,6 +164,7 @@ def authenticate_side_effect(identity): else: self.assertEquals(0, self._plugin.oauth2helper.identify.call_count) + self.assertEquals(expected_user, g_mock.user) self.assertEquals(expected_user, plugin.toolkit.c.user) if expected_user is None: @@ -176,38 +178,6 @@ def authenticate_side_effect(identity): self._plugin.oauth2helper.refresh_token.assert_called_once_with(expected_user) self.assertEquals(newtoken, plugin.toolkit.c.usertoken) - @parameterized.expand([ - (), - (None, None, '/dashboard'), - ('/about', None, '/about'), - ('/about', '/ckan-admin', '/ckan-admin'), - (None, '/ckan-admin', '/ckan-admin'), - ('/', None, '/dashboard'), - ('/user/logged_out_redirect', None, '/dashboard'), - ('/', '/ckan-admin', '/ckan-admin'), - ('/user/logged_out_redirect', '/ckan-admin', '/ckan-admin'), - ('http://google.es', None, '/dashboard'), - ('http://google.es', None, '/dashboard') - ]) - def test_login(self, referer=None, came_from=None, expected_referer='/dashboard'): - - # The login function will check these variables - plugin.toolkit.request.headers = {} - plugin.toolkit.request.params = {} - - self._plugin.oauth2helper.challenge = MagicMock() - - if referer: - plugin.toolkit.request.headers['Referer'] = referer - - if came_from: - plugin.toolkit.request.params['came_from'] = came_from - - # Call the function - self._plugin.login() - - self._plugin.oauth2helper.challenge.assert_called_once_with(expected_referer) - @parameterized.expand([ (), ('user', None, None, None, '/'),