Merge pull request #885 from ragnarbull/ragnarbull-auth-pwd-fixes

Password fixes - enforce max length, add checks (pwd breach, PII, low entropy), improved UX, deprecate common-passwords api

backend/src/controllers/v1/authController.ts

@@ -1,32 +1,20 @@
 import { Request, Response } from "express";
-import fs from "fs";
-import path from "path";
 import jwt from "jsonwebtoken";
 import * as bigintConversion from "bigint-conversion";
 // eslint-disable-next-line @typescript-eslint/no-var-requires
 const jsrp = require("jsrp");
-import { 
-  LoginSRPDetail, 
-  TokenVersion,
-  User,
-} from "../../models";
+import { LoginSRPDetail, TokenVersion, User } from "../../models";
 import { clearTokens, createToken, issueAuthTokens } from "../../helpers/auth";
 import { checkUserDevice } from "../../helpers/user";
-import {
-  ACTION_LOGIN,
-  ACTION_LOGOUT,
-} from "../../variables";
-import { 
-  BadRequestError,
-  UnauthorizedRequestError,
-} from "../../utils/errors";
+import { ACTION_LOGIN, ACTION_LOGOUT } from "../../variables";
+import { BadRequestError, UnauthorizedRequestError } from "../../utils/errors";
 import { EELogService } from "../../ee/services";
 import { getUserAgentType } from "../../utils/posthog";
 import {
   getHttpsEnabled,
   getJwtAuthLifetime,
   getJwtAuthSecret,
-  getJwtRefreshSecret,
+  getJwtRefreshSecret
 } from "../../config";
 import { ActorType } from "../../ee/models";
 
