From d7e378b16f35ca37527b0e560de2d7c6c09ad956 Mon Sep 17 00:00:00 2001 From: Thibaud Desodt Date: Wed, 13 Jan 2021 23:00:30 +0100 Subject: [PATCH] refactor(user): no longer expose user id and rely only on login --- docs/swagger.yaml | 3 --- src/api/__testTools__/Fixtures.ts | 1 - src/api/__tests__/builds-api.spec.ts | 4 ++-- src/api/__tests__/diagnostics-api.spec.ts | 1 - src/api/__tests__/dynamic-api.spec.ts | 4 ++-- src/api/api-types.ts | 1 - src/api/auth/BearerAuthenticationProvider.ts | 1 - src/api/auth/IAuthentication.ts | 1 - src/api/controllers/BuildInfoController.ts | 2 +- src/api/controllers/DiagnosticsController.ts | 1 - src/domain/IUserRepository.ts | 1 - src/infra/github/UserRepository.ts | 1 - src/infra/github/__testTools__/TestConstants.ts | 2 +- src/infra/memory/InMemoryUserRepository.ts | 1 - 14 files changed, 6 insertions(+), 18 deletions(-) diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 6fda3f7..06d53c5 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -287,8 +287,6 @@ components: additionalProperties: false WhoAmIResponse: properties: - id: - type: string login: type: string name: @@ -299,7 +297,6 @@ components: type: string type: array required: - - id - login - name - scopes diff --git a/src/api/__testTools__/Fixtures.ts b/src/api/__testTools__/Fixtures.ts index 60d3cb4..e2f01c1 100644 --- a/src/api/__testTools__/Fixtures.ts +++ b/src/api/__testTools__/Fixtures.ts @@ -3,7 +3,6 @@ import { User, UserWithScopes } from '../../domain/IUserRepository'; export class Fixtures { public static userWithScopes(scopes: string[], options: Partial = {}): UserWithScopes { return { - id: options.id ?? `USER_ID_${new Date().getTime()}`, login: options.login ?? `USER_LOGIN_${new Date().getTime()}`, name: options.name === undefined ? `User Name ${new Date().getTime()}` : options.name, scopes, diff --git a/src/api/__tests__/builds-api.spec.ts b/src/api/__tests__/builds-api.spec.ts index 3ad9615..43a3f4c 100644 --- a/src/api/__tests__/builds-api.spec.ts +++ b/src/api/__tests__/builds-api.spec.ts @@ -86,7 +86,7 @@ describe('/builds', () => { const response = await agent.get('/builds').set('Authorization', `Bearer ${token}`).send(); const body = response.body as DynamicBuildInfoMetadataResponse; - expect(body.currentUser).toEqual({ id: user.id, name: user.name }); + expect(body.currentUser).toEqual({ id: user.login, name: user.name }); }); test('should return login when user has no name', async () => { @@ -99,7 +99,7 @@ describe('/builds', () => { .send(); const body = response.body as DynamicBuildInfoMetadataResponse; - expect(body.currentUser).toEqual({ id: userWithNoName.id, name: userWithNoName.login }); + expect(body.currentUser).toEqual({ id: userWithNoName.login, name: userWithNoName.login }); }); test('should return spaces of user', async () => { diff --git a/src/api/__tests__/diagnostics-api.spec.ts b/src/api/__tests__/diagnostics-api.spec.ts index c77cd92..402f0e4 100644 --- a/src/api/__tests__/diagnostics-api.spec.ts +++ b/src/api/__tests__/diagnostics-api.spec.ts @@ -52,7 +52,6 @@ describe('Public API /_/* (diagnostics)', () => { expect(response.status).toBe(200); expect(response.type).toBe('application/json'); expect(response.body).toEqual({ - id: existingUser.id, name: existingUser.name, login: existingUser.login, scopes: existingUser.scopes, diff --git a/src/api/__tests__/dynamic-api.spec.ts b/src/api/__tests__/dynamic-api.spec.ts index 91b61f2..74b324c 100644 --- a/src/api/__tests__/dynamic-api.spec.ts +++ b/src/api/__tests__/dynamic-api.spec.ts @@ -86,7 +86,7 @@ describe('/dynamic', () => { const response = await agent.get('/dynamic').set('Authorization', `Bearer ${token}`).send(); const body = response.body as DynamicBuildInfoMetadataResponse; - expect(body.currentUser).toEqual({ id: user.id, name: user.name }); + expect(body.currentUser).toEqual({ id: user.login, name: user.name }); }); test('should return login when user has no name', async () => { @@ -99,7 +99,7 @@ describe('/dynamic', () => { .send(); const body = response.body as DynamicBuildInfoMetadataResponse; - expect(body.currentUser).toEqual({ id: userWithNoName.id, name: userWithNoName.login }); + expect(body.currentUser).toEqual({ id: userWithNoName.login, name: userWithNoName.login }); }); test('should return spaces of user', async () => { diff --git a/src/api/api-types.ts b/src/api/api-types.ts index aabef02..91ef0f2 100644 --- a/src/api/api-types.ts +++ b/src/api/api-types.ts @@ -14,7 +14,6 @@ export interface HealthCheckResponse { } export interface WhoAmIResponse { - readonly id: string; readonly login: string; readonly name: string | null; readonly scopes: string[]; diff --git a/src/api/auth/BearerAuthenticationProvider.ts b/src/api/auth/BearerAuthenticationProvider.ts index 862b50d..2acaacd 100644 --- a/src/api/auth/BearerAuthenticationProvider.ts +++ b/src/api/auth/BearerAuthenticationProvider.ts @@ -50,7 +50,6 @@ export class BearerAuthenticationProvider implements IAuthenticationProvider { const user = await this.users.getUserFromToken(token); return { - id: user.id, name: user.name, login: user.login, token, diff --git a/src/api/auth/IAuthentication.ts b/src/api/auth/IAuthentication.ts index c2dcab9..f8de593 100644 --- a/src/api/auth/IAuthentication.ts +++ b/src/api/auth/IAuthentication.ts @@ -1,7 +1,6 @@ import { Request } from 'express'; export interface IAuthenticatedUser { - readonly id: string; readonly login: string; readonly name: string | null; readonly token: string; diff --git a/src/api/controllers/BuildInfoController.ts b/src/api/controllers/BuildInfoController.ts index bb2bc28..bc10ef0 100644 --- a/src/api/controllers/BuildInfoController.ts +++ b/src/api/controllers/BuildInfoController.ts @@ -174,7 +174,7 @@ export class BuildInfoController extends Controller { private mapToUser(authUser: IAuthenticatedUser): catlightCore.User { return { - id: authUser.id, + id: authUser.login, name: authUser.name ?? authUser.login, }; } diff --git a/src/api/controllers/DiagnosticsController.ts b/src/api/controllers/DiagnosticsController.ts index 37c0533..67b9a5d 100644 --- a/src/api/controllers/DiagnosticsController.ts +++ b/src/api/controllers/DiagnosticsController.ts @@ -29,7 +29,6 @@ export class DiagnosticsController extends Controller { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const currentUser = request.user!; return { - id: currentUser.id, name: currentUser.name, login: currentUser.login, scopes: currentUser.tokenScopes, diff --git a/src/domain/IUserRepository.ts b/src/domain/IUserRepository.ts index 5744c2e..a8e0cf6 100644 --- a/src/domain/IUserRepository.ts +++ b/src/domain/IUserRepository.ts @@ -1,7 +1,6 @@ export interface User { readonly login: string; readonly name: string | null; - readonly id: string; } export interface UserWithScopes extends User { diff --git a/src/infra/github/UserRepository.ts b/src/infra/github/UserRepository.ts index 7a84112..b4ed4a7 100644 --- a/src/infra/github/UserRepository.ts +++ b/src/infra/github/UserRepository.ts @@ -14,7 +14,6 @@ export class UserRepository implements IUserRepository { .filter((x) => x !== ''); return { - id: response.data.id.toString(), login: response.data.login, name: response.data.name, scopes: userScopes, diff --git a/src/infra/github/__testTools__/TestConstants.ts b/src/infra/github/__testTools__/TestConstants.ts index da79969..6f327fc 100644 --- a/src/infra/github/__testTools__/TestConstants.ts +++ b/src/infra/github/__testTools__/TestConstants.ts @@ -1,7 +1,7 @@ import { Repo, RepoName, Workflow } from '../../../domain/IRepoRepository'; import { User } from '../../../domain/IUserRepository'; -export const THIS_REPO_OWNER: User = { id: '160544', login: 'tsimbalar', name: 'Thibaud Desodt' }; +export const THIS_REPO_OWNER: User = { login: 'tsimbalar', name: 'Thibaud Desodt' }; export const THIS_REPO_NAME = new RepoName(THIS_REPO_OWNER.login, 'gha-build-monitor'); diff --git a/src/infra/memory/InMemoryUserRepository.ts b/src/infra/memory/InMemoryUserRepository.ts index 5511cb9..5ef5140 100644 --- a/src/infra/memory/InMemoryUserRepository.ts +++ b/src/infra/memory/InMemoryUserRepository.ts @@ -12,7 +12,6 @@ export class InMemoryUserRepository implements IUserRepository { } return { - id: storedUser.id, name: storedUser.name, login: storedUser.login, scopes: storedUser.scopes,