Skip to content
Snippets Groups Projects
Verified Commit 3c51ae60 authored by Andreas Ellewsen's avatar Andreas Ellewsen
Browse files

Handle wrong person following invitation link

If someone that already exists in greg follows an invite, we are kind
enough to give the new role to the existing person. This introduced a
security risk if the invitation was actually meant for someone else.

Because of this situation, we introduce a security mechanism where we
disable the invitationlink if the name of the existing person is too
different from the name used in the invitation.

Resolves: GREG-166
parent 053a1dca
No related branches found
No related tags found
1 merge request!282Handle wrong person following invitation link
...@@ -10,13 +10,13 @@ from django.conf import settings ...@@ -10,13 +10,13 @@ from django.conf import settings
from django.contrib.auth import get_user_model from django.contrib.auth import get_user_model
from django.contrib.auth.backends import BaseBackend, UserModel from django.contrib.auth.backends import BaseBackend, UserModel
from django.core.exceptions import SuspiciousOperation from django.core.exceptions import SuspiciousOperation
from django.utils.timezone import make_aware from django.utils import timezone
from mozilla_django_oidc.auth import OIDCAuthenticationBackend from mozilla_django_oidc.auth import OIDCAuthenticationBackend
from greg.models import Identity, Person, Sponsor, InvitationLink from greg.models import Identity, Person, Sponsor, InvitationLink
from gregui.models import GregUserProfile from gregui.models import GregUserProfile
from gregui.utils import name_diff from gregui.utils import name_diff_too_large
logger = structlog.getLogger(__name__) logger = structlog.getLogger(__name__)
...@@ -339,7 +339,7 @@ class GregOIDCBackend(ValidatingOIDCBackend): ...@@ -339,7 +339,7 @@ class GregOIDCBackend(ValidatingOIDCBackend):
existing_nin.source = source existing_nin.source = source
existing_nin.value = new_nin existing_nin.value = new_nin
existing_nin.verified = Identity.Verified.AUTOMATIC existing_nin.verified = Identity.Verified.AUTOMATIC
existing_nin.verified_at = make_aware(datetime.datetime.now()) existing_nin.verified_at = timezone.make_aware(datetime.datetime.now())
existing_nin.save() existing_nin.save()
logger.info("identity_update", identity_id=existing_nin.id) logger.info("identity_update", identity_id=existing_nin.id)
...@@ -357,7 +357,7 @@ class GregOIDCBackend(ValidatingOIDCBackend): ...@@ -357,7 +357,7 @@ class GregOIDCBackend(ValidatingOIDCBackend):
type=Identity.IdentityType.NORWEGIAN_NATIONAL_ID_NUMBER, type=Identity.IdentityType.NORWEGIAN_NATIONAL_ID_NUMBER,
value=new_nin, value=new_nin,
verified=Identity.Verified.AUTOMATIC, verified=Identity.Verified.AUTOMATIC,
verified_at=make_aware(datetime.datetime.now()), verified_at=timezone.make_aware(datetime.datetime.now()),
) )
nin_id.save() nin_id.save()
logger.info("identity_add", identity_id=nin_id.id) logger.info("identity_add", identity_id=nin_id.id)
...@@ -399,7 +399,7 @@ class GregOIDCBackend(ValidatingOIDCBackend): ...@@ -399,7 +399,7 @@ class GregOIDCBackend(ValidatingOIDCBackend):
existing_feide_id.source = source existing_feide_id.source = source
existing_feide_id.value = new_feide_id existing_feide_id.value = new_feide_id
existing_feide_id.verified = Identity.Verified.AUTOMATIC existing_feide_id.verified = Identity.Verified.AUTOMATIC
existing_feide_id.verified_at = make_aware(datetime.datetime.now()) existing_feide_id.verified_at = timezone.make_aware(datetime.datetime.now())
existing_feide_id.save() existing_feide_id.save()
else: else:
...@@ -421,7 +421,7 @@ class GregOIDCBackend(ValidatingOIDCBackend): ...@@ -421,7 +421,7 @@ class GregOIDCBackend(ValidatingOIDCBackend):
type=Identity.IdentityType.FEIDE_ID, type=Identity.IdentityType.FEIDE_ID,
value=new_feide_id, value=new_feide_id,
verified=Identity.Verified.AUTOMATIC, verified=Identity.Verified.AUTOMATIC,
verified_at=make_aware(datetime.datetime.now()), verified_at=timezone.make_aware(datetime.datetime.now()),
) )
feide_id.save() feide_id.save()
logger.info("identity_add", identity_id=feide_id.id, feide_id=new_feide_id) logger.info("identity_add", identity_id=feide_id.id, feide_id=new_feide_id)
...@@ -445,7 +445,7 @@ class GregOIDCBackend(ValidatingOIDCBackend): ...@@ -445,7 +445,7 @@ class GregOIDCBackend(ValidatingOIDCBackend):
old_email = existing_email.value old_email = existing_email.value
existing_email.value = new_email existing_email.value = new_email
existing_email.verified = Identity.Verified.AUTOMATIC existing_email.verified = Identity.Verified.AUTOMATIC
existing_email.verified_at = make_aware(datetime.datetime.now()) existing_email.verified_at = timezone.make_aware(datetime.datetime.now())
existing_email.save() existing_email.save()
logger.info( logger.info(
"identity_update", "identity_update",
...@@ -472,19 +472,19 @@ class GregOIDCBackend(ValidatingOIDCBackend): ...@@ -472,19 +472,19 @@ class GregOIDCBackend(ValidatingOIDCBackend):
type=Identity.IdentityType.FEIDE_EMAIL, type=Identity.IdentityType.FEIDE_EMAIL,
value=new_email, value=new_email,
verified=Identity.Verified.AUTOMATIC, verified=Identity.Verified.AUTOMATIC,
verified_at=make_aware(datetime.datetime.now()), verified_at=timezone.make_aware(datetime.datetime.now()),
) )
email_id.save() email_id.save()
logger.info("identity_add", identity_id=email_id.id, value=new_email) logger.info("identity_add", identity_id=email_id.id, value=new_email)
def _update_person_name( def _update_person_name(
self, person: Person, first_name: str, last_name: str, source: str self, person: Person, first_name: str, last_name: str
) -> None: ) -> None:
new_combined_name = f"{first_name} {last_name}" new_combined_name = f"{first_name} {last_name}"
existing_combined_name = f"{person.first_name} {person.last_name}" existing_combined_name = f"{person.first_name} {person.last_name}"
if name_diff(existing_combined_name, new_combined_name) > 4: if name_diff_too_large(existing_combined_name, new_combined_name, 4):
logger.warning( logger.warning(
"name_diff_to_large", "name_diff_to_large",
existing_name=existing_combined_name, existing_name=existing_combined_name,
...@@ -518,9 +518,7 @@ class GregOIDCBackend(ValidatingOIDCBackend): ...@@ -518,9 +518,7 @@ class GregOIDCBackend(ValidatingOIDCBackend):
self._update_person_feide_id(person, userinfo["userid_feide"], "feide") self._update_person_feide_id(person, userinfo["userid_feide"], "feide")
if "email" in userinfo: if "email" in userinfo:
self._update_person_email(person, userinfo["email"], "feide") self._update_person_email(person, userinfo["email"], "feide")
self._update_person_name( self._update_person_name(person, userinfo["first_name"], userinfo["last_name"])
person, userinfo["first_name"], userinfo["last_name"], "feide"
)
def _update_sponsor_from_userinfo(self, userinfo: dict, sponsor: Sponsor) -> None: def _update_sponsor_from_userinfo(self, userinfo: dict, sponsor: Sponsor) -> None:
"""Updates a sponsor object with data from OIDC.""" """Updates a sponsor object with data from OIDC."""
...@@ -578,26 +576,19 @@ class GregOIDCBackend(ValidatingOIDCBackend): ...@@ -578,26 +576,19 @@ class GregOIDCBackend(ValidatingOIDCBackend):
def _get_or_create_greg_user_profile(self, userinfo: dict, user: UserModel): def _get_or_create_greg_user_profile(self, userinfo: dict, user: UserModel):
"""Get or create a GregUserProfile.""" """Get or create a GregUserProfile."""
def _get_invite_person( def _get_invitation_link(session) -> Optional[InvitationLink]:
session, old_person: Optional[Person]
) -> Optional[Person]:
try: try:
invitation_link = InvitationLink.objects.get(uuid=session["invite_id"]) return InvitationLink.objects.get(uuid=session["invite_id"])
except InvitationLink.DoesNotExist: except InvitationLink.DoesNotExist:
logger.error("invite_not_found", invite_id=session["invite_id"]) logger.error("invite_not_found", invite_id=session["invite_id"])
logger.bind(authenticating_invite=False) logger.bind(authenticating_invite=False)
return old_person return None
def _get_invite_person(invitation_link) -> Person:
logger.bind(authenticating_invite=True) logger.bind(authenticating_invite=True)
role = invitation_link.invitation.role role = invitation_link.invitation.role
invite_person = role.person invite_person = role.person
logger.info("invite_found", invite_person=invite_person.id) logger.info("invite_found", invite_person=invite_person.id)
if old_person and not old_person == invite_person:
role.person = old_person
role.save()
invite_person.delete()
return old_person
return invite_person return invite_person
logger.bind(user=user) logger.bind(user=user)
...@@ -618,7 +609,33 @@ class GregOIDCBackend(ValidatingOIDCBackend): ...@@ -618,7 +609,33 @@ class GregOIDCBackend(ValidatingOIDCBackend):
old_person = None old_person = None
if user_profile and user_profile.person: if user_profile and user_profile.person:
old_person = self._get_person_from_userinfo(userinfo) old_person = self._get_person_from_userinfo(userinfo)
person = _get_invite_person(session, old_person) person = old_person
invitation_link = _get_invitation_link(session)
if invitation_link:
inv_person = _get_invite_person(invitation_link)
if old_person and old_person != inv_person:
# Logged in person has a person object in greg but has followed an
# invite.
inv_name = f"{inv_person.first_name} {inv_person.last_name}"
old_name = f"{old_person.first_name} {old_person.last_name}"
if (
not inv_person.registration_completed_date
and not name_diff_too_large(old_name, inv_name, 4)
):
# The name is close and the invited person has not completed
# registration. Give the role to the existing person, and
# delete the invited one.
role = invitation_link.invitation.role
role.person = old_person
role.save()
inv_person.delete()
else:
# The logged in user has gotten someone else's invitation, and
# the invitation should be disabled.
invitation_link.expire = timezone.now()
invitation_link.save()
else:
person = inv_person
else: else:
person = self._get_person_from_userinfo(userinfo) person = self._get_person_from_userinfo(userinfo)
logger.bind(authenticating_invite=False) logger.bind(authenticating_invite=False)
......
from gregui.utils import name_diff, restricted_damarau_levenshtein from gregui.utils import name_diff, name_diff_too_large, restricted_damarau_levenshtein
def test_rdm_replace(): def test_rdm_replace():
...@@ -21,3 +21,9 @@ def test_name_longer(): ...@@ -21,3 +21,9 @@ def test_name_longer():
def test_name_threshold(): def test_name_threshold():
"""Verify continues with raised threshold""" """Verify continues with raised threshold"""
assert name_diff("Abcdefgh Ijklmnop", "Qrstuvw Xyz", 100) == 10 assert name_diff("Abcdefgh Ijklmnop", "Qrstuvw Xyz", 100) == 10
def test_name_too_large():
"""Verify boolean version responds with expected boolean"""
assert not name_diff_too_large("Test Name", "Testing Name", 3)
assert name_diff_too_large("Test Name", "Testing Name", 2)
...@@ -60,3 +60,7 @@ def name_diff(full_name1: str, full_name2: str, threshold: int = 2) -> int: ...@@ -60,3 +60,7 @@ def name_diff(full_name1: str, full_name2: str, threshold: int = 2) -> int:
if total_difference > threshold: if total_difference > threshold:
break break
return total_difference return total_difference
def name_diff_too_large(full_name1: str, full_name2: str, threshold: int) -> bool:
return name_diff(full_name1, full_name2, threshold) > threshold
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment