From 2f64be21d6be2e2c38d3f08c99d019a66178d56e Mon Sep 17 00:00:00 2001 From: Joost De Cock Date: Fri, 18 Aug 2023 17:26:23 +0200 Subject: [PATCH] feat(backend): Be more strict about validating tokens accros backends --- sites/backend/prisma/schema.prisma | 1 + sites/backend/src/config.mjs | 6 ++++-- sites/backend/src/controllers/flows.mjs | 4 ++-- sites/backend/src/middleware.mjs | 13 ++++++++++--- sites/backend/src/models/apikey.mjs | 1 + sites/backend/src/models/flow.mjs | 15 ++++++++------- sites/backend/src/models/user.mjs | 2 +- sites/backend/src/routes/flows.mjs | 16 +++++++++------- 8 files changed, 36 insertions(+), 22 deletions(-) diff --git a/sites/backend/prisma/schema.prisma b/sites/backend/prisma/schema.prisma index 0812554db0a..8b18930698e 100644 --- a/sites/backend/prisma/schema.prisma +++ b/sites/backend/prisma/schema.prisma @@ -10,6 +10,7 @@ datasource db { model Apikey { id String @id @default(uuid()) + aud String @default("") createdAt DateTime @default(now()) expiresAt DateTime name String @default("") diff --git a/sites/backend/src/config.mjs b/sites/backend/src/config.mjs index 686fdc56f23..01edf595d19 100644 --- a/sites/backend/src/config.mjs +++ b/sites/backend/src/config.mjs @@ -50,6 +50,8 @@ const baseConfig = { env: process.env.NODE_ENV || 'development', // Maintainer contact maintainer: 'joost@freesewing.org', + // Instance + instance: process.env.BACKEND_INSTANCE || Date.now(), // Feature flags use: { github: envToBool(process.env.BACKEND_ENABLE_GITHUB), @@ -110,8 +112,7 @@ const baseConfig = { }, jwt: { secretOrKey: encryptionKey, - issuer: process.env.BACKEND_JWT_ISSUER || 'freesewing.org', - audience: process.env.BACKEND_JWT_ISSUER || 'freesewing.org', + issuer: api, expiresIn: process.env.BACKEND_JWT_EXPIRY || '7d', }, languages, @@ -232,6 +233,7 @@ export const cloudflareImages = config.cloudflareImages || {} export const forwardmx = config.forwardmx || {} export const website = config.website export const githubToken = config.github.token +export const instance = config.instance const vars = { BACKEND_DB_URL: ['required', 'db.url'], diff --git a/sites/backend/src/controllers/flows.mjs b/sites/backend/src/controllers/flows.mjs index 7ac31575c86..e131cb1fc9a 100644 --- a/sites/backend/src/controllers/flows.mjs +++ b/sites/backend/src/controllers/flows.mjs @@ -50,9 +50,9 @@ FlowsController.prototype.removeImage = async (req, res, tools) => { * Creates a pull request for a new showcase post * See: https://freesewing.dev/reference/backend/api */ -FlowsController.prototype.createShowcasePr = async (req, res, tools) => { +FlowsController.prototype.createPostPr = async (req, res, tools, type) => { const Flow = new FlowModel(tools) - await Flow.createShowcasePr(req) + await Flow.createPostPr(req, type) return Flow.sendResponse(res) } diff --git a/sites/backend/src/middleware.mjs b/sites/backend/src/middleware.mjs index 71ed9755fed..e1ddbea66e9 100644 --- a/sites/backend/src/middleware.mjs +++ b/sites/backend/src/middleware.mjs @@ -3,6 +3,7 @@ import http from 'passport-http' import jwt from 'passport-jwt' import { ApikeyModel } from './models/apikey.mjs' import { UserModel } from './models/user.mjs' +import { api, instance } from './config.mjs' /* * In v2 we ended up with a bug where we did not properly track the last login @@ -10,8 +11,14 @@ import { UserModel } from './models/user.mjs' * this field. It's a bit of a perf hit to write to the database on ever API call * but it's worth it to actually know which accounts are used and which are not. */ -async function checkAccess(uid, tools, type) { +async function checkAccess(payload, tools, type) { + /* + * Don't allow tokens/keys to be used on different instances, + * even with the same encryption key + */ + if (payload.aud !== `${api}/${instance}`) return false const User = new UserModel(tools) + const uid = payload.userId || payload._id const ok = await User.papersPlease(uid, type) return ok @@ -29,7 +36,7 @@ function loadPassportMiddleware(passport, tools) { /* * We check more than merely the API key */ - const ok = Apikey.verified ? await checkAccess(Apikey.record.userId, tools, 'key') : false + const ok = Apikey.verified ? await checkAccess(Apikey.record, tools, 'key') : false return ok ? done(null, { @@ -50,7 +57,7 @@ function loadPassportMiddleware(passport, tools) { /* * We check more than merely the token */ - const ok = await checkAccess(jwt_payload._id, tools, 'jwt') + const ok = await checkAccess(jwt_payload, tools, 'jwt') return ok ? done(null, { diff --git a/sites/backend/src/models/apikey.mjs b/sites/backend/src/models/apikey.mjs index 90e688b9cee..56073fd1caa 100644 --- a/sites/backend/src/models/apikey.mjs +++ b/sites/backend/src/models/apikey.mjs @@ -261,6 +261,7 @@ ApikeyModel.prototype.create = async function ({ body, user }) { try { this.record = await this.prisma.apikey.create({ data: this.cloak({ + aud: `${this.config.api}/${this.config.instance}`, expiresAt, name: body.name, level: body.level, diff --git a/sites/backend/src/models/flow.mjs b/sites/backend/src/models/flow.mjs index f4296500472..ef2af8acc7a 100644 --- a/sites/backend/src/models/flow.mjs +++ b/sites/backend/src/models/flow.mjs @@ -261,13 +261,14 @@ to English prior to merging. ` /* - * Create a (GitHub) pull request for a new showcase post + * Create a (GitHub) pull request for a new blog or showcase post * * @param {body} object - The request body * @param {user} object - The user as loaded by auth middleware + * @param {type} string - One of blog or showcase * @returns {FlowModel} object - The FlowModel */ -FlowModel.prototype.createShowcasePr = async function ({ body, user }) { +FlowModel.prototype.createPostPr = async function ({ body, user }, type) { /* * Is markdown set? */ @@ -283,16 +284,16 @@ FlowModel.prototype.createShowcasePr = async function ({ body, user }) { /* * Create a new feature branch for this */ - const branchName = `showcase-${body.slug}` + const branchName = `${type}-${body.slug}` const branch = await createBranch({ name: branchName }) /* * Create the file */ const file = await createFile({ - path: `markdown/org/showcase/${body.slug}/en.md`, + path: `markdown/org/${type}/${body.slug}/en.md`, body: { - message: `feat: New showcase post ${body.slug} by ${this.User.record.username}${ + message: `feat: New ${type} post ${body.slug} by ${this.User.record.username}${ body.language !== 'en' ? nonEnWarning : '' }`, content: new Buffer.from(body.markdown).toString('base64'), @@ -308,8 +309,8 @@ FlowModel.prototype.createShowcasePr = async function ({ body, user }) { * New create the pull request */ const pr = await createPullRequest({ - title: `feat: New showcase post ${body.slug} by ${this.User.record.username}`, - body: `Hey @joostdecock you should check out this awesome showcase post.${ + title: `feat: New ${type} post ${body.slug} by ${this.User.record.username}`, + body: `Paging @joostdecock to check out this proposed ${type} post.${ body.language !== 'en' ? nonEnWarning : '' }`, from: branchName, diff --git a/sites/backend/src/models/user.mjs b/sites/backend/src/models/user.mjs index 52b464efeeb..6a3f5686280 100644 --- a/sites/backend/src/models/user.mjs +++ b/sites/backend/src/models/user.mjs @@ -1281,7 +1281,7 @@ UserModel.prototype.getToken = function () { username: this.record.username, role: this.record.role, status: this.record.status, - aud: this.config.jwt.audience, + aud: `${this.config.api}/${this.config.instance}`, iss: this.config.jwt.issuer, }, this.config.jwt.secretOrKey, diff --git a/sites/backend/src/routes/flows.mjs b/sites/backend/src/routes/flows.mjs index 9ca39eeda69..03c1b28f2f7 100644 --- a/sites/backend/src/routes/flows.mjs +++ b/sites/backend/src/routes/flows.mjs @@ -39,13 +39,15 @@ export function flowsRoutes(tools) { Flow.removeImage(req, res, tools) ) - // Submit a pull request for a new showcase - app.post('/flows/pr/showcase/jwt', passport.authenticate(...jwt), (req, res) => - Flow.createShowcasePr(req, res, tools) - ) - app.post('/flows/pr/showcase/key', passport.authenticate(...bsc), (req, res) => - Flow.createShowcasePr(req, res, tools) - ) + // Submit a pull request for a new showcase or blog post + for (const type of ['blog', 'showcase']) { + app.post(`/flows/pr/${type}/jwt`, passport.authenticate(...jwt), (req, res) => + Flow.createPostPr(req, res, tools, type) + ) + app.post(`/flows/pr/${type}/key`, passport.authenticate(...bsc), (req, res) => + Flow.createPostPr(req, res, tools, type) + ) + } // Create Issue - No auth needed app.post('/issues', (req, res) => Flow.createIssue(req, res, tools))