feat: Validate OIDC ID token during Sentry OAuth flow

codecov/settings_base.py

@@ -457,6 +457,7 @@
 SENTRY_USER_WEBHOOK_URL = get_config("setup", "sentry", "webhook_url", default=None)
 SENTRY_OAUTH_CLIENT_ID = get_config("setup", "sentry", "oauth_client_id")
 SENTRY_OAUTH_CLIENT_SECRET = get_config("setup", "sentry", "oauth_client_secret")
+SENTRY_OIDC_SHARED_SECRET = get_config("setup", "sentry", "oidc_shared_secret")
 
 # list of repo IDs that will use the new-style report builder
 # TODO: we can eventually get rid of this once it's confirmed working well for many repos

codecov_auth/tests/unit/views/test_sentry.py

@@ -1,3 +1,4 @@
+import jwt
 import pytest
 from django.conf import settings
 from django.contrib import auth
@@ -23,6 +24,13 @@ def mocked_sentry_request(mocker):
                         "email": "[email protected]",
                         "name": "Some User",
                     },
+                    "id_token": jwt.encode(
+                        {
+                            "iss": "https://sentry.io",
+                            "aud": "test-client-id",
+                        },
+                        key="test-oidc-shared-secret",
+                    ),
                 }
             ),
         ),
@@ -39,6 +47,10 @@ def test_sentry_redirect_to_consent(client):
     )
 
 
