Refactor app tokens (#9438)

* Save last_4 chars of current app tokens and store their hashes

* Update app mutations, commands and tests

* Update changelog

Co-authored-by: Marcin Gębala <5421321+maarcingebala@users.noreply.github.com>

CHANGELOG.md

@@ -24,6 +24,8 @@ All notable, unreleased changes to this project will be documented in this file.
 - Fix access to own resources by App - #9425 by @korycins
 - Introduce custom prices - #9393 by @IKarbowiak
   - Add `HANDLE_CHECKOUTS` permission (only for apps)
+- Refactor app tokens - #9438 by @IKarbowiak
+  - Store app tokens hashes instead of plain text.
 
 
 # 3.1.7

saleor/app/installation_utils.py

@@ -62,13 +62,11 @@ def install_app(
         )
         extension.permissions.set(extension_data.get("permissions", []))
 
-    token = app.tokens.create(name="Default token")
+    _, token = app.tokens.create(name="Default token")
 
     try:
-        send_app_token(
-            target_url=manifest_data.get("tokenTargetUrl"), token=token.auth_token
-        )
+        send_app_token(target_url=manifest_data.get("tokenTargetUrl"), token=token)
     except requests.RequestException as e:
         app.delete()
         raise e
-    return app
+    return app, token

saleor/app/management/commands/create_app.py

@@ -59,9 +59,9 @@ def handle(self, *args: Any, **options: Any) -> Optional[str]:
         permissions = clean_permissions(permissions)
         app = App.objects.create(name=name, is_active=is_active)
         app.permissions.set(permissions)
-        token_obj = app.tokens.create()
+        _, auth_token = app.tokens.create()
         data = {
-            "auth_token": token_obj.auth_token,
+            "auth_token": auth_token,
         }
         if target_url:
             self.send_app_data(target_url, data)

saleor/app/management/commands/install_app.py

@@ -53,11 +53,10 @@ def handle(self, *args: Any, **options: Any) -> Optional[str]:
             app_job.permissions.set(permissions)
 
         try:
-            app = install_app(app_job, activate)
+            _, token = install_app(app_job, activate)
             app_job.delete()
         except Exception as e:
             app_job.status = JobStatus.FAILED
             app_job.save()
             raise e
-        token = app.tokens.first()
-        return json.dumps({"auth_token": token.auth_token})
+        return json.dumps({"auth_token": token})

saleor/app/migrations/0009_apptoken_token_last_4.py

@@ -0,0 +1,23 @@
+# Generated by Django 3.2.12 on 2022-03-29 07:49
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("app", "0008_appextension_target"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="apptoken",
+            name="token_last_4",
+            field=models.CharField(max_length=4, null=True),
+        ),
+        migrations.AlterField(
+            model_name="apptoken",
+            name="auth_token",
+            field=models.CharField(max_length=128, unique=True),
+        ),
+    ]

saleor/app/migrations/0010_update_app_tokens.py

@@ -0,0 +1,48 @@
+# Generated by Django 3.2.12 on 2022-03-29 10:04
+
+from django.contrib.auth.hashers import make_password
+from django.db import migrations
+
+BATCH_SIZE = 10000
+
+
+def update_app_tokens(apps, schema_editor):
+    AppToken = apps.get_model("app", "AppToken")
+    queryset = AppToken.objects.all()
+    for batch_pks in queryset_in_batches(queryset):
+        app_tokens = AppToken.objects.filter(pk__in=batch_pks)
+        for app_token in app_tokens:
+            app_token.token_last_4 = app_token.auth_token[-4:]
+            app_token.auth_token = make_password(app_token.auth_token)
+
+        AppToken.objects.bulk_update(app_tokens, ["auth_token", "token_last_4"])
+
+
+def queryset_in_batches(queryset):
+    """Slice a queryset into batches.
+
+    Input queryset should be sorted be pk.
+    """
+    start_pk = 0
+
+    while True:
+        qs = queryset.filter(pk__gt=start_pk)[:BATCH_SIZE]
+        pks = list(qs.values_list("pk", flat=True))
+
+        if not pks:
+            break
+
+        yield pks
+
+        start_pk = pks[-1]
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("app", "0009_apptoken_token_last_4"),
+    ]
+
+    operations = [
+        migrations.RunPython(update_app_tokens, migrations.RunPython.noop),
+    ]

