From f58270a8cb27b2cca50999c4e23f84d5e22c2b6e Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 11 Mar 2024 10:41:45 -0500 Subject: [PATCH 1/8] Factor out api hostname --- app/services/authn.server.ts | 2 ++ app/services/rfdApi.server.ts | 2 +- app/utils/rfdApiStrategy.ts | 17 +++++++++++------ 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/app/services/authn.server.ts b/app/services/authn.server.ts index c9c7ab5..02fd8de 100644 --- a/app/services/authn.server.ts +++ b/app/services/authn.server.ts @@ -88,6 +88,7 @@ const auth = new Authenticator(sessionStorage) auth.use( new RfdApiStrategy( { + host: process.env.RFD_API || '', clientID: process.env.RFD_API_CLIENT_ID || '', clientSecret: process.env.RFD_API_CLIENT_SECRET || '', callbackURL: process.env.RFD_API_GOOGLE_CALLBACK_URL || '', @@ -103,6 +104,7 @@ auth.use( auth.use( new RfdApiStrategy( { + host: process.env.RFD_API || '', clientID: process.env.RFD_API_CLIENT_ID || '', clientSecret: process.env.RFD_API_CLIENT_SECRET || '', callbackURL: process.env.RFD_API_GITHUB_CALLBACK_URL || '', diff --git a/app/services/rfdApi.server.ts b/app/services/rfdApi.server.ts index 9d0acff..d210997 100644 --- a/app/services/rfdApi.server.ts +++ b/app/services/rfdApi.server.ts @@ -18,7 +18,7 @@ export async function apiRequest( headers['Authorization'] = `Bearer ${accessToken}` } - const url = `https://rfd-api.shared.oxide.computer/${path.replace(/^\//, '')}` + const url = `${process.env.RFD_API}/${path.replace(/^\//, '')}` console.info(`Requesting ${url} from the RFD API`) const response = await fetch(url, { headers }) diff --git a/app/utils/rfdApiStrategy.ts b/app/utils/rfdApiStrategy.ts index 43d9c1d..e4877e9 100644 --- a/app/utils/rfdApiStrategy.ts +++ b/app/utils/rfdApiStrategy.ts @@ -17,6 +17,7 @@ import { import type { RfdApiPermission, RfdApiProvider, RfdScope } from './rfdApi' export type RfdApiStrategyOptions = { + host: string clientID: string clientSecret: string callbackURL: string @@ -85,11 +86,10 @@ export class RfdApiStrategy extends OAuth2Strategy< RfdApiExtraParams > { public name = `rfd-api` - - private readonly userInfoURL = 'https://rfd-api.shared.oxide.computer/self' + protected host = `` constructor( - { clientID, clientSecret, callbackURL, remoteProvider, scope }: RfdApiStrategyOptions, + { host, clientID, clientSecret, callbackURL, remoteProvider, scope }: RfdApiStrategyOptions, verify: StrategyVerifyCallback< User, OAuth2StrategyVerifyParams @@ -100,13 +100,18 @@ export class RfdApiStrategy extends OAuth2Strategy< clientID, clientSecret, callbackURL, - authorizationURL: `https://rfd-api.shared.oxide.computer/login/oauth/${remoteProvider}/code/authorize`, - tokenURL: `https://rfd-api.shared.oxide.computer/login/oauth/${remoteProvider}/code/token`, + authorizationURL: `${host}/login/oauth/${remoteProvider}/code/authorize`, + tokenURL: `${host}/login/oauth/${remoteProvider}/code/token`, }, verify, ) this.name = `${this.name}-${remoteProvider}` this.scope = this.parseScope(scope) + this.host = host + } + + protected userInfoUrl(): string { + return `${this.host}/self` } protected authorizationParams(): URLSearchParams { @@ -115,7 +120,7 @@ export class RfdApiStrategy extends OAuth2Strategy< } protected async userProfile(accessToken: string): Promise { - const response = await fetch(this.userInfoURL, { + const response = await fetch(this.userInfoUrl(), { headers: { Authorization: `Bearer ${accessToken}`, }, From da480c27714d5450eb37593850f0f44a2a412b9b Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 11 Mar 2024 10:43:24 -0500 Subject: [PATCH 2/8] Compute in constructor --- app/utils/rfdApiStrategy.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/app/utils/rfdApiStrategy.ts b/app/utils/rfdApiStrategy.ts index e4877e9..d9f148c 100644 --- a/app/utils/rfdApiStrategy.ts +++ b/app/utils/rfdApiStrategy.ts @@ -86,7 +86,7 @@ export class RfdApiStrategy extends OAuth2Strategy< RfdApiExtraParams > { public name = `rfd-api` - protected host = `` + protected userInfoUrl = `` constructor( { host, clientID, clientSecret, callbackURL, remoteProvider, scope }: RfdApiStrategyOptions, @@ -107,11 +107,7 @@ export class RfdApiStrategy extends OAuth2Strategy< ) this.name = `${this.name}-${remoteProvider}` this.scope = this.parseScope(scope) - this.host = host - } - - protected userInfoUrl(): string { - return `${this.host}/self` + this.userInfoUrl = `${host}/self` } protected authorizationParams(): URLSearchParams { @@ -120,7 +116,7 @@ export class RfdApiStrategy extends OAuth2Strategy< } protected async userProfile(accessToken: string): Promise { - const response = await fetch(this.userInfoUrl(), { + const response = await fetch(this.userInfoUrl, { headers: { Authorization: `Bearer ${accessToken}`, }, From 3fcf70250460e1ab2174149166117ce0c0c6b53b Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 11 Mar 2024 10:46:04 -0500 Subject: [PATCH 3/8] Fmt --- app/utils/rfdApiStrategy.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/utils/rfdApiStrategy.ts b/app/utils/rfdApiStrategy.ts index d9f148c..38140a5 100644 --- a/app/utils/rfdApiStrategy.ts +++ b/app/utils/rfdApiStrategy.ts @@ -89,7 +89,14 @@ export class RfdApiStrategy extends OAuth2Strategy< protected userInfoUrl = `` constructor( - { host, clientID, clientSecret, callbackURL, remoteProvider, scope }: RfdApiStrategyOptions, + { + host, + clientID, + clientSecret, + callbackURL, + remoteProvider, + scope, + }: RfdApiStrategyOptions, verify: StrategyVerifyCallback< User, OAuth2StrategyVerifyParams From 17a5d4db016e9460e5e318dc20d5fdcc4c97c8fd Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 11 Mar 2024 11:01:56 -0500 Subject: [PATCH 4/8] Explain env vars --- README.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/README.md b/README.md index 2010f02..0f3e48c 100644 --- a/README.md +++ b/README.md @@ -96,6 +96,26 @@ current branch. Missing RFDs will 404. If you are working on two RFDs and they'r different branches, you cannot preview both at the same time unless you make a temporary combined branch that contains both. +### Configuration + +When running in a non-local mode, the following settings must be specified: + +* `SESSION_SECRET` - Key that will be used to signed cookies + +* `RFD_API` - Backend RFD API to communicate with (i.e. https://api.server.com) +* `RFD_API_CLIENT_ID` - OAuth client id create via the RFD API +* `RFD_API_CLIENT_SECRET` - OAuth client secret create via the RFD API +* `RFD_API_GOOGLE_CALLBACK_URL` - Should be of the form of `https://{rfd_site_hostname}/auth/google/callback` +* `RFD_API_GITHUB_CALLBACK_URL` - Should be of the form of `https://{rfd_site_hostname}/auth/github/callback` + +* `STORAGE_URL` - Url of bucket for static assets +* `STORAGE_KEY_NAME` - Name of the key defined in `STORAGE_KEY` +* `STORAGE_KEY` - Key for generating signed static asset urls + +* `GITHUB_APP_ID` - App id for fetching GitHub PR discussions +* `GITHUB_INSTALLATION_ID` - Installation id of GitHub App +* `GITHUB_PRIVATE_KEY` - Private key of the GitHub app for discussion fetching + ## License Unless otherwise noted, all components are licensed under the From aa5e290f1dada9b64ccbebdd706338700c9018ab Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 11 Mar 2024 11:03:35 -0500 Subject: [PATCH 5/8] Fmt --- README.md | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 0f3e48c..5ca8994 100644 --- a/README.md +++ b/README.md @@ -100,21 +100,23 @@ combined branch that contains both. When running in a non-local mode, the following settings must be specified: -* `SESSION_SECRET` - Key that will be used to signed cookies - -* `RFD_API` - Backend RFD API to communicate with (i.e. https://api.server.com) -* `RFD_API_CLIENT_ID` - OAuth client id create via the RFD API -* `RFD_API_CLIENT_SECRET` - OAuth client secret create via the RFD API -* `RFD_API_GOOGLE_CALLBACK_URL` - Should be of the form of `https://{rfd_site_hostname}/auth/google/callback` -* `RFD_API_GITHUB_CALLBACK_URL` - Should be of the form of `https://{rfd_site_hostname}/auth/github/callback` - -* `STORAGE_URL` - Url of bucket for static assets -* `STORAGE_KEY_NAME` - Name of the key defined in `STORAGE_KEY` -* `STORAGE_KEY` - Key for generating signed static asset urls - -* `GITHUB_APP_ID` - App id for fetching GitHub PR discussions -* `GITHUB_INSTALLATION_ID` - Installation id of GitHub App -* `GITHUB_PRIVATE_KEY` - Private key of the GitHub app for discussion fetching +- `SESSION_SECRET` - Key that will be used to signed cookies + +- `RFD_API` - Backend RFD API to communicate with (i.e. https://api.server.com) +- `RFD_API_CLIENT_ID` - OAuth client id create via the RFD API +- `RFD_API_CLIENT_SECRET` - OAuth client secret create via the RFD API +- `RFD_API_GOOGLE_CALLBACK_URL` - Should be of the form of + `https://{rfd_site_hostname}/auth/google/callback` +- `RFD_API_GITHUB_CALLBACK_URL` - Should be of the form of + `https://{rfd_site_hostname}/auth/github/callback` + +- `STORAGE_URL` - Url of bucket for static assets +- `STORAGE_KEY_NAME` - Name of the key defined in `STORAGE_KEY` +- `STORAGE_KEY` - Key for generating signed static asset urls + +- `GITHUB_APP_ID` - App id for fetching GitHub PR discussions +- `GITHUB_INSTALLATION_ID` - Installation id of GitHub App +- `GITHUB_PRIVATE_KEY` - Private key of the GitHub app for discussion fetching ## License From 098f268ebe1a5d2e018bd134c39753fafa37d015 Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Mon, 11 Mar 2024 16:17:44 +0000 Subject: [PATCH 6/8] Bump commit From 897dbfe1711aa9e7cf11db04b99d567c3c944d83 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 11 Mar 2024 11:32:57 -0500 Subject: [PATCH 7/8] Require RFD_API in non-local mode --- app/services/authn.server.ts | 6 +++--- app/services/rfdApi.server.ts | 19 ++++++++++++++++++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/app/services/authn.server.ts b/app/services/authn.server.ts index 02fd8de..4fd43cf 100644 --- a/app/services/authn.server.ts +++ b/app/services/authn.server.ts @@ -19,7 +19,7 @@ import { import { returnToCookie } from './cookies.server' import { getUserRedirect } from './redirect.server' import { isLocalMode } from './rfd.server' -import { apiRequest } from './rfdApi.server' +import { apiRequest, getRfdApiUrl } from './rfdApi.server' export type AuthenticationService = 'github' | 'google' | 'local' @@ -88,7 +88,7 @@ const auth = new Authenticator(sessionStorage) auth.use( new RfdApiStrategy( { - host: process.env.RFD_API || '', + host: getRfdApiUrl(), clientID: process.env.RFD_API_CLIENT_ID || '', clientSecret: process.env.RFD_API_CLIENT_SECRET || '', callbackURL: process.env.RFD_API_GOOGLE_CALLBACK_URL || '', @@ -104,7 +104,7 @@ auth.use( auth.use( new RfdApiStrategy( { - host: process.env.RFD_API || '', + host: getRfdApiUrl(), clientID: process.env.RFD_API_CLIENT_ID || '', clientSecret: process.env.RFD_API_CLIENT_SECRET || '', callbackURL: process.env.RFD_API_GITHUB_CALLBACK_URL || '', diff --git a/app/services/rfdApi.server.ts b/app/services/rfdApi.server.ts index d210997..c750dd9 100644 --- a/app/services/rfdApi.server.ts +++ b/app/services/rfdApi.server.ts @@ -6,6 +6,23 @@ * Copyright Oxide Computer Company */ +import { isLocalMode } from './rfd.server' + +export function getRfdApiUrl(): string { + // If we are loading in local mode, then the API is not used, and it is fine to return + // and invalid value + if (isLocalMode) { + return '' + } + + // Otherwise crash the system if we do not have an API target set + if (!process.env.RFD_API) { + throw Error('Env var RFD_API must be set when not running in local mode') + } + + return process.env.RFD_API +} + export async function apiRequest( path: string, accessToken: string | undefined, @@ -18,7 +35,7 @@ export async function apiRequest( headers['Authorization'] = `Bearer ${accessToken}` } - const url = `${process.env.RFD_API}/${path.replace(/^\//, '')}` + const url = `${getRfdApiUrl()}/${path.replace(/^\//, '')}` console.info(`Requesting ${url} from the RFD API`) const response = await fetch(url, { headers }) From 77d9dfbc117c06686bf4f29771404c739873c79f Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 11 Mar 2024 12:17:10 -0500 Subject: [PATCH 8/8] Move canUser to the appropriate location --- app/services/rfd.server.ts | 12 +++++++++++- app/utils/permission.ts | 13 ------------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/app/services/rfd.server.ts b/app/services/rfd.server.ts index 9d7f93a..948aa90 100644 --- a/app/services/rfd.server.ts +++ b/app/services/rfd.server.ts @@ -15,7 +15,7 @@ import { Octokit } from 'octokit' import { generateAuthors, type Author } from '~/components/rfd/RfdPreview' import { isTruthy } from '~/utils/isTruthy' import { parseRfdNum } from '~/utils/parseRfdNum' -import { canUser } from '~/utils/permission' +import { can, Permission } from '~/utils/permission' import type { GroupResponse, RfdListResponseItem, RfdResponse } from '~/utils/rfdApi' import type { Group, User } from './authn.server' @@ -55,6 +55,16 @@ export type RfdListItem = { const localRepo = process.env.LOCAL_RFD_REPO export const isLocalMode = process.env.NODE_ENV === 'development' && localRepo +async function canUser(user: User, permission: Permission): Promise { + const groups = (await fetchGroups(user)).filter((group) => + user.groups.includes(group.name), + ) + const allPermissions = user.permissions.concat( + groups.flatMap((group) => group.permissions), + ) + return can(allPermissions, permission) +} + function findLineStartingWith(content: string, prefixRegex: string): string | undefined { // (^|\n) is required to match either the first line (beginning of file) or // subsequent lines diff --git a/app/utils/permission.ts b/app/utils/permission.ts index 2a4609d..862ca74 100644 --- a/app/utils/permission.ts +++ b/app/utils/permission.ts @@ -6,23 +6,10 @@ * Copyright Oxide Computer Company */ -import type { User } from '~/services/authn.server' -import { fetchGroups } from '~/services/rfd.server' - import type { RfdApiPermission } from './rfdApi' export type Permission = { k: 'ReadDiscussions' } | { k: 'ReadRfd'; v: number } -export async function canUser(user: User, permission: Permission): Promise { - const groups = (await fetchGroups(user)).filter((group) => - user.groups.includes(group.name), - ) - const allPermissions = user.permissions.concat( - groups.flatMap((group) => group.permissions), - ) - return can(allPermissions, permission) -} - export function can(allPermissions: RfdApiPermission[], permission: Permission): boolean { const checks = createChecks(permission) const allowed = checks.some((check) => performCheck(allPermissions, check))