diff --git a/pirates/admin.py b/pirates/admin.py deleted file mode 100644 index a29303ad8942a81b2216c73fb2afe86950b6bff9..0000000000000000000000000000000000000000 --- a/pirates/admin.py +++ /dev/null @@ -1,22 +0,0 @@ -from django.contrib import admin -from django.contrib.auth import admin as auth_admin -from django.contrib.auth import get_user_model - -from .models import Team - -User = get_user_model() - - -@admin.register(User) -class UserAdmin(auth_admin.UserAdmin): - fieldsets = (("User", {"fields": ("teams",)}),) + auth_admin.UserAdmin.fieldsets - - def has_add_permission(self, request): - return False - - -@admin.register(Team) -class TeamAdmin(admin.ModelAdmin): - def has_add_permission(self, request): - # TODO when auto sync is done, disable manual add - return True diff --git a/pirates/auth.py b/pirates/auth.py index 9b92120dba129289d8c75dcfaf17405acefc7538..e45133e9460ed43e1b44cb3826d86e9451180304 100644 --- a/pirates/auth.py +++ b/pirates/auth.py @@ -1,30 +1,30 @@ from mozilla_django_oidc.auth import OIDCAuthenticationBackend -class CustomOIDCAuthenticationBackend(OIDCAuthenticationBackend): +class PiratesOIDCAuthenticationBackend(OIDCAuthenticationBackend): """ - Custom OIDC Authentication Backend. + Pirates OIDC Authentication Backend. - Instead of `email` uses claim `sub` (as `username`) to identify users. Which + Instead of `email` uses claim `sub` (as `sso_id`) to identify users. Which allows for email change. """ - def get_username(self, claims): + def get_sso_id(self, claims): return claims.get("sub") def filter_users_by_claims(self, claims): - username = self.get_username(claims) - if not username: + sso_id = self.get_sso_id(claims) + if not sso_id: return self.UserModel.objects.none() - return self.UserModel.objects.filter(username=username) + return self.UserModel.objects.filter(sso_id=sso_id) def create_user(self, claims): - username = self.get_username(claims) + sso_id = self.get_sso_id(claims) first_name = claims.get("given_name", "") last_name = claims.get("family_name", "") email = claims.get("email", "") - return self.UserModel.objects.create_user( - username=username, first_name=first_name, last_name=last_name, email=email + return self.UserModel.objects.create( + sso_id=sso_id, first_name=first_name, last_name=last_name, email=email ) def update_user(self, user, claims): diff --git a/pirates/models.py b/pirates/models.py index 581907951e27cd330379134fd5511404e32e124d..bceae18c0979ed31b92fead8f8267c012c4d4f3c 100644 --- a/pirates/models.py +++ b/pirates/models.py @@ -1,20 +1,78 @@ -from django.contrib.auth.models import AbstractUser +from django.conf import settings +from django.contrib.auth.models import PermissionsMixin from django.db import models -from django.urls import reverse +from django.utils import timezone +from django.utils.translation import gettext_lazy as _ -class User(AbstractUser): - teams = models.ManyToManyField("Team") +class AbstractUser(PermissionsMixin): + """ + An abstract base class implementing a fully featured User model with + admin-compliant permissions. + """ - def get_absolute_url(self): - return reverse("users:detail", kwargs={"username": self.username}) + sso_id = models.CharField( + _("SSO ID"), + max_length=150, + unique=True, + error_messages={"unique": _("A user with that SSO ID already exists.")}, + ) + first_name = models.CharField(_("first name"), max_length=150, blank=True) + last_name = models.CharField(_("last name"), max_length=150, blank=True) + email = models.EmailField(_("email address"), blank=True) + is_staff = models.BooleanField( + _("staff status"), + default=False, + help_text=_("Designates whether the user can log into this admin site."), + ) + is_active = models.BooleanField( + _("active"), + default=True, + help_text=_( + "Designates whether this user should be treated as active. " + "Unselect this instead of deleting accounts." + ), + ) + date_joined = models.DateTimeField(_("date joined"), default=timezone.now) + + class Meta: + abstract = True + verbose_name = _("user") + verbose_name_plural = _("users") + + def get_full_name(self): + """Return the first_name plus the last_name, with a space in between.""" + full_name = f"{self.first_name} {self.last_name}" + return full_name.strip() + + def get_short_name(self): + """Return the short name for the user.""" + return self.first_name + + +class AbstractTeam(models.Model): + name = models.CharField(max_length=250, unique=True) + members = models.ManyToManyField(settings.AUTH_USER_MODEL) + + class Meta: + abstract = True + ordering = ["name"] + verbose_name = _("team") + verbose_name_plural = _("teams") + + def __str__(self): + return self.name -class Team(models.Model): - name = models.CharField(max_length=1000) +class AbstractOrgGroup(models.Model): + name = models.CharField(max_length=250, unique=True) + members = models.ManyToManyField(settings.AUTH_USER_MODEL) class Meta: + abstract = True ordering = ["name"] + verbose_name = _("group") + verbose_name_plural = _("groups") def __str__(self): return self.name diff --git a/tests/factories.py b/tests/factories.py index dff336cf50dbd1b6e233177a8115cc8df483d3f5..8035c761a0d27fa05a5f876fe455fdc50bd25c41 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -1,28 +1,15 @@ from django.contrib.auth import get_user_model -from factory import DjangoModelFactory, Faker, post_generation +from factory import DjangoModelFactory, Faker -from pirates.models import Team +from .models import OrgGroup, Team class UserFactory(DjangoModelFactory): - username = Faker("user_name") - email = Faker("email") - - @post_generation - def password(self, create, extracted, **kwargs): - password = Faker( - "password", - length=42, - special_chars=True, - digits=True, - upper_case=True, - lower_case=True, - ).generate(extra_kwargs={}) - self.set_password(password) + sso_id = Faker("random_letters") class Meta: model = get_user_model() - django_get_or_create = ["username"] + django_get_or_create = ["sso_id"] class TeamFactory(DjangoModelFactory): @@ -30,3 +17,12 @@ class TeamFactory(DjangoModelFactory): class Meta: model = Team + django_get_or_create = ["name"] + + +class OrgGroupFactory(DjangoModelFactory): + name = Faker("company") + + class Meta: + model = OrgGroup + django_get_or_create = ["name"] diff --git a/tests/models.py b/tests/models.py new file mode 100644 index 0000000000000000000000000000000000000000..4e227b8f547f3a13d3d490ae3074eff08996d2a4 --- /dev/null +++ b/tests/models.py @@ -0,0 +1,17 @@ +""" +Models for tests. +""" + +from pirates.models import AbstractOrgGroup, AbstractTeam, AbstractUser + + +class User(AbstractUser): + pass + + +class Team(AbstractTeam): + pass + + +class OrgGroup(AbstractOrgGroup): + pass diff --git a/tests/settings.py b/tests/settings.py index 042a15731881f122682d4927259c1f7c9f8a5149..b8820aebca9ca45445bc40c2fcd5ca50b9ca1b4b 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -4,9 +4,11 @@ DEBUG = True USE_TZ = True -CACHES = {"default": {"BACKEND": "django.core.cache.backends.locmem.LocMemCache",}} +CACHES = {"default": {"BACKEND": "django.core.cache.backends.locmem.LocMemCache"}} -DATABASES = {"default": {"ENGINE": "django.db.backends.sqlite3",}} +DATABASES = {"default": {"ENGINE": "django.db.backends.sqlite3"}} + +AUTH_USER_MODEL = "tests.User" ROOT_URLCONF = "pirates.urls" @@ -17,6 +19,7 @@ INSTALLED_APPS = [ "django.contrib.sites", "mozilla_django_oidc", "pirates", + "tests", ] SESSION_ENGINE = "django.contrib.sessions.backends.cache" diff --git a/tests/test_auth.py b/tests/test_auth.py index b0bbffd7c473296f458b975115d5cf016f33bda7..b9f69522fad862d9013822aa8e6de53af5775beb 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -2,7 +2,7 @@ import pytest from django.contrib.auth import get_user_model from faker import Faker -from pirates.auth import CustomOIDCAuthenticationBackend +from pirates.auth import PiratesOIDCAuthenticationBackend pytestmark = pytest.mark.django_db @@ -11,13 +11,13 @@ fake = Faker() @pytest.fixture def backend(): - return CustomOIDCAuthenticationBackend() + return PiratesOIDCAuthenticationBackend() -def test_auth_backend__get_username(backend): - sub = fake.user_name() +def test_auth_backend__get_sso_id(backend): + sub = fake.random_letters() claims = {"sub": sub} - assert backend.get_username(claims) == sub + assert backend.get_sso_id(claims) == sub def test_auth_backend__filter_users_by_claims__missing_sub_in_claims(backend): @@ -27,26 +27,26 @@ def test_auth_backend__filter_users_by_claims__missing_sub_in_claims(backend): def test_auth_backend__filter_users_by_claims__unknown_user(backend): - claims = {"sub": fake.user_name()} + claims = {"sub": fake.random_letters()} users = backend.filter_users_by_claims(claims) assert users.exists() is False def test_auth_backend__filter_users_by_claims__known_user(backend, user): - claims = {"sub": user.username} + claims = {"sub": user.sso_id} users = backend.filter_users_by_claims(claims) assert list(users) == [user] def test_auth_backend__create_user(backend): claims = { - "sub": fake.user_name(), + "sub": fake.random_letters(), "given_name": fake.first_name(), "family_name": fake.last_name(), "email": fake.email(), } user = backend.create_user(claims) - assert user.username == claims["sub"] + assert user.sso_id == claims["sub"] assert user.first_name == claims["given_name"] assert user.last_name == claims["family_name"] assert user.email == claims["email"] @@ -55,7 +55,7 @@ def test_auth_backend__create_user(backend): def test_auth_backend__update_user(backend, user): claims = { - "sub": user.username, + "sub": user.sso_id, "given_name": fake.first_name(), "family_name": fake.last_name(), "email": fake.email(), @@ -64,7 +64,7 @@ def test_auth_backend__update_user(backend, user): assert user.last_name != claims["family_name"] assert user.email != claims["email"] updated_user = backend.update_user(user, claims) - assert updated_user.username == user.username + assert updated_user.sso_id == user.sso_id assert updated_user.first_name == claims["given_name"] assert updated_user.last_name == claims["family_name"] assert updated_user.email == claims["email"]