HP-2530 | Include more information for profile's login methods

.pre-commit-config.yaml

@@ -4,24 +4,28 @@ default_install_hook_types: [pre-commit, commit-msg]
 default_stages: [pre-commit, manual]
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.5.0
+    rev: v5.0.0
     hooks:
       - id: trailing-whitespace
       - id: end-of-file-fixer
+        exclude: open_city_profile/static/open-city-profile/swagger/
       - id: check-yaml
       - id: check-toml
       - id: check-added-large-files
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.7.1
+    rev: v0.8.0
     hooks:
       - id: ruff
         name: ruff lint
       - id: ruff-format
         name: ruff format
-        args: [ --check ]
   - repo: https://github.com/alessandrojcm/commitlint-pre-commit-hook
-    rev: v9.18.0
+    rev: v9.19.0
     hooks:
       - id: commitlint
         stages: [commit-msg, manual]
         additional_dependencies: ["@commitlint/config-conventional"]
+  - repo: https://github.com/koalaman/shellcheck-precommit
+    rev: v0.10.0
+    hooks:
+      - id: shellcheck

Dockerfile

@@ -27,12 +27,11 @@ COPY --chown=appuser:appuser docker-entrypoint.sh /entrypoint/docker-entrypoint.
 ENTRYPOINT ["/entrypoint/docker-entrypoint.sh"]
 
 # ==============================
-FROM appbase as development
+FROM appbase AS development
 # ==============================
 
 RUN pip install --no-cache-dir -r /app/requirements-dev.txt
 
-
 ENV DEV_SERVER=1
 
 COPY --chown=appuser:appuser . /app/
@@ -41,15 +40,15 @@ USER appuser
 EXPOSE 8080/tcp
 
 # ==============================
-FROM appbase as staticbuilder
+FROM appbase AS staticbuilder
 # ==============================
 
 ENV VAR_ROOT /app
 COPY --chown=appuser:appuser . /app
 RUN python manage.py collectstatic --noinput
 
 # ==============================
-FROM appbase as production
+FROM appbase AS production
 # ==============================
 
 COPY --from=staticbuilder --chown=appuser:appuser /app/static /app/static

docker-entrypoint.sh

@@ -2,7 +2,7 @@
 
 set -e
 
-if [ -z "$SKIP_DATABASE_CHECK" -o "$SKIP_DATABASE_CHECK" = "0" ]; then
+if [ -z "$SKIP_DATABASE_CHECK" ] || [ "$SKIP_DATABASE_CHECK" = "0" ]; then
     until nc --verbose --wait 30 -z "$DATABASE_HOST" 5432
     do
       echo "Waiting for postgres database connection..."
@@ -35,10 +35,10 @@ if [[ "$CREATE_SUPERUSER" = "1" ]]; then
 fi
 
 # Start server
-if [[ ! -z "$@" ]]; then
+if [[ -n "$*" ]]; then
     "$@"
 elif [[ "$DEV_SERVER" = "1" ]]; then
-    python ./manage.py runserver 0.0.0.0:8080
+    python -Wd ./manage.py runserver 0.0.0.0:8080
 else
     uwsgi --ini .prod/uwsgi.ini
 fi

open_city_profile/tests/snapshots/snap_test_graphql_api_schema.py

@@ -4,6 +4,7 @@
 
 from snapshottest import Snapshot
 
+
 snapshots = Snapshot()
 
 snapshots['test_graphql_schema_matches_the_reference 1'] = '''type Query {
@@ -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
@@ -222,6 +224,12 @@
   SUOMI_FI
 }
 
+type LoginMethodNode {
+  method: LoginMethodType!
+  createdAt: DateTime
+  userLabel: String
+}
+
 type SensitiveDataNode implements Node {
   id: ID!
   ssn: String!

profiles/keycloak_integration.py

@@ -1,3 +1,5 @@
+import datetime
+
 from django.conf import settings
 from django.core.signals import setting_changed
 from django.dispatch import receiver
@@ -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)

profiles/models.py

@@ -121,6 +121,7 @@ class Meta:
         "language",
         "contact_method",
         "login_methods",
+        "available_login_methods",
     ]
 
     def resolve_profile(self):

profiles/schema.py

@@ -1,4 +1,5 @@
 import logging
+from collections.abc import Iterable
 from itertools import chain
 
 import django.dispatch
@@ -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,
@@ -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()
@@ -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."
@@ -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.",
     )
@@ -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(
@@ -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):