@@ -44,13 +32,10 @@ declare module "jsonwebtoken" {
  * @returns
  */
 export const login1 = async (req: Request, res: Response) => {
-  const {
-    email,
-    clientPublicKey,
-  }: { email: string; clientPublicKey: string } = req.body;
+  const { email, clientPublicKey }: { email: string; clientPublicKey: string } = req.body;
 
   const user = await User.findOne({
-    email,
+    email
   }).select("+salt +verifier");
 
   if (!user) throw new Error("Failed to find user");
@@ -59,21 +44,25 @@ export const login1 = async (req: Request, res: Response) => {
   server.init(
     {
       salt: user.salt,
-      verifier: user.verifier,
+      verifier: user.verifier
     },
     async () => {
       // generate server-side public key
       const serverPublicKey = server.getPublicKey();
 
-      await LoginSRPDetail.findOneAndReplace({ email: email }, {
-        email: email,
-        clientPublicKey: clientPublicKey,
-        serverBInt: bigintConversion.bigintToBuf(server.bInt),
-      }, { upsert: true, returnNewDocument: false })
+      await LoginSRPDetail.findOneAndReplace(
+        { email: email },
+        {
+          email: email,
+          clientPublicKey: clientPublicKey,
+          serverBInt: bigintConversion.bigintToBuf(server.bInt)
+        },
+        { upsert: true, returnNewDocument: false }
+      );
 
       return res.status(200).send({
         serverPublicKey,
-        salt: user.salt,
+        salt: user.salt
       });
     }
   );
@@ -89,15 +78,19 @@ export const login1 = async (req: Request, res: Response) => {
 export const login2 = async (req: Request, res: Response) => {
   const { email, clientProof } = req.body;
   const user = await User.findOne({
-    email,
+    email
   }).select("+salt +verifier +publicKey +encryptedPrivateKey +iv +tag");
 
   if (!user) throw new Error("Failed to find user");
 
-  const loginSRPDetailFromDB = await LoginSRPDetail.findOneAndDelete({ email: email })
+  const loginSRPDetailFromDB = await LoginSRPDetail.findOneAndDelete({ email: email });
 
   if (!loginSRPDetailFromDB) {
-    return BadRequestError(Error("It looks like some details from the first login are not found. Please try login one again"))
+    return BadRequestError(
+      Error(
+        "It looks like some details from the first login are not found. Please try login one again"
+      )
+    );
   }
 
   const server = new jsrp.server();
@@ -105,7 +98,7 @@ export const login2 = async (req: Request, res: Response) => {
     {
       salt: user.salt,
       verifier: user.verifier,
-      b: loginSRPDetailFromDB.serverBInt,
+      b: loginSRPDetailFromDB.serverBInt
     },
     async () => {
       server.setClientPublicKey(loginSRPDetailFromDB.clientPublicKey);
@@ -117,13 +110,13 @@ export const login2 = async (req: Request, res: Response) => {
         await checkUserDevice({
           user,
           ip: req.realIP,
-          userAgent: req.headers["user-agent"] ?? "",
+          userAgent: req.headers["user-agent"] ?? ""
         });
 
-        const tokens = await issueAuthTokens({ 
+        const tokens = await issueAuthTokens({
           userId: user._id,
           ip: req.realIP,
-          userAgent: req.headers["user-agent"] ?? "",
+          userAgent: req.headers["user-agent"] ?? ""
         });
 
         // store (refresh) token in httpOnly cookie
@@ -131,20 +124,21 @@ export const login2 = async (req: Request, res: Response) => {
           httpOnly: true,
           path: "/",
           sameSite: "strict",
-          secure: await getHttpsEnabled(),
+          secure: await getHttpsEnabled()
         });
 
         const loginAction = await EELogService.createAction({
           name: ACTION_LOGIN,
-          userId: user._id,
+          userId: user._id
         });
 
-        loginAction && await EELogService.createLog({
-          userId: user._id,
-          actions: [loginAction],
-          channel: getUserAgentType(req.headers["user-agent"]),
-          ipAddress: req.realIP,
-        });
+        loginAction &&
+          (await EELogService.createLog({
+            userId: user._id,
+            actions: [loginAction],
+            channel: getUserAgentType(req.headers["user-agent"]),
+            ipAddress: req.realIP
+          }));
 
         // return (access) token in response
         return res.status(200).send({
@@ -152,12 +146,12 @@ export const login2 = async (req: Request, res: Response) => {
           publicKey: user.publicKey,
           encryptedPrivateKey: user.encryptedPrivateKey,
           iv: user.iv,
-          tag: user.tag,
+          tag: user.tag
         });
       }
 
       return res.status(400).send({
-        message: "Failed to authenticate. Try again?",
+        message: "Failed to authenticate. Try again?"
       });
     }
   );
@@ -171,7 +165,7 @@ export const login2 = async (req: Request, res: Response) => {
  */
 export const logout = async (req: Request, res: Response) => {
   if (req.authData.actor.type === ActorType.USER && req.authData.tokenVersionId) {
-    await clearTokens(req.authData.tokenVersionId)
+    await clearTokens(req.authData.tokenVersionId);
   }
 
   // clear httpOnly cookie
@@ -179,49 +173,44 @@ export const logout = async (req: Request, res: Response) => {
     httpOnly: true,
     path: "/",
     sameSite: "strict",
-    secure: (await getHttpsEnabled()) as boolean,
+    secure: (await getHttpsEnabled()) as boolean
   });
 
   const logoutAction = await EELogService.createAction({
     name: ACTION_LOGOUT,
-    userId: req.user._id,
+    userId: req.user._id
   });
 
-  logoutAction && await EELogService.createLog({
-    userId: req.user._id,
-    actions: [logoutAction],
-    channel: getUserAgentType(req.headers["user-agent"]),
-    ipAddress: req.realIP,
-  });
+  logoutAction &&
+    (await EELogService.createLog({
+      userId: req.user._id,
+      actions: [logoutAction],
+      channel: getUserAgentType(req.headers["user-agent"]),
+      ipAddress: req.realIP
+    }));
 
   return res.status(200).send({
-    message: "Successfully logged out.",
+    message: "Successfully logged out."
   });
 };
 
-export const getCommonPasswords = async (req: Request, res: Response) => {
-  const commonPasswords = fs.readFileSync(
-		path.resolve(__dirname, "../../data/" + "common_passwords.txt"),
-		"utf8"
-	).split("\n");	
-
-  return res.status(200).send(commonPasswords);
-}
-
 export const revokeAllSessions = async (req: Request, res: Response) => {
-  await TokenVersion.updateMany({
-    user: req.user._id,
-  }, {
-    $inc: {
-      refreshVersion: 1,
-      accessVersion: 1,
+  await TokenVersion.updateMany(
+    {
+      user: req.user._id
     },
-  });
+    {
+      $inc: {
+        refreshVersion: 1,
+        accessVersion: 1
+      }
+    }
+  );
 
   return res.status(200).send({
-    message: "Successfully revoked all sessions.",
-  }); 
-}
+    message: "Successfully revoked all sessions."
+  });
+};
 
 /**
  * Return user is authenticated
@@ -231,9 +220,9 @@ export const revokeAllSessions = async (req: Request, res: Response) => {
  */
 export const checkAuth = async (req: Request, res: Response) => {
   return res.status(200).send({
-    message: "Authenticated",
+    message: "Authenticated"
   });
-}
+};
 
 /**
  * Return new JWT access token by first validating the refresh token
@@ -244,47 +233,47 @@ export const checkAuth = async (req: Request, res: Response) => {
 export const getNewToken = async (req: Request, res: Response) => {
   const refreshToken = req.cookies.jid;
 
-  if (!refreshToken) throw BadRequestError({
-    message: "Failed to find refresh token in request cookies"
-  });
+  if (!refreshToken)
+    throw BadRequestError({
+      message: "Failed to find refresh token in request cookies"
+    });
 
-  const decodedToken = <jwt.UserIDJwtPayload>(
-    jwt.verify(refreshToken, await getJwtRefreshSecret())
-  );
+  const decodedToken = <jwt.UserIDJwtPayload>jwt.verify(refreshToken, await getJwtRefreshSecret());
 
   const user = await User.findOne({
-    _id: decodedToken.userId,
+    _id: decodedToken.userId
   }).select("+publicKey +refreshVersion +accessVersion");
 
   if (!user) throw new Error("Failed to authenticate unfound user");
-  if (!user?.publicKey)
-    throw new Error("Failed to authenticate not fully set up account");
-  
+  if (!user?.publicKey) throw new Error("Failed to authenticate not fully set up account");
+
   const tokenVersion = await TokenVersion.findById(decodedToken.tokenVersionId);
 
-  if (!tokenVersion) throw UnauthorizedRequestError({
-    message: "Failed to validate refresh token",
-  });
+  if (!tokenVersion)
+    throw UnauthorizedRequestError({
+      message: "Failed to validate refresh token"
+    });
 
-  if (decodedToken.refreshVersion !== tokenVersion.refreshVersion) throw BadRequestError({
-    message: "Failed to validate refresh token",
-  });
+  if (decodedToken.refreshVersion !== tokenVersion.refreshVersion)
+    throw BadRequestError({
+      message: "Failed to validate refresh token"
+    });
 
   const token = createToken({
     payload: {
       userId: decodedToken.userId,
       tokenVersionId: tokenVersion._id.toString(),
-      accessVersion: tokenVersion.refreshVersion,
+      accessVersion: tokenVersion.refreshVersion
     },
     expiresIn: await getJwtAuthLifetime(),
-    secret: await getJwtAuthSecret(),
+    secret: await getJwtAuthSecret()
   });
 
   return res.status(200).send({
-    token,
+    token
   });
 };
 
 export const handleAuthProviderCallback = (req: Request, res: Response) => {
   res.redirect(`/login/provider/success?token=${encodeURIComponent(req.providerAuthToken)}`);
-}
+};

backend/src/index.ts

@@ -24,7 +24,7 @@ import {
   secretSnapshot as eeSecretSnapshotRouter,
   users as eeUsersRouter,
   workspace as eeWorkspaceRouter,
-  secretScanning as v1SecretScanningRouter,
+  secretScanning as v1SecretScanningRouter
 } from "./ee/routes/v1";
 import {
   auth as v1AuthRouter,
@@ -58,7 +58,7 @@ import {
   signup as v2SignupRouter,
   tags as v2TagsRouter,
   users as v2UsersRouter,
-  workspace as v2WorkspaceRouter,
+  workspace as v2WorkspaceRouter
 } from "./routes/v2";
 import {
   auth as v3AuthRouter,
@@ -70,14 +70,21 @@ import { healthCheck } from "./routes/status";
 import { getLogger } from "./utils/logger";
 import { RouteNotFoundError } from "./utils/errors";
 import { requestErrorHandler } from "./middleware/requestErrorHandler";
-import { getNodeEnv, getPort, getSecretScanningGitAppId, getSecretScanningPrivateKey, getSecretScanningWebhookProxy, getSecretScanningWebhookSecret, getSiteURL } from "./config";
+import {
+  getNodeEnv,
+  getPort,
+  getSecretScanningGitAppId,
+  getSecretScanningPrivateKey,
+  getSecretScanningWebhookProxy,
+  getSecretScanningWebhookSecret,
+  getSiteURL
+} from "./config";
 import { setup } from "./utils/setup";
 import { syncSecretsToThirdPartyServices } from "./queues/integrations/syncSecretsToThirdPartyServices";
 import { githubPushEventSecretScan } from "./queues/secret-scanning/githubScanPushEvent";
-const SmeeClient = require('smee-client') // eslint-disable-line
+const SmeeClient = require("smee-client"); // eslint-disable-line
 
 const main = async () => {
-
   await setup();
 
   await EELicenseService.initGlobalFeatureSet();
@@ -94,11 +101,15 @@ const main = async () => {
     })
   );
 
-  if (await getSecretScanningGitAppId() && await getSecretScanningWebhookSecret() && await getSecretScanningPrivateKey()) {
+  if (
+    (await getSecretScanningGitAppId()) &&
+    (await getSecretScanningWebhookSecret()) &&
+    (await getSecretScanningPrivateKey())
+  ) {
     const probot = new Probot({
       appId: await getSecretScanningGitAppId(),
       privateKey: await getSecretScanningPrivateKey(),
-      secret: await getSecretScanningWebhookSecret(),
+      secret: await getSecretScanningWebhookSecret()
     });
 
     if ((await getNodeEnv()) != "production") {
@@ -106,12 +117,14 @@ const main = async () => {
         source: await getSecretScanningWebhookProxy(),
         target: "http://backend:4000/ss-webhook",
         logger: console
-      })
+      });
 
-      smee.start()
+      smee.start();
     }
 
-    app.use(createNodeMiddleware(GithubSecretScanningService, { probot, webhooksPath: "/ss-webhook" })); // secret scanning webhook
+    app.use(
+      createNodeMiddleware(GithubSecretScanningService, { probot, webhooksPath: "/ss-webhook" })
+    ); // secret scanning webhook
   }
 
   if ((await getNodeEnv()) === "production") {
@@ -207,8 +220,8 @@ const main = async () => {
 
   server.on("close", async () => {
     await DatabaseService.closeDatabase();
-    syncSecretsToThirdPartyServices.close()
-    githubPushEventSecretScan.close()
+    syncSecretsToThirdPartyServices.close();
+    githubPushEventSecretScan.close();
   });
 
   return server;

backend/src/routes/v1/auth.ts

@@ -8,7 +8,8 @@ import { AuthMode } from "../../variables";
 
 router.post("/token", validateRequest, authController.getNewToken);
 
-router.post( // TODO endpoint: deprecate (moved to api/v3/auth/login1)
+router.post(
+  // TODO endpoint: deprecate (moved to api/v3/auth/login1)
   "/login1",
   authLimiter,
   body("email").exists().trim().notEmpty().toLowerCase(),
@@ -17,7 +18,8 @@ router.post( // TODO endpoint: deprecate (moved to api/v3/auth/login1)
   authController.login1
 );
 
-router.post( // TODO endpoint: deprecate (moved to api/v3/auth/login2)
+router.post(
+  // TODO endpoint: deprecate (moved to api/v3/auth/login2)
   "/login2",
   authLimiter,
   body("email").exists().trim().notEmpty().toLowerCase(),
@@ -30,7 +32,7 @@ router.post(
   "/logout",
   authLimiter,
   requireAuth({
-    acceptedAuthModes: [AuthMode.JWT],
+    acceptedAuthModes: [AuthMode.JWT]
   }),
   authController.logout
 );
@@ -38,24 +40,19 @@ router.post(
 router.post(
   "/checkAuth",
   requireAuth({
-    acceptedAuthModes: [AuthMode.JWT],
+    acceptedAuthModes: [AuthMode.JWT]
   }),
   authController.checkAuth
 );
 
-router.get(
-  "/common-passwords",
-  authLimiter,
-  authController.getCommonPasswords
-);
-
-router.delete( // TODO endpoint: deprecate (moved to DELETE v2/users/me/sessions)
+router.delete(
+  // TODO endpoint: deprecate (moved to DELETE v2/users/me/sessions)
   "/sessions",
   authLimiter,
   requireAuth({
-    acceptedAuthModes: [AuthMode.JWT],
-  }), 
+    acceptedAuthModes: [AuthMode.JWT]
+  }),
   authController.revokeAllSessions
 );
 
-export default router;
+export default router;