Skip to content

Commit

Permalink
feat: add ProfileNode.availableLoginMethods
Browse files Browse the repository at this point in the history
This extends the information already available from loginMethods
field by adding ceatedAt and userLabel fields to the data. At the
same time loginMethods field is deprecated since this new fields
is meant to replace it.

Refs: HP-2530
  • Loading branch information
charn committed Dec 11, 2024
1 parent 10644b5 commit 2f04126
Show file tree
Hide file tree
Showing 6 changed files with 245 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from snapshottest import Snapshot


snapshots = Snapshot()

snapshots['test_graphql_schema_matches_the_reference 1'] = '''type Query {
Expand Down Expand Up @@ -122,7 +123,8 @@
phones(offset: Int, before: String, after: String, first: Int, last: Int): PhoneNodeConnection
addresses(offset: Int, before: String, after: String, first: Int, last: Int): AddressNodeConnection
contactMethod: ContactMethod
loginMethods: [LoginMethodType]
loginMethods: [LoginMethodType] @deprecated(reason: "This field is deprecated, use availableLoginMethods.")
availableLoginMethods: [LoginMethodNode]
sensitivedata: SensitiveDataNode
serviceConnections(offset: Int, before: String, after: String, first: Int, last: Int): ServiceConnectionTypeConnection
verifiedPersonalInformation: VerifiedPersonalInformationNode
Expand Down Expand Up @@ -222,6 +224,12 @@
SUOMI_FI
}
type LoginMethodNode {
method: LoginMethodType!
createdAt: DateTime
userLabel: String
}
type SensitiveDataNode implements Node {
id: ID!
ssn: String!
Expand Down
35 changes: 25 additions & 10 deletions profiles/keycloak_integration.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import datetime

from django.conf import settings
from django.core.signals import setting_changed
from django.dispatch import receiver
Expand Down Expand Up @@ -105,27 +107,40 @@ def send_profile_changes_to_keycloak(instance):
pass


def get_user_identity_providers(user_id) -> set[str]:
def get_user_identity_providers(user_id) -> list[dict]:
if not _keycloak_admin_client:
return set()
return []

try:
user_data = _keycloak_admin_client.get_user_federated_identities(user_id)
return {ip["identityProvider"] for ip in user_data}
return [{"method": user_data["identityProvider"]} for user_data in user_data]
except keycloak.UserNotFoundError:
return set()
return []


def get_user_credential_types(user_id) -> set[str]:
def get_user_credential_types(user_id) -> list[dict]:
if not _keycloak_admin_client:
return set()
return []

try:
user_data = _keycloak_admin_client.get_user_credentials(user_id)
return {cred["type"] for cred in user_data}
credentials = []
for c in user_data:
created_at = (
datetime.datetime.fromtimestamp(c["createdDate"] / 1000, datetime.UTC)
if c.get("createdDate")
else None
)
credential = {
"method": c["type"],
"created_at": created_at,
"user_label": c.get("userLabel"),
}
credentials.append(credential)
return credentials
except keycloak.UserNotFoundError:
return set()
return []


def get_user_login_methods(user_id) -> set[str]:
return get_user_identity_providers(user_id) | get_user_credential_types(user_id)
def get_user_login_methods(user_id) -> list[dict]:
return get_user_identity_providers(user_id) + get_user_credential_types(user_id)
1 change: 1 addition & 0 deletions profiles/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ class Meta:
"language",
"contact_method",
"login_methods",
"available_login_methods",
]

def resolve_profile(self):
Expand Down
96 changes: 73 additions & 23 deletions profiles/schema.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging
from collections.abc import Iterable
from itertools import chain

import django.dispatch
Expand Down Expand Up @@ -57,7 +58,10 @@
download_connected_service_data,
)
from .enums import AddressType, EmailType, LoginMethodType, PhoneType
from .keycloak_integration import delete_profile_from_keycloak, get_user_login_methods
from .keycloak_integration import (
delete_profile_from_keycloak,
get_user_login_methods,
)
from .models import (
Address,
ClaimToken,
Expand Down Expand Up @@ -92,7 +96,9 @@
AllowedAddressType = graphene.Enum.from_enum(
AddressType, description=lambda e: e.label if e else ""
)

LoginMethodTypeEnum = graphene.Enum.from_enum(
LoginMethodType, description=lambda e: e.label if e else ""
)

"""Provides the updated Profile instance as a keyword argument called `instance`."""
profile_updated = django.dispatch.Signal()
Expand Down Expand Up @@ -438,6 +444,16 @@ def __resolve_reference(self, info, **kwargs):
)


class LoginMethodNode(graphene.ObjectType):
method = LoginMethodTypeEnum(required=True, description="The login method used.")
created_at = graphene.DateTime(
description="Time when the login method was created or edited."
)
user_label = graphene.String(
description="User-friendly label for the login method."
)


class VerifiedPersonalInformationAddressNode(graphene.ObjectType):
street_address = graphene.String(
required=True, description="Street address with possible house number etc."
Expand Down Expand Up @@ -585,9 +601,13 @@ class Meta:
filterset_class = ProfileFilter

login_methods = graphene.List(
graphene.Enum.from_enum(
LoginMethodType, description=lambda e: e.label if e else ""
),
LoginMethodTypeEnum,
description="List of login methods that the profile has used to authenticate. "
"Only visible to the user themselves.",
deprecation_reason="This field is deprecated, use availableLoginMethods.",
)
available_login_methods = graphene.List(
LoginMethodNode,
description="List of login methods that the profile has used to authenticate. "
"Only visible to the user themselves.",
)
Expand All @@ -605,6 +625,42 @@ class Meta:
"privileges to access this information.",
)

@staticmethod
def _has_correct_amr_claim(amr: set):
"""
For future software archeologists:
This field was added to the API to support the front-end's need to know
which login methods the user has used. It's only needed for profiles
with helsinki-tunnus or Suomi.fi, so for other cases, save a couple
API calls and return an empty list. There's no other reasoning for the
logic here.
Can remove this after Tunnistamo is no longer in use. Related ticket: HP-2495
"""
return amr.intersection({"helsinki_tunnus", "heltunnistussuomifi", "suomi_fi"})

@staticmethod
def _get_login_methods(user_uuid, *, extended=False) -> Iterable:
login_methods = get_user_login_methods(user_uuid)

login_methods_in_enum = [
val
for val in login_methods
if val["method"] in enum_values(LoginMethodType)
]

if unknown_login_methods := set([val["method"] for val in login_methods]) - set(
val["method"] for val in login_methods_in_enum
):
logger.warning(
"Found login methods which are not part of the LoginMethodType enum: %s", # noqa: E501
unknown_login_methods,
)

if extended:
return login_methods_in_enum
else:
return [val["method"] for val in login_methods_in_enum]

def resolve_login_methods(self: Profile, info, **kwargs):
if info.context.user != self.user:
raise PermissionDenied(
Expand All @@ -613,26 +669,20 @@ def resolve_login_methods(self: Profile, info, **kwargs):

amr = set(force_list(info.context.user_auth.data.get("amr")))

# For future software archeologists:
# This field was added to the API to support the front-end's need to know
# which login methods the user has used. It's only needed for profiles
# with helsinki-tunnus or Suomi.fi, so for other cases, save a couple
# API calls and return an empty list. There's no other reasoning for the
# logic here.
# Can remove this after Tunnistamo is no longer in use. Related ticket: HP-2495
if amr.intersection({"helsinki_tunnus", "heltunnistussuomifi", "suomi_fi"}):
login_methods = get_user_login_methods(self.user.uuid)
login_methods_in_enum = {
val for val in login_methods if val in enum_values(LoginMethodType)
}
if unknown_login_methods := login_methods - login_methods_in_enum:
logger.warning(
"Found login methods which are not part of the LoginMethodType enum: %s", # noqa: E501
unknown_login_methods,
)
if ProfileNode._has_correct_amr_claim(amr):
return ProfileNode._get_login_methods(self.user.uuid, extended=False)
return []

return login_methods_in_enum
def resolve_available_login_methods(self: Profile, info, **kwargs):
if info.context.user != self.user:
raise PermissionDenied(
"No permission to read login methods of another user."
)

amr = set(force_list(info.context.user_auth.data.get("amr")))

if ProfileNode._has_correct_amr_claim(amr):
return ProfileNode._get_login_methods(self.user.uuid, extended=True)
return []

def resolve_service_connections(self: Profile, info, **kwargs):
Expand Down
57 changes: 48 additions & 9 deletions profiles/tests/test_gql_my_profile_query.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import datetime

import pytest

from open_city_profile.tests import to_graphql_name
Expand Down Expand Up @@ -620,11 +622,21 @@ def test_my_profile_checks_allowed_data_fields_for_multiple_queries(
def test_user_can_see_own_login_methods_with_correct_amr_claim(
user_gql_client, profile, group, service, monkeypatch, amr_claim_value
):
def mock_return(*_, **__):
return {"suomi_fi", "password"}

monkeypatch.setattr(
"profiles.keycloak_integration.get_user_identity_providers", mock_return
"profiles.keycloak_integration.get_user_credential_types",
lambda _: [
{
"created_at": datetime.datetime(
2024, 12, 5, 11, 54, 21, 491000, tzinfo=datetime.UTC
),
"method": "password",
"user_label": None,
},
],
)
monkeypatch.setattr(
"profiles.keycloak_integration.get_user_identity_providers",
lambda _: [{"method": "suomi_fi"}],
)

profile = ProfileFactory(user=user_gql_client.user)
Expand All @@ -634,25 +646,42 @@ def mock_return(*_, **__):
{
myProfile {
loginMethods
availableLoginMethods {
method
createdAt
userLabel
}
}
}
"""
executed = user_gql_client.execute(
query, auth_token_payload={"amr": amr_claim_value}, service=service
)
assert "errors" not in executed
assert set(executed["data"]["myProfile"]["loginMethods"]) == {
assert executed["data"]["myProfile"]["loginMethods"] == [
"SUOMI_FI",
"PASSWORD",
}
]
assert executed["data"]["myProfile"]["availableLoginMethods"] == [
{
"createdAt": None,
"method": "SUOMI_FI",
"userLabel": None,
},
{
"createdAt": "2024-12-05T11:54:21.491000+00:00",
"method": "PASSWORD",
"userLabel": None,
},
]


@pytest.mark.parametrize("amr_claim_value", [None, "helsinkiad"])
def test_user_cannot_see_own_login_methods_with_other_amr_claims(
user_gql_client, profile, group, service, monkeypatch, amr_claim_value
):
def mock_return(*_, **__):
return {"this should not show up"}
raise Exception("This should not be called")

monkeypatch.setattr(
"profiles.keycloak_integration.get_user_identity_providers", mock_return
Expand All @@ -665,6 +694,9 @@ def mock_return(*_, **__):
{
myProfile {
loginMethods
availableLoginMethods {
method
}
}
}
"""
Expand All @@ -673,13 +705,14 @@ def mock_return(*_, **__):
)
assert "errors" not in executed
assert executed["data"]["myProfile"]["loginMethods"] == []
assert executed["data"]["myProfile"]["availableLoginMethods"] == []


def test_user_does_not_see_non_enum_login_methods(
user_gql_client, profile, group, service, monkeypatch
):
def mock_return(*_, **__):
return {"password", "this should not show up"}
return [{"method": "password"}, {"method": "this should not show up"}]

monkeypatch.setattr(
"profiles.keycloak_integration.get_user_identity_providers", mock_return
Expand All @@ -692,11 +725,17 @@ def mock_return(*_, **__):
{
myProfile {
loginMethods
availableLoginMethods {
method
}
}
}
"""
executed = user_gql_client.execute(
query, auth_token_payload={"amr": "helsinki_tunnus"}, service=service
)
assert "errors" not in executed
assert set(executed["data"]["myProfile"]["loginMethods"]) == {"PASSWORD"}
assert executed["data"]["myProfile"]["loginMethods"] == ["PASSWORD"]
assert executed["data"]["myProfile"]["availableLoginMethods"] == [
{"method": "PASSWORD"}
]
Loading

0 comments on commit 2f04126

Please sign in to comment.