Skip to content

Commit

Permalink
Add auth refresh token (#727)
Browse files Browse the repository at this point in the history
* Add authRefreshToken cookie
* Ignore standaloneAjv for CodeQL

---------

Signed-off-by: Carl Gieringer <78054+carlgieringer@users.noreply.github.com>
  • Loading branch information
carlgieringer authored Sep 1, 2024
1 parent 170b2d6 commit a95df32
Show file tree
Hide file tree
Showing 73 changed files with 1,262 additions and 361 deletions.
9 changes: 6 additions & 3 deletions .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:

# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
uses: github/codeql-action/init@v2
uses: github/codeql-action/init@v3
with:
languages: ${{ matrix.language }}
# If you wish to specify custom queries, you can do so here or in a config file.
Expand All @@ -38,16 +38,19 @@ jobs:

# Details on CodeQL's query packs refer to: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs
queries: security-and-quality
config: |
paths-ignore:
- howdju-common/lib/standaloneAjv.js
# Autobuild attempts to build any compiled languages (C/C++, C#, Go, or Java).
# If this step fails, then you should remove it and run the build manually (see below)
- name: Autobuild
uses: github/codeql-action/autobuild@v2
uses: github/codeql-action/autobuild@v3

# ℹ️ Command-line programs to run using the OS shell.
# 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun

- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@v2
uses: github/codeql-action/analyze@v3
with:
category: "/language:${{ matrix.language }}"
2 changes: 1 addition & 1 deletion bin/check-todo-format.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ target=${2:-}

# Equivalent for the whole repo:
# egrep --exclude=check-todo-format\.sh --exclude-dir=node_modules --exclude-dir=dist --exclude-dir=coverage --exclude-dir=Pods --exclude-dir=\.git -RI '\bTODO\b' . | egrep -v '\bTODO\((\d+,?)+\)'
output=$(git diff $base $target ':(exclude)bin/check-todo-format.sh' ':(exclude).github/workflows/ci.yml' ':(exclude)howdju-text-fragments/dist/*')
output=$(git diff $base $target ':(exclude)bin/check-todo-format.sh' ':(exclude).github/workflows/ci.yml' ':(exclude)howdju-text-fragments/dist/*' ':(exclude)docs/misc/Web app TODO.md')

if [[ $? -ne 0 ]]; then
echo Error getting diff for TODO check
Expand Down
5 changes: 1 addition & 4 deletions docs/misc/Web app TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,6 @@ Followed all directions here: https://web.archive.org/web/20201020180006/https:/

- move out configuration

- Security
- Add an HttpOnly, Secure cookie so that authentication can't be taken off site (no XSS can steal all the credentials
necessary to make a change. Keep the local storage token as that prevents CSRF - the attacker needs a script on teh page
to read that.)
- Support hidden source maps?
- https://docs.sentry.io/learn/cli/releases/#upload-source-maps

Expand All @@ -210,6 +206,7 @@ Improvements to propositions trail:
- https://stackoverflow.com/a/39507279/39396

- local env has production sentry env?

- This occurs when we deploy to production while serving dev. The deployment replaces index.html

