Skip to content

Commit

Permalink
Code cleanup: fix linting errors reported by Pylint. (#152)
Browse files Browse the repository at this point in the history
  • Loading branch information
zamzterz authored Feb 10, 2023
1 parent 176482e commit 3724019
Show file tree
Hide file tree
Showing 10 changed files with 52 additions and 53 deletions.
4 changes: 2 additions & 2 deletions src/flask_pyoidc/auth_response_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ def process_auth_response(self, auth_response, auth_request):

try:
self._client.verify_id_token(id_token, auth_request)
except PyoidcError as e:
raise InvalidIdTokenError(str(e))
except PyoidcError as ex:
raise InvalidIdTokenError(str(ex)) from ex

id_token_claims = id_token.to_dict()
id_token_jwt = token_resp.get('id_token_jwt')
Expand Down
19 changes: 9 additions & 10 deletions src/flask_pyoidc/flask_pyoidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,10 @@ def _handle_authentication_response(self):

try:
result = AuthResponseHandler(client).process_auth_response(authn_resp, auth_request)
except AuthResponseErrorResponseError as e:
return self._handle_error_response(e.error_response, is_processing_fragment_encoded_response)
except AuthResponseProcessError as e:
return self._handle_error_response({'error': 'unexpected_error', 'error_description': str(e)},
except AuthResponseErrorResponseError as ex:
return self._handle_error_response(ex.error_response, is_processing_fragment_encoded_response)
except AuthResponseProcessError as ex:
return self._handle_error_response({'error': 'unexpected_error', 'error_description': str(ex)},
is_processing_fragment_encoded_response)

if current_app.config.get('OIDC_SESSION_PERMANENT', True):
Expand Down Expand Up @@ -208,8 +208,7 @@ def oidc_auth(self, provider_name: str):

if provider_name not in self._provider_configurations:
raise ValueError(
"Provider name '{}' not in configured providers: {}.".format(provider_name,
self._provider_configurations.keys())
f"Provider name '{provider_name}' not in configured providers: {self._provider_configurations.keys()}."
)

def oidc_decorator(view_func):
Expand Down Expand Up @@ -238,7 +237,7 @@ def _logout(self, post_logout_redirect_uri):
logger.debug('user logout')
try:
session = UserSession(flask.session)
except UninitialisedSession as e:
except UninitialisedSession:
logger.info('user was already logged out, doing nothing')
return None

Expand Down Expand Up @@ -390,19 +389,19 @@ def introspect_token(self, request, client, scopes: list = None) ->\
logger.debug(result)
# Check if access_token is valid, active can be True or False
if not result.get('active'):
return
return None
# Check if client_id is in audience claim
if client._client.client_id not in result['aud']:
# log the exception if client_id is not in audience and returns
# False, you can configure audience with Identity Provider
logger.info('Token is valid but required audience is missing.')
return
return None
# Check if the scopes associated with the access_token are the ones
# required by the endpoint and not something else which is not
# permitted.
if scopes and not set(scopes).issubset(set(result['scope'])):
logger.info('Token is valid but does not have required scopes.')
return
return None
return result

def token_auth(self, provider_name, scopes_required: list = None):
Expand Down
4 changes: 2 additions & 2 deletions src/flask_pyoidc/provider_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def __init__(self, *args, **kwargs):
args (List[Tuple[String, String]]): key-value pairs to store
kwargs (Dict[string, string]): key-value pairs to store
"""
self.store = dict()
self.store = {}
self.update(dict(*args, **kwargs))

def __getitem__(self, key):
Expand Down Expand Up @@ -109,7 +109,7 @@ def __init__(self, client_id=None, client_secret=None, **kwargs):
the OP
kwargs (dict): key-value pairs
"""
super(ClientMetadata, self).__init__(client_id=client_id, client_secret=client_secret, **kwargs)
super().__init__(client_id=client_id, client_secret=client_secret, **kwargs)


class ProviderConfiguration:
Expand Down
1 change: 0 additions & 1 deletion src/flask_pyoidc/pyoidc_facade.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import base64
import logging

from oic.extension.client import Client as ClientExtension
Expand Down
4 changes: 2 additions & 2 deletions tests/test_auth_response_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def test_should_detect_state_mismatch(self, client_mock):
with pytest.raises(AuthResponseUnexpectedStateError):
AuthResponseHandler(client_mock).process_auth_response(self.AUTH_RESPONSE, auth_request)

def test_should_detect_nonce_mismatch(self, client_mock):
def test_should_detect_nonce_mismatch(self):
client = PyoidcFacade(
ProviderConfiguration(provider_metadata=ProviderMetadata(issuer=self.ISSUER),
client_metadata=ClientMetadata(client_id=self.CLIENT_ID)),
Expand Down Expand Up @@ -93,7 +93,7 @@ def test_should_handle_auth_response_without_authorization_code(self, client_moc
assert result.id_token_jwt == self.TOKEN_RESPONSE['id_token_jwt']
assert result.id_token_claims == self.TOKEN_RESPONSE['id_token'].to_dict()
assert result.userinfo_claims == self.USERINFO_RESPONSE.to_dict()
assert result.refresh_token == None
assert result.refresh_token is None

def test_should_handle_token_response_without_id_token(self, client_mock):
token_response = {'access_token': 'test_token'}
Expand Down
55 changes: 29 additions & 26 deletions tests/test_flask_pyoidc.py
Original file line number Diff line number Diff line change
@@ -1,25 +1,24 @@
import json
import logging

import flask
import pytest
import responses
import time
from datetime import datetime
from flask import Flask
from flask_pyoidc.redirect_uri_config import RedirectUriConfig
from http.cookies import SimpleCookie
from jwkest import jws
from oic.oic import AuthorizationResponse
from oic.oic.message import IdToken
from unittest.mock import MagicMock, patch
from urllib.parse import parse_qsl, urlparse, urlencode
from werkzeug.exceptions import Forbidden, Unauthorized

import flask
import pytest
import responses
from flask import Flask
from flask_pyoidc import OIDCAuthentication
from flask_pyoidc.provider_configuration import (ProviderConfiguration, ProviderMetadata, ClientMetadata,
ClientRegistrationInfo)
from flask_pyoidc.redirect_uri_config import RedirectUriConfig
from flask_pyoidc.user_session import UserSession
from jwkest import jws
from oic.oic import AuthorizationResponse
from oic.oic.message import IdToken
from werkzeug.exceptions import Forbidden, Unauthorized
from werkzeug.routing import BuildError

from .util import signed_id_token
Expand Down Expand Up @@ -302,7 +301,7 @@ def test_handle_authentication_response(self, time_mock, utc_time_sans_frac_mock
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)):
with self.app.test_request_context(f'/redirect_uri?state={state}&code=test'):
UserSession(flask.session, self.PROVIDER_NAME)
flask.session['destination'] = '/'
flask.session['auth_request'] = json.dumps({'state': state, 'nonce': nonce})
Expand Down Expand Up @@ -348,7 +347,7 @@ def test_handle_implicit_authentication_response(self, time_mock, utc_time_sans_
userinfo_endpoint = self.PROVIDER_BASEURL + '/userinfo'
responses.add(responses.GET, userinfo_endpoint, json=userinfo)

authn = self.init_app(provider_metadata_extras={'userinfo_endpoint': userinfo_endpoint})
self.init_app(provider_metadata_extras={'userinfo_endpoint': userinfo_endpoint})
state = 'test_state'
auth_response = AuthorizationResponse(
**{'state': state, 'access_token': access_token, 'token_type': 'Bearer', 'id_token': id_token_jwt})
Expand All @@ -359,7 +358,7 @@ def test_handle_implicit_authentication_response(self, time_mock, utc_time_sans_
session['destination'] = '/'
session['auth_request'] = json.dumps({'state': state, 'nonce': nonce})
session['fragment_encoded_response'] = True
client.get('/redirect_uri#{}'.format(auth_response.to_urlencoded()))
client.get(f'/redirect_uri#{auth_response.to_urlencoded()}')
assert 'auth_request' in session # stored auth_request should not have been removed yet

# fake the POST request from the 'parse_fragment.html' template
Expand Down Expand Up @@ -419,7 +418,8 @@ def test_handle_authentication_response_without_initialised_session(self):
authn.error_view(error_view_mock)
result = authn._handle_authentication_response()
self.assert_view_mock(error_view_mock, result)
error_view_mock.assert_called_with(**{'error': 'unsolicited_response', 'error_description': 'No initialised user session.'})
error_view_mock.assert_called_with(
**{'error': 'unsolicited_response', 'error_description': 'No initialised user session.'})

def test_handle_authentication_response_without_stored_auth_request(self):
authn = self.init_app()
Expand All @@ -435,7 +435,8 @@ def test_handle_authentication_response_without_stored_auth_request(self):
authn.error_view(error_view_mock)
result = authn._handle_authentication_response()
self.assert_view_mock(error_view_mock, result)
error_view_mock.assert_called_with(**{'error': 'unsolicited_response', 'error_description': 'No authentication request stored.'})
error_view_mock.assert_called_with(
**{'error': 'unsolicited_response', 'error_description': 'No authentication request stored.'})

def test_handle_authentication_response_fragment_encoded(self):
authn = self.init_app()
Expand Down Expand Up @@ -489,7 +490,7 @@ def test_session_expiration_set_to_configured_lifetime(self, time_mock, utc_time
UserSession(session, self.PROVIDER_NAME)
session['destination'] = '/'
session['auth_request'] = json.dumps({'state': state, 'nonce': nonce, 'response_type': 'code'})
resp = client.get('/redirect_uri?state={}&code=test'.format(state))
resp = client.get(f'/redirect_uri?state={state}&code=test')

cookies = SimpleCookie()
cookies.load(resp.headers['Set-Cookie'])
Expand Down Expand Up @@ -526,7 +527,7 @@ def test_logout_redirects_to_provider_if_end_session_endpoint_is_configured(self
assert end_session_redirect.location.startswith(end_session_endpoint)
assert IdToken().from_jwt(parsed_request['id_token_hint']) == id_token

assert parsed_request['post_logout_redirect_uri'] == 'http://{}/logout'.format(self.CLIENT_DOMAIN)
assert parsed_request['post_logout_redirect_uri'] == f'http://{self.CLIENT_DOMAIN}/logout'
assert not logout_view_mock.called

@responses.activate
Expand Down Expand Up @@ -632,7 +633,7 @@ def test_logout_handles_redirect_back_from_provider(self):
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)):
with self.app.test_request_context(f'/logout?state={state}'):
flask.session['end_session_state'] = state
result = authn.oidc_logout(logout_view_mock)()
assert 'end_session_state' not in flask.session
Expand All @@ -643,15 +644,15 @@ def test_logout_handles_redirect_back_from_provider_with_incorrect_state(self, c
authn = self.init_app()
logout_view_mock = self.get_view_mock()
state = 'some_state'
with self.app.test_request_context('/logout?state={}'.format(state)):
with self.app.test_request_context(f'/logout?state={state}'):
flask.session['end_session_state'] = 'other_state'
result = authn.oidc_logout(logout_view_mock)()
assert 'end_session_state' not in flask.session

self.assert_view_mock(logout_view_mock, result)
assert caplog.record_tuples[-1] == ('flask_pyoidc.flask_pyoidc',
logging.ERROR,
"Got unexpected state '{}' after logout redirect.".format(state))
f"Got unexpected state '{state}' after logout redirect.")

def test_logout_handles_no_user_session(self):
authn = self.init_app()
Expand All @@ -667,8 +668,7 @@ def test_authentication_error_response_calls_to_error_view_if_set(self):
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),
state=state)):
with self.app.test_request_context(f'/redirect_uri?{urlencode(error_response)}&state={state}'):
UserSession(flask.session, self.PROVIDER_NAME)
flask.session['auth_request'] = json.dumps({'state': state, 'nonce': 'test_nonce'})
result = authn._handle_authentication_response()
Expand All @@ -679,9 +679,12 @@ 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.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))):
config = {
'provider_configuration_info': {'issuer': self.PROVIDER_BASEURL},
'client_registration_info': {'client_id': 'abc', 'client_secret': 'foo'}
}
authn = self.init_app(config)
with self.app.test_request_context(f'/redirect_uri?{urlencode(error_response)}'):
UserSession(flask.session, self.PROVIDER_NAME)
flask.session['state'] = state
flask.session['nonce'] = 'test_nonce'
Expand All @@ -698,7 +701,7 @@ def test_token_error_response_calls_to_error_view_if_set(self):
error_view_mock = self.get_view_mock()
authn.error_view(error_view_mock)
state = 'test_tate'
with self.app.test_request_context('/redirect_uri?code=foo&state={}'.format(state)):
with self.app.test_request_context(f'/redirect_uri?code=foo&state={state}'):
UserSession(flask.session, self.PROVIDER_NAME)
flask.session['auth_request'] = json.dumps({'state': state, 'nonce': 'test_nonce'})
result = authn._handle_authentication_response()
Expand Down
3 changes: 1 addition & 2 deletions tests/test_provider_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import pytest
import responses

