Repository: saleor/saleor
Pull request: Update last_login on various auth flows
I want to merge this change because it updates `last_login` on authentication flows across saleor.
Resolves https://github.com/saleor/saleor/issues/13435

<!-- Please mention all relevant issue numbers. -->

# Impact

* [ ] New migrations
* [ ] New/Updated API fields or mutations
* [ ] Deprecated API fields or mutations
* [ ] Removed API types, fields, or mutations
* [ ] Documentation needs to be updated

# Pull Request Checklist

<!-- Please keep this section. It will make the maintainer's life easier. -->

* [ ] Privileged queries and mutations are either absent or guarded by proper permission checks
* [ ] Database queries are optimized and the number of queries is constant
* [ ] Database migrations are either absent or optimized for zero downtime
* [ ] The changes are covered by test cases


Problem statement:
externalRefresh (token) mutation does not update the user last_login
### What are you trying to achieve?

Make the user last_login updated when using token related mutations because other mechanisms depends on it.

### Steps to reproduce the problem

Ex. Use externalRefresh mutation. Watch the token is issued but user `last_login` is not updated.

### What did you expect to happen?

The token is issued AND `last_login` is updated. QA can test if oAuth works as expected and when any token-based request is made after 15 mins from one another, user last_login should get updated.

### Logs

_No response_

### Environment

Saleor version: …
OS and version: …

Hints from issue discussion:

Commit message: Fix openid connect

Patch to review:
diff --git a/saleor/graphql/account/mutations/authentication/external_refresh.py b/saleor/graphql/account/mutations/authentication/external_refresh.py
index 4a96bf6751f..f378b37d34c 100644
--- a/saleor/graphql/account/mutations/authentication/external_refresh.py
+++ b/saleor/graphql/account/mutations/authentication/external_refresh.py
@@ -1,4 +1,5 @@
 import graphene
+from django.utils import timezone
 
 from ....core import ResolveInfo
 from ....core.doc_category import DOC_CATEGORY_AUTH
