Skip to content

Commit

Permalink
fix orcid checksum char (#65)
Browse files Browse the repository at this point in the history
* new test cases that should be valid

* <bot> update dependencies*.log files(s)

* better orcid validation

* <bot> update dependencies*.log files(s)

* fix flake8

* fix test

---------

Co-authored-by: github-actions <[email protected]>
  • Loading branch information
dsschult and github-actions authored Oct 24, 2023
1 parent 257c227 commit 9c861b4
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 8 deletions.
4 changes: 2 additions & 2 deletions cypress/integration/home.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ context('Home Page', () => {

cy.get('.profile [name="loginShell"]').should('exist').should('not.have.attr', 'disabled')

cy.get('.profile [name="orcid"]').should('exist').type('1234-1234-1234-1234')
cy.get('.profile [name="orcid"]').should('exist').type('1234-1234-1234-123X')
cy.get('.profile button').click()

cy.wait('@api-user-profile-put').should(({ request, response }) => {
Expand All @@ -80,7 +80,7 @@ context('Home Page', () => {
'author_firstName': '',
'author_lastName': '',
'author_email': '',
'orcid': '1234-1234-1234-1234',
'orcid': '1234-1234-1234-123X',
'phd_year': '',
'github': '',
'slack': '',
Expand Down
8 changes: 4 additions & 4 deletions dependencies-from-Dockerfile.log
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ aiormq==6.7.7
cachetools==5.3.1
certifi==2023.7.22
cffi==1.16.0
charset-normalizer==3.3.0
cryptography==41.0.4
charset-normalizer==3.3.1
cryptography==41.0.5
dnspython==2.4.2
idna==3.4
ldap3==2.9.1
Expand All @@ -22,9 +22,9 @@ requests-futures==1.0.1
tornado==6.3.3
typing_extensions==4.8.0
Unidecode==1.3.7
urllib3==2.0.6
urllib3==2.0.7
-e /app
wipac-dev-tools==1.7.0
wipac-keycloak-rest-services==1.4.44
wipac-keycloak-rest-services==1.4.45
wipac-rest-tools==1.6.0
yarl==1.9.2
20 changes: 19 additions & 1 deletion tests/test_api_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,20 @@ def test_invalid_usernames():
assert not user_mgmt.users.Username._username_valid('foo-bār')


def test_invalid_orcid():
assert user_mgmt.users.is_orcid('0001-0002-0003-0004')
assert user_mgmt.users.is_orcid('0000-0001-8945-6722')
assert user_mgmt.users.is_orcid('0001-0002-0003-000X')
with pytest.raises(Exception):
assert user_mgmt.users.is_orcid('0001-0002-0003-000Y')
with pytest.raises(Exception):
assert user_mgmt.users.is_orcid('0001-0002-0003-00XX')
with pytest.raises(Exception):
assert user_mgmt.users.is_orcid('1-2-3-4')
with pytest.raises(Exception):
assert user_mgmt.users.is_orcid('foo')


@pytest.mark.asyncio
async def test_user(server):
rest, krs_client, *_ = server
Expand Down Expand Up @@ -59,7 +73,11 @@ async def test_user_put(server):

await client.request('PUT', '/api/users/test', {'orcid': '0000-0001-8945-6722'})
ret = await krs.users.user_info('test', rest_client=krs_client)
assert ret['attributes']['orcid'] == '0000-0001-8945-6722'
assert ret['attributes']['orcid'] == '0000-0001-8945-6722'

await client.request('PUT', '/api/users/test', {'orcid': '0001-0002-0003-000X'})
ret = await krs.users.user_info('test', rest_client=krs_client)
assert ret['attributes']['orcid'] == '0001-0002-0003-000X'

with pytest.raises(Exception):
await client.request('PUT', '/api/users/test', {'orcid': '1-2-3-4'})
Expand Down
22 changes: 21 additions & 1 deletion user_mgmt/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,26 @@
'email': 'email',
}


def is_orcid(value):
"""ORCID validation"""
logging.debug('testing orcid %r', value)
parts = value.split('-')
if len(parts) != 4:
return False
if any(len(x) != 4 or not x.isdigit() for x in parts[:3]):
return False
x = parts[3]
logging.debug('handle last part: %s', x)
if len(x) != 4 or not x[:3].isdigit():
return False
logging.debug('handle last char: %s', x[3])
# special handling for checksum X
if not (x[3].isdigit() or x[3] == 'X'):
return False
return True


#: attr name and validation function
EXTRA_ATTRS = {
'mailing_list_email': lambda x: x == '' or '@' in x,
Expand All @@ -33,7 +53,7 @@
'author_firstName': lambda x: True,
'author_lastName': lambda x: True,
'author_email': lambda x: x == '' or '@' in x,
'orcid': lambda x: x == '' or (len(x.split('-')) == 4 and all(len(y) == 4 and y.isdigit() for y in x.split('-'))),
'orcid': lambda x: x == '' or is_orcid(x),
'phd_year': lambda x: x == '' or (len(x) == 4 and x.isdigit()),
'loginShell': lambda x: x in ('', '/bin/bash', '/bin/zsh', '/bin/tcsh', '/sbin/nologin'),
}
Expand Down

0 comments on commit 9c861b4

Please sign in to comment.