Skip to content

Commit

Permalink
fix: anonymiser les urls de profil des utilisateurs (#832)
Browse files Browse the repository at this point in the history
## Description

#### fix 1

⚠️ Le `username` est utilisé dans l'url de la page de profil des
utilisateurs. Il ne doit contenir aucune donnée d'identification.
🦺 Corriger l'hydratation du `username` des utilisateurs créés lors de la
première connection avec le lien magique

#### fix 2

⚠️ Les utilisateurs créés via le magic link, qui se connectent plus tard
via `ProConnect` doivent être reconnus comme existant.
🦺 Mettre à jour l'`identity_provider` de ces utiliisateurs lors de leur
première connection avec `ProConnect`

#### fix 3

🦺 mettre à jour les `username` égaux aux `emails` avec un `uuid4`

## Type de changement

🪲 Correction de bug (changement non cassant qui corrige un problème).
🚧 technique

---------

Co-authored-by: leo-naeka <[email protected]>
  • Loading branch information
vincentporte and leo-naeka authored Nov 26, 2024
1 parent cd4dede commit 6f03e8f
Show file tree
Hide file tree
Showing 9 changed files with 112 additions and 41 deletions.
2 changes: 1 addition & 1 deletion lacommunaute/openid_connect/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def create_or_update_user(self):
except User.DoesNotExist:
try:
# look for a user with the same email but not yet migrated from Inclusion Connect to Pro Connect
user = User.objects.get(email=self.email, identity_provider=IdentityProvider.INCLUSION_CONNECT)
user = User.objects.get(email=self.email)
created = False
except User.DoesNotExist:
user = User.objects.create_user(**user_data_dict)
Expand Down
71 changes: 33 additions & 38 deletions lacommunaute/openid_connect/tests/tests_model.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from unittest import mock

import pytest
from django.test import TestCase, override_settings
from django.utils import timezone

Expand Down Expand Up @@ -102,42 +103,36 @@ def test_update_user_from_user_info(self):
self.assertEqual(1, User.objects.count())
self.assertTrue(ForumProfile.objects.get(user=user))

def test_get_existing_user_with_existing_email(self):
"""
If there already is an existing django user with email OpenID_ sent us, we do not create it again,
We use it and we update it with the data form
"""
proc_user_data = OIDConnectUserData.from_user_info(OIDC_USERINFO)
UserFactory(
first_name=proc_user_data.first_name,
last_name=proc_user_data.last_name,
email=proc_user_data.email,
username=proc_user_data.username,
)
user, created = proc_user_data.create_or_update_user()
self.assertFalse(created)
self.assertEqual(user.last_name, OIDC_USERINFO["usual_name"])
self.assertEqual(user.first_name, OIDC_USERINFO["given_name"])
self.assertEqual(user.username, OIDC_USERINFO["sub"])
self.assertEqual(user.identity_provider, IdentityProvider.PRO_CONNECT)

def test_get_existing_unmigrated_user_with_existing_email(self):
"""
If there already is an existing user with email sent us,
but with another identity provider, we do not create it again,
we use it and we update it with the data form. Identity_provider is set to PRO_CONNECT
"""
proc_user_data = OIDConnectUserData.from_user_info(OIDC_USERINFO)
UserFactory(
first_name=proc_user_data.first_name,
last_name=proc_user_data.last_name,
email=proc_user_data.email,
username="another_username",
identity_provider=IdentityProvider.INCLUSION_CONNECT,
)
user, created = proc_user_data.create_or_update_user()
self.assertFalse(created)
self.assertEqual(user.last_name, OIDC_USERINFO["usual_name"])
self.assertEqual(user.first_name, OIDC_USERINFO["given_name"])
self.assertEqual(user.username, OIDC_USERINFO["sub"])
self.assertEqual(user.identity_provider, IdentityProvider.PRO_CONNECT)
@pytest.fixture(autouse=True)
def setup_proconnect():
constants.OPENID_CONNECT_BASE_URL = "https://openid.connect.fake"
constants.OPENID_CONNECT_REALM = "foobar"
constants.OPENID_CONNECT_CLIENT_ID = "OID_CLIENT_ID_123"
constants.OPENID_CONNECT_CLIENT_SECRET = "OID_CLIENT_SECRET_123"
yield


@pytest.mark.parametrize(
"username,identity_provider",
[
("af6b26f9-85cd-484e-beb9-bea5be13e30f", IdentityProvider.PRO_CONNECT),
("another_username", IdentityProvider.INCLUSION_CONNECT),
("random_username", IdentityProvider.MAGIC_LINK),
],
)
def test_get_existing_email(db, username, identity_provider):
proc_user_data = OIDConnectUserData.from_user_info(OIDC_USERINFO)
UserFactory(
first_name=proc_user_data.first_name,
last_name=proc_user_data.last_name,
email=proc_user_data.email,
username=username,
identity_provider=identity_provider,
)
user, created = proc_user_data.create_or_update_user()
assert not created
assert user.last_name == OIDC_USERINFO["usual_name"]
assert user.first_name == OIDC_USERINFO["given_name"]
assert user.username == OIDC_USERINFO["sub"]
assert user.identity_provider == IdentityProvider.PRO_CONNECT
5 changes: 5 additions & 0 deletions lacommunaute/users/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
from .models import User


class UserAdmin(UserAdmin):
list_display = ("username", "email", "first_name", "last_name", "identity_provider", "is_staff")
list_filter = ("is_staff", "is_superuser", "is_active", "identity_provider", "groups")


admin.site.register(User, UserAdmin)


Expand Down
20 changes: 20 additions & 0 deletions lacommunaute/users/migrations/0004_alter_user_managers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Generated by Django 5.0.9 on 2024-11-26 13:03

from django.db import migrations

import lacommunaute.users.models


class Migration(migrations.Migration):
dependencies = [
("users", "0003_alter_user_identity_provider"),
]

operations = [
migrations.AlterModelManagers(
name="user",
managers=[
("objects", lacommunaute.users.models.UserManager()),
],
),
]
29 changes: 29 additions & 0 deletions lacommunaute/users/migrations/0005_run_management.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Generated by Django 5.0.9 on 2024-11-25 15:15

from uuid import uuid4

from django.db import migrations
from django.db.models import F

from lacommunaute.users.models import User


def cleanup_email_in_username(apps, schema_editor):
users_to_migrate = User.objects.filter(username=F("email"))

while batch_users := users_to_migrate[:1000]:
for user in batch_users:
user.username = str(uuid4())
User.objects.bulk_update(batch_users, ["username"])


class Migration(migrations.Migration):
dependencies = [
("users", "0004_alter_user_managers"),
]

operations = [
migrations.RunPython(cleanup_email_in_username, migrations.RunPython.noop),
]

elidable = True
13 changes: 12 additions & 1 deletion lacommunaute/users/models.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
from django.contrib.auth.models import AbstractUser
from uuid import uuid4

from django.contrib.auth.models import AbstractUser, UserManager as BaseUserManager
from django.db import models

from lacommunaute.users.enums import IdentityProvider


class UserManager(BaseUserManager):
def create_user(self, username=None, email=None, password=None, **extra_fields):
if not username:
username = str(uuid4())
return super().create_user(username, email, password, **extra_fields)


class User(AbstractUser):
identity_provider = models.CharField(
max_length=2,
Expand All @@ -12,5 +21,7 @@ class User(AbstractUser):
blank=False,
)

objects = UserManager()

def __str__(self):
return self.email
9 changes: 9 additions & 0 deletions lacommunaute/users/tests/tests_models.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import re

from lacommunaute.users.models import User


def test_create_user_without_username(db):
user = User.objects.create_user(email="[email protected]")
assert re.match(r"^[a-f0-9]{8}(-[a-f0-9]{4}){3}-[a-f0-9]{12}$", user.username)
assert user.email == "[email protected]"
3 changes: 3 additions & 0 deletions lacommunaute/users/tests/tests_views.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import re
from unittest import mock
from urllib.parse import urlencode

Expand Down Expand Up @@ -184,6 +185,8 @@ def test_post_new_email(
assert str(content) == snapshot(name="create_user_view_content")

created_user = User.objects.get(email=email)
pattern = r"^[a-f0-9]{8}(-[a-f0-9]{4}){3}-[a-f0-9]{12}$"
assert re.match(pattern, created_user.username)
assert created_user.email == email
assert created_user.first_name == "John"
assert created_user.last_name == "Travolta"
Expand Down
1 change: 0 additions & 1 deletion lacommunaute/users/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ def form_valid(self, form):
except User.DoesNotExist:
user = User.objects.create_user(
email=email,
username=email,
first_name=first_name,
last_name=last_name,
identity_provider=IdentityProvider.MAGIC_LINK,
Expand Down

0 comments on commit 6f03e8f

Please sign in to comment.