@@ -47,6 +48,8 @@ def perform_mutation(  # type: ignore[override]
 
         if access_tokens_response.user and access_tokens_response.user.id:
             info.context._cached_user = access_tokens_response.user
+            access_tokens_response.user.last_login = timezone.now()
+            access_tokens_response.user.save(update_fields=["last_login", "updated_at"])
 
         return cls(
             token=access_tokens_response.token,
diff --git a/saleor/graphql/account/mutations/authentication/refresh_token.py b/saleor/graphql/account/mutations/authentication/refresh_token.py
index f626de0465a..cf4fb63484b 100644
--- a/saleor/graphql/account/mutations/authentication/refresh_token.py
+++ b/saleor/graphql/account/mutations/authentication/refresh_token.py
@@ -2,6 +2,7 @@
 
 import graphene
 from django.core.exceptions import ValidationError
+from django.utils import timezone
 
 from .....account.error_codes import AccountErrorCode
 from .....core.jwt import (
@@ -133,4 +134,7 @@ def perform_mutation(
         if audience := payload.get("aud"):
             additional_payload["aud"] = audience
         token = create_access_token(user, additional_payload=additional_payload)
+        if user and not user.is_anonymous:
+            user.last_login = timezone.now()
+            user.save(update_fields=["last_login", "updated_at"])
         return cls(errors=[], user=user, token=token)
diff --git a/saleor/graphql/account/tests/mutations/authentication/test_external_refresh.py b/saleor/graphql/account/tests/mutations/authentication/test_external_refresh.py
index de418026477..320e6956acf 100644
--- a/saleor/graphql/account/tests/mutations/authentication/test_external_refresh.py
+++ b/saleor/graphql/account/tests/mutations/authentication/test_external_refresh.py
@@ -1,6 +1,8 @@
 import json
 from unittest.mock import Mock, patch
 
+from freezegun import freeze_time
+
 from ......plugins.base_plugin import ExternalAccessTokens
 from .....tests.utils import get_graphql_content
 
@@ -33,6 +35,7 @@ def test_external_refresh_plugin_not_active(api_client, customer_user):
     assert data["user"] is None
 
 
+@freeze_time("2018-05-31 12:00:00")
 @patch("saleor.core.middleware.jwt_decode_with_exception_handler")
 def test_external_refresh(
     mock_refresh_token_middleware, api_client, customer_user, monkeypatch, rf
@@ -59,5 +62,8 @@ def test_external_refresh(
     assert data["refreshToken"] == expected_refresh_token
     assert data["csrfToken"] == expected_csrf_token
     assert data["user"]["email"] == customer_user.email
+    assert customer_user.last_login
+    last_login = customer_user.last_login.strftime("%Y-%m-%d %H:%M:%S")
+    assert last_login == "2018-05-31 12:00:00"
     assert mocked_plugin_fun.called
     assert mock_refresh_token_middleware.called
diff --git a/saleor/graphql/account/tests/mutations/authentication/test_token_refresh.py b/saleor/graphql/account/tests/mutations/authentication/test_token_refresh.py
index e2754c26ff2..e969dbcbf49 100644
--- a/saleor/graphql/account/tests/mutations/authentication/test_token_refresh.py
+++ b/saleor/graphql/account/tests/mutations/authentication/test_token_refresh.py
@@ -58,6 +58,10 @@ def test_refresh_token_with_audience(api_client, customer_user, settings):
     assert payload["type"] == JWT_ACCESS_TYPE
     assert payload["token"] == customer_user.jwt_token_key
     assert payload["aud"] == token_audience
+    customer_user.refresh_from_db()
+    assert customer_user.last_login
+    last_login = customer_user.last_login.strftime("%Y-%m-%d %H:%M:%S")
+    assert last_login == "2020-03-18 12:00:00"
 
 
 @freeze_time("2020-03-18 12:00:00")
diff --git a/saleor/plugins/openid_connect/tests/test_utils.py b/saleor/plugins/openid_connect/tests/test_utils.py
index ea514d4187c..47e045cce38 100644
--- a/saleor/plugins/openid_connect/tests/test_utils.py
+++ b/saleor/plugins/openid_connect/tests/test_utils.py
@@ -1,7 +1,7 @@
 import json
 import time
 import warnings
-from datetime import datetime
+from datetime import datetime, timedelta
 from unittest import mock
 from unittest.mock import MagicMock, Mock
 
@@ -10,7 +10,7 @@
 import requests
 from authlib.jose import JWTClaims
 from django.core.exceptions import ValidationError
-from django.utils.timezone import make_aware
+from django.utils import timezone
 from freezegun import freeze_time
 from requests import Response
 
@@ -381,7 +381,7 @@ def test_get_or_create_user_from_payload_with_last_login(customer_user, settings
     oauth_url = "https://saleor.io/oauth"
     sub_id = "oauth|1234"
 
-    customer_user.last_login = make_aware(
+    customer_user.last_login = timezone.make_aware(
         datetime.fromtimestamp(current_ts - 10), timezone=pytz.timezone("UTC")
     )
     customer_user.save()
@@ -393,13 +393,60 @@ def test_get_or_create_user_from_payload_with_last_login(customer_user, settings
     )
 
     customer_user.refresh_from_db()
-    assert customer_user.last_login == make_aware(
+    assert customer_user.last_login == timezone.make_aware(
         datetime.fromtimestamp(current_ts), timezone=pytz.timezone("UTC")
     )
     assert user_from_payload.email == customer_user.email
     assert user_from_payload.private_metadata[f"oidc-{oauth_url}"] == sub_id
 
 
+@freeze_time("2019-03-18 12:00:00")
+def test_get_or_create_user_from_payload_update_last_login(customer_user, settings):
+    assert customer_user.last_login is None
+    oauth_url = "https://saleor.io/oauth"
+    sub_id = "oauth|1234"
+    get_or_create_user_from_payload(
+        payload={"sub": sub_id, "email": customer_user.email},
+        oauth_url=oauth_url,
+    )
+
+    customer_user.refresh_from_db()
+    assert customer_user.last_login
+    last_login = customer_user.last_login.strftime("%Y-%m-%d %H:%M:%S")
+    assert last_login == "2019-03-18 12:00:00"
+
+
+def test_get_or_create_user_from_payload_last_login_stays_same(customer_user, settings):
+    last_login = timezone.now() - timedelta(hours=11)
+    customer_user.last_login = last_login
+    customer_user.save()
+    oauth_url = "https://saleor.io/oauth"
+    sub_id = "oauth|1234"
+    get_or_create_user_from_payload(
+        payload={"sub": sub_id, "email": customer_user.email},
+        oauth_url=oauth_url,
+    )
+
+    customer_user.refresh_from_db()
+    assert customer_user.last_login == last_login
+
+
+def test_get_or_create_user_from_payload_last_login_modifies(customer_user, settings):
+    last_login = timezone.now() - timedelta(days=2)
+    customer_user.last_login = last_login
+    customer_user.save()
+    oauth_url = "https://saleor.io/oauth"
+    sub_id = "oauth|1234"
+    get_or_create_user_from_payload(
+        payload={"sub": sub_id, "email": customer_user.email},
+        oauth_url=oauth_url,
+    )
+
+    customer_user.refresh_from_db()
+    assert customer_user.last_login
+    assert customer_user.last_login != last_login
+
+
 def test_jwt_token_without_expiration_claim(monkeypatch, decoded_access_token):
     monkeypatch.setattr(
         "saleor.plugins.openid_connect.utils.get_user_info_from_cache_or_fetch",
diff --git a/saleor/plugins/openid_connect/utils.py b/saleor/plugins/openid_connect/utils.py
index e8ee59ac90d..7432e997e2c 100644
--- a/saleor/plugins/openid_connect/utils.py
+++ b/saleor/plugins/openid_connect/utils.py
@@ -12,7 +12,7 @@
 from django.core.exceptions import ValidationError
 from django.core.validators import URLValidator
 from django.db.models import QuerySet
-from django.utils.timezone import make_aware
+from django.utils import timezone
 from jwt import PyJWTError
 
 from ...account.models import Group, User
@@ -457,9 +457,13 @@ def _update_user_details(
         fields_to_save.append("email")
     if last_login:
         if not user.last_login or user.last_login.timestamp() < last_login:
-            login_time = make_aware(datetime.fromtimestamp(last_login))
+            login_time = timezone.make_aware(datetime.fromtimestamp(last_login))
             user.last_login = login_time
             fields_to_save.append("last_login")
+    else:
+        if not user.last_login or (timezone.now() - user.last_login).days > 1:
+            user.last_login = timezone.now()
+            fields_to_save.append("last_login")
 
     if fields_to_save:
         user.save(update_fields=fields_to_save)


Code review comments:
diff_hunk: @@ -457,9 +457,13 @@ def _update_user_details(
         fields_to_save.append("email")
     if last_login:
         if not user.last_login or user.last_login.timestamp() < last_login:
-            login_time = make_aware(datetime.fromtimestamp(last_login))
+            login_time = timezone.make_aware(datetime.fromtimestamp(last_login))
             user.last_login = login_time
             fields_to_save.append("last_login")
+    else:
+        if not user.last_login or (timezone.now() - user.last_login).days > 1:
path: saleor/plugins/openid_connect/utils.py
text: @author:
Do not update last_login on each user request (`authenticate_user` func) , but update it if it's older than a day