From 2d04df03d7d92c18be1acaf3a074cfafc6fd8911 Mon Sep 17 00:00:00 2001 From: Chris Topaloudis Date: Fri, 18 Sep 2020 00:21:51 +0200 Subject: [PATCH 1/2] global: clear roles and cern_resource on logout --- invenio_oauthclient/contrib/cern.py | 4 ++- invenio_oauthclient/contrib/cern_openid.py | 4 ++- setup.py | 2 +- tests/conftest.py | 3 +- tests/test_contrib_cern.py | 18 +++++++---- tests/test_contrib_cern_openid.py | 18 ++++++++--- tests/test_contrib_cern_openid_rest.py | 18 ++++++++--- tests/test_contrib_cern_rest.py | 35 ++++++++++++++++++---- 8 files changed, 78 insertions(+), 24 deletions(-) diff --git a/invenio_oauthclient/contrib/cern.py b/invenio_oauthclient/contrib/cern.py index 4cca799c..7caadb14 100644 --- a/invenio_oauthclient/contrib/cern.py +++ b/invenio_oauthclient/contrib/cern.py @@ -356,7 +356,8 @@ def extend_identity(identity, groups): def disconnect_identity(identity): """Disconnect identity from CERN groups.""" - provides = session.pop(OAUTHCLIENT_CERN_SESSION_KEY, {}) + session.pop("cern_resource", None) + provides = session.pop(OAUTHCLIENT_CERN_SESSION_KEY, set()) identity.provides -= provides @@ -504,6 +505,7 @@ def on_identity_changed(sender, identity): :param identity: The user identity where information are stored. """ if isinstance(identity, AnonymousIdentity): + disconnect_identity(identity) return client_id = current_app.config['CERN_APP_CREDENTIALS']['consumer_key'] diff --git a/invenio_oauthclient/contrib/cern_openid.py b/invenio_oauthclient/contrib/cern_openid.py index fa3780f3..88c0f36d 100644 --- a/invenio_oauthclient/contrib/cern_openid.py +++ b/invenio_oauthclient/contrib/cern_openid.py @@ -210,11 +210,12 @@ def extend_identity(identity, roles): def disconnect_identity(identity): """Disconnect identity from CERN groups.""" + session.pop("cern_resource", None) key = current_app.config.get( "OAUTHCLIENT_CERN_OPENID_SESSION_KEY", OAUTHCLIENT_CERN_OPENID_SESSION_KEY, ) - provides = session.pop(key, {}) + provides = session.pop(key, set()) identity.provides -= provides @@ -366,6 +367,7 @@ def on_identity_changed(sender, identity): :param identity: The user identity where information are stored. """ if isinstance(identity, AnonymousIdentity): + disconnect_identity(identity) return client_id = current_app.config["CERN_APP_OPENID_CREDENTIALS"][ diff --git a/setup.py b/setup.py index cadec68d..598b4a8d 100644 --- a/setup.py +++ b/setup.py @@ -68,7 +68,7 @@ 'Flask-OAuthlib>=0.9.5', 'blinker>=1.4', 'invenio-accounts>=1.3.0', - 'invenio-base>=1.2.2', + 'invenio-base>=1.2.3', 'invenio-i18n>=1.2.0', 'invenio-mail>=1.0.0', 'uritools>=1.0.1', diff --git a/tests/conftest.py b/tests/conftest.py index d4d142e4..1d6b3892 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -17,12 +17,11 @@ import tempfile import pytest -import werkzeug from flask import Flask from flask_babelex import Babel from flask_mail import Mail from flask_menu import Menu as FlaskMenu -from invenio_accounts import InvenioAccounts, InvenioAccountsREST +from invenio_accounts import InvenioAccounts from invenio_db import InvenioDB, db from invenio_userprofiles import InvenioUserProfiles, UserProfile from invenio_userprofiles.views import blueprint_ui_init diff --git a/tests/test_contrib_cern.py b/tests/test_contrib_cern.py index 45888303..e52ba7f1 100644 --- a/tests/test_contrib_cern.py +++ b/tests/test_contrib_cern.py @@ -10,16 +10,14 @@ from __future__ import absolute_import -import pytest from flask import g, session, url_for -from flask_security import login_user +from flask_security import login_user, logout_user from helpers import get_state, mock_remote_get, mock_response from six.moves.urllib_parse import parse_qs, urlparse -from invenio_oauthclient.contrib.cern import account_info, \ - disconnect_handler, fetch_extra_data, fetch_groups, \ +from invenio_oauthclient.contrib.cern import OAUTHCLIENT_CERN_SESSION_KEY, \ + account_info, disconnect_handler, fetch_extra_data, fetch_groups, \ get_dict_from_response -from invenio_oauthclient.errors import OAuthCERNRejectedAccountError def test_fetch_groups(app, example_cern): @@ -138,6 +136,16 @@ def test_account_setup(app, example_cern, models_fixture): login_user(user) assert len(g.identity.provides) == 7 + + logout_user() + assert len(g.identity.provides) == 1 + assert "cern_resource" not in session + assert OAUTHCLIENT_CERN_SESSION_KEY not in session + + # Login again to test the disconnect handler + login_user(user) + assert len(g.identity.provides) == 7 + disconnect_handler(ioc.remote_apps['cern']) diff --git a/tests/test_contrib_cern_openid.py b/tests/test_contrib_cern_openid.py index 00aa8921..d77bbd0c 100644 --- a/tests/test_contrib_cern_openid.py +++ b/tests/test_contrib_cern_openid.py @@ -14,13 +14,13 @@ import pytest from flask import g, session, url_for -from flask_security import login_user +from flask_security import login_user, logout_user from helpers import get_state, mock_remote_get, mock_response from six.moves.urllib_parse import parse_qs, urlparse -from invenio_oauthclient.contrib.cern_openid import account_info, \ - disconnect_handler, fetch_extra_data, get_dict_from_response -from invenio_oauthclient.errors import OAuthCERNRejectedAccountError +from invenio_oauthclient.contrib.cern_openid import \ + OAUTHCLIENT_CERN_OPENID_SESSION_KEY, account_info, disconnect_handler, \ + fetch_extra_data, get_dict_from_response from flask_oauthlib.client import OAuthResponse # noqa isort:skip @@ -118,6 +118,16 @@ def test_account_setup(app, example_cern_openid, models_fixture): login_user(user) assert len(g.identity.provides) == 3 + + logout_user() + assert len(g.identity.provides) == 1 + assert "cern_resource" not in session + assert OAUTHCLIENT_CERN_OPENID_SESSION_KEY not in session + + # Login again to test the disconnect handler + login_user(user) + assert len(g.identity.provides) == 3 + disconnect_handler(ioc.remote_apps['cern_openid']) diff --git a/tests/test_contrib_cern_openid_rest.py b/tests/test_contrib_cern_openid_rest.py index 475ddf19..62aeb3ed 100644 --- a/tests/test_contrib_cern_openid_rest.py +++ b/tests/test_contrib_cern_openid_rest.py @@ -14,14 +14,14 @@ import pytest from flask import g, session, url_for -from flask_security import login_user +from flask_security import login_user, logout_user from helpers import check_response_redirect_url_args, get_state, \ mock_remote_get, mock_response from six.moves.urllib_parse import parse_qs, urlparse -from invenio_oauthclient.contrib.cern_openid import account_info_rest, \ +from invenio_oauthclient.contrib.cern_openid import \ + OAUTHCLIENT_CERN_OPENID_SESSION_KEY, account_info_rest, \ disconnect_rest_handler, fetch_extra_data, get_dict_from_response -from invenio_oauthclient.errors import OAuthCERNRejectedAccountError from flask_oauthlib.client import OAuthResponse # noqa isort:skip @@ -124,6 +124,16 @@ def test_account_setup(app_rest, example_cern_openid_rest, models_fixture): login_user(user) assert len(g.identity.provides) == 3 + + logout_user() + assert len(g.identity.provides) == 1 + assert "cern_resource" not in session + assert OAUTHCLIENT_CERN_OPENID_SESSION_KEY not in session + + # Login again to test the disconnect handler + login_user(user) + assert len(g.identity.provides) == 3 + disconnect_rest_handler(ioc.remote_apps['cern_openid']) @@ -178,8 +188,8 @@ def test_account_info_not_allowed_account(app_rest, example_cern_openid_rest): example_response, _, example_account_info = example_cern_openid_rest mock_remote_get(ioc, 'cern_openid', example_response) - resp = account_info_rest(ioc.remote_apps['cern_openid'], None) + assert resp.status_code == 302 expected_url_args = { "message": "CERN account not allowed.", diff --git a/tests/test_contrib_cern_rest.py b/tests/test_contrib_cern_rest.py index 1ce319e7..f36ae65b 100644 --- a/tests/test_contrib_cern_rest.py +++ b/tests/test_contrib_cern_rest.py @@ -10,17 +10,16 @@ from __future__ import absolute_import -import pytest from flask import g, session, url_for -from flask_security import login_user +from flask_principal import AnonymousIdentity, Identity, RoleNeed, UserNeed +from flask_security import login_user, logout_user from helpers import check_response_redirect_url_args, get_state, \ mock_remote_get, mock_response from six.moves.urllib_parse import parse_qs, urlparse -from invenio_oauthclient.contrib.cern import account_info_rest, \ - disconnect_rest_handler, fetch_extra_data, fetch_groups, \ - get_dict_from_response -from invenio_oauthclient.errors import OAuthCERNRejectedAccountError +from invenio_oauthclient.contrib.cern import OAUTHCLIENT_CERN_SESSION_KEY, \ + account_info_rest, disconnect_rest_handler, fetch_extra_data, \ + fetch_groups, get_dict_from_response def test_fetch_groups(app_rest, example_cern): @@ -130,7 +129,31 @@ def test_account_setup(app_rest, example_cern, models_fixture): assert resp.status_code >= 300 login_user(user) + assert isinstance(g.identity, Identity) + assert g.identity.provides == set([ + UserNeed(4), + UserNeed('test.account@cern.ch'), + RoleNeed('Group1@cern.ch'), + RoleNeed('Group2@cern.ch'), + RoleNeed('Group3@cern.ch'), + RoleNeed('Group4@cern.ch'), + RoleNeed('Group5@cern.ch'), + ]) + + logout_user() + assert isinstance(g.identity, AnonymousIdentity) + # NOTE: Wrong role, g.identity.provides = {Need(['id', 4])} read more + # https://github.com/inveniosoftware/invenio-access/blob/e28e76d5361a29202b94d498f1968454c24c5c80/tests/test_loaders.py#L47 + assert len(g.identity.provides) == 1 + + assert "cern_resource" not in session + assert OAUTHCLIENT_CERN_SESSION_KEY not in session + + # Login again to test the disconnect handler + login_user(user) + assert isinstance(g.identity, Identity) assert len(g.identity.provides) == 7 + disconnect_rest_handler(ioc.remote_apps['cern']) From e4e37902daec4e7e433f022ca15d90ad9a279dd4 Mon Sep 17 00:00:00 2001 From: Chris Topaloudis Date: Wed, 30 Sep 2020 17:47:36 +0200 Subject: [PATCH 2/2] upgrade invenio-pytest 1.4.0 --- docs/conf.py | 6 +++--- invenio_oauthclient/handlers/ui.py | 20 +++++--------------- invenio_oauthclient/handlers/utils.py | 26 +++++++++++++------------- invenio_oauthclient/utils.py | 3 +-- pytest.ini | 5 ++--- setup.cfg | 3 +++ setup.py | 4 ++-- tests/test_app.py | 7 ++++--- 8 files changed, 33 insertions(+), 41 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index b11b17fe..38512942 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -12,8 +12,6 @@ import os -import sphinx.environment - # -- General configuration ------------------------------------------------ # If your documentation needs a minimal Sphinx version, state it here. @@ -321,7 +319,9 @@ # Example configuration for intersphinx: refer to the Python standard library. intersphinx_mapping = { 'https://docs.python.org/': None, - 'invenio-accounts': ('https://invenio-accounts.readthedocs.io/en/latest/', None), + 'invenio-accounts': ( + 'https://invenio-accounts.readthedocs.io/en/latest/', None + ), } # Autodoc configuraton. diff --git a/invenio_oauthclient/handlers/ui.py b/invenio_oauthclient/handlers/ui.py index 7b07440c..4f2d2a42 100644 --- a/invenio_oauthclient/handlers/ui.py +++ b/invenio_oauthclient/handlers/ui.py @@ -12,36 +12,26 @@ from functools import partial, wraps -import six from flask import current_app, flash, redirect, render_template, request, \ - session, url_for + url_for from flask_babelex import gettext as _ -from flask_login import current_user from invenio_db import db -from werkzeug.utils import import_string from ..errors import AlreadyLinkedError, OAuthClientAlreadyAuthorized, \ OAuthClientError, OAuthClientMustRedirectLogin, \ OAuthClientMustRedirectSignup, OAuthClientTokenNotFound, \ OAuthClientTokenNotSet, OAuthClientUnAuthorized, \ - OAuthClientUserNotRegistered, OAuthError, OAuthRejectedRequestError, \ - OAuthResponseError -from ..models import RemoteAccount, RemoteToken -from ..proxies import current_oauthclient -from ..signals import account_info_received, account_setup_committed, \ - account_setup_received -from ..utils import create_csrf_disabled_registrationform, \ - create_registrationform, fill_form, oauth_authenticate, oauth_register + OAuthClientUserNotRegistered, OAuthError, OAuthRejectedRequestError +from ..utils import create_registrationform from .base import base_authorized_signup_handler, base_disconnect_handler, \ base_signup_handler -from .utils import get_session_next_url, response_token_setter, token_getter, \ - token_session_key, token_setter +from .utils import response_token_setter def _oauth_error_handler(remote, f, *args, **kwargs): """Function to handle exceptions.""" try: - return f(remote, *args, **kwargs) + return f(remote, *args, **kwargs) except OAuthClientError as e: current_app.logger.warning(e.message, exc_info=True) return oauth2_handle_error( diff --git a/invenio_oauthclient/handlers/utils.py b/invenio_oauthclient/handlers/utils.py index ac986489..385eb86a 100644 --- a/invenio_oauthclient/handlers/utils.py +++ b/invenio_oauthclient/handlers/utils.py @@ -203,8 +203,8 @@ def token_delete(remote, token=''): """Remove OAuth access tokens from session. :param remote: The remote application. - :param token: Type of token to get. Data passed from ``oauth.request()`` to - identify which token to retrieve. (Default: ``''``) + :param token: Type of token to get. Data passed from ``oauth.request()`` + to identify which token to retrieve. (Default: ``''``) :returns: The token. """ session_key = token_session_key(remote.name) @@ -245,14 +245,14 @@ def make_token_getter(remote): def authorized_handler(f, authorized_response): - """Handles an OAuth callback. - - As authorized_handler is deprecated in favor of authorized_response, - it's has to be wrapped with handler which will be executed - at the proper time. - """ - @wraps(f) - def decorated(*args, **kwargs): - data = authorized_response() - return f(*((data,) + args), **kwargs) - return decorated + """Handles an OAuth callback. + + As authorized_handler is deprecated in favor of authorized_response, + it's has to be wrapped with handler which will be executed + at the proper time. + """ + @wraps(f) + def decorated(*args, **kwargs): + data = authorized_response() + return f(*((data,) + args), **kwargs) + return decorated diff --git a/invenio_oauthclient/utils.py b/invenio_oauthclient/utils.py index 939942d6..48c4a7e2 100644 --- a/invenio_oauthclient/utils.py +++ b/invenio_oauthclient/utils.py @@ -18,12 +18,11 @@ from invenio_accounts.utils import register_user from invenio_db import db from invenio_db.utils import rebuild_encrypted_properties -from itsdangerous import BadData, TimedJSONWebSignatureSerializer +from itsdangerous import TimedJSONWebSignatureSerializer from sqlalchemy.exc import IntegrityError from uritools import uricompose, urisplit from werkzeug.local import LocalProxy from werkzeug.utils import import_string -from wtforms.fields.core import FormField from .errors import AlreadyLinkedError from .models import RemoteAccount, RemoteToken, UserIdentity diff --git a/pytest.ini b/pytest.ini index c646d1c8..510dbf02 100644 --- a/pytest.ini +++ b/pytest.ini @@ -7,7 +7,6 @@ # under the terms of the MIT License; see LICENSE file for more details. [pytest] -pep8ignore = docs/conf.py ALL -addopts = --pep8 --doctest-glob="*.rst" --doctest-modules --cov=invenio_oauthclient --cov-report=term-missing +addopts = --isort --pydocstyle --pycodestyle --doctest-glob="*.rst" --doctest-modules --cov=invenio_oauthclient --cov-report=term-missing testpaths = docs tests invenio_oauthclient -filterwarnings = ignore::pytest.PytestDeprecationWarning \ No newline at end of file +filterwarnings = ignore::pytest.PytestDeprecationWarning diff --git a/setup.cfg b/setup.cfg index c075323f..68d6525c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -34,5 +34,8 @@ output-dir = invenio_oauthclient/translations/ input-file = invenio_oauthclient/translations/messages.pot output-dir = invenio_oauthclient/translations/ +[pycodestyle] +exclude = docs/conf.py + [pydocstyle] add_ignore = D401 diff --git a/setup.py b/setup.py index 598b4a8d..b75d3465 100644 --- a/setup.py +++ b/setup.py @@ -16,7 +16,7 @@ history = open('CHANGES.rst').read() tests_require = [ - 'pytest-invenio>=1.3.2', + 'pytest-invenio>=1.4.0', 'SQLAlchemy-Continuum>=1.2.1', 'httpretty>=0.8.14', 'invenio-userprofiles>=1.0.0', @@ -31,7 +31,7 @@ 'invenio-admin>=1.0.0', ], 'docs': [ - 'Sphinx>=1.5.1', + 'Sphinx>=3.0.0', ], 'github': [ 'github3.py>=1.0.0a4', diff --git a/tests/test_app.py b/tests/test_app.py index 2a119078..6bf3ffae 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -118,9 +118,10 @@ def teardown(): request.addfinalizer(teardown) with app.app_context(): - if str(db.engine.url) != 'sqlite://' and \ - not database_exists(str(db.engine.url)): - create_database(str(db.engine.url)) + is_sqllite = str(db.engine.url) == 'sqlite://' + db_exists = database_exists(str(db.engine.url)) + if not is_sqllite and not db_exists: + create_database(str(db.engine.url)) db.create_all() tables = list(filter(lambda table: table.startswith('oauthclient'), db.metadata.tables.keys()))