From a13e956af00b0acdafcc22d4067b63459bc03bc4 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Sun, 16 Oct 2022 00:46:13 +0200 Subject: [PATCH 01/13] make authorization token granting OAuth 2.0 compatible This is basically a shim on top of the existing API. Instead of the 3rd party, the web UI generates the authorization session. The data that the API returns is slightly adjusted so that only one API call is necessary instead of two. --- locales/en-US.yml | 2 + .../api/endpoints/auth/session/generate.ts | 40 ++++-- packages/client/src/pages/auth.vue | 117 ++++++++++++++---- packages/client/src/router.ts | 3 + 4 files changed, 134 insertions(+), 28 deletions(-) diff --git a/locales/en-US.yml b/locales/en-US.yml index a6a8a2110..10e0e4c65 100644 --- a/locales/en-US.yml +++ b/locales/en-US.yml @@ -828,6 +828,8 @@ setTag: "Set tag" addTag: "Add tag" removeTag: "Remove tag" externalCssSnippets: "Some CSS snippets for your inspiration (not managed by FoundKey)" +oauthErrorGoBack: "An error happened while trying to authenticate a 3rd party app.\ + \ Please go back and try again." _emailUnavailable: used: "This email address is already being used" format: "The format of this email address is invalid" diff --git a/packages/backend/src/server/api/endpoints/auth/session/generate.ts b/packages/backend/src/server/api/endpoints/auth/session/generate.ts index eeb51abc6..e1b498a49 100644 --- a/packages/backend/src/server/api/endpoints/auth/session/generate.ts +++ b/packages/backend/src/server/api/endpoints/auth/session/generate.ts @@ -23,6 +23,19 @@ export const meta = { optional: false, nullable: false, format: 'url', }, + // stuff that auth/session/show would respond with + id: { + type: 'string', + description: 'The ID of the authentication session. Same as returned by `auth/session/show`.', + optional: false, nullable: false, + format: 'id', + }, + app: { + type: 'object', + description: 'The App requesting permissions. Same as returned by `auth/session/show`.', + optional: false, nullable: false, + ref: 'App', + }, }, }, @@ -30,17 +43,27 @@ export const meta = { } as const; export const paramDef = { - type: 'object', - properties: { - appSecret: { type: 'string' }, - }, - required: ['appSecret'], + oneOf: [{ + type: 'object', + properties: { + clientId: { type: 'string' }, + }, + required: ['clientId'] + }, { + type: 'object', + properties: { + appSecret: { type: 'string' }, + }, + required: ['appSecret'], + }], } as const; // eslint-disable-next-line import/no-default-export export default define(meta, paramDef, async (ps) => { // Lookup app - const app = await Apps.findOneBy({ + const app = await Apps.findOneBy(ps.clientId ? { + id: ps.clientId, + } : { secret: ps.appSecret, }); @@ -50,10 +73,11 @@ export default define(meta, paramDef, async (ps) => { // Generate token const token = uuid(); + const id = genId(); // Create session token document const doc = await AuthSessions.insert({ - id: genId(), + id, createdAt: new Date(), appId: app.id, token, @@ -62,5 +86,7 @@ export default define(meta, paramDef, async (ps) => { return { token: doc.token, url: `${config.authUrl}/${doc.token}`, + id, + app: await Apps.pack(app), }; }); diff --git a/packages/client/src/pages/auth.vue b/packages/client/src/pages/auth.vue index 3b3ff7bac..ac9a3e051 100644 --- a/packages/client/src/pages/auth.vue +++ b/packages/client/src/pages/auth.vue @@ -6,7 +6,7 @@ ref="form" class="form" :session="session" - @denied="state = 'denied'" + @denied="denied" @accepted="accepted" />
@@ -20,6 +20,9 @@

{{ i18n.ts.somethingHappened }}

+
+

{{ i18n.ts.oauthErrorGoBack }}

