Skip to content

Commit cbca429

Browse files
committed
CR response: simplify duduping and add docstring
1 parent 72dae64 commit cbca429

File tree

3 files changed

+11
-17
lines changed

3 files changed

+11
-17
lines changed

api/institutions/authentication.py

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,11 @@ def authenticate(self, request):
226226

227227
# Deduplicate full name first if it is provided
228228
if fullname:
229-
fullname = deduplicate_sso_attributes(institution, sso_identity, 'fullname', fullname)
229+
fullname = deduplicate_sso_attributes('fullname', fullname)
230230
# Use given name and family name to build full name if it is not provided
231231
if given_name and family_name and not fullname:
232-
given_name = deduplicate_sso_attributes(institution, sso_identity, 'given_name', given_name)
233-
family_name = deduplicate_sso_attributes(institution, sso_identity, 'family_name', family_name)
232+
given_name = deduplicate_sso_attributes('given_name', given_name)
233+
family_name = deduplicate_sso_attributes('family_name', family_name)
234234
fullname = given_name + ' ' + family_name
235235

236236
# Non-empty full name is required. Fail the auth and inform sentry if not provided.
@@ -243,13 +243,7 @@ def authenticate(self, request):
243243

244244
# Deduplicate sso email, currently we only handle duplicate sso email instead of multiple sso email
245245
try:
246-
sso_email = deduplicate_sso_attributes(
247-
institution,
248-
sso_identity,
249-
'sso_email',
250-
sso_email,
251-
ignore_errors=False,
252-
)
246+
sso_email = deduplicate_sso_attributes('sso_email', sso_email)
253247
except MultipleSSOEmailError:
254248
message = f'Institution SSO Error: multiple SSO email [sso_email={sso_email}, sso_identity={sso_identity}, institution={institution._id}]'
255249
sentry.log_message(message)

framework/auth/__init__.py

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -159,17 +159,16 @@ def get_or_create_institutional_user(fullname, sso_email, sso_identity, primary_
159159
return user, True, None, None, sso_identity
160160

161161

162-
def deduplicate_sso_attributes(institution, sso_identity, attr_name, attr_value, delimiter=';', ignore_errors=True):
162+
def deduplicate_sso_attributes(attr_name, attr_value, delimiter=';'):
163163
if delimiter not in attr_value:
164164
return attr_value
165165
value_set = set(attr_value.split(delimiter))
166166
if len(value_set) != 1:
167-
message = (f'Multiple values {attr_value} found for SSO attribute {attr_name}: '
168-
f'[institution_id={institution._id}, sso_identity={sso_identity}]')
169-
if ignore_errors:
170-
sentry.log_message(message)
171-
return attr_value
172-
raise MultipleSSOEmailError(message)
167+
message = f'Multiple values found for SSO attribute: [{attr_name}={attr_value}]'
168+
sentry.log_message(message)
169+
if attr_name == 'sso_email':
170+
raise MultipleSSOEmailError(message)
171+
return attr_value
173172
return value_set.pop()
174173

175174

framework/auth/exceptions.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,4 +68,5 @@ class MergeConflictError(EmailConfirmTokenError):
6868

6969

7070
class MultipleSSOEmailError(AuthError):
71+
"""Raised if institution SSO provides multiple emails which OSF cannot deduplicate."""
7172
pass

0 commit comments

Comments
 (0)