From 79e3c2018966a9645f4bcc86edea57a8367d3837 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Sat, 5 Nov 2022 22:17:51 +0100 Subject: [PATCH] server: allow to grant tokens with more restricted privileges This also simplifies API authentication a bit by not having to fetch the App that is related to a token. The restriction of 1 token per app is also lifted. This was not a constraint in the database but it was enforced by the code and kinda wrong schema the auth_session table had. --- .../1667653936442-token-permissions.js | 26 ++++++++ .../src/models/entities/auth-session.ts | 12 ++-- .../backend/src/server/api/authenticate.ts | 22 +------ .../backend/src/server/api/common/oauth.ts | 31 +++++---- .../src/server/api/endpoints/auth/accept.ts | 66 +++++++++++-------- .../api/endpoints/auth/session/userkey.ts | 23 ++++--- packages/client/src/pages/auth.form.vue | 9 +-- packages/client/src/pages/auth.vue | 14 ++++ 8 files changed, 118 insertions(+), 85 deletions(-) create mode 100644 packages/backend/migration/1667653936442-token-permissions.js diff --git a/packages/backend/migration/1667653936442-token-permissions.js b/packages/backend/migration/1667653936442-token-permissions.js new file mode 100644 index 000000000..d6c3e0fba --- /dev/null +++ b/packages/backend/migration/1667653936442-token-permissions.js @@ -0,0 +1,26 @@ +export class tokenPermissions1667653936442 { + name = 'tokenPermissions1667653936442' + + async up(queryRunner) { + // Carry over the permissions from the app for tokens that have an associated app. + await queryRunner.query(`UPDATE "access_token" SET permission = (SELECT permission FROM "app" WHERE "app"."id" = "access_token"."appId") WHERE "appId" IS NOT NULL AND CARDINALITY("permission") = 0`); + // The permission column should now always be set explicitly, so the default is not needed any more. + await queryRunner.query(`ALTER TABLE "access_token" ALTER COLUMN "permission" DROP DEFAULT`); + // Refactor scheme to allow multiple access tokens per app. + await queryRunner.query(`ALTER TABLE "auth_session" DROP CONSTRAINT "FK_c072b729d71697f959bde66ade0"`); + await queryRunner.query(`ALTER TABLE "auth_session" RENAME COLUMN "userId" TO "accessTokenId"`); + await queryRunner.query(`ALTER TABLE "auth_session" ADD CONSTRAINT "UQ_8e001e5a101c6dca37df1a76d66" UNIQUE ("accessTokenId")`); + await queryRunner.query(`ALTER TABLE "auth_session" ADD CONSTRAINT "FK_8e001e5a101c6dca37df1a76d66" FOREIGN KEY ("accessTokenId") REFERENCES "access_token"("id") ON DELETE CASCADE ON UPDATE NO ACTION`); + } + + async down(queryRunner) { + await queryRunner.query(`ALTER TABLE "auth_session" DROP CONSTRAINT "FK_8e001e5a101c6dca37df1a76d66"`); + await queryRunner.query(`ALTER TABLE "auth_session" DROP CONSTRAINT "UQ_8e001e5a101c6dca37df1a76d66"`); + await queryRunner.query(`ALTER TABLE "access_token" ALTER COLUMN "permission" DROP DEFAULT`); + await queryRunner.query(`ALTER TABLE "auth_session" RENAME COLUMN "accessTokenId" TO "userId"`); + await queryRunner.query(`ALTER TABLE "auth_session" ADD CONSTRAINT "FK_c072b729d71697f959bde66ade0" FOREIGN KEY ("userId") REFERENCES "user"("id") ON DELETE CASCADE ON UPDATE NO ACTION`); + + await queryRunner.query(`ALTER TABLE "access_token" ALTER COLUMN "permission" SET DEFAULT '{}'::varchar[]`); + await queryRunner.query(`UPDATE "access_token" SET permission = '{}'::varchar[] WHERE "appId" IS NOT NULL`); + } +} diff --git a/packages/backend/src/models/entities/auth-session.ts b/packages/backend/src/models/entities/auth-session.ts index 51f79e475..a68ead2da 100644 --- a/packages/backend/src/models/entities/auth-session.ts +++ b/packages/backend/src/models/entities/auth-session.ts @@ -1,6 +1,6 @@ -import { Entity, PrimaryColumn, Index, Column, ManyToOne, JoinColumn } from 'typeorm'; +import { Entity, PrimaryColumn, Index, Column, ManyToOne, OneToOne, JoinColumn } from 'typeorm'; import { id } from '../id.js'; -import { User } from './user.js'; +import { AccessToken } from './access-token.js'; import { App } from './app.js'; @Entity() @@ -23,19 +23,19 @@ export class AuthSession { ...id(), nullable: true, }) - public userId: User['id'] | null; + public accessTokenId: AccessToken['id'] | null; - @ManyToOne(type => User, { + @ManyToOne(() => AccessToken, { onDelete: 'CASCADE', nullable: true, }) @JoinColumn() - public user: User | null; + public accessToken: AccessToken | null; @Column(id()) public appId: App['id']; - @ManyToOne(type => App, { + @ManyToOne(() => App, { onDelete: 'CASCADE', }) @JoinColumn() diff --git a/packages/backend/src/server/api/authenticate.ts b/packages/backend/src/server/api/authenticate.ts index f6d6a646a..25e87b75e 100644 --- a/packages/backend/src/server/api/authenticate.ts +++ b/packages/backend/src/server/api/authenticate.ts @@ -1,16 +1,9 @@ -import { CacheableLocalUser, ILocalUser } from '@/models/entities/user.js'; -import { Users, AccessTokens, Apps } from '@/models/index.js'; +import { CacheableLocalUser } from '@/models/entities/user.js'; +import { Users, AccessTokens } from '@/models/index.js'; import { AccessToken } from '@/models/entities/access-token.js'; -import { Cache } from '@/misc/cache.js'; -import { App } from '@/models/entities/app.js'; import { userByIdCache, localUserByNativeTokenCache } from '@/services/user-cache.js'; import isNativeToken from './common/is-native-token.js'; -const appCache = new Cache( - Infinity, - (id) => Apps.findOneByOrFail({ id }), -); - export class AuthenticationError extends Error { constructor(message: string) { super(message); @@ -71,15 +64,6 @@ export default async (authorization: string | null | undefined, bodyToken: strin // can't authorize remote users if (!Users.isLocalUser(user)) return [null, null]; - if (accessToken.appId) { - const app = await appCache.fetch(accessToken.appId); - - return [user, { - id: accessToken.id, - permission: app.permission, - } as AccessToken]; - } else { - return [user, accessToken]; - } + return [user, accessToken]; } }; diff --git a/packages/backend/src/server/api/common/oauth.ts b/packages/backend/src/server/api/common/oauth.ts index 731cd73fa..fcf235d08 100644 --- a/packages/backend/src/server/api/common/oauth.ts +++ b/packages/backend/src/server/api/common/oauth.ts @@ -51,11 +51,16 @@ export async function oauth(ctx: Koa.Context): void { id: client_id, secret: client_secret, }), - AuthSessions.findOneBy({ - appId: client_id, - token: code, - // only check for approved auth sessions - userId: Not(IsNull()), + AuthSessions.findOne({ + where: { + appId: client_id, + token: code, + // only check for approved auth sessions + accessTokenId: Not(IsNull()), + }, + relations: { + accessToken: true, + }, }), ]); if (app == null) { @@ -75,20 +80,14 @@ export async function oauth(ctx: Koa.Context): void { return; } - const [ token ] = await Promise.all([ - AccessTokens.findOneByOrFail({ - appId: client_id, - userId: session.userId, - }), - // session is single use - AuthSessions.delete(session.id), - ]); + // session is single use + await AuthSessions.delete(session.id), ctx.response.status = 200; ctx.response.body = { - access_token: token.token, + access_token: session.accessToken.token, token_type: 'bearer', - // FIXME: per-token permissions - scope: app.permission.join(' '), + scope: session.accessToken.permission.join(' '), }; + }; diff --git a/packages/backend/src/server/api/endpoints/auth/accept.ts b/packages/backend/src/server/api/endpoints/auth/accept.ts index 691b4a867..736c1e3f6 100644 --- a/packages/backend/src/server/api/endpoints/auth/accept.ts +++ b/packages/backend/src/server/api/endpoints/auth/accept.ts @@ -2,6 +2,7 @@ import * as crypto from 'node:crypto'; import { AuthSessions, AccessTokens, Apps } from '@/models/index.js'; import { genId } from '@/misc/gen-id.js'; import { secureRndstr } from '@/misc/secure-rndstr.js'; +import { kinds } from '@/misc/api-permissions.js'; import define from '../../define.js'; import { ApiError } from '../../error.js'; @@ -19,6 +20,17 @@ export const paramDef = { type: 'object', properties: { token: { type: 'string' }, + permission: { + description: 'The permissions which the user wishes to grant in this token. ' + + 'Permissions that the app has not registered before will be removed. ' + + 'Defaults to all permissions the app was registered with if not provided.', + type: 'array', + uniqueItems: true, + items: { + type: 'string', + enum: kinds, + }, + }, }, required: ['token'], } as const; @@ -34,37 +46,35 @@ export default define(meta, paramDef, async (ps, user) => { // Generate access token const accessToken = secureRndstr(32, true); - // Fetch exist access token - const exist = await AccessTokens.findOneBy({ + // Check for existing access token. + const app = await Apps.findOneByOrFail({ id: session.appId }); + + // Generate Hash + const sha256 = crypto.createHash('sha256'); + sha256.update(accessToken + app.secret); + const hash = sha256.digest('hex'); + + const now = new Date(); + + // Calculate the set intersection between requested permissions and + // permissions that the app registered with. If no specific permissions + // are given, grant all permissions the app registered with. + const permission = ps.permission?.filter(x => app.permission.includes(x)) ?? app.permission; + + const accessTokenId = genId(); + + // Insert access token doc + await AccessTokens.insert({ + id: accessTokenId, + createdAt: now, + lastUsedAt: now, appId: session.appId, userId: user.id, + token: accessToken, + hash, + permission, }); - if (exist == null) { - // Lookup app - const app = await Apps.findOneByOrFail({ id: session.appId }); - - // Generate Hash - const sha256 = crypto.createHash('sha256'); - sha256.update(accessToken + app.secret); - const hash = sha256.digest('hex'); - - const now = new Date(); - - // Insert access token doc - await AccessTokens.insert({ - id: genId(), - createdAt: now, - lastUsedAt: now, - appId: session.appId, - userId: user.id, - token: accessToken, - hash, - }); - } - // Update session - await AuthSessions.update(session.id, { - userId: user.id, - }); + await AuthSessions.update(session.id, { accessTokenId }); }); diff --git a/packages/backend/src/server/api/endpoints/auth/session/userkey.ts b/packages/backend/src/server/api/endpoints/auth/session/userkey.ts index 3a741db44..3e857645a 100644 --- a/packages/backend/src/server/api/endpoints/auth/session/userkey.ts +++ b/packages/backend/src/server/api/endpoints/auth/session/userkey.ts @@ -46,27 +46,26 @@ export default define(meta, paramDef, async (ps) => { if (app == null) throw new ApiError('NO_SUCH_APP'); // Fetch token - const session = await AuthSessions.findOneBy({ - token: ps.token, - appId: app.id, + const session = await AuthSessions.findOne({ + where: { + token: ps.token, + appId: app.id, + }, + relations: { + accessToken: true, + }, }); if (session == null) throw new ApiError('NO_SUCH_SESSION'); - if (session.userId == null) throw new ApiError('PENDING_SESSION'); - - // Lookup access token - const accessToken = await AccessTokens.findOneByOrFail({ - appId: app.id, - userId: session.userId, - }); + if (session.accessTokenId == null) throw new ApiError('PENDING_SESSION'); // Delete session AuthSessions.delete(session.id); return { - accessToken: accessToken.token, - user: await Users.pack(session.userId, null, { + accessToken: session.accessToken.token, + user: await Users.pack(session.accessToken.userId, null, { detail: true, }), }; diff --git a/packages/client/src/pages/auth.form.vue b/packages/client/src/pages/auth.form.vue index 035ffac9a..92226eff1 100644 --- a/packages/client/src/pages/auth.form.vue +++ b/packages/client/src/pages/auth.form.vue @@ -7,8 +7,8 @@

{{ i18n.ts._auth.permissionAsk }}

-
    -
  • {{ i18n.t(`_permissions.${p}`) }}
    • +
      +
    • {{ i18n.t(`_permissions.${p}`) }}