+
@@ -37,43 +40,115 @@ import { i18n } from '@/i18n'; import { query, appendQuery } from '@/scripts/url'; const props = defineProps<{ - token: string; + token?: string; }>(); -let state: 'fetching' | 'waiting' | 'denied' | 'accepted' | 'fetch-session-error' = $ref('fetching'); +let state: 'fetching' | 'waiting' | 'denied' | 'accepted' | 'fetch-session-error' | 'oauth-error' = $ref('fetching'); let session = $ref(null); -onMounted(() => { +// if this is an OAuth request, will contain the respective parameters +let oauth: { state: string | null, callback: string } | null = null; + +onMounted(async () => { if (!$i) return; - // Fetch session - os.api('auth/session/show', { - token: props.token, - }).then(fetchedSession => { - session = fetchedSession; + // detect whether this is actual OAuth or "legacy" auth + const params = new URLSearchParams(location.search); + if (params.get('response_type') === 'code') { + // OAuth request detected! - // 既に連携していた場合 - if (session.app.isAuthorized) { - os.api('auth/accept', { - token: session.token, - }).then(() => { - this.accepted(); - }); - } else { - state = 'waiting'; + // as a kind of hack, we first have to start the session for the OAuth client + const clientId = params.get('client_id'); + if (!clientId) { + state = 'fetch-session-error'; + return; } - }).catch(() => { + + session = await os.api('auth/session/generate', { + clientId, + }).catch(e => { + const response = { + error: 'server_error', + ...(oauth.state ? { state: oauth.state } : {}), + }; + // try to determine the cause of the error + if (e.code === 'NO_SUCH_APP') { + response.error = 'invalid_request'; + response.error_description = 'unknown client_id'; + } else if (e.message) { + response.error_description = e.message; + } + + if (params.has('redirect_uri')) { + location.href = appendQuery(params.get('redirect_uri'), query(response)); + } else { + state = 'oauth-error'; + } + }); + + oauth = { + state: params.get('state'), + callback: params.get('redirect_uri') ?? session.app.callbackUrl, + }; + } else if (!props.token) { state = 'fetch-session-error'; - }); + } else { + session = await os.api('auth/session/show', { + token: props.token, + }).catch(() => { + state = 'fetch-session-error'; + }); + } + + // abort if an error occurred + if (['fetch-session-error', 'oauth-error'].includes(state)) return; + + // check whether the user already authorized the app earlier + if (session.app.isAuthorized) { + // already authorized, move on through! + os.api('auth/accept', { + token: session.token, + }).then(() => { + accepted(); + }); + } else { + // user still has to give consent + state = 'waiting'; + } }); function accepted(): void { state = 'accepted'; - if (session.app.callbackUrl) { + if (oauth) { + // redirect with authorization token + const params = { + code: session.token, + ...(oauth.state ? { state: oauth.state } : {}), + }; + + location.href = appendQuery(oauth.callback, query(params)); + } else if (session.app.callbackUrl) { + // do whatever the legacy auth did location.href = appendQuery(session.app.callbackUrl, query({ token: session.token })); } } +function denied(): void { + state = 'denied'; + if (oauth) { + // redirect with error code + const params = { + error: 'access_denied', + error_description: 'The user denied permission.', + ...(oauth.state ? { state: oauth.state } : {}), + }; + + location.href = appendQuery(oauth.callback, query(params)); + } else { + // legacy auth didn't do anything in this case... + } +} + function onLogin(res): void { login(res.i); } diff --git a/packages/client/src/router.ts b/packages/client/src/router.ts index 6d3011688..791076e71 100644 --- a/packages/client/src/router.ts +++ b/packages/client/src/router.ts @@ -94,6 +94,9 @@ export const routes = [{ }, { path: '/preview', component: page(() => import('./pages/preview.vue')), +}, { + path: '/auth', + component: page(() => import('./pages/auth.vue')), }, { path: '/auth/:token', component: page(() => import('./pages/auth.vue')), From 7db7fdd9e2d7114fa8c99aa0bfe9d88ac043536a Mon Sep 17 00:00:00 2001 From: Johann150 Date: Sun, 16 Oct 2022 01:11:47 +0200 Subject: [PATCH 02/13] add API route for OAuth access token retrieval --- .../backend/src/server/api/common/oauth.ts | 94 +++++++++++++++++++ .../api/endpoints/auth/session/oauth.ts | 5 + packages/backend/src/server/api/index.ts | 4 + 3 files changed, 103 insertions(+) create mode 100644 packages/backend/src/server/api/common/oauth.ts create mode 100644 packages/backend/src/server/api/endpoints/auth/session/oauth.ts diff --git a/packages/backend/src/server/api/common/oauth.ts b/packages/backend/src/server/api/common/oauth.ts new file mode 100644 index 000000000..731cd73fa --- /dev/null +++ b/packages/backend/src/server/api/common/oauth.ts @@ -0,0 +1,94 @@ +import Koa from 'koa'; +import { IsNull, Not } from 'typeorm'; +import { Apps, AuthSessions, AccessTokens } from '@/models/index.js'; +import config from '@/config/index.js'; + +export async function oauth(ctx: Koa.Context): void { + const { + grant_type, + code, + // TODO: check redirect_uri + // since this is also not checked in the legacy app authentication + // it seems pointless to check it here, and it is also not stored. + redirect_uri, + } = ctx.request.body; + + // check if any of the parameters are null or empty string + if ([grant_type, code].some(x => !x)) { + ctx.response.status = 400; + ctx.response.body = { + error: 'invalid_request', + }; + return; + } + + if (grant_type !== 'authorization_code') { + ctx.response.status = 400; + ctx.response.body = { + error: 'unsupported_grant_type', + error_description: 'only authorization_code grants are supported', + }; + return; + } + + const authHeader = ctx.headers.authorization; + if (!authHeader?.toLowerCase().startsWith('basic ')) { + ctx.response.status = 401; + ctx.response.set('WWW-Authenticate', 'Basic'); + ctx.response.body = { + error: 'invalid_client', + error_description: 'HTTP Basic Authentication required', + }; + return; + } + + const [client_id, client_secret] = new Buffer(authHeader.slice(6), 'base64') + .toString('ascii') + .split(':', 2); + + const [app, session] = await Promise.all([ + Apps.findOneBy({ + id: client_id, + secret: client_secret, + }), + AuthSessions.findOneBy({ + appId: client_id, + token: code, + // only check for approved auth sessions + userId: Not(IsNull()), + }), + ]); + if (app == null) { + ctx.response.status = 401; + ctx.response.set('WWW-Authenticate', 'Basic'); + ctx.response.body = { + error: 'invalid_client', + error_description: 'authentication failed', + }; + return; + } + if (session == null) { + ctx.response.status = 400; + ctx.response.body = { + error: 'invalid_grant', + }; + return; + } + + const [ token ] = await Promise.all([ + AccessTokens.findOneByOrFail({ + appId: client_id, + userId: session.userId, + }), + // session is single use + AuthSessions.delete(session.id), + ]); + + ctx.response.status = 200; + ctx.response.body = { + access_token: token.token, + token_type: 'bearer', + // FIXME: per-token permissions + scope: app.permission.join(' '), + }; +}; diff --git a/packages/backend/src/server/api/endpoints/auth/session/oauth.ts b/packages/backend/src/server/api/endpoints/auth/session/oauth.ts new file mode 100644 index 000000000..d6aa6caab --- /dev/null +++ b/packages/backend/src/server/api/endpoints/auth/session/oauth.ts @@ -0,0 +1,5 @@ +/* +This route is already in use, but the functionality is provided +by '@/server/api/common/oauth.ts'. The route is not here because +that route requires more deep level access to HTTP data. +*/ diff --git a/packages/backend/src/server/api/index.ts b/packages/backend/src/server/api/index.ts index 140649dcc..456f5ff11 100644 --- a/packages/backend/src/server/api/index.ts +++ b/packages/backend/src/server/api/index.ts @@ -15,6 +15,7 @@ import { handler } from './api-handler.js'; import signup from './private/signup.js'; import signin from './private/signin.js'; import signupPending from './private/signup-pending.js'; +import { oauth } from './common/oauth.js'; import discord from './service/discord.js'; import github from './service/github.js'; import twitter from './service/twitter.js'; @@ -74,6 +75,9 @@ for (const endpoint of endpoints) { } } +// the OAuth endpoint does some shenanigans and can not use the normal API handler +router.post('/auth/session/oauth', oauth); + router.post('/signup', signup); router.post('/signin', signin); router.post('/signup-pending', signupPending); From 2b19b341962f0d7122f77646768127c16f47698c Mon Sep 17 00:00:00 2001 From: Johann150 Date: Sat, 15 Oct 2022 16:14:16 +0200 Subject: [PATCH 03/13] update OpenAPI docs to OAuth --- packages/backend/src/misc/api-permissions.ts | 71 ++++++++++--------- .../src/server/api/openapi/gen-spec.ts | 26 +++++-- 2 files changed, 56 insertions(+), 41 deletions(-) diff --git a/packages/backend/src/misc/api-permissions.ts b/packages/backend/src/misc/api-permissions.ts index 160cdf9fd..c086286c9 100644 --- a/packages/backend/src/misc/api-permissions.ts +++ b/packages/backend/src/misc/api-permissions.ts @@ -1,35 +1,38 @@ -export const kinds = [ - 'read:account', - 'write:account', - 'read:blocks', - 'write:blocks', - 'read:drive', - 'write:drive', - 'read:favorites', - 'write:favorites', - 'read:following', - 'write:following', - 'read:messaging', - 'write:messaging', - 'read:mutes', - 'write:mutes', - 'write:notes', - 'read:notifications', - 'write:notifications', - 'read:reactions', - 'write:reactions', - 'write:votes', - 'read:pages', - 'write:pages', - 'write:page-likes', - 'read:page-likes', - 'read:user-groups', - 'write:user-groups', - 'read:channels', - 'write:channels', - 'read:gallery', - 'write:gallery', - 'read:gallery-likes', - 'write:gallery-likes', -]; // IF YOU ADD KINDS(PERMISSIONS), YOU MUST ADD TRANSLATIONS (under _permissions). + +// short English descriptions used for the documentation +export const descriptions = { + 'read:account': 'Read the accounts data.', + 'write:account': 'Write the accounts data.', + 'read:blocks': 'Read which users are blocked.', + 'write:blocks': 'Create, change and delete blocks.', + 'read:drive': 'List files and folders in the drive.', + 'write:drive': 'Create, change and delete files from the drive.', + 'read:favourites': 'List favourited notes.', + 'write:favourites': 'Favourite or unfavourite notes.', + 'read:following': 'Read who the user is following.', + 'write:following': 'Follow or unfollow other users.', + 'read:messaging': 'Read chat messages and history.', + 'write:messaging': 'Create and delete chat messages.', + 'read:mutes': 'List users which are muted or whose renotes are muted.', + 'write:mutes': 'Create or delete (renote) mutes.', + 'write:notes': 'Create or delete notes.', + 'read:notifications': 'Read notifications.', + 'write:notifications': 'Mark notifications as read or create notifications.', + 'write:reactions': 'Create or delete reactions.', + 'write:votes': 'Vote in polls.', + 'read:pages': 'List and read pages.', + 'write:pages': 'Create, modify and delete pages.', + 'read:page-likes': 'List page likes.', + 'write:page-likes': 'Like or unlike pages.', + 'read:user-groups': 'List joined, owned and invited to groups.', + 'write:user-groups': 'Create, modify, delete, transfer, join, or leave groups. Invite or ban others from groups. Accept or reject group invitations.', + 'read:channels': 'List followed and owned channels.', + 'write:channels': 'Create, modify, follow or unfollow channels.', + 'read:gallery': 'Read gallery posts.', + 'write:gallery': 'Create, modify or delete gallery posts.', + 'read:gallery-likes': 'List which gallery posts are liked.', + 'write:gallery-likes': 'Like or unlike gallery posts.', +}; + +export const kinds = Object.keys(descriptions); diff --git a/packages/backend/src/server/api/openapi/gen-spec.ts b/packages/backend/src/server/api/openapi/gen-spec.ts index f9795884d..b79558456 100644 --- a/packages/backend/src/server/api/openapi/gen-spec.ts +++ b/packages/backend/src/server/api/openapi/gen-spec.ts @@ -3,6 +3,7 @@ import { errors as errorDefinitions } from '../error.js'; import endpoints from '../endpoints.js'; import { schemas, convertSchemaToOpenApiSchema } from './schemas.js'; import { httpCodes } from './http-codes.js'; +import { descriptions as scopes } from '@/misc/api-permissions.js'; export function genOpenapiSpec() { const spec = { @@ -34,10 +35,15 @@ export function genOpenapiSpec() { in: 'body', name: 'i', }, - // TODO: change this to oauth2 when the remaining oauth stuff is set up - Bearer: { - type: 'http', - scheme: 'bearer', + OAuth: { + type: 'oauth2', + flows: { + authorizationCode: { + authorizationUrl: `${config.url}/auth`, + tokenUrl: `${config.apiUrl}/auth/session/oauth`, + scopes, + }, + }, }, }, }, @@ -137,10 +143,16 @@ export function genOpenapiSpec() { { ApiKeyAuth: [], }, - { - Bearer: [], - }, ]; + if (endpoint.meta.kind) { + security.push({ + OAuth: [endpoint.meta.kind], + }); + } else { + security.push({ + OAuth: [], + }); + } if (!endpoint.meta.requireCredential) { // add this to make authentication optional security.push({}); From 418c88bb8fb48f54dfe14f884143f9956f42be55 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Sun, 16 Oct 2022 06:24:55 +0200 Subject: [PATCH 04/13] expire AuthSessions after 15 min --- .../backend/src/queue/processors/system/check-expired.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/queue/processors/system/check-expired.ts b/packages/backend/src/queue/processors/system/check-expired.ts index 5608dc43c..fe7012b1d 100644 --- a/packages/backend/src/queue/processors/system/check-expired.ts +++ b/packages/backend/src/queue/processors/system/check-expired.ts @@ -1,6 +1,6 @@ import Bull from 'bull'; import { In, LessThan } from 'typeorm'; -import { AttestationChallenges, Mutings, PasswordResetRequests, Signins } from '@/models/index.js'; +import { AttestationChallenges, AuthSessions, Mutings, PasswordResetRequests, Signins } from '@/models/index.js'; import { publishUserEvent } from '@/services/stream.js'; import { MINUTE, DAY } from '@/const.js'; import { queueLogger } from '@/queue/logger.js'; @@ -40,7 +40,11 @@ export async function checkExpired(job: Bull.Job>, done: createdAt: LessThan(new Date(new Date().getTime() - 30 * MINUTE)), }); - logger.succ('Deleted expired mutes, signins and attestation challenges.'); + await AuthSessions.delete({ + createdAt: LessThan(new Date(new Date().getTime() - 15 * MINUTE)), + }); + + logger.succ('Deleted expired data.'); done(); } From c65fdebe2615068a02c6e8c5653f17d3fec4edfd Mon Sep 17 00:00:00 2001 From: Johann150 Date: Sun, 16 Oct 2022 20:59:21 +0200 Subject: [PATCH 05/13] server: add missing auth/deny endpoint This endpoint is hinted at in the client, but is not actually defined in the backend. This commit defines it. --- packages/backend/src/server/api/endpoints.ts | 2 + .../src/server/api/endpoints/auth/deny.ts | 38 +++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 packages/backend/src/server/api/endpoints/auth/deny.ts diff --git a/packages/backend/src/server/api/endpoints.ts b/packages/backend/src/server/api/endpoints.ts index 3237935d5..c85bb6632 100644 --- a/packages/backend/src/server/api/endpoints.ts +++ b/packages/backend/src/server/api/endpoints.ts @@ -67,6 +67,7 @@ import * as ep___ap_show from './endpoints/ap/show.js'; import * as ep___app_create from './endpoints/app/create.js'; import * as ep___app_show from './endpoints/app/show.js'; import * as ep___auth_accept from './endpoints/auth/accept.js'; +import * as ep___auth_deny from './endpoints/auth/deny.js'; import * as ep___auth_session_generate from './endpoints/auth/session/generate.js'; import * as ep___auth_session_show from './endpoints/auth/session/show.js'; import * as ep___auth_session_userkey from './endpoints/auth/session/userkey.js'; @@ -375,6 +376,7 @@ const eps = [ ['app/create', ep___app_create], ['app/show', ep___app_show], ['auth/accept', ep___auth_accept], + ['auth/deny', ep___auth_deny], ['auth/session/generate', ep___auth_session_generate], ['auth/session/show', ep___auth_session_show], ['auth/session/userkey', ep___auth_session_userkey], diff --git a/packages/backend/src/server/api/endpoints/auth/deny.ts b/packages/backend/src/server/api/endpoints/auth/deny.ts new file mode 100644 index 000000000..ca1a585c7 --- /dev/null +++ b/packages/backend/src/server/api/endpoints/auth/deny.ts @@ -0,0 +1,38 @@ +import { AuthSessions } from '@/models/index.js'; +import define from '../../define.js'; +import { ApiError } from '../../error.js'; + +export const meta = { + tags: ['auth'], + + requireCredential: true, + + secure: true, + + errors: { + noSuchSession: { + message: 'No such session.', + code: 'NO_SUCH_SESSION', + id: '9c72d8de-391a-43c1-9d06-08d29efde8df', + }, + }, +} as const; + +export const paramDef = { + type: 'object', + properties: { + token: { type: 'string' }, + }, + required: ['token'], +} as const; + +// eslint-disable-next-line import/no-default-export +export default define(meta, paramDef, async (ps, user) => { + const result = await AuthSessions.delete({ + token: ps.token, + }); + + if (result.affected == 0) { + throw new ApiError(meta.errors.noSuchSession); + } +}); From c5568cfdf3f9d414ec8b387cba95e5a313230792 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Sun, 16 Oct 2022 21:09:30 +0200 Subject: [PATCH 06/13] client: fix auth page layout This also includes better rendering when no permissions are requested. Also removed the app's id from the page as it makes no sense to show this to a user. Changelog: Fixed --- locales/en-US.yml | 2 + packages/client/src/pages/auth.form.vue | 6 ++- packages/client/src/pages/auth.vue | 68 ++++++++++++++----------- 3 files changed, 45 insertions(+), 31 deletions(-) diff --git a/locales/en-US.yml b/locales/en-US.yml index 10e0e4c65..2051d16c2 100644 --- a/locales/en-US.yml +++ b/locales/en-US.yml @@ -830,6 +830,8 @@ removeTag: "Remove tag" externalCssSnippets: "Some CSS snippets for your inspiration (not managed by FoundKey)" oauthErrorGoBack: "An error happened while trying to authenticate a 3rd party app.\ \ Please go back and try again." +appAuthorization: "App authorization" +noPermissionsRequested: "(No permissions requested.)" _emailUnavailable: used: "This email address is already being used" format: "The format of this email address is invalid" diff --git a/packages/client/src/pages/auth.form.vue b/packages/client/src/pages/auth.form.vue index d4af3b6db..035ffac9a 100644 --- a/packages/client/src/pages/auth.form.vue +++ b/packages/client/src/pages/auth.form.vue @@ -3,14 +3,16 @@
{{ i18n.t('_auth.shareAccess', { name: app.name }) }}

{{ app.name }}

-

{{ app.id }}

{{ app.description }}

{{ i18n.ts._auth.permissionAsk }}

-
    +
    • {{ i18n.t(`_permissions.${p}`) }}
    +
{{ i18n.ts.cancel }} diff --git a/packages/client/src/pages/auth.vue b/packages/client/src/pages/auth.vue index ac9a3e051..d2c57320f 100644 --- a/packages/client/src/pages/auth.vue +++ b/packages/client/src/pages/auth.vue @@ -1,32 +1,37 @@ From 2f2e6a58a409129b967f361fe04b1368ed72aac7 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Wed, 19 Oct 2022 09:04:45 +0200 Subject: [PATCH 07/13] docs: read scope descriptions from locale strings --- locales/en-US.yml | 63 ++++++++-------- packages/backend/src/misc/api-permissions.ts | 71 +++++++++---------- .../src/server/api/openapi/gen-spec.ts | 10 ++- 3 files changed, 73 insertions(+), 71 deletions(-) diff --git a/locales/en-US.yml b/locales/en-US.yml index 2051d16c2..109e03b94 100644 --- a/locales/en-US.yml +++ b/locales/en-US.yml @@ -1082,38 +1082,37 @@ _2fa: \ authentication via hardware security keys that support FIDO2 to further secure\ \ your account." _permissions: - "read:account": "View your account information" - "write:account": "Edit your account information" - "read:blocks": "View your list of blocked users" - "write:blocks": "Edit your list of blocked users" - "read:drive": "Access your Drive files and folders" - "write:drive": "Edit or delete your Drive files and folders" - "read:favorites": "View your list of favorites" - "write:favorites": "Edit your list of favorites" - "read:following": "View information on who you follow" - "write:following": "Follow or unfollow other accounts" - "read:messaging": "View your chats" - "write:messaging": "Compose or delete chat messages" - "read:mutes": "View your list of muted users" - "write:mutes": "Edit your list of muted users" - "write:notes": "Compose or delete notes" - "read:notifications": "View your notifications" - "write:notifications": "Manage your notifications" - "read:reactions": "View your reactions" - "write:reactions": "Edit your reactions" - "write:votes": "Vote on a poll" - "read:pages": "View your pages" - "write:pages": "Edit or delete your pages" - "read:page-likes": "View your likes on pages" - "write:page-likes": "Edit your likes on pages" - "read:user-groups": "View your user groups" - "write:user-groups": "Edit or delete your user groups" - "read:channels": "View your channels" - "write:channels": "Edit your channels" - "read:gallery": "View your gallery" - "write:gallery": "Edit your gallery" - "read:gallery-likes": "View your list of liked gallery posts" - "write:gallery-likes": "Edit your list of liked gallery posts" + "read:account": "Read account information" + "write:account": "Edit account information" + "read:blocks": "Read which users are blocked" + "write:blocks": "Block and unblock users" + "read:drive": "List files and folders in the drive" + "write:drive": "Create, change and delete files in the drive" + "read:favorites": "List favourited notes" + "write:favorites": "Favorite and unfavorite notes" + "read:following": "List followed and following users" + "write:following": "Follow and unfollow other users" + "read:messaging": "View chat messages and history" + "write:messaging": "Create and delete chat messages" + "read:mutes": "List users which are muted or whose renotes are muted" + "write:mutes": "Mute and unmute users or their renotes" + "write:notes": "Create and delete notes" + "read:notifications": "Read notifications" + "write:notifications": "Mark notifications as read and create custom notifications" + "write:reactions": "Create and delete reactions" + "write:votes": "Vote in polls" + "read:pages": "List and read pages" + "write:pages": "Create, change and delete pages" + "read:page-likes": "List and read page likes" + "write:page-likes": "Like and unlike pages" + "read:user-groups": "List and view joined, owned and invited to groups" + "write:user-groups": "Create, modify, delete, transfer, join and leave groups. Invite and ban others from groups. Accept and reject group invitations." + "read:channels": "List and read followed and joined channels" + "write:channels": "Create, modify, follow and unfollow channels" + "read:gallery": "List and read gallery posts" + "write:gallery": "Create, modify and delete gallery posts" + "read:gallery-likes": "List and read gallery post likes" + "write:gallery-likes": "Like and unlike gallery posts" _auth: shareAccess: "Would you like to authorize \"{name}\" to access this account?" shareAccessAsk: "Are you sure you want to authorize this application to access your\ diff --git a/packages/backend/src/misc/api-permissions.ts b/packages/backend/src/misc/api-permissions.ts index c086286c9..160cdf9fd 100644 --- a/packages/backend/src/misc/api-permissions.ts +++ b/packages/backend/src/misc/api-permissions.ts @@ -1,38 +1,35 @@ +export const kinds = [ + 'read:account', + 'write:account', + 'read:blocks', + 'write:blocks', + 'read:drive', + 'write:drive', + 'read:favorites', + 'write:favorites', + 'read:following', + 'write:following', + 'read:messaging', + 'write:messaging', + 'read:mutes', + 'write:mutes', + 'write:notes', + 'read:notifications', + 'write:notifications', + 'read:reactions', + 'write:reactions', + 'write:votes', + 'read:pages', + 'write:pages', + 'write:page-likes', + 'read:page-likes', + 'read:user-groups', + 'write:user-groups', + 'read:channels', + 'write:channels', + 'read:gallery', + 'write:gallery', + 'read:gallery-likes', + 'write:gallery-likes', +]; // IF YOU ADD KINDS(PERMISSIONS), YOU MUST ADD TRANSLATIONS (under _permissions). - -// short English descriptions used for the documentation -export const descriptions = { - 'read:account': 'Read the accounts data.', - 'write:account': 'Write the accounts data.', - 'read:blocks': 'Read which users are blocked.', - 'write:blocks': 'Create, change and delete blocks.', - 'read:drive': 'List files and folders in the drive.', - 'write:drive': 'Create, change and delete files from the drive.', - 'read:favourites': 'List favourited notes.', - 'write:favourites': 'Favourite or unfavourite notes.', - 'read:following': 'Read who the user is following.', - 'write:following': 'Follow or unfollow other users.', - 'read:messaging': 'Read chat messages and history.', - 'write:messaging': 'Create and delete chat messages.', - 'read:mutes': 'List users which are muted or whose renotes are muted.', - 'write:mutes': 'Create or delete (renote) mutes.', - 'write:notes': 'Create or delete notes.', - 'read:notifications': 'Read notifications.', - 'write:notifications': 'Mark notifications as read or create notifications.', - 'write:reactions': 'Create or delete reactions.', - 'write:votes': 'Vote in polls.', - 'read:pages': 'List and read pages.', - 'write:pages': 'Create, modify and delete pages.', - 'read:page-likes': 'List page likes.', - 'write:page-likes': 'Like or unlike pages.', - 'read:user-groups': 'List joined, owned and invited to groups.', - 'write:user-groups': 'Create, modify, delete, transfer, join, or leave groups. Invite or ban others from groups. Accept or reject group invitations.', - 'read:channels': 'List followed and owned channels.', - 'write:channels': 'Create, modify, follow or unfollow channels.', - 'read:gallery': 'Read gallery posts.', - 'write:gallery': 'Create, modify or delete gallery posts.', - 'read:gallery-likes': 'List which gallery posts are liked.', - 'write:gallery-likes': 'Like or unlike gallery posts.', -}; - -export const kinds = Object.keys(descriptions); diff --git a/packages/backend/src/server/api/openapi/gen-spec.ts b/packages/backend/src/server/api/openapi/gen-spec.ts index b79558456..a7f5d5cc9 100644 --- a/packages/backend/src/server/api/openapi/gen-spec.ts +++ b/packages/backend/src/server/api/openapi/gen-spec.ts @@ -3,7 +3,10 @@ import { errors as errorDefinitions } from '../error.js'; import endpoints from '../endpoints.js'; import { schemas, convertSchemaToOpenApiSchema } from './schemas.js'; import { httpCodes } from './http-codes.js'; -import { descriptions as scopes } from '@/misc/api-permissions.js'; +import { kinds } from '@/misc/api-permissions.js'; +import { I18n } from '@/misc/i18n.js'; + +const i18n = new I18n('en-US'); export function genOpenapiSpec() { const spec = { @@ -41,7 +44,10 @@ export function genOpenapiSpec() { authorizationCode: { authorizationUrl: `${config.url}/auth`, tokenUrl: `${config.apiUrl}/auth/session/oauth`, - scopes, + scopes: kinds.reduce((acc, kind) => { + acc[kind] = i18n.ts['_permissions'][kind]; + return acc; + }, {}), }, }, }, From 79e3c2018966a9645f4bcc86edea57a8367d3837 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Sat, 5 Nov 2022 22:17:51 +0100 Subject: [PATCH 08/13] 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}`) }}