From 5ea56929234bbf3a5170702f580bf23ba8636240 Mon Sep 17 00:00:00 2001 From: BichraiX <160038319+BichraiX@users.noreply.github.com> Date: Mon, 19 Aug 2024 16:26:02 +0400 Subject: [PATCH] fix : removed the fetch in /delete and replaced it with auxiliary function --- .../src/__testData__/registerConf.json | 3 +- .../src/account/3pid/3pid.test.ts | 46 +--- .../src/account/3pid/delete.ts | 200 ++++++++---------- packages/matrix-client-server/src/config.json | 3 +- .../src/register/index.ts | 27 ++- .../src/register/register.test.ts | 2 - packages/matrix-client-server/src/types.ts | 1 + .../src/user/openid/requestToken.ts | 42 ++-- 8 files changed, 130 insertions(+), 194 deletions(-) diff --git a/packages/matrix-client-server/src/__testData__/registerConf.json b/packages/matrix-client-server/src/__testData__/registerConf.json index 5503b884..890636e8 100644 --- a/packages/matrix-client-server/src/__testData__/registerConf.json +++ b/packages/matrix-client-server/src/__testData__/registerConf.json @@ -55,5 +55,6 @@ "is_sso_login_enabled": true, "is_msisdn_login_enabled": true, "registration_required_3pid": [], - "capabilities": {} + "capabilities": {}, + "open_id_token_lifetime": 3600000 } diff --git a/packages/matrix-client-server/src/account/3pid/3pid.test.ts b/packages/matrix-client-server/src/account/3pid/3pid.test.ts index 6fc715ee..7c79f70b 100644 --- a/packages/matrix-client-server/src/account/3pid/3pid.test.ts +++ b/packages/matrix-client-server/src/account/3pid/3pid.test.ts @@ -474,19 +474,6 @@ describe('Use configuration file', () => { expect(response.body).toHaveProperty('error', 'Invalid phone number') }) it('should unbind from the id server provided in the request body', async () => { - const mockOpenIDResponse = Promise.resolve({ - ok: true, - status: 200, - // eslint-disable-next-line @typescript-eslint/promise-function-async - json: () => - Promise.resolve({ - access_token: 'openIdToken', - expires_in: 3600, - matrix_server_name: 'example.com', - token_type: 'Bearer' - }) - }) - const mockResolveResponse = Promise.resolve({ ok: true, status: 200, @@ -518,8 +505,6 @@ describe('Use configuration file', () => { }) }) // @ts-expect-error mock is unknown - fetch.mockImplementationOnce(async () => await mockOpenIDResponse) - // @ts-expect-error mock is unknown fetch.mockImplementationOnce(async () => await mockResolveResponse) // @ts-expect-error mock is unknown fetch.mockImplementationOnce(async () => await mockRegisterResponse) @@ -564,19 +549,6 @@ describe('Use configuration file', () => { added_at: epoch() }) - const mockOpenIDResponse = Promise.resolve({ - ok: true, - status: 200, - // eslint-disable-next-line @typescript-eslint/promise-function-async - json: () => - Promise.resolve({ - access_token: 'openIdToken', - expires_in: 3600, - matrix_server_name: 'example.com', - token_type: 'Bearer' - }) - }) - const mockResolveResponse = Promise.resolve({ ok: true, status: 200, @@ -608,8 +580,6 @@ describe('Use configuration file', () => { }) }) // @ts-expect-error mock is unknown - fetch.mockImplementationOnce(async () => await mockOpenIDResponse) - // @ts-expect-error mock is unknown fetch.mockImplementationOnce(async () => await mockResolveResponse) // @ts-expect-error mock is unknown fetch.mockImplementationOnce(async () => await mockRegisterResponse) @@ -655,19 +625,6 @@ describe('Use configuration file', () => { ) }) it('should return an error if the unbind was unsuccessful on the id-server', async () => { - const mockOpenIDResponse = Promise.resolve({ - ok: true, - status: 200, - // eslint-disable-next-line @typescript-eslint/promise-function-async - json: () => - Promise.resolve({ - access_token: 'openIdToken', - expires_in: 3600, - matrix_server_name: 'example.com', - token_type: 'Bearer' - }) - }) - const mockResolveResponse = Promise.resolve({ ok: true, status: 200, @@ -698,8 +655,7 @@ describe('Use configuration file', () => { errcode: 'M_INVALID_PARAM' }) }) - // @ts-expect-error mock is unknown - fetch.mockImplementationOnce(async () => await mockOpenIDResponse) + // @ts-expect-error mock is unknown fetch.mockImplementationOnce(async () => await mockResolveResponse) // @ts-expect-error mock is unknown diff --git a/packages/matrix-client-server/src/account/3pid/delete.ts b/packages/matrix-client-server/src/account/3pid/delete.ts index f7cdb7c9..cce654c8 100644 --- a/packages/matrix-client-server/src/account/3pid/delete.ts +++ b/packages/matrix-client-server/src/account/3pid/delete.ts @@ -14,6 +14,8 @@ import type { ServerResponse } from 'http' import type e from 'express' import { MatrixResolve } from 'matrix-resolve' import { isAdmin } from '../../utils/utils' +import { insertOpenIdToken } from '../../user/openid/requestToken' +import { randomString } from '@twake/crypto' interface RequestBody { address: string @@ -25,13 +27,6 @@ interface RegisterResponseBody { token: string } -interface OpenIDResponseBody { - access_token: string - expires_in: number - matrix_server_name: string - token_type: string -} - const schema = { address: true, id_server: false, @@ -43,101 +38,86 @@ const deleteAndSend = async ( body: RequestBody, clientServer: MatrixClientServer, idServer: string, - data: TokenContent, - token: string | null + data: TokenContent ): Promise => { - const openIDResponse = await fetch( - `https://${clientServer.conf.server_name}/_matrix/client/v3/user/${data.sub}/openid/request_token`, - { - method: 'POST', - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json', - Authorization: `Bearer ${token as string}` - }, - body: JSON.stringify({ - user_id: data.sub - }) - } - ) - // istanbul ignore if - if (!openIDResponse.ok) { - // istanbul ignore next - send( - res, - 500, - errMsg('unknown', 'Error while requesting OpenID token'), - clientServer.logger - ) - // istanbul ignore next - return - } - const openIDResponseBody: OpenIDResponseBody = - (await openIDResponse.json()) as OpenIDResponseBody - - const matrixResolve = new MatrixResolve({ - cache: 'toad-cache' - }) - const baseUrl: string | string[] = await matrixResolve.resolve(idServer) - const registerResponse = await fetch( - `https://${baseUrl as string}/_matrix/identity/v2/account/register`, - { - method: 'POST', - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json' - }, - body: JSON.stringify(openIDResponseBody) - } - ) - const validToken = ((await registerResponse.json()) as RegisterResponseBody) - .token - const UnbindResponse = await fetch( - `https://${baseUrl as string}/_matrix/identity/v2/3pid/unbind`, - { - method: 'POST', - headers: { - Authorization: `Bearer ${validToken}`, - Accept: 'application/json', - 'Content-Type': 'application/json' - }, - body: JSON.stringify({ - address: body.address, - medium: body.medium - }) - } - ) - if (UnbindResponse.ok) { - const deleteAdd = clientServer.matrixDb.deleteWhere('user_threepids', [ - { field: 'address', value: body.address, operator: '=' }, - { field: 'medium', value: body.medium, operator: '=' }, - { field: 'user_id', value: data.sub, operator: '=' } - ]) - const deleteBind = clientServer.matrixDb.deleteWhere( - 'user_threepid_id_server', - [ - { field: 'address', value: body.address, operator: '=' }, - { field: 'medium', value: body.medium, operator: '=' }, - { field: 'user_id', value: data.sub, operator: '=' }, - { field: 'id_server', value: idServer, operator: '=' } - ] - ) - Promise.all([deleteAdd, deleteBind]) - .then(() => { - send(res, 200, { id_server_unbind_result: 'success' }) + insertOpenIdToken(clientServer, data.sub, randomString(64)) + .then(async (openIDRows) => { + const matrixResolve = new MatrixResolve({ + cache: 'toad-cache' }) - .catch((e) => { - // istanbul ignore next - clientServer.logger.error('Error while deleting user_threepids', e) + const baseUrl: string | string[] = await matrixResolve.resolve(idServer) + const registerResponse = await fetch( + `https://${baseUrl as string}/_matrix/identity/v2/account/register`, + { + method: 'POST', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json' + }, + body: JSON.stringify({ + access_token: openIDRows[0].token, + expires_in: clientServer.conf.open_id_token_lifetime, + matrix_server_name: clientServer.conf.server_name, + token_type: 'Bearer' + }) + } + ) + const validToken = ( + (await registerResponse.json()) as RegisterResponseBody + ).token + const UnbindResponse = await fetch( + `https://${baseUrl as string}/_matrix/identity/v2/3pid/unbind`, + { + method: 'POST', + headers: { + Authorization: `Bearer ${validToken}`, + Accept: 'application/json', + 'Content-Type': 'application/json' + }, + body: JSON.stringify({ + address: body.address, + medium: body.medium + }) + } + ) + if (UnbindResponse.ok) { + const deleteAdd = clientServer.matrixDb.deleteWhere('user_threepids', [ + { field: 'address', value: body.address, operator: '=' }, + { field: 'medium', value: body.medium, operator: '=' }, + { field: 'user_id', value: data.sub, operator: '=' } + ]) + const deleteBind = clientServer.matrixDb.deleteWhere( + 'user_threepid_id_server', + [ + { field: 'address', value: body.address, operator: '=' }, + { field: 'medium', value: body.medium, operator: '=' }, + { field: 'user_id', value: data.sub, operator: '=' }, + { field: 'id_server', value: idServer, operator: '=' } + ] + ) + Promise.all([deleteAdd, deleteBind]) + .then(() => { + send(res, 200, { id_server_unbind_result: 'success' }) + }) + .catch((e) => { + // istanbul ignore next + clientServer.logger.error('Error while deleting user_threepids', e) + // istanbul ignore next + send(res, 500, errMsg('unknown', e), clientServer.logger) + }) + } else { // istanbul ignore next - send(res, 500, errMsg('unknown', e), clientServer.logger) - }) - } else { - // istanbul ignore next - send(res, UnbindResponse.status, { - id_server_unbind_result: 'no-support' + send(res, UnbindResponse.status, { + id_server_unbind_result: 'no-support' + }) + } + }) + .catch((e) => { + // istanbul ignore next + clientServer.logger.error('Error while inserting openID token', e) + // istanbul ignore next + send(res, 500, errMsg('unknown', e), clientServer.logger) }) - } } const delete3pid = (clientServer: MatrixClientServer): expressAppHandler => { @@ -201,19 +181,14 @@ const delete3pid = (clientServer: MatrixClientServer): expressAppHandler => { // eslint-disable-next-line @typescript-eslint/strict-boolean-expressions if (typeof body.id_server === 'string' && body.id_server) { idServer = body.id_server - deleteAndSend( - res, - body, - clientServer, - idServer, - data, - token - ).catch((e) => { - // istanbul ignore next - clientServer.logger.error('Error while deleting 3pid', e) - // istanbul ignore next - send(res, 500, errMsg('unknown', e), clientServer.logger) - }) + deleteAndSend(res, body, clientServer, idServer, data).catch( + (e) => { + // istanbul ignore next + clientServer.logger.error('Error while deleting 3pid', e) + // istanbul ignore next + send(res, 500, errMsg('unknown', e), clientServer.logger) + } + ) } else { clientServer.matrixDb .get('user_threepid_id_server', ['id_server'], { @@ -236,8 +211,7 @@ const delete3pid = (clientServer: MatrixClientServer): expressAppHandler => { body, clientServer, rows[0].id_server as string, - data, - token + data ).catch((e) => { // istanbul ignore next clientServer.logger.error('Error while deleting 3pid', e) diff --git a/packages/matrix-client-server/src/config.json b/packages/matrix-client-server/src/config.json index dfd4474f..43da286f 100644 --- a/packages/matrix-client-server/src/config.json +++ b/packages/matrix-client-server/src/config.json @@ -101,5 +101,6 @@ "is_password_login_enabled": true, "is_sso_login_enabled": true, "is_msisdn_login_enabled": true, - "registration_required_3pid": [] + "registration_required_3pid": [], + "open_id_token_lifetime": 3600000 } diff --git a/packages/matrix-client-server/src/register/index.ts b/packages/matrix-client-server/src/register/index.ts index 02d1c404..c45d45d9 100644 --- a/packages/matrix-client-server/src/register/index.ts +++ b/packages/matrix-client-server/src/register/index.ts @@ -90,15 +90,13 @@ const sendSuccessResponse = ( send(res, 200, { access_token: accessToken, device_id: deviceId, - user_id: userId, - expires_in_ms: 60000 // Arbitrary value, should probably be defined in the server config // TODO : Add this in the config + user_id: userId }) } else { send(res, 200, { access_token: accessToken, device_id: deviceId, user_id: userId, - expires_in_ms: 60000, // Arbitrary value, should probably be defined in the server config // TODO : Add this in the config refresh_token: refreshToken }) } @@ -123,7 +121,8 @@ const registerAccount = ( const accessToken = randomString(64) const refreshToken = randomString(64) const refreshTokenId = randomString(64) - const createUserPromise = (): Promise => { + // eslint-disable-next-line @typescript-eslint/no-invalid-void-type + const createUserPromise = (): Promise | void => { const commonUserData: InsertedData = { name: userId, creation_ts: epoch(), @@ -136,14 +135,15 @@ const registerAccount = ( if (password) { if (typeof password !== 'string' || password.length > 512) { send(res, 400, errMsg('invalidParam', 'Invalid password')) - } - const hash = new Hash() - return hash.ready.then(() => { - return clientServer.matrixDb.insert('users', { - ...commonUserData, - password_hash: hash.sha256(password) // TODO: Handle other hashing algorithms + } else { + const hash = new Hash() + return hash.ready.then(() => { + return clientServer.matrixDb.insert('users', { + ...commonUserData, + password_hash: hash.sha256(password) // TODO: Handle other hashing algorithms + }) }) - }) + } } else { return clientServer.matrixDb.insert('users', { ...commonUserData }) } @@ -170,16 +170,15 @@ const registerAccount = ( id: refreshTokenId, user_id: userId, device_id: deviceId, - token: refreshToken // TODO : maybe add expiry_ts here + token: refreshToken }) const accessTokenPromise = clientServer.matrixDb.insert('access_tokens', { id: randomString(64), // To be fixed later user_id: userId, token: accessToken, device_id: deviceId, - valid_until_ms: 0, refresh_token_id: refreshTokenId - }) // TODO : Add a token_lifetime in the config, replace the id with a correct one, and fill the 'puppets_user_id' row with the right value + }) // TODO : replace the id with a correct one, and fill the 'puppets_user_id' row with the right value const promisesToExecute = body.inhibit_login ? [userIpPromise, userPromise, fillPoliciesPromise] : [ diff --git a/packages/matrix-client-server/src/register/register.test.ts b/packages/matrix-client-server/src/register/register.test.ts index 1750d8e9..ae2eb93e 100644 --- a/packages/matrix-client-server/src/register/register.test.ts +++ b/packages/matrix-client-server/src/register/register.test.ts @@ -493,7 +493,6 @@ describe('Use configuration file', () => { }) expect(response.statusCode).toBe(200) expect(response.body).toHaveProperty('user_id') - expect(response.body).toHaveProperty('expires_in_ms') expect(response.body).toHaveProperty('access_token') expect(response.body).toHaveProperty('device_id') }) @@ -652,7 +651,6 @@ describe('Use configuration file', () => { .send({}) expect(response.statusCode).toBe(200) expect(response.body).toHaveProperty('user_id') - expect(response.body).toHaveProperty('expires_in_ms') expect(response.body).toHaveProperty('access_token') expect(response.body).toHaveProperty('device_id') guestToken = response.body.access_token diff --git a/packages/matrix-client-server/src/types.ts b/packages/matrix-client-server/src/types.ts index 26144a09..29d66598 100644 --- a/packages/matrix-client-server/src/types.ts +++ b/packages/matrix-client-server/src/types.ts @@ -20,6 +20,7 @@ export type Config = MIdentityServerConfig & { is_registration_token_login_enabled: boolean registration_required_3pid: string[] user_directory: UserDirectoryConfig + open_id_token_lifetime: number } export type DbGetResult = Array< diff --git a/packages/matrix-client-server/src/user/openid/requestToken.ts b/packages/matrix-client-server/src/user/openid/requestToken.ts index 0af9afbe..4928251f 100644 --- a/packages/matrix-client-server/src/user/openid/requestToken.ts +++ b/packages/matrix-client-server/src/user/openid/requestToken.ts @@ -7,16 +7,22 @@ import { } from '@twake/utils' import type MatrixClientServer from '../..' import { randomString } from '@twake/crypto' +import { type DbGetResult } from '@twake/matrix-identity-server' interface Parameters { userId: string } -interface ResponseBody { - access_token: string - expires_in: number - matrix_server_name: string - token_type: string +export const insertOpenIdToken = async ( + clientServer: MatrixClientServer, + userId: string, + token: string +): Promise => { + return await clientServer.matrixDb.insert('open_id_tokens', { + token, + user_id: userId, + ts_valid_until_ms: epoch() + clientServer.conf.open_id_token_lifetime + }) } const requestToken = (clientServer: MatrixClientServer): expressAppHandler => { @@ -40,20 +46,20 @@ const requestToken = (clientServer: MatrixClientServer): expressAppHandler => { send(res, 403, errMsg('forbidden'), clientServer.logger) return } - const responseBody: ResponseBody = { - access_token: randomString(64), - expires_in: 64000, // TODO : Put expiry time in the config - matrix_server_name: clientServer.conf.server_name, - token_type: 'Bearer' - } - clientServer.matrixDb - .insert('open_id_tokens', { - token: responseBody.access_token, - user_id: userId, - ts_valid_until_ms: epoch() + responseBody.expires_in - }) + const accessToken = randomString(64) + insertOpenIdToken(clientServer, userId, accessToken) .then(() => { - send(res, 200, responseBody, clientServer.logger) + send( + res, + 200, + { + access_token: accessToken, + expires_in: clientServer.conf.open_id_token_lifetime, + matrix_server_name: clientServer.conf.server_name, + token_type: 'Bearer' + }, + clientServer.logger + ) }) .catch((e) => { // istanbul ignore next