+@override_settings(
+    SENTRY_OAUTH_CLIENT_ID="test-client-id",
+    SENTRY_OIDC_SHARED_SECRET="test-oidc-shared-secret",
+)
 def test_sentry_perform_login(client, mocked_sentry_request, db):
     res = client.get(
         reverse("sentry-login"),
@@ -76,6 +88,10 @@ def test_sentry_perform_login(client, mocked_sentry_request, db):
     assert user == current_user
 
 
+@override_settings(
+    SENTRY_OAUTH_CLIENT_ID="test-client-id",
+    SENTRY_OIDC_SHARED_SECRET="test-oidc-shared-secret",
+)
 def test_sentry_perform_login_authenticated(client, mocked_sentry_request, db):
     user = UserFactory()
     client.force_login(user=user)
@@ -103,6 +119,10 @@ def test_sentry_perform_login_authenticated(client, mocked_sentry_request, db):
     assert user == current_user
 
 
+@override_settings(
+    SENTRY_OAUTH_CLIENT_ID="test-client-id",
+    SENTRY_OIDC_SHARED_SECRET="test-oidc-shared-secret",
+)
 def test_sentry_perform_login_existing_sentry_user(client, mocked_sentry_request, db):
     sentry_user = SentryUserFactory(sentry_id="test-id")
 
@@ -121,6 +141,10 @@ def test_sentry_perform_login_existing_sentry_user(client, mocked_sentry_request
     assert current_user == sentry_user.user
 
 
+@override_settings(
+    SENTRY_OAUTH_CLIENT_ID="test-client-id",
+    SENTRY_OIDC_SHARED_SECRET="test-oidc-shared-secret",
+)
 def test_sentry_perform_login_authenticated_existing_sentry_user(
     client, mocked_sentry_request, db
 ):
@@ -143,6 +167,10 @@ def test_sentry_perform_login_authenticated_existing_sentry_user(
     assert current_user == sentry_user.user
 
 
+@override_settings(
+    SENTRY_OAUTH_CLIENT_ID="test-client-id",
+    SENTRY_OIDC_SHARED_SECRET="test-oidc-shared-secret",
+)
 def test_sentry_perform_login_existing_sentry_user_existing_owner(
     client, mocked_sentry_request, db
 ):
@@ -165,6 +193,10 @@ def test_sentry_perform_login_existing_sentry_user_existing_owner(
     assert current_user == sentry_user.user
 
 
+@override_settings(
+    SENTRY_OAUTH_CLIENT_ID="test-client-id",
+    SENTRY_OIDC_SHARED_SECRET="test-oidc-shared-secret",
+)
 def test_sentry_perform_login_error(client, mocker):
     mocker.patch(
         "codecov_auth.views.sentry.requests.post",
@@ -182,3 +214,68 @@ def test_sentry_perform_login_error(client, mocker):
 
     assert res.status_code == 302
     assert res.url == f"{settings.CODECOV_DASHBOARD_URL}/login"
+
+
+@override_settings(
+    SENTRY_OAUTH_CLIENT_ID="test-client-id",
+    SENTRY_OIDC_SHARED_SECRET="invalid-oidc-shared-secret",
+)
+def test_sentry_perform_login_invalid_id_token(client, mocked_sentry_request, db):
+    res = client.get(
+        reverse("sentry-login"),
+        data={
+            "code": "test-code",
+        },
+    )
+
+    assert res.status_code == 302
+    assert res.url == f"{settings.CODECOV_DASHBOARD_URL}/login"
+
+    # does not login user
+    current_user = auth.get_user(client)
+    assert current_user.is_anonymous
+
+
+@override_settings(
+    SENTRY_OAUTH_CLIENT_ID="test-client-id",
+    SENTRY_OIDC_SHARED_SECRET="test-oidc-shared-secret",
+)
+def test_sentry_perform_login_invalid_id_token_issuer(client, mocker, db):
+    mocker.patch(
+        "codecov_auth.views.sentry.requests.post",
+        return_value=mocker.MagicMock(
+            status_code=200,
+            json=mocker.MagicMock(
+                return_value={
+                    "access_token": "test-access-token",
+                    "refresh_token": "test-refresh-token",
+                    "user": {
+                        "id": "test-id",
+                        "email": "[email protected]",
+                        "name": "Some User",
+                    },
+                    "id_token": jwt.encode(
+                        {
+                            "iss": "invalid-issuer",
+                            "aud": "test-client-id",
+                        },
+                        key="test-oidc-shared-secret",
+                    ),
+                }
+            ),
+        ),
+    )
+
+    res = client.get(
+        reverse("sentry-login"),
+        data={
+            "code": "test-code",
+        },
+    )
+
+    assert res.status_code == 302
+    assert res.url == f"{settings.CODECOV_DASHBOARD_URL}/login"
+
+    # does not login user
+    current_user = auth.get_user(client)
+    assert current_user.is_anonymous

codecov_auth/views/sentry.py

@@ -2,6 +2,7 @@
 from typing import Dict, Optional
 from urllib.parse import urlencode
 
+import jwt
 import requests
 from django.conf import settings
 from django.contrib.auth import login, logout
@@ -51,14 +52,46 @@ def _redirect_to_consent(self) -> HttpResponse:
         self.store_to_cookie_utm_tags(response)
         return response
 
+    def _verify_id_token(self, id_token: str) -> bool:
+        try:
+            id_payload = jwt.decode(
+                id_token,
+                settings.SENTRY_OIDC_SHARED_SECRET,
+                algorithms=["HS256"],
+                audience=settings.SENTRY_OAUTH_CLIENT_ID,
+            )
+
+            if id_payload["iss"] != "https://sentry.io":
+                log.warning(
+                    "Invalid issuer of OIDC ID token",
+                    exc_info=True,
+                    extra=dict(
+                        id_payload=id_payload,
+                    ),
+                )
+                return False
+
+            return True
+        except jwt.exceptions.InvalidSignatureError:
+            id_payload = jwt.decode(id_token, options={"verify_signature": False})
+            log.warning(
+                "Unable to verify signature of OIDC ID token",
+                exc_info=True,
+                extra=dict(
+                    id_payload=id_payload,
+                ),
+            )
+            return False
+
     def _perform_login(self, request: HttpRequest) -> HttpResponse:
         code = request.GET.get("code")
         user_data = self._fetch_user_data(code)
         if user_data is None:
             log.warning("Unable to log in due to problem on Sentry", exc_info=True)
             return redirect(f"{settings.CODECOV_DASHBOARD_URL}/login")
 
-        # TODO: verify `id_token` by decoding the JWT using our shared secret
+        if not self._verify_id_token(user_data["id_token"]):
+            return redirect(f"{settings.CODECOV_DASHBOARD_URL}/login")
 
         current_user = self._login_user(request, user_data)