Skip to content

Commit

Permalink
Allow the user session lifetime to be configured via Flask. (#35)
Browse files Browse the repository at this point in the history 
Don't use the ID Token expiration time since that is unrelated to the
user sessions kept by the client.
  • Loading branch information
@zamzterz
zamzterz authored Oct 6, 2018
1 parent 42f5580 commit e410055
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 34 deletions.
6 changes: 4 additions & 2 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 3 additions & 0 deletions example/app.py
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import datetime

import flask
import logging
from flask import Flask, jsonify
Expand All @@ -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})

Expand Down
7 changes: 3 additions & 4 deletions src/flask_pyoidc/flask_pyoidc.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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'),
Expand Down
65 changes: 37 additions & 28 deletions tests/test_flask_pyoidc.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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',
Expand Down Expand Up @@ -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)()
Expand All @@ -79,15 +80,15 @@ 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()
result = authn.oidc_auth(self.PROVIDER_NAME)(view_mock)()
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()
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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'})

Expand All @@ -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'):
Expand All @@ -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)):
Expand All @@ -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)):
Expand All @@ -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),
Expand All @@ -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
Expand All @@ -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'
Expand All @@ -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
Expand All @@ -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)

0 comments on commit e410055

Please sign in to comment.