From 93a0e41f03a937722da7d50b9006384f52b64815 Mon Sep 17 00:00:00 2001
From: Tom Hacohen <tom@stosb.com>
Date: Thu, 14 May 2020 15:42:42 +0300
Subject: [PATCH] Change login flow to better verify all relevant fields.

---
 django_etesync/serializers.py | 20 ++++++-----
 django_etesync/views.py       | 66 ++++++++++++++++++++---------------
 2 files changed, 48 insertions(+), 38 deletions(-)

diff --git a/django_etesync/serializers.py b/django_etesync/serializers.py
index 3497d78..fabc7cb 100644
--- a/django_etesync/serializers.py
+++ b/django_etesync/serializers.py
@@ -244,18 +244,20 @@ class AuthenticationLoginChallengeSerializer(serializers.Serializer):
         raise NotImplementedError()
 
 
-class AuthenticationLoginSerializer(AuthenticationLoginChallengeSerializer):
-    challenge = BinaryBase64Field()
-    host = serializers.CharField()
+class AuthenticationLoginSerializer(serializers.Serializer):
+    response = BinaryBase64Field()
     signature = BinaryBase64Field()
 
-    def validate(self, data):
-        host = self.context.get('host', None)
-        if data['host'] != host:
-            raise serializers.ValidationError(
-                'Found wrong host name. Got: "{}" expected: "{}"'.format(data['host'], host))
+    def create(self, validated_data):
+        raise NotImplementedError()
 
-        return super().validate(data)
+    def update(self, instance, validated_data):
+        raise NotImplementedError()
+
+
+class AuthenticationLoginInnerSerializer(AuthenticationLoginChallengeSerializer):
+    challenge = BinaryBase64Field()
+    host = serializers.CharField()
 
     def create(self, validated_data):
         raise NotImplementedError()
diff --git a/django_etesync/views.py b/django_etesync/views.py
index c628ab0..cb52ca5 100644
--- a/django_etesync/views.py
+++ b/django_etesync/views.py
@@ -40,6 +40,7 @@ from .serializers import (
         AuthenticationSignupSerializer,
         AuthenticationLoginChallengeSerializer,
         AuthenticationLoginSerializer,
+        AuthenticationLoginInnerSerializer,
         CollectionSerializer,
         CollectionItemSerializer,
         CollectionItemRevisionSerializer,
@@ -368,35 +369,42 @@ class AuthenticationViewSet(viewsets.ViewSet):
     def login(self, request):
         from datetime import datetime
 
-        serializer = AuthenticationLoginSerializer(
-            data=request.data, context={'host': request.get_host()})
-        if serializer.is_valid():
-            user = self.get_login_user(serializer)
-            challenge = serializer.validated_data['challenge']
-            signature = serializer.validated_data['signature']
-
-            salt = user.userinfo.salt
-            enc_key = self.get_encryption_key(salt)
-            box = nacl.secret.SecretBox(enc_key)
-
-            challenge_data = json.loads(box.decrypt(challenge).decode())
-            now = int(datetime.now().timestamp())
-            if now - challenge_data['timestamp'] > app_settings.CHALLENGE_VALID_SECONDS:
-                content = {'code': 'challenge_expired', 'detail': 'Login challange has expired'}
-                return Response(content, status=status.HTTP_400_BAD_REQUEST)
-            elif challenge_data['userId'] != user.id:
-                content = {'code': 'wrong_user', 'detail': 'This challenge is for the wrong user'}
-                return Response(content, status=status.HTTP_400_BAD_REQUEST)
-
-            host_hash = nacl.hash.blake2b(
-                serializer.validated_data['host'].encode(), encoder=nacl.encoding.RawEncoder)
-            verify_key = nacl.signing.VerifyKey(user.userinfo.pubkey, encoder=nacl.encoding.RawEncoder)
-            verify_key.verify(challenge + host_hash, signature)
-
-            data = {
-                'token': Token.objects.get_or_create(user=user)[0].key,
-            }
-            return Response(data, status=status.HTTP_200_OK)
+        outer_serializer = AuthenticationLoginSerializer(data=request.data)
+        if outer_serializer.is_valid():
+            response_raw = outer_serializer.validated_data['response']
+            response = json.loads(response_raw.decode())
+            signature = outer_serializer.validated_data['signature']
+
+            serializer = AuthenticationLoginInnerSerializer(data=response, context={'host': request.get_host()})
+            if serializer.is_valid():
+                user = self.get_login_user(serializer)
+                host = serializer.validated_data['host']
+                challenge = serializer.validated_data['challenge']
+
+                salt = user.userinfo.salt
+                enc_key = self.get_encryption_key(salt)
+                box = nacl.secret.SecretBox(enc_key)
+
+                challenge_data = json.loads(box.decrypt(challenge).decode())
+                now = int(datetime.now().timestamp())
+                if now - challenge_data['timestamp'] > app_settings.CHALLENGE_VALID_SECONDS:
+                    content = {'code': 'challenge_expired', 'detail': 'Login challange has expired'}
+                    return Response(content, status=status.HTTP_400_BAD_REQUEST)
+                elif challenge_data['userId'] != user.id:
+                    content = {'code': 'wrong_user', 'detail': 'This challenge is for the wrong user'}
+                    return Response(content, status=status.HTTP_400_BAD_REQUEST)
+                elif host != request.get_host():
+                    detail = 'Found wrong host name. Got: "{}" expected: "{}"'.format(host, request.get_host())
+                    content = {'code': 'wrong_host', 'detail': detail}
+                    return Response(content, status=status.HTTP_400_BAD_REQUEST)
+
+                verify_key = nacl.signing.VerifyKey(user.userinfo.pubkey, encoder=nacl.encoding.RawEncoder)
+                verify_key.verify(response_raw, signature)
+
+                data = {
+                    'token': Token.objects.get_or_create(user=user)[0].key,
+                }
+                return Response(data, status=status.HTTP_200_OK)
 
         return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)