fix: encrypt stored GitHub credentials (#797)

src/server/routes/connection.ts

@@ -10,7 +10,7 @@ import config from "../../config";
 import UserModel from "../../core/model/users/users.model";
 import { IUserDocument } from "../../core/model/users/users.types";
 import AnonymousError from "../../core/AnonymousError";
-import AnonymizedPullRequestModel from "../../core/model/anonymizedPullRequests/anonymizedPullRequests.model";
+import { setCredential } from "../../core/credentials";
 import { hashToken } from "./token-auth";
 import { createLogger, serializeError } from "../../core/logger";
 import { getLoginToken, isDisabledAccount } from "./auth-utils";
@@ -30,13 +30,12 @@ export function ensureAuthenticated(
 
 const verify = async (
   accessToken: string,
-  refreshToken: string,
+  _refreshToken: string,
   profile: Profile,
   done: OAuth2Strategy.VerifyCallback
 ): Promise<void> => {
   let user: IUserDocument | null;
   try {
-    const now = new Date();
     user = await UserModel.findOne({ "externalIDs.github": profile.id });
     if (user) {
       if (isDisabledAccount(user.status)) {
@@ -48,20 +47,6 @@ const verify = async (
         );
         return;
       }
-      await UserModel.updateOne(
-        { _id: user._id },
-        {
-          $set: {
-            "accessTokens.github": accessToken,
-            "accessTokenDates.github": now,
-          },
-        }
-      );
-      await AnonymizedPullRequestModel.updateMany(
-        { owner: user._id },
-        { "source.accessToken": accessToken }
-      );
-      user = await UserModel.findById(user._id);
     } else {
       // Check if a user with this username already exists (e.g. created
       // manually without externalIDs.github). Link the GitHub ID to the
@@ -87,8 +72,6 @@ const verify = async (
           {
             $set: {
               "externalIDs.github": profile.id,
-              "accessTokens.github": accessToken,
-              "accessTokenDates.github": now,
             },
           }
         );
@@ -97,12 +80,6 @@ const verify = async (
         const photo = profile.photos ? profile.photos[0]?.value : null;
         user = new UserModel({
           username: profile.username,
-          accessTokens: {
-            github: accessToken,
-          },
-          accessTokenDates: {
-            github: now,
-          },
           externalIDs: {
             github: profile.id,
           },
@@ -126,13 +103,8 @@ const verify = async (
       );
       return;
     }
-    done(null, {
-      username: profile.username,
-      accessToken,
-      refreshToken,
-      profile,
-      user,
-    });
+    await setCredential(String(user!._id), accessToken);
+    done(null, { username: user!.username, user });
   } catch (error) {
     logger.error("verify failed", serializeError(error));
     done(
@@ -157,11 +129,20 @@ passport.use(
 );
 
 passport.serializeUser((user: Express.User, done) => {
-  done(null, user);
+  const id = (user as { user?: { _id?: unknown } }).user?._id;
+  done(null, String(id));
 });
 
-passport.deserializeUser((user: Express.User, done) => {
-  done(null, user);
+passport.deserializeUser(async (id: string, done) => {
+  // Reject the old session format, which included plaintext credentials.
+  if (typeof id !== "string" || !/^[a-f0-9]{24}$/i.test(id)) return done(null, false);
+  try {
+    const user = await UserModel.findById(id);
+    if (!user || isDisabledAccount(user.status)) return done(null, false);
+    done(null, { username: user.username, user });
+  } catch {
+    done(new Error("Session user lookup failed"));
+  }
 });
 
 export function initSession() {
@@ -229,7 +210,6 @@ router.post(
       }
       const synthUser = {
         username: model.username,
-        accessToken: model.accessTokens?.github,
         profile: undefined,
         user: model,
       };

src/server/routes/gist-private.ts

@@ -249,7 +249,6 @@ router.post("/", async (req: express.Request, res: express.Response) => {
     gist.model.owner = user.id;
 
     updateGistModel(gist.model, gistUpdate);
-    gist.source.accessToken = user.accessToken;
     gist.source.gistId = gistUpdate.source.gistId;
 
     gist.model.conference = gistUpdate.conference;

src/server/routes/pullRequest-private.ts

@@ -266,7 +266,6 @@ router.post("/", async (req: express.Request, res: express.Response) => {
     pullRequest.model.owner = user.id;
 
     updatePullRequestModel(pullRequest.model, pullRequestUpdate);
-    pullRequest.source.accessToken = user.accessToken;
     pullRequest.source.pullRequestId = pullRequestUpdate.source.pullRequestId;
     pullRequest.source.repositoryFullName =
       pullRequestUpdate.source.repositoryFullName;

src/server/routes/repository-private.ts

@@ -1,3 +1,4 @@
+import { getCredentialToken } from "../../core/credentials";
 import * as express from "express";
 import { ensureAuthenticated } from "./connection";
 
@@ -14,14 +15,12 @@ import { getRepositoryFromGitHub } from "../../core/source/GitHubRepository";
 import gh = require("parse-github-url");
 import AnonymizedRepositoryModel from "../../core/model/anonymizedRepositories/anonymizedRepositories.model";
 import { IAnonymizedRepositoryDocument } from "../../core/model/anonymizedRepositories/anonymizedRepositories.types";
-import UserModel from "../../core/model/users/users.model";
 import ConferenceModel from "../../core/model/conference/conferences.model";
 import AnonymousError from "../../core/AnonymousError";
 import { addRemovalJob, downloadQueue } from "../../queue";
 import RepositoryModel from "../../core/model/repositories/repositories.model";
 import User from "../../core/User";
 import { RepositoryStatus } from "../../core/types";
-import { IUserDocument } from "../../core/model/users/users.types";
 import { checkToken, octokit, getRedisGateResetAt, getToken } from "../../core/GitHubUtils";
 import { createLogger, serializeError } from "../../core/logger";
 
@@ -40,23 +39,14 @@ async function getTokenForAdmin(user: User, req: express.Request) {
           "source.repositoryName": `${req.params.owner}/${req.params.repo}`,
         },
         {
-          "source.accessToken": 1,
           owner: 1,
         }
-      ).populate({
-        path: "owner",
-        model: UserModel,
-      });
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      const user: IUserDocument = existingRepo?.owner as any;
-      if (user instanceof UserModel) {
-        const check = await checkToken(user.accessTokens.github);
-        if (check) {
-          return user.accessTokens.github;
-        }
-      }
-      if (existingRepo) {
-        return existingRepo.source.accessToken;
+      );
+      if (existingRepo?.owner) {
+        const token = await getCredentialToken(String(existingRepo.owner), "github", {
+          collection: "anonymizedrepositories", id: existingRepo._id,
+        });
+        if (token && await checkToken(token)) return token;
       }
     } catch (error) {
       logger.warn("getToken lookup failed", serializeError(error));
@@ -100,7 +90,7 @@ router.post("/claim", async (req: express.Request, res: express.Response) => {
       owner: r.owner,
       repo: r.name,
       repositoryID: req.query.repositoryID as string,
-      accessToken: user.accessToken,
+      accessToken: await user.getAccessToken(),
     });
     if (!repo) {
       throw new AnonymousError("repo_not_found", {
@@ -256,7 +246,7 @@ router.get(
   async (req: express.Request, res: express.Response) => {
     try {
       const user = await getUser(req);
-      let token = user.accessToken;
+      let token = await user.getAccessToken();
       if (user.isAdmin) {
         token = (await getTokenForAdmin(user, req)) || token;
       }
@@ -279,7 +269,7 @@ router.get(
   async (req: express.Request, res: express.Response) => {
     try {
       const user = await getUser(req);
-      let token = user.accessToken;
+      let token = await user.getAccessToken();
       if (user.isAdmin) {
         token = (await getTokenForAdmin(user, req)) || token;
       }
@@ -307,7 +297,7 @@ router.get(
   async (req: express.Request, res: express.Response) => {
     try {
       const user = await getUser(req);
-      let token = user.accessToken;
+      let token = await user.getAccessToken();
       if (user.isAdmin) {
         token = (await getTokenForAdmin(user, req)) || token;
       }
@@ -521,7 +511,7 @@ router.post(
           });
         }
         const repository = await getRepositoryFromGitHub({
-          accessToken: user.accessToken,
+          accessToken: await user.getAccessToken(),
           owner: parsedRepository.owner,
           repo: parsedRepository.name,
         });
@@ -533,7 +523,7 @@ router.post(
           });
         }
         await repository.getCommitInfo(repoUpdate.source.commit, {
-          accessToken: user.accessToken,
+          accessToken: await user.getAccessToken(),
         });
         repo.model.source.repositoryId = repository.model.id;
         repo.model.source.repositoryName =
@@ -648,7 +638,7 @@ router.post("/", async (req: express.Request, res: express.Response) => {
       });
     }
     const repository = await getRepositoryFromGitHub({
-      accessToken: user.accessToken,
+      accessToken: await user.getAccessToken(),
       owner: r.owner,
       repo: r.name,
     });
@@ -661,7 +651,7 @@ router.post("/", async (req: express.Request, res: express.Response) => {
     }
 
     await repository.getCommitInfo(repoUpdate.source.commit, {
-      accessToken: user.accessToken,
+      accessToken: await user.getAccessToken(),
     });
 
     const repo = new AnonymizedRepositoryModel();
@@ -671,7 +661,6 @@ router.post("/", async (req: express.Request, res: express.Response) => {
 
     updateRepoModel(repo, repoUpdate);
     repo.source.type = "GitHubStream";
-    repo.source.accessToken = user.accessToken;
     repo.source.repositoryId = repository.model.id;
     repo.source.repositoryName = repoUpdate.fullName;
 
@@ -762,7 +751,7 @@ router.post(
       }
 
       // verify the GitHub user exists and capture identity fields
-      const oct = octokit(user.accessToken);
+      const oct = octokit(await user.getAccessToken());
       let ghUser;
       try {
         const r = await oct.users.getByUsername({ username });

src/server/routes/user.ts

@@ -1,3 +1,4 @@
+import CredentialModel from "../../core/model/credentials/credentials.model";
 import * as express from "express";
 import got from "got";
 import config from "../../config";
@@ -177,15 +178,15 @@ router.delete("/", async (req: express.Request, res: express.Response) => {
     await Promise.all([
       AnonymizedRepositoryModel.updateMany(
         { owner: user.model._id },
-        { $unset: { "source.accessToken": "" } }
+        { $unset: { "source.accessToken": "", accessToken: "" } }
       ).exec(),
       AnonymizedPullRequestModel.updateMany(
         { owner: user.model._id },
-        { $unset: { "source.accessToken": "" } }
+        { $unset: { "source.accessToken": "", accessToken: "" } }
       ).exec(),
       AnonymizedGistModel.updateMany(
         { owner: user.model._id },
-        { $unset: { "source.accessToken": "" } }
+        { $unset: { "source.accessToken": "", accessToken: "" } }
       ).exec(),
     ]);
 
@@ -199,13 +200,15 @@ router.delete("/", async (req: express.Request, res: express.Response) => {
           username: config.CLIENT_ID,
           password: config.CLIENT_SECRET,
           headers: { accept: "application/vnd.github+json" },
-          json: { access_token: user.accessToken },
+          json: { access_token: await user.getAccessToken() },
         }
       );
     } catch (error) {
       logger.warn("oauth grant revocation failed", serializeError(error));
     }
 
+    await CredentialModel.deleteMany({ ownerId: user.model._id });
+
     await UserModel.updateOne(
       { _id: user.model._id },
       {
@@ -296,7 +299,7 @@ router.get(
       if (!q || q.length < 2) {
         return res.json([]);
       }
-      const oct = octokit(user.accessToken);
+      const oct = octokit(await user.getAccessToken());
       const r = await oct.search.users({ q, per_page: 10 });
       res.json(
         r.data.items.map((u) => ({