From 91bdab1a9dc2edbbf1905e3e33bce00f44152909 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Mon, 18 Jul 2022 17:41:08 +0200 Subject: [PATCH 1/5] add OAuth 2.0 Bearer Token authentication --- .../backend/src/server/api/api-handler.ts | 3 ++- .../backend/src/server/api/authenticate.ts | 21 +++++++++++++++++-- packages/backend/src/server/api/streaming.ts | 2 +- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/server/api/api-handler.ts b/packages/backend/src/server/api/api-handler.ts index c22c868c8..9146b2180 100644 --- a/packages/backend/src/server/api/api-handler.ts +++ b/packages/backend/src/server/api/api-handler.ts @@ -34,7 +34,8 @@ export default (endpoint: IEndpoint, ctx: Koa.Context) => new Promise((res }; // Authentication - authenticate(body['i']).then(([user, app]) => { + // for GET requests, do not even pass on the body parameter as it is considered unsafe + authenticate(ctx.headers.authorization, ctx.method === 'GET' ? null : body['i']).then(([user, app]) => { // API invoking call(endpoint.name, user, app, body, ctx).then((res: any) => { if (ctx.method === 'GET' && endpoint.meta.cacheSec && !body['i'] && !user) { diff --git a/packages/backend/src/server/api/authenticate.ts b/packages/backend/src/server/api/authenticate.ts index 65ccfcf55..192f20ebc 100644 --- a/packages/backend/src/server/api/authenticate.ts +++ b/packages/backend/src/server/api/authenticate.ts @@ -15,8 +15,25 @@ export class AuthenticationError extends Error { } } -export default async (token: string | null): Promise<[CacheableLocalUser | null | undefined, AccessToken | null | undefined]> => { - if (token == null) { +export default async (authorization: string | null | undefined, bodyToken: string | null): Promise<[CacheableLocalUser | null | undefined, AccessToken | null | undefined]> => { + let token: string | null = null; + + // check if there is an authorization header set + if (authorization != null) { + if (bodyToken != null) { + throw new AuthenticationError('using multiple authorization schemes'); + } + + // check if OAuth 2.0 Bearer tokens are being used + // Authorization schemes are case insensitive + if (authorization.substring(0, 7).toLowerCase() === 'bearer ') { + token = authorization.substring(7); + } else { + throw new AuthenticationError('unsupported authentication scheme'); + } + } else if (bodyToken != null) { + token = bodyToken; + } else { return [null, null]; } diff --git a/packages/backend/src/server/api/streaming.ts b/packages/backend/src/server/api/streaming.ts index f8e42d27f..35d0c0fc0 100644 --- a/packages/backend/src/server/api/streaming.ts +++ b/packages/backend/src/server/api/streaming.ts @@ -20,7 +20,7 @@ export const initializeStreamingServer = (server: http.Server) => { // TODO: トークンが間違ってるなどしてauthenticateに失敗したら // コネクション切断するなりエラーメッセージ返すなりする // (現状はエラーがキャッチされておらずサーバーのログに流れて邪魔なので) - const [user, app] = await authenticate(q.i as string); + const [user, app] = await authenticate(request.httpRequest.headers.authorization, q.i); if (user?.isSuspended) { request.reject(400); -- 2.34.1 From edac21e8f73fccd6da204b8bca08318d8762a9ee Mon Sep 17 00:00:00 2001 From: Johann150 Date: Mon, 18 Jul 2022 17:42:14 +0200 Subject: [PATCH 2/5] improve authentication errors --- packages/backend/src/server/api/api-handler.ts | 10 +++++++--- packages/backend/src/server/api/authenticate.ts | 4 ++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/backend/src/server/api/api-handler.ts b/packages/backend/src/server/api/api-handler.ts index 9146b2180..956096367 100644 --- a/packages/backend/src/server/api/api-handler.ts +++ b/packages/backend/src/server/api/api-handler.ts @@ -47,11 +47,15 @@ export default (endpoint: IEndpoint, ctx: Koa.Context) => new Promise((res }); }).catch(e => { if (e instanceof AuthenticationError) { - reply(403, new ApiError({ - message: 'Authentication failed. Please ensure your token is correct.', + ctx.response.status = 403; + ctx.response.set('WWW-Authenticate', 'Bearer'); + ctx.response.body = { + message: 'Authentication failed: ' + e.message, code: 'AUTHENTICATION_FAILED', id: 'b0a7f5f8-dc2f-4171-b91f-de88ad238e14', - })); + kind: 'client', + }; + res(); } else { reply(500, new ApiError()); } diff --git a/packages/backend/src/server/api/authenticate.ts b/packages/backend/src/server/api/authenticate.ts index 192f20ebc..39be06c29 100644 --- a/packages/backend/src/server/api/authenticate.ts +++ b/packages/backend/src/server/api/authenticate.ts @@ -42,7 +42,7 @@ export default async (authorization: string | null | undefined, bodyToken: strin () => Users.findOneBy({ token }) as Promise); if (user == null) { - throw new AuthenticationError('user not found'); + throw new AuthenticationError('unknown token'); } return [user, null]; @@ -56,7 +56,7 @@ export default async (authorization: string | null | undefined, bodyToken: strin }); if (accessToken == null) { - throw new AuthenticationError('invalid signature'); + throw new AuthenticationError('unknown token'); } AccessTokens.update(accessToken.id, { -- 2.34.1 From 660f6dba3062873c944690c7a1c0fecd62a3a55b Mon Sep 17 00:00:00 2001 From: Johann150 Date: Mon, 18 Jul 2022 18:37:41 +0200 Subject: [PATCH 3/5] update openapi spec generator --- .../src/server/api/openapi/gen-spec.ts | 41 ++++++++++++++----- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/packages/backend/src/server/api/openapi/gen-spec.ts b/packages/backend/src/server/api/openapi/gen-spec.ts index 68fa81404..289bd8088 100644 --- a/packages/backend/src/server/api/openapi/gen-spec.ts +++ b/packages/backend/src/server/api/openapi/gen-spec.ts @@ -15,7 +15,7 @@ export function genOpenapiSpec() { externalDocs: { description: 'Repository', - url: 'https://github.com/misskey-dev/misskey', + url: 'https://akkoma.dev/FoundKeyGang/FoundKey', }, servers: [{ @@ -33,6 +33,11 @@ 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', + } }, }, }; @@ -71,22 +76,29 @@ export function genOpenapiSpec() { schema.required.push('file'); } + const security = [ + { + ApiKeyAuth: [], + }, + { + Bearer: [], + }, + ]; + if (!endpoint.meta.requireCredential) { + // add this to make authentication optional + security.push({}); + } + const info = { operationId: endpoint.name, summary: endpoint.name, description: desc, externalDocs: { description: 'Source code', - url: `https://github.com/misskey-dev/misskey/blob/develop/packages/backend/src/server/api/endpoints/${endpoint.name}.ts`, + url: `https://akkoma.dev/FoundKeyGang/FoundKey/src/branch/main/packages/backend/src/server/api/endpoints/${endpoint.name}.ts`, }, - ...(endpoint.meta.tags ? { - tags: [endpoint.meta.tags[0]], - } : {}), - ...(endpoint.meta.requireCredential ? { - security: [{ - ApiKeyAuth: [], - }], - } : {}), + tags: endpoint.meta.tags || undefined, + security, requestBody: { required: true, content: { @@ -181,9 +193,16 @@ export function genOpenapiSpec() { }, }; - spec.paths['/' + endpoint.name] = { + const path = { post: info, }; + if (endpoint.meta.allowGet) { + path.get = { ...info }; + // API Key authentication is not permitted for GET requests + path.get.security = path.get.security.filter(elem => !Object.prototype.hasOwnProperty.call(elem, 'ApiKeyAuth')); + } + + spec.paths['/' + endpoint.name] = path; } return spec; -- 2.34.1 From ff75382af30463c60a1f74e1e2488be491d612c6 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Mon, 18 Jul 2022 23:32:03 +0200 Subject: [PATCH 4/5] handle authentication errors in stream API --- packages/backend/src/server/api/streaming.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/backend/src/server/api/streaming.ts b/packages/backend/src/server/api/streaming.ts index 35d0c0fc0..cfe209d09 100644 --- a/packages/backend/src/server/api/streaming.ts +++ b/packages/backend/src/server/api/streaming.ts @@ -17,10 +17,14 @@ export const initializeStreamingServer = (server: http.Server) => { ws.on('request', async (request) => { const q = request.resourceURL.query as ParsedUrlQuery; - // TODO: トークンが間違ってるなどしてauthenticateに失敗したら - // コネクション切断するなりエラーメッセージ返すなりする - // (現状はエラーがキャッチされておらずサーバーのログに流れて邪魔なので) - const [user, app] = await authenticate(request.httpRequest.headers.authorization, q.i); + const [user, app] = await authenticate(request.httpRequest.headers.authorization, q.i) + .catch(err => { + request.reject(403, err.message); + return []; + }); + if (typeof user === 'undefined') { + return; + } if (user?.isSuspended) { request.reject(400); -- 2.34.1 From fddf3573a107890ae63b2bd30b46fa43a286ac72 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Tue, 19 Jul 2022 00:49:40 +0200 Subject: [PATCH 5/5] client: use bearer token authorization --- packages/client/src/components/cropper-dialog.vue | 4 +++- packages/client/src/components/page/page.post.vue | 4 +++- packages/client/src/os.ts | 15 +++++++++------ packages/client/src/scripts/upload.ts | 2 +- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/packages/client/src/components/cropper-dialog.vue b/packages/client/src/components/cropper-dialog.vue index a8bde6ea0..47335af6a 100644 --- a/packages/client/src/components/cropper-dialog.vue +++ b/packages/client/src/components/cropper-dialog.vue @@ -62,7 +62,6 @@ const ok = async () => { croppedCanvas.toBlob(blob => { const formData = new FormData(); formData.append('file', blob); - formData.append('i', $i.token); if (defaultStore.state.uploadFolder) { formData.append('folderId', defaultStore.state.uploadFolder); } @@ -70,6 +69,9 @@ const ok = async () => { fetch(apiUrl + '/drive/files/create', { method: 'POST', body: formData, + headers: { + authorization: `Bearer ${$i.token}`, + }, }) .then(response => response.json()) .then(f => { diff --git a/packages/client/src/components/page/page.post.vue b/packages/client/src/components/page/page.post.vue index 3401f945b..1b11e6f48 100644 --- a/packages/client/src/components/page/page.post.vue +++ b/packages/client/src/components/page/page.post.vue @@ -54,7 +54,6 @@ export default defineComponent({ canvas.toBlob(blob => { const formData = new FormData(); formData.append('file', blob); - formData.append('i', this.$i.token); if (this.$store.state.uploadFolder) { formData.append('folderId', this.$store.state.uploadFolder); } @@ -62,6 +61,9 @@ export default defineComponent({ fetch(apiUrl + '/drive/files/create', { method: 'POST', body: formData, + headers: { + authorization: `Bearer ${this.$i.token}`, + }, }) .then(response => response.json()) .then(f => { diff --git a/packages/client/src/os.ts b/packages/client/src/os.ts index 64fb9b0ea..fa7c41357 100644 --- a/packages/client/src/os.ts +++ b/packages/client/src/os.ts @@ -23,17 +23,16 @@ export const api = ((endpoint: string, data: Record = {}, token?: s pendingApiRequestsCount.value--; }; - const promise = new Promise((resolve, reject) => { - // Append a credential - if ($i) (data as any).i = $i.token; - if (token !== undefined) (data as any).i = token; + const authorizationToken = token ?? $i?.token ?? undefined; + const authorization = authorizationToken ? `Bearer ${authorizationToken}` : undefined; - // Send request + const promise = new Promise((resolve, reject) => { fetch(endpoint.indexOf('://') > -1 ? endpoint : `${apiUrl}/${endpoint}`, { method: 'POST', body: JSON.stringify(data), credentials: 'omit', cache: 'no-cache', + headers: { authorization }, }).then(async (res) => { const body = res.status === 204 ? null : await res.json(); @@ -52,7 +51,7 @@ export const api = ((endpoint: string, data: Record = {}, token?: s return promise; }) as typeof apiClient.request; -export const apiGet = ((endpoint: string, data: Record = {}) => { +export const apiGet = ((endpoint: string, data: Record = {}, token?: string | null | undefined) => { pendingApiRequestsCount.value++; const onFinally = () => { @@ -61,12 +60,16 @@ export const apiGet = ((endpoint: string, data: Record = {}) => { const query = new URLSearchParams(data); + const authorizationToken = token ?? $i?.token ?? undefined; + const authorization = authorizationToken ? `Bearer ${authorizationToken}` : undefined; + const promise = new Promise((resolve, reject) => { // Send request fetch(`${apiUrl}/${endpoint}?${query}`, { method: 'GET', credentials: 'omit', cache: 'default', + headers: { authorization }, }).then(async (res) => { const body = res.status === 204 ? null : await res.json(); diff --git a/packages/client/src/scripts/upload.ts b/packages/client/src/scripts/upload.ts index 2f7b30b58..c333826ce 100644 --- a/packages/client/src/scripts/upload.ts +++ b/packages/client/src/scripts/upload.ts @@ -70,7 +70,6 @@ export function uploadFile( } const formData = new FormData(); - formData.append('i', $i.token); formData.append('force', 'true'); formData.append('file', resizedImage || file); formData.append('name', ctx.name); @@ -78,6 +77,7 @@ export function uploadFile( const xhr = new XMLHttpRequest(); xhr.open('POST', apiUrl + '/drive/files/create', true); + xhr.setRequestHeader('Authorization', `Bearer ${$i.token}`); xhr.onload = (ev) => { if (xhr.status !== 200 || ev.target == null || ev.target.response == null) { // TODO: 消すのではなくて再送できるようにしたい -- 2.34.1