feat(bitbucket): Migrate to OAuth 2.0

apps/codecov-api/codecov_auth/tests/unit/views/test_bitbucket.py

@@ -8,18 +8,16 @@
 from codecov_auth.models import Owner
 from codecov_auth.views.bitbucket import BitbucketLoginView
 from shared.torngit.bitbucket import Bitbucket
-from shared.torngit.exceptions import TorngitServer5xxCodeError
-from utils.encryption import encryptor
+from shared.torngit.exceptions import (
+    TorngitClientGeneralError,
+)
 
 
 def test_get_bitbucket_redirect(client, settings, mocker):
-    mocked_get = mocker.patch.object(
+    mocked_generate = mocker.patch.object(
         Bitbucket,
-        "generate_request_token",
-        return_value={
-            "oauth_token": "testy6r2of6ajkmrub",
-            "oauth_token_secret": "testzibw5q01scpl8qeeupzh8u9yu8hz",
-        },
+        "generate_redirect_url",
+        return_value="https://bitbucket.org/site/oauth2/authorize?client_id=testqmo19ebdkseoby&response_type=code&redirect_uri=http%3A%2F%2Flocalhost&state=teststate",
     )
     settings.BITBUCKET_REDIRECT_URI = "http://localhost"
     settings.BITBUCKET_CLIENT_ID = "testqmo19ebdkseoby"
@@ -28,30 +26,30 @@ def test_get_bitbucket_redirect(client, settings, mocker):
     res = client.get(url, SERVER_NAME="localhost:8000")
     assert res.status_code == 302
 
-    assert "_oauth_request_token" in res.cookies
-    cookie = res.cookies["_oauth_request_token"]
+    assert "_bb_oauth_state" in res.cookies
+    cookie = res.cookies["_bb_oauth_state"]
     assert cookie.value
     assert cookie.get("domain") == settings.COOKIES_DOMAIN
-    assert (
-        res.url
-        == "https://bitbucket.org/api/1.0/oauth/authenticate?oauth_token=testy6r2of6ajkmrub"
-    )
-    mocked_get.assert_called_with(settings.BITBUCKET_REDIRECT_URI)
+    assert mocked_generate.call_count == 1
+    # state kwarg was passed through
+    _, kwargs = mocked_generate.call_args
+    assert kwargs.get("state") is not None
 
 
-def test_get_bitbucket_redirect_bitbucket_unavailable(client, settings, mocker):
-    mocked_get = mocker.patch.object(
-        Bitbucket, "generate_request_token", side_effect=TorngitServer5xxCodeError()
+def test_get_bitbucket_redirect_bitbucket_error(client, settings, mocker):
+    mocker.patch.object(
+        Bitbucket,
+        "generate_redirect_url",
+        side_effect=TorngitClientGeneralError(400, {}, "bad request"),
     )
     settings.BITBUCKET_REDIRECT_URI = "http://localhost"
     settings.BITBUCKET_CLIENT_ID = "testqmo19ebdkseoby"
     settings.BITBUCKET_CLIENT_SECRET = "testfi8hzehvz453qj8mhv21ca4rf83f"
     url = reverse("bitbucket-login")
     res = client.get(url, SERVER_NAME="localhost:8000")
     assert res.status_code == 302
-    assert "_oauth_request_token" not in res.cookies
+    assert "_bb_oauth_state" not in res.cookies
     assert res.url == url
-    mocked_get.assert_called_with(settings.BITBUCKET_REDIRECT_URI)
 
 
 async def fake_get_authenticated_user():
@@ -110,23 +108,22 @@ async def fake_list_teams():
         "generate_access_token",
         return_value={
             "key": "test6tl3evq7c8vuyn",
-            "secret": "testdm61tppb5x0tam7nae3qajhcepzz",
+            "secret": "testrefreshtoken",
         },
     )
     settings.BITBUCKET_REDIRECT_URI = "http://localhost"
     settings.BITBUCKET_CLIENT_ID = "testqmo19ebdkseoby"
     settings.BITBUCKET_CLIENT_SECRET = "testfi8hzehvz453qj8mhv21ca4rf83f"
     settings.CODECOV_DASHBOARD_URL = "dashboard.value"
     settings.COOKIE_SECRET = "aaaaa"
+
+    state = "test_state_value_abc123"
     url = reverse("bitbucket-login")
-    oauth_request_token = (
-        "dGVzdDZ0bDNldnE3Yzh2dXlu|dGVzdGRtNjF0cHBiNXgwdGFtN25hZTNxYWpoY2Vweno="
-    )
     client.cookies = SimpleCookie(
         {
-            "_oauth_request_token": signing.get_cookie_signer(
-                salt="_oauth_request_token"
-            ).sign(encryptor.encode(oauth_request_token).decode())
+            "_bb_oauth_state": signing.get_cookie_signer(salt="_bb_oauth_state").sign(
+                state
+            )
         }
     )
     mock_create_user_onboarding_metric = mocker.patch(
@@ -135,17 +132,17 @@ async def fake_list_teams():
 
     res = client.get(
         url,
-        {"oauth_verifier": 8519288973, "oauth_token": "test1daxl4jnhegoh4"},
+        {"code": "auth_code_from_bitbucket", "state": state},
         SERVER_NAME="localhost:8000",
     )
     assert res.status_code == 302
     assert res.url == "dashboard.value/bb"
-    assert "_oauth_request_token" in res.cookies
-    cookie = res.cookies["_oauth_request_token"]
+    assert "_bb_oauth_state" in res.cookies
+    cookie = res.cookies["_bb_oauth_state"]
     assert cookie.value == ""
     assert cookie.get("domain") == settings.COOKIES_DOMAIN
     mocked_get.assert_called_with(
-        "test6tl3evq7c8vuyn", "testdm61tppb5x0tam7nae3qajhcepzz", "8519288973"
+        "auth_code_from_bitbucket", settings.BITBUCKET_REDIRECT_URI
     )
     owner = Owner.objects.get(username="ThiagoCodecov", service="bitbucket")
     expected_call = call(
@@ -155,13 +152,8 @@ async def fake_list_teams():
     )
     assert mock_create_user_onboarding_metric.call_args_list == [expected_call]
 
-    assert (
-        encryptor.decode(owner.oauth_token)
-        == "test6tl3evq7c8vuyn:testdm61tppb5x0tam7nae3qajhcepzz"
-    )
 
-
-def test_get_bitbucket_already_token_no_cookie(
+def test_get_bitbucket_already_token_no_state_cookie(
     client, settings, mocker, db, mock_redis
 ):
     mocker.patch(
@@ -175,7 +167,7 @@ def test_get_bitbucket_already_token_no_cookie(
         "generate_access_token",
         return_value={
             "key": "test6tl3evq7c8vuyn",
-            "secret": "testdm61tppb5x0tam7nae3qajhcepzz",
+            "secret": "testrefreshtoken",
         },
     )
     settings.BITBUCKET_REDIRECT_URI = "http://localhost"
@@ -184,7 +176,39 @@ def test_get_bitbucket_already_token_no_cookie(
     url = reverse("bitbucket-login")
     res = client.get(
         url,
-        {"oauth_verifier": 8519288973, "oauth_token": "test1daxl4jnhegoh4"},
+        {"code": "auth_code_from_bitbucket", "state": "some_state"},
+        SERVER_NAME="localhost:8000",
+    )
+    assert res.status_code == 302
+    assert res.url == "/login/bitbucket"
+    assert not mocked_get.called
+
+
+def test_get_bitbucket_state_mismatch(client, settings, mocker, db, mock_redis):
+    mocked_get = mocker.patch.object(
+        Bitbucket,
+        "generate_access_token",
+        return_value={
+            "key": "test6tl3evq7c8vuyn",
+            "secret": "testrefreshtoken",
+        },
+    )
+    settings.BITBUCKET_REDIRECT_URI = "http://localhost"
+    settings.BITBUCKET_CLIENT_ID = "testqmo19ebdkseoby"
+    settings.BITBUCKET_CLIENT_SECRET = "testfi8hzehvz453qj8mhv21ca4rf83f"
+    settings.COOKIE_SECRET = "aaaaa"
+
+    url = reverse("bitbucket-login")
+    client.cookies = SimpleCookie(
+        {
+            "_bb_oauth_state": signing.get_cookie_signer(salt="_bb_oauth_state").sign(
+                "legit_state"
+            )
+        }
+    )
+    res = client.get(
+        url,
+        {"code": "auth_code_from_bitbucket", "state": "attacker_injected_state"},
         SERVER_NAME="localhost:8000",
     )
     assert res.status_code == 302

apps/codecov-api/codecov_auth/views/bitbucket.py

@@ -1,6 +1,5 @@
-import base64
 import logging
-from urllib.parse import urlencode
+import secrets
 
 from asgiref.sync import async_to_sync
 from django.conf import settings
@@ -13,8 +12,10 @@
     UserOnboardingMetricsService,
 )
 from shared.torngit import Bitbucket
-from shared.torngit.exceptions import TorngitServerFailureError
-from utils.encryption import encryptor
+from shared.torngit.exceptions import (
+    TorngitClientGeneralError,
+    TorngitServerFailureError,
+)
 
 log = logging.getLogger(__name__)
 
@@ -53,23 +54,16 @@ def redirect_to_bitbucket_step(self, request):
                 "secret": settings.BITBUCKET_CLIENT_SECRET,
             }
         )
-        oauth_token_pair = repo_service.generate_request_token(
-            settings.BITBUCKET_REDIRECT_URI
+        state = secrets.token_urlsafe(32)
+        url_to_redirect = repo_service.generate_redirect_url(
+            settings.BITBUCKET_REDIRECT_URI, state=state
         )
-        oauth_token = oauth_token_pair["oauth_token"]
-        oauth_token_secret = oauth_token_pair["oauth_token_secret"]
-        url_params = urlencode({"oauth_token": oauth_token})
-        url_to_redirect = f"{Bitbucket._OAUTH_AUTHORIZE_URL}?{url_params}"
         response = redirect(url_to_redirect)
-        data = (
-            base64.b64encode(oauth_token.encode())
-            + b"|"
-            + base64.b64encode(oauth_token_secret.encode())
-        ).decode()
         response.set_signed_cookie(
-            "_oauth_request_token",
-            encryptor.encode(data).decode(),
+            "_bb_oauth_state",
+            state,
             domain=settings.COOKIES_DOMAIN,
+            httponly=True,
         )
         self.store_to_cookie_utm_tags(response)
         return response
@@ -81,19 +75,13 @@ def actual_login_step(self, request):
                 "secret": settings.BITBUCKET_CLIENT_SECRET,
             }
         )
-        oauth_verifier = request.GET.get("oauth_verifier")
-        request_cookie = request.get_signed_cookie("_oauth_request_token", default=None)
-        if not request_cookie:
-            log.warning(
-                "Request arrived with proper url params but not the proper cookies"
-            )
+        expected_state = request.get_signed_cookie("_bb_oauth_state", default=None)
+        if not expected_state or request.GET.get("state") != expected_state:
+            log.warning("Bitbucket OAuth state mismatch — possible CSRF attempt")
             return redirect(reverse("bitbucket-login"))
-        request_cookie = encryptor.decode(request_cookie)
-        cookie_key, cookie_secret = [
-            base64.b64decode(i).decode() for i in request_cookie.split("|")
-        ]
+        code = request.GET.get("code")
         token = repo_service.generate_access_token(
-            cookie_key, cookie_secret, oauth_verifier
+            code, settings.BITBUCKET_REDIRECT_URI
         )
         user_dict = self.fetch_user_data(token)
         user = self.get_and_modify_owner(user_dict, request)
@@ -102,7 +90,7 @@ def actual_login_step(self, request):
             redirection_url, user
         )
         response = redirect(redirection_url)
-        response.delete_cookie("_oauth_request_token", domain=settings.COOKIES_DOMAIN)
+        response.delete_cookie("_bb_oauth_state", domain=settings.COOKIES_DOMAIN)
         self.login_owner(user, request, response)
         log.info("User successfully logged in", extra={"ownerid": user.ownerid})
         UserOnboardingMetricsService.create_user_onboarding_metric(
@@ -115,7 +103,7 @@ def get(self, request):
             return redirect(f"{settings.CODECOV_DASHBOARD_URL}/login")
 
         try:
-            if request.GET.get("oauth_verifier"):
+            if request.GET.get("code"):
                 log.info("Logging into bitbucket after authorization")
                 return self.actual_login_step(request)
             else:
@@ -124,3 +112,6 @@ def get(self, request):
         except TorngitServerFailureError:
             log.warning("Bitbucket not available for login")
             return redirect(reverse("bitbucket-login"))
+        except TorngitClientGeneralError:
+            log.warning("Bitbucket OAuth error during login")
+            return redirect(reverse("bitbucket-login"))