From 2dbfaf155742479e639fd6e56cecfa40189ab78b Mon Sep 17 00:00:00 2001 From: Matchu Date: Wed, 17 Aug 2022 15:24:17 -0700 Subject: [PATCH] Support actual login via db?? :0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Yeah cool the login button seems to. work now? And subsequent requests serve user data correctly based on that, and let you edit stuff. I also tested the following attacks: - Using the wrong password indeed fails! lol basic one - Changing the userId or createdAt fields in the cookie causes the auth token to be rejected for an invalid signature. Tbh that's all that comes to mind… like, you either attack us by tricking the login itself into giving you a token when it shouldn't, or you attack us by tricking the subsequent requests into accepting a token when it shouldn't. Seems like we're covered? 😳🤞 Still need to add logout, but yeah, this is… looking surprisingly feature-parity with our Auth0 integration already lmao. Maybe it'll be ready to launch sooner than expected? --- src/app/apolloClient.js | 18 +++- src/app/components/LoginModal.js | 25 +++-- src/app/components/useCurrentUser.js | 138 +++++++++++++++++++++++++-- src/server/auth-by-db.js | 62 +++++++++--- src/server/index.js | 42 ++++++-- 5 files changed, 250 insertions(+), 35 deletions(-) diff --git a/src/app/apolloClient.js b/src/app/apolloClient.js index 3df1923e..9c3ebdd1 100644 --- a/src/app/apolloClient.js +++ b/src/app/apolloClient.js @@ -3,7 +3,10 @@ import { setContext } from "@apollo/client/link/context"; import { createPersistedQueryLink } from "apollo-link-persisted-queries"; import cachedZones from "./cached-data/zones.json"; -import { readCypressLoginData } from "./components/useCurrentUser"; +import { + getAuthModeFeatureFlag, + readCypressLoginData, +} from "./components/useCurrentUser"; // Teach Apollo to load certain fields from the cache, to avoid extra network // requests. This happens a lot - e.g. reusing data from item search on the @@ -176,6 +179,18 @@ const buildAuthLink = (getAuth0) => } }); +// This is a temporary way to pass the DTIAuthMode feature flag back to the +// server! +const authModeLink = setContext((_, { headers = {} }) => { + const authMode = getAuthModeFeatureFlag(); + return { + headers: { + ...headers, + "DTI-Auth-Mode": authMode, + }, + }; +}); + async function getAccessToken(getAuth0) { // Our Cypress tests store login data separately. Use it if available! const cypressToken = readCypressLoginData()?.encodedToken; @@ -210,6 +225,7 @@ for (const zone of cachedZones) { const buildLink = (getAuth0) => buildAuthLink(getAuth0) + .concat(authModeLink) .concat( createPersistedQueryLink({ useGETForHashedQueries: true, diff --git a/src/app/components/LoginModal.js b/src/app/components/LoginModal.js index 65cf34e1..354dabca 100644 --- a/src/app/components/LoginModal.js +++ b/src/app/components/LoginModal.js @@ -58,14 +58,27 @@ function LoginForm({ onSuccess }) { const [ sendLoginMutation, { loading, error, data, called, reset }, - ] = useMutation(gql` - mutation LoginForm_Login($username: String!, $password: String!) { - login(username: $username, password: $password) { - id - username + ] = useMutation( + gql` + mutation LoginForm_Login($username: String!, $password: String!) { + login(username: $username, password: $password) { + id + } } + `, + { + update: (cache) => { + // Evict the `currentUser` from the cache, which will force all queries + // on the page that depend on it to update. (This includes the + // GlobalHeader that shows who you're logged in as!) + // + // I don't do any optimistic UI here, because auth is complex enough + // that I'd rather only show login success after validating it through + // an actual server round-trip. + cache.evict({ id: "ROOT_QUERY", fieldName: "currentUser" }); + }, } - `); + ); return (
{ + // On error, we don't report anything to the user, but we do keep a + // record in the console. We figure that most errors are likely to be + // solvable by retrying the login button and creating a new session, + // which the user would do without an error prompt anyway; and if not, + // they'll either get an error when they try, or they'll see their + // login state continue to not work, which should be a clear hint that + // something is wrong and they need to reach out. + console.error("[useCurrentUser] Couldn't get current user:", error); + }, + // We set this option so that, when we enter the loading state after + // logging in and evicting `currentUser` from the cache, we'll see the + // `loading: true` state. Otherwise, Apollo just leaves the return value + // as-is until the new data comes in, so the user sees the logged-out + // state until the behind-the-scenes update to this query finishes. + notifyOnNetworkStatusChange: true, + } + ); + + if (!isEnabled) { + return NOT_LOGGED_IN_USER; + } else if (loading) { + return { ...NOT_LOGGED_IN_USER, isLoading: true }; + } else if (data?.currentUser == null) { + return NOT_LOGGED_IN_USER; + } else { + return { + isLoading: false, + isLoggedIn: true, + id: data.currentUser.id, + username: data.currentUser.username, + }; + } } export function readCypressLoginData() { @@ -56,7 +149,7 @@ export function readCypressLoginData() { } } -function getUserInfo(user) { +function getUserInfoFromAuth0Data(user) { return { id: user.sub?.match(/^auth0\|impress-([0-9]+)$/)?.[1], username: user["https://oauth.impress-2020.openneo.net/username"], @@ -114,7 +207,7 @@ export function useAuthModeFeatureFlag() { // default to `null` instead of "auth0", I want to be unambiguous that this // is the *absence* of a localStorage value, and not risk accidentally // setting this override value to auth0 on everyone's devices 😅) - const [savedValue] = useLocalStorage("DTIAuthModeFeatureFlag", null); + let [savedValue] = useLocalStorage("DTIAuthModeFeatureFlag", null); useEffect(() => { window.setAuthModeFeatureFlag = setAuthModeFeatureFlag; @@ -122,10 +215,37 @@ export function useAuthModeFeatureFlag() { if (!["auth0", "db", null].includes(savedValue)) { console.warn( - `Unexpected DTIAuthModeFeatureFlag value: %o. Treating as null.`, + `Unexpected DTIAuthModeFeatureFlag value: %o. Ignoring.`, savedValue ); - return null; + savedValue = null; + } + + return savedValue || "auth0"; +} + +/** + * getAuthModeFeatureFlag returns the authMode at the time it's called. + * It's generally preferable to use `useAuthModeFeatureFlag` in a React + * setting, but we use this instead for Apollo stuff! + */ +export function getAuthModeFeatureFlag() { + const savedValueString = localStorage.getItem("DTIAuthModeFeatureFlag"); + + let savedValue; + try { + savedValue = JSON.parse(savedValueString); + } catch (error) { + console.warn(`DTIAuthModeFeatureFlag was not valid JSON. Ignoring.`); + savedValue = null; + } + + if (!["auth0", "db", null].includes(savedValue)) { + console.warn( + `Unexpected DTIAuthModeFeatureFlag value: %o. Ignoring.`, + savedValue + ); + savedValue = null; } return savedValue || "auth0"; diff --git a/src/server/auth-by-db.js b/src/server/auth-by-db.js index 294dc560..dfcb958e 100644 --- a/src/server/auth-by-db.js +++ b/src/server/auth-by-db.js @@ -65,23 +65,11 @@ export async function getAuthToken({ username, password }, db) { // algorithm so we chose it again), and the key this time is a secret global // value called `DTI_AUTH_TOKEN_SECRET`. This proves that the auth token was // generated by the app, because only the app knows the secret. - if (process.env["DTI_AUTH_TOKEN_SECRET"] == null) { - throw new Error( - `The DTI_AUTH_TOKEN_SECRET environment variable is missing. ` + - `The server admin should create a random secret, and save it in the ` + - `.env file.` - ); - } const unsignedAuthToken = { userId: impressId, createdAt: new Date().toISOString(), }; - const authTokenHmac = createHmac( - "sha256", - process.env["DTI_AUTH_TOKEN_SECRET"] - ); - authTokenHmac.update(JSON.stringify(unsignedAuthToken)); - const signature = authTokenHmac.digest("hex"); + const signature = computeSignatureForAuthToken(unsignedAuthToken); const authToken = { ...unsignedAuthToken, signature }; // Login success! Return the auth token. The caller will handle setting it to @@ -89,3 +77,51 @@ export async function getAuthToken({ username, password }, db) { console.debug(`[getAuthToken] Succeeded: ${JSON.stringify(authToken)}`); return authToken; } + +export async function getUserIdFromToken(authToken) { + // Check the auth token's signature, to make sure we're the ones who created + // it. (The signature depends on the DTI_AUTH_TOKEN_SECRET, so we should be + // the only ones who can generate accurate signatures.) + const { signature, ...unsignedAuthToken } = authToken; + const actualSignature = computeSignatureForAuthToken(unsignedAuthToken); + if (signature !== actualSignature) { + console.warn( + `[getUserIdFromToken] Signature ${signature} did not match auth ` + + `token. Rejecting.` + ); + return null; + } + + // Then, check that the cookie was created within the past week. If not, + // treat it as expired; we'll have the user log in again, as a general + // security practice. + const oneWeekAgo = new Date(); + oneWeekAgo.setDate(oneWeekAgo.getDate() - 7); + if (authToken.createdAt < oneWeekAgo) { + console.warn( + `[getUserIdFromToken] Auth token expired, was created at ` + + `${authToken.createdAt}. Rejecting.` + ); + return null; + } + + // Okay, it passed validation: this is a real auth token generated by us, and + // it hasn't expired. Now we can safely trust it: return its own userId! + return authToken.userId; +} + +function computeSignatureForAuthToken(unsignedAuthToken) { + if (process.env["DTI_AUTH_TOKEN_SECRET"] == null) { + throw new Error( + `The DTI_AUTH_TOKEN_SECRET environment variable is missing. ` + + `The server admin should create a random secret, and save it in the ` + + `.env file.` + ); + } + const authTokenHmac = createHmac( + "sha256", + process.env["DTI_AUTH_TOKEN_SECRET"] + ); + authTokenHmac.update(JSON.stringify(unsignedAuthToken)); + return authTokenHmac.digest("hex"); +} diff --git a/src/server/index.js b/src/server/index.js index 286ad48b..82470808 100644 --- a/src/server/index.js +++ b/src/server/index.js @@ -1,10 +1,13 @@ import { beelinePlugin } from "./lib/beeline-graphql"; import { gql, makeExecutableSchema } from "apollo-server"; -import { getUserIdFromToken } from "./auth"; +import { getUserIdFromToken as getUserIdFromTokenViaAuth0 } from "./auth"; import connectToDb from "./db"; import buildLoaders from "./loaders"; import { plugin as cacheControlPluginFork } from "./lib/apollo-cache-control-fork"; -import { getAuthToken } from "./auth-by-db"; +import { + getAuthToken, + getUserIdFromToken as getUserIdFromTokenViaDb, +} from "./auth-by-db"; const rootTypeDefs = gql` enum CacheScope { @@ -64,10 +67,21 @@ const config = { context: async ({ req, res }) => { const db = await connectToDb(); - const auth = (req && req.headers && req.headers.authorization) || ""; - const authMatch = auth.match(/^Bearer (.+)$/); - const token = authMatch && authMatch[1]; - const currentUserId = await getUserIdFromToken(token); + let authMode = req.headers["dti-auth-mode"] || "auth0"; + let currentUserId; + if (authMode === "auth0") { + const auth = (req && req.headers && req.headers.authorization) || ""; + const authMatch = auth.match(/^Bearer (.+)$/); + const token = authMatch && authMatch[1]; + currentUserId = await getUserIdFromTokenViaAuth0(token); + } else if (authMode === "db") { + currentUserId = await getCurrentUserIdViaDb(req); + } else { + console.warn( + `Unexpected auth mode: ${JSON.stringify(authMode)}. Skipping auth.` + ); + currentUserId = null; + } return { db, @@ -118,6 +132,22 @@ const config = { }, }; +async function getCurrentUserIdViaDb(req) { + const authTokenCookieString = req.cookies.DTIAuthToken; + if (!authTokenCookieString) { + return null; + } + + let authTokenFromCookie = null; + try { + authTokenFromCookie = JSON.parse(authTokenCookieString); + } catch (error) { + console.warn(`DTIAuthToken cookie was not valid JSON, ignoring.`); + } + + return await getUserIdFromTokenViaDb(authTokenFromCookie); +} + if (require.main === module) { const { ApolloServer } = require("apollo-server"); const server = new ApolloServer(config);