from flask_pyoidc.provider_configuration import ProviderConfiguration, ClientRegistrationInfo, ProviderMetadata, \
ClientMetadata, OIDCData
from oic.oic import Client
Expand Down Expand Up @@ -183,5 +182,5 @@ def test_copy_should_overwrite_existing_value(self):
def test_del_and_len(self):
data = OIDCData(abc='xyz', qwe='rty')
assert len(data) == 2
data.__delitem__('qwe')
del data['qwe']
assert data.to_dict() == {'abc': 'xyz'}
9 changes: 4 additions & 5 deletions tests/test_pyoidc_facade.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
import base64
import time
from urllib.parse import parse_qsl

import pytest
import responses
from oic.oic import (AccessTokenResponse, AuthorizationErrorResponse, AuthorizationResponse, Grant, OpenIDSchema,
TokenErrorResponse)
from urllib.parse import parse_qsl

from flask_pyoidc.provider_configuration import ProviderConfiguration, ClientMetadata, ProviderMetadata, \
ClientRegistrationInfo
from flask_pyoidc.pyoidc_facade import PyoidcFacade
from oic.oic import (AccessTokenResponse, AuthorizationErrorResponse, AuthorizationResponse, Grant, OpenIDSchema,
TokenErrorResponse)

from .util import signed_id_token

REDIRECT_URI = 'https://rp.example.com/redirect_uri'
Expand Down
2 changes: 1 addition & 1 deletion tests/test_redirect_uri_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from flask_pyoidc.redirect_uri_config import RedirectUriConfig


class TestRedirectUriConfig(object):
class TestRedirectUriConfig:
LEGACY_CONFIG = {'SERVER_NAME': 'example.com', 'PREFERRED_URL_SCHEME': 'http'}

def test_legacy_config_defaults(self):
Expand Down
4 changes: 2 additions & 2 deletions tests/test_user_session.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import pytest
import time
from unittest.mock import patch

import pytest
from flask_pyoidc.user_session import UserSession, UninitialisedSession


class TestUserSession(object):
class TestUserSession:
PROVIDER_NAME = 'test_provider'

def initialised_session(self, session_storage):
Expand Down

0 comments on commit 3724019

Please sign in to comment.