Skip to content

Commit

Permalink
global: migrate to new userprofiles
Browse files Browse the repository at this point in the history
Co-authored-by: Zacharias Zacharodimos <[email protected]>
  • Loading branch information
Pablo Panero and zzacharo committed May 18, 2022
1 parent 50efe6f commit 9872588
Show file tree
Hide file tree
Showing 11 changed files with 58 additions and 30 deletions.
2 changes: 0 additions & 2 deletions invenio_oauthclient/contrib/cern_openid.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,6 @@ def _account_info(remote, resp):
)

email = resource["email"]
person_id = resource.get("cern_person_id")
external_id = resource["cern_upn"]
nice = resource["preferred_username"]
name = resource["name"]
Expand Down Expand Up @@ -363,7 +362,6 @@ def account_setup(remote, token, resp):
resource = get_resource(remote, resp)

with db.session.begin_nested():
person_id = resource.get("cern_person_id")
external_id = resource.get("cern_upn")

# Set CERN person ID in extra_data.
Expand Down
4 changes: 2 additions & 2 deletions invenio_oauthclient/handlers/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,8 @@ def signup_handler(remote, *args, **kwargs):
remote.name]
try:
form = create_csrf_disabled_registrationform(remote)
json_data = {} if not request.data else json.loads(request.data)
form = fill_form(form, json_data)
data = request.form.to_dict()
form = fill_form(form, data)
next_url = base_signup_handler(remote, form, *args, **kwargs)
if form.is_submitted():
response_payload = dict(
Expand Down
25 changes: 22 additions & 3 deletions invenio_oauthclient/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
from invenio_db import db
from invenio_db.utils import rebuild_encrypted_properties
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
Expand Down Expand Up @@ -155,6 +154,20 @@ def remove_csrf_tokens(user_data):
remove_csrf_tokens(value)


def remove_none_values(user_data):
"""Remove None values from the user data."""
del_keys = []
for key, value in list(user_data.items()):
if isinstance(value, dict):
remove_none_values(value)
if value == {}:
del_keys.append(key)
if value is None:
del_keys.append(key)
for key in del_keys:
del user_data[key]


def oauth_register(form, user_info=None, precedence_mask=None):
"""Register user if possible.
Expand All @@ -163,15 +176,21 @@ def oauth_register(form, user_info=None, precedence_mask=None):
:returns: A :class:`invenio_accounts.models.User` instance.
"""
if form.validate():
data = form.to_dict()
form_data = form.data

# let relevant information from the OAuth service's user info
# have precedence over the values specified by the user
if user_info:
default_mask = {"email": True}
# act on form data so the `profile` is updated correctly
filter_user_info(user_info, precedence_mask or default_mask)
patch_dictionary(data, user_info)
patch_dictionary(form_data, user_info)

# re-populate the form after appyling the precedence mask and
# convert the form data to user model's data
data = fill_form(form, form_data).to_dict()
# see https://github.com/inveniosoftware/invenio-oauthclient/issues/275
remove_none_values(data)
# remove the CSRF tokens to avoid unexpected keyword arguments
remove_csrf_tokens(data)
user = register_user(**data)
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

tests_require = [
'httpretty>=0.8.14',
'invenio-userprofiles>=1.0.0',
'invenio-userprofiles>=2.0.0.dev3',
'mock>=1.3.0',
'oauthlib>=1.1.2,<3.0.0',
'pytest-invenio>=1.4.2',
Expand Down
24 changes: 14 additions & 10 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ def base_app(request):
SQLALCHEMY_TRACK_MODIFICATIONS=False,
SECURITY_PASSWORD_HASH='plaintext',
SECURITY_PASSWORD_SCHEMES=['plaintext'],
SECURITY_PASSWORD_SINGLE_HASH=None,
APP_ALLOWED_HOSTS=['localhost'],
APP_THEME=['semantic-ui'],
THEME_ICONS={
Expand Down Expand Up @@ -488,10 +489,12 @@ def user(app_with_userprofiles):
"""Create users."""
with db.session.begin_nested():
datastore = app_with_userprofiles.extensions['security'].datastore
user1 = datastore.create_user(email='[email protected]',
password='tester', active=True)
profile = UserProfile(username='mynick', user=user1)
db.session.add(profile)
user1 = datastore.create_user(
email='[email protected]',
password='tester',
active=True,
username='mynick',
)
db.session.commit()
return user1

Expand All @@ -501,10 +504,12 @@ def user_rest(app_rest_with_userprofiles):
"""Create users."""
with db.session.begin_nested():
datastore = app_rest_with_userprofiles.extensions['security'].datastore
user1 = datastore.create_user(email='[email protected]',
password='tester', active=True)
profile = UserProfile(username='mynick', user=user1)
db.session.add(profile)
user1 = datastore.create_user(
email='[email protected]',
password='tester',
active=True,
username='mynick',
)
db.session.commit()
return user1

Expand All @@ -515,8 +520,7 @@ def form_test_data():
return dict(
email='[email protected]',
profile=dict(
full_name='Test Tester',
username='test123',
full_name='Test Tester', username='test123',
),
)

Expand Down
1 change: 1 addition & 0 deletions tests/test_contrib_github.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ def test_authorized_signup_username_already_exists(app, example_github, user):
'password': '123456',
'profile.username': 'pippo2',
'profile.full_name': 'pluto',
'profile.affiliations': 'CERN'
}
)
assert resp.status_code == 302
Expand Down
1 change: 1 addition & 0 deletions tests/test_contrib_github_rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ def test_authorized_signup_username_already_exists(
'password': '123456',
'profile.username': 'pippo2',
'profile.full_name': 'pluto',
'profile.affiliations': 'CERN'
}
)
assert resp.status_code == 200
Expand Down
2 changes: 1 addition & 1 deletion tests/test_contrib_keycloak.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def test_authorized_signup_valid_user(app_with_userprofiles,
user = User.query.filter_by(email=example_keycloak["email"]).one()
assert user is not None
assert user.email == example_keycloak["email"]
assert user.profile.full_name == "Max Moser"
assert user.user_profile["full_name"] == "Max Moser"
assert user.active

# check that the user has a linked Keycloak account
Expand Down
10 changes: 7 additions & 3 deletions tests/test_contrib_orcid.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,11 @@ def test_account_info(app, example_orcid):
ioc.remote_apps['orcid'], example_data) == example_account_info