saleor/app/migrations/0011_alter_apptoken_token_last_4.py

@@ -0,0 +1,18 @@
+# Generated by Django 3.2.12 on 2022-03-29 10:09
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("app", "0010_update_app_tokens"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="apptoken",
+            name="token_last_4",
+            field=models.CharField(max_length=4),
+        ),
+    ]

saleor/app/models.py

@@ -1,5 +1,6 @@
 from typing import Set
 
+from django.contrib.auth.hashers import make_password
 from django.contrib.auth.models import Permission
 from django.db import models
 from oauthlib.common import generate_token
@@ -99,10 +100,28 @@ def has_perm(self, perm):
         return perm_value in self.get_permissions()
 
 
+class AppTokenManager(models.Manager):
+    def create(self, app, name="", auth_token=None, **extra_fields):
+        """Create an app token with the given name."""
+        if not auth_token:
+            auth_token = generate_token()
+        app_token = self.model(app=app, name=name, **extra_fields)
+        app_token.set_auth_token(auth_token)
+        app_token.save()
+        return app_token, auth_token
+
+
 class AppToken(models.Model):
     app = models.ForeignKey(App, on_delete=models.CASCADE, related_name="tokens")
     name = models.CharField(blank=True, default="", max_length=128)
-    auth_token = models.CharField(default=generate_token, unique=True, max_length=30)
+    auth_token = models.CharField(unique=True, max_length=128)
+    token_last_4 = models.CharField(max_length=4)
+
+    objects = AppTokenManager()
+
+    def set_auth_token(self, raw_token=None):
+        self.auth_token = make_password(raw_token)
+        self.token_last_4 = raw_token[-4:]
 
 
 class AppExtension(models.Model):

saleor/app/tests/test_app_commands.py

@@ -55,8 +55,6 @@ def test_creates_app_from_manifest_sends_token(monkeypatch):
 
     call_command("install_app", manifest_url)
 
-    app = App.objects.get()
-    token = app.tokens.all()[0].auth_token
     mocked_post.assert_called_once_with(
         "http://localhost:3000/register",
         headers={
@@ -65,7 +63,7 @@ def test_creates_app_from_manifest_sends_token(monkeypatch):
             "x-saleor-domain": "mirumee.com",
             "saleor-domain": "mirumee.com",
         },
-        json={"auth_token": token},
+        json={"auth_token": ANY},
         timeout=ANY,
     )
 
@@ -120,15 +118,13 @@ def test_sends_data_to_target_url(monkeypatch):
 
     call_command("create_app", name, permission=permissions, target_url=target_url)
 
-    app = App.objects.filter(name=name)[0]
-    token = app.tokens.all()[0].auth_token
     mocked_post.assert_called_once_with(
         target_url,
         headers={
             # X- headers will be deprecated in Saleor 4.0, proper headers are without X-
             "x-saleor-domain": "mirumee.com",
             "saleor-domain": "mirumee.com",
         },
-        json={"auth_token": token},
+        json={"auth_token": ANY},
         timeout=ANY,
     )

saleor/app/tests/test_installation_utils.py

