From e41005555cf1b29b01ce826d99bd6451418883c1 Mon Sep 17 00:00:00 2001 From: Samuel Gulliksson Date: Sat, 6 Oct 2018 17:43:37 +0200 Subject: [PATCH] Allow the user session lifetime to be configured via Flask. (#35) Don't use the ID Token expiration time since that is unrelated to the user sessions kept by the client. --- README.md | 6 ++- example/app.py | 3 ++ src/flask_pyoidc/flask_pyoidc.py | 7 ++-- tests/test_flask_pyoidc.py | 65 ++++++++++++++++++-------------- 4 files changed, 47 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 235d93f..3c4256a 100644 --- a/README.md +++ b/README.md @@ -79,8 +79,10 @@ The application using this extension **MUST** set the following You may also configure the way the user sessions created by this extension are handled: -* `OIDC_SESSION_PERMANENT`: If set to `True` (which is the default) the user session will live until the ID Token - expiration time. If set to `False` the session will be deleted when the user closes the browser. +* `OIDC_SESSION_PERMANENT`: If set to `True` (which is the default) the user session will be kept until the configured + session lifetime (see below). If set to `False` the session will be deleted when the user closes the browser. +* `PERMANENT_SESSION_LIFETIME`: Control how long a user session is valid, see + [Flask documentation](http://flask.pocoo.org/docs/1.0/config/#PERMANENT_SESSION_LIFETIME) for more information. ### Session refresh diff --git a/example/app.py b/example/app.py index 82b6034..ab37853 100644 --- a/example/app.py +++ b/example/app.py @@ -1,3 +1,5 @@ +import datetime + import flask import logging from flask import Flask, jsonify @@ -10,6 +12,7 @@ # See http://flask.pocoo.org/docs/0.12/config/ app.config.update({'SERVER_NAME': 'localhost:5000', 'SECRET_KEY': 'dev_key', # make sure to change this!! + 'PERMANENT_SESSION_LIFETIME': datetime.timedelta(days=7).total_seconds(), 'PREFERRED_URL_SCHEME': 'http', 'DEBUG': True}) diff --git a/src/flask_pyoidc/flask_pyoidc.py b/src/flask_pyoidc/flask_pyoidc.py index 36eccc5..684db0e 100644 --- a/src/flask_pyoidc/flask_pyoidc.py +++ b/src/flask_pyoidc/flask_pyoidc.py @@ -127,10 +127,6 @@ def _handle_authentication_response(self): raise ValueError('The \'nonce\' parameter does not match.') id_token_claims = id_token.to_dict() - # set the session as requested by the OP if we have no default - if current_app.config.get('OIDC_SESSION_PERMANENT', True): - flask.session.permanent = True - flask.session.permanent_session_lifetime = id_token['exp'] - time.time() # do userinfo request userinfo = client.userinfo_request(access_token) @@ -141,6 +137,9 @@ def _handle_authentication_response(self): if id_token_claims and userinfo_claims and userinfo_claims['sub'] != id_token_claims['sub']: raise ValueError('The \'sub\' of userinfo does not match \'sub\' of ID Token.') + if current_app.config.get('OIDC_SESSION_PERMANENT', True): + flask.session.permanent = True + UserSession(flask.session).update(access_token, id_token_claims, token_resp.get('id_token_jwt'), diff --git a/tests/test_flask_pyoidc.py b/tests/test_flask_pyoidc.py index c1d0b4c..259ad72 100644 --- a/tests/test_flask_pyoidc.py +++ b/tests/test_flask_pyoidc.py @@ -9,6 +9,7 @@ from flask import Flask from mock import MagicMock, patch from oic.oic.message import IdToken +from six.moves.http_cookies import SimpleCookie from six.moves.urllib.parse import parse_qsl, urlparse, urlencode from flask_pyoidc.flask_pyoidc import OIDCAuthentication @@ -30,7 +31,7 @@ def create_flask_app(self): self.app = Flask(__name__) self.app.config.update({'SERVER_NAME': self.CLIENT_DOMAIN, 'SECRET_KEY': 'test_key'}) - def get_authn_instance(self, provider_metadata_extras=None, client_metadata_extras=None, **kwargs): + def init_app(self, provider_metadata_extras=None, client_metadata_extras=None, **kwargs): required_provider_metadata = { 'issuer': self.PROVIDER_BASEURL, 'authorization_endpoint': self.PROVIDER_BASEURL + '/auth', @@ -70,7 +71,7 @@ def assert_view_mock(self, callback_mock, result): assert result == self.CALLBACK_RETURN_VALUE def test_should_authenticate_if_no_session(self): - authn = self.get_authn_instance() + authn = self.init_app() view_mock = self.get_view_mock() with self.app.test_request_context('/'): auth_redirect = authn.oidc_auth(self.PROVIDER_NAME)(view_mock)() @@ -79,7 +80,7 @@ def test_should_authenticate_if_no_session(self): assert not view_mock.called def test_should_not_authenticate_if_session_exists(self): - authn = self.get_authn_instance() + authn = self.init_app() view_mock = self.get_view_mock() with self.app.test_request_context('/'): UserSession(flask.session, self.PROVIDER_NAME).update() @@ -87,7 +88,7 @@ def test_should_not_authenticate_if_session_exists(self): self.assert_view_mock(view_mock, result) def test_reauthenticate_silent_if_session_expired(self): - authn = self.get_authn_instance(session_refresh_interval_seconds=1) + authn = self.init_app(session_refresh_interval_seconds=1) view_mock = self.get_view_mock() with self.app.test_request_context('/'): now = time.time() @@ -101,7 +102,7 @@ def test_reauthenticate_silent_if_session_expired(self): assert not view_mock.called def test_dont_reauthenticate_silent_if_session_not_expired(self): - authn = self.get_authn_instance(session_refresh_interval_seconds=999) + authn = self.init_app(session_refresh_interval_seconds=999) view_mock = self.get_view_mock() with self.app.test_request_context('/'): UserSession(flask.session, self.PROVIDER_NAME).update() # freshly authenticated @@ -175,8 +176,8 @@ def test_handle_authentication_response(self, time_mock, utc_time_sans_frac_mock userinfo_endpoint = self.PROVIDER_BASEURL + '/userinfo' responses.add(responses.GET, userinfo_endpoint, json=userinfo) - authn = self.get_authn_instance(provider_metadata_extras={'token_endpoint': token_endpoint, - 'userinfo_endpoint': userinfo_endpoint}) + authn = self.init_app(provider_metadata_extras={'token_endpoint': token_endpoint, + 'userinfo_endpoint': userinfo_endpoint}) state = 'test_state' with self.app.test_request_context('/redirect_uri?state={}&code=test'.format(state)): UserSession(flask.session, self.PROVIDER_NAME) @@ -193,7 +194,7 @@ def test_handle_authentication_response(self, time_mock, utc_time_sans_frac_mock @patch('time.time') @patch('oic.utils.time_util.utc_time_sans_frac') # used internally by pyoidc when verifying ID Token @responses.activate - def test_session_expiration_set_to_id_token_exp(self, time_mock, utc_time_sans_frac_mock): + def test_session_expiration_set_to_configured_lifetime(self, time_mock, utc_time_sans_frac_mock): timestamp = time.mktime(datetime(2017, 1, 1).timetuple()) time_mock.return_value = timestamp utc_time_sans_frac_mock.return_value = int(timestamp) @@ -211,19 +212,28 @@ def test_session_expiration_set_to_id_token_exp(self, time_mock, utc_time_sans_f token_endpoint = self.PROVIDER_BASEURL + '/token' responses.add(responses.POST, token_endpoint, json=token_response) - authn = self.get_authn_instance(provider_metadata_extras={'token_endpoint': token_endpoint}) - with self.app.test_request_context('/redirect_uri?state={}&code=test'.format(state)): - UserSession(flask.session, self.PROVIDER_NAME) - flask.session['destination'] = '/' - flask.session['state'] = state - flask.session['nonce'] = nonce - authn._handle_authentication_response() - assert flask.session.permanent - assert int(flask.session.permanent_session_lifetime) == exp_time + session_lifetime = 1234 + self.app.config['PERMANENT_SESSION_LIFETIME'] = session_lifetime + self.init_app(provider_metadata_extras={'token_endpoint': token_endpoint}) + + with self.app.test_client() as client: + with client.session_transaction() as session: + UserSession(session, self.PROVIDER_NAME) + session['destination'] = '/' + session['state'] = state + session['nonce'] = nonce + resp = client.get('/redirect_uri?state={}&code=test'.format(state)) + + cookies = SimpleCookie() + cookies.load(resp.headers['Set-Cookie']) + session_cookie_expiration = cookies[self.app.config['SESSION_COOKIE_NAME']]['expires'] + parsed_expiration = datetime.strptime(session_cookie_expiration, '%a, %d-%b-%Y %H:%M:%S GMT') + cookie_lifetime = (parsed_expiration - datetime.utcnow()).total_seconds() + assert cookie_lifetime == pytest.approx(session_lifetime, abs=1) def test_logout_redirects_to_provider_if_end_session_endpoint_is_configured(self): end_session_endpoint = 'https://provider.example.com/end_session' - authn = self.get_authn_instance(provider_metadata_extras={'end_session_endpoint': end_session_endpoint}) + authn = self.init_app(provider_metadata_extras={'end_session_endpoint': end_session_endpoint}) logout_view_mock = self.get_view_mock() id_token = IdToken(**{'sub': 'sub1', 'nonce': 'nonce'}) @@ -249,7 +259,7 @@ def test_logout_redirects_to_provider_if_end_session_endpoint_is_configured(self assert not logout_view_mock.called def test_logout_handles_provider_without_end_session_endpoint(self): - authn = self.get_authn_instance() + authn = self.init_app() id_token = IdToken(**{'sub': 'sub1', 'nonce': 'nonce'}) logout_view_mock = self.get_view_mock() with self.app.test_request_context('/logout'): @@ -264,7 +274,7 @@ def test_logout_handles_provider_without_end_session_endpoint(self): self.assert_view_mock(logout_view_mock, logout_result) def test_logout_handles_redirect_back_from_provider(self): - authn = self.get_authn_instance() + authn = self.init_app() logout_view_mock = self.get_view_mock() state = 'end_session_123' with self.app.test_request_context('/logout?state={}'.format(state)): @@ -275,7 +285,7 @@ def test_logout_handles_redirect_back_from_provider(self): self.assert_view_mock(logout_view_mock, result) def test_logout_handles_redirect_back_from_provider_with_incorrect_state(self, caplog): - authn = self.get_authn_instance() + authn = self.init_app() logout_view_mock = self.get_view_mock() state = 'some_state' with self.app.test_request_context('/logout?state={}'.format(state)): @@ -288,11 +298,10 @@ def test_logout_handles_redirect_back_from_provider_with_incorrect_state(self, c logging.ERROR, "Got unexpected state '{}' after logout redirect.".format(state)) - def test_authentication_error_response_calls_to_error_view_if_set(self): state = 'test_tate' error_response = {'error': 'invalid_request', 'error_description': 'test error'} - authn = self.get_authn_instance() + authn = self.init_app() error_view_mock = self.get_view_mock() authn.error_view(error_view_mock) with self.app.test_request_context('/redirect_uri?{error}&state={state}'.format(error=urlencode(error_response), @@ -307,8 +316,8 @@ def test_authentication_error_response_calls_to_error_view_if_set(self): def test_authentication_error_response_returns_default_error_if_no_error_view_set(self): state = 'test_tate' error_response = {'error': 'invalid_request', 'error_description': 'test error', 'state': state} - authn = self.get_authn_instance(dict(provider_configuration_info={'issuer': self.PROVIDER_BASEURL}, - client_registration_info=dict(client_id='abc', client_secret='foo'))) + authn = self.init_app(dict(provider_configuration_info={'issuer': self.PROVIDER_BASEURL}, + client_registration_info=dict(client_id='abc', client_secret='foo'))) with self.app.test_request_context('/redirect_uri?{}'.format(urlencode(error_response))): UserSession(flask.session, self.PROVIDER_NAME) flask.session['state'] = state @@ -321,7 +330,7 @@ def test_token_error_response_calls_to_error_view_if_set(self): error_response = {'error': 'invalid_request', 'error_description': 'test error'} responses.add(responses.POST, token_endpoint, json=error_response) - authn = self.get_authn_instance(provider_metadata_extras={'token_endpoint': token_endpoint}) + authn = self.init_app(provider_metadata_extras={'token_endpoint': token_endpoint}) error_view_mock = self.get_view_mock() authn.error_view(error_view_mock) state = 'test_tate' @@ -340,7 +349,7 @@ def test_token_error_response_returns_default_error_if_no_error_view_set(self): error_response = {'error': 'invalid_request', 'error_description': 'test error', 'state': state} responses.add(responses.POST, token_endpoint, json=error_response) - authn = self.get_authn_instance(provider_metadata_extras={'token_endpoint': token_endpoint}) + authn = self.init_app(provider_metadata_extras={'token_endpoint': token_endpoint}) with self.app.test_request_context('/redirect_uri?code=foo&state=' + state): UserSession(flask.session, self.PROVIDER_NAME) flask.session['state'] = state @@ -349,5 +358,5 @@ def test_token_error_response_returns_default_error_if_no_error_view_set(self): def test_using_unknown_provider_name_should_raise_exception(self): with pytest.raises(ValueError) as exc_info: - self.get_authn_instance().oidc_auth('unknown') + self.init_app().oidc_auth('unknown') assert 'unknown' in str(exc_info.value)