feat: github app refresh token

Adding support for github app refresh tokens. This is currently an opt-in feature for github apps (what we use to auth users). closes codecov/engineering-team#163
codecov · Aug 16, 2023 · a79b65b · a79b65b
1 parent 5fc16f4
commit a79b65b
Show file tree

Hide file tree

Showing 6 changed files with 345 additions and 33 deletions.
diff --git a/shared/encryption/token.py b/shared/encryption/token.py
@@ -20,18 +20,17 @@ def decode_token(_oauth: str) -> OauthConsumerToken:
     This function decrypts a oauth_token into its different parts.
     At the moment it does different things depending on the provider.
 
-    - github
-        Only stores the "key" as the entire token
     - bitbucket
         Encodes the token as f"{key}:{secret}"
+    - github
     - gitlab
         Encodes the token as f"{key}: :{refresh_token}"
         (notice the space where {secret} should go to avoid having '::', used by decode function)
     """
     token = {}
     colon_count = _oauth.count(":")
     if colon_count > 1:
-        # Gitlab (after refresh tokens)
+        # Github + Gitlab  (post refresh tokens)
         token["key"], token["secret"], token["refresh_token"] = _oauth.split(":", 2)
         if token["secret"] == " ":
             # We remove the secret if it's our placeholder value
@@ -40,7 +39,7 @@ def decode_token(_oauth: str) -> OauthConsumerToken:
         # Bitbucket
         token["key"], token["secret"] = _oauth.split(":", 1)
     else:
-        # Github (and Gitlab pre refresh tokens)
+        # Github + Gitlab (pre refresh tokens)
         token["key"] = _oauth
         token["secret"] = None
     return token
diff --git a/shared/torngit/github.py b/shared/torngit/github.py
@@ -4,6 +4,7 @@
 import os
 from base64 import b64decode
 from typing import List
+from urllib.parse import parse_qs, urlencode
 
 import httpx
 from httpx import Response
@@ -13,17 +14,20 @@
 from shared.torngit.cache import torngit_cache
 from shared.torngit.enums import Endpoints
 from shared.torngit.exceptions import (
+    TorngitCantRefreshTokenError,
     TorngitClientError,
     TorngitClientGeneralError,
     TorngitMisconfiguredCredentials,
     TorngitObjectNotFoundError,
     TorngitRateLimitError,
+    TorngitRefreshTokenFailedError,
     TorngitRepoNotFoundError,
     TorngitServer5xxCodeError,
     TorngitServerUnreachableError,
     TorngitUnauthorizedError,
 )
 from shared.torngit.status import Status
+from shared.typings.oauth_token_types import OauthConsumerToken
 from shared.utils.urls import url_concat
 
 log = logging.getLogger(__name__)
@@ -146,6 +150,7 @@ async def make_http_call(
             json=body if body else None, headers=_headers, follow_redirects=False
         )
         max_number_retries = 3
+        tried_refresh = False
         for current_retry in range(1, max_number_retries + 1):
             try:
                 with metrics.timer(f"{METRICS_PREFIX}.api.run") as timer:
@@ -173,6 +178,21 @@ async def make_http_call(
                 raise TorngitServerUnreachableError(
                     "GitHub was not able to be reached."
                 )
+            if (
+                # I don't think Github have any specific message for trying to use an expired token
+                res.status_code == 401
+                and not tried_refresh
+            ):
+                tried_refresh = True
+                # Refresh token and retry
+                log.debug("Token is invalid. Refreshing")
+                token = await self.refresh_token(client)
+                _headers["Authorization"] = "token %s" % token["key"]
+                if callable(self._on_token_refresh):
+                    await self._on_token_refresh(token)
+                # Skip the rest of the validations and try again.
+                # It does consume one of the retries
+                continue
             if (
                 not statuses_to_retry
                 or res.status_code not in statuses_to_retry
@@ -229,6 +249,55 @@ async def make_http_call(
                     extra=dict(status=res.status_code, **log_dict),
                 )
 
+    async def refresh_token(self, client: httpx.AsyncClient) -> OauthConsumerToken:
+        """
+        This function requests a refresh token from Github.
+        The refresh_token value is stored as part of the oauth token dict.
+
+        ! side effect: updates the self._token value
+        ! raises TorngitCantRefreshTokenError
+        ! raises TorngitRefreshTokenFailedError
+        """
+        creds_from_token = self._oauth_consumer_token()
+        creds_to_send = dict(
+            client_id=creds_from_token["key"], client_secret=creds_from_token["secret"]
+        )
+
+        if self.token.get("refresh_token") is None:
+            raise TorngitCantRefreshTokenError(
+                "Token doesn't have refresh token information"
+            )
+
+        # https://docs.github.com/en/apps/creating-github-apps/authenticating-with-a-github-app/refreshing-user-access-tokens#refreshing-a-user-access-token-with-a-refresh-token
+        # Returns response as application/x-www-form-urlencoded
+        params = urlencode(
+            dict(
+                refresh_token=self.token["refresh_token"],
+                grant_type="refresh_token",
+                **creds_to_send,
+            )
+        )
+        res = await client.request(
+            "POST",
+            self.service_url + "/login/oauth/access_token",
+            params=params,
+        )
+        if res.status_code >= 300:
+            raise TorngitRefreshTokenFailedError(res)
+        response_text = self._parse_response(res)
+        session = parse_qs(response_text)
+
+        if session.get("access_token"):
+            self.set_token(
+                {
+                    # parse_qs put values in a list for reasons
+                    "key": session["access_token"][0],
+                    "refresh_token": session["refresh_token"][0],
+                }
+            )
+            return self.token
+        raise TorngitRefreshTokenFailedError(f"No access token in response - {res}")
+
     # Generic
     # -------
     async def get_branches(self, token=None):
@@ -269,7 +338,12 @@ async def get_authenticated_user(self, code):
 
             if session.get("access_token"):
                 # set current token
-                self.set_token(dict(key=session["access_token"]))
+                self.set_token(
+                    dict(
+                        key=session["access_token"],
+                        refresh_token=session["refresh_token"],
+                    )
+                )
 
                 user = await self.api(client, "get", "/user")
                 user.update(session or {})

diff --git a/tests/integration/cassetes/test_github/TestGithubTestCase/test_get_authenticated_user.yaml b/tests/integration/cassetes/test_github/TestGithubTestCase/test_get_authenticated_user.yaml
@@ -15,7 +15,7 @@ interactions:
     method: GET
     uri: https://github.com/login/oauth/access_token?code=dc38acf492b071cc4dce&client_id=999247146557c3ba045c&client_secret=testo8lnq6ihj7zsf896r15yxujnl06og9o0fqiu
   response:
-    content: '{"access_token":"testw5efy5qccduniyucsk5tesu08s4640xtoymv","token_type":"bearer","scope":"read:org,repo:status,user:email,write:repo_hook"}'
+    content: '{"refresh_token": "testblahblahblahblahsfas", "access_token":"testw5efy5qccduniyucsk5tesu08s4640xtoymv","token_type":"bearer","scope":"read:org,repo:status,user:email,write:repo_hook"}'
     headers:
       Cache-Control:
       - max-age=0, private, must-revalidate

diff --git a/.../integration/cassetes/test_github/TestGithubTestCase/test_is_student_not_capable_app.yaml b/.../integration/cassetes/test_github/TestGithubTestCase/test_is_student_not_capable_app.yaml
@@ -21,42 +21,115 @@ interactions:
       - no-cache
       Content-Security-Policy:
       - 'default-src ''self'' dwa5x7aod66zk.cloudfront.net; object-src ''self''; frame-src
-        *.vimeo.com platform.twitter.com connect.facebook.net *.facebook.com speakerdeck.com
-        *.youtube.com embed.twitch.tv; connect-src ''self'' github-education-web.s3.amazonaws.com
-        *.mapbox.com *.clearbit.com www.google.com *.githubusercontent.com geoip-js.com;
-        img-src ''self'' blob: dwa5x7aod66zk.cloudfront.net data: github.com *.mapbox.com
-        *.clearbit.com avatars.githubusercontent.com user-images.githubusercontent.com
-        github-education-web.s3.amazonaws.com analytics.twitter.com t.com *.facebook.com
-        *.twitter.com *.youtube.com raw.githubusercontent.com static-cdn.jtvnw.net;
-        font-src ''self'' dwa5x7aod66zk.cloudfront.net; script-src ''self'' dwa5x7aod66zk.cloudfront.net
-        ''unsafe-eval'' ''unsafe-inline'' www.google.com api.demandbase.com speakerdeck.com
-        platform.twitter.com connect.facebook.net *.facebook.com s3-eu-west-1.amazonaws.com/share.typeform.com/*
+        *.vimeo.com platform.twitter.com connect.facebook.net cdn.usefathom.com *.facebook.com
+        speakerdeck.com *.youtube.com *.youtube-nocookie.com embed.twitch.tv player.twitch.tv;
+        connect-src ''self'' github-education-web.s3.amazonaws.com *.clearbit.com
+        www.google.com *.githubusercontent.com geoip-js.com *.virtualearth.net *.bing.com
+        *.virtualearth.net api.github.com atlas.microsoft.com; img-src ''self'' blob:
+        dwa5x7aod66zk.cloudfront.net data: github.com blobaccountproduction.blob.core.windows.net
+        cdn.usefathom.com *.clearbit.com avatars.githubusercontent.com user-images.githubusercontent.com
+        github-education-web.s3.amazonaws.com analytics.twitter.com t.co t.com *.facebook.com
+        *.twitter.com *.youtube.com raw.githubusercontent.com static-cdn.jtvnw.net
+        dl.airtable.com *.bing.com *.virtualearth.net atlas.microsoft.com; font-src
+        ''self'' dwa5x7aod66zk.cloudfront.net atlas.microsoft.com; script-src ''self''
+        dwa5x7aod66zk.cloudfront.net www.google.com cdn.usefathom.com api.demandbase.com
+        speakerdeck.com platform.twitter.com connect.facebook.net *.facebook.com s3-eu-west-1.amazonaws.com/share.typeform.com/*
         *.github.com *.githubassets.com js.maxmind.com static.ads-twitter.com snap.licdn.com
-        geoip-js.com embed.twitch.tv; style-src ''self'' dwa5x7aod66zk.cloudfront.net
-        ''unsafe-inline'' www.google.com ajax.googleapis.com *.github.com'
+        geoip-js.com embed.twitch.tv analytics.twitter.com *.bing.com *.virtualearth.net
+        *.jquery.com *.cloudflare.com *.bootstrapcdn.com unpkg.com ''nonce-9d92aIH0h3eHkrivYZlcWg=='';
+        style-src ''self'' dwa5x7aod66zk.cloudfront.net ''unsafe-inline'' www.google.com
+        ajax.googleapis.com *.github.com *.bing.com *.virtualearth.net unpkg.com'
       Content-Type:
       - application/json
       Date:
-      - Fri, 21 May 2021 18:04:32 GMT
+      - Wed, 16 Aug 2023 17:44:54 GMT
       Referrer-Policy:
       - strict-origin-when-cross-origin
       Server:
-      - nginx/1.15.12
+      - nginx/1.25.1
       Set-Cookie:
-      - _octo=GH1.1.282664351.1621620272; domain=.github.com; path=/; expires=Sat,
-        21 May 2022 18:04:32 GMT
-      - _education_session=ZnNualVUSGRtbFlkZkFzZXBLd254SkR3M2xZM2tOaTBYdmJQQ091Q3ExTGZqZGFCV1B6R1R1c1cwQjIxTG1zOXd3L1hiY011c0V0dlVpenZySFcvUVE9PS0tbGo2OWMxU3ZyWmNxSDFMazI5TjZodz09--665bc5b6df3bdd5d371ece9f8f778b6d0e4cc19b;
-        path=/; secure; HttpOnly
+      - _octo=GH1.1.373487652.1692207893; domain=.github.com; path=/; expires=Fri,
+        16 Aug 2024 17:44:53 GMT
       Transfer-Encoding:
       - chunked
       X-Content-Type-Options:
       - nosniff
       X-Frame-Options:
       - DENY
       X-GitHub-Request-Id:
-      - 1874:6016:894CB:6BE870:60A7F630
+      - E02C:4EEA:7A50D:105E79:64DD0B15
       X-Runtime:
-      - '0.040949'
+      - '0.167302'
+      X-XSS-Protection:
+      - 1; mode=block
+      x-download-options:
+      - noopen
+      x-github-backend:
+      - Kubernetes
+      x-permitted-cross-domain-policies:
+      - none
+    http_version: HTTP/1.1
+    status_code: 401
+- request:
+    body: ''
+    headers:
+      accept:
+      - application/json
+      accept-encoding:
+      - gzip, deflate
+      connection:
+      - keep-alive
+      cookie:
+      - _octo=GH1.1.373487652.1692207893
+      host:
+      - education.github.com
+      user-agent:
+      - Default
+    method: GET
+    uri: https://education.github.com/api/user
+  response:
+    content: ''
+    headers:
+      Cache-Control:
+      - no-cache
+      Content-Security-Policy:
+      - 'default-src ''self'' dwa5x7aod66zk.cloudfront.net; object-src ''self''; frame-src
+        *.vimeo.com platform.twitter.com connect.facebook.net cdn.usefathom.com *.facebook.com
+        speakerdeck.com *.youtube.com *.youtube-nocookie.com embed.twitch.tv player.twitch.tv;
+        connect-src ''self'' github-education-web.s3.amazonaws.com *.clearbit.com
+        www.google.com *.githubusercontent.com geoip-js.com *.virtualearth.net *.bing.com
+        *.virtualearth.net api.github.com atlas.microsoft.com; img-src ''self'' blob:
+        dwa5x7aod66zk.cloudfront.net data: github.com blobaccountproduction.blob.core.windows.net
+        cdn.usefathom.com *.clearbit.com avatars.githubusercontent.com user-images.githubusercontent.com
+        github-education-web.s3.amazonaws.com analytics.twitter.com t.co t.com *.facebook.com
+        *.twitter.com *.youtube.com raw.githubusercontent.com static-cdn.jtvnw.net
+        dl.airtable.com *.bing.com *.virtualearth.net atlas.microsoft.com; font-src
+        ''self'' dwa5x7aod66zk.cloudfront.net atlas.microsoft.com; script-src ''self''
+        dwa5x7aod66zk.cloudfront.net www.google.com cdn.usefathom.com api.demandbase.com
+        speakerdeck.com platform.twitter.com connect.facebook.net *.facebook.com s3-eu-west-1.amazonaws.com/share.typeform.com/*
+        *.github.com *.githubassets.com js.maxmind.com static.ads-twitter.com snap.licdn.com
+        geoip-js.com embed.twitch.tv analytics.twitter.com *.bing.com *.virtualearth.net
+        *.jquery.com *.cloudflare.com *.bootstrapcdn.com unpkg.com ''nonce-ntysTM3E0mxWir4S9dFfuw=='';
+        style-src ''self'' dwa5x7aod66zk.cloudfront.net ''unsafe-inline'' www.google.com
+        ajax.googleapis.com *.github.com *.bing.com *.virtualearth.net unpkg.com'
+      Content-Type:
+      - application/json
+      Date:
+      - Wed, 16 Aug 2023 17:44:54 GMT
+      Referrer-Policy:
+      - strict-origin-when-cross-origin
+      Server:
+      - nginx/1.25.1
+      Transfer-Encoding:
+      - chunked
+      X-Content-Type-Options:
+      - nosniff
+      X-Frame-Options:
+      - DENY
+      X-GitHub-Request-Id:
+      - E02C:4EEA:7A526:105EAC:64DD0B16
+      X-Runtime:
+      - '0.089019'
       X-XSS-Protection:
       - 1; mode=block
       x-download-options:
@@ -65,8 +138,6 @@ interactions:
       - Kubernetes
       x-permitted-cross-domain-policies:
       - none
-      x-request-id:
-      - e943af0f-689a-4302-9ab2-34011a25d603
     http_version: HTTP/1.1
     status_code: 401
 version: 1
diff --git a/tests/integration/test_github.py b/tests/integration/test_github.py
@@ -23,7 +23,7 @@ def generic_valid_handler():
     return Github(
         repo=dict(name="example-python"),
         owner=dict(username="codecove2e"),
-        token=dict(key=10 * "a280"),
+        token=dict(key=10 * "a280", refresh_token=10 * "a180"),
     )
 
 
@@ -134,6 +134,7 @@ async def test_get_authenticated_user(self, codecov_vcr):
             "created_at": "2018-10-22T17:51:44Z",
             "updated_at": "2020-10-14T17:58:13Z",
             "access_token": "testw5efy5qccduniyucsk5tesu08s4640xtoymv",
+            "refresh_token": "testblahblahblahblahsfas",
             "token_type": "bearer",
             "scope": "read:org,repo:status,user:email,write:repo_hook",
         }
@@ -1307,9 +1308,13 @@ async def test_is_student_not_student(
         assert not res
 
     @pytest.mark.asyncio
-    async def test_is_student_not_capable_app(self, valid_handler, codecov_vcr):
-        res = await valid_handler.is_student()
+    async def test_is_student_not_capable_app(
+        self, generic_valid_handler, codecov_vcr, mocker
+    ):
+        mock_refresh = mocker.patch.object(Github, "refresh_token")
+        res = await generic_valid_handler.is_student()
         assert not res
+        assert mock_refresh.call_count == 1
 
     @pytest.mark.asyncio
     async def test_is_student_github_education_503(self, valid_handler, codecov_vcr):