From d86a3dee42d6f6ca41ad1d9a1c220ea0266c6783 Mon Sep 17 00:00:00 2001 From: Moriz Wahl Date: Mon, 27 Sep 2021 19:04:27 +0200 Subject: [PATCH 1/3] refactor: Provide pubKey in context to avoid API calls --- backend/src/auth/auth.ts | 3 ++- backend/src/graphql/resolvers/BalanceResolver.ts | 12 ++---------- backend/src/graphql/resolvers/GdtResolver.ts | 8 ++------ backend/src/graphql/resolvers/UserResolver.ts | 6 +++++- backend/src/jwt/decode.ts | 14 +++++++++++--- backend/src/jwt/encode.ts | 4 ++-- backend/src/server/context.ts | 3 +++ backend/src/server/plugins.ts | 3 +++ backend/src/typeorm/entity/User.ts | 5 +++++ 9 files changed, 35 insertions(+), 23 deletions(-) diff --git a/backend/src/auth/auth.ts b/backend/src/auth/auth.ts index a287bfd8f..527c84394 100644 --- a/backend/src/auth/auth.ts +++ b/backend/src/auth/auth.ts @@ -15,7 +15,8 @@ export const isAuthorized: AuthChecker = async ({ root, args, context, info `${CONFIG.LOGIN_API_URL}checkSessionState?session_id=${decoded.sessionId}`, ) context.sessionId = decoded.sessionId - context.setHeaders.push({ key: 'token', value: encode(decoded.sessionId) }) + context.pubKey = decoded.pubKey + context.setHeaders.push({ key: 'token', value: encode(decoded.sessionId, decoded.pubKey) }) return result.success } } diff --git a/backend/src/graphql/resolvers/BalanceResolver.ts b/backend/src/graphql/resolvers/BalanceResolver.ts index 34aedac37..38c2984f1 100644 --- a/backend/src/graphql/resolvers/BalanceResolver.ts +++ b/backend/src/graphql/resolvers/BalanceResolver.ts @@ -2,9 +2,7 @@ /* eslint-disable @typescript-eslint/explicit-module-boundary-types */ import { Resolver, Query, Ctx, Authorized } from 'type-graphql' -import CONFIG from '../../config' import { Balance } from '../models/Balance' -import { apiGet } from '../../apis/HttpRequest' import { User as dbUser } from '../../typeorm/entity/User' import { Balance as dbBalance } from '../../typeorm/entity/Balance' import calculateDecay from '../../util/decay' @@ -15,20 +13,14 @@ export class BalanceResolver { @Authorized() @Query(() => Balance) async balance(@Ctx() context: any): Promise { - // get public key for current logged in user - const result = await apiGet(CONFIG.LOGIN_API_URL + 'login?session_id=' + context.sessionId) - if (!result.success) throw new Error(result.data) - // load user and balance - const userEntity = await dbUser.findByPubkeyHex(result.data.user.public_hex) + const userEntity = await dbUser.findByPubkeyHex(context.pubKey) const balanceEntity = await dbBalance.findByUser(userEntity.id) const now = new Date() - const balance = new Balance({ + return new Balance({ balance: roundFloorFrom4(balanceEntity.amount), decay: roundFloorFrom4(calculateDecay(balanceEntity.amount, balanceEntity.recordDate, now)), decay_date: now.toString(), }) - - return balance } } diff --git a/backend/src/graphql/resolvers/GdtResolver.ts b/backend/src/graphql/resolvers/GdtResolver.ts index 4396c5ac9..0e9af5ad6 100644 --- a/backend/src/graphql/resolvers/GdtResolver.ts +++ b/backend/src/graphql/resolvers/GdtResolver.ts @@ -18,18 +18,14 @@ export class GdtResolver { { currentPage = 1, pageSize = 5, order = 'DESC' }: GdtTransactionSessionIdInput, @Ctx() context: any, ): Promise { - // get public key for current logged in user - const result = await apiGet(CONFIG.LOGIN_API_URL + 'login?session_id=' + context.sessionId) - if (!result.success) throw new Error(result.data) - // load user - const userEntity = await dbUser.findByPubkeyHex(result.data.user.public_hex) + const userEntity = await dbUser.findByPubkeyHex(context.pubKey) const resultGDT = await apiGet( `${CONFIG.GDT_API_URL}/GdtEntries/listPerEmailApi/${userEntity.email}/${currentPage}/${pageSize}/${order}`, ) if (!resultGDT.success) { - throw new Error(result.data) + throw new Error(resultGDT.data) } return new GdtEntryList(resultGDT.data) diff --git a/backend/src/graphql/resolvers/UserResolver.ts b/backend/src/graphql/resolvers/UserResolver.ts index 06b10daec..8997fb264 100644 --- a/backend/src/graphql/resolvers/UserResolver.ts +++ b/backend/src/graphql/resolvers/UserResolver.ts @@ -22,6 +22,7 @@ import { klicktippNewsletterStateMiddleware, } from '../../middleware/klicktippMiddleware' import { CheckEmailResponse } from '../models/CheckEmailResponse' + @Resolver() export class UserResolver { @Query(() => User) @@ -35,7 +36,10 @@ export class UserResolver { throw new Error(result.data) } - context.setHeaders.push({ key: 'token', value: encode(result.data.session_id) }) + context.setHeaders.push({ + key: 'token', + value: encode(result.data.session_id, result.data.user.public_hex), + }) return new User(result.data.user) } diff --git a/backend/src/jwt/decode.ts b/backend/src/jwt/decode.ts index 2e24386b3..086267bc2 100644 --- a/backend/src/jwt/decode.ts +++ b/backend/src/jwt/decode.ts @@ -1,18 +1,26 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ /* eslint-disable @typescript-eslint/explicit-module-boundary-types */ -import jwt from 'jsonwebtoken' +import jwt, { JwtPayload } from 'jsonwebtoken' import CONFIG from '../config/' +interface CustomJwtPayload extends JwtPayload { + sessionId: number + pubKey: Buffer +} + export default (token: string): any => { if (!token) return new Error('401 Unauthorized') let sessionId = null + let pubKey = null try { - const decoded = jwt.verify(token, CONFIG.JWT_SECRET) - sessionId = decoded.sub + const decoded = jwt.verify(token, CONFIG.JWT_SECRET) + sessionId = decoded.sessionId + pubKey = decoded.pubKey return { token, sessionId, + pubKey, } } catch (err) { throw new Error('403.13 - Client certificate revoked') diff --git a/backend/src/jwt/encode.ts b/backend/src/jwt/encode.ts index 9c5145e6d..fde28b467 100644 --- a/backend/src/jwt/encode.ts +++ b/backend/src/jwt/encode.ts @@ -5,8 +5,8 @@ import jwt from 'jsonwebtoken' import CONFIG from '../config/' // Generate an Access Token -export default function encode(sessionId: string): string { - const token = jwt.sign({ sessionId }, CONFIG.JWT_SECRET, { +export default function encode(sessionId: number, pubKey: Buffer): string { + const token = jwt.sign({ sessionId, pubKey }, CONFIG.JWT_SECRET, { expiresIn: CONFIG.JWT_EXPIRES_IN, subject: sessionId.toString(), }) diff --git a/backend/src/server/context.ts b/backend/src/server/context.ts index 2ad4b520d..6de2adce4 100644 --- a/backend/src/server/context.ts +++ b/backend/src/server/context.ts @@ -1,3 +1,6 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +/* eslint-disable @typescript-eslint/explicit-module-boundary-types */ + const context = (args: any) => { const authorization = args.req.headers.authorization let token = null diff --git a/backend/src/server/plugins.ts b/backend/src/server/plugins.ts index 6b27d19ea..5436d595b 100644 --- a/backend/src/server/plugins.ts +++ b/backend/src/server/plugins.ts @@ -1,3 +1,6 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +/* eslint-disable @typescript-eslint/explicit-module-boundary-types */ + const plugins = [ { requestDidStart() { diff --git a/backend/src/typeorm/entity/User.ts b/backend/src/typeorm/entity/User.ts index cb9de27c8..f7b8ed7c1 100644 --- a/backend/src/typeorm/entity/User.ts +++ b/backend/src/typeorm/entity/User.ts @@ -1,6 +1,8 @@ import { BaseEntity, Entity, PrimaryGeneratedColumn, Column } from 'typeorm' // import { Group } from "./Group" + +// Moriz: I do not like the idea of having two user tables @Entity('state_users') export class User extends BaseEntity { @PrimaryGeneratedColumn() @@ -27,6 +29,9 @@ export class User extends BaseEntity { @Column() disabled: boolean + // Moriz: I am voting for the data mapper implementation. + // see: https://typeorm.io/#/active-record-data-mapper/what-is-the-data-mapper-pattern + // We should discuss this ASAP static findByPubkeyHex(pubkeyHex: string): Promise { return this.createQueryBuilder('user') .where('hex(user.pubkey) = :pubkeyHex', { pubkeyHex }) From 007b193a4d3c838f930a4f37cd9954b7073b0ede Mon Sep 17 00:00:00 2001 From: Moriz Wahl Date: Tue, 28 Sep 2021 12:11:17 +0200 Subject: [PATCH 2/3] throw errors instead of returning them, define interface for jwt payload --- backend/src/auth/auth.ts | 2 +- backend/src/jwt/decode.ts | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/backend/src/auth/auth.ts b/backend/src/auth/auth.ts index 527c84394..7be67764b 100644 --- a/backend/src/auth/auth.ts +++ b/backend/src/auth/auth.ts @@ -20,5 +20,5 @@ export const isAuthorized: AuthChecker = async ({ root, args, context, info return result.success } } - return false + throw new Error('401 Unauthorized') } diff --git a/backend/src/jwt/decode.ts b/backend/src/jwt/decode.ts index 086267bc2..34b3ed836 100644 --- a/backend/src/jwt/decode.ts +++ b/backend/src/jwt/decode.ts @@ -1,6 +1,3 @@ -/* eslint-disable @typescript-eslint/no-explicit-any */ -/* eslint-disable @typescript-eslint/explicit-module-boundary-types */ - import jwt, { JwtPayload } from 'jsonwebtoken' import CONFIG from '../config/' @@ -9,8 +6,14 @@ interface CustomJwtPayload extends JwtPayload { pubKey: Buffer } -export default (token: string): any => { - if (!token) return new Error('401 Unauthorized') +type DecodedJwt = { + token: string + sessionId: number + pubKey: Buffer +} + +export default (token: string): DecodedJwt => { + if (!token) throw new Error('401 Unauthorized') let sessionId = null let pubKey = null try { From f9b0e2caee85b8eb4e6fbe14d3d21ac4b667141c Mon Sep 17 00:00:00 2001 From: Moriz Wahl Date: Fri, 1 Oct 2021 12:43:43 +0200 Subject: [PATCH 3/3] fix frontemd unit tests --- .../Pages/UserProfile/UserCard_Language.spec.js | 14 +++++++------- .../Pages/UserProfile/UserCard_Newsletter.spec.js | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/frontend/src/views/Pages/UserProfile/UserCard_Language.spec.js b/frontend/src/views/Pages/UserProfile/UserCard_Language.spec.js index 49134ce91..774bc9e32 100644 --- a/frontend/src/views/Pages/UserProfile/UserCard_Language.spec.js +++ b/frontend/src/views/Pages/UserProfile/UserCard_Language.spec.js @@ -57,7 +57,7 @@ describe('UserCard_Language', () => { }) it('has change language as text', () => { - expect(wrapper.find('a').text()).toBe('form.changeLanguage') + expect(wrapper.find('a').text()).toBe('settings.language.changeLanguage') }) it('has no select field by default', () => { @@ -87,23 +87,23 @@ describe('UserCard_Language', () => { describe('change language', () => { it('does not enable the submit button when same language is chosen', () => { - wrapper.findAll('option').at(1).setSelected() + wrapper.findAll('option').at(0).setSelected() expect(wrapper.find('button[type="submit"]').attributes('disabled')).toBe('disabled') }) it('enables the submit button when other language is chosen', async () => { - await wrapper.findAll('option').at(2).setSelected() + await wrapper.findAll('option').at(1).setSelected() expect(wrapper.find('button[type="submit"]').attributes('disabled')).toBe(undefined) }) it('updates language data in component', async () => { - await wrapper.findAll('option').at(2).setSelected() + await wrapper.findAll('option').at(1).setSelected() expect(wrapper.vm.language).toBe('en') }) describe('cancel edit', () => { beforeEach(async () => { - await wrapper.findAll('option').at(2).setSelected() + await wrapper.findAll('option').at(1).setSelected() wrapper.find('a').trigger('click') }) @@ -118,7 +118,7 @@ describe('UserCard_Language', () => { describe('submit', () => { beforeEach(async () => { - await wrapper.findAll('option').at(2).setSelected() + await wrapper.findAll('option').at(1).setSelected() wrapper.find('form').trigger('submit') }) @@ -147,7 +147,7 @@ describe('UserCard_Language', () => { }) it('toasts a success message', () => { - expect(toastSuccessMock).toBeCalledWith('languages.success') + expect(toastSuccessMock).toBeCalledWith('settings.language.success') }) }) diff --git a/frontend/src/views/Pages/UserProfile/UserCard_Newsletter.spec.js b/frontend/src/views/Pages/UserProfile/UserCard_Newsletter.spec.js index af19a8fbd..37031769b 100644 --- a/frontend/src/views/Pages/UserProfile/UserCard_Newsletter.spec.js +++ b/frontend/src/views/Pages/UserProfile/UserCard_Newsletter.spec.js @@ -104,7 +104,7 @@ describe('UserCard_Newsletter', () => { }) it('toasts a success message', () => { - expect(toastSuccessMock).toBeCalledWith('setting.newsletterFalse') + expect(toastSuccessMock).toBeCalledWith('settings.newsletter.newsletterFalse') }) })