assert account_info(ioc.remote_apps['orcid'], {}) == \
dict(external_id=None, external_method='orcid', user=dict(profile=dict(
full_name=None)))
dict(
external_id=None,
external_method='orcid',
user=dict(profile=dict(full_name=None))
)


def test_login(app, example_orcid):
Expand Down Expand Up @@ -93,6 +96,7 @@ def test_authorized_signup(app_with_userprofiles, example_orcid, orcid_bio):
'password': '123456',
'profile.username': 'pippo',
'profile.full_name': account_info['user']['profile']['full_name'],
'profile.affiliations': 'CERN',
}

# Mock request to ORCID to get user bio.
Expand All @@ -119,7 +123,7 @@ def test_authorized_signup(app_with_userprofiles, example_orcid, orcid_bio):
id=example_data['orcid']
).one()
# FIXME see contrib/orcid.py line 167
assert user.profile.full_name == 'Josiah Carberry'
assert user.user_profile["full_name"] == 'Josiah Carberry'
# assert user.given_names == 'Josiah'
# assert user.family_name == 'Carberry'
# check that the user's email is not yet validated
Expand Down
11 changes: 7 additions & 4 deletions tests/test_contrib_orcid_rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,11 @@ def test_account_info(app_rest, example_orcid):
ioc.remote_apps['orcid'], example_data) == example_account_info

assert account_info(ioc.remote_apps['orcid'], {}) == \
dict(external_id=None, external_method='orcid', user=dict(profile=dict(
full_name=None)))
dict(
external_id=None,
external_method='orcid',
user=dict(profile=dict(full_name=None))
)


def test_login(app_rest, example_orcid):
Expand Down Expand Up @@ -95,6 +98,7 @@ def test_authorized_signup(app_rest_with_userprofiles,
'password': '123456',
'profile.username': 'pippo',
'profile.full_name': account_info['user']['profile']['full_name'],
'profile.affiliations': 'CERN'
}

# Mock request to ORCID to get user bio.
Expand All @@ -120,8 +124,7 @@ def test_authorized_signup(app_rest_with_userprofiles,
method='orcid', id_user=user.id,
id=example_data['orcid']
).one()
# FIXME see contrib/orcid.py line 167
assert user.profile.full_name == 'Josiah Carberry'
assert user.user_profile["full_name"] == 'Josiah Carberry'
# assert user.given_names == 'Josiah'
# assert user.family_name == 'Carberry'
# check that the user's email is not yet validated
Expand Down
6 changes: 2 additions & 4 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,7 @@ def test_patch_dictionary():
"""Test the dictionary patch function."""
orig_dict = {
"email": "[email protected]",
"profile": {
"username": "user"
},
"username": "user",
}

# patch some existing properties, add new ones, and leave some as is
Expand All @@ -244,8 +242,8 @@ def test_patch_dictionary():

expected = {
"email": "[email protected]",
"username": "user",
"profile": {
"username": "user",
"full_name": "Test User",
},
"extra": [1, 2, 3],
Expand Down

0 comments on commit 9872588

Please sign in to comment.