@@ -23,7 +23,7 @@ def test_install_app_created_app(
     app_installation.permissions.set([permission_manage_products])
 
     # when
-    app = install_app(app_installation, activate=True)
+    app, _ = install_app(app_installation, activate=True)
 
     # then
     assert App.objects.get().id == app.id
@@ -60,7 +60,7 @@ def test_install_app_with_extension(
     )
 
     # when
-    app = install_app(app_installation, activate=True)
+    app, _ = install_app(app_installation, activate=True)
 
     # then
     assert App.objects.get().id == app.id

saleor/graphql/app/mutations.py

@@ -2,7 +2,9 @@
 
 import graphene
 import requests
+from django.contrib.auth.hashers import check_password
 from django.core.exceptions import ValidationError
+from oauthlib.common import generate_token
 
 from ...app import models
 from ...app.error_codes import AppErrorCode
@@ -61,11 +63,13 @@ def perform_mutation(cls, root, info, **data):
         instance = cls.get_instance(info, **data)
         cleaned_input = cls.clean_input(info, instance, input_data)
         instance = cls.construct_instance(instance, cleaned_input)
+        auth_token = generate_token()
+        instance.set_auth_token(auth_token)
         cls.clean_instance(info, instance)
         cls.save(info, instance, cleaned_input)
         cls._save_m2m(info, instance, cleaned_input)
         response = cls.success_response(instance)
-        response.auth_token = instance.auth_token
+        response.auth_token = auth_token
         return response
 
     @classmethod
@@ -120,10 +124,11 @@ class Meta:
     @classmethod
     def perform_mutation(cls, root, info, **data):
         token = data.get("token")
-        app_token = models.AppToken.objects.filter(
-            auth_token=token, app__is_active=True
-        ).first()
-        return AppTokenVerify(valid=bool(app_token))
+        tokens = models.AppToken.objects.filter(
+            app__is_active=True, token_last_4=token[-4:]
+        ).values_list("auth_token", flat=True)
+        valid = any([check_password(token, auth_token) for auth_token in tokens])
+        return AppTokenVerify(valid=valid)
 
 
 class AppCreate(ModelMutation):
@@ -162,19 +167,23 @@ def clean_input(cls, info, instance, data, input_cls=None):
 
     @classmethod
     @staff_member_required
-    def perform_mutation(cls, root, info, **data):
-        return super().perform_mutation(root, info, **data)
+    def perform_mutation(cls, _root, info, **data):
+        instance = cls.get_instance(info, **data)
+        data = data.get("input")
+        cleaned_input = cls.clean_input(info, instance, data)
+        instance = cls.construct_instance(instance, cleaned_input)
+        cls.clean_instance(info, instance)
+        auth_token = cls.save(info, instance, cleaned_input)
+        cls._save_m2m(info, instance, cleaned_input)
+        response = cls.success_response(instance)
+        response.auth_token = auth_token
+        return response
 
     @classmethod
     def save(cls, info, instance, cleaned_input):
         instance.save()
-        instance.tokens.create(name="Default")
-
-    @classmethod
-    def success_response(cls, instance):
-        response = super().success_response(instance)
-        response.auth_token = instance.tokens.get().auth_token
-        return response
+        _, auth_token = instance.tokens.create(name="Default")
+        return auth_token
 
 
 class AppUpdate(ModelMutation):

saleor/graphql/app/tests/mutations/test_app_create.py

@@ -57,7 +57,8 @@ def test_app_create_mutation(
     assert app_data["isActive"] == app.is_active
     assert app_data["name"] == app.name
     assert list(app.permissions.all()) == [permission_manage_products]
-    assert default_token == app.tokens.get().auth_token
+    assert default_token
+    assert default_token[-4:] == app.tokens.get().token_last_4
 
 
 def test_app_is_not_allowed_to_call_create_mutation_for_app(
@@ -135,7 +136,8 @@ def test_app_create_mutation_superuser_can_create_app_with_any_perms(
     assert app_data["isActive"] == app.is_active
     assert app_data["name"] == app.name
     assert list(app.permissions.all()) == [permission_manage_products]
-    assert default_token == app.tokens.get().auth_token
+    assert default_token
+    assert default_token[-4:] == app.tokens.get().token_last_4
 
 
 def test_app_create_mutation_no_permissions(