Skip to content

Commit

Permalink
IDPF-275 Enable creation of inactive users via sso (#94)
Browse files Browse the repository at this point in the history
  • Loading branch information
alsyx authored Feb 7, 2025
1 parent 4fbbf11 commit 4851ab2
Show file tree
Hide file tree
Showing 16 changed files with 172 additions and 91 deletions.
2 changes: 1 addition & 1 deletion core/api/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
def get_user(request, id: str):
"""Just a demo, do not build against this."""
try:
return core_services.get_by_id(id)
return core_services.get_identity_by_id(id=id)
except Profile.DoesNotExist:
return 404, {
"message": "Unable to find user",
Expand Down
2 changes: 1 addition & 1 deletion core/api/people_finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
def get_user(request, id: str):
"""Just a demo, do not build against this"""
try:
return core_services.get_by_id(id)
return core_services.get_identity_by_id(id=id)
except Profile.DoesNotExist:
return 404, {
"message": "Unable to find user",
Expand Down
12 changes: 4 additions & 8 deletions core/api/scim.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
def get_user(request, id: str):
"""Returns the Identity record (internally: Profile) with the given ID"""
try:
return core_services.get_by_id(id)
return core_services.get_identity_by_id(id=id, include_inactive=True)
except Profile.DoesNotExist:
return 404, {
"status": "404",
Expand All @@ -45,11 +45,6 @@ def get_user(request, id: str):
)
def create_user(request, scim_user: CreateUserRequest) -> tuple[int, User | dict]:
"""Creates the given Identity record; will not update"""
if not scim_user.active:
return 400, {
"status": "400",
"detail": "Cannot create inactive user via SCIM",
}

if not scim_user.emails:
return 400, {
Expand All @@ -67,6 +62,7 @@ def create_user(request, scim_user: CreateUserRequest) -> tuple[int, User | dict
all_emails=emails,
primary_email=scim_user.get_primary_email(),
contact_email=scim_user.get_contact_email(),
is_active=scim_user.active,
)
return 201, user

Expand Down Expand Up @@ -96,7 +92,7 @@ def update_user(
primary_email = scim_user.get_primary_email()
contact_email = scim_user.get_contact_email()

profile = core_services.get_by_id(id=id)
profile = core_services.get_identity_by_id(id=id, include_inactive=True)

core_services.update_identity(
profile=profile,
Expand All @@ -122,7 +118,7 @@ def delete_user(
) -> int | tuple[int, dict]:
"""Deleted the Identity record with the given ID"""
try:
profile = core_services.get_by_id(id=id)
profile = core_services.get_identity_by_id(id=id, include_inactive=True)
core_services.delete_identity(
profile=profile,
)
Expand Down
2 changes: 1 addition & 1 deletion core/api/sso_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
def get_user(request, id: str):
"""Optimised, low-flexibility endpoint to return a minimal Identity record (internally: Profile)"""
try:
return core_services.get_by_id(id)
return core_services.get_identity_by_id(id=id)
except Profile.DoesNotExist:
return 404, {
"message": "Unable to find user",
Expand Down
5 changes: 5 additions & 0 deletions core/management/commands/create_identity_fixture.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ def handle(self, *args, **kwargs):
last_name = kwargs["last_name"]
staff = kwargs["staff"]
superuser = kwargs["superuser"]
is_active = kwargs["is_active"]
dry_run = kwargs["dry_run"]

factory_faker = faker.Faker()
Expand All @@ -50,6 +51,9 @@ def handle(self, *args, **kwargs):
if last_name is None:
last_name = factory_faker.last_name()

if is_active is None:
is_active = factory_faker.is_active()

self.stdout.write(
f"Creating user with id: {sso_email_id}, email: {email}, first_name: {first_name} and last_name: {last_name}"
)
Expand All @@ -62,6 +66,7 @@ def handle(self, *args, **kwargs):
first_name=first_name,
last_name=last_name,
all_emails=[email],
is_active=is_active,
)

if superuser:
Expand Down
91 changes: 49 additions & 42 deletions core/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,21 @@
logger = logging.getLogger(__name__)


def get_by_id(id: str):
return profile_services.get_by_id(sso_email_id=id)
def get_identity_by_id(id: str, include_inactive: bool = False) -> Profile:
"""
Retrieve an identity by its User ID.
"""
return profile_services.get_by_id(
sso_email_id=id, include_inactive=include_inactive
)


def create_identity(
id: str,
first_name: str,
last_name: str,
all_emails: list[str],
is_active: bool,
primary_email: str | Unset | None = None,
contact_email: str | Unset | None = None,
) -> Profile:
Expand All @@ -43,7 +49,10 @@ def create_identity(
Returns the combined Profile
"""
user_services.create(sso_email_id=id)
user_services.create(
sso_email_id=id,
is_active=is_active,
)
return profile_services.create_from_sso(
sso_email_id=id,
first_name=first_name,
Expand All @@ -67,6 +76,13 @@ def update_identity(
Function for updating an existing user (archive / unarchive) and their profile information.
"""

user = User.objects.get(sso_email_id=profile.sso_email_id)
if user.is_active != is_active:
if user.is_active == False:
user_services.unarchive(user)
else:
user_services.archive(user)

profile_services.update_from_sso(
profile=profile,
first_name=first_name,
Expand All @@ -76,13 +92,6 @@ def update_identity(
contact_email=contact_email,
)

user = User.objects.get(sso_email_id=profile.sso_email_id)
if user.is_active != is_active:
if is_active:
user_services.unarchive(user)
else:
user_services.archive(user)


def delete_identity(profile: Profile) -> None:
"""
Expand Down Expand Up @@ -123,7 +132,7 @@ def bulk_delete_identity_users_from_sso(sso_users: list[dict[str, Any]]) -> None

id_users_to_delete = id_users.exclude(sso_email_id__in=sso_user_ids)
for user in id_users_to_delete:
profile = get_by_id(user.sso_email_id)
profile = get_identity_by_id(user.sso_email_id, include_inactive=True)
# log Staff SSO objects that are no longer in the S3 file.
logger.info(f"ingest_staff_sso_s3: Deactivating account {user.sso_email_id}")

Expand All @@ -137,40 +146,38 @@ def bulk_create_and_update_identity_users_from_sso(
Creates and updates existing Staff SSO users in the Identity database
"""
id_user_ids = User.objects.all().values_list("sso_email_id", flat=True)

for sso_user in sso_users:
if sso_user[SSO_USER_STATUS] == "active":
if sso_user[SSO_USER_EMAIL_ID] not in id_user_ids:
primary_email, contact_email, all_emails = extract_emails_from_sso_user(
sso_user
)
create_identity(
id=sso_user[SSO_USER_EMAIL_ID],
first_name=sso_user[SSO_FIRST_NAME],
last_name=sso_user[SSO_LAST_NAME],
all_emails=all_emails,
primary_email=primary_email,
contact_email=contact_email,
)
else:
profile = get_by_id(id=sso_user[SSO_USER_EMAIL_ID])
primary_email, contact_email, all_emails = extract_emails_from_sso_user(
sso_user
)
update_identity(
profile=profile,
first_name=sso_user[SSO_FIRST_NAME],
last_name=sso_user[SSO_LAST_NAME],
all_emails=all_emails,
is_active=(
True if sso_user[SSO_USER_STATUS] == "active" else False
),
primary_email=primary_email,
contact_email=contact_email,
)
if sso_user[SSO_USER_EMAIL_ID] not in id_user_ids:
primary_email, contact_email, all_emails = extract_emails_from_sso_user(
sso_user
)
create_identity(
id=sso_user[SSO_USER_EMAIL_ID],
first_name=sso_user[SSO_FIRST_NAME],
last_name=sso_user[SSO_LAST_NAME],
all_emails=all_emails,
is_active=sso_user[SSO_USER_STATUS] == "active",
primary_email=primary_email,
contact_email=contact_email,
)
else:
profile = get_identity_by_id(
id=sso_user[SSO_USER_EMAIL_ID], include_inactive=True
)
primary_email, contact_email, all_emails = extract_emails_from_sso_user(
sso_user
)
update_identity(
profile=profile,
first_name=sso_user[SSO_FIRST_NAME],
last_name=sso_user[SSO_LAST_NAME],
all_emails=all_emails,
is_active=sso_user[SSO_USER_STATUS] == "active",
primary_email=primary_email,
contact_email=contact_email,
)
# if inactive sso user is currently active in ID, they should be archived
if sso_user[SSO_USER_EMAIL_ID] in id_user_ids:
if sso_user[SSO_USER_STATUS] == "inactive":
user = User.objects.get(sso_email_id=sso_user[SSO_USER_EMAIL_ID])
if user.is_active:
user_services.archive(user)
Expand Down
35 changes: 26 additions & 9 deletions core/tests/test_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,24 @@ def test_create_identity(mocker):
)
# User is created
profile = services.create_identity(
id="john[email protected]",
id="billy[email protected]",
first_name="Billy",
last_name="Bob",
all_emails=[
"[email protected]",
"[email protected]",
"[email protected]",
],
is_active=False,
primary_email="[email protected]",
contact_email="[email protected]",
)
mock_user_create.assert_called_once_with(
sso_email_id="[email protected]",
sso_email_id="[email protected]",
is_active=False,
)
mock_profiles_create.assert_called_once_with(
sso_email_id="john[email protected]",
sso_email_id="billy[email protected]",
first_name="Billy",
last_name="Bob",
all_emails=[
Expand All @@ -64,6 +66,7 @@ def test_existing_user(basic_user) -> None:
first_name="Billy",
last_name="Bob",
all_emails=["[email protected]"],
is_active=True,
)


Expand All @@ -73,6 +76,7 @@ def test_new_user() -> None:
first_name="Billy",
last_name="Bob",
all_emails=["[email protected]"],
is_active=True,
)
assert isinstance(profile, Profile)
assert profile.pk
Expand All @@ -86,6 +90,7 @@ def test_update_identity() -> None:
"Billy",
"Bob",
["[email protected]"],
is_active=True,
)
assert User.objects.get(sso_email_id="[email protected]").is_active

Expand All @@ -109,14 +114,16 @@ def test_delete_identity() -> None:
"Billy",
"Bob",
["[email protected]"],
is_active=True,
)

services.delete_identity(
profile,
)
with pytest.raises(Profile.DoesNotExist) as pex:
services.get_by_id(
services.get_identity_by_id(
id=profile.sso_email_id,
include_inactive=True,
)

assert str(pex.value.args[0]) == "Profile matching query does not exist."
Expand All @@ -135,12 +142,14 @@ def test_bulk_delete_identity_users_from_sso(mocker) -> None:
first_name="Billy",
last_name="Bob",
all_emails=["[email protected]"],
is_active=True,
)
services.create_identity(
id="[email protected]",
first_name="Gilly",
last_name="Bob",
all_emails=["[email protected]"],
is_active=True,
)

mock_delete_identity = mocker.patch(
Expand All @@ -152,14 +161,16 @@ def test_bulk_delete_identity_users_from_sso(mocker) -> None:
SSO_USER_EMAIL_ID: "[email protected]",
SSO_FIRST_NAME: "Gilly",
SSO_LAST_NAME: "Bob",
SSO_USER_STATUS: "active",
SSO_USER_STATUS: "inactive",
SSO_EMAIL_ADDRESSES: ["[email protected]"],
SSO_CONTACT_EMAIL_ADDRESS: "[email protected]",
},
]

profile1_to_delete = services.get_by_id("[email protected]")
profile2_to_delete = services.get_by_id(id)
profile1_to_delete = services.get_identity_by_id(
"[email protected]", include_inactive=True
)
profile2_to_delete = services.get_identity_by_id(id, include_inactive=True)
calls = [call(profile=profile1_to_delete), call(profile=profile2_to_delete)]

services.bulk_delete_identity_users_from_sso(sso_users=sso_users)
Expand All @@ -173,6 +184,7 @@ def test_bulk_create_and_update_identity_users_from_sso(mocker) -> None:
first_name="Gilly",
last_name="Bob",
all_emails=["[email protected]"],
is_active=True,
)
mock_create_identity = mocker.patch(
"core.services.create_identity", return_value="__profile__"
Expand All @@ -194,7 +206,7 @@ def test_bulk_create_and_update_identity_users_from_sso(mocker) -> None:
SSO_USER_EMAIL_ID: "[email protected]",
SSO_FIRST_NAME: "Alice",
SSO_LAST_NAME: "Smith",
SSO_USER_STATUS: "active",
SSO_USER_STATUS: "inactive",
SSO_EMAIL_ADDRESSES: ["[email protected]"],
SSO_CONTACT_EMAIL_ADDRESS: "[email protected]",
},
Expand All @@ -205,11 +217,14 @@ def test_bulk_create_and_update_identity_users_from_sso(mocker) -> None:
first_name="Alice",
last_name="Smith",
all_emails=["[email protected]", "[email protected]"],
is_active=False,
primary_email="[email protected]",
contact_email="[email protected]",
)
mock_update_identity.assert_called_once_with(
profile=services.get_by_id("[email protected]"),
profile=services.get_identity_by_id(
id="[email protected]", include_inactive=True
),
first_name="Jane",
last_name="Doe",
all_emails=["[email protected]", "[email protected]"],
Expand All @@ -225,12 +240,14 @@ def test_sync_bulk_sso_users(mocker) -> None:
first_name="Billy",
last_name="Bob",
all_emails=["[email protected]"],
is_active=True,
)
services.create_identity(
id="[email protected]",
first_name="Gilly",
last_name="Bob",
all_emails=["[email protected]"],
is_active=True,
)

mock_get_bulk_user_records = mocker.patch(
Expand Down
Loading

0 comments on commit 4851ab2

Please sign in to comment.