- While researching these old propositions, it's really annoying not to be able to know where the citations are hidden.
Expand Down
18 changes: 11 additions & 7 deletions howdju-common/lib/codes/apiErrorCodes.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,25 @@
/** Explain why a particular HTTP Status occurred */
export const apiErrorCodes = {
/** The request lacks authentication credentials */
UNAUTHENTICATED: "UNAUTHENTICATED",

/** The data submitted was invalid */
VALIDATION_ERROR: "VALIDATION_ERROR",

/** The client sent an invalid request. The response should explain what was invalid. */
INVALID_REQUEST: "INVALID_REQUEST",

AUTHENTICATION_ERROR: "AUTHENTICATION_ERROR",
/** The request lacks authentication credentials, but the requested resource requires them. */
UNAUTHENTICATED: "UNAUTHENTICATED",

/** The user tried to login with invalid login credentials */
INVALID_LOGIN_CREDENTIALS: "INVALID_LOGIN_CREDENTIALS",
/** The client's auth token has expired. They can try to refresh it. */
AUTHENTICATION_EXPIRED: "AUTHENTICATION_EXPIRED",

/** The principle authenticated by the request lacks authority to perform the action */
AUTHORIZATION_ERROR: "AUTHORIZATION_ERROR",
UNAUTHORIZED: "UNAUTHORIZED",

/** The clients auth refresh token has expired. They must reathenticate. */
REAUTHENTICATION_REQUIRED: "REAUTHENTICATION_REQUIRED",

/** The user tried to login with invalid login credentials */
INVALID_LOGIN_CREDENTIALS: "INVALID_LOGIN_CREDENTIALS",

/** The user account is inactive. Should only respond with this if the correct password was provided */
USER_IS_INACTIVE_ERROR: "USER_IS_INACTIVE_ERROR",
Expand Down
24 changes: 22 additions & 2 deletions howdju-common/lib/entities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,31 @@ export type ContinuationToken = string;
/**
* A token authorizing clients to take actions as users.
*
* Clients must keep this secret, they must expire them before too long, and
* we must enforce the expirations server-side.
* Clients must:
*
* - not persist them
* - keep them secret
* - expire them on the order of hours to days. (Recommendation: 1 hour; max: 24 hours)
*
* The server must also enforce expirations that are not client-based.
*/
export type AuthToken = string;

/**
* A token that can be used to refresh an {@link AuthToken}.
*
* Clients should persist these securely. An example of acceptable storage is a
* secure, HTTP-only cookie. When this expires, the user must reauthenticate.
*
* Clients must:
*
* - keep them secret
* - expire them on the order of weeks to months. (Recommendation: 3 months)
*
* The server must also enforce expirations that are not client-based.
*/
export type AuthRefreshToken = string;

/**
* A timestamp .
*
Expand Down
2 changes: 1 addition & 1 deletion howdju-common/lib/standaloneAjv.js

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions howdju-common/lib/zodSchemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1605,6 +1605,8 @@ export type CreateTagVote = z.infer<typeof CreateTagVote>;

const EntityType = z.enum([
"APPEARANCE",
"AUTH_TOKEN",
"AUTH_REFRESH_TOKEN",
"JUSTIFICATION",
"JUSTIFICATION_VOTE",
"MEDIA_EXCERPT",
Expand Down
4 changes: 3 additions & 1 deletion howdju-service-common/lib/config.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import moment from "moment";

export const baseConfig = {
authRefreshCookie: { isSecure: true },
auth: {
bcrypt: {
saltRounds: 10,
},
},
authTokenDuration: { days: 30 },
authTokenDuration: { hours: 2 },
authRefreshTokenDuration: { days: 90 },
contentReportNotificationEmails: [] as string[],
corsAllowOrigin: [] as string[],
/** Whether to prevent responses that indicate whether an email has been registered with the system */
Expand Down
38 changes: 38 additions & 0 deletions howdju-service-common/lib/cookies.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { Moment } from "moment";
import { z } from "zod";

import { AuthRefreshToken, momentObject } from "howdju-common";

export const Cookie = z.object({
name: z.string(),
value: z.string(),
domain: z.string().optional(),
path: z.string().optional(),
expires: momentObject,
secure: z.boolean().optional(),
sameSite: z.enum(["strict", "lax", "none"]).optional(),
httpOnly: z.boolean().optional(),
});
export type Cookie = z.infer<typeof Cookie>;

export const cookieNames = {
AUTH_REFRESH_TOKEN: "auth-refresh-token",
};

export function createAuthRefreshCookie(
token: AuthRefreshToken,
expires: Moment,
isSecure: boolean
): Cookie {
return {
name: cookieNames.AUTH_REFRESH_TOKEN,
value: token,
// undefined sets it to the current domain, ignoring subdomains.
domain: undefined,
path: "/",
expires,
secure: isSecure,
sameSite: "strict",
httpOnly: true,
};
}
100 changes: 100 additions & 0 deletions howdju-service-common/lib/daos/AuthDao.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import { momentAdd, utcNow } from "howdju-common";
import { expectToBeSameMomentDeep, mockLogger } from "howdju-test-common";

import {
endPoolAndDropDb,
initDb,
makeTestClientProvider,
makeTestDbConfig,
} from "@/util/testUtil";
import { Database, PoolClientProvider } from "../database";
import { makeTestProvider } from "@/initializers/TestProvider";
import TestHelper from "@/initializers/TestHelper";
import { AuthDao } from "./AuthDao";

const dbConfig = makeTestDbConfig();

describe("AuthDao", () => {
let dbName: string;
let clientProvider: PoolClientProvider;
let database: Database;

let dao: AuthDao;
let testHelper: TestHelper;
beforeEach(async () => {
dbName = await initDb(dbConfig);

clientProvider = makeTestClientProvider({
...dbConfig,
database: dbName,
});
database = new Database(mockLogger, clientProvider);

const provider = makeTestProvider(database);

dao = provider.authDao;
testHelper = provider.testHelper;
});
afterEach(async () => {
await endPoolAndDropDb(clientProvider, dbConfig, dbName);
});

describe("createAuthRefreshToken", () => {
test("cannot create duplicate auth refresh tokens", async () => {
const { user } = await testHelper.makeUser();
const { id: userId } = user;
const authRefreshToken = "the-auth-refresh-token";
const created = utcNow();
const expires = momentAdd(utcNow(), { days: 1 });
await dao.createAuthRefreshToken(
userId,
authRefreshToken,
created,
expires
);

await expect(
dao.createAuthRefreshToken(userId, authRefreshToken, created, expires)
).rejects.toThrow("duplicate key value violates unique constraint");
});
});
describe("readAuthRefreshToken", () => {
test("can read an auth refresh token", async () => {
const { user } = await testHelper.makeUser();
const { id: userId } = user;
const authRefreshToken = "the-auth-refresh-token";
const created = utcNow();
const expires = momentAdd(utcNow(), { days: 1 });
await dao.createAuthRefreshToken(
userId,
authRefreshToken,
created,
expires
);

const result = await dao.readAuthRefreshToken(authRefreshToken);

expect(result).toEqual(expectToBeSameMomentDeep({ userId, expires }));
});
});
describe("deleteAuthRefreshToken", () => {
test("cannot read a deleted auth refresh token", async () => {
const { user } = await testHelper.makeUser();
const { id: userId } = user;
const authRefreshToken = "the-auth-refresh-token";
const created = utcNow();
const expires = momentAdd(utcNow(), { days: 1 });
await dao.createAuthRefreshToken(
userId,
authRefreshToken,
created,
expires
);

await dao.deleteAuthRefreshToken(authRefreshToken);

const result = await dao.readAuthRefreshToken(authRefreshToken);
expect(result).toBeUndefined();
});
});
});
56 changes: 54 additions & 2 deletions howdju-service-common/lib/daos/AuthDao.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { get, toString } from "lodash";
import { Moment } from "moment";

import { AuthToken, EntityId, utcNow } from "howdju-common";
import { AuthRefreshToken, AuthToken, EntityId, utcNow } from "howdju-common";

import { toUserHash } from "./orm";
import { Database, UserHashRow } from "..";
Expand Down Expand Up @@ -80,7 +80,7 @@ export class AuthDao {
return toUserHash(rows[0]);
}

async insertAuthToken(
async createAuthToken(
userId: EntityId,
authToken: AuthToken,
created: Moment,
Expand All @@ -96,6 +96,58 @@ export class AuthDao {
return get(row, "auth_token");
}

async createAuthRefreshToken(
userId: EntityId,
authRefreshToken: AuthRefreshToken,
created: Moment,
expires: Moment
) {
const {
rows: [row],
} = await this.database.query(
"insertAuthRefreshToken",
`insert into auth_refresh_tokens
(user_id, auth_refresh_token, created, expires)
values ($1, $2, $3, $4)
returning auth_refresh_token`,
[userId, authRefreshToken, created, expires]
);
return row.auth_refresh_token;
}

async readAuthRefreshToken(
authRefreshToken: AuthRefreshToken
): Promise<{ userId: EntityId; expires: Moment } | undefined> {
const {
rows: [row],
} = await this.database.query(
"readAuthRefreshToken",
`select user_id, expires
from auth_refresh_tokens
where auth_refresh_token = $1 and deleted is null`,
[authRefreshToken]
);
if (!row) {
return undefined;
}
const { user_id: userId, expires } = row;
return { userId, expires };
}

async deleteAuthRefreshToken(authRefreshToken: AuthRefreshToken) {
const {
rows: [row],
} = await this.database.query(
"deleteAuthRefreshToken",
`update auth_refresh_tokens
set deleted = now()
where auth_refresh_token = $1
returning auth_refresh_token_id`,
[authRefreshToken]
);
return row?.auth_refresh_token_id;
}

async deleteAuthToken(authToken: AuthToken) {
const {
rows: [row],
Expand Down
4 changes: 3 additions & 1 deletion howdju-service-common/lib/daos/UsersDao.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ export class UsersDao {
rows: [row],
} = await this.database.query<UserRow>(
"readUserForId",
"select * from users join user_external_ids using (user_id) where user_id = $1 and deleted is null",
`select *
from users left outer join user_external_ids using (user_id)
where user_id = $1 and deleted is null`,
[userId]
);
return toUser(row);
Expand Down
20 changes: 20 additions & 0 deletions howdju-service-common/lib/database/pg.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import moment from "moment";

import { mockLogger } from "howdju-test-common";

import { makeTimestampToUtcMomentParser } from "./pg";

const parser = makeTimestampToUtcMomentParser(mockLogger);

describe("makeTimestampToUtcMomentParser", () => {
test("parses a timezone-naive timestamp", () => {
expect(parser("2024-08-31 21:24:30.45")).toBeSameMoment(
moment.utc("2024-08-31 21:24:30.45")
);
});
test("parses a timezone-aware timestamp", () => {
expect(parser("2024-11-29 20:20:00.119+00")).toBeSameMoment(
moment.utc("2024-11-29 20:20:00.119+00")
);
});
});
Loading

0 comments on commit a95df32

Please sign in to comment.