Merge pull request #1734 from jleclanche/battlenet/fixes

Battle.net provider fixes

allauth/socialaccount/providers/battlenet/tests.py

@@ -18,15 +18,44 @@ def get_mocked_response(self):
         data = {"battletag": self._battletag, "id": self._uid}
         return MockedResponse(200, json.dumps(data))
 
+    def test_valid_response_no_battletag(self):
+        data = {"id": 12345}
+        response = MockedResponse(200, json.dumps(data))
+        self.assertEqual(_check_errors(response), data)
+
     def test_invalid_data(self):
+        response = MockedResponse(200, json.dumps({}))
         with self.assertRaises(OAuth2Error):
             # No id, raises
-            _check_errors({})
+            _check_errors(response)
+
+    def test_profile_invalid_response(self):
+        data = {"code": 403, "type": "Forbidden", "detail": "Account Inactive"}
+        response = MockedResponse(401, json.dumps(data))
+
+        with self.assertRaises(OAuth2Error):
+            # no id, 4xx code, raises
+            _check_errors(response)
 
+    def test_error_response(self):
+        body = json.dumps({"error": "invalid_token"})
+        response = MockedResponse(400, body)
+
+        with self.assertRaises(OAuth2Error):
+            # no id, 4xx code, raises
+            _check_errors(response)
+
+    def test_service_not_found(self):
+        response = MockedResponse(596, "<h1>596 Service Not Found</h1>")
         with self.assertRaises(OAuth2Error):
-            _check_errors({"error": "invalid_token"})
+            # bad json, 5xx code, raises
+            _check_errors(response)
 
-        _check_errors({"id": 12345})  # Does not raise
+    def test_invalid_response(self):
+        response = MockedResponse(200, "invalid json data")
+        with self.assertRaises(OAuth2Error):
+            # bad json, raises
+            _check_errors(response)
 
     def test_extra_data(self):
         self.login(self.get_mocked_response())

allauth/socialaccount/providers/battlenet/views.py

@@ -24,25 +24,38 @@
 from .provider import BattleNetProvider
 
 
-def _check_errors(data):
-    # The expected output from the Battle.net API follows this format:
-    # {"id": 12345, "battletag": "Example#12345"}
-    # The battletag is optional.
-    if "error" in data:
+def _check_errors(response):
+    try:
+        data = response.json()
+    except ValueError:  # JSONDecodeError on py3
+        raise OAuth2Error(
+            "Invalid JSON from Battle.net API: %r" % (response.text)
+        )
+
+    if response.status_code >= 400 or "error" in data:
         # For errors, we expect the following format:
         # {"error": "error_name", "error_description": "Oops!"}
         # For example, if the token is not valid, we will get:
         # {
         #   "error": "invalid_token",
         #   "error_description": "Invalid access token: abcdef123456"
         # }
-        error = data["error"]
-        desc = data.get("error_description", "")
+        # For the profile API, this may also look like the following:
+        # {"code": 403, "type": "Forbidden", "detail": "Account Inactive"}
+        error = data.get("error", "") or data.get("type", "")
+        desc = data.get("error_description", "") or data.get("detail", "")
+
         raise OAuth2Error("Battle.net error: %s (%s)" % (error, desc))
-    elif "id" not in data:
+
+    # The expected output from the API follows this format:
+    # {"id": 12345, "battletag": "Example#12345"}
+    # The battletag is optional.
+    if "id" not in data:
         # If the id is not present, the output is not usable (no UID)
         raise OAuth2Error("Invalid data from Battle.net API: %r" % (data))
 
+    return data
+
 
 class BattleNetOAuth2Adapter(OAuth2Adapter):
     """
@@ -59,6 +72,9 @@ class BattleNetOAuth2Adapter(OAuth2Adapter):
     @property
     def battlenet_region(self):
         region = self.request.GET.get("region", "").lower()
+        if region == "sea":
+            # South-East Asia uses the same region as US everywhere
+            return "us"
         if region in self.valid_regions:
             return region
         return "us"
@@ -68,7 +84,7 @@ def battlenet_base_url(self):
         region = self.battlenet_region
         if region == "cn":
             return "https://www.battlenet.com.cn"
-        return "https://%s.battle.net" % ("us" if region == "sea" else region)
+        return "https://%s.battle.net" % (region)
 
     @property
     def battlenet_api_url(self):
@@ -91,8 +107,7 @@ def profile_url(self):
     def complete_login(self, request, app, token, **kwargs):
         params = {"access_token": token.token}
         response = requests.get(self.profile_url, params=params)
-        data = response.json()
-        _check_errors(data)
+        data = _check_errors(response)
 
         # Add the region to the data so that we can have it in `extra_data`.
         data["region"] = self.battlenet_region