App: Handle time difference between system and GitHub API time

This adds error handling behavior for when app authentication fails due
to a mismatch in system times between the local machine and GitHub's API
servers. If an expiration (exp) or issued_at (iat) claim in the JWT
payload is invalid, the difference between the system time and GitHub's
server time is stored and further requests are made with JWTs that
include that time difference when creating the exp and iat claims.

src/get-app-authentication.ts

@@ -2,11 +2,22 @@ import { githubAppJwt } from "universal-github-app-jwt";
 
 import { AppAuthentication } from "./types";
 
+// Time skew between system time and GitHub's API time for JWT issued_at (iat) and expiration (exp) timestamps.
+let timeDifference = 0;
+
+export function setTimeDifference(diff: number) {
+  timeDifference = diff;
+}
+
 export async function getAppAuthentication(
   id: number,
   privateKey: string
 ): Promise<AppAuthentication> {
-  const appAuthentication = await githubAppJwt({ id, privateKey });
+  const appAuthentication = await githubAppJwt({
+    id,
+    privateKey,
+    timeDifference,
+  });
 
   return {
     type: "app",

src/hook.ts

@@ -1,4 +1,7 @@
-import { getAppAuthentication } from "./get-app-authentication";
+import {
+  getAppAuthentication,
+  setTimeDifference,
+} from "./get-app-authentication";
 import { getInstallationAuthentication } from "./get-installation-authentication";
 import { requiresAppAuth } from "./requires-app-auth";
 import {
@@ -9,9 +12,22 @@ import {
   Route,
   State,
 } from "./types";
+import { RequestError } from "@octokit/request-error";
+import { OctokitResponse } from "@octokit/types";
 
 const SIXTY_SECONDS_IN_MS = 60 * 1000;
 
+function isTimeSkewError(error: RequestError) {
+  return (
+    error.message.match(
+      /'Expiration time' claim \('exp'\) must be a numeric value representing the future time at which the assertion expires/
+    ) ||
+    error.message.match(
+      /'Issued at' claim \('iat'\) must be an Integer representing the time that the assertion was issued/
+    )
+  );
+}
+
 export async function hook(
   state: State,
   request: RequestInterface,
@@ -28,7 +44,40 @@ export async function hook(
     const { token } = await getAppAuthentication(state.id, state.privateKey);
     endpoint.headers.authorization = `bearer ${token}`;
 
-    return request(endpoint as EndpointOptions);
+    let response;
+    try {
+      response = await request(endpoint as EndpointOptions);
+    } catch (error) {
+      // If there's an issue with the expiration, regenerate the token and try again.
+      // Otherwise rethrow the error for upstream handling.
+      if (isTimeSkewError(error)) {
+        // If the date header is missing, we can't correct the system time skew.
+        // Throw the error to be handled upstream.
+        if (typeof error.headers.date === "undefined") {
+          throw error;
+        }
+
+        const diff =
+          (Date.parse(error.headers.date) - Date.parse(new Date().toString())) /
+          1000;
+        setTimeDifference(diff);
+        console.warn(error.message);
+        console.warn(
+          `GitHub API time and system time are different by ${diff} seconds. Retrying request with the difference accounted for.`
+        );
+
+        const { token } = await getAppAuthentication(
+          state.id,
+          state.privateKey
+        );
+        endpoint.headers.authorization = `bearer ${token}`;
+        return request(endpoint as EndpointOptions);
+      }
+
+      throw error;
+    }
+
+    return response;
   }
 
   const { token, createdAt } = await getInstallationAuthentication(

test/index.test.ts

@@ -3,6 +3,7 @@ import { request } from "@octokit/request";
 import { install, Clock } from "@sinonjs/fake-timers";
 
 import { createAppAuth } from "../src/index";
+import { setTimeDifference } from "../src/get-app-authentication";
 
 const APP_ID = 1;
 const PRIVATE_KEY = `-----BEGIN RSA PRIVATE KEY-----
@@ -39,6 +40,7 @@ const BEARER =
 let clock: Clock;
 beforeEach(() => {
   clock = install({ now: 0, toFake: ["Date", "setTimeout"] });
+  setTimeDifference(0);
 });
 
 test("README example for app auth", async () => {
@@ -1210,6 +1212,197 @@ test("auth.hook() uses app auth for marketplace URL", async () => {
   expect(mock.done()).toBe(true);
 });
 
+test("auth.hook(): handle 401 due to an exp timestamp in the past", async () => {
+  const mock = fetchMock
+    .sandbox()
+    .get("https://api.github.com/app", (_url, options) => {
+      const auth = (options.headers as { [key: string]: string | undefined })[
+        "authorization"
+      ];
+      const [_, jwt] = (auth || "").split(" ");
+      const payload = JSON.parse(atob(jwt.split(".")[1]));
+
+      // By default the mocked time will set the payload to 570 (10 minutes - 30 seconds in seconds)
+      // By returning an error for that exp with an API time of 30 seconds in the future,
+      // the new request will be made with a JWT that has an expiration set 30 seconds further in the future.
+      if (payload.exp < 600) {
+        return {
+          status: 401,
+          body: {
+            message:
+              "'Expiration time' claim ('exp') must be a numeric value representing the future time at which the assertion expires.",
+            documentation_url: "https://developer.github.com/v3",
+          },
+          headers: {
+            date: new Date(Date.now() + 30000).toUTCString(),
+          },
+        };
+      }
+
+      return {
+        status: 200,
+        body: [],
+      };
+    });
+
+  const auth = createAppAuth({
+    id: APP_ID,
+    privateKey: PRIVATE_KEY,
+  });
+
+  const requestWithMock = request.defaults({
+    request: {
+      fetch: mock,
+    },
+  });
+  const requestWithAuth = requestWithMock.defaults({
+    request: {
+      hook: auth.hook,
+    },
+  });
+
+  global.console.warn = jest.fn();
+
+  const promise = requestWithAuth("GET /app");
+
+  const { data } = await promise;
+
+  expect(data).toStrictEqual([]);
+  expect(mock.done()).toBe(true);
+
+  // @ts-ignore
+  expect(global.console.warn.mock.calls.length).toEqual(2);
+  expect(global.console.warn).toHaveBeenNthCalledWith(
+    1,
+    "'Expiration time' claim ('exp') must be a numeric value representing the future time at which the assertion expires."
+  );
+  expect(global.console.warn).toHaveBeenNthCalledWith(
+    2,
+    `GitHub API time and system time are different by 30 seconds. Retrying request with the difference accounted for.`
+  );
+});
+
+test("auth.hook(): handle 401 due to an iat timestamp in the future", async () => {
+  const mock = fetchMock
+    .sandbox()
+    .get("https://api.github.com/app", (_url, options) => {
+      const auth = (options.headers as { [key: string]: string | undefined })[
+        "authorization"
+      ];
+      const [_, jwt] = (auth || "").split(" ");
+      const payload = JSON.parse(atob(jwt.split(".")[1]));
+
+      // By default the mocked time will set the payload.iat to -30.
+      // By returning an error for that exp with an API time of 30 seconds in the future,
+      // the new request will be made with a JWT that has an issued_at set 30 seconds further in the future.
+      if (payload.iat < 0) {
+        return {
+          status: 401,
+          body: {
+            message:
+              "'Issued at' claim ('iat') must be an Integer representing the time that the assertion was issued.",
+            documentation_url: "https://developer.github.com/v3",
+          },
+          headers: {
+            date: new Date(Date.now() + 30000).toUTCString(),
+          },
+        };
+      }
+
+      return {
+        status: 200,
+        body: [],
+      };
+    });
+
+  const auth = createAppAuth({
+    id: APP_ID,
+    privateKey: PRIVATE_KEY,
+  });
+
+  const requestWithMock = request.defaults({
+    request: {
+      fetch: mock,
+    },
+  });
+  const requestWithAuth = requestWithMock.defaults({
+    request: {
+      hook: auth.hook,
+    },
+  });
+
+  global.console.warn = jest.fn();
+
+  const promise = requestWithAuth("GET /app");
+  const { data } = await promise;
+
+  expect(data).toStrictEqual([]);
+  expect(mock.done()).toBe(true);
+
+  // @ts-ignore
+  expect(global.console.warn.mock.calls.length).toEqual(2);
+  expect(global.console.warn).toHaveBeenNthCalledWith(
+    1,
+    "'Issued at' claim ('iat') must be an Integer representing the time that the assertion was issued."
+  );
+  expect(global.console.warn).toHaveBeenNthCalledWith(
+    2,
+    `GitHub API time and system time are different by 30 seconds. Retrying request with the difference accounted for.`
+  );
+});
+
+test("auth.hook(): throw 401 error in app auth flow without timing errors", async () => {
+  const mock = fetchMock
+    .sandbox()
+    .get("https://api.github.com/app", {
+      status: 401,
+      body: {
+        message: "Bad credentials",
+        documentation_url: "https://developer.github.com/v3",
+      },
+    })
+    .get("https://api.github.com/marketplace_listing/plan", {
+      status: 401,
+      body: {
+        message:
+          "'Issued at' claim ('iat') must be an Integer representing the time that the assertion was issued.",
+        documentation_url: "https://developer.github.com/v3",
+      },
+    });
+
+  const auth = createAppAuth({
+    id: APP_ID,
+    privateKey: PRIVATE_KEY,
+  });
+
+  const requestWithMock = request.defaults({
+    request: {
+      fetch: mock,
+    },
+  });
+  const requestWithAuth = requestWithMock.defaults({
+    request: {
+      hook: auth.hook,
+    },
+  });
+
+  global.console.warn = jest.fn();
+
+  try {
+    await requestWithAuth("GET /app");
+    throw new Error("Should not resolve");
+  } catch (error) {
+    expect(error.status).toEqual(401);
+  }
+
+  try {
+    await requestWithAuth("GET /marketplace_listing/plan");
+    throw new Error("Should not resolve");
+  } catch (error) {
+    expect(error.status).toEqual(401);
+  }
+});
+
 test("auth.hook(): handle 401 in first minute (#65)", async () => {
   let requestCount = 0;
   const ONE_MINUTE_IN_MS = 1000 * 60;