From 29f7eb4482354d85656f3b5fc016f0a70486330e Mon Sep 17 00:00:00 2001 From: Johann150 Date: Mon, 18 Jul 2022 17:41:08 +0200 Subject: [PATCH 1/6] 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 ec71ddd2c0..3fecea3fdd 100644 --- a/packages/backend/src/server/api/api-handler.ts +++ b/packages/backend/src/server/api/api-handler.ts @@ -43,7 +43,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 65ccfcf551..192f20ebcd 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 f8e42d27fe..35d0c0fc09 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); From 4ca48908be90060ec23d9d32d0da52f4901e9e67 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Mon, 18 Jul 2022 17:42:14 +0200 Subject: [PATCH 2/6] 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 3fecea3fdd..3cb94f10f6 100644 --- a/packages/backend/src/server/api/api-handler.ts +++ b/packages/backend/src/server/api/api-handler.ts @@ -81,11 +81,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 192f20ebcd..39be06c29f 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, { From 0131ec4a58d3134f4b632c8f1f9134fe0fd822e7 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Mon, 18 Jul 2022 18:37:41 +0200 Subject: [PATCH 3/6] update openapi spec generator Properly document GET API endpoints. Added Bearer token authentication. --- .../src/server/api/openapi/gen-spec.ts | 37 ++++++++++++++----- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/packages/backend/src/server/api/openapi/gen-spec.ts b/packages/backend/src/server/api/openapi/gen-spec.ts index 68fa814041..86f2f42280 100644 --- a/packages/backend/src/server/api/openapi/gen-spec.ts +++ b/packages/backend/src/server/api/openapi/gen-spec.ts @@ -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,6 +76,19 @@ 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, @@ -79,14 +97,8 @@ export function genOpenapiSpec() { description: 'Source code', url: `https://github.com/misskey-dev/misskey/blob/develop/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; From c4e2a222f025de3d70ab83c7fc8e1551683f34c8 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Mon, 18 Jul 2022 23:32:03 +0200 Subject: [PATCH 4/6] 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 35d0c0fc09..cfe209d090 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); From b27e3fb6348393b5fcdfd36402570a63c5635123 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Tue, 19 Jul 2022 00:49:40 +0200 Subject: [PATCH 5/6] 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 a8bde6ea05..47335af6a2 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 3401f945bd..1b11e6f48e 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 00dae867d6..9defc55df8 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 51f1c1b86f..6f50e9bd9c 100644 --- a/packages/client/src/scripts/upload.ts +++ b/packages/client/src/scripts/upload.ts @@ -71,7 +71,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); @@ -79,6 +78,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: 消すのではなくて(ネットワーク的なエラーなら)再送できるようにしたい From 5466f547cd056d9e3a1a66d332137450034c42e6 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Wed, 20 Jul 2022 18:40:23 +0200 Subject: [PATCH 6/6] fix: not logged in clients send correct header --- packages/client/src/os.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/client/src/os.ts b/packages/client/src/os.ts index 9defc55df8..ecf422a56e 100644 --- a/packages/client/src/os.ts +++ b/packages/client/src/os.ts @@ -32,7 +32,7 @@ export const api = ((endpoint: string, data: Record = {}, token?: s body: JSON.stringify(data), credentials: 'omit', cache: 'no-cache', - headers: { authorization }, + headers: authorization ? { authorization } : {}, }).then(async (res) => { const body = res.status === 204 ? null : await res.json(); @@ -69,7 +69,7 @@ export const apiGet = ((endpoint: string, data: Record = {}, token? method: 'GET', credentials: 'omit', cache: 'default', - headers: { authorization }, + headers: authorization ? { authorization } : {}, }).then(async (res) => { const body = res.status === 204 ? null : await res.json();