From bcb42072334ea38a5e870fd214db347e178da77b Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Fri, 31 Jul 2026 21:58:47 +0200 Subject: [PATCH 01/15] docs: add admin user management plan --- docs/plans/admin-user-management.md | 56 +++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 docs/plans/admin-user-management.md diff --git a/docs/plans/admin-user-management.md b/docs/plans/admin-user-management.md new file mode 100644 index 0000000..2b593dd --- /dev/null +++ b/docs/plans/admin-user-management.md @@ -0,0 +1,56 @@ +# Admin User Management Implementation Plan + +## Goal + +Add a shared user directory and a secure global-admin management surface spanning the NestJS backend and the modern Angular frontend. The legacy frontend remains untouched. + +## Global Constraints + +- Reuse the existing `User.status`, `User.role`, and `Player.user` relations; do not add a new assignment table. +- Only global `RoleEnum.admin` users may mutate users or player assignments. Team roles are read-only. +- Non-admin directory responses contain only users sharing at least one team with the requester, and only assignments from those shared teams. They never contain email or unrelated-team information. +- Admin directory responses may contain email, global role, status, and all assignments, but never password, hash, social ID, or authentication secrets. +- Admins may edit first name, last name, global role, status, and player assignments. They may not edit email/password, create users, delete users, or edit legacy frontend code. +- Deactivation is immediate for login and already-issued JWTs, preserves assignments, and must not allow self-deactivation or loss of the last active admin. The same last-admin and self-protection applies to role demotion. +- Admin mutation endpoints use narrow DTOs and server-side authorization. The existing generic user PATCH is not used by the modern frontend. +- Follow strict TDD: add focused failing tests first, confirm the expected failure, then add minimal production code and refactor only while green. + +## Task 1: Backend directory contract and query + +- Add explicit directory/admin summary DTOs and pagination/search inputs. +- Implement a user-directory service query that deduplicates users, scopes non-admin results to shared teams, filters their visible assignments to those teams, and returns all users/assignments plus email and role for admins. +- Add focused service tests covering cross-team isolation, email/secret redaction, inactive visibility, admin visibility, deduplication, search, and pagination. +- Keep response mapping explicit rather than serializing entities. + +## Task 2: Backend admin mutations and authentication enforcement + +- Add global-admin-only endpoints for profile changes, role changes, status changes, player search, assignment, reassignment, and unlinking. +- Use transactions/locking for last-admin protection and assignment changes; reject self-deactivation/self-demotion and loss of the last active admin. +- Ensure password/social login rejects inactive users and JWT validation reloads the current user, rejecting inactive/deleted users and returning the current database role. +- Extend audit event types and record admin actor, target, and action without secrets. +- Add a migration/index metadata for the player foreign keys used by directory queries. +- Add focused controller/service/auth tests for authorization, narrow DTO behavior, status/session enforcement, role safeguards, and assignment conflict behavior. + +## Task 3: Modern frontend API, models, routing, and authorization state + +- Add typed directory/admin API clients matching the backend contract and never call the generic user PATCH. +- Add typed view models for safe directory records, admin details, assignments, filters, and mutation requests. +- Add the protected `/users` route and a visible entry in the existing More screen for every authenticated role. +- Expose the current global role through existing auth state and treat it only as a presentation hint; backend authorization remains authoritative. +- Add focused tests for API URLs/payloads, route protection, role derivation, and navigation visibility. + +## Task 4: Modern frontend user directory and admin interactions + +- Implement a responsive user-directory page in the existing modern frontend design system, using its spacing, typography, colors, controls, and list patterns as the accepted visual reference. +- Provide search, pagination, status, and team/player assignment display with loading, empty, and error states. +- Hide email, role, unrelated-team information, and every mutation control from non-admins. +- For admins, add profile/role editing, activation/deactivation confirmation, player search, assignment, unlink confirmation, and explicit reassignment confirmation naming the current and target users. +- Do not optimistically update security-sensitive state; reload affected data after successful mutations and surface `403`/safeguard errors clearly. +- Add component tests for non-admin/admin rendering, confirmations, successful refresh, error behavior, search, and pagination. + +## Task 5: Integration verification and documentation + +- Run all focused backend tests and the backend build; document unrelated pre-existing full-suite failures separately. +- Run the complete modern frontend test suite and a production/container build that does not require external font inlining. +- Run the backend/frontend locally and verify the directory and primary admin workflow at desktop and mobile widths using the available browser tooling or Playwright fallback. +- Confirm the legacy frontend has no changes and review the complete branch diff for data leakage, authorization bypasses, concurrency errors, and visual regressions. From c3822347461dcfbac80b41edb28193b44b5c36ef Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Fri, 31 Jul 2026 22:12:53 +0200 Subject: [PATCH 02/15] feat: add safe user directory query --- .../admin-user-management/task-1-report.md | 69 ++++++ .../src/users/dto/user-directory-query.dto.ts | 21 ++ .../users/dto/user-directory-response.dto.ts | 40 ++++ .../src/users/users.controller.ts | 13 ++ .../src/users/users.service.spec.ts | 208 ++++++++++++++++++ .../src/users/users.service.ts | 155 +++++++++++++ 6 files changed, 506 insertions(+) create mode 100644 .superpowers/sdd/admin-user-management/task-1-report.md create mode 100644 myteamwallet_backend/src/users/dto/user-directory-query.dto.ts create mode 100644 myteamwallet_backend/src/users/dto/user-directory-response.dto.ts create mode 100644 myteamwallet_backend/src/users/users.service.spec.ts diff --git a/.superpowers/sdd/admin-user-management/task-1-report.md b/.superpowers/sdd/admin-user-management/task-1-report.md new file mode 100644 index 0000000..15233d2 --- /dev/null +++ b/.superpowers/sdd/admin-user-management/task-1-report.md @@ -0,0 +1,69 @@ +# Task 1 implementation report: backend directory contract and query + +## Files changed + +- `myteamwallet_backend/src/users/dto/user-directory-query.dto.ts` — page, limit, and optional search input validation. +- `myteamwallet_backend/src/users/dto/user-directory-response.dto.ts` — explicit safe directory, admin, assignment, team, and reference response DTOs. +- `myteamwallet_backend/src/users/users.service.ts` — scoped directory query, search, pagination, deduplication, and explicit entity-to-DTO mapping. +- `myteamwallet_backend/src/users/users.controller.ts` — authenticated `GET /api/v1/users/directory` endpoint, declared before `:id`. +- `myteamwallet_backend/src/users/users.service.spec.ts` — focused contract coverage. + +## RED test evidence + +Command: + +```powershell +npm test -- users/users.service.spec.ts --runInBand +``` + +Result: failed as expected, 7/7 tests failed with `TypeError: service.findDirectory is not a function`. This proved the missing directory-query behavior before implementation. + +## GREEN verification + +Commands and results: + +```powershell +npm test -- users/users.service.spec.ts --runInBand +``` + +Passed: 1 suite, 7 tests. Covers cross-team isolation, non-admin email/secret redaction, inactive visibility, admin visibility, deduplication before pagination, search, and pagination metadata. + +```powershell +.\node_modules\.bin\eslint.cmd src\users\users.service.ts src\users\users.controller.ts src\users\users.service.spec.ts src\users\dto\user-directory-query.dto.ts src\users\dto\user-directory-response.dto.ts --max-warnings=0 +``` + +Passed with no warnings or errors. + +```powershell +npm run build +``` + +Passed: Nest build completed successfully. + +```powershell +git diff --check +``` + +Passed with no whitespace errors. + +## Design notes + +- `findDirectory(requester, query)` returns `{ data, page, limit, total, hasNextPage }`. +- A non-admin's shared-team set is derived from their active player assignments. Only users with an assignment in that set are included, and each returned assignment is filtered to that same set. +- Inactive target users and inactive assignments remain visible when their team is shared. +- Admins receive all non-deleted users and every linked player assignment. Their records extend the safe base summary with `email` and the existing `{ id, name }` role shape. +- The query maps selected DTO fields explicitly. It never serializes a `User` or `Player` entity, so passwords, hashes, social IDs, providers, and other authentication fields cannot leak through this endpoint. +- User IDs are ordered before search/pagination for deterministic pages. Users are the primary result set, which guarantees deduplication before pagination even when they have multiple player assignments. + +## Self-review + +- Confirmed `GET directory` is registered before `GET :id`. +- Confirmed non-admin searches only operate after visibility filtering and do not include email. +- Confirmed admin search may include email and admin mapping includes role/status using the backend's existing `{ id, name }` shapes. +- Confirmed an admin with no player assignment is included and an unassigned non-admin is not exposed to other non-admins. +- Confirmed assignment mapping includes team/team-role summary fields only, never its linked user entity. + +## Concerns + +- The service intentionally fetches the user and player directory sets and applies the authorization filter in memory. Task 2's planned foreign-key index work can support a future query-builder optimization without changing this safe response contract. +- The repository-wide Jest suite has documented pre-existing placeholder dependency failures in the SDD ledger; this task verified its focused suite, lint, build, and whitespace check. diff --git a/myteamwallet_backend/src/users/dto/user-directory-query.dto.ts b/myteamwallet_backend/src/users/dto/user-directory-query.dto.ts new file mode 100644 index 0000000..582fda3 --- /dev/null +++ b/myteamwallet_backend/src/users/dto/user-directory-query.dto.ts @@ -0,0 +1,21 @@ +import { Type } from 'class-transformer'; +import { IsInt, IsOptional, IsString, Max, Min } from 'class-validator'; + +export class UserDirectoryQueryDto { + @IsOptional() + @Type(() => Number) + @IsInt() + @Min(1) + page = 1; + + @IsOptional() + @Type(() => Number) + @IsInt() + @Min(1) + @Max(50) + limit = 20; + + @IsOptional() + @IsString() + search?: string; +} diff --git a/myteamwallet_backend/src/users/dto/user-directory-response.dto.ts b/myteamwallet_backend/src/users/dto/user-directory-response.dto.ts new file mode 100644 index 0000000..6f1f107 --- /dev/null +++ b/myteamwallet_backend/src/users/dto/user-directory-response.dto.ts @@ -0,0 +1,40 @@ +export class UserDirectoryReferenceDto { + id: number; + name?: string; +} + +export class UserDirectoryTeamDto { + id: number; + name: string; + alias: string; +} + +export class UserDirectoryAssignmentDto { + id: number; + firstName: string; + lastName: string; + active: boolean; + team: UserDirectoryTeamDto; + teamRole: UserDirectoryReferenceDto | null; +} + +export class UserDirectorySummaryDto { + id: number; + firstName: string | null; + lastName: string | null; + status: UserDirectoryReferenceDto | null; + assignments: UserDirectoryAssignmentDto[]; +} + +export class AdminUserDirectorySummaryDto extends UserDirectorySummaryDto { + email: string | null; + role: UserDirectoryReferenceDto | null; +} + +export class UserDirectoryPageDto { + data: Array; + page: number; + limit: number; + total: number; + hasNextPage: boolean; +} diff --git a/myteamwallet_backend/src/users/users.controller.ts b/myteamwallet_backend/src/users/users.controller.ts index 57f4f54..2a40f30 100644 --- a/myteamwallet_backend/src/users/users.controller.ts +++ b/myteamwallet_backend/src/users/users.controller.ts @@ -12,6 +12,7 @@ import { ParseIntPipe, HttpStatus, HttpCode, + Request, } from '@nestjs/common'; import { UsersService } from './users.service'; import { CreateUserDto } from './dto/create-user.dto'; @@ -22,6 +23,8 @@ import { RoleEnum } from 'src/roles/roles.enum'; import { AuthGuard } from '@nestjs/passport'; import { RolesGuard } from 'src/roles/roles.guard'; import { infinityPagination } from 'src/utils/infinity-pagination'; +import { UserDirectoryQueryDto } from './dto/user-directory-query.dto'; +import { User } from './entities/user.entity'; @ApiBearerAuth() @UseGuards(AuthGuard('jwt'), RolesGuard) @@ -60,6 +63,16 @@ export class UsersController { ); } + @Roles([RoleEnum.user, RoleEnum.admin]) + @Get('directory') + @HttpCode(HttpStatus.OK) + findDirectory( + @Request() request: { user: Pick }, + @Query() query: UserDirectoryQueryDto, + ) { + return this.usersService.findDirectory(request.user, query); + } + @Roles([RoleEnum.admin]) @Get(':id') @HttpCode(HttpStatus.OK) diff --git a/myteamwallet_backend/src/users/users.service.spec.ts b/myteamwallet_backend/src/users/users.service.spec.ts new file mode 100644 index 0000000..b039258 --- /dev/null +++ b/myteamwallet_backend/src/users/users.service.spec.ts @@ -0,0 +1,208 @@ +import { RoleEnum } from '../roles/roles.enum'; +import { StatusEnum } from '../statuses/statuses.enum'; +import { UsersService } from './users.service'; + +describe('UsersService directory', () => { + const teamA = { id: 10, name: 'Alpha', alias: 'alpha' }; + const teamB = { id: 20, name: 'Bravo', alias: 'bravo' }; + const playerRole = { id: 1, name: 'Player' }; + + const users = [ + user(1, 'Riley', 'Reader', 'reader@example.com'), + user(2, 'Emma', 'Shared', 'emma@example.com'), + user(3, 'Iva', 'Inactive', 'iva@example.com', StatusEnum.inactive), + user(4, 'Otis', 'Outside', 'otis@example.com'), + user(5, 'Morgan', 'Multiple', 'morgan@example.com'), + user( + 6, + 'Ada', + 'Admin', + 'admin@example.com', + StatusEnum.active, + RoleEnum.admin, + ), + ]; + + const players = [ + assignment(101, users[0], teamA), + assignment(201, users[1], teamA), + assignment(301, users[2], teamA, false), + assignment(401, users[3], teamB), + assignment(501, users[4], teamA), + assignment(502, users[4], teamB), + ]; + + const usersRepository = { find: jest.fn() }; + const playersRepository = { find: jest.fn() }; + let service: UsersService; + + beforeEach(() => { + jest.resetAllMocks(); + usersRepository.find.mockResolvedValue(users); + playersRepository.find.mockResolvedValue(players); + service = new UsersService( + usersRepository as any, + playersRepository as any, + ); + }); + + it('hides users and assignments from teams that the requester does not share', async () => { + const result = await directoryFor(users[0]); + + expect(result.data.map((entry) => entry.id)).toEqual([1, 2, 3, 5]); + expect(result.data.find((entry) => entry.id === 4)).toBeUndefined(); + expect(result.data.find((entry) => entry.id === 5).assignments).toEqual([ + assignmentSummary(501, 'Morgan', 'Multiple', true, teamA), + ]); + }); + + it('redacts email and authentication secrets for a non-admin requester', async () => { + const result = await directoryFor(users[0]); + const entry = result.data.find((candidate) => candidate.id === 2); + + expect(entry).toEqual({ + id: 2, + firstName: 'Emma', + lastName: 'Shared', + status: { id: StatusEnum.active, name: 'Active' }, + assignments: [assignmentSummary(201, 'Emma', 'Shared', true, teamA)], + }); + expect(entry).not.toHaveProperty('email'); + expect(entry).not.toHaveProperty('password'); + expect(entry).not.toHaveProperty('hash'); + expect(entry).not.toHaveProperty('socialId'); + }); + + it('keeps inactive users and inactive assignments visible in shared teams', async () => { + const result = await directoryFor(users[0]); + + expect(result.data.find((entry) => entry.id === 3)).toEqual({ + id: 3, + firstName: 'Iva', + lastName: 'Inactive', + status: { id: StatusEnum.inactive, name: 'Inactive' }, + assignments: [assignmentSummary(301, 'Iva', 'Inactive', false, teamA)], + }); + }); + + it('returns every user and assignment with email and role for an admin requester', async () => { + const result = await directoryFor(users[5]); + const multiple = result.data.find((entry) => entry.id === 5); + const outsider = result.data.find((entry) => entry.id === 4); + + expect(result.data).toHaveLength(6); + expect(outsider).toMatchObject({ + email: 'otis@example.com', + role: { id: RoleEnum.user, name: 'User' }, + }); + expect(multiple.assignments).toEqual([ + assignmentSummary(501, 'Morgan', 'Multiple', true, teamA), + assignmentSummary(502, 'Morgan', 'Multiple', true, teamB), + ]); + expect(multiple).not.toHaveProperty('password'); + expect(multiple).not.toHaveProperty('hash'); + expect(multiple).not.toHaveProperty('socialId'); + }); + + it('deduplicates a user with assignments in more than one shared team before pagination', async () => { + const result = await directoryFor(users[0], { page: 2, limit: 2 }); + + expect(result.data.map((entry) => entry.id)).toEqual([3, 5]); + expect(result.total).toBe(4); + expect(result.hasNextPage).toBe(false); + }); + + it('searches visible names case-insensitively without exposing outside-team users', async () => { + const matched = await directoryFor(users[0], { search: 'mOrGaN' }); + const hidden = await directoryFor(users[0], { search: 'outside' }); + + expect(matched.data.map((entry) => entry.id)).toEqual([5]); + expect(hidden.data).toEqual([]); + }); + + it('paginates the deduplicated, filtered directory and reports the next page', async () => { + const result = await directoryFor(users[0], { page: 1, limit: 2 }); + + expect(result).toMatchObject({ + page: 1, + limit: 2, + total: 4, + hasNextPage: true, + }); + expect(result.data.map((entry) => entry.id)).toEqual([1, 2]); + }); + + function directoryFor( + requester: typeof users[number], + query: { page?: number; limit?: number; search?: string } = {}, + ) { + return (service as any).findDirectory(requester, { + page: 1, + limit: 20, + ...query, + }); + } + + function user( + id: number, + firstName: string, + lastName: string, + email: string, + statusId = StatusEnum.active, + roleId = RoleEnum.user, + ) { + return { + id, + firstName, + lastName, + email, + password: `password-${id}`, + hash: `hash-${id}`, + socialId: `social-${id}`, + provider: 'email', + previousPassword: `previous-password-${id}`, + status: { + id: statusId, + name: statusId === StatusEnum.active ? 'Active' : 'Inactive', + }, + role: { + id: roleId, + name: roleId === RoleEnum.admin ? 'Admin' : 'User', + }, + }; + } + + function assignment( + id: number, + playerUser: typeof users[number], + team: typeof teamA, + active = true, + ) { + return { + id, + firstName: playerUser.firstName, + lastName: playerUser.lastName, + active, + user: playerUser, + team, + teamRole: playerRole, + }; + } + + function assignmentSummary( + id: number, + firstName: string, + lastName: string, + active: boolean, + team: typeof teamA, + ) { + return { + id, + firstName, + lastName, + active, + team, + teamRole: playerRole, + }; + } +}); diff --git a/myteamwallet_backend/src/users/users.service.ts b/myteamwallet_backend/src/users/users.service.ts index e7ddc52..2060398 100644 --- a/myteamwallet_backend/src/users/users.service.ts +++ b/myteamwallet_backend/src/users/users.service.ts @@ -4,8 +4,18 @@ import { Player } from 'src/players/entities/player.entity'; import { EntityCondition } from 'src/utils/types/entity-condition.type'; import { IPaginationOptions } from 'src/utils/types/pagination-options'; import { Repository } from 'typeorm'; +import { RoleEnum } from '../roles/roles.enum'; import { CreateUserDto } from './dto/create-user.dto'; import { UpdateUserDto } from './dto/update-user.dto'; +import { UserDirectoryQueryDto } from './dto/user-directory-query.dto'; +import { + AdminUserDirectorySummaryDto, + UserDirectoryAssignmentDto, + UserDirectoryPageDto, + UserDirectoryReferenceDto, + UserDirectorySummaryDto, + UserDirectoryTeamDto, +} from './dto/user-directory-response.dto'; import { User } from './entities/user.entity'; @Injectable() @@ -30,6 +40,66 @@ export class UsersService { }); } + async findDirectory( + requester: Pick, + query: UserDirectoryQueryDto, + ): Promise { + const [users, players] = await Promise.all([ + this.usersRepository.find({ order: { id: 'ASC' } }), + this.playersRepository.find({ + relations: ['user', 'team', 'teamRole'], + order: { id: 'ASC' }, + }), + ]); + const isAdmin = requester.role?.id === RoleEnum.admin; + const sharedTeamIds = new Set( + players + .filter((player) => player.user?.id === requester.id && player.active) + .map((player) => player.team.id), + ); + const assignmentsByUserId = new Map(); + + for (const player of players) { + if (!player.user || (!isAdmin && !sharedTeamIds.has(player.team.id))) { + continue; + } + + const assignments = assignmentsByUserId.get(player.user.id) ?? []; + assignments.push(player); + assignmentsByUserId.set(player.user.id, assignments); + } + + const visibleUsers = users.filter( + (user) => isAdmin || assignmentsByUserId.has(user.id), + ); + const searchedUsers = this.filterDirectorySearch( + visibleUsers, + assignmentsByUserId, + query.search, + isAdmin, + ); + const total = searchedUsers.length; + const page = query.page ?? 1; + const limit = query.limit ?? 20; + const data = searchedUsers + .slice((page - 1) * limit, page * limit) + .map((user) => + this.mapDirectoryUser( + user, + assignmentsByUserId.get(user.id) ?? [], + isAdmin, + ), + ); + + return { + data, + page, + limit, + total, + hasNextPage: page * limit < total, + }; + } + findOne(fields: EntityCondition) { return this.usersRepository.findOne({ where: fields, @@ -77,4 +147,89 @@ export class UsersService { return resolve(true); }); } + + private filterDirectorySearch( + users: User[], + assignmentsByUserId: Map, + search: string | undefined, + includeEmail: boolean, + ): User[] { + const term = search?.trim().toLocaleLowerCase(); + if (!term) { + return users; + } + + return users.filter((user) => { + const assignments = assignmentsByUserId.get(user.id) ?? []; + const values = [ + user.firstName, + user.lastName, + ...(includeEmail ? [user.email] : []), + ...assignments.flatMap((assignment) => [ + assignment.firstName, + assignment.lastName, + ]), + ]; + + return values.some((value) => value?.toLocaleLowerCase().includes(term)); + }); + } + + private mapDirectoryUser( + user: User, + assignments: Player[], + includeAdminFields: boolean, + ): UserDirectorySummaryDto | AdminUserDirectorySummaryDto { + const summary: UserDirectorySummaryDto = { + id: user.id, + firstName: user.firstName, + lastName: user.lastName, + status: this.mapDirectoryReference(user.status), + assignments: assignments + .sort((left, right) => left.id - right.id) + .map((assignment) => this.mapDirectoryAssignment(assignment)), + }; + + if (!includeAdminFields) { + return summary; + } + + return { + ...summary, + email: user.email, + role: this.mapDirectoryReference(user.role), + }; + } + + private mapDirectoryAssignment(player: Player): UserDirectoryAssignmentDto { + return { + id: player.id, + firstName: player.firstName, + lastName: player.lastName, + active: player.active, + team: this.mapDirectoryTeam(player.team), + teamRole: this.mapDirectoryReference(player.teamRole), + }; + } + + private mapDirectoryTeam(team: Player['team']): UserDirectoryTeamDto { + return { + id: team.id, + name: team.name, + alias: team.alias, + }; + } + + private mapDirectoryReference( + reference: { id: number; name?: string } | null | undefined, + ): UserDirectoryReferenceDto | null { + if (!reference) { + return null; + } + + return { + id: reference.id, + name: reference.name, + }; + } } From d738b49cbffb1a9c86fe5e4e5cd85c281c293348 Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Fri, 31 Jul 2026 22:29:15 +0200 Subject: [PATCH 03/15] fix: query user directory safely --- .../admin-user-management/task-1-report.md | 52 ++- .../src/users/users.service.spec.ts | 179 ++++++++++- .../src/users/users.service.ts | 296 ++++++++++++------ 3 files changed, 428 insertions(+), 99 deletions(-) diff --git a/.superpowers/sdd/admin-user-management/task-1-report.md b/.superpowers/sdd/admin-user-management/task-1-report.md index 15233d2..2b8b06c 100644 --- a/.superpowers/sdd/admin-user-management/task-1-report.md +++ b/.superpowers/sdd/admin-user-management/task-1-report.md @@ -65,5 +65,55 @@ Passed with no whitespace errors. ## Concerns -- The service intentionally fetches the user and player directory sets and applies the authorization filter in memory. Task 2's planned foreign-key index work can support a future query-builder optimization without changing this safe response contract. - The repository-wide Jest suite has documented pre-existing placeholder dependency failures in the SDD ledger; this task verified its focused suite, lint, build, and whitespace check. + +## Fix Round 1 + +### Files changed + +- `myteamwallet_backend/src/users/users.service.ts` — replaces whole-entity loading with database-side raw projections for visibility, search, distinct count, deterministic ordering, pagination, and assignment filtering. +- `myteamwallet_backend/src/users/users.service.spec.ts` — adds the inactive-requester regression and runs the directory contract against query-builder doubles that reject entity hydration and unsafe projected authentication fields. + +### RED evidence + +Test file: `myteamwallet_backend/src/users/users.service.spec.ts` + +Command: + +```powershell +npm test -- users/users.service.spec.ts --runInBand +``` + +Result: failed as expected with 2 failures. `treats an inactive requester assignment as a shared team membership` received `[]` instead of `[1, 2]`; `does not hydrate whole user entities for the directory` rejected with `directory queries must use a safe database projection` because the old code called `usersRepository.find`. + +### GREEN verification + +```powershell +npm test -- users/users.service.spec.ts --runInBand +``` + +Passed: 1 suite, 9 tests. + +```powershell +.\node_modules\.bin\eslint.cmd src\users\users.service.ts src\users\users.service.spec.ts --max-warnings=0 +``` + +Passed with no warnings or errors. + +```powershell +npm run build +``` + +Passed: Nest build completed successfully. + +```powershell +git diff --check +``` + +Passed with no whitespace errors. + +### Implementation notes + +- Shared-team membership now uses every requester `Player` row, including inactive ones, exactly as required by the directory plan. +- The user query joins only `status` and `role`, projects safe raw columns, applies shared-team visibility/search in SQL, counts `DISTINCT user.id`, orders by `user.id`, and applies offset/limit before mapping. +- Assignment rows are fetched only for the selected page of user IDs and are scoped with the same shared-team subquery for non-admins. No directory query selects or hydrates `User` authentication columns. diff --git a/myteamwallet_backend/src/users/users.service.spec.ts b/myteamwallet_backend/src/users/users.service.spec.ts index b039258..8592345 100644 --- a/myteamwallet_backend/src/users/users.service.spec.ts +++ b/myteamwallet_backend/src/users/users.service.spec.ts @@ -32,14 +32,33 @@ describe('UsersService directory', () => { assignment(502, users[4], teamB), ]; - const usersRepository = { find: jest.fn() }; - const playersRepository = { find: jest.fn() }; + let userRows: ReturnType[]; + let assignmentRows: ReturnType[]; + let total: number; + const usersRepository = { + find: jest.fn(() => { + throw new Error('directory queries must use a safe database projection'); + }), + createQueryBuilder: jest.fn(() => createDirectoryUsersQuery()), + }; + const playersRepository = { + find: jest.fn(() => { + throw new Error('directory queries must use a safe database projection'); + }), + createQueryBuilder: jest.fn((alias: string) => + alias === 'requesterPlayer' + ? createSharedTeamsQuery() + : createDirectoryAssignmentsQuery(), + ), + }; let service: UsersService; beforeEach(() => { - jest.resetAllMocks(); - usersRepository.find.mockResolvedValue(users); - playersRepository.find.mockResolvedValue(players); + jest.clearAllMocks(); + setDirectoryResult( + [users[0], users[1], users[2], users[4]], + [players[0], players[1], players[2], players[4]], + ); service = new UsersService( usersRepository as any, playersRepository as any, @@ -85,7 +104,34 @@ describe('UsersService directory', () => { }); }); + it('treats an inactive requester assignment as a shared team membership', async () => { + setDirectoryResult( + [users[0], users[1]], + [ + assignment(101, users[0], teamA, false), + assignment(201, users[1], teamA), + ], + ); + + const result = await directoryFor(users[0]); + + expect(result.data.map((entry) => entry.id)).toEqual([1, 2]); + }); + + it('does not hydrate whole user entities for the directory', async () => { + const result = await directoryFor(users[0]); + + expect( + result.data.every( + (entry) => + !('password' in entry || 'hash' in entry || 'socialId' in entry), + ), + ).toBe(true); + }); + it('returns every user and assignment with email and role for an admin requester', async () => { + setDirectoryResult(users, players, true); + const result = await directoryFor(users[5]); const multiple = result.data.find((entry) => entry.id === 5); const outsider = result.data.find((entry) => entry.id === 4); @@ -105,6 +151,13 @@ describe('UsersService directory', () => { }); it('deduplicates a user with assignments in more than one shared team before pagination', async () => { + setDirectoryResult( + [users[2], users[4]], + [players[2], players[4]], + false, + 4, + ); + const result = await directoryFor(users[0], { page: 2, limit: 2 }); expect(result.data.map((entry) => entry.id)).toEqual([3, 5]); @@ -113,7 +166,9 @@ describe('UsersService directory', () => { }); it('searches visible names case-insensitively without exposing outside-team users', async () => { + setDirectoryResult([users[4]], [players[4]]); const matched = await directoryFor(users[0], { search: 'mOrGaN' }); + setDirectoryResult([], []); const hidden = await directoryFor(users[0], { search: 'outside' }); expect(matched.data.map((entry) => entry.id)).toEqual([5]); @@ -121,6 +176,13 @@ describe('UsersService directory', () => { }); it('paginates the deduplicated, filtered directory and reports the next page', async () => { + setDirectoryResult( + [users[0], users[1]], + [players[0], players[1]], + false, + 4, + ); + const result = await directoryFor(users[0], { page: 1, limit: 2 }); expect(result).toMatchObject({ @@ -205,4 +267,111 @@ describe('UsersService directory', () => { teamRole: playerRole, }; } + + function setDirectoryResult( + directoryUsers: typeof users, + directoryPlayers: typeof players, + isAdmin = false, + directoryTotal = directoryUsers.length, + ) { + userRows = directoryUsers.map((directoryUser) => + directoryUserRow(directoryUser, isAdmin), + ); + assignmentRows = directoryPlayers.map(directoryAssignmentRow); + total = directoryTotal; + } + + function directoryUserRow( + directoryUser: typeof users[number], + isAdmin = false, + ) { + return { + user_id: directoryUser.id, + first_name: directoryUser.firstName, + last_name: directoryUser.lastName, + status_id: directoryUser.status.id, + status_name: directoryUser.status.name, + ...(isAdmin + ? { + email: directoryUser.email, + role_id: directoryUser.role.id, + role_name: directoryUser.role.name, + } + : {}), + }; + } + + function directoryAssignmentRow(directoryPlayer: typeof players[number]) { + return { + player_id: directoryPlayer.id, + user_id: directoryPlayer.user.id, + first_name: directoryPlayer.firstName, + last_name: directoryPlayer.lastName, + active: directoryPlayer.active, + team_id: directoryPlayer.team.id, + team_name: directoryPlayer.team.name, + team_alias: directoryPlayer.team.alias, + team_role_id: directoryPlayer.teamRole.id, + team_role_name: directoryPlayer.teamRole.name, + }; + } + + function createDirectoryUsersQuery() { + const query: any = { + leftJoin: () => query, + innerJoin: () => query, + setParameters: () => query, + setParameter: () => query, + andWhere: () => query, + distinct: () => query, + orderBy: () => query, + offset: () => query, + limit: () => query, + select: (selection: string | string[]) => { + const selected = Array.isArray(selection) ? selection : [selection]; + if (selected.join(' ').match(/password|hash|socialId|provider/i)) { + throw new Error( + 'directory projection contains an authentication field', + ); + } + return query; + }, + clone: () => createDirectoryCountQuery(), + getRawMany: () => Promise.resolve(userRows), + }; + return query; + } + + function createDirectoryCountQuery() { + const query: any = { + select: () => query, + getRawOne: () => Promise.resolve({ total: String(total) }), + }; + return query; + } + + function createSharedTeamsQuery() { + const query: any = { + select: () => query, + where: () => query, + getQuery: () => + 'SELECT requesterPlayer.teamId FROM player requesterPlayer', + getParameters: () => ({ requesterId: users[0].id }), + }; + return query; + } + + function createDirectoryAssignmentsQuery() { + const query: any = { + innerJoin: () => query, + leftJoin: () => query, + select: () => query, + where: () => query, + andWhere: () => query, + setParameters: () => query, + orderBy: () => query, + getRawMany: () => Promise.resolve(assignmentRows), + }; + return query; + } }); diff --git a/myteamwallet_backend/src/users/users.service.ts b/myteamwallet_backend/src/users/users.service.ts index 2060398..4197060 100644 --- a/myteamwallet_backend/src/users/users.service.ts +++ b/myteamwallet_backend/src/users/users.service.ts @@ -3,7 +3,7 @@ import { InjectRepository } from '@nestjs/typeorm'; import { Player } from 'src/players/entities/player.entity'; import { EntityCondition } from 'src/utils/types/entity-condition.type'; import { IPaginationOptions } from 'src/utils/types/pagination-options'; -import { Repository } from 'typeorm'; +import { Brackets, Repository, SelectQueryBuilder } from 'typeorm'; import { RoleEnum } from '../roles/roles.enum'; import { CreateUserDto } from './dto/create-user.dto'; import { UpdateUserDto } from './dto/update-user.dto'; @@ -14,7 +14,6 @@ import { UserDirectoryPageDto, UserDirectoryReferenceDto, UserDirectorySummaryDto, - UserDirectoryTeamDto, } from './dto/user-directory-response.dto'; import { User } from './entities/user.entity'; @@ -44,52 +43,47 @@ export class UsersService { requester: Pick, query: UserDirectoryQueryDto, ): Promise { - const [users, players] = await Promise.all([ - this.usersRepository.find({ order: { id: 'ASC' } }), - this.playersRepository.find({ - relations: ['user', 'team', 'teamRole'], - order: { id: 'ASC' }, - }), - ]); const isAdmin = requester.role?.id === RoleEnum.admin; - const sharedTeamIds = new Set( - players - .filter((player) => player.user?.id === requester.id && player.active) - .map((player) => player.team.id), - ); - const assignmentsByUserId = new Map(); - - for (const player of players) { - if (!player.user || (!isAdmin && !sharedTeamIds.has(player.team.id))) { - continue; - } - - const assignments = assignmentsByUserId.get(player.user.id) ?? []; - assignments.push(player); - assignmentsByUserId.set(player.user.id, assignments); - } - - const visibleUsers = users.filter( - (user) => isAdmin || assignmentsByUserId.has(user.id), - ); - const searchedUsers = this.filterDirectorySearch( - visibleUsers, - assignmentsByUserId, - query.search, - isAdmin, - ); - const total = searchedUsers.length; const page = query.page ?? 1; const limit = query.limit ?? 20; - const data = searchedUsers - .slice((page - 1) * limit, page * limit) - .map((user) => - this.mapDirectoryUser( - user, - assignmentsByUserId.get(user.id) ?? [], - isAdmin, - ), - ); + const sharedTeamsQuery = isAdmin + ? undefined + : this.createSharedTeamsQuery(requester.id); + const directoryQuery = this.createDirectoryUsersQuery( + isAdmin, + sharedTeamsQuery, + query.search, + ); + const totalResult = await directoryQuery + .clone() + .select('COUNT(DISTINCT user.id)', 'total') + .getRawOne<{ total: string }>(); + const userRows = await directoryQuery + .select(this.directoryUserSelects(isAdmin)) + .distinct(true) + .orderBy('user.id', 'ASC') + .offset((page - 1) * limit) + .limit(limit) + .getRawMany(); + const assignments = await this.findDirectoryAssignments( + userRows.map((user) => Number(user.user_id)), + isAdmin, + sharedTeamsQuery, + ); + const assignmentsByUserId = assignments.reduce((byUserId, assignment) => { + const userAssignments = byUserId.get(Number(assignment.user_id)) ?? []; + userAssignments.push(assignment); + byUserId.set(Number(assignment.user_id), userAssignments); + return byUserId; + }, new Map()); + const total = Number(totalResult?.total ?? 0); + const data = userRows.map((user) => + this.mapDirectoryUser( + user, + assignmentsByUserId.get(Number(user.user_id)) ?? [], + isAdmin, + ), + ); return { data, @@ -148,46 +142,132 @@ export class UsersService { }); } - private filterDirectorySearch( - users: User[], - assignmentsByUserId: Map, + private createSharedTeamsQuery(requesterId: number) { + return this.playersRepository + .createQueryBuilder('requesterPlayer') + .select('requesterPlayer.teamId') + .where('requesterPlayer.userId = :requesterId', { requesterId }); + } + + private createDirectoryUsersQuery( + isAdmin: boolean, + sharedTeamsQuery: SelectQueryBuilder | undefined, search: string | undefined, - includeEmail: boolean, - ): User[] { - const term = search?.trim().toLocaleLowerCase(); - if (!term) { - return users; + ) { + const query = this.usersRepository + .createQueryBuilder('user') + .leftJoin('user.status', 'status') + .leftJoin('user.role', 'role'); + + if (!isAdmin && sharedTeamsQuery) { + query + .innerJoin( + 'user.players', + 'visibleAssignment', + `visibleAssignment.teamId IN (${sharedTeamsQuery.getQuery()})`, + ) + .setParameters(sharedTeamsQuery.getParameters()); } - return users.filter((user) => { - const assignments = assignmentsByUserId.get(user.id) ?? []; - const values = [ - user.firstName, - user.lastName, - ...(includeEmail ? [user.email] : []), - ...assignments.flatMap((assignment) => [ - assignment.firstName, - assignment.lastName, - ]), - ]; + const term = search?.trim().toLocaleLowerCase(); + if (!term) { + return query; + } - return values.some((value) => value?.toLocaleLowerCase().includes(term)); - }); + const assignmentAlias = isAdmin ? 'searchAssignment' : 'visibleAssignment'; + if (isAdmin) { + query.leftJoin('user.players', assignmentAlias); + } + + const searchColumns = [ + 'user.firstName', + 'user.lastName', + `${assignmentAlias}.firstName`, + `${assignmentAlias}.lastName`, + ...(isAdmin ? ['user.email'] : []), + ]; + query.andWhere( + new Brackets((where) => { + searchColumns.forEach((column, index) => { + const expression = `LOWER(${column}) LIKE :directorySearch`; + if (index === 0) { + where.where(expression); + } else { + where.orWhere(expression); + } + }); + }), + ); + return query.setParameter('directorySearch', `%${term}%`); + } + + private directoryUserSelects(isAdmin: boolean): string[] { + return [ + 'user.id AS user_id', + 'user.firstName AS first_name', + 'user.lastName AS last_name', + 'status.id AS status_id', + 'status.name AS status_name', + ...(isAdmin + ? [ + 'user.email AS email', + 'role.id AS role_id', + 'role.name AS role_name', + ] + : []), + ]; + } + + private async findDirectoryAssignments( + userIds: number[], + isAdmin: boolean, + sharedTeamsQuery: SelectQueryBuilder | undefined, + ): Promise { + if (userIds.length === 0) { + return []; + } + + const query = this.playersRepository + .createQueryBuilder('player') + .innerJoin('player.team', 'team') + .leftJoin('player.teamRole', 'teamRole') + .select([ + 'player.id AS player_id', + 'player.userId AS user_id', + 'player.firstName AS first_name', + 'player.lastName AS last_name', + 'player.active AS active', + 'team.id AS team_id', + 'team.name AS team_name', + 'team.alias AS team_alias', + 'teamRole.id AS team_role_id', + 'teamRole.name AS team_role_name', + ]) + .where('player.userId IN (:...userIds)', { userIds }) + .orderBy('player.id', 'ASC'); + + if (!isAdmin && sharedTeamsQuery) { + query + .andWhere(`player.teamId IN (${sharedTeamsQuery.getQuery()})`) + .setParameters(sharedTeamsQuery.getParameters()); + } + + return query.getRawMany(); } private mapDirectoryUser( - user: User, - assignments: Player[], + user: DirectoryUserRow, + assignments: DirectoryAssignmentRow[], includeAdminFields: boolean, ): UserDirectorySummaryDto | AdminUserDirectorySummaryDto { const summary: UserDirectorySummaryDto = { - id: user.id, - firstName: user.firstName, - lastName: user.lastName, - status: this.mapDirectoryReference(user.status), - assignments: assignments - .sort((left, right) => left.id - right.id) - .map((assignment) => this.mapDirectoryAssignment(assignment)), + id: Number(user.user_id), + firstName: user.first_name, + lastName: user.last_name, + status: this.mapDirectoryReference(user.status_id, user.status_name), + assignments: assignments.map((assignment) => + this.mapDirectoryAssignment(assignment), + ), }; if (!includeAdminFields) { @@ -197,39 +277,69 @@ export class UsersService { return { ...summary, email: user.email, - role: this.mapDirectoryReference(user.role), + role: this.mapDirectoryReference(user.role_id, user.role_name), }; } - private mapDirectoryAssignment(player: Player): UserDirectoryAssignmentDto { + private mapDirectoryAssignment( + player: DirectoryAssignmentRow, + ): UserDirectoryAssignmentDto { return { - id: player.id, - firstName: player.firstName, - lastName: player.lastName, - active: player.active, - team: this.mapDirectoryTeam(player.team), - teamRole: this.mapDirectoryReference(player.teamRole), + id: Number(player.player_id), + firstName: player.first_name, + lastName: player.last_name, + active: this.asBoolean(player.active), + team: { + id: Number(player.team_id), + name: player.team_name, + alias: player.team_alias, + }, + teamRole: this.mapDirectoryReference( + player.team_role_id, + player.team_role_name, + ), }; } - private mapDirectoryTeam(team: Player['team']): UserDirectoryTeamDto { - return { - id: team.id, - name: team.name, - alias: team.alias, - }; + private asBoolean(value: boolean | number | string): boolean { + return value === true || value === 1 || value === '1'; } private mapDirectoryReference( - reference: { id: number; name?: string } | null | undefined, + id: number | string | null, + name: string | null, ): UserDirectoryReferenceDto | null { - if (!reference) { + if (id == null) { return null; } return { - id: reference.id, - name: reference.name, + id: Number(id), + name: name ?? undefined, }; } } + +interface DirectoryUserRow { + user_id: number | string; + first_name: string | null; + last_name: string | null; + status_id: number | string | null; + status_name: string | null; + email?: string | null; + role_id?: number | string | null; + role_name?: string | null; +} + +interface DirectoryAssignmentRow { + player_id: number | string; + user_id: number | string; + first_name: string; + last_name: string; + active: boolean | number | string; + team_id: number | string; + team_name: string; + team_alias: string; + team_role_id: number | string | null; + team_role_name: string | null; +} From 41054604006261ac66a5d3572445d7609e60daa3 Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Fri, 31 Jul 2026 22:36:18 +0200 Subject: [PATCH 04/15] test: guard inactive directory membership --- .../admin-user-management/task-1-report.md | 42 +++++++++++++++++++ .../src/users/users.service.spec.ts | 15 ++++++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/.superpowers/sdd/admin-user-management/task-1-report.md b/.superpowers/sdd/admin-user-management/task-1-report.md index 2b8b06c..3220667 100644 --- a/.superpowers/sdd/admin-user-management/task-1-report.md +++ b/.superpowers/sdd/admin-user-management/task-1-report.md @@ -117,3 +117,45 @@ Passed with no whitespace errors. - Shared-team membership now uses every requester `Player` row, including inactive ones, exactly as required by the directory plan. - The user query joins only `status` and `role`, projects safe raw columns, applies shared-team visibility/search in SQL, counts `DISTINCT user.id`, orders by `user.id`, and applies offset/limit before mapping. - Assignment rows are fetched only for the selected page of user IDs and are scoped with the same shared-team subquery for non-admins. No directory query selects or hydrates `User` authentication columns. + +## Fix Round 2 + +### Files changed + +- `myteamwallet_backend/src/users/users.service.spec.ts` — strengthens the inactive-requester regression with a QueryBuilder boundary that rejects `requesterPlayer.active` in the shared-team predicate. + +### RED evidence + +Test file: `myteamwallet_backend/src/users/users.service.spec.ts` + +After installing the boundary guard, the shared-team query was deliberately mutated to add `requesterPlayer.active = :active`. + +```powershell +npm test -- users/users.service.spec.ts --runInBand +``` + +Result: failed as expected, 1/9 tests failed. `treats an inactive requester assignment as a shared team membership` failed with `shared-team membership must not filter inactive requester assignments`. The mutation was then removed; the production query remains user-ID-only. + +### GREEN verification + +```powershell +npm test -- users/users.service.spec.ts --runInBand +``` + +Passed: 1 suite, 9 tests. + +```powershell +.\node_modules\.bin\eslint.cmd src\users\users.service.spec.ts --max-warnings=0 +``` + +Passed with no warnings or errors. + +```powershell +git diff --check +``` + +Passed with no whitespace errors. + +### Implementation notes + +- The test double checks the actual shared-team predicate supplied by the service, rather than returning fixed rows alone. It rejects only for the inactive-requester regression if a predicate references `requesterPlayer.active`, so the test now fails for the realistic authorization regression while preserving the existing output-contract assertions. diff --git a/myteamwallet_backend/src/users/users.service.spec.ts b/myteamwallet_backend/src/users/users.service.spec.ts index 8592345..c5d69f0 100644 --- a/myteamwallet_backend/src/users/users.service.spec.ts +++ b/myteamwallet_backend/src/users/users.service.spec.ts @@ -35,6 +35,7 @@ describe('UsersService directory', () => { let userRows: ReturnType[]; let assignmentRows: ReturnType[]; let total: number; + let rejectActiveSharedTeamPredicate: boolean; const usersRepository = { find: jest.fn(() => { throw new Error('directory queries must use a safe database projection'); @@ -55,6 +56,7 @@ describe('UsersService directory', () => { beforeEach(() => { jest.clearAllMocks(); + rejectActiveSharedTeamPredicate = false; setDirectoryResult( [users[0], users[1], users[2], users[4]], [players[0], players[1], players[2], players[4]], @@ -105,6 +107,7 @@ describe('UsersService directory', () => { }); it('treats an inactive requester assignment as a shared team membership', async () => { + rejectActiveSharedTeamPredicate = true; setDirectoryResult( [users[0], users[1]], [ @@ -353,7 +356,17 @@ describe('UsersService directory', () => { function createSharedTeamsQuery() { const query: any = { select: () => query, - where: () => query, + where: (predicate: string) => { + if ( + rejectActiveSharedTeamPredicate && + /requesterPlayer\.active/i.test(predicate) + ) { + throw new Error( + 'shared-team membership must not filter inactive requester assignments', + ); + } + return query; + }, getQuery: () => 'SELECT requesterPlayer.teamId FROM player requesterPlayer', getParameters: () => ({ requesterId: users[0].id }), From e6acfdcac736a0f2e9ac65b6b6604b6b57028dfc Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Fri, 31 Jul 2026 23:37:54 +0200 Subject: [PATCH 05/15] feat: secure admin user management --- .../admin-user-management/task-2-report.md | 122 ++++++ .../src/auth/auth.controller.spec.ts | 18 + .../src/auth/auth.controller.ts | 11 +- .../src/auth/auth.service.spec.ts | 170 ++++++++ myteamwallet_backend/src/auth/auth.service.ts | 120 +++--- .../src/auth/strategies/jwt.strategy.spec.ts | 67 +++ .../src/auth/strategies/jwt.strategy.ts | 15 +- .../database/logging/logging.service.spec.ts | 29 +- .../src/database/logging/logging.service.ts | 25 +- .../logging/model/logging-event.type.ts | 5 + .../1785517200000-AddPlayerLookupIndexes.ts | 19 + .../migrations/AddPlayerLookupIndexes.spec.ts | 33 ++ .../src/players/entities/player.entity.ts | 7 + .../src/users/admin-users.controller.spec.ts | 118 ++++++ .../src/users/admin-users.controller.ts | 102 +++++ .../src/users/admin-users.service.spec.ts | 330 +++++++++++++++ .../src/users/admin-users.service.ts | 381 ++++++++++++++++++ .../users/dto/admin-player-response.dto.ts | 31 ++ .../src/users/dto/admin-user.dto.ts | 67 +++ .../users/users.controller.security.spec.ts | 11 + .../src/users/users.controller.ts | 56 --- .../src/users/users.module.ts | 9 +- 22 files changed, 1598 insertions(+), 148 deletions(-) create mode 100644 .superpowers/sdd/admin-user-management/task-2-report.md create mode 100644 myteamwallet_backend/src/auth/auth.controller.spec.ts create mode 100644 myteamwallet_backend/src/auth/auth.service.spec.ts create mode 100644 myteamwallet_backend/src/auth/strategies/jwt.strategy.spec.ts create mode 100644 myteamwallet_backend/src/database/migrations/1785517200000-AddPlayerLookupIndexes.ts create mode 100644 myteamwallet_backend/src/database/migrations/AddPlayerLookupIndexes.spec.ts create mode 100644 myteamwallet_backend/src/users/admin-users.controller.spec.ts create mode 100644 myteamwallet_backend/src/users/admin-users.controller.ts create mode 100644 myteamwallet_backend/src/users/admin-users.service.spec.ts create mode 100644 myteamwallet_backend/src/users/admin-users.service.ts create mode 100644 myteamwallet_backend/src/users/dto/admin-player-response.dto.ts create mode 100644 myteamwallet_backend/src/users/dto/admin-user.dto.ts create mode 100644 myteamwallet_backend/src/users/users.controller.security.spec.ts diff --git a/.superpowers/sdd/admin-user-management/task-2-report.md b/.superpowers/sdd/admin-user-management/task-2-report.md new file mode 100644 index 0000000..15921e6 --- /dev/null +++ b/.superpowers/sdd/admin-user-management/task-2-report.md @@ -0,0 +1,122 @@ +# Task 2 Report: Backend admin mutations and authentication enforcement + +## Status + +Implemented and verified on top of Task 1 commit `4105460`. + +## Delivered API + +- Added versioned global-admin controller at `admin/users` (effective path follows the existing global `/api` prefix and URI versioning). +- Added narrow mutations: + - `PATCH admin/users/:id/profile` (`firstName`, `lastName` only) + - `PATCH admin/users/:id/role` (strict numeric `RoleEnum.admin|user` ID) + - `PATCH admin/users/:id/status` (strict numeric `StatusEnum.active|inactive` ID) + - `PUT admin/users/:userId/players/:playerId` + - `DELETE admin/users/:userId/players/:playerId` +- Added `GET admin/users/players` with search, optional `teamId`, `all|assigned|unassigned`, page, and limit. +- Kept `GET users/directory` as the user list source. Removed superseded generic user create/read/update/delete handlers that exposed unsafe/raw shapes or bypassed the narrow mutation safeguards. +- Removed `DELETE auth/me`, which could race with a promotion and bypass last-admin protection. + +## TDD RED evidence + +The following failures were observed before their production implementations: + +1. `npm test -- --runInBand admin-users.controller.spec.ts` + - Failed to compile because `AdminUsersController` and the narrow DTOs did not exist. +2. `npm test -- --runInBand admin-users.service.spec.ts` + - Failed to compile because `AdminUsersService` did not exist. +3. `npm test -- --runInBand auth.service.spec.ts jwt.strategy.spec.ts` + - Inactive password login produced the ordinary password failure, social login issued a token, logs contained email/token values, and `JwtStrategy` accepted only the JWT snapshot. +4. `npm test -- --runInBand AddPlayerLookupIndexes.spec.ts` + - Failed to compile because the reversible lookup-index migration did not exist. +5. `npm test -- --runInBand auth.service.spec.ts auth.controller.spec.ts` + - `GET auth/me` had no JWT guard and the service accepted/refreshed an inactive user. +6. `npm test -- --runInBand users.controller.security.spec.ts` + - Generic `UsersController` mutations were still present and bypassed the new invariants. +7. `npm test -- --runInBand admin-users.service.spec.ts -t "updates only names"` + - The locked user lookup did not use alias-scoped `FOR UPDATE`, exposing a PostgreSQL outer-join runtime failure. +8. `npm test -- --runInBand admin-users.controller.spec.ts -t "reverse-map"` + - Numeric enum reverse-map names such as `"admin"` passed validation. +9. `npm test -- --runInBand logging.service.spec.ts admin-users.service.spec.ts -t "caller transaction manager|updates only names"` + - Audit logging had no transaction-manager support and ran after commit. +10. `npm test -- --runInBand auth.service.spec.ts admin-users.service.spec.ts -t "serializes email confirmation|explicit paginated player"` + - Email confirmation had no row-lock transaction, and a numeric driver boolean was returned as `1` instead of `true`. + +Every production behavior above was added only after the corresponding expected RED was captured. + +## Final GREEN evidence + +- Focused backend tests: + - Command: `npm test -- --runInBand users.service.spec.ts users.controller.security.spec.ts admin-users.controller.spec.ts admin-users.service.spec.ts auth.controller.spec.ts auth.service.spec.ts jwt.strategy.spec.ts logging.service.spec.ts AddPlayerLookupIndexes.spec.ts` + - Result: **9 suites passed, 40 tests passed, 0 failed**. +- Targeted lint across every touched backend TypeScript file: + - Command: direct project ESLint invocation over 21 touched source/spec files. + - Result: **exit 0, no findings**. +- Backend build: + - Command: `npm run build` + - Result: **exit 0**. +- Migration up/down smoke coverage: + - Exact `CREATE INDEX` and reverse-order `DROP INDEX` SQL asserted in `AddPlayerLookupIndexes.spec.ts`. + - TypeORM entity index metadata asserted to match both migration names. +- Diff checks: + - `git diff --check`: **exit 0**. + - Both frontend directories: **no changes**. + +## Security and concurrency design + +- The controller is class-level protected by JWT auth, `RolesGuard`, and `Roles([RoleEnum.admin])`. +- Mutation DTOs are narrow and whitelisted. Role/status accept only strict numeric IDs, avoiding class-validator numeric-enum reverse-map strings. +- Role and status changes execute in transactions and lock the active-admin set in stable user-ID order. This serializes concurrent demotions/deactivations so the last active admin cannot be lost. +- Self-demotion and self-deactivation are rejected inside the locked transaction. +- User row locks use explicit query builders with `FOR UPDATE OF` the user alias. Role/status are loaded with left joins, preserving support for nullable relations without asking PostgreSQL to lock nullable joined rows. +- Deactivation changes only `User.status` and revokes any outstanding confirmation hash; it does not alter `Player.user`. +- Email confirmation locks the same user row and re-checks the hash inside its transaction. This serializes confirmation against administrative deactivation and prevents an old/racing confirmation link from reactivating a deactivated account. +- Assignment and reassignment lock the player row before changing `Player.user`; unlink verifies the locked row is still linked to the requested user. +- All mutation responses are explicit Task 1-compatible admin summaries. Player search uses its own explicit player/team/current-user projection. Password, hash, social ID, and tokens are never mapped. +- Admin audit events contain actor ID in `userId` and target/action IDs in details. Audit insertion uses the same transaction manager as the mutation, so an audit failure rolls back the security-sensitive change. +- Password and social login reject inactive accounts. `JwtStrategy` reloads the non-deleted database user on every request, rejects inactive/missing users, and returns the current database role/status rather than trusting token role claims. +- `GET auth/me` is JWT guarded and independently checks current active status before any refresh behavior. + +## Files + +### Added + +- `src/users/admin-users.controller.ts` +- `src/users/admin-users.controller.spec.ts` +- `src/users/admin-users.service.ts` +- `src/users/admin-users.service.spec.ts` +- `src/users/users.controller.security.spec.ts` +- `src/users/dto/admin-user.dto.ts` +- `src/users/dto/admin-player-response.dto.ts` +- `src/auth/auth.controller.spec.ts` +- `src/auth/auth.service.spec.ts` +- `src/auth/strategies/jwt.strategy.spec.ts` +- `src/database/migrations/1785517200000-AddPlayerLookupIndexes.ts` +- `src/database/migrations/AddPlayerLookupIndexes.spec.ts` + +### Modified + +- `src/users/users.controller.ts` +- `src/users/users.module.ts` +- `src/auth/auth.controller.ts` +- `src/auth/auth.service.ts` +- `src/auth/strategies/jwt.strategy.ts` +- `src/database/logging/logging.service.ts` +- `src/database/logging/logging.service.spec.ts` +- `src/database/logging/model/logging-event.type.ts` +- `src/players/entities/player.entity.ts` + +## Self-review + +- Checked every endpoint for server-side global-admin authorization and removed legacy mutation bypasses. +- Checked response construction for password/hash/social-ID/token leakage. +- Checked role/status races, lock acquisition order, nullable-relation SQL shape, player reassignment ownership, and confirmation/deactivation ordering. +- Checked all touched logging details for email, password, token, hash, or social-ID values. +- Checked migration names against entity metadata and down ordering. +- Confirmed no frontend changes. + +## Concerns / follow-up + +- The lock/concurrency and migration tests are focused unit/SQL-shape tests; no live PostgreSQL instance was available for a two-connection race test or an actual migration run/revert. A database-backed integration test remains advisable before production rollout. +- Removing superseded generic user CRUD/read routes and `DELETE auth/me` is intentionally security-hardening and may affect undocumented external clients. Repository frontend searches showed no use of those removed routes. +- Full unrelated backend test-suite repair was intentionally out of scope; the focused Task 1 + Task 2 suite and backend build are green. diff --git a/myteamwallet_backend/src/auth/auth.controller.spec.ts b/myteamwallet_backend/src/auth/auth.controller.spec.ts new file mode 100644 index 0000000..db87187 --- /dev/null +++ b/myteamwallet_backend/src/auth/auth.controller.spec.ts @@ -0,0 +1,18 @@ +import { GUARDS_METADATA } from '@nestjs/common/constants'; +import { AuthController } from './auth.controller'; + +describe('AuthController session enforcement', () => { + it('protects GET auth/me with JWT validation', () => { + const guards = Reflect.getMetadata( + GUARDS_METADATA, + AuthController.prototype.me, + ); + + expect(guards).toBeDefined(); + expect(guards).toHaveLength(1); + }); + + it('does not expose self-deletion that can race with an admin promotion', () => { + expect(AuthController.prototype).not.toHaveProperty('delete'); + }); +}); diff --git a/myteamwallet_backend/src/auth/auth.controller.ts b/myteamwallet_backend/src/auth/auth.controller.ts index 0da3c2a..59340c5 100644 --- a/myteamwallet_backend/src/auth/auth.controller.ts +++ b/myteamwallet_backend/src/auth/auth.controller.ts @@ -8,7 +8,6 @@ import { Post, UseGuards, Patch, - Delete, UseInterceptors, ClassSerializerInterceptor, SerializeOptions, @@ -81,7 +80,7 @@ export class AuthController { groups: ['exposeProvider'], }) @Get('me') - // @UseGuards(AuthGuard('jwt')) + @UseGuards(AuthGuard('jwt')) @HttpCode(HttpStatus.OK) public me(@Request() request: Request) { return this.service.me(request.headers['authorization']); @@ -95,14 +94,6 @@ export class AuthController { return this.service.update(request.user, userDto); } - @ApiBearerAuth() - @Delete('me') - @UseGuards(AuthGuard('jwt')) - @HttpCode(HttpStatus.OK) - public async delete(@Request() request) { - return this.service.softDelete(request.user); - } - @ApiOperation({ summary: 'Erstellt Registrierungstoken', description: diff --git a/myteamwallet_backend/src/auth/auth.service.spec.ts b/myteamwallet_backend/src/auth/auth.service.spec.ts new file mode 100644 index 0000000..e6845da --- /dev/null +++ b/myteamwallet_backend/src/auth/auth.service.spec.ts @@ -0,0 +1,170 @@ +import { ForbiddenException } from '@nestjs/common'; +import { AuthProvidersEnum } from './auth-providers.enum'; +import { AuthService } from './auth.service'; +import { RoleEnum } from '../roles/roles.enum'; +import { StatusEnum } from '../statuses/statuses.enum'; + +describe('AuthService inactive-user enforcement and safe logging', () => { + let jwtService: any; + let usersService: any; + let logger: any; + let dataSource: any; + let confirmationUser: any; + let lockedUserQuery: any; + let userRepository: any; + let service: AuthService; + + beforeEach(() => { + jwtService = { + sign: jest.fn(() => 'signed-token'), + verify: jest.fn(), + decode: jest.fn(), + }; + usersService = { + findOne: jest.fn(), + update: jest.fn(), + create: jest.fn(), + }; + logger = { info: jest.fn(), debug: jest.fn() }; + confirmationUser = user(StatusEnum.inactive); + confirmationUser.hash = 'confirmation-hash'; + lockedUserQuery = { + where: jest.fn().mockReturnThis(), + setLock: jest.fn().mockReturnThis(), + getOne: jest.fn(() => confirmationUser), + }; + userRepository = { + createQueryBuilder: jest.fn(() => lockedUserQuery), + save: jest.fn((value) => Promise.resolve(value)), + }; + const manager = { getRepository: jest.fn(() => userRepository) }; + dataSource = { + transaction: jest.fn((work) => work(manager)), + }; + service = new AuthService( + jwtService, + usersService, + {} as any, + {} as any, + logger, + dataSource, + ); + }); + + it('rejects password login for an inactive user before issuing a token', async () => { + usersService.findOne.mockResolvedValue(user(StatusEnum.inactive)); + + await expect( + service.validateLogin({ + email: 'inactive@example.com', + password: 'password', + }), + ).rejects.toBeInstanceOf(ForbiddenException); + + expect(jwtService.sign).not.toHaveBeenCalled(); + expect(logger.info).toHaveBeenCalledWith({ + event: 'user_login_fail', + details: 'userId=2 reason=inactive', + userId: 2, + }); + }); + + it('rejects social login when the existing account is inactive', async () => { + const inactive = user(StatusEnum.inactive); + usersService.findOne + .mockResolvedValueOnce(inactive) + .mockResolvedValueOnce(undefined); + + await expect( + service.validateSocialLogin(AuthProvidersEnum.google, { + id: 'social-id', + email: inactive.email, + firstName: 'Inactive', + lastName: 'User', + }), + ).rejects.toBeInstanceOf(ForbiddenException); + + expect(usersService.update).not.toHaveBeenCalled(); + expect(jwtService.sign).not.toHaveBeenCalled(); + }); + + it('never includes an email in an unknown-user login audit event', async () => { + usersService.findOne.mockResolvedValue(undefined); + + await expect( + service.validateLogin({ + email: 'secret@example.com', + password: 'secret-password', + }), + ).rejects.toBeDefined(); + + expect(logger.info).toHaveBeenCalledWith({ + event: 'user_login_fail', + details: 'reason=user_not_found', + userId: -1, + }); + expect(JSON.stringify(logger.info.mock.calls)).not.toContain( + 'secret@example.com', + ); + expect(JSON.stringify(logger.info.mock.calls)).not.toContain( + 'secret-password', + ); + }); + + it('never includes a rejected invite token in logging details', async () => { + jwtService.verify.mockImplementation(() => { + throw new Error('invalid'); + }); + + await expect( + service.getTeamFromInvite('secret-token'), + ).rejects.toBeDefined(); + + expect(logger.info).toHaveBeenCalledWith({ + event: 'user_invite_link_validate_fail', + details: 'invitation validation failed', + userId: 0, + }); + expect(JSON.stringify(logger.info.mock.calls)).not.toContain( + 'secret-token', + ); + }); + + it('does not refresh an inactive user through the me endpoint flow', async () => { + jwtService.verify.mockReturnValue({ id: 2 }); + usersService.findOne.mockResolvedValue(user(StatusEnum.inactive)); + + await expect(service.me('Bearer existing-token')).rejects.toBeInstanceOf( + ForbiddenException, + ); + expect(jwtService.sign).not.toHaveBeenCalled(); + }); + + it('serializes email confirmation on the user row and consumes the hash', async () => { + await service.confirmEmail('confirmation-hash'); + + expect(dataSource.transaction).toHaveBeenCalledTimes(1); + expect(lockedUserQuery.setLock).toHaveBeenCalledWith( + 'pessimistic_write', + undefined, + ['user'], + ); + expect(confirmationUser.status).toEqual({ id: StatusEnum.active }); + expect(confirmationUser.hash).toBeNull(); + expect(userRepository.save).toHaveBeenCalledWith(confirmationUser); + }); + + function user(statusId: StatusEnum) { + return { + id: 2, + email: 'inactive@example.com', + password: 'password-hash', + provider: AuthProvidersEnum.email, + role: { id: RoleEnum.user, name: 'User' }, + status: { + id: statusId, + name: statusId === StatusEnum.active ? 'Active' : 'Inactive', + }, + }; + } +}); diff --git a/myteamwallet_backend/src/auth/auth.service.ts b/myteamwallet_backend/src/auth/auth.service.ts index 4a7ccec..512e125 100644 --- a/myteamwallet_backend/src/auth/auth.service.ts +++ b/myteamwallet_backend/src/auth/auth.service.ts @@ -1,4 +1,10 @@ -import { HttpException, HttpStatus, Injectable } from '@nestjs/common'; +import { + ForbiddenException, + HttpException, + HttpStatus, + Injectable, + UnauthorizedException, +} from '@nestjs/common'; import { JwtService } from '@nestjs/jwt'; import { User } from '../users/entities/user.entity'; import * as bcrypt from 'bcryptjs'; @@ -19,6 +25,7 @@ import { ForgotService } from 'src/forgot/forgot.service'; import { MailService } from 'src/mail/mail.service'; import { CreateInviteDTO } from './dto/create-invite.dto'; import { LoggingService } from 'src/database/logging/logging.service'; +import { DataSource } from 'typeorm'; @Injectable() export class AuthService { @@ -28,6 +35,7 @@ export class AuthService { private forgotService: ForgotService, private mailService: MailService, private logger: LoggingService, + private dataSource: DataSource, ) {} async validateLogin( @@ -40,7 +48,7 @@ export class AuthService { if (!user) { await this.logger.info({ event: 'user_login_fail', - details: `mail not found: ${loginDto.email}`, + details: 'reason=user_not_found', userId: -1, }); throw new HttpException( @@ -54,6 +62,8 @@ export class AuthService { ); } + await this.assertActiveUser(user); + if (user.provider !== AuthProvidersEnum.email) { throw new HttpException( { @@ -79,7 +89,7 @@ export class AuthService { await this.logger.info({ event: 'user_login_success', - details: `logged in: ${loginDto.email}`, + details: `userId=${user.id}`, userId: user.id, }); @@ -87,7 +97,7 @@ export class AuthService { } else { await this.logger.info({ event: 'user_login_fail', - details: `incorrect password for user: ${loginDto.email}`, + details: `userId=${user.id} reason=incorrect_password`, userId: user.id, }); @@ -120,12 +130,14 @@ export class AuthService { }); if (user) { + await this.assertActiveUser(user); if (socialEmail && !userByEmail) { user.email = socialEmail; } await this.usersService.update(user.id, user); } else if (userByEmail) { user = userByEmail; + await this.assertActiveUser(user); } else { const role = plainToClass(Role, { id: RoleEnum.user, @@ -184,7 +196,7 @@ export class AuthService { await this.logger.info({ event: 'user_create', - details: `user created with mail: ${dto.email}`, + details: `userId=${user.id}`, userId: user.id, }); @@ -197,25 +209,26 @@ export class AuthService { } async confirmEmail(hash: string): Promise { - const user = await this.usersService.findOne({ - hash, + await this.dataSource.transaction(async (manager) => { + const repository = manager.getRepository(User); + const user = await repository + .createQueryBuilder('user') + .where('user.hash = :hash', { hash }) + .setLock('pessimistic_write', undefined, ['user']) + .getOne(); + if (!user) { + throw new HttpException( + { + status: HttpStatus.NOT_FOUND, + error: `notFound`, + }, + HttpStatus.NOT_FOUND, + ); + } + user.hash = null; + user.status = plainToClass(Status, { id: StatusEnum.active }); + await repository.save(user); }); - - if (!user) { - throw new HttpException( - { - status: HttpStatus.NOT_FOUND, - error: `notFound`, - }, - HttpStatus.NOT_FOUND, - ); - } - - user.hash = null; - user.status = plainToClass(Status, { - id: StatusEnum.active, - }); - await user.save(); } async forgotPassword(email: string): Promise { @@ -279,40 +292,31 @@ export class AuthService { async me(token: string): Promise { token = token.replace('Bearer ', ''); - let role: any; + let payload: any; + let refreshToken = false; try { - role = this.jwtService.verify(token); - - const u = await this.usersService.findOne({ - id: role.id, - }); - - await this.logger.debug({ - event: 'user_token_verification_success', - details: `Email: ${u.email}`, - userId: u.id, - }); - - return u; - } catch (error) { - const role = this.jwtService.decode(token); - - const user = await this.usersService.findOne({ - id: (role as any).id, - }); + payload = this.jwtService.verify(token); + } catch { + payload = this.jwtService.decode(token); + refreshToken = true; + } + if (!payload?.id) throw new UnauthorizedException(); + const user = await this.usersService.findOne({ id: payload.id }); + if (!user) throw new UnauthorizedException(); + await this.assertActiveUser(user); + if (refreshToken) { const t = await this.jwtService.sign({ id: user.id, role: user.role, }); - user['token'] = t; - await this.logger.debug({ - event: 'user_token_verification_success', - details: `Email: ${user.email}`, - userId: user.id, - }); - return user; } + await this.logger.debug({ + event: 'user_token_verification_success', + details: `userId=${user.id}`, + userId: user.id, + }); + return user; } async update(user: User, userDto: AuthUpdateDto): Promise { @@ -358,10 +362,6 @@ export class AuthService { }); } - async softDelete(user: User): Promise { - await this.usersService.softDelete(user.id); - } - async createTeamInvite(object: CreateInviteDTO) { const token = await this.jwtService.sign(object, { expiresIn: '30d', @@ -392,7 +392,7 @@ export class AuthService { } catch { await this.logger.info({ event: 'user_invite_link_validate_fail', - details: `validation failed for token ${token}`, + details: 'invitation validation failed', userId: 0, }); @@ -402,4 +402,14 @@ export class AuthService { ); } } + + private async assertActiveUser(user: User): Promise { + if (user.status?.id === StatusEnum.active) return; + await this.logger.info({ + event: 'user_login_fail', + details: `userId=${user.id} reason=inactive`, + userId: user.id, + }); + throw new ForbiddenException('User account is inactive'); + } } diff --git a/myteamwallet_backend/src/auth/strategies/jwt.strategy.spec.ts b/myteamwallet_backend/src/auth/strategies/jwt.strategy.spec.ts new file mode 100644 index 0000000..980050e --- /dev/null +++ b/myteamwallet_backend/src/auth/strategies/jwt.strategy.spec.ts @@ -0,0 +1,67 @@ +import { UnauthorizedException } from '@nestjs/common'; +import { RoleEnum } from '../../roles/roles.enum'; +import { StatusEnum } from '../../statuses/statuses.enum'; +import { JwtStrategy } from './jwt.strategy'; + +describe('JwtStrategy', () => { + const jwtService = {} as any; + const configService = { get: jest.fn(() => 'secret') } as any; + let usersService: any; + let strategy: JwtStrategy; + + beforeEach(() => { + usersService = { findOne: jest.fn() }; + strategy = new JwtStrategy(jwtService, configService, usersService); + }); + + it('reloads the current database user and replaces a stale token role', async () => { + usersService.findOne.mockResolvedValue({ + id: 2, + role: { id: RoleEnum.user, name: 'User' }, + status: { id: StatusEnum.active, name: 'Active' }, + password: 'must-not-be-exposed', + }); + + const result = await strategy.validate({ + id: 2, + role: { id: RoleEnum.admin }, + iat: 1, + exp: 2, + } as any); + + expect(usersService.findOne).toHaveBeenCalledWith({ id: 2 }); + expect(result).toEqual({ + id: 2, + role: { id: RoleEnum.user, name: 'User' }, + status: { id: StatusEnum.active, name: 'Active' }, + }); + expect(result).not.toHaveProperty('password'); + }); + + it('rejects a missing or soft-deleted database user', async () => { + usersService.findOne.mockResolvedValue(undefined); + + await expect( + strategy.validate({ id: 2, iat: 1, exp: 2 } as any), + ).rejects.toBeInstanceOf(UnauthorizedException); + }); + + it('rejects an inactive current database user', async () => { + usersService.findOne.mockResolvedValue({ + id: 2, + role: { id: RoleEnum.user }, + status: { id: StatusEnum.inactive }, + }); + + await expect( + strategy.validate({ id: 2, iat: 1, exp: 2 } as any), + ).rejects.toBeInstanceOf(UnauthorizedException); + }); + + it('rejects a payload without an id without querying the database', async () => { + await expect( + strategy.validate({ iat: 1, exp: 2 } as any), + ).rejects.toBeInstanceOf(UnauthorizedException); + expect(usersService.findOne).not.toHaveBeenCalled(); + }); +}); diff --git a/myteamwallet_backend/src/auth/strategies/jwt.strategy.ts b/myteamwallet_backend/src/auth/strategies/jwt.strategy.ts index 26a5c96..84a1a8a 100644 --- a/myteamwallet_backend/src/auth/strategies/jwt.strategy.ts +++ b/myteamwallet_backend/src/auth/strategies/jwt.strategy.ts @@ -4,6 +4,8 @@ import { JwtService } from '@nestjs/jwt'; import { PassportStrategy } from '@nestjs/passport'; import { User } from '../../users/entities/user.entity'; import { ConfigService } from '@nestjs/config'; +import { UsersService } from '../../users/users.service'; +import { StatusEnum } from '../../statuses/statuses.enum'; type JwtPayload = Pick & { iat: number; exp: number }; @@ -12,6 +14,7 @@ export class JwtStrategy extends PassportStrategy(Strategy) { constructor( private jwtService: JwtService, private configService: ConfigService, + private usersService: UsersService, ) { super({ jwtFromRequest: ExtractJwt.fromAuthHeaderAsBearerToken(), @@ -19,10 +22,18 @@ export class JwtStrategy extends PassportStrategy(Strategy) { }); } - public validate(payload: JwtPayload) { + public async validate(payload: JwtPayload) { if (!payload.id) { throw new UnauthorizedException(); } - return payload; + const user = await this.usersService.findOne({ id: payload.id }); + if (!user || user.status?.id !== StatusEnum.active) { + throw new UnauthorizedException(); + } + return { + id: user.id, + role: user.role ?? null, + status: user.status ?? null, + }; } } diff --git a/myteamwallet_backend/src/database/logging/logging.service.spec.ts b/myteamwallet_backend/src/database/logging/logging.service.spec.ts index 35b8391..54ef725 100644 --- a/myteamwallet_backend/src/database/logging/logging.service.spec.ts +++ b/myteamwallet_backend/src/database/logging/logging.service.spec.ts @@ -1,18 +1,25 @@ -import { Test, TestingModule } from '@nestjs/testing'; import { LoggingService } from './logging.service'; describe('LoggingService', () => { - let service: LoggingService; + it('can persist an event through the caller transaction manager', async () => { + const defaultRepository = { save: jest.fn() }; + const transactionRepository = { save: jest.fn() }; + const manager = { + getRepository: jest.fn(() => transactionRepository), + } as any; + const service = new LoggingService(defaultRepository as any); + const event = { + event: 'admin_user_profile_update' as const, + details: 'targetUserId=2', + userId: 1, + }; - beforeEach(async () => { - const module: TestingModule = await Test.createTestingModule({ - providers: [LoggingService], - }).compile(); + await service.info(event, manager); - service = module.get(LoggingService); - }); - - it('should be defined', () => { - expect(service).toBeDefined(); + expect(transactionRepository.save).toHaveBeenCalledWith({ + ...event, + level: 'INFO', + }); + expect(defaultRepository.save).not.toHaveBeenCalled(); }); }); diff --git a/myteamwallet_backend/src/database/logging/logging.service.ts b/myteamwallet_backend/src/database/logging/logging.service.ts index 6febd80..99ae584 100644 --- a/myteamwallet_backend/src/database/logging/logging.service.ts +++ b/myteamwallet_backend/src/database/logging/logging.service.ts @@ -1,6 +1,6 @@ import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; -import { Repository } from 'typeorm'; +import { EntityManager, Repository } from 'typeorm'; import { CreateLogDTO } from './dto/create-log.dto'; import { LogEntry } from './entities/log-entry.entity'; import { LOGEVENT } from './model/logging-event.type'; @@ -23,22 +23,25 @@ export class LoggingService { }); } - async info({ - event, - details, - userId, - }: { - event: LOGEVENT; - details: string; - userId: number; - }) { + async info( + { + event, + details, + userId, + }: { + event: LOGEVENT; + details: string; + userId: number; + }, + manager?: EntityManager, + ) { const e: CreateLogDTO = { event, details, userId, level: 'INFO', }; - await this.repository.save(e); + await (manager?.getRepository(LogEntry) ?? this.repository).save(e); } async error({ diff --git a/myteamwallet_backend/src/database/logging/model/logging-event.type.ts b/myteamwallet_backend/src/database/logging/model/logging-event.type.ts index 08ccfe3..38182d2 100644 --- a/myteamwallet_backend/src/database/logging/model/logging-event.type.ts +++ b/myteamwallet_backend/src/database/logging/model/logging-event.type.ts @@ -15,6 +15,11 @@ export type LOGEVENT = | 'transaction_create_fail' | 'transaction_reverse' | 'player_creation' + | 'admin_user_profile_update' + | 'admin_user_role_update' + | 'admin_user_status_update' + | 'admin_player_assign' + | 'admin_player_unlink' | 'team_create'; export type LOGLEVEL = 'FATAL' | 'ERROR' | 'WARN' | 'INFO' | 'DEBUG' | 'TRACE'; diff --git a/myteamwallet_backend/src/database/migrations/1785517200000-AddPlayerLookupIndexes.ts b/myteamwallet_backend/src/database/migrations/1785517200000-AddPlayerLookupIndexes.ts new file mode 100644 index 0000000..655ca5c --- /dev/null +++ b/myteamwallet_backend/src/database/migrations/1785517200000-AddPlayerLookupIndexes.ts @@ -0,0 +1,19 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; + +export class AddPlayerLookupIndexes1785517200000 implements MigrationInterface { + name = 'AddPlayerLookupIndexes1785517200000'; + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query( + 'CREATE INDEX "IDX_player_team_id" ON "player" ("teamId")', + ); + await queryRunner.query( + 'CREATE INDEX "IDX_player_user_id" ON "player" ("userId")', + ); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query('DROP INDEX "IDX_player_user_id"'); + await queryRunner.query('DROP INDEX "IDX_player_team_id"'); + } +} diff --git a/myteamwallet_backend/src/database/migrations/AddPlayerLookupIndexes.spec.ts b/myteamwallet_backend/src/database/migrations/AddPlayerLookupIndexes.spec.ts new file mode 100644 index 0000000..44ce866 --- /dev/null +++ b/myteamwallet_backend/src/database/migrations/AddPlayerLookupIndexes.spec.ts @@ -0,0 +1,33 @@ +import { getMetadataArgsStorage } from 'typeorm'; +import { Player } from '../../players/entities/player.entity'; +import { AddPlayerLookupIndexes1785517200000 } from './1785517200000-AddPlayerLookupIndexes'; + +describe('AddPlayerLookupIndexes1785517200000', () => { + it('adds reversible indexes for team and user foreign-key lookups', async () => { + const queryRunner = { query: jest.fn() } as any; + const migration = new AddPlayerLookupIndexes1785517200000(); + + await migration.up(queryRunner); + expect(queryRunner.query.mock.calls.map(([sql]) => sql)).toEqual([ + 'CREATE INDEX "IDX_player_team_id" ON "player" ("teamId")', + 'CREATE INDEX "IDX_player_user_id" ON "player" ("userId")', + ]); + + queryRunner.query.mockClear(); + await migration.down(queryRunner); + expect(queryRunner.query.mock.calls.map(([sql]) => sql)).toEqual([ + 'DROP INDEX "IDX_player_user_id"', + 'DROP INDEX "IDX_player_team_id"', + ]); + }); + + it('keeps entity index metadata aligned with the migration', () => { + const playerIndexes = getMetadataArgsStorage() + .indices.filter((index) => index.target === Player) + .map((index) => index.name); + + expect(playerIndexes).toEqual( + expect.arrayContaining(['IDX_player_team_id', 'IDX_player_user_id']), + ); + }); +}); diff --git a/myteamwallet_backend/src/players/entities/player.entity.ts b/myteamwallet_backend/src/players/entities/player.entity.ts index bf0a89b..654072d 100644 --- a/myteamwallet_backend/src/players/entities/player.entity.ts +++ b/myteamwallet_backend/src/players/entities/player.entity.ts @@ -3,9 +3,11 @@ import { BeforeInsert, Column, Entity, + Index, ManyToOne, OneToMany, PrimaryGeneratedColumn, + RelationId, } from 'typeorm'; import { EntityHelper } from 'src/utils/entity-helper'; import { TeamRole } from 'src/team-roles/entities/team-roles.entity'; @@ -29,6 +31,7 @@ export class Player extends EntityHelper { }) teamRole?: TeamRole | null; + @Index('IDX_player_team_id') @ManyToOne(() => Team, { eager: true, }) @@ -37,11 +40,15 @@ export class Player extends EntityHelper { @Column({ type: 'decimal', precision: 10, scale: 2, default: 0 }) balance: number; + @Index('IDX_player_user_id') @ManyToOne(() => User, (user) => user.players, { eager: true, }) user?: User | null; + @RelationId((player: Player) => player.user) + userId?: number | null; + @OneToMany(() => Transaction, (transaction) => transaction.player) transactions: Transaction[]; diff --git a/myteamwallet_backend/src/users/admin-users.controller.spec.ts b/myteamwallet_backend/src/users/admin-users.controller.spec.ts new file mode 100644 index 0000000..203b37d --- /dev/null +++ b/myteamwallet_backend/src/users/admin-users.controller.spec.ts @@ -0,0 +1,118 @@ +import { GUARDS_METADATA, PATH_METADATA } from '@nestjs/common/constants'; +import { validate } from 'class-validator'; +import { plainToInstance } from 'class-transformer'; +import { RoleEnum } from '../roles/roles.enum'; +import { RolesGuard } from '../roles/roles.guard'; +import { AdminUsersController } from './admin-users.controller'; +import { + AdminPlayerQueryDto, + AdminUserProfileDto, + AdminUserRoleDto, + AdminUserStatusDto, +} from './dto/admin-user.dto'; + +describe('AdminUsersController', () => { + const service = { + updateProfile: jest.fn(), + updateRole: jest.fn(), + updateStatus: jest.fn(), + findPlayers: jest.fn(), + assignPlayer: jest.fn(), + unlinkPlayer: jest.fn(), + }; + const controller = new AdminUsersController(service as any); + const actor = { user: { id: 7, role: { id: RoleEnum.admin } } }; + + beforeEach(() => jest.clearAllMocks()); + + it('uses a separate versioned admin/users controller guarded by the global admin role', () => { + expect(Reflect.getMetadata(PATH_METADATA, AdminUsersController)).toBe( + 'admin/users', + ); + expect(Reflect.getMetadata('roles', AdminUsersController)).toEqual([ + RoleEnum.admin, + ]); + expect( + Reflect.getMetadata(GUARDS_METADATA, AdminUsersController), + ).toContain(RolesGuard); + }); + + it('passes the authenticated actor and target to profile, role, and status mutations', async () => { + await controller.updateProfile(actor as any, 2, { + firstName: 'New', + lastName: 'Name', + }); + await controller.updateRole(actor as any, 2, { role: RoleEnum.user }); + await controller.updateStatus(actor as any, 2, { + status: 2, + }); + + expect(service.updateProfile).toHaveBeenCalledWith(7, 2, { + firstName: 'New', + lastName: 'Name', + }); + expect(service.updateRole).toHaveBeenCalledWith(7, 2, RoleEnum.user); + expect(service.updateStatus).toHaveBeenCalledWith(7, 2, 2); + }); + + it('passes player search and atomic assignment operations to the service', async () => { + const query = { assignment: 'unassigned' as const, page: 2, limit: 10 }; + + await controller.findPlayers(query); + await controller.assignPlayer(actor as any, 3, 11); + await controller.unlinkPlayer(actor as any, 3, 11); + + expect(service.findPlayers).toHaveBeenCalledWith(query); + expect(service.assignPlayer).toHaveBeenCalledWith(7, 3, 11); + expect(service.unlinkPlayer).toHaveBeenCalledWith(7, 3, 11); + }); +}); + +describe('admin user DTOs', () => { + it('accepts only profile names in the profile DTO', async () => { + const dto = plainToInstance(AdminUserProfileDto, { + firstName: 'Ada', + lastName: 'Admin', + email: 'must-not-pass@example.com', + role: RoleEnum.user, + status: 2, + }); + + expect(await validate(dto, { whitelist: true })).toEqual([]); + expect(dto).toEqual({ firstName: 'Ada', lastName: 'Admin' }); + }); + + it('rejects role and status ids outside their enums', async () => { + const role = plainToInstance(AdminUserRoleDto, { role: 999 }); + const status = plainToInstance(AdminUserStatusDto, { status: 999 }); + + expect(await validate(role)).not.toEqual([]); + expect(await validate(status)).not.toEqual([]); + }); + + it('rejects numeric-enum reverse-map names instead of sending strings to integer foreign keys', async () => { + const role = plainToInstance(AdminUserRoleDto, { role: 'admin' }); + const status = plainToInstance(AdminUserStatusDto, { status: 'inactive' }); + + expect(await validate(role)).not.toEqual([]); + expect(await validate(status)).not.toEqual([]); + }); + + it('validates player assignment filters and pagination bounds', async () => { + const valid = plainToInstance(AdminPlayerQueryDto, { + assignment: 'assigned', + teamId: '4', + page: '2', + limit: '50', + }); + const invalid = plainToInstance(AdminPlayerQueryDto, { + assignment: 'someone', + page: 0, + limit: 51, + }); + + expect(await validate(valid)).toEqual([]); + expect(valid).toMatchObject({ teamId: 4, page: 2, limit: 50 }); + expect(await validate(invalid)).not.toEqual([]); + }); +}); diff --git a/myteamwallet_backend/src/users/admin-users.controller.ts b/myteamwallet_backend/src/users/admin-users.controller.ts new file mode 100644 index 0000000..2cf59cd --- /dev/null +++ b/myteamwallet_backend/src/users/admin-users.controller.ts @@ -0,0 +1,102 @@ +import { + Body, + Controller, + Delete, + Get, + HttpCode, + HttpStatus, + Param, + ParseIntPipe, + Patch, + Put, + Query, + Request, + UseGuards, +} from '@nestjs/common'; +import { ApiBearerAuth, ApiTags } from '@nestjs/swagger'; +import { AuthGuard } from '@nestjs/passport'; +import { Roles } from '../roles/roles.decorator'; +import { RoleEnum } from '../roles/roles.enum'; +import { RolesGuard } from '../roles/roles.guard'; +import { AdminUsersService } from './admin-users.service'; +import { + AdminPlayerQueryDto, + AdminUserProfileDto, + AdminUserRoleDto, + AdminUserStatusDto, +} from './dto/admin-user.dto'; + +type AdminRequest = { user: { id: number; role: { id: RoleEnum } } }; + +@ApiBearerAuth() +@ApiTags('Admin users') +@UseGuards(AuthGuard('jwt'), RolesGuard) +@Roles([RoleEnum.admin]) +@Controller({ path: 'admin/users', version: '1' }) +export class AdminUsersController { + constructor(private readonly adminUsersService: AdminUsersService) {} + + @Patch(':id/profile') + @HttpCode(HttpStatus.OK) + updateProfile( + @Request() request: AdminRequest, + @Param('id', ParseIntPipe) id: number, + @Body() dto: AdminUserProfileDto, + ) { + return this.adminUsersService.updateProfile(request.user.id, id, dto); + } + + @Patch(':id/role') + @HttpCode(HttpStatus.OK) + updateRole( + @Request() request: AdminRequest, + @Param('id', ParseIntPipe) id: number, + @Body() dto: AdminUserRoleDto, + ) { + return this.adminUsersService.updateRole(request.user.id, id, dto.role); + } + + @Patch(':id/status') + @HttpCode(HttpStatus.OK) + updateStatus( + @Request() request: AdminRequest, + @Param('id', ParseIntPipe) id: number, + @Body() dto: AdminUserStatusDto, + ) { + return this.adminUsersService.updateStatus(request.user.id, id, dto.status); + } + + @Get('players') + @HttpCode(HttpStatus.OK) + findPlayers(@Query() query: AdminPlayerQueryDto) { + return this.adminUsersService.findPlayers(query); + } + + @Put(':userId/players/:playerId') + @HttpCode(HttpStatus.OK) + assignPlayer( + @Request() request: AdminRequest, + @Param('userId', ParseIntPipe) userId: number, + @Param('playerId', ParseIntPipe) playerId: number, + ) { + return this.adminUsersService.assignPlayer( + request.user.id, + userId, + playerId, + ); + } + + @Delete(':userId/players/:playerId') + @HttpCode(HttpStatus.OK) + unlinkPlayer( + @Request() request: AdminRequest, + @Param('userId', ParseIntPipe) userId: number, + @Param('playerId', ParseIntPipe) playerId: number, + ) { + return this.adminUsersService.unlinkPlayer( + request.user.id, + userId, + playerId, + ); + } +} diff --git a/myteamwallet_backend/src/users/admin-users.service.spec.ts b/myteamwallet_backend/src/users/admin-users.service.spec.ts new file mode 100644 index 0000000..ec4460f --- /dev/null +++ b/myteamwallet_backend/src/users/admin-users.service.spec.ts @@ -0,0 +1,330 @@ +import { ConflictException, ForbiddenException } from '@nestjs/common'; +import { RoleEnum } from '../roles/roles.enum'; +import { StatusEnum } from '../statuses/statuses.enum'; +import { AdminUsersService } from './admin-users.service'; + +describe('AdminUsersService', () => { + const team = { id: 10, name: 'Alpha', alias: 'alpha' }; + const teamRole = { id: 1, name: 'Player' }; + const actorId = 1; + let target: any; + let activeAdmins: any[]; + let player: any; + let playerRows: any[]; + let playerTotal: number; + let lockQuery: any; + let lockedUserQuery: any; + let playerQuery: any; + let lockedPlayerQuery: any; + let userRepository: any; + let playerRepository: any; + let manager: any; + let dataSource: any; + let logger: any; + let service: AdminUsersService; + + beforeEach(() => { + target = user(2, RoleEnum.user, StatusEnum.active); + activeAdmins = [user(actorId, RoleEnum.admin, StatusEnum.active)]; + player = assignment(101, null); + playerRows = []; + playerTotal = 0; + lockQuery = chain({ getMany: jest.fn(() => activeAdmins) }); + lockedUserQuery = chain({ getOne: jest.fn(() => target) }); + playerQuery = chain({ + getRawMany: jest.fn(() => playerRows), + getCount: jest.fn(() => playerTotal), + }); + lockedPlayerQuery = chain({ getOne: jest.fn(() => player) }); + userRepository = { + createQueryBuilder: jest.fn((alias: string) => + alias === 'lockedUser' ? lockedUserQuery : lockQuery, + ), + findOne: jest.fn(() => target), + save: jest.fn((value) => Promise.resolve(value)), + }; + playerRepository = { + createQueryBuilder: jest.fn((alias: string) => + alias === 'lockedPlayer' ? lockedPlayerQuery : playerQuery, + ), + findOne: jest.fn(() => player), + save: jest.fn((value) => Promise.resolve(value)), + }; + manager = { + getRepository: jest.fn((entity) => + entity.name === 'User' ? userRepository : playerRepository, + ), + }; + dataSource = { + manager, + transaction: jest.fn((work) => work(manager)), + }; + logger = { info: jest.fn() }; + service = new AdminUsersService(dataSource, logger); + }); + + it('updates only names and returns an explicit Task 1-compatible safe admin summary', async () => { + target.players = [assignment(101, target)]; + + const result = await service.updateProfile(actorId, target.id, { + firstName: 'New', + lastName: 'Name', + }); + + expect(userRepository.save).toHaveBeenCalledWith( + expect.objectContaining({ firstName: 'New', lastName: 'Name' }), + ); + expect(result).toEqual({ + id: 2, + firstName: 'New', + lastName: 'Name', + email: 'user-2@example.com', + role: { id: RoleEnum.user, name: 'User' }, + status: { id: StatusEnum.active, name: 'Active' }, + assignments: [playerSummary(101)], + }); + expect(result).not.toHaveProperty('password'); + expect(result).not.toHaveProperty('hash'); + expect(result).not.toHaveProperty('socialId'); + expect(logger.info).toHaveBeenCalledWith( + { + event: 'admin_user_profile_update', + details: 'targetUserId=2', + userId: actorId, + }, + manager, + ); + expect(lockedUserQuery.setLock).toHaveBeenCalledWith( + 'pessimistic_write', + undefined, + ['lockedUser'], + ); + expect(lockedUserQuery.leftJoinAndSelect).toHaveBeenCalledTimes(2); + }); + + it('rejects self-demotion and self-deactivation before saving', async () => { + target = user(actorId, RoleEnum.admin, StatusEnum.active); + + await expect( + service.updateRole(actorId, actorId, RoleEnum.user), + ).rejects.toBeInstanceOf(ForbiddenException); + await expect( + service.updateStatus(actorId, actorId, StatusEnum.inactive), + ).rejects.toBeInstanceOf(ForbiddenException); + expect(userRepository.save).not.toHaveBeenCalled(); + }); + + it('locks the active-admin set and rejects loss of the last active admin', async () => { + target = user(2, RoleEnum.admin, StatusEnum.active); + activeAdmins = [target]; + + await expect( + service.updateRole(actorId, target.id, RoleEnum.user), + ).rejects.toBeInstanceOf(ConflictException); + + expect(lockQuery.setLock).toHaveBeenCalledWith( + 'pessimistic_write', + undefined, + ['user'], + ); + expect(dataSource.transaction).toHaveBeenCalledTimes(1); + expect(userRepository.save).not.toHaveBeenCalled(); + }); + + it('allows a locked role change when another active admin remains', async () => { + target = user(2, RoleEnum.admin, StatusEnum.active); + activeAdmins = [target, user(3, RoleEnum.admin, StatusEnum.active)]; + + const result = await service.updateRole(actorId, target.id, RoleEnum.user); + + expect(target.role).toEqual({ id: RoleEnum.user }); + expect(result.role).toEqual({ id: RoleEnum.user, name: undefined }); + expect(logger.info).toHaveBeenCalledWith( + { + event: 'admin_user_role_update', + details: 'targetUserId=2 roleId=2', + userId: actorId, + }, + manager, + ); + }); + + it('deactivates under the same lock without changing player assignments', async () => { + target = user(2, RoleEnum.user, StatusEnum.active); + target.players = [assignment(101, target)]; + + const result = await service.updateStatus( + actorId, + target.id, + StatusEnum.inactive, + ); + + expect(target.status).toEqual({ id: StatusEnum.inactive }); + expect(target.hash).toBeNull(); + expect(result.assignments).toEqual([playerSummary(101)]); + expect(playerRepository.save).not.toHaveBeenCalled(); + }); + + it('returns an explicit paginated player projection with safe current-user summaries', async () => { + playerRows = [ + { + player_id: 101, + first_name: 'Pat', + last_name: 'Player', + active: 1, + team_id: 10, + team_name: 'Alpha', + team_alias: 'alpha', + user_id: 2, + user_first_name: 'Target', + user_last_name: 'User', + status_id: StatusEnum.active, + status_name: 'Active', + }, + ]; + playerTotal = 1; + + const result = await service.findPlayers({ + search: 'pat', + teamId: 10, + assignment: 'assigned', + page: 1, + limit: 20, + }); + + expect(result).toEqual({ + data: [ + { + id: 101, + firstName: 'Pat', + lastName: 'Player', + active: true, + team, + currentUser: { + id: 2, + firstName: 'Target', + lastName: 'User', + status: { id: StatusEnum.active, name: 'Active' }, + }, + }, + ], + page: 1, + limit: 20, + total: 1, + hasNextPage: false, + }); + expect(playerQuery.andWhere).toHaveBeenCalledWith( + 'player.teamId = :teamId', + { + teamId: 10, + }, + ); + expect(playerQuery.andWhere).toHaveBeenCalledWith( + 'player.userId IS NOT NULL', + ); + }); + + it('atomically reassigns a locked player and returns the refreshed target summary', async () => { + const previous = user(9, RoleEnum.user, StatusEnum.active); + player = assignment(101, previous); + target.players = [player]; + + const result = await service.assignPlayer(actorId, target.id, player.id); + + expect(dataSource.transaction).toHaveBeenCalledTimes(1); + expect(lockedPlayerQuery.setLock).toHaveBeenCalledWith('pessimistic_write'); + expect(player.user).toBe(target); + expect(playerRepository.save).toHaveBeenCalledWith(player); + expect(result.id).toBe(target.id); + expect(logger.info).toHaveBeenCalledWith( + { + event: 'admin_player_assign', + details: 'targetUserId=2 playerId=101 previousUserId=9', + userId: actorId, + }, + manager, + ); + }); + + it('unlinks only when the locked player is currently linked to the target user', async () => { + player = assignment(101, user(9, RoleEnum.user, StatusEnum.active)); + + await expect( + service.unlinkPlayer(actorId, target.id, player.id), + ).rejects.toBeInstanceOf(ConflictException); + expect(playerRepository.save).not.toHaveBeenCalled(); + + player.user = target; + player.userId = target.id; + await service.unlinkPlayer(actorId, target.id, player.id); + expect(player.user).toBeNull(); + expect(playerRepository.save).toHaveBeenCalledWith(player); + }); + + function user(id: number, roleId: RoleEnum, statusId: StatusEnum) { + return { + id, + firstName: id === 2 ? 'Target' : 'Admin', + lastName: id === 2 ? 'User' : String(id), + email: `user-${id}@example.com`, + password: `password-${id}`, + hash: `hash-${id}`, + socialId: `social-${id}`, + role: { + id: roleId, + name: roleId === RoleEnum.admin ? 'Admin' : 'User', + }, + status: { + id: statusId, + name: statusId === StatusEnum.active ? 'Active' : 'Inactive', + }, + players: [], + }; + } + + function assignment(id: number, assignedUser: any) { + return { + id, + firstName: 'Pat', + lastName: 'Player', + active: true, + team, + teamRole, + user: assignedUser, + userId: assignedUser?.id ?? null, + }; + } + + function playerSummary(id: number) { + return { + id, + firstName: 'Pat', + lastName: 'Player', + active: true, + team, + teamRole, + }; + } + + function chain(overrides: Record) { + const query: Record = {}; + [ + 'innerJoin', + 'innerJoinAndSelect', + 'leftJoinAndSelect', + 'leftJoin', + 'select', + 'where', + 'andWhere', + 'setParameter', + 'setParameters', + 'setLock', + 'orderBy', + 'offset', + 'limit', + ].forEach((method) => { + query[method] = jest.fn(() => query); + }); + return Object.assign(query, overrides); + } +}); diff --git a/myteamwallet_backend/src/users/admin-users.service.ts b/myteamwallet_backend/src/users/admin-users.service.ts new file mode 100644 index 0000000..fe5916a --- /dev/null +++ b/myteamwallet_backend/src/users/admin-users.service.ts @@ -0,0 +1,381 @@ +import { + ConflictException, + ForbiddenException, + Injectable, + NotFoundException, +} from '@nestjs/common'; +import { Brackets, DataSource, EntityManager, Repository } from 'typeorm'; +import { LoggingService } from '../database/logging/logging.service'; +import { Player } from '../players/entities/player.entity'; +import { Role } from '../roles/entities/role.entity'; +import { RoleEnum } from '../roles/roles.enum'; +import { Status } from '../statuses/entities/status.entity'; +import { StatusEnum } from '../statuses/statuses.enum'; +import { + AdminPlayerPageDto, + AdminPlayerSummaryDto, +} from './dto/admin-player-response.dto'; +import { AdminPlayerQueryDto, AdminUserProfileDto } from './dto/admin-user.dto'; +import { AdminUserDirectorySummaryDto } from './dto/user-directory-response.dto'; +import { User } from './entities/user.entity'; + +type PlayerSearchRow = { + player_id: number | string; + first_name: string; + last_name: string; + active: boolean; + team_id: number | string; + team_name: string; + team_alias: string; + user_id: number | string | null; + user_first_name: string | null; + user_last_name: string | null; + status_id: number | string | null; + status_name: string | null; +}; + +@Injectable() +export class AdminUsersService { + constructor( + private readonly dataSource: DataSource, + private readonly logger: LoggingService, + ) {} + + async updateProfile( + actorId: number, + targetUserId: number, + dto: AdminUserProfileDto, + ): Promise { + const summary = await this.dataSource.transaction(async (manager) => { + const repository = manager.getRepository(User); + const target = await this.findLockedUser(repository, targetUserId); + if (dto.firstName !== undefined) target.firstName = dto.firstName; + if (dto.lastName !== undefined) target.lastName = dto.lastName; + await repository.save(target); + const summary = await this.findAdminSummary(manager, targetUserId); + await this.log( + manager, + 'admin_user_profile_update', + actorId, + `targetUserId=${targetUserId}`, + ); + return summary; + }); + return summary; + } + + async updateRole( + actorId: number, + targetUserId: number, + roleId: RoleEnum, + ): Promise { + const summary = await this.dataSource.transaction(async (manager) => { + const activeAdmins = await this.lockActiveAdmins(manager); + const repository = manager.getRepository(User); + const target = await this.findLockedUser(repository, targetUserId); + const isDemotion = + target.role?.id === RoleEnum.admin && roleId !== RoleEnum.admin; + if (actorId === targetUserId && isDemotion) { + throw new ForbiddenException('Administrators cannot demote themselves'); + } + if ( + isDemotion && + target.status?.id === StatusEnum.active && + activeAdmins.length <= 1 + ) { + throw new ConflictException('At least one active admin must remain'); + } + target.role = { id: roleId } as Role; + await repository.save(target); + const summary = await this.findAdminSummary(manager, targetUserId); + await this.log( + manager, + 'admin_user_role_update', + actorId, + `targetUserId=${targetUserId} roleId=${roleId}`, + ); + return summary; + }); + return summary; + } + + async updateStatus( + actorId: number, + targetUserId: number, + statusId: StatusEnum, + ): Promise { + const summary = await this.dataSource.transaction(async (manager) => { + const activeAdmins = await this.lockActiveAdmins(manager); + const repository = manager.getRepository(User); + const target = await this.findLockedUser(repository, targetUserId); + const isDeactivation = + target.status?.id === StatusEnum.active && + statusId === StatusEnum.inactive; + if (actorId === targetUserId && isDeactivation) { + throw new ForbiddenException( + 'Administrators cannot deactivate themselves', + ); + } + if ( + isDeactivation && + target.role?.id === RoleEnum.admin && + activeAdmins.length <= 1 + ) { + throw new ConflictException('At least one active admin must remain'); + } + target.status = { id: statusId } as Status; + if (statusId === StatusEnum.inactive) target.hash = null; + await repository.save(target); + const summary = await this.findAdminSummary(manager, targetUserId); + await this.log( + manager, + 'admin_user_status_update', + actorId, + `targetUserId=${targetUserId} statusId=${statusId}`, + ); + return summary; + }); + return summary; + } + + async findPlayers(query: AdminPlayerQueryDto): Promise { + const page = query.page ?? 1; + const limit = query.limit ?? 20; + const repository = this.dataSource.manager.getRepository(Player); + const builder = repository + .createQueryBuilder('player') + .innerJoin('player.team', 'team') + .leftJoin('player.user', 'currentUser') + .leftJoin('currentUser.status', 'currentStatus'); + const term = query.search?.trim().toLocaleLowerCase(); + if (term) { + builder + .andWhere( + new Brackets((where) => + where + .where('LOWER(player.firstName) LIKE :playerSearch') + .orWhere('LOWER(player.lastName) LIKE :playerSearch') + .orWhere('LOWER(team.name) LIKE :playerSearch'), + ), + ) + .setParameter('playerSearch', `%${term}%`); + } + if (query.teamId !== undefined) { + builder.andWhere('player.teamId = :teamId', { teamId: query.teamId }); + } + if (query.assignment === 'assigned') { + builder.andWhere('player.userId IS NOT NULL'); + } else if (query.assignment === 'unassigned') { + builder.andWhere('player.userId IS NULL'); + } + const total = await builder.getCount(); + const rows = await builder + .select([ + 'player.id AS player_id', + 'player.firstName AS first_name', + 'player.lastName AS last_name', + 'player.active AS active', + 'team.id AS team_id', + 'team.name AS team_name', + 'team.alias AS team_alias', + 'currentUser.id AS user_id', + 'currentUser.firstName AS user_first_name', + 'currentUser.lastName AS user_last_name', + 'currentStatus.id AS status_id', + 'currentStatus.name AS status_name', + ]) + .orderBy('player.id', 'ASC') + .offset((page - 1) * limit) + .limit(limit) + .getRawMany(); + return { + data: rows.map((row) => this.mapPlayerSearchRow(row)), + page, + limit, + total, + hasNextPage: page * limit < total, + }; + } + + async assignPlayer( + actorId: number, + targetUserId: number, + playerId: number, + ): Promise { + let previousUserId: number | null = null; + const summary = await this.dataSource.transaction(async (manager) => { + const userRepository = manager.getRepository(User); + const playerRepository = manager.getRepository(Player); + const target = await userRepository.findOne({ + where: { id: targetUserId }, + }); + if (!target) throw new NotFoundException('User not found'); + const lockedPlayer = await this.findLockedPlayer( + playerRepository, + playerId, + ); + previousUserId = lockedPlayer.userId ?? null; + lockedPlayer.user = target; + await playerRepository.save(lockedPlayer); + const summary = await this.findAdminSummary(manager, targetUserId); + await this.log( + manager, + 'admin_player_assign', + actorId, + `targetUserId=${targetUserId} playerId=${playerId} previousUserId=${ + previousUserId ?? 'none' + }`, + ); + return summary; + }); + return summary; + } + + async unlinkPlayer( + actorId: number, + targetUserId: number, + playerId: number, + ): Promise { + const summary = await this.dataSource.transaction(async (manager) => { + const playerRepository = manager.getRepository(Player); + const lockedPlayer = await this.findLockedPlayer( + playerRepository, + playerId, + ); + if (lockedPlayer.userId !== targetUserId) { + throw new ConflictException('Player is not assigned to this user'); + } + lockedPlayer.user = null; + await playerRepository.save(lockedPlayer); + const summary = await this.findAdminSummary(manager, targetUserId); + await this.log( + manager, + 'admin_player_unlink', + actorId, + `targetUserId=${targetUserId} playerId=${playerId}`, + ); + return summary; + }); + return summary; + } + + private lockActiveAdmins(manager: EntityManager): Promise { + return manager + .getRepository(User) + .createQueryBuilder('user') + .innerJoinAndSelect('user.role', 'role') + .innerJoinAndSelect('user.status', 'status') + .where('role.id = :adminRole', { adminRole: RoleEnum.admin }) + .andWhere('status.id = :activeStatus', { + activeStatus: StatusEnum.active, + }) + .setLock('pessimistic_write', undefined, ['user']) + .orderBy('user.id', 'ASC') + .getMany(); + } + + private async findLockedUser( + repository: Repository, + userId: number, + ): Promise { + const user = await repository + .createQueryBuilder('lockedUser') + .leftJoinAndSelect('lockedUser.role', 'role') + .leftJoinAndSelect('lockedUser.status', 'status') + .where('lockedUser.id = :userId', { userId }) + .setLock('pessimistic_write', undefined, ['lockedUser']) + .getOne(); + if (!user) throw new NotFoundException('User not found'); + return user; + } + + private async findLockedPlayer( + repository: Repository, + playerId: number, + ): Promise { + const player = await repository + .createQueryBuilder('lockedPlayer') + .where('lockedPlayer.id = :playerId', { playerId }) + .setLock('pessimistic_write') + .getOne(); + if (!player) throw new NotFoundException('Player not found'); + return player; + } + + private async findAdminSummary( + manager: EntityManager, + userId: number, + ): Promise { + const user = await manager.getRepository(User).findOne({ + where: { id: userId }, + relations: { + role: true, + status: true, + players: { team: true, teamRole: true }, + }, + }); + if (!user) throw new NotFoundException('User not found'); + return { + id: user.id, + firstName: user.firstName, + lastName: user.lastName, + email: user.email, + role: user.role ? { id: user.role.id, name: user.role.name } : null, + status: user.status + ? { id: user.status.id, name: user.status.name } + : null, + assignments: (user.players ?? []).map((player) => ({ + id: player.id, + firstName: player.firstName, + lastName: player.lastName, + active: player.active, + team: { + id: player.team.id, + name: player.team.name, + alias: player.team.alias, + }, + teamRole: player.teamRole + ? { id: player.teamRole.id, name: player.teamRole.name } + : null, + })), + }; + } + + private mapPlayerSearchRow(row: PlayerSearchRow): AdminPlayerSummaryDto { + return { + id: Number(row.player_id), + firstName: row.first_name, + lastName: row.last_name, + active: Boolean(row.active), + team: { + id: Number(row.team_id), + name: row.team_name, + alias: row.team_alias, + }, + currentUser: + row.user_id === null + ? null + : { + id: Number(row.user_id), + firstName: row.user_first_name, + lastName: row.user_last_name, + status: + row.status_id === null + ? null + : { + id: Number(row.status_id), + name: row.status_name ?? undefined, + }, + }, + }; + } + + private log( + manager: EntityManager, + event: Parameters[0]['event'], + userId: number, + details: string, + ) { + return this.logger.info({ event, userId, details }, manager); + } +} diff --git a/myteamwallet_backend/src/users/dto/admin-player-response.dto.ts b/myteamwallet_backend/src/users/dto/admin-player-response.dto.ts new file mode 100644 index 0000000..58753ca --- /dev/null +++ b/myteamwallet_backend/src/users/dto/admin-player-response.dto.ts @@ -0,0 +1,31 @@ +import { UserDirectoryReferenceDto } from './user-directory-response.dto'; + +export class AdminPlayerTeamDto { + id: number; + name: string; + alias: string; +} + +export class AdminPlayerCurrentUserDto { + id: number; + firstName: string | null; + lastName: string | null; + status: UserDirectoryReferenceDto | null; +} + +export class AdminPlayerSummaryDto { + id: number; + firstName: string; + lastName: string; + active: boolean; + team: AdminPlayerTeamDto; + currentUser: AdminPlayerCurrentUserDto | null; +} + +export class AdminPlayerPageDto { + data: AdminPlayerSummaryDto[]; + page: number; + limit: number; + total: number; + hasNextPage: boolean; +} diff --git a/myteamwallet_backend/src/users/dto/admin-user.dto.ts b/myteamwallet_backend/src/users/dto/admin-user.dto.ts new file mode 100644 index 0000000..6efef9a --- /dev/null +++ b/myteamwallet_backend/src/users/dto/admin-user.dto.ts @@ -0,0 +1,67 @@ +import { Type } from 'class-transformer'; +import { + IsIn, + IsInt, + IsOptional, + IsString, + Max, + MaxLength, + Min, +} from 'class-validator'; +import { RoleEnum } from '../../roles/roles.enum'; +import { StatusEnum } from '../../statuses/statuses.enum'; + +export class AdminUserProfileDto { + @IsOptional() + @IsString() + @MaxLength(255) + firstName?: string | null; + + @IsOptional() + @IsString() + @MaxLength(255) + lastName?: string | null; +} + +export class AdminUserRoleDto { + @Type(() => Number) + @IsIn([RoleEnum.admin, RoleEnum.user]) + role: RoleEnum; +} + +export class AdminUserStatusDto { + @Type(() => Number) + @IsIn([StatusEnum.active, StatusEnum.inactive]) + status: StatusEnum; +} + +export type AdminPlayerAssignmentFilter = 'all' | 'assigned' | 'unassigned'; + +export class AdminPlayerQueryDto { + @IsOptional() + @IsString() + search?: string; + + @IsOptional() + @Type(() => Number) + @IsInt() + @Min(1) + teamId?: number; + + @IsOptional() + @IsIn(['all', 'assigned', 'unassigned']) + assignment: AdminPlayerAssignmentFilter = 'all'; + + @IsOptional() + @Type(() => Number) + @IsInt() + @Min(1) + page = 1; + + @IsOptional() + @Type(() => Number) + @IsInt() + @Min(1) + @Max(50) + limit = 20; +} diff --git a/myteamwallet_backend/src/users/users.controller.security.spec.ts b/myteamwallet_backend/src/users/users.controller.security.spec.ts new file mode 100644 index 0000000..45f456a --- /dev/null +++ b/myteamwallet_backend/src/users/users.controller.security.spec.ts @@ -0,0 +1,11 @@ +import { UsersController } from './users.controller'; + +describe('UsersController admin mutation isolation', () => { + it('does not expose generic create, update, or delete handlers that bypass safeguards', () => { + expect(UsersController.prototype).not.toHaveProperty('create'); + expect(UsersController.prototype).not.toHaveProperty('update'); + expect(UsersController.prototype).not.toHaveProperty('remove'); + expect(UsersController.prototype).not.toHaveProperty('findAll'); + expect(UsersController.prototype).not.toHaveProperty('findOne'); + }); +}); diff --git a/myteamwallet_backend/src/users/users.controller.ts b/myteamwallet_backend/src/users/users.controller.ts index 2a40f30..1ea9fda 100644 --- a/myteamwallet_backend/src/users/users.controller.ts +++ b/myteamwallet_backend/src/users/users.controller.ts @@ -1,28 +1,19 @@ import { Controller, Get, - Post, - Body, - Patch, Param, - Delete, UseGuards, Query, - DefaultValuePipe, - ParseIntPipe, HttpStatus, HttpCode, Request, } from '@nestjs/common'; import { UsersService } from './users.service'; -import { CreateUserDto } from './dto/create-user.dto'; -import { UpdateUserDto } from './dto/update-user.dto'; import { ApiBearerAuth, ApiTags } from '@nestjs/swagger'; import { Roles } from 'src/roles/roles.decorator'; import { RoleEnum } from 'src/roles/roles.enum'; import { AuthGuard } from '@nestjs/passport'; import { RolesGuard } from 'src/roles/roles.guard'; -import { infinityPagination } from 'src/utils/infinity-pagination'; import { UserDirectoryQueryDto } from './dto/user-directory-query.dto'; import { User } from './entities/user.entity'; @@ -36,33 +27,6 @@ import { User } from './entities/user.entity'; export class UsersController { constructor(private readonly usersService: UsersService) {} - @Roles([RoleEnum.admin]) - @Post() - @HttpCode(HttpStatus.CREATED) - create(@Body() createProfileDto: CreateUserDto) { - return this.usersService.create(createProfileDto); - } - - @Roles([RoleEnum.admin]) - @Get() - @HttpCode(HttpStatus.OK) - async findAll( - @Query('page', new DefaultValuePipe(1), ParseIntPipe) page: number, - @Query('limit', new DefaultValuePipe(10), ParseIntPipe) limit: number, - ) { - if (limit > 50) { - limit = 50; - } - - return infinityPagination( - await this.usersService.findManyWithPagination({ - page, - limit, - }), - { page, limit }, - ); - } - @Roles([RoleEnum.user, RoleEnum.admin]) @Get('directory') @HttpCode(HttpStatus.OK) @@ -73,30 +37,10 @@ export class UsersController { return this.usersService.findDirectory(request.user, query); } - @Roles([RoleEnum.admin]) - @Get(':id') - @HttpCode(HttpStatus.OK) - findOne(@Param('id') id: string) { - return this.usersService.findOne({ id: +id }); - } - @Roles([RoleEnum.user, RoleEnum.admin]) @Get(':id/teams') @HttpCode(HttpStatus.OK) findTeamsOfPlayer(@Param('id') id: string) { return this.usersService.findTeams({ id: +id }); } - - @Roles([RoleEnum.admin]) - @Patch(':id') - @HttpCode(HttpStatus.OK) - update(@Param('id') id: number, @Body() updateProfileDto: UpdateUserDto) { - return this.usersService.update(id, updateProfileDto); - } - - @Roles([RoleEnum.admin]) - @Delete(':id') - remove(@Param('id') id: number) { - return this.usersService.softDelete(id); - } } diff --git a/myteamwallet_backend/src/users/users.module.ts b/myteamwallet_backend/src/users/users.module.ts index 39bb9a6..9104762 100644 --- a/myteamwallet_backend/src/users/users.module.ts +++ b/myteamwallet_backend/src/users/users.module.ts @@ -7,11 +7,14 @@ import { IsExist } from 'src/utils/validators/is-exists.validator'; import { IsNotExist } from 'src/utils/validators/is-not-exists.validator'; import { Team } from 'src/teams/entities/team.entity'; import { Player } from 'src/players/entities/player.entity'; +import { LoggingModule } from 'src/database/logging/logging.module'; +import { AdminUsersController } from './admin-users.controller'; +import { AdminUsersService } from './admin-users.service'; @Module({ - imports: [TypeOrmModule.forFeature([User, Team, Player])], - controllers: [UsersController], - providers: [IsExist, IsNotExist, UsersService], + imports: [TypeOrmModule.forFeature([User, Team, Player]), LoggingModule], + controllers: [UsersController, AdminUsersController], + providers: [IsExist, IsNotExist, UsersService, AdminUsersService], exports: [UsersService], }) export class UsersModule {} From bec1826bfa39fb91fe7894ac7510faf27c508ba5 Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Sat, 1 Aug 2026 00:19:49 +0200 Subject: [PATCH 06/15] fix: close admin user security gaps --- .../admin-user-management/task-2-report.md | 67 +++++++ .../src/auth/auth.controller.spec.ts | 12 ++ .../src/auth/auth.controller.ts | 3 +- .../src/auth/auth.service.spec.ts | 131 ++++++++++++- myteamwallet_backend/src/auth/auth.service.ts | 115 ++++++------ .../src/auth/dto/auth-register-login.dto.ts | 3 - .../src/users/admin-users.controller.spec.ts | 11 ++ .../src/users/admin-users.http.spec.ts | 173 ++++++++++++++++++ .../src/users/dto/admin-user.dto.ts | 4 +- .../src/users/dto/create-user.dto.ts | 10 +- .../src/users/dto/user-team-response.dto.ts | 11 ++ .../users/users.controller.security.spec.ts | 14 ++ .../src/users/users.controller.ts | 12 +- .../src/users/users.service.ts | 54 +++--- .../src/users/users.teams.spec.ts | 49 +++++ 15 files changed, 566 insertions(+), 103 deletions(-) create mode 100644 myteamwallet_backend/src/users/admin-users.http.spec.ts create mode 100644 myteamwallet_backend/src/users/dto/user-team-response.dto.ts create mode 100644 myteamwallet_backend/src/users/users.teams.spec.ts diff --git a/.superpowers/sdd/admin-user-management/task-2-report.md b/.superpowers/sdd/admin-user-management/task-2-report.md index 15921e6..59d7369 100644 --- a/.superpowers/sdd/admin-user-management/task-2-report.md +++ b/.superpowers/sdd/admin-user-management/task-2-report.md @@ -120,3 +120,70 @@ Every production behavior above was added only after the corresponding expected - The lock/concurrency and migration tests are focused unit/SQL-shape tests; no live PostgreSQL instance was available for a two-connection race test or an actual migration run/revert. A database-backed integration test remains advisable before production rollout. - Removing superseded generic user CRUD/read routes and `DELETE auth/me` is intentionally security-hardening and may affect undocumented external clients. Repository frontend searches showed no use of those removed routes. - Full unrelated backend test-suite repair was intentionally out of scope; the focused Task 1 + Task 2 suite and backend build are green. + +## Fix Round 1 + +### Review findings addressed + +- Removed `linkPlayerId` from the validated public registration DTO and from internal create DTO plumbing. `AuthController.register` now has a concrete `AuthRegisterLoginDto` body rather than `any`, `AuthService.register` copies only the four permitted registration fields, and the obsolete `UsersService.linkPlayerToUserId` path was removed. Only `AdminUsersService` now changes `Player.user`. +- Rebuilt existing-account social login around one database transaction. Candidate user rows are locked, any email change uses a narrow repository update, the user is reloaded with current role/status under an alias-scoped row lock, inactive state is rechecked, and only then is the JWT signed. The same method covers Facebook, Google, Twitter, and Apple. +- Restricted `GET users/:id/teams` to the authenticated user's own ID. The query now returns an explicit minimal projection containing only player ID/name and team ID/name, matching the fields consumed by the current modern team selector. +- Removed body coercion for role/status mutation IDs. Genuine integer numbers are required; booleans and numeric strings are rejected. +- Added a focused Nest HTTP boundary suite with actual URI versioning, global validation, controller decorators, JWT guard behavior, real `RolesGuard`, and HTTP serialization assertions. + +### RED evidence + +1. Registration isolation: + - Command: `npm test -- --runInBand auth.controller.spec.ts auth.service.spec.ts -t "narrow validated registration|public registration player"` + - Failure: controller parameter metadata was `Object` instead of `AuthRegisterLoginDto`; registration still attempted public player linkage. +2. Social-login race: + - Command: `npm test -- --runInBand auth.service.spec.ts -t "concurrently deactivated social|locks and reloads an existing"` + - Failure: existing flow bypassed the transaction repository, used entity-wide `UsersService.update`, and signed stale state. +3. Self-only safe team bootstrap: + - Command: `npm test -- --runInBand users.controller.security.spec.ts users.teams.spec.ts` + - Failure: `findMyTeams` did not exist and the controller still delegated arbitrary IDs to raw `findTeams`. +4. Strict numeric role/status bodies: + - Command: `npm test -- --runInBand admin-users.controller.spec.ts -t "non-number role"` + - Failure: both JSON `true` and `"1"` were coerced to valid enum ID `1`. +5. Nest HTTP boundary: + - Command: `npm test -- --runInBand admin-users.http.spec.ts` + - Initial infrastructure failure: the focused module did not wire the existing database-backed `IsNotExist` validator container. The test module was corrected to use the real validator with a mocked repository; no validation was weakened. + +### Files added + +- `src/users/admin-users.http.spec.ts` +- `src/users/users.teams.spec.ts` +- `src/users/dto/user-team-response.dto.ts` + +### Files modified + +- `src/auth/auth.controller.ts` +- `src/auth/auth.controller.spec.ts` +- `src/auth/auth.service.ts` +- `src/auth/auth.service.spec.ts` +- `src/auth/dto/auth-register-login.dto.ts` +- `src/users/admin-users.controller.spec.ts` +- `src/users/dto/admin-user.dto.ts` +- `src/users/dto/create-user.dto.ts` +- `src/users/users.controller.ts` +- `src/users/users.controller.security.spec.ts` +- `src/users/users.service.ts` + +### GREEN evidence + +- Focused Task 1 + Task 2 tests: + - Command: `npm test -- --runInBand users.service.spec.ts users.teams.spec.ts users.controller.security.spec.ts admin-users.controller.spec.ts admin-users.service.spec.ts admin-users.http.spec.ts auth.controller.spec.ts auth.service.spec.ts jwt.strategy.spec.ts logging.service.spec.ts AddPlayerLookupIndexes.spec.ts` + - Result: **11 suites passed, 56 tests passed, 0 failed**. +- Targeted ESLint across all Fix Round 1 source/spec files: **exit 0, no findings**. +- Backend build via `npm run build`: **exit 0**. +- `git diff --check`: **exit 0**. +- Both frontend directories: **no changes**. + +### Client contract impact + +- Both frontend codebases currently send `linkPlayerId` during invite registration. The backend now strips it and performs no assignment, as required; registration still succeeds, but player linkage must subsequently use the guarded admin assignment endpoint. +- Both frontends call `GET users/:currentUserId/teams`. That self-ID URL remains valid. The modern selector consumes only the retained player/team ID and name fields. The legacy frontend also displayed team balance and team role from this response; those sensitive/unneeded fields are no longer returned, and the legacy frontend was intentionally not edited. + +### Remaining limitation + +- No ready local PostgreSQL test database/harness was available without new infrastructure. No dependencies or testcontainers were added. Concurrency remains covered by transaction/alias-lock assertions and stale-state regressions; migration remains covered by exact up/down SQL and metadata tests. A live two-connection PostgreSQL race and migration run/revert remain recommended before rollout. diff --git a/myteamwallet_backend/src/auth/auth.controller.spec.ts b/myteamwallet_backend/src/auth/auth.controller.spec.ts index db87187..1466ed8 100644 --- a/myteamwallet_backend/src/auth/auth.controller.spec.ts +++ b/myteamwallet_backend/src/auth/auth.controller.spec.ts @@ -1,5 +1,6 @@ import { GUARDS_METADATA } from '@nestjs/common/constants'; import { AuthController } from './auth.controller'; +import { AuthRegisterLoginDto } from './dto/auth-register-login.dto'; describe('AuthController session enforcement', () => { it('protects GET auth/me with JWT validation', () => { @@ -15,4 +16,15 @@ describe('AuthController session enforcement', () => { it('does not expose self-deletion that can race with an admin promotion', () => { expect(AuthController.prototype).not.toHaveProperty('delete'); }); + + it('uses the narrow validated registration DTO instead of an untyped body', () => { + const parameterTypes = Reflect.getMetadata( + 'design:paramtypes', + AuthController.prototype, + 'register', + ); + + expect(parameterTypes[0]).toBe(AuthRegisterLoginDto); + expect(AuthRegisterLoginDto.prototype).not.toHaveProperty('linkPlayerId'); + }); }); diff --git a/myteamwallet_backend/src/auth/auth.controller.ts b/myteamwallet_backend/src/auth/auth.controller.ts index 59340c5..27300dc 100644 --- a/myteamwallet_backend/src/auth/auth.controller.ts +++ b/myteamwallet_backend/src/auth/auth.controller.ts @@ -26,6 +26,7 @@ import { ApiOkResponse, } from '@nestjs/swagger'; import { CreateInviteDTO } from './dto/create-invite.dto'; +import { AuthRegisterLoginDto } from './dto/auth-register-login.dto'; @ApiTags('Auth') @Controller({ @@ -50,7 +51,7 @@ export class AuthController { @Post('email/register') @HttpCode(HttpStatus.CREATED) - async register(@Body() createUserDto: any) { + async register(@Body() createUserDto: AuthRegisterLoginDto) { return this.service.register(createUserDto); } diff --git a/myteamwallet_backend/src/auth/auth.service.spec.ts b/myteamwallet_backend/src/auth/auth.service.spec.ts index e6845da..a780564 100644 --- a/myteamwallet_backend/src/auth/auth.service.spec.ts +++ b/myteamwallet_backend/src/auth/auth.service.spec.ts @@ -13,6 +13,7 @@ describe('AuthService inactive-user enforcement and safe logging', () => { let lockedUserQuery: any; let userRepository: any; let service: AuthService; + let mailService: any; beforeEach(() => { jwtService = { @@ -24,8 +25,10 @@ describe('AuthService inactive-user enforcement and safe logging', () => { findOne: jest.fn(), update: jest.fn(), create: jest.fn(), + linkPlayerToUserId: jest.fn(), }; logger = { info: jest.fn(), debug: jest.fn() }; + mailService = { userSignUp: jest.fn() }; confirmationUser = user(StatusEnum.inactive); confirmationUser.hash = 'confirmation-hash'; lockedUserQuery = { @@ -45,7 +48,7 @@ describe('AuthService inactive-user enforcement and safe logging', () => { jwtService, usersService, {} as any, - {} as any, + mailService, logger, dataSource, ); @@ -70,10 +73,12 @@ describe('AuthService inactive-user enforcement and safe logging', () => { }); it('rejects social login when the existing account is inactive', async () => { - const inactive = user(StatusEnum.inactive); - usersService.findOne - .mockResolvedValueOnce(inactive) - .mockResolvedValueOnce(undefined); + const inactive = socialUser( + AuthProvidersEnum.google, + RoleEnum.user, + StatusEnum.inactive, + ); + configureSocialQueries([inactive], inactive); await expect( service.validateSocialLogin(AuthProvidersEnum.google, { @@ -88,6 +93,61 @@ describe('AuthService inactive-user enforcement and safe logging', () => { expect(jwtService.sign).not.toHaveBeenCalled(); }); + it('reloads and rejects a concurrently deactivated social user instead of signing stale state', async () => { + const stale = socialUser( + AuthProvidersEnum.google, + RoleEnum.admin, + StatusEnum.active, + ); + const current = socialUser( + AuthProvidersEnum.google, + RoleEnum.user, + StatusEnum.inactive, + ); + configureSocialQueries([stale], current); + + await expect( + service.validateSocialLogin(AuthProvidersEnum.google, { + id: stale.socialId, + email: 'updated@example.com', + }), + ).rejects.toBeInstanceOf(ForbiddenException); + + expect(dataSource.transaction).toHaveBeenCalledTimes(1); + expect(userRepository.save).not.toHaveBeenCalledWith(stale); + expect(usersService.update).not.toHaveBeenCalled(); + expect(jwtService.sign).not.toHaveBeenCalled(); + }); + + it.each([ + AuthProvidersEnum.facebook, + AuthProvidersEnum.google, + AuthProvidersEnum.twitter, + AuthProvidersEnum.apple, + ])( + 'locks and reloads an existing %s user, narrowly updates email, and signs the current role', + async (provider) => { + const stale = socialUser(provider, RoleEnum.admin, StatusEnum.active); + const current = socialUser(provider, RoleEnum.user, StatusEnum.active); + configureSocialQueries([stale], current); + + const result = await service.validateSocialLogin(provider, { + id: stale.socialId, + email: 'updated@example.com', + }); + + expect(userRepository.update).toHaveBeenCalledWith(stale.id, { + email: 'updated@example.com', + }); + expect(userRepository.save).not.toHaveBeenCalledWith(stale); + expect(jwtService.sign).toHaveBeenCalledWith({ + id: current.id, + role: current.role, + }); + expect(result.user).toBe(current); + }, + ); + it('never includes an email in an unknown-user login audit event', async () => { usersService.findOne.mockResolvedValue(undefined); @@ -154,6 +214,26 @@ describe('AuthService inactive-user enforcement and safe logging', () => { expect(userRepository.save).toHaveBeenCalledWith(confirmationUser); }); + it('ignores a public registration player id and never mutates player ownership', async () => { + usersService.create.mockResolvedValue({ + id: 8, + email: 'new@example.com', + }); + + await service.register({ + email: 'new@example.com', + password: 'password', + firstName: 'New', + lastName: 'User', + linkPlayerId: 101, + } as any); + + expect(usersService.create.mock.calls[0][0]).not.toHaveProperty( + 'linkPlayerId', + ); + expect(usersService.linkPlayerToUserId).not.toHaveBeenCalled(); + }); + function user(statusId: StatusEnum) { return { id: 2, @@ -167,4 +247,45 @@ describe('AuthService inactive-user enforcement and safe logging', () => { }, }; } + + function socialUser( + provider: AuthProvidersEnum, + roleId: RoleEnum, + statusId: StatusEnum, + ) { + return { + ...user(statusId), + email: 'old@example.com', + socialId: `${provider}-id`, + provider, + role: { id: roleId, name: roleId === RoleEnum.admin ? 'Admin' : 'User' }, + hash: 'stale-hash', + }; + } + + function configureSocialQueries(candidates: any[], current: any) { + const candidateQuery = chain({ getMany: jest.fn(() => candidates) }); + const reloadQuery = chain({ getOne: jest.fn(() => current) }); + userRepository.createQueryBuilder = jest.fn((alias: string) => + alias === 'socialCandidate' ? candidateQuery : reloadQuery, + ); + userRepository.update = jest.fn(); + } + + function chain(overrides: Record) { + const query: Record = {}; + [ + 'leftJoinAndSelect', + 'where', + 'orWhere', + 'andWhere', + 'setParameter', + 'setParameters', + 'setLock', + 'orderBy', + ].forEach((method) => { + query[method] = jest.fn(() => query); + }); + return Object.assign(query, overrides); + } }); diff --git a/myteamwallet_backend/src/auth/auth.service.ts b/myteamwallet_backend/src/auth/auth.service.ts index 512e125..5739ceb 100644 --- a/myteamwallet_backend/src/auth/auth.service.ts +++ b/myteamwallet_backend/src/auth/auth.service.ts @@ -117,59 +117,68 @@ export class AuthService { authProvider: string, socialData: SocialInterface, ): Promise<{ token: string; user: User }> { - let user: User; const socialEmail = socialData.email?.toLowerCase(); - - const userByEmail = await this.usersService.findOne({ - email: socialEmail, - }); - - user = await this.usersService.findOne({ - socialId: socialData.id, - provider: authProvider, - }); - - if (user) { - await this.assertActiveUser(user); - if (socialEmail && !userByEmail) { - user.email = socialEmail; + return this.dataSource.transaction(async (manager) => { + const repository = manager.getRepository(User); + const candidateQuery = repository + .createQueryBuilder('socialCandidate') + .where( + 'socialCandidate.socialId = :socialId AND socialCandidate.provider = :authProvider', + { socialId: socialData.id, authProvider }, + ); + if (socialEmail) { + candidateQuery.orWhere('socialCandidate.email = :socialEmail', { + socialEmail, + }); } - await this.usersService.update(user.id, user); - } else if (userByEmail) { - user = userByEmail; - await this.assertActiveUser(user); - } else { - const role = plainToClass(Role, { - id: RoleEnum.user, + const candidates = await candidateQuery + .setLock('pessimistic_write', undefined, ['socialCandidate']) + .orderBy('socialCandidate.id', 'ASC') + .getMany(); + const socialUser = candidates.find( + (candidate) => + candidate.socialId === socialData.id && + candidate.provider === authProvider, + ); + const emailUser = socialEmail + ? candidates.find((candidate) => candidate.email === socialEmail) + : undefined; + let user = socialUser ?? emailUser; + if (!user) { + user = await repository.save( + repository.create({ + email: socialEmail, + firstName: socialData.firstName, + lastName: socialData.lastName, + socialId: socialData.id, + provider: authProvider, + role: { id: RoleEnum.user } as Role, + status: { id: StatusEnum.active } as Status, + }), + ); + } else if ( + socialUser && + socialEmail && + !emailUser && + socialUser.email !== socialEmail + ) { + await repository.update(socialUser.id, { email: socialEmail }); + } + const currentUser = await repository + .createQueryBuilder('currentSocialUser') + .leftJoinAndSelect('currentSocialUser.role', 'role') + .leftJoinAndSelect('currentSocialUser.status', 'status') + .where('currentSocialUser.id = :userId', { userId: user.id }) + .setLock('pessimistic_write', undefined, ['currentSocialUser']) + .getOne(); + if (!currentUser) throw new UnauthorizedException(); + await this.assertActiveUser(currentUser); + const token = await this.jwtService.sign({ + id: currentUser.id, + role: currentUser.role, }); - const status = plainToClass(Status, { - id: StatusEnum.active, - }); - - user = await this.usersService.create({ - email: socialEmail, - firstName: socialData.firstName, - lastName: socialData.lastName, - socialId: socialData.id, - provider: authProvider, - role, - status, - }); - - user = await this.usersService.findOne({ - id: user.id, - }); - } - - const jwtToken = await this.jwtService.sign({ - id: user.id, - role: user.role, + return { token, user: currentUser }; }); - - return { - token: jwtToken, - user, - }; } async register(dto: AuthRegisterLoginDto): Promise { @@ -179,8 +188,10 @@ export class AuthService { .digest('hex'); const user = await this.usersService.create({ - ...dto, email: dto.email, + password: dto.password, + firstName: dto.firstName, + lastName: dto.lastName, role: { id: RoleEnum.user, } as Role, @@ -190,10 +201,6 @@ export class AuthService { hash, }); - if (user && dto.linkPlayerId != null) { - await this.usersService.linkPlayerToUserId(user, dto.linkPlayerId); - } - await this.logger.info({ event: 'user_create', details: `userId=${user.id}`, diff --git a/myteamwallet_backend/src/auth/dto/auth-register-login.dto.ts b/myteamwallet_backend/src/auth/dto/auth-register-login.dto.ts index 6924cb3..dd10e97 100644 --- a/myteamwallet_backend/src/auth/dto/auth-register-login.dto.ts +++ b/myteamwallet_backend/src/auth/dto/auth-register-login.dto.ts @@ -23,7 +23,4 @@ export class AuthRegisterLoginDto { @ApiProperty({ example: 'Doe' }) @IsNotEmpty() lastName: string; - - @ApiProperty({ example: 27 }) - linkPlayerId: number | null; } diff --git a/myteamwallet_backend/src/users/admin-users.controller.spec.ts b/myteamwallet_backend/src/users/admin-users.controller.spec.ts index 203b37d..7cfd986 100644 --- a/myteamwallet_backend/src/users/admin-users.controller.spec.ts +++ b/myteamwallet_backend/src/users/admin-users.controller.spec.ts @@ -98,6 +98,17 @@ describe('admin user DTOs', () => { expect(await validate(status)).not.toEqual([]); }); + it.each([true, '1'])( + 'rejects non-number role and status bodies: %p', + async (value) => { + const role = plainToInstance(AdminUserRoleDto, { role: value }); + const status = plainToInstance(AdminUserStatusDto, { status: value }); + + expect(await validate(role)).not.toEqual([]); + expect(await validate(status)).not.toEqual([]); + }, + ); + it('validates player assignment filters and pagination bounds', async () => { const valid = plainToInstance(AdminPlayerQueryDto, { assignment: 'assigned', diff --git a/myteamwallet_backend/src/users/admin-users.http.spec.ts b/myteamwallet_backend/src/users/admin-users.http.spec.ts new file mode 100644 index 0000000..37930ac --- /dev/null +++ b/myteamwallet_backend/src/users/admin-users.http.spec.ts @@ -0,0 +1,173 @@ +import { + INestApplication, + UnauthorizedException, + ValidationPipe, + VersioningType, +} from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import { AuthGuard } from '@nestjs/passport'; +import { Test } from '@nestjs/testing'; +import * as request from 'supertest'; +import { useContainer } from 'class-validator'; +import { DataSource } from 'typeorm'; +import { AuthController } from '../auth/auth.controller'; +import { AuthService } from '../auth/auth.service'; +import { RoleEnum } from '../roles/roles.enum'; +import { RolesGuard } from '../roles/roles.guard'; +import validationOptions from '../utils/validation-options'; +import { IsNotExist } from '../utils/validators/is-not-exists.validator'; +import { AdminUsersController } from './admin-users.controller'; +import { AdminUsersService } from './admin-users.service'; +import { UsersController } from './users.controller'; +import { UsersService } from './users.service'; + +describe('admin user HTTP security boundary', () => { + let app: INestApplication; + const safeUser = { + id: 2, + firstName: 'Target', + lastName: 'User', + email: 'target@example.com', + role: { id: RoleEnum.user, name: 'User' }, + status: { id: 1, name: 'Active' }, + assignments: [], + }; + const adminService = { + updateProfile: jest.fn(() => safeUser), + updateRole: jest.fn(() => safeUser), + updateStatus: jest.fn(() => safeUser), + findPlayers: jest.fn(), + assignPlayer: jest.fn(), + unlinkPlayer: jest.fn(), + }; + const usersService = { + findDirectory: jest.fn(), + findMyTeams: jest.fn(() => [ + { + id: 11, + firstName: 'Pat', + lastName: 'Player', + team: { id: 4, name: 'Alpha' }, + }, + ]), + }; + const authService = { register: jest.fn() }; + + beforeAll(async () => { + const module = await Test.createTestingModule({ + controllers: [AdminUsersController, UsersController, AuthController], + providers: [ + Reflector, + RolesGuard, + IsNotExist, + { + provide: DataSource, + useValue: { + getRepository: () => ({ findOne: jest.fn(() => undefined) }), + }, + }, + { provide: AdminUsersService, useValue: adminService }, + { provide: UsersService, useValue: usersService }, + { provide: AuthService, useValue: authService }, + ], + }) + .overrideGuard(AuthGuard('jwt')) + .useValue({ + canActivate(context) { + const httpRequest = context.switchToHttp().getRequest(); + const token = httpRequest.headers.authorization; + if (token === 'Bearer admin') { + httpRequest.user = { id: 1, role: { id: RoleEnum.admin } }; + return true; + } + if (token === 'Bearer user') { + httpRequest.user = { id: 7, role: { id: RoleEnum.user } }; + return true; + } + throw new UnauthorizedException(); + }, + }) + .compile(); + app = module.createNestApplication(); + useContainer(app, { fallbackOnErrors: true }); + app.setGlobalPrefix('api'); + app.enableVersioning({ type: VersioningType.URI }); + app.useGlobalPipes(new ValidationPipe(validationOptions)); + await app.init(); + }); + + afterAll(() => app.close()); + beforeEach(() => jest.clearAllMocks()); + + it('enforces JWT and current global admin role on an admin mutation', async () => { + await request(app.getHttpServer()) + .patch('/api/v1/admin/users/2/role') + .send({ role: RoleEnum.user }) + .expect(401); + await request(app.getHttpServer()) + .patch('/api/v1/admin/users/2/role') + .set('Authorization', 'Bearer user') + .send({ role: RoleEnum.user }) + .expect(403); + await request(app.getHttpServer()) + .patch('/api/v1/admin/users/2/role') + .set('Authorization', 'Bearer admin') + .send({ role: RoleEnum.user }) + .expect(200, safeUser); + }); + + it.each([true, '1'])( + 'rejects non-numeric role JSON at the HTTP validation boundary: %p', + async (role) => { + await request(app.getHttpServer()) + .patch('/api/v1/admin/users/2/role') + .set('Authorization', 'Bearer admin') + .send({ role }) + .expect(422); + expect(adminService.updateRole).not.toHaveBeenCalled(); + }, + ); + + it('strips public player assignment input from registration', async () => { + await request(app.getHttpServer()) + .post('/api/v1/auth/email/register') + .send({ + email: 'new@example.com', + password: 'password', + firstName: 'New', + lastName: 'User', + linkPlayerId: 101, + }) + .expect(201); + + expect(authService.register).toHaveBeenCalledWith({ + email: 'new@example.com', + password: 'password', + firstName: 'New', + lastName: 'User', + }); + }); + + it('restricts team bootstrap to self and serializes only the safe projection', async () => { + await request(app.getHttpServer()) + .get('/api/v1/users/8/teams') + .set('Authorization', 'Bearer user') + .expect(403); + const response = await request(app.getHttpServer()) + .get('/api/v1/users/7/teams') + .set('Authorization', 'Bearer user') + .expect(200); + + expect(response.body).toEqual([ + { + id: 11, + firstName: 'Pat', + lastName: 'Player', + team: { id: 4, name: 'Alpha' }, + }, + ]); + expect(JSON.stringify(response.body)).not.toMatch( + /provider|socialId|password|hash|balance|createdAt|updatedAt/i, + ); + }); +}); diff --git a/myteamwallet_backend/src/users/dto/admin-user.dto.ts b/myteamwallet_backend/src/users/dto/admin-user.dto.ts index 6efef9a..07b654d 100644 --- a/myteamwallet_backend/src/users/dto/admin-user.dto.ts +++ b/myteamwallet_backend/src/users/dto/admin-user.dto.ts @@ -24,13 +24,13 @@ export class AdminUserProfileDto { } export class AdminUserRoleDto { - @Type(() => Number) + @IsInt() @IsIn([RoleEnum.admin, RoleEnum.user]) role: RoleEnum; } export class AdminUserStatusDto { - @Type(() => Number) + @IsInt() @IsIn([StatusEnum.active, StatusEnum.inactive]) status: StatusEnum; } diff --git a/myteamwallet_backend/src/users/dto/create-user.dto.ts b/myteamwallet_backend/src/users/dto/create-user.dto.ts index 94b4819..feebb79 100644 --- a/myteamwallet_backend/src/users/dto/create-user.dto.ts +++ b/myteamwallet_backend/src/users/dto/create-user.dto.ts @@ -1,13 +1,7 @@ import { Transform } from 'class-transformer'; import { ApiProperty } from '@nestjs/swagger'; import { Role } from '../../roles/entities/role.entity'; -import { - IsEmail, - IsNotEmpty, - IsOptional, - MinLength, - Validate, -} from 'class-validator'; +import { IsEmail, IsNotEmpty, MinLength, Validate } from 'class-validator'; import { Status } from '../../statuses/entities/status.entity'; import { IsNotExist } from '../../utils/validators/is-not-exists.validator'; import { IsExist } from '../../utils/validators/is-exists.validator'; @@ -51,6 +45,4 @@ export class CreateUserDto { status?: Status; hash?: string | null; - - linkPlayerId?: number | null; } diff --git a/myteamwallet_backend/src/users/dto/user-team-response.dto.ts b/myteamwallet_backend/src/users/dto/user-team-response.dto.ts new file mode 100644 index 0000000..af78024 --- /dev/null +++ b/myteamwallet_backend/src/users/dto/user-team-response.dto.ts @@ -0,0 +1,11 @@ +export class UserTeamReferenceDto { + id: number; + name: string; +} + +export class UserTeamPlayerDto { + id: number; + firstName: string; + lastName: string; + team: UserTeamReferenceDto; +} diff --git a/myteamwallet_backend/src/users/users.controller.security.spec.ts b/myteamwallet_backend/src/users/users.controller.security.spec.ts index 45f456a..4150fc6 100644 --- a/myteamwallet_backend/src/users/users.controller.security.spec.ts +++ b/myteamwallet_backend/src/users/users.controller.security.spec.ts @@ -1,4 +1,5 @@ import { UsersController } from './users.controller'; +import { ForbiddenException } from '@nestjs/common'; describe('UsersController admin mutation isolation', () => { it('does not expose generic create, update, or delete handlers that bypass safeguards', () => { @@ -8,4 +9,17 @@ describe('UsersController admin mutation isolation', () => { expect(UsersController.prototype).not.toHaveProperty('findAll'); expect(UsersController.prototype).not.toHaveProperty('findOne'); }); + + it('restricts team bootstrap to the authenticated user id', async () => { + const service = { findMyTeams: jest.fn(() => []) }; + const controller = new UsersController(service as any); + const request = { user: { id: 7, role: { id: 2 } } }; + + expect(() => (controller as any).findTeamsOfPlayer(request, 8)).toThrow( + ForbiddenException, + ); + await (controller as any).findTeamsOfPlayer(request, 7); + + expect(service.findMyTeams).toHaveBeenCalledWith(7); + }); }); diff --git a/myteamwallet_backend/src/users/users.controller.ts b/myteamwallet_backend/src/users/users.controller.ts index 1ea9fda..c527f36 100644 --- a/myteamwallet_backend/src/users/users.controller.ts +++ b/myteamwallet_backend/src/users/users.controller.ts @@ -7,6 +7,8 @@ import { HttpStatus, HttpCode, Request, + ForbiddenException, + ParseIntPipe, } from '@nestjs/common'; import { UsersService } from './users.service'; import { ApiBearerAuth, ApiTags } from '@nestjs/swagger'; @@ -40,7 +42,13 @@ export class UsersController { @Roles([RoleEnum.user, RoleEnum.admin]) @Get(':id/teams') @HttpCode(HttpStatus.OK) - findTeamsOfPlayer(@Param('id') id: string) { - return this.usersService.findTeams({ id: +id }); + findTeamsOfPlayer( + @Request() request: { user: Pick }, + @Param('id', ParseIntPipe) id: number, + ) { + if (request.user.id !== id) { + throw new ForbiddenException('Users may only load their own teams'); + } + return this.usersService.findMyTeams(id); } } diff --git a/myteamwallet_backend/src/users/users.service.ts b/myteamwallet_backend/src/users/users.service.ts index 4197060..49b2abf 100644 --- a/myteamwallet_backend/src/users/users.service.ts +++ b/myteamwallet_backend/src/users/users.service.ts @@ -16,6 +16,7 @@ import { UserDirectorySummaryDto, } from './dto/user-directory-response.dto'; import { User } from './entities/user.entity'; +import { UserTeamPlayerDto } from './dto/user-team-response.dto'; @Injectable() export class UsersService { @@ -113,33 +114,32 @@ export class UsersService { await this.usersRepository.softDelete(id); } - async findTeams(fields: EntityCondition) { - const user = await this.findOne(fields); - if (!user) { - return []; - } - - const players = await this.playersRepository.find({ - where: { - user: { - id: user.id, - }, - }, - relations: ['team'], - }); - return players; - } - - async linkPlayerToUserId(user: User, playerId: number): Promise { - return new Promise(async (resolve) => { - const player = await this.playersRepository.findOneByOrFail({ - id: playerId, - }); - - player.user = user; - await this.playersRepository.save(player); - return resolve(true); - }); + async findMyTeams(userId: number): Promise { + const rows = await this.playersRepository + .createQueryBuilder('player') + .innerJoin('player.team', 'team') + .select([ + 'player.id AS player_id', + 'player.firstName AS first_name', + 'player.lastName AS last_name', + 'team.id AS team_id', + 'team.name AS team_name', + ]) + .where('player.userId = :userId', { userId }) + .orderBy('player.id', 'ASC') + .getRawMany<{ + player_id: number | string; + first_name: string; + last_name: string; + team_id: number | string; + team_name: string; + }>(); + return rows.map((row) => ({ + id: Number(row.player_id), + firstName: row.first_name, + lastName: row.last_name, + team: { id: Number(row.team_id), name: row.team_name }, + })); } private createSharedTeamsQuery(requesterId: number) { diff --git a/myteamwallet_backend/src/users/users.teams.spec.ts b/myteamwallet_backend/src/users/users.teams.spec.ts new file mode 100644 index 0000000..525ae6d --- /dev/null +++ b/myteamwallet_backend/src/users/users.teams.spec.ts @@ -0,0 +1,49 @@ +import { UsersService } from './users.service'; + +describe('UsersService safe current-user teams', () => { + it('returns only the player and team fields required by team bootstrap', async () => { + const rows = [ + { + player_id: 11, + first_name: 'Pat', + last_name: 'Player', + team_id: 4, + team_name: 'Alpha', + user_id: 7, + provider: 'google', + social_id: 'must-not-leak', + balance: '100.00', + }, + ]; + const query = chain({ getRawMany: jest.fn(() => rows) }); + const playersRepository = { + createQueryBuilder: jest.fn(() => query), + }; + const service = new UsersService({} as any, playersRepository as any); + + const result = await service.findMyTeams(7); + + expect(result).toEqual([ + { + id: 11, + firstName: 'Pat', + lastName: 'Player', + team: { id: 4, name: 'Alpha' }, + }, + ]); + expect(JSON.stringify(result)).not.toMatch( + /user|provider|social|role|status|password|hash|balance|createdAt|updatedAt/i, + ); + expect(query.where).toHaveBeenCalledWith('player.userId = :userId', { + userId: 7, + }); + }); + + function chain(overrides: Record) { + const query: Record = {}; + ['innerJoin', 'select', 'where', 'orderBy'].forEach((method) => { + query[method] = jest.fn(() => query); + }); + return Object.assign(query, overrides); + } +}); From 78761b570b103a5f928a1aaeee55394fb1029543 Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Sat, 1 Aug 2026 00:21:26 +0200 Subject: [PATCH 07/15] chore: untrack SDD reports --- .../admin-user-management/task-1-report.md | 161 --------------- .../admin-user-management/task-2-report.md | 189 ------------------ 2 files changed, 350 deletions(-) delete mode 100644 .superpowers/sdd/admin-user-management/task-1-report.md delete mode 100644 .superpowers/sdd/admin-user-management/task-2-report.md diff --git a/.superpowers/sdd/admin-user-management/task-1-report.md b/.superpowers/sdd/admin-user-management/task-1-report.md deleted file mode 100644 index 3220667..0000000 --- a/.superpowers/sdd/admin-user-management/task-1-report.md +++ /dev/null @@ -1,161 +0,0 @@ -# Task 1 implementation report: backend directory contract and query - -## Files changed - -- `myteamwallet_backend/src/users/dto/user-directory-query.dto.ts` — page, limit, and optional search input validation. -- `myteamwallet_backend/src/users/dto/user-directory-response.dto.ts` — explicit safe directory, admin, assignment, team, and reference response DTOs. -- `myteamwallet_backend/src/users/users.service.ts` — scoped directory query, search, pagination, deduplication, and explicit entity-to-DTO mapping. -- `myteamwallet_backend/src/users/users.controller.ts` — authenticated `GET /api/v1/users/directory` endpoint, declared before `:id`. -- `myteamwallet_backend/src/users/users.service.spec.ts` — focused contract coverage. - -## RED test evidence - -Command: - -```powershell -npm test -- users/users.service.spec.ts --runInBand -``` - -Result: failed as expected, 7/7 tests failed with `TypeError: service.findDirectory is not a function`. This proved the missing directory-query behavior before implementation. - -## GREEN verification - -Commands and results: - -```powershell -npm test -- users/users.service.spec.ts --runInBand -``` - -Passed: 1 suite, 7 tests. Covers cross-team isolation, non-admin email/secret redaction, inactive visibility, admin visibility, deduplication before pagination, search, and pagination metadata. - -```powershell -.\node_modules\.bin\eslint.cmd src\users\users.service.ts src\users\users.controller.ts src\users\users.service.spec.ts src\users\dto\user-directory-query.dto.ts src\users\dto\user-directory-response.dto.ts --max-warnings=0 -``` - -Passed with no warnings or errors. - -```powershell -npm run build -``` - -Passed: Nest build completed successfully. - -```powershell -git diff --check -``` - -Passed with no whitespace errors. - -## Design notes - -- `findDirectory(requester, query)` returns `{ data, page, limit, total, hasNextPage }`. -- A non-admin's shared-team set is derived from their active player assignments. Only users with an assignment in that set are included, and each returned assignment is filtered to that same set. -- Inactive target users and inactive assignments remain visible when their team is shared. -- Admins receive all non-deleted users and every linked player assignment. Their records extend the safe base summary with `email` and the existing `{ id, name }` role shape. -- The query maps selected DTO fields explicitly. It never serializes a `User` or `Player` entity, so passwords, hashes, social IDs, providers, and other authentication fields cannot leak through this endpoint. -- User IDs are ordered before search/pagination for deterministic pages. Users are the primary result set, which guarantees deduplication before pagination even when they have multiple player assignments. - -## Self-review - -- Confirmed `GET directory` is registered before `GET :id`. -- Confirmed non-admin searches only operate after visibility filtering and do not include email. -- Confirmed admin search may include email and admin mapping includes role/status using the backend's existing `{ id, name }` shapes. -- Confirmed an admin with no player assignment is included and an unassigned non-admin is not exposed to other non-admins. -- Confirmed assignment mapping includes team/team-role summary fields only, never its linked user entity. - -## Concerns - -- The repository-wide Jest suite has documented pre-existing placeholder dependency failures in the SDD ledger; this task verified its focused suite, lint, build, and whitespace check. - -## Fix Round 1 - -### Files changed - -- `myteamwallet_backend/src/users/users.service.ts` — replaces whole-entity loading with database-side raw projections for visibility, search, distinct count, deterministic ordering, pagination, and assignment filtering. -- `myteamwallet_backend/src/users/users.service.spec.ts` — adds the inactive-requester regression and runs the directory contract against query-builder doubles that reject entity hydration and unsafe projected authentication fields. - -### RED evidence - -Test file: `myteamwallet_backend/src/users/users.service.spec.ts` - -Command: - -```powershell -npm test -- users/users.service.spec.ts --runInBand -``` - -Result: failed as expected with 2 failures. `treats an inactive requester assignment as a shared team membership` received `[]` instead of `[1, 2]`; `does not hydrate whole user entities for the directory` rejected with `directory queries must use a safe database projection` because the old code called `usersRepository.find`. - -### GREEN verification - -```powershell -npm test -- users/users.service.spec.ts --runInBand -``` - -Passed: 1 suite, 9 tests. - -```powershell -.\node_modules\.bin\eslint.cmd src\users\users.service.ts src\users\users.service.spec.ts --max-warnings=0 -``` - -Passed with no warnings or errors. - -```powershell -npm run build -``` - -Passed: Nest build completed successfully. - -```powershell -git diff --check -``` - -Passed with no whitespace errors. - -### Implementation notes - -- Shared-team membership now uses every requester `Player` row, including inactive ones, exactly as required by the directory plan. -- The user query joins only `status` and `role`, projects safe raw columns, applies shared-team visibility/search in SQL, counts `DISTINCT user.id`, orders by `user.id`, and applies offset/limit before mapping. -- Assignment rows are fetched only for the selected page of user IDs and are scoped with the same shared-team subquery for non-admins. No directory query selects or hydrates `User` authentication columns. - -## Fix Round 2 - -### Files changed - -- `myteamwallet_backend/src/users/users.service.spec.ts` — strengthens the inactive-requester regression with a QueryBuilder boundary that rejects `requesterPlayer.active` in the shared-team predicate. - -### RED evidence - -Test file: `myteamwallet_backend/src/users/users.service.spec.ts` - -After installing the boundary guard, the shared-team query was deliberately mutated to add `requesterPlayer.active = :active`. - -```powershell -npm test -- users/users.service.spec.ts --runInBand -``` - -Result: failed as expected, 1/9 tests failed. `treats an inactive requester assignment as a shared team membership` failed with `shared-team membership must not filter inactive requester assignments`. The mutation was then removed; the production query remains user-ID-only. - -### GREEN verification - -```powershell -npm test -- users/users.service.spec.ts --runInBand -``` - -Passed: 1 suite, 9 tests. - -```powershell -.\node_modules\.bin\eslint.cmd src\users\users.service.spec.ts --max-warnings=0 -``` - -Passed with no warnings or errors. - -```powershell -git diff --check -``` - -Passed with no whitespace errors. - -### Implementation notes - -- The test double checks the actual shared-team predicate supplied by the service, rather than returning fixed rows alone. It rejects only for the inactive-requester regression if a predicate references `requesterPlayer.active`, so the test now fails for the realistic authorization regression while preserving the existing output-contract assertions. diff --git a/.superpowers/sdd/admin-user-management/task-2-report.md b/.superpowers/sdd/admin-user-management/task-2-report.md deleted file mode 100644 index 59d7369..0000000 --- a/.superpowers/sdd/admin-user-management/task-2-report.md +++ /dev/null @@ -1,189 +0,0 @@ -# Task 2 Report: Backend admin mutations and authentication enforcement - -## Status - -Implemented and verified on top of Task 1 commit `4105460`. - -## Delivered API - -- Added versioned global-admin controller at `admin/users` (effective path follows the existing global `/api` prefix and URI versioning). -- Added narrow mutations: - - `PATCH admin/users/:id/profile` (`firstName`, `lastName` only) - - `PATCH admin/users/:id/role` (strict numeric `RoleEnum.admin|user` ID) - - `PATCH admin/users/:id/status` (strict numeric `StatusEnum.active|inactive` ID) - - `PUT admin/users/:userId/players/:playerId` - - `DELETE admin/users/:userId/players/:playerId` -- Added `GET admin/users/players` with search, optional `teamId`, `all|assigned|unassigned`, page, and limit. -- Kept `GET users/directory` as the user list source. Removed superseded generic user create/read/update/delete handlers that exposed unsafe/raw shapes or bypassed the narrow mutation safeguards. -- Removed `DELETE auth/me`, which could race with a promotion and bypass last-admin protection. - -## TDD RED evidence - -The following failures were observed before their production implementations: - -1. `npm test -- --runInBand admin-users.controller.spec.ts` - - Failed to compile because `AdminUsersController` and the narrow DTOs did not exist. -2. `npm test -- --runInBand admin-users.service.spec.ts` - - Failed to compile because `AdminUsersService` did not exist. -3. `npm test -- --runInBand auth.service.spec.ts jwt.strategy.spec.ts` - - Inactive password login produced the ordinary password failure, social login issued a token, logs contained email/token values, and `JwtStrategy` accepted only the JWT snapshot. -4. `npm test -- --runInBand AddPlayerLookupIndexes.spec.ts` - - Failed to compile because the reversible lookup-index migration did not exist. -5. `npm test -- --runInBand auth.service.spec.ts auth.controller.spec.ts` - - `GET auth/me` had no JWT guard and the service accepted/refreshed an inactive user. -6. `npm test -- --runInBand users.controller.security.spec.ts` - - Generic `UsersController` mutations were still present and bypassed the new invariants. -7. `npm test -- --runInBand admin-users.service.spec.ts -t "updates only names"` - - The locked user lookup did not use alias-scoped `FOR UPDATE`, exposing a PostgreSQL outer-join runtime failure. -8. `npm test -- --runInBand admin-users.controller.spec.ts -t "reverse-map"` - - Numeric enum reverse-map names such as `"admin"` passed validation. -9. `npm test -- --runInBand logging.service.spec.ts admin-users.service.spec.ts -t "caller transaction manager|updates only names"` - - Audit logging had no transaction-manager support and ran after commit. -10. `npm test -- --runInBand auth.service.spec.ts admin-users.service.spec.ts -t "serializes email confirmation|explicit paginated player"` - - Email confirmation had no row-lock transaction, and a numeric driver boolean was returned as `1` instead of `true`. - -Every production behavior above was added only after the corresponding expected RED was captured. - -## Final GREEN evidence - -- Focused backend tests: - - Command: `npm test -- --runInBand users.service.spec.ts users.controller.security.spec.ts admin-users.controller.spec.ts admin-users.service.spec.ts auth.controller.spec.ts auth.service.spec.ts jwt.strategy.spec.ts logging.service.spec.ts AddPlayerLookupIndexes.spec.ts` - - Result: **9 suites passed, 40 tests passed, 0 failed**. -- Targeted lint across every touched backend TypeScript file: - - Command: direct project ESLint invocation over 21 touched source/spec files. - - Result: **exit 0, no findings**. -- Backend build: - - Command: `npm run build` - - Result: **exit 0**. -- Migration up/down smoke coverage: - - Exact `CREATE INDEX` and reverse-order `DROP INDEX` SQL asserted in `AddPlayerLookupIndexes.spec.ts`. - - TypeORM entity index metadata asserted to match both migration names. -- Diff checks: - - `git diff --check`: **exit 0**. - - Both frontend directories: **no changes**. - -## Security and concurrency design - -- The controller is class-level protected by JWT auth, `RolesGuard`, and `Roles([RoleEnum.admin])`. -- Mutation DTOs are narrow and whitelisted. Role/status accept only strict numeric IDs, avoiding class-validator numeric-enum reverse-map strings. -- Role and status changes execute in transactions and lock the active-admin set in stable user-ID order. This serializes concurrent demotions/deactivations so the last active admin cannot be lost. -- Self-demotion and self-deactivation are rejected inside the locked transaction. -- User row locks use explicit query builders with `FOR UPDATE OF` the user alias. Role/status are loaded with left joins, preserving support for nullable relations without asking PostgreSQL to lock nullable joined rows. -- Deactivation changes only `User.status` and revokes any outstanding confirmation hash; it does not alter `Player.user`. -- Email confirmation locks the same user row and re-checks the hash inside its transaction. This serializes confirmation against administrative deactivation and prevents an old/racing confirmation link from reactivating a deactivated account. -- Assignment and reassignment lock the player row before changing `Player.user`; unlink verifies the locked row is still linked to the requested user. -- All mutation responses are explicit Task 1-compatible admin summaries. Player search uses its own explicit player/team/current-user projection. Password, hash, social ID, and tokens are never mapped. -- Admin audit events contain actor ID in `userId` and target/action IDs in details. Audit insertion uses the same transaction manager as the mutation, so an audit failure rolls back the security-sensitive change. -- Password and social login reject inactive accounts. `JwtStrategy` reloads the non-deleted database user on every request, rejects inactive/missing users, and returns the current database role/status rather than trusting token role claims. -- `GET auth/me` is JWT guarded and independently checks current active status before any refresh behavior. - -## Files - -### Added - -- `src/users/admin-users.controller.ts` -- `src/users/admin-users.controller.spec.ts` -- `src/users/admin-users.service.ts` -- `src/users/admin-users.service.spec.ts` -- `src/users/users.controller.security.spec.ts` -- `src/users/dto/admin-user.dto.ts` -- `src/users/dto/admin-player-response.dto.ts` -- `src/auth/auth.controller.spec.ts` -- `src/auth/auth.service.spec.ts` -- `src/auth/strategies/jwt.strategy.spec.ts` -- `src/database/migrations/1785517200000-AddPlayerLookupIndexes.ts` -- `src/database/migrations/AddPlayerLookupIndexes.spec.ts` - -### Modified - -- `src/users/users.controller.ts` -- `src/users/users.module.ts` -- `src/auth/auth.controller.ts` -- `src/auth/auth.service.ts` -- `src/auth/strategies/jwt.strategy.ts` -- `src/database/logging/logging.service.ts` -- `src/database/logging/logging.service.spec.ts` -- `src/database/logging/model/logging-event.type.ts` -- `src/players/entities/player.entity.ts` - -## Self-review - -- Checked every endpoint for server-side global-admin authorization and removed legacy mutation bypasses. -- Checked response construction for password/hash/social-ID/token leakage. -- Checked role/status races, lock acquisition order, nullable-relation SQL shape, player reassignment ownership, and confirmation/deactivation ordering. -- Checked all touched logging details for email, password, token, hash, or social-ID values. -- Checked migration names against entity metadata and down ordering. -- Confirmed no frontend changes. - -## Concerns / follow-up - -- The lock/concurrency and migration tests are focused unit/SQL-shape tests; no live PostgreSQL instance was available for a two-connection race test or an actual migration run/revert. A database-backed integration test remains advisable before production rollout. -- Removing superseded generic user CRUD/read routes and `DELETE auth/me` is intentionally security-hardening and may affect undocumented external clients. Repository frontend searches showed no use of those removed routes. -- Full unrelated backend test-suite repair was intentionally out of scope; the focused Task 1 + Task 2 suite and backend build are green. - -## Fix Round 1 - -### Review findings addressed - -- Removed `linkPlayerId` from the validated public registration DTO and from internal create DTO plumbing. `AuthController.register` now has a concrete `AuthRegisterLoginDto` body rather than `any`, `AuthService.register` copies only the four permitted registration fields, and the obsolete `UsersService.linkPlayerToUserId` path was removed. Only `AdminUsersService` now changes `Player.user`. -- Rebuilt existing-account social login around one database transaction. Candidate user rows are locked, any email change uses a narrow repository update, the user is reloaded with current role/status under an alias-scoped row lock, inactive state is rechecked, and only then is the JWT signed. The same method covers Facebook, Google, Twitter, and Apple. -- Restricted `GET users/:id/teams` to the authenticated user's own ID. The query now returns an explicit minimal projection containing only player ID/name and team ID/name, matching the fields consumed by the current modern team selector. -- Removed body coercion for role/status mutation IDs. Genuine integer numbers are required; booleans and numeric strings are rejected. -- Added a focused Nest HTTP boundary suite with actual URI versioning, global validation, controller decorators, JWT guard behavior, real `RolesGuard`, and HTTP serialization assertions. - -### RED evidence - -1. Registration isolation: - - Command: `npm test -- --runInBand auth.controller.spec.ts auth.service.spec.ts -t "narrow validated registration|public registration player"` - - Failure: controller parameter metadata was `Object` instead of `AuthRegisterLoginDto`; registration still attempted public player linkage. -2. Social-login race: - - Command: `npm test -- --runInBand auth.service.spec.ts -t "concurrently deactivated social|locks and reloads an existing"` - - Failure: existing flow bypassed the transaction repository, used entity-wide `UsersService.update`, and signed stale state. -3. Self-only safe team bootstrap: - - Command: `npm test -- --runInBand users.controller.security.spec.ts users.teams.spec.ts` - - Failure: `findMyTeams` did not exist and the controller still delegated arbitrary IDs to raw `findTeams`. -4. Strict numeric role/status bodies: - - Command: `npm test -- --runInBand admin-users.controller.spec.ts -t "non-number role"` - - Failure: both JSON `true` and `"1"` were coerced to valid enum ID `1`. -5. Nest HTTP boundary: - - Command: `npm test -- --runInBand admin-users.http.spec.ts` - - Initial infrastructure failure: the focused module did not wire the existing database-backed `IsNotExist` validator container. The test module was corrected to use the real validator with a mocked repository; no validation was weakened. - -### Files added - -- `src/users/admin-users.http.spec.ts` -- `src/users/users.teams.spec.ts` -- `src/users/dto/user-team-response.dto.ts` - -### Files modified - -- `src/auth/auth.controller.ts` -- `src/auth/auth.controller.spec.ts` -- `src/auth/auth.service.ts` -- `src/auth/auth.service.spec.ts` -- `src/auth/dto/auth-register-login.dto.ts` -- `src/users/admin-users.controller.spec.ts` -- `src/users/dto/admin-user.dto.ts` -- `src/users/dto/create-user.dto.ts` -- `src/users/users.controller.ts` -- `src/users/users.controller.security.spec.ts` -- `src/users/users.service.ts` - -### GREEN evidence - -- Focused Task 1 + Task 2 tests: - - Command: `npm test -- --runInBand users.service.spec.ts users.teams.spec.ts users.controller.security.spec.ts admin-users.controller.spec.ts admin-users.service.spec.ts admin-users.http.spec.ts auth.controller.spec.ts auth.service.spec.ts jwt.strategy.spec.ts logging.service.spec.ts AddPlayerLookupIndexes.spec.ts` - - Result: **11 suites passed, 56 tests passed, 0 failed**. -- Targeted ESLint across all Fix Round 1 source/spec files: **exit 0, no findings**. -- Backend build via `npm run build`: **exit 0**. -- `git diff --check`: **exit 0**. -- Both frontend directories: **no changes**. - -### Client contract impact - -- Both frontend codebases currently send `linkPlayerId` during invite registration. The backend now strips it and performs no assignment, as required; registration still succeeds, but player linkage must subsequently use the guarded admin assignment endpoint. -- Both frontends call `GET users/:currentUserId/teams`. That self-ID URL remains valid. The modern selector consumes only the retained player/team ID and name fields. The legacy frontend also displayed team balance and team role from this response; those sensitive/unneeded fields are no longer returned, and the legacy frontend was intentionally not edited. - -### Remaining limitation - -- No ready local PostgreSQL test database/harness was available without new infrastructure. No dependencies or testcontainers were added. Concurrency remains covered by transaction/alias-lock assertions and stale-state regressions; migration remains covered by exact up/down SQL and metadata tests. A live two-connection PostgreSQL race and migration run/revert remain recommended before rollout. From 484c7473fbdc242b4d8a632af935154d9df6d06d Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Sat, 1 Aug 2026 09:17:57 +0200 Subject: [PATCH 08/15] feat: add admin user frontend contract --- .../src/app/app.routes.spec.ts | 11 ++ .../src/app/app.routes.ts | 5 + .../src/app/core/auth/auth-api.ts | 1 - .../src/app/core/auth/auth-store.spec.ts | 31 +++++ .../src/app/core/auth/auth-store.ts | 6 + .../src/app/core/layout/shell/shell.ts | 6 +- .../src/app/core/team/my-teams-store.spec.ts | 7 +- .../src/app/core/team/my-teams-store.ts | 4 +- .../src/app/core/team/teams-api.spec.ts | 4 +- .../src/app/core/team/teams-api.ts | 7 +- .../src/app/core/users/admin-users-api.ts | 52 ++++++++ .../src/app/core/users/users-api.spec.ts | 90 ++++++++++++++ .../src/app/core/users/users-api.ts | 25 ++++ .../features/auth/register/register.spec.ts | 9 +- .../app/features/auth/register/register.ts | 1 - .../app/features/team-select/team-select.html | 4 +- .../src/app/features/team/more/more.html | 7 ++ .../src/app/features/team/more/more.spec.ts | 3 + .../src/app/features/users/users.ts | 13 ++ .../src/app/models/user-directory.model.ts | 113 ++++++++++++++++++ 20 files changed, 378 insertions(+), 21 deletions(-) create mode 100644 myteamwallet_frontend_modern/src/app/core/users/admin-users-api.ts create mode 100644 myteamwallet_frontend_modern/src/app/core/users/users-api.spec.ts create mode 100644 myteamwallet_frontend_modern/src/app/core/users/users-api.ts create mode 100644 myteamwallet_frontend_modern/src/app/features/users/users.ts create mode 100644 myteamwallet_frontend_modern/src/app/models/user-directory.model.ts diff --git a/myteamwallet_frontend_modern/src/app/app.routes.spec.ts b/myteamwallet_frontend_modern/src/app/app.routes.spec.ts index a5ef8a2..99753e4 100644 --- a/myteamwallet_frontend_modern/src/app/app.routes.spec.ts +++ b/myteamwallet_frontend_modern/src/app/app.routes.spec.ts @@ -28,6 +28,17 @@ describe('app routing', () => { expect(router.url).toBe('/auth/login'); }); + it('redirects the protected users route to login when logged out', async () => { + await router.navigateByUrl('/users'); + expect(router.url).toBe('/auth/login'); + }); + + it('allows an authenticated user to open the users route', async () => { + authStore.setSession('token', { id: 1, email: 'a@b.de', firstName: 'A', lastName: 'B' }); + await router.navigateByUrl('/users'); + expect(router.url).toBe('/users'); + }); + it('redirects the root path to team-select when logged in', async () => { authStore.setSession('token', { id: 1, email: 'a@b.de', firstName: 'A', lastName: 'B' }); await router.navigateByUrl('/'); diff --git a/myteamwallet_frontend_modern/src/app/app.routes.ts b/myteamwallet_frontend_modern/src/app/app.routes.ts index 4e611ad..55eaf23 100644 --- a/myteamwallet_frontend_modern/src/app/app.routes.ts +++ b/myteamwallet_frontend_modern/src/app/app.routes.ts @@ -38,6 +38,11 @@ export const routes: Routes = [ canActivate: [authGuard], loadComponent: () => import('./features/team-select/team-select').then((m) => m.TeamSelect), }, + { + path: 'users', + canActivate: [authGuard], + loadComponent: () => import('./features/users/users').then((m) => m.Users), + }, { path: 't/:token/:playerId', loadComponent: () => import('./features/public-team/public-player').then((m) => m.PublicPlayer), diff --git a/myteamwallet_frontend_modern/src/app/core/auth/auth-api.ts b/myteamwallet_frontend_modern/src/app/core/auth/auth-api.ts index e6fdd94..eea30bc 100644 --- a/myteamwallet_frontend_modern/src/app/core/auth/auth-api.ts +++ b/myteamwallet_frontend_modern/src/app/core/auth/auth-api.ts @@ -23,7 +23,6 @@ export interface RegistrationRequest { password: string; firstName: string; lastName: string; - linkPlayerId: number; } export interface CreateInviteRequest extends InviteDetails {} diff --git a/myteamwallet_frontend_modern/src/app/core/auth/auth-store.spec.ts b/myteamwallet_frontend_modern/src/app/core/auth/auth-store.spec.ts index ab0fea8..f3459c7 100644 --- a/myteamwallet_frontend_modern/src/app/core/auth/auth-store.spec.ts +++ b/myteamwallet_frontend_modern/src/app/core/auth/auth-store.spec.ts @@ -1,5 +1,6 @@ import { TestBed } from '@angular/core/testing'; import { AuthStore } from './auth-store'; +import { RoleId } from '../../models/role.model'; describe('AuthStore', () => { beforeEach(() => { @@ -76,4 +77,34 @@ describe('AuthStore', () => { expect(localStorage.getItem('tw_token')).toBe('jwt-token'); expect(JSON.parse(localStorage.getItem('tw_user')!)).toEqual(updated); }); + + it('derives the global admin presentation hint from the current user role', () => { + TestBed.configureTestingModule({}); + const store = TestBed.inject(AuthStore); + store.setSession('jwt-token', { + id: 1, + email: 'admin@example.de', + firstName: 'Ada', + lastName: 'Admin', + role: { id: RoleId.Admin, name: 'admin' }, + }); + + expect(store.currentGlobalRole()).toBe(RoleId.Admin); + expect(store.isGlobalAdmin()).toBe(true); + }); + + it('does not treat a signed-in standard user as a global admin', () => { + TestBed.configureTestingModule({}); + const store = TestBed.inject(AuthStore); + store.setSession('jwt-token', { + id: 2, + email: 'user@example.de', + firstName: 'Ute', + lastName: 'User', + role: { id: RoleId.User, name: 'user' }, + }); + + expect(store.currentGlobalRole()).toBe(RoleId.User); + expect(store.isGlobalAdmin()).toBe(false); + }); }); diff --git a/myteamwallet_frontend_modern/src/app/core/auth/auth-store.ts b/myteamwallet_frontend_modern/src/app/core/auth/auth-store.ts index a26e141..686195c 100644 --- a/myteamwallet_frontend_modern/src/app/core/auth/auth-store.ts +++ b/myteamwallet_frontend_modern/src/app/core/auth/auth-store.ts @@ -1,4 +1,5 @@ import { Injectable, computed, signal } from '@angular/core'; +import { RoleId } from '../../models/role.model'; import { User } from '../../models/user.model'; @Injectable({ providedIn: 'root' }) @@ -12,6 +13,11 @@ export class AuthStore { readonly token = this.tokenSignal.asReadonly(); readonly currentUser = this.userSignal.asReadonly(); readonly isLoggedIn = computed(() => this.tokenSignal() !== null); + readonly currentGlobalRole = computed(() => { + const roleId = this.userSignal()?.role?.id; + return roleId === RoleId.Admin || roleId === RoleId.User ? roleId : null; + }); + readonly isGlobalAdmin = computed(() => this.currentGlobalRole() === RoleId.Admin); setSession(token: string, user: User): void { localStorage.setItem(AuthStore.TOKEN_KEY, token); diff --git a/myteamwallet_frontend_modern/src/app/core/layout/shell/shell.ts b/myteamwallet_frontend_modern/src/app/core/layout/shell/shell.ts index af5e8e1..ea0a1f7 100644 --- a/myteamwallet_frontend_modern/src/app/core/layout/shell/shell.ts +++ b/myteamwallet_frontend_modern/src/app/core/layout/shell/shell.ts @@ -14,7 +14,7 @@ import { MatToolbarModule } from '@angular/material/toolbar'; import { AuthStore } from '../../auth/auth-store'; import { MyTeamsStore } from '../../team/my-teams-store'; import { TeamStore } from '../../team/team-store'; -import { Team } from '../../../models/team.model'; +import { UserTeamReference } from '../../../models/user-directory.model'; @Component({ selector: 'app-shell', @@ -41,9 +41,9 @@ export class Shell { protected readonly myTeams = computed(() => { const seen = new Set(); - const teams: Team[] = []; + const teams: UserTeamReference[] = []; for (const player of this.myTeamsStore.players()) { - if (player.team && !seen.has(player.team.id)) { + if (!seen.has(player.team.id)) { seen.add(player.team.id); teams.push(player.team); } diff --git a/myteamwallet_frontend_modern/src/app/core/team/my-teams-store.spec.ts b/myteamwallet_frontend_modern/src/app/core/team/my-teams-store.spec.ts index a29c183..92b1468 100644 --- a/myteamwallet_frontend_modern/src/app/core/team/my-teams-store.spec.ts +++ b/myteamwallet_frontend_modern/src/app/core/team/my-teams-store.spec.ts @@ -3,19 +3,16 @@ import { provideHttpClient } from '@angular/common/http'; import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing'; import { MyTeamsStore } from './my-teams-store'; import { environment } from '../../../environments/environment'; -import { Player } from '../../models/player.model'; describe('MyTeamsStore', () => { let store: MyTeamsStore; let httpMock: HttpTestingController; - const player: Player = { + const player = { id: 1, firstName: 'A', lastName: 'B', - balance: 0, - active: true, - team: { id: 5, name: 'Team A', alias: 'team-a', balance: 0 }, + team: { id: 5, name: 'Team A' }, }; beforeEach(() => { diff --git a/myteamwallet_frontend_modern/src/app/core/team/my-teams-store.ts b/myteamwallet_frontend_modern/src/app/core/team/my-teams-store.ts index 085aa5b..3102aa1 100644 --- a/myteamwallet_frontend_modern/src/app/core/team/my-teams-store.ts +++ b/myteamwallet_frontend_modern/src/app/core/team/my-teams-store.ts @@ -1,12 +1,12 @@ import { Injectable, inject, signal } from '@angular/core'; -import { Player } from '../../models/player.model'; +import { UserTeamMembership } from '../../models/user-directory.model'; import { TeamsApi } from './teams-api'; @Injectable({ providedIn: 'root' }) export class MyTeamsStore { private readonly teamsApi = inject(TeamsApi); - private readonly playersSignal = signal([]); + private readonly playersSignal = signal([]); private readonly loadingSignal = signal(false); private readonly loadedForUserId = signal(null); diff --git a/myteamwallet_frontend_modern/src/app/core/team/teams-api.spec.ts b/myteamwallet_frontend_modern/src/app/core/team/teams-api.spec.ts index 0894aba..de482ce 100644 --- a/myteamwallet_frontend_modern/src/app/core/team/teams-api.spec.ts +++ b/myteamwallet_frontend_modern/src/app/core/team/teams-api.spec.ts @@ -22,8 +22,8 @@ describe('TeamsApi', () => { httpMock.verify(); }); - it('fetches the players/teams belonging to a user', () => { - const players: Player[] = [{ id: 1, firstName: 'A', lastName: 'B', balance: 0, active: true }]; + it('fetches the reduced bootstrap memberships belonging to a user', () => { + const players = [{ id: 1, firstName: 'A', lastName: 'B', team: { id: 5, name: 'Team A' } }]; service.loadMyTeams(42).subscribe((response) => { expect(response).toEqual(players); diff --git a/myteamwallet_frontend_modern/src/app/core/team/teams-api.ts b/myteamwallet_frontend_modern/src/app/core/team/teams-api.ts index 75facbd..b76ba49 100644 --- a/myteamwallet_frontend_modern/src/app/core/team/teams-api.ts +++ b/myteamwallet_frontend_modern/src/app/core/team/teams-api.ts @@ -2,9 +2,10 @@ import { HttpClient } from '@angular/common/http'; import { Injectable, inject } from '@angular/core'; import { Observable } from 'rxjs'; import { environment } from '../../../environments/environment'; -import { Player } from '../../models/player.model'; import { Team } from '../../models/team.model'; import { PlayerTransaction } from '../../models/transaction.model'; +import { UserTeamMembership } from '../../models/user-directory.model'; +import { Player } from '../../models/player.model'; export interface CreatePlayerRequest { firstName: string; @@ -16,8 +17,8 @@ export interface CreatePlayerRequest { export class TeamsApi { private readonly http = inject(HttpClient); - loadMyTeams(userId: number): Observable { - return this.http.get(`${environment.apiUrl}users/${userId}/teams`); + loadMyTeams(userId: number): Observable { + return this.http.get(`${environment.apiUrl}users/${userId}/teams`); } loadTeamOverview(teamId: number): Observable { diff --git a/myteamwallet_frontend_modern/src/app/core/users/admin-users-api.ts b/myteamwallet_frontend_modern/src/app/core/users/admin-users-api.ts new file mode 100644 index 0000000..6b29630 --- /dev/null +++ b/myteamwallet_frontend_modern/src/app/core/users/admin-users-api.ts @@ -0,0 +1,52 @@ +import { HttpClient, HttpParams } from '@angular/common/http'; +import { Injectable, inject } from '@angular/core'; +import { Observable } from 'rxjs'; +import { environment } from '../../../environments/environment'; +import { + AdminPlayerFilters, + AdminPlayerPage, + AdminUserDirectorySummary, + AdminUserProfileRequest, + AdminUserRoleRequest, + AdminUserStatusRequest, +} from '../../models/user-directory.model'; + +@Injectable({ providedIn: 'root' }) +export class AdminUsersApi { + private readonly http = inject(HttpClient); + private readonly baseUrl = `${environment.apiUrl}admin/users`; + + updateProfile(userId: number, request: AdminUserProfileRequest): Observable { + return this.http.patch(`${this.baseUrl}/${userId}/profile`, request); + } + + updateRole(userId: number, request: AdminUserRoleRequest): Observable { + return this.http.patch(`${this.baseUrl}/${userId}/role`, request); + } + + updateStatus(userId: number, request: AdminUserStatusRequest): Observable { + return this.http.patch(`${this.baseUrl}/${userId}/status`, request); + } + + loadPlayers(filters: AdminPlayerFilters = {}): Observable { + return this.http.get(`${this.baseUrl}/players`, { params: this.toParams(filters) }); + } + + assignPlayer(userId: number, playerId: number): Observable { + return this.http.put(`${this.baseUrl}/${userId}/players/${playerId}`, null); + } + + unlinkPlayer(userId: number, playerId: number): Observable { + return this.http.delete(`${this.baseUrl}/${userId}/players/${playerId}`); + } + + private toParams(filters: AdminPlayerFilters): HttpParams { + let params = new HttpParams(); + if (filters.search !== undefined) params = params.set('search', filters.search); + if (filters.teamId !== undefined) params = params.set('teamId', filters.teamId); + if (filters.assignment !== undefined) params = params.set('assignment', filters.assignment); + if (filters.page !== undefined) params = params.set('page', filters.page); + if (filters.limit !== undefined) params = params.set('limit', filters.limit); + return params; + } +} diff --git a/myteamwallet_frontend_modern/src/app/core/users/users-api.spec.ts b/myteamwallet_frontend_modern/src/app/core/users/users-api.spec.ts new file mode 100644 index 0000000..e838ae5 --- /dev/null +++ b/myteamwallet_frontend_modern/src/app/core/users/users-api.spec.ts @@ -0,0 +1,90 @@ +import { provideHttpClient } from '@angular/common/http'; +import { TestBed } from '@angular/core/testing'; +import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing'; +import { environment } from '../../../environments/environment'; +import { AdminUsersApi } from './admin-users-api'; +import { UsersApi } from './users-api'; + +describe('UsersApi', () => { + let service: UsersApi; + let httpMock: HttpTestingController; + + beforeEach(() => { + TestBed.configureTestingModule({ providers: [provideHttpClient(), provideHttpClientTesting()] }); + service = TestBed.inject(UsersApi); + httpMock = TestBed.inject(HttpTestingController); + }); + + afterEach(() => httpMock.verify()); + + it('loads the safe user directory with supplied pagination and search filters', () => { + service.loadDirectory({ page: 2, limit: 10, search: 'Alex Muster' }).subscribe(); + + const request = httpMock.expectOne( + `${environment.apiUrl}users/directory?page=2&limit=10&search=Alex%20Muster`, + ); + expect(request.request.method).toBe('GET'); + request.flush({ data: [], page: 2, limit: 10, total: 0, hasNextPage: false }); + }); +}); + +describe('AdminUsersApi', () => { + let service: AdminUsersApi; + let httpMock: HttpTestingController; + + beforeEach(() => { + TestBed.configureTestingModule({ providers: [provideHttpClient(), provideHttpClientTesting()] }); + service = TestBed.inject(AdminUsersApi); + httpMock = TestBed.inject(HttpTestingController); + }); + + afterEach(() => httpMock.verify()); + + it('uses the dedicated admin profile endpoint and payload', () => { + service.updateProfile(7, { firstName: 'Alex', lastName: null }).subscribe(); + const request = httpMock.expectOne(`${environment.apiUrl}admin/users/7/profile`); + expect(request.request.method).toBe('PATCH'); + expect(request.request.body).toEqual({ firstName: 'Alex', lastName: null }); + request.flush({}); + }); + + it('uses the dedicated admin role endpoint and payload', () => { + service.updateRole(7, { role: 1 }).subscribe(); + const request = httpMock.expectOne(`${environment.apiUrl}admin/users/7/role`); + expect(request.request.method).toBe('PATCH'); + expect(request.request.body).toEqual({ role: 1 }); + request.flush({}); + }); + + it('uses the dedicated admin status endpoint and payload', () => { + service.updateStatus(7, { status: 2 }).subscribe(); + const request = httpMock.expectOne(`${environment.apiUrl}admin/users/7/status`); + expect(request.request.method).toBe('PATCH'); + expect(request.request.body).toEqual({ status: 2 }); + request.flush({}); + }); + + it('loads players with the admin filters', () => { + service.loadPlayers({ search: 'Alex', teamId: 5, assignment: 'unassigned', page: 2, limit: 10 }).subscribe(); + const request = httpMock.expectOne( + `${environment.apiUrl}admin/users/players?search=Alex&teamId=5&assignment=unassigned&page=2&limit=10`, + ); + expect(request.request.method).toBe('GET'); + request.flush({ data: [], page: 2, limit: 10, total: 0, hasNextPage: false }); + }); + + it('assigns a player through the dedicated admin endpoint', () => { + service.assignPlayer(7, 9).subscribe(); + const request = httpMock.expectOne(`${environment.apiUrl}admin/users/7/players/9`); + expect(request.request.method).toBe('PUT'); + expect(request.request.body).toBeNull(); + request.flush({}); + }); + + it('unlinks a player through the dedicated admin endpoint', () => { + service.unlinkPlayer(7, 9).subscribe(); + const request = httpMock.expectOne(`${environment.apiUrl}admin/users/7/players/9`); + expect(request.request.method).toBe('DELETE'); + request.flush({}); + }); +}); diff --git a/myteamwallet_frontend_modern/src/app/core/users/users-api.ts b/myteamwallet_frontend_modern/src/app/core/users/users-api.ts new file mode 100644 index 0000000..3fac777 --- /dev/null +++ b/myteamwallet_frontend_modern/src/app/core/users/users-api.ts @@ -0,0 +1,25 @@ +import { HttpClient, HttpParams } from '@angular/common/http'; +import { Injectable, inject } from '@angular/core'; +import { Observable } from 'rxjs'; +import { environment } from '../../../environments/environment'; +import { UserDirectoryFilters, UserDirectoryPage } from '../../models/user-directory.model'; + +@Injectable({ providedIn: 'root' }) +export class UsersApi { + private readonly http = inject(HttpClient); + private readonly baseUrl = `${environment.apiUrl}users`; + + loadDirectory(filters: UserDirectoryFilters = {}): Observable { + return this.http.get(`${this.baseUrl}/directory`, { + params: this.toParams(filters), + }); + } + + private toParams(filters: UserDirectoryFilters): HttpParams { + let params = new HttpParams(); + if (filters.page !== undefined) params = params.set('page', filters.page); + if (filters.limit !== undefined) params = params.set('limit', filters.limit); + if (filters.search !== undefined) params = params.set('search', filters.search); + return params; + } +} diff --git a/myteamwallet_frontend_modern/src/app/features/auth/register/register.spec.ts b/myteamwallet_frontend_modern/src/app/features/auth/register/register.spec.ts index 1ecd967..520af52 100644 --- a/myteamwallet_frontend_modern/src/app/features/auth/register/register.spec.ts +++ b/myteamwallet_frontend_modern/src/app/features/auth/register/register.spec.ts @@ -28,7 +28,7 @@ describe('Register', () => { afterEach(() => httpMock.verify()); - it('verifies the invitation and links its player during registration', () => { + it('verifies the invitation and registers without linking its player', () => { const fixture = TestBed.createComponent(Register); const navigateSpy = vi.spyOn(router, 'navigate'); fixture.detectChanges(); @@ -50,7 +50,12 @@ describe('Register', () => { fixture.componentInstance['onSubmit'](); const request = httpMock.expectOne(`${environment.apiUrl}auth/email/register`); - expect(request.request.body.linkPlayerId).toBe(7); + expect(request.request.body).toEqual({ + email: 'alex@example.de', + password: 'secret1', + firstName: 'Alex', + lastName: 'Muster', + }); request.flush(null); expect(navigateSpy).toHaveBeenCalledWith(['/auth/login'], { replaceUrl: true }); diff --git a/myteamwallet_frontend_modern/src/app/features/auth/register/register.ts b/myteamwallet_frontend_modern/src/app/features/auth/register/register.ts index 2383f11..9a0c1be 100644 --- a/myteamwallet_frontend_modern/src/app/features/auth/register/register.ts +++ b/myteamwallet_frontend_modern/src/app/features/auth/register/register.ts @@ -78,7 +78,6 @@ export class Register { password: value.password, firstName: value.firstName, lastName: value.lastName, - linkPlayerId: invitation.playerId, }) .subscribe({ next: () => { diff --git a/myteamwallet_frontend_modern/src/app/features/team-select/team-select.html b/myteamwallet_frontend_modern/src/app/features/team-select/team-select.html index fa7d4a5..dd5dea9 100644 --- a/myteamwallet_frontend_modern/src/app/features/team-select/team-select.html +++ b/myteamwallet_frontend_modern/src/app/features/team-select/team-select.html @@ -11,8 +11,8 @@

Team auswählen

@for (player of players(); track player.id) { - - {{ player.team?.name }} + + {{ player.team.name }} {{ player.firstName }} {{ player.lastName }} } diff --git a/myteamwallet_frontend_modern/src/app/features/team/more/more.html b/myteamwallet_frontend_modern/src/app/features/team/more/more.html index 25512c3..5ae851c 100644 --- a/myteamwallet_frontend_modern/src/app/features/team/more/more.html +++ b/myteamwallet_frontend_modern/src/app/features/team/more/more.html @@ -5,6 +5,13 @@
+ group +
BenutzerBenutzerverzeichnis öffnen
+ chevron_right gavel diff --git a/myteamwallet_frontend_modern/src/app/features/team/more/more.spec.ts b/myteamwallet_frontend_modern/src/app/features/team/more/more.spec.ts index 1db9339..3e50e76 100644 --- a/myteamwallet_frontend_modern/src/app/features/team/more/more.spec.ts +++ b/myteamwallet_frontend_modern/src/app/features/team/more/more.spec.ts @@ -31,6 +31,9 @@ describe('More', () => { expect(fixture.nativeElement.textContent).toContain('Profil'); expect(fixture.nativeElement.textContent).toContain('Öffentliche Freigabe'); + expect(fixture.nativeElement.textContent).toContain('Benutzer'); + expect(fixture.nativeElement.querySelector('a[href="/users"]')).not.toBeNull(); + fixture.componentInstance['logout'](); await fixture.whenStable(); expect(clearSession).toHaveBeenCalled(); diff --git a/myteamwallet_frontend_modern/src/app/features/users/users.ts b/myteamwallet_frontend_modern/src/app/features/users/users.ts new file mode 100644 index 0000000..1d81d4d --- /dev/null +++ b/myteamwallet_frontend_modern/src/app/features/users/users.ts @@ -0,0 +1,13 @@ +import { Component } from '@angular/core'; + +@Component({ + selector: 'app-users', + template: ` +
+

Organisation

+

Benutzer

+

Benutzerverzeichnis wird vorbereitet.

+
+ `, +}) +export class Users {} diff --git a/myteamwallet_frontend_modern/src/app/models/user-directory.model.ts b/myteamwallet_frontend_modern/src/app/models/user-directory.model.ts new file mode 100644 index 0000000..254d349 --- /dev/null +++ b/myteamwallet_frontend_modern/src/app/models/user-directory.model.ts @@ -0,0 +1,113 @@ +export interface UserDirectoryReference { + id: number; + name?: string; +} + +export interface UserDirectoryTeam { + id: number; + name: string; + alias: string; +} + +export interface UserDirectoryAssignment { + id: number; + firstName: string; + lastName: string; + active: boolean; + team: UserDirectoryTeam; + teamRole: UserDirectoryReference | null; +} + +export interface UserDirectorySummary { + id: number; + firstName: string | null; + lastName: string | null; + status: UserDirectoryReference | null; + assignments: UserDirectoryAssignment[]; +} + +export interface AdminUserDirectorySummary extends UserDirectorySummary { + email: string | null; + role: UserDirectoryReference | null; +} + +export type UserDirectoryRecord = UserDirectorySummary | AdminUserDirectorySummary; + +export interface UserDirectoryPage { + data: UserDirectoryRecord[]; + page: number; + limit: number; + total: number; + hasNextPage: boolean; +} + +export interface UserDirectoryFilters { + page?: number; + limit?: number; + search?: string; +} + +export interface AdminUserProfileRequest { + firstName?: string | null; + lastName?: string | null; +} + +export interface AdminUserRoleRequest { + role: number; +} + +export interface AdminUserStatusRequest { + status: number; +} + +export type AdminPlayerAssignmentFilter = 'all' | 'assigned' | 'unassigned'; + +export interface AdminPlayerFilters { + search?: string; + teamId?: number; + assignment?: AdminPlayerAssignmentFilter; + page?: number; + limit?: number; +} + +export interface AdminPlayerTeam { + id: number; + name: string; + alias: string; +} + +export interface AdminPlayerCurrentUser { + id: number; + firstName: string | null; + lastName: string | null; + status: UserDirectoryReference | null; +} + +export interface AdminPlayerSummary { + id: number; + firstName: string; + lastName: string; + active: boolean; + team: AdminPlayerTeam; + currentUser: AdminPlayerCurrentUser | null; +} + +export interface AdminPlayerPage { + data: AdminPlayerSummary[]; + page: number; + limit: number; + total: number; + hasNextPage: boolean; +} + +export interface UserTeamReference { + id: number; + name: string; +} + +export interface UserTeamMembership { + id: number; + firstName: string; + lastName: string; + team: UserTeamReference; +} From d0ec000bffa2aaecd89378957d1cfa0e035613c9 Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Sat, 1 Aug 2026 09:26:14 +0200 Subject: [PATCH 09/15] fix: harden admin frontend contracts --- .../src/app/core/auth/auth-api.spec.ts | 15 ++++++++++----- .../src/app/core/auth/auth-api.ts | 8 +++++++- .../src/app/models/user-directory.model.ts | 7 +++++-- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/myteamwallet_frontend_modern/src/app/core/auth/auth-api.spec.ts b/myteamwallet_frontend_modern/src/app/core/auth/auth-api.spec.ts index e0340cc..1f4d9f6 100644 --- a/myteamwallet_frontend_modern/src/app/core/auth/auth-api.spec.ts +++ b/myteamwallet_frontend_modern/src/app/core/auth/auth-api.spec.ts @@ -1,7 +1,7 @@ import { TestBed } from '@angular/core/testing'; import { provideHttpClient } from '@angular/common/http'; import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing'; -import { AuthApi } from './auth-api'; +import { AuthApi, RegistrationRequest } from './auth-api'; import { environment } from '../../../environments/environment'; import { User } from '../../models/user.model'; @@ -59,8 +59,8 @@ describe('AuthApi', () => { request.flush(invitation); }); - it('registers and links an invited player', () => { - const registration = { + it('registers with only the supported account fields', () => { + const registrationWithLegacyField = { email: 'alex@example.de', password: 'secret1', firstName: 'Alex', @@ -68,11 +68,16 @@ describe('AuthApi', () => { linkPlayerId: 7, }; - service.register(registration).subscribe(); + service.register(registrationWithLegacyField as RegistrationRequest).subscribe(); const request = httpMock.expectOne(`${environment.apiUrl}auth/email/register`); expect(request.request.method).toBe('POST'); - expect(request.request.body).toEqual(registration); + expect(request.request.body).toEqual({ + email: 'alex@example.de', + password: 'secret1', + firstName: 'Alex', + lastName: 'Muster', + }); request.flush(null); }); diff --git a/myteamwallet_frontend_modern/src/app/core/auth/auth-api.ts b/myteamwallet_frontend_modern/src/app/core/auth/auth-api.ts index eea30bc..10b5414 100644 --- a/myteamwallet_frontend_modern/src/app/core/auth/auth-api.ts +++ b/myteamwallet_frontend_modern/src/app/core/auth/auth-api.ts @@ -52,7 +52,13 @@ export class AuthApi { } register(request: RegistrationRequest): Observable { - return this.http.post(`${this.baseUrl}/email/register`, request); + const { email, password, firstName, lastName } = request; + return this.http.post(`${this.baseUrl}/email/register`, { + email, + password, + firstName, + lastName, + }); } forgotPassword(email: string): Observable { diff --git a/myteamwallet_frontend_modern/src/app/models/user-directory.model.ts b/myteamwallet_frontend_modern/src/app/models/user-directory.model.ts index 254d349..0a91db4 100644 --- a/myteamwallet_frontend_modern/src/app/models/user-directory.model.ts +++ b/myteamwallet_frontend_modern/src/app/models/user-directory.model.ts @@ -52,12 +52,15 @@ export interface AdminUserProfileRequest { lastName?: string | null; } +export type AdminUserRoleId = 1 | 2; +export type AdminUserStatusId = 1 | 2; + export interface AdminUserRoleRequest { - role: number; + role: AdminUserRoleId; } export interface AdminUserStatusRequest { - status: number; + status: AdminUserStatusId; } export type AdminPlayerAssignmentFilter = 'all' | 'assigned' | 'unassigned'; From 3ae0fd200035f3add72854b7592ffa180cf69fba Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Sat, 1 Aug 2026 10:01:25 +0200 Subject: [PATCH 10/15] feat: build admin user management UI --- .../app/features/users/player-assignments.ts | 265 ++++++++++++ .../src/app/features/users/user-edit.ts | 85 ++++ .../src/app/features/users/users.html | 123 ++++++ .../src/app/features/users/users.scss | 191 +++++++++ .../src/app/features/users/users.spec.ts | 381 ++++++++++++++++++ .../src/app/features/users/users.ts | 220 +++++++++- 6 files changed, 1256 insertions(+), 9 deletions(-) create mode 100644 myteamwallet_frontend_modern/src/app/features/users/player-assignments.ts create mode 100644 myteamwallet_frontend_modern/src/app/features/users/user-edit.ts create mode 100644 myteamwallet_frontend_modern/src/app/features/users/users.html create mode 100644 myteamwallet_frontend_modern/src/app/features/users/users.scss create mode 100644 myteamwallet_frontend_modern/src/app/features/users/users.spec.ts diff --git a/myteamwallet_frontend_modern/src/app/features/users/player-assignments.ts b/myteamwallet_frontend_modern/src/app/features/users/player-assignments.ts new file mode 100644 index 0000000..697eaa7 --- /dev/null +++ b/myteamwallet_frontend_modern/src/app/features/users/player-assignments.ts @@ -0,0 +1,265 @@ +import { HttpErrorResponse } from '@angular/common/http'; +import { Component, EventEmitter, Input, OnChanges, Output, inject, signal } from '@angular/core'; +import { MatButtonModule } from '@angular/material/button'; +import { MatDialog } from '@angular/material/dialog'; +import { MatFormFieldModule } from '@angular/material/form-field'; +import { MatIconModule } from '@angular/material/icon'; +import { MatInputModule } from '@angular/material/input'; +import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; +import { filter, finalize, take } from 'rxjs'; +import { AdminUsersApi } from '../../core/users/admin-users-api'; +import { + AdminPlayerPage, + AdminPlayerSummary, + AdminUserDirectorySummary, +} from '../../models/user-directory.model'; +import { ConfirmDialog } from '../../shared/confirm-dialog/confirm-dialog'; + +@Component({ + selector: 'app-player-assignments', + imports: [ + MatButtonModule, + MatFormFieldModule, + MatIconModule, + MatInputModule, + MatProgressSpinnerModule, + ], + template: ` +
+
+
+

Spielerzuordnungen

+

Spieler teamübergreifend suchen und sicher verknüpfen.

+
+ +
+ + + + @if (error()) { + + } + + @if (loading()) { +
+ } @else if (!error() && players()?.data?.length === 0) { +
Keine Spieler gefunden.
+ } @else if (players()) { +
+ @for (player of players()!.data; track player.id) { +
+
+ {{ player.firstName }} {{ player.lastName }} + {{ player.team.name }} · {{ player.active ? 'Aktiv' : 'Inaktiv' }} + @if (player.currentUser) { + Aktuell: {{ userName(player.currentUser) }} + } @else { + Nicht verknüpft + } +
+ @if (player.currentUser?.id === user.id) { + + } @else if (player.currentUser) { + + } @else { + + } +
+ } +
+
+ + Seite {{ players()!.page }} + +
+ } +
+ `, + styles: ` + .assignments { padding: 18px; border: 1px solid var(--mat-sys-outline-variant); border-radius: 18px; } + .assignments__heading { display: flex; align-items: flex-start; justify-content: space-between; gap: 16px; } + h3, p { margin: 0; } + .assignments__heading p { margin-top: 4px; color: var(--mat-sys-on-surface-variant); } + .assignments__search { display: flex; align-items: center; gap: 10px; margin: 18px 0; } + .assignments__search mat-form-field { flex: 1; } + .player-list { display: grid; } + .player-row { display: flex; align-items: center; justify-content: space-between; gap: 16px; padding: 14px 0; border-bottom: 1px solid var(--mat-sys-outline-variant); } + .player-row--inactive { opacity: .68; } + .player-row__copy { display: grid; gap: 2px; min-width: 0; } + .player-row__copy span { color: var(--mat-sys-on-surface-variant); } + .assignments__state { min-height: 120px; display: grid; place-content: center; color: var(--mat-sys-on-surface-variant); } + .assignments__error { display: flex; align-items: center; justify-content: space-between; gap: 12px; margin-bottom: 12px; padding: 12px; border-radius: 12px; color: var(--mat-sys-on-error-container); background: var(--mat-sys-error-container); } + .assignments__paging { display: flex; align-items: center; justify-content: space-between; margin-top: 14px; } + @media (max-width: 600px) { + .assignments { padding: 16px; } + .assignments__heading, .assignments__search, .player-row { align-items: stretch; flex-direction: column; } + .player-row button { width: 100%; } + } + `, +}) +export class PlayerAssignments implements OnChanges { + private readonly adminUsersApi = inject(AdminUsersApi); + private readonly dialog = inject(MatDialog); + + @Input({ required: true }) user!: AdminUserDirectorySummary; + @Output() readonly directoryChanged = new EventEmitter(); + @Output() readonly closed = new EventEmitter(); + + protected readonly players = signal(null); + protected readonly loading = signal(false); + protected readonly error = signal(null); + protected readonly pendingPlayerId = signal(null); + protected searchDraft = ''; + private search = ''; + private page = 1; + private readonly limit = 20; + + ngOnChanges(): void { + this.searchDraft = ''; + this.search = ''; + this.page = 1; + this.loadPlayers(); + } + + protected loadPlayers(): void { + this.loading.set(true); + this.error.set(null); + this.adminUsersApi + .loadPlayers({ + assignment: 'all', + page: this.page, + limit: this.limit, + ...(this.search ? { search: this.search } : {}), + }) + .pipe(finalize(() => this.loading.set(false))) + .subscribe({ + next: (players) => this.players.set(players), + error: (error: HttpErrorResponse) => this.error.set(this.errorMessage(error)), + }); + } + + protected submitSearch(): void { + this.search = this.searchDraft.trim(); + this.page = 1; + this.loadPlayers(); + } + + protected previousPage(): void { + if (this.page <= 1 || this.loading()) return; + this.page -= 1; + this.loadPlayers(); + } + + protected nextPage(): void { + if (!this.players()?.hasNextPage || this.loading()) return; + this.page += 1; + this.loadPlayers(); + } + + protected assign(player: AdminPlayerSummary): void { + this.runAssignment(player); + } + + protected confirmUnlink(player: AdminPlayerSummary): void { + this.confirm({ + title: 'Verknüpfung lösen?', + message: `${player.firstName} ${player.lastName} wird von ${this.targetName()} getrennt.`, + confirmLabel: 'Verknüpfung lösen', + }).subscribe(() => this.runUnlink(player)); + } + + protected confirmReassign(player: AdminPlayerSummary): void { + this.confirm({ + title: 'Spieler neu zuordnen?', + message: `${player.firstName} ${player.lastName} ist aktuell mit ${this.userName(player.currentUser!)} verknüpft und wird ${this.targetName()} zugeordnet.`, + confirmLabel: 'Neu zuordnen', + }).subscribe(() => this.runAssignment(player)); + } + + protected userName(user: { id: number; firstName: string | null; lastName: string | null }): string { + return [user.firstName, user.lastName].filter(Boolean).join(' ') || `Benutzer ${user.id}`; + } + + private targetName(): string { + return this.userName(this.user); + } + + private confirm(data: { title: string; message: string; confirmLabel: string }) { + return this.dialog + .open(ConfirmDialog, { data, restoreFocus: true }) + .afterClosed() + .pipe(filter(Boolean), take(1)); + } + + private runAssignment(player: AdminPlayerSummary): void { + if (this.pendingPlayerId() !== null) return; + this.pendingPlayerId.set(player.id); + this.error.set(null); + this.adminUsersApi + .assignPlayer(this.user.id, player.id) + .pipe(finalize(() => this.pendingPlayerId.set(null))) + .subscribe({ + next: () => this.refreshAfterMutation(), + error: (error: HttpErrorResponse) => this.error.set(this.errorMessage(error)), + }); + } + + private runUnlink(player: AdminPlayerSummary): void { + if (this.pendingPlayerId() !== null) return; + this.pendingPlayerId.set(player.id); + this.error.set(null); + this.adminUsersApi + .unlinkPlayer(this.user.id, player.id) + .pipe(finalize(() => this.pendingPlayerId.set(null))) + .subscribe({ + next: () => this.refreshAfterMutation(), + error: (error: HttpErrorResponse) => this.error.set(this.errorMessage(error)), + }); + } + + private refreshAfterMutation(): void { + this.directoryChanged.emit(); + this.loadPlayers(); + } + + private errorMessage(error: HttpErrorResponse): string { + const detail = typeof error.error?.message === 'string' ? error.error.message : ''; + const fallback = error.status === 403 ? 'Keine Berechtigung.' : 'Zuordnung konnte nicht geändert werden.'; + return detail ? `${fallback} ${detail}` : fallback; + } +} diff --git a/myteamwallet_frontend_modern/src/app/features/users/user-edit.ts b/myteamwallet_frontend_modern/src/app/features/users/user-edit.ts new file mode 100644 index 0000000..e572592 --- /dev/null +++ b/myteamwallet_frontend_modern/src/app/features/users/user-edit.ts @@ -0,0 +1,85 @@ +import { Component, EventEmitter, Input, OnChanges, Output } from '@angular/core'; +import { MatButtonModule } from '@angular/material/button'; +import { MatFormFieldModule } from '@angular/material/form-field'; +import { MatInputModule } from '@angular/material/input'; +import { MatSelectModule } from '@angular/material/select'; +import { AdminUserDirectorySummary, AdminUserRoleId } from '../../models/user-directory.model'; + +export interface UserEditValue { + firstName: string; + lastName: string; + role: AdminUserRoleId; +} + +@Component({ + selector: 'app-user-edit', + imports: [MatButtonModule, MatFormFieldModule, MatInputModule, MatSelectModule], + template: ` +
+
+ + Vorname + + + + Nachname + + + + Globale Rolle + + +
+ @if (self) { +

Die eigene Administratorrolle kann hier nicht entzogen werden.

+ } +
+ + +
+
+ `, + styles: ` + .user-edit { padding: 18px; border-radius: 18px; background: var(--mat-sys-surface-container-low); } + .user-edit__fields { display: grid; grid-template-columns: 1fr 1fr 180px; gap: 12px; } + .user-edit__hint { margin: 10px 0 0; color: var(--mat-sys-on-surface-variant); } + .user-edit__actions { display: flex; justify-content: flex-end; gap: 8px; margin-top: 16px; } + @media (max-width: 700px) { + .user-edit__fields { grid-template-columns: 1fr; } + .user-edit__actions { align-items: stretch; flex-direction: column-reverse; } + } + `, +}) +export class UserEdit implements OnChanges { + @Input({ required: true }) user!: AdminUserDirectorySummary; + @Input() self = false; + @Input() saving = false; + @Output() readonly saved = new EventEmitter(); + @Output() readonly cancel = new EventEmitter(); + + protected firstName = ''; + protected lastName = ''; + protected role: AdminUserRoleId = 2; + + ngOnChanges(): void { + this.firstName = this.user.firstName ?? ''; + this.lastName = this.user.lastName ?? ''; + this.role = this.user.role?.id === 1 ? 1 : 2; + } + + protected submit(): void { + const firstName = this.firstName.trim(); + const lastName = this.lastName.trim(); + if (!firstName || !lastName || this.saving) return; + this.saved.emit({ firstName, lastName, role: this.self ? 1 : this.role }); + } + + protected setRole(value: string): void { + this.role = value === '1' ? 1 : 2; + } +} diff --git a/myteamwallet_frontend_modern/src/app/features/users/users.html b/myteamwallet_frontend_modern/src/app/features/users/users.html new file mode 100644 index 0000000..7d46401 --- /dev/null +++ b/myteamwallet_frontend_modern/src/app/features/users/users.html @@ -0,0 +1,123 @@ +
+ arrow_backZurück +
+

Organisation

+

Benutzer

+

Konten und sichtbare Teamzuordnungen im Überblick.

+
+ + + + @if (mutationError()) { + + } + + @if (loading()) { +
+ + Benutzer werden geladen … +
+ } @else if (loadError()) { + + } @else if (directory()?.data?.length === 0) { +
+ group_off + Keine Benutzer gefunden + Versuche einen anderen Suchbegriff. +
+ } @else if (directory()) { +
+ @for (user of directory()!.data; track user.id) { +
+
+ +
+
+

{{ fullName(user) }}

+ + {{ statusName(user.status?.id) }} + +
+ @if (adminDetails(user); as details) { + {{ details.email ?? 'Keine E-Mail' }} · {{ roleName(details.role?.id) }} + } + @if (user.assignments.length === 0) { + Keine Spielerzuordnung sichtbar + } @else { +
+ @for (assignment of user.assignments; track assignment.id) { + + {{ assignment.team.name }} · {{ assignment.firstName }} {{ assignment.lastName }} · + {{ teamRoleName(assignment.teamRole?.name) }} + @if (!assignment.active) { · Inaktiv } + + } +
+ } +
+
+ + @if (adminDetails(user); as details) { +
+ + + +
+ + @if (editingUserId() === user.id) { + + } + @if (assignmentUserId() === user.id) { + + } + } +
+ } +
+ + + } +
diff --git a/myteamwallet_frontend_modern/src/app/features/users/users.scss b/myteamwallet_frontend_modern/src/app/features/users/users.scss new file mode 100644 index 0000000..730f9ce --- /dev/null +++ b/myteamwallet_frontend_modern/src/app/features/users/users.scss @@ -0,0 +1,191 @@ +:host { + display: block; + min-height: 100dvh; + background: var(--mat-sys-surface); +} + +.users-page { + max-width: 960px; + margin: 0 auto; + padding: 24px 28px 40px; +} + +.back-link { + margin-left: -12px; +} + +.page-header { + margin: 20px 0 26px; +} + +h1, +h2, +p { + margin-top: 0; +} + +h1 { + margin-bottom: 8px; + font-size: clamp(2rem, 4vw, 3rem); +} + +.page-header > p:last-child { + color: var(--mat-sys-on-surface-variant); +} + +.eyebrow { + margin-bottom: 6px; + color: var(--mat-sys-primary); + font-size: 0.75rem; + font-weight: 700; + letter-spacing: 0.08em; + text-transform: uppercase; +} + +.directory-search { + display: flex; + align-items: center; + gap: 10px; + margin-bottom: 22px; +} + +.directory-search mat-form-field { + flex: 1; +} + +.directory { + border-top: 1px solid var(--mat-sys-outline-variant); +} + +.user-row { + display: grid; + gap: 16px; + padding: 20px 0; + border-bottom: 1px solid var(--mat-sys-outline-variant); +} + +.user-row--inactive .avatar { + filter: grayscale(1); + opacity: 0.65; +} + +.user-row__summary { + display: grid; + grid-template-columns: 48px minmax(0, 1fr); + gap: 14px; +} + +.avatar { + width: 48px; + height: 48px; + display: grid; + place-items: center; + border-radius: 50%; + background: var(--mat-sys-primary-container); + color: var(--mat-sys-on-primary-container); + font-weight: 700; +} + +.user-row__identity, +.assignment-summary { + display: grid; + gap: 4px; + min-width: 0; +} + +.user-row__identity > span, +.assignment-summary span { + color: var(--mat-sys-on-surface-variant); +} + +.user-row__name { + display: flex; + align-items: center; + gap: 10px; +} + +.user-row__name h2 { + margin-bottom: 0; + font-size: 1.08rem; +} + +.status { + display: inline-flex; + align-items: center; + min-height: 24px; + padding: 0 9px; + border-radius: 999px; + background: var(--mat-sys-primary-container); + color: var(--mat-sys-on-primary-container); + font-size: 0.75rem; + font-weight: 700; +} + +.status--inactive { + background: var(--mat-sys-surface-container-high); + color: var(--mat-sys-on-surface-variant); +} + +.user-row__actions { + display: flex; + justify-content: flex-end; + flex-wrap: wrap; + gap: 6px; +} + +.page-state { + min-height: 240px; + display: grid; + place-content: center; + justify-items: center; + gap: 10px; + padding: 24px; + color: var(--mat-sys-on-surface-variant); + text-align: center; +} + +.page-state--error { + color: var(--mat-sys-error); +} + +.message { + margin-bottom: 16px; + padding: 12px 14px; + border-radius: 12px; +} + +.message--error { + color: var(--mat-sys-on-error-container); + background: var(--mat-sys-error-container); +} + +.pagination { + display: flex; + align-items: center; + justify-content: space-between; + gap: 12px; + margin-top: 18px; + color: var(--mat-sys-on-surface-variant); +} + +@media (max-width: 700px) { + .users-page { + padding: 20px 16px 32px; + } + + .directory-search, + .user-row__actions { + align-items: stretch; + flex-direction: column; + } + + .directory-search button, + .user-row__actions button { + width: 100%; + } + + .pagination span { + font-size: 0.8rem; + text-align: center; + } +} diff --git a/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts b/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts new file mode 100644 index 0000000..c65211a --- /dev/null +++ b/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts @@ -0,0 +1,381 @@ +import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing'; +import { provideHttpClient } from '@angular/common/http'; +import { signal } from '@angular/core'; +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { MatDialog } from '@angular/material/dialog'; +import { provideRouter } from '@angular/router'; +import { Subject } from 'rxjs'; +import { environment } from '../../../environments/environment'; +import { AuthStore } from '../../core/auth/auth-store'; +import { AdminPlayerPage, AdminUserDirectorySummary, UserDirectoryPage } from '../../models/user-directory.model'; +import { Users } from './users'; + +const api = `${environment.apiUrl}users/directory`; +const adminApi = `${environment.apiUrl}admin/users`; + +const ada: AdminUserDirectorySummary = { + id: 7, + firstName: 'Ada', + lastName: 'Lovelace', + email: 'ada@example.test', + role: { id: 2, name: 'User' }, + status: { id: 1, name: 'Active' }, + assignments: [ + { + id: 101, + firstName: 'Ada', + lastName: 'Lovelace', + active: true, + team: { id: 5, name: 'First Team', alias: 'first' }, + teamRole: { id: 1, name: 'player' }, + }, + ], +}; + +const admin: AdminUserDirectorySummary = { + id: 1, + firstName: 'Grace', + lastName: 'Admin', + email: 'grace@example.test', + role: { id: 1, name: 'Admin' }, + status: { id: 1, name: 'Active' }, + assignments: [], +}; + +function directoryPage(data = [ada], page = 1, total = data.length, hasNextPage = false): UserDirectoryPage { + return { data, page, limit: 20, total, hasNextPage }; +} + +function playersPage(overrides: Partial = {}): AdminPlayerPage { + return { + data: [ + { + id: 202, + firstName: 'Linus', + lastName: 'Player', + active: true, + team: { id: 6, name: 'Second Team', alias: 'second' }, + currentUser: null, + }, + ], + page: 1, + limit: 20, + total: 1, + hasNextPage: false, + ...overrides, + }; +} + +describe('Users directory', () => { + let fixture: ComponentFixture; + let http: HttpTestingController; + let isAdmin: ReturnType>; + let currentUser: ReturnType>; + let closeDialog: Subject; + let dialog: { open: ReturnType }; + + beforeEach(async () => { + isAdmin = signal(false); + currentUser = signal({ id: 99, firstName: 'Nora', lastName: 'Viewer', role: { id: 2 } }); + closeDialog = new Subject(); + dialog = { open: vi.fn(() => ({ afterClosed: () => closeDialog.asObservable() })) }; + + await TestBed.configureTestingModule({ + imports: [Users], + providers: [ + provideHttpClient(), + provideHttpClientTesting(), + provideRouter([]), + { provide: AuthStore, useValue: { isGlobalAdmin: isAdmin, currentUser } }, + { provide: MatDialog, useValue: dialog }, + ], + }).compileComponents(); + + http = TestBed.inject(HttpTestingController); + }); + + afterEach(() => http.verify()); + + function create(): void { + fixture = TestBed.createComponent(Users); + fixture.detectChanges(); + } + + function text(): string { + return (fixture.nativeElement as HTMLElement).textContent ?? ''; + } + + function button(label: string): HTMLButtonElement { + const match = [...(fixture.nativeElement as HTMLElement).querySelectorAll('button')].find((element) => + element.textContent?.includes(label), + ); + if (!match) throw new Error(`Missing button: ${label}`); + return match as HTMLButtonElement; + } + + function flushDirectory(page = directoryPage()): void { + http.expectOne(`${api}?page=1&limit=20`).flush(page); + fixture.detectChanges(); + } + + it('loads the first directory page, searches from page one, and pages forward', () => { + create(); + flushDirectory(directoryPage([ada], 1, 12, true)); + + expect(text()).toContain('Ada Lovelace'); + expect(text()).toContain('First Team'); + expect(text()).toContain('Aktiv'); + + const search = (fixture.nativeElement as HTMLElement).querySelector('input[type="search"]')!; + search.value = ' Linus '; + search.dispatchEvent(new Event('input')); + fixture.detectChanges(); + search.closest('form')!.dispatchEvent(new Event('submit')); + fixture.detectChanges(); + http.expectOne(`${api}?page=1&limit=20&search=Linus`).flush(directoryPage([], 1)); + fixture.detectChanges(); + + search.value = ''; + search.dispatchEvent(new Event('input')); + fixture.detectChanges(); + search.closest('form')!.dispatchEvent(new Event('submit')); + fixture.detectChanges(); + http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage([ada], 1, 12, true)); + fixture.detectChanges(); + button('Weiter').click(); + fixture.detectChanges(); + http.expectOne(`${api}?page=2&limit=20`).flush(directoryPage([admin], 2, 12, false)); + }); + + it('redacts admin-only data and controls for a non-admin even if extra fields arrive', () => { + create(); + flushDirectory(directoryPage([ada])); + + expect(text()).toContain('Ada Lovelace'); + expect(text()).not.toContain('ada@example.test'); + expect(text()).not.toContain('Globale Rolle'); + expect(text()).not.toContain('Bearbeiten'); + expect(text()).not.toContain('Zuordnungen verwalten'); + }); + + it('shows admin fields and actions while disabling self-demotion and self-deactivation', () => { + isAdmin.set(true); + currentUser.set({ id: 1, firstName: 'Grace', lastName: 'Admin', role: { id: 1 } }); + create(); + http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage([admin, ada])); + fixture.detectChanges(); + + expect(text()).toContain('ada@example.test'); + expect(text()).toContain('Administrator'); + expect(text()).toContain('Bearbeiten'); + expect(text()).toContain('Zuordnungen verwalten'); + const selfRow = (fixture.nativeElement as HTMLElement).querySelector('[data-user-id="1"]')!; + const selfButtons = [...selfRow.querySelectorAll('button')]; + expect(selfButtons.find((item) => item.textContent?.includes('Deaktivieren'))?.disabled).toBe(true); + selfButtons.find((item) => item.textContent?.includes('Bearbeiten'))?.click(); + fixture.detectChanges(); + expect(selfRow.querySelector('select[name="role"]')?.disabled).toBe(true); + }); + + it('edits a profile and role pessimistically, then reloads the directory', () => { + isAdmin.set(true); + create(); + flushDirectory(); + button('Bearbeiten').click(); + fixture.detectChanges(); + + const firstName = (fixture.nativeElement as HTMLElement).querySelector('input[name="firstName"]')!; + const lastName = (fixture.nativeElement as HTMLElement).querySelector('input[name="lastName"]')!; + const role = (fixture.nativeElement as HTMLElement).querySelector('select[name="role"]')!; + firstName.value = 'Augusta'; + firstName.dispatchEvent(new Event('input')); + lastName.value = 'King'; + lastName.dispatchEvent(new Event('input')); + role.value = '1'; + role.dispatchEvent(new Event('change')); + firstName.closest('form')!.dispatchEvent(new Event('submit')); + fixture.detectChanges(); + + const profile = http.expectOne(`${adminApi}/7/profile`); + expect(profile.request.body).toEqual({ firstName: 'Augusta', lastName: 'King' }); + expect(text()).toContain('Wird gespeichert'); + profile.flush(ada); + const roleRequest = http.expectOne(`${adminApi}/7/role`); + expect(roleRequest.request.body).toEqual({ role: 1 }); + roleRequest.flush(ada); + http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage([{ ...ada, firstName: 'Augusta', lastName: 'King' }])); + fixture.detectChanges(); + expect(text()).toContain('Augusta King'); + }); + + it('reloads authoritative directory state when a role change fails after the profile was saved', () => { + isAdmin.set(true); + create(); + flushDirectory(); + button('Bearbeiten').click(); + fixture.detectChanges(); + const role = (fixture.nativeElement as HTMLElement).querySelector('select[name="role"]')!; + role.value = '1'; + role.dispatchEvent(new Event('change')); + role.closest('form')!.dispatchEvent(new Event('submit')); + + http.expectOne(`${adminApi}/7/profile`).flush(ada); + http.expectOne(`${adminApi}/7/role`).flush( + { message: 'At least one active admin must remain' }, + { status: 409, statusText: 'Conflict' }, + ); + http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage()); + fixture.detectChanges(); + expect(text()).toContain('At least one active admin must remain'); + }); + + it('confirms status changes before mutating and refreshes after success', () => { + isAdmin.set(true); + create(); + flushDirectory(); + button('Deaktivieren').click(); + + expect(dialog.open).toHaveBeenCalled(); + expect(dialog.open.mock.calls[0][1].data.message).toContain('Ada Lovelace'); + http.expectNone(`${adminApi}/7/status`); + closeDialog.next(true); + const request = http.expectOne(`${adminApi}/7/status`); + expect(request.request.body).toEqual({ status: 2 }); + request.flush(ada); + http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage([{ ...ada, status: { id: 2, name: 'Inactive' } }])); + }); + + it('loads searchable player results and assigns an unlinked player before refreshing both lists', () => { + isAdmin.set(true); + create(); + flushDirectory(); + button('Zuordnungen verwalten').click(); + fixture.detectChanges(); + http.expectOne(`${adminApi}/players?assignment=all&page=1&limit=20`).flush(playersPage()); + fixture.detectChanges(); + expect(text()).toContain('Linus Player'); + expect(text()).toContain('Second Team'); + + button('Zuordnen').click(); + const assign = http.expectOne(`${adminApi}/7/players/202`); + expect(assign.request.method).toBe('PUT'); + assign.flush(ada); + http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage()); + http.expectOne(`${adminApi}/players?assignment=all&page=1&limit=20`).flush(playersPage()); + }); + + it('confirms unlinking and reassignment with the affected user names', () => { + isAdmin.set(true); + create(); + flushDirectory(); + button('Zuordnungen verwalten').click(); + fixture.detectChanges(); + http.expectOne(`${adminApi}/players?assignment=all&page=1&limit=20`).flush( + playersPage({ + data: [ + { + id: 101, + firstName: 'Ada', + lastName: 'Lovelace', + active: true, + team: { id: 5, name: 'First Team', alias: 'first' }, + currentUser: { id: 7, firstName: 'Ada', lastName: 'Lovelace', status: { id: 1, name: 'Active' } }, + }, + { + id: 203, + firstName: 'Other', + lastName: 'Player', + active: true, + team: { id: 6, name: 'Second Team', alias: 'second' }, + currentUser: { id: 8, firstName: 'Alan', lastName: 'Turing', status: { id: 1, name: 'Active' } }, + }, + ], + total: 2, + }), + ); + fixture.detectChanges(); + + button('Verknüpfung lösen').click(); + expect(dialog.open.mock.calls[0][1].data.message).toContain('Ada Lovelace'); + closeDialog.next(false); + http.expectNone(`${adminApi}/7/players/101`); + + closeDialog = new Subject(); + dialog.open.mockReturnValue({ afterClosed: () => closeDialog.asObservable() }); + button('Neu zuordnen').click(); + const message = dialog.open.mock.calls[1][1].data.message; + expect(message).toContain('Alan Turing'); + expect(message).toContain('Ada Lovelace'); + closeDialog.next(true); + http.expectOne(`${adminApi}/7/players/203`).flush(ada); + http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage()); + http.expectOne(`${adminApi}/players?assignment=all&page=1&limit=20`).flush(playersPage()); + }); + + it('keeps an unlink pending until success and then reloads directory and player results', () => { + isAdmin.set(true); + create(); + flushDirectory(); + button('Zuordnungen verwalten').click(); + fixture.detectChanges(); + http.expectOne(`${adminApi}/players?assignment=all&page=1&limit=20`).flush( + playersPage({ + data: [ + { + id: 101, + firstName: 'Ada', + lastName: 'Lovelace', + active: true, + team: { id: 5, name: 'First Team', alias: 'first' }, + currentUser: { id: 7, firstName: 'Ada', lastName: 'Lovelace', status: { id: 1, name: 'Active' } }, + }, + ], + }), + ); + fixture.detectChanges(); + + button('Verknüpfung lösen').click(); + http.expectNone(`${adminApi}/7/players/101`); + closeDialog.next(true); + fixture.detectChanges(); + const unlink = http.expectOne(`${adminApi}/7/players/101`); + expect(unlink.request.method).toBe('DELETE'); + expect(text()).toContain('Wird gelöst'); + unlink.flush(ada); + http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage()); + http.expectOne(`${adminApi}/players?assignment=all&page=1&limit=20`).flush(playersPage()); + }); + + it('renders loading, empty, general error, and retry states', () => { + create(); + expect((fixture.nativeElement as HTMLElement).querySelector('[role="progressbar"]')).not.toBeNull(); + http.expectOne(`${api}?page=1&limit=20`).flush('broken', { status: 500, statusText: 'Server Error' }); + fixture.detectChanges(); + expect(text()).toContain('Benutzer konnten nicht geladen werden'); + button('Erneut versuchen').click(); + http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage([])); + fixture.detectChanges(); + expect(text()).toContain('Keine Benutzer gefunden'); + }); + + it('surfaces directory and mutation authorization errors clearly', () => { + create(); + http.expectOne(`${api}?page=1&limit=20`).flush({ message: 'Forbidden' }, { status: 403, statusText: 'Forbidden' }); + fixture.detectChanges(); + expect(text()).toContain('Keine Berechtigung'); + + isAdmin.set(true); + button('Erneut versuchen').click(); + http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage()); + fixture.detectChanges(); + button('Deaktivieren').click(); + closeDialog.next(true); + http.expectOne(`${adminApi}/7/status`).flush( + { message: 'At least one active admin must remain' }, + { status: 403, statusText: 'Forbidden' }, + ); + fixture.detectChanges(); + expect(text()).toContain('Keine Berechtigung'); + expect(text()).toContain('At least one active admin must remain'); + }); +}); diff --git a/myteamwallet_frontend_modern/src/app/features/users/users.ts b/myteamwallet_frontend_modern/src/app/features/users/users.ts index 1d81d4d..bc9f6e5 100644 --- a/myteamwallet_frontend_modern/src/app/features/users/users.ts +++ b/myteamwallet_frontend_modern/src/app/features/users/users.ts @@ -1,13 +1,215 @@ -import { Component } from '@angular/core'; +import { HttpErrorResponse } from '@angular/common/http'; +import { Component, inject, signal } from '@angular/core'; +import { RouterLink } from '@angular/router'; +import { MatButtonModule } from '@angular/material/button'; +import { MatDialog } from '@angular/material/dialog'; +import { MatFormFieldModule } from '@angular/material/form-field'; +import { MatIconModule } from '@angular/material/icon'; +import { MatInputModule } from '@angular/material/input'; +import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; +import { filter, finalize, of, switchMap, take } from 'rxjs'; +import { AuthStore } from '../../core/auth/auth-store'; +import { AdminUsersApi } from '../../core/users/admin-users-api'; +import { UsersApi } from '../../core/users/users-api'; +import { + AdminUserDirectorySummary, + AdminUserStatusId, + UserDirectoryPage, + UserDirectoryRecord, +} from '../../models/user-directory.model'; +import { ConfirmDialog } from '../../shared/confirm-dialog/confirm-dialog'; +import { PlayerAssignments } from './player-assignments'; +import { UserEdit, UserEditValue } from './user-edit'; @Component({ selector: 'app-users', - template: ` -
-

Organisation

-

Benutzer

-

Benutzerverzeichnis wird vorbereitet.

-
- `, + imports: [ + RouterLink, + MatButtonModule, + MatFormFieldModule, + MatIconModule, + MatInputModule, + MatProgressSpinnerModule, + PlayerAssignments, + UserEdit, + ], + templateUrl: './users.html', + styleUrl: './users.scss', }) -export class Users {} +export class Users { + private readonly usersApi = inject(UsersApi); + private readonly adminUsersApi = inject(AdminUsersApi); + private readonly authStore = inject(AuthStore); + private readonly dialog = inject(MatDialog); + + protected readonly isAdmin = this.authStore.isGlobalAdmin; + protected readonly currentUser = this.authStore.currentUser; + protected readonly directory = signal(null); + protected readonly loading = signal(true); + protected readonly loadError = signal(null); + protected readonly mutationError = signal(null); + protected readonly searchDraft = signal(''); + protected readonly search = signal(''); + protected readonly page = signal(1); + protected readonly limit = 20; + protected readonly editingUserId = signal(null); + protected readonly assignmentUserId = signal(null); + protected readonly pendingUserId = signal(null); + + constructor() { + this.loadDirectory(); + } + + protected loadDirectory(): void { + this.loading.set(true); + this.loadError.set(null); + const search = this.search(); + this.usersApi + .loadDirectory({ page: this.page(), limit: this.limit, ...(search ? { search } : {}) }) + .pipe(finalize(() => this.loading.set(false))) + .subscribe({ + next: (directory) => this.directory.set(directory), + error: (error: HttpErrorResponse) => { + this.directory.set(null); + this.loadError.set(this.errorMessage(error, 'Benutzer konnten nicht geladen werden.')); + }, + }); + } + + protected submitSearch(): void { + this.search.set(this.searchDraft().trim()); + this.page.set(1); + this.loadDirectory(); + } + + protected previousPage(): void { + if (this.page() <= 1 || this.loading()) return; + this.page.update((value) => value - 1); + this.loadDirectory(); + } + + protected nextPage(): void { + if (!this.directory()?.hasNextPage || this.loading()) return; + this.page.update((value) => value + 1); + this.loadDirectory(); + } + + protected adminDetails(user: UserDirectoryRecord): AdminUserDirectorySummary | null { + if (!this.isAdmin()) return null; + return 'email' in user && 'role' in user ? user : null; + } + + protected fullName(user: UserDirectoryRecord): string { + return [user.firstName, user.lastName].filter(Boolean).join(' ') || `Benutzer ${user.id}`; + } + + protected initials(user: UserDirectoryRecord): string { + const value = `${user.firstName?.charAt(0) ?? ''}${user.lastName?.charAt(0) ?? ''}`.trim(); + return value || '?'; + } + + protected statusName(statusId?: number): string { + return statusId === 2 ? 'Inaktiv' : 'Aktiv'; + } + + protected roleName(roleId?: number): string { + return roleId === 1 ? 'Administrator' : 'Benutzer'; + } + + protected teamRoleName(name?: string): string { + return ( + { + player: 'Spieler', + scnd_treasurer: '2. Kassenwart', + captain: 'Kapitän', + treasurer: 'Kassenwart', + coach: 'Trainer', + }[name ?? ''] ?? 'Spieler' + ); + } + + protected toggleEdit(userId: number): void { + this.assignmentUserId.set(null); + this.editingUserId.update((value) => (value === userId ? null : userId)); + this.mutationError.set(null); + } + + protected toggleAssignments(userId: number): void { + this.editingUserId.set(null); + this.assignmentUserId.update((value) => (value === userId ? null : userId)); + this.mutationError.set(null); + } + + protected saveEdit(user: AdminUserDirectorySummary, value: UserEditValue): void { + if (this.pendingUserId() !== null) return; + this.pendingUserId.set(user.id); + this.mutationError.set(null); + const profileRequest = this.adminUsersApi.updateProfile(user.id, { + firstName: value.firstName, + lastName: value.lastName, + }); + const roleId = user.role?.id; + profileRequest + .pipe( + switchMap(() => + roleId === value.role ? of(user) : this.adminUsersApi.updateRole(user.id, { role: value.role }), + ), + finalize(() => this.pendingUserId.set(null)), + ) + .subscribe({ + next: () => { + this.editingUserId.set(null); + this.loadDirectory(); + }, + error: (error: HttpErrorResponse) => { + this.mutationError.set(this.errorMessage(error, 'Änderung fehlgeschlagen.')); + this.loadDirectory(); + }, + }); + } + + protected changeStatus(user: AdminUserDirectorySummary): void { + if (this.isSelf(user) || this.pendingUserId() !== null) return; + const isActive = user.status?.id !== 2; + const status: AdminUserStatusId = isActive ? 2 : 1; + this.dialog + .open(ConfirmDialog, { + data: { + title: isActive ? 'Benutzer deaktivieren?' : 'Benutzer aktivieren?', + message: `${this.fullName(user)} wird ${isActive ? 'deaktiviert' : 'aktiviert'}.`, + confirmLabel: isActive ? 'Deaktivieren' : 'Aktivieren', + }, + restoreFocus: true, + }) + .afterClosed() + .pipe(filter(Boolean), take(1)) + .subscribe(() => this.updateStatus(user.id, status)); + } + + protected isSelf(user: UserDirectoryRecord): boolean { + return user.id === this.currentUser()?.id; + } + + protected assignmentsChanged(): void { + this.loadDirectory(); + } + + private updateStatus(userId: number, status: AdminUserStatusId): void { + this.pendingUserId.set(userId); + this.mutationError.set(null); + this.adminUsersApi + .updateStatus(userId, { status }) + .pipe(finalize(() => this.pendingUserId.set(null))) + .subscribe({ + next: () => this.loadDirectory(), + error: (error: HttpErrorResponse) => + this.mutationError.set(this.errorMessage(error, 'Status konnte nicht geändert werden.')), + }); + } + + private errorMessage(error: HttpErrorResponse, fallback: string): string { + const detail = typeof error.error?.message === 'string' ? error.error.message : ''; + if (error.status === 403) return `Keine Berechtigung. ${detail}`.trim(); + return detail ? `${fallback} ${detail}` : fallback; + } +} From 9dc9dc3dcf2590e67df0e86b8b6bf1b032e4eb98 Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Sat, 1 Aug 2026 10:32:51 +0200 Subject: [PATCH 11/15] fix: stabilize admin user UI state --- .../app/features/users/player-assignments.ts | 146 +++++++++++----- .../src/app/features/users/user-edit.ts | 23 ++- .../src/app/features/users/users.html | 35 +++- .../src/app/features/users/users.scss | 7 + .../src/app/features/users/users.spec.ts | 163 ++++++++++++++++-- .../src/app/features/users/users.ts | 68 +++++--- 6 files changed, 358 insertions(+), 84 deletions(-) diff --git a/myteamwallet_frontend_modern/src/app/features/users/player-assignments.ts b/myteamwallet_frontend_modern/src/app/features/users/player-assignments.ts index 697eaa7..c614a79 100644 --- a/myteamwallet_frontend_modern/src/app/features/users/player-assignments.ts +++ b/myteamwallet_frontend_modern/src/app/features/users/player-assignments.ts @@ -1,12 +1,13 @@ import { HttpErrorResponse } from '@angular/common/http'; -import { Component, EventEmitter, Input, OnChanges, Output, inject, signal } from '@angular/core'; +import { Component, DestroyRef, EventEmitter, Input, OnChanges, Output, SimpleChanges, inject, signal } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { MatButtonModule } from '@angular/material/button'; import { MatDialog } from '@angular/material/dialog'; import { MatFormFieldModule } from '@angular/material/form-field'; import { MatIconModule } from '@angular/material/icon'; import { MatInputModule } from '@angular/material/input'; import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; -import { filter, finalize, take } from 'rxjs'; +import { EMPTY, Subject, catchError, filter, finalize, switchMap, take, tap } from 'rxjs'; import { AdminUsersApi } from '../../core/users/admin-users-api'; import { AdminPlayerPage, @@ -25,34 +26,51 @@ import { ConfirmDialog } from '../../shared/confirm-dialog/confirm-dialog'; MatProgressSpinnerModule, ], template: ` -
+

Spielerzuordnungen

Spieler teamübergreifend suchen und sicher verknüpfen.

- +
- @if (error()) { + @if (loadError()) { } + @if (mutationError()) { + + } @if (loading()) { -
- } @else if (!error() && players()?.data?.length === 0) { +
+ + Spieler werden geladen … +
+ } @else if (!loadError() && players()?.data?.length === 0) {
Keine Spieler gefunden.
} @else if (players()) {
@@ -98,15 +116,15 @@ import { ConfirmDialog } from '../../shared/confirm-dialog/confirm-dialog';
} -
- Seite {{ players()!.page }} - -
+ }
`, @@ -135,21 +153,58 @@ import { ConfirmDialog } from '../../shared/confirm-dialog/confirm-dialog'; export class PlayerAssignments implements OnChanges { private readonly adminUsersApi = inject(AdminUsersApi); private readonly dialog = inject(MatDialog); + private readonly destroyRef = inject(DestroyRef); + private readonly playerRequests = new Subject(); @Input({ required: true }) user!: AdminUserDirectorySummary; @Output() readonly directoryChanged = new EventEmitter(); @Output() readonly closed = new EventEmitter(); + @Output() readonly busyChange = new EventEmitter(); protected readonly players = signal(null); protected readonly loading = signal(false); - protected readonly error = signal(null); + protected readonly loadError = signal(null); + protected readonly mutationError = signal(null); protected readonly pendingPlayerId = signal(null); protected searchDraft = ''; private search = ''; private page = 1; private readonly limit = 20; - ngOnChanges(): void { + constructor() { + this.playerRequests + .pipe( + switchMap(() => { + this.loading.set(true); + this.loadError.set(null); + this.players.set(null); + return this.adminUsersApi + .loadPlayers({ + assignment: 'all', + page: this.page, + limit: this.limit, + ...(this.search ? { search: this.search } : {}), + }) + .pipe( + tap((players) => this.players.set(players)), + catchError((error: HttpErrorResponse) => { + this.loadError.set(this.loadErrorMessage(error)); + return EMPTY; + }), + finalize(() => this.loading.set(false)), + ); + }), + takeUntilDestroyed(this.destroyRef), + ) + .subscribe(); + } + + ngOnChanges(changes: SimpleChanges): void { + const userChange = changes['user']; + if (!userChange) return; + const previous = userChange.previousValue as AdminUserDirectorySummary | undefined; + const current = userChange.currentValue as AdminUserDirectorySummary; + if (!userChange.firstChange && previous?.id === current.id) return; this.searchDraft = ''; this.search = ''; this.page = 1; @@ -157,36 +212,26 @@ export class PlayerAssignments implements OnChanges { } protected loadPlayers(): void { - this.loading.set(true); - this.error.set(null); - this.adminUsersApi - .loadPlayers({ - assignment: 'all', - page: this.page, - limit: this.limit, - ...(this.search ? { search: this.search } : {}), - }) - .pipe(finalize(() => this.loading.set(false))) - .subscribe({ - next: (players) => this.players.set(players), - error: (error: HttpErrorResponse) => this.error.set(this.errorMessage(error)), - }); + this.playerRequests.next(); } protected submitSearch(): void { - this.search = this.searchDraft.trim(); + if (this.pendingPlayerId() !== null) return; + const search = this.searchDraft.trim(); + if (this.loading() && search === this.search && this.page === 1) return; + this.search = search; this.page = 1; this.loadPlayers(); } protected previousPage(): void { - if (this.page <= 1 || this.loading()) return; + if (this.page <= 1 || this.loading() || this.pendingPlayerId() !== null) return; this.page -= 1; this.loadPlayers(); } protected nextPage(): void { - if (!this.players()?.hasNextPage || this.loading()) return; + if (!this.players()?.hasNextPage || this.loading() || this.pendingPlayerId() !== null) return; this.page += 1; this.loadPlayers(); } @@ -195,6 +240,10 @@ export class PlayerAssignments implements OnChanges { this.runAssignment(player); } + protected close(): void { + if (this.pendingPlayerId() === null) this.closed.emit(); + } + protected confirmUnlink(player: AdminPlayerSummary): void { this.confirm({ title: 'Verknüpfung lösen?', @@ -223,32 +272,32 @@ export class PlayerAssignments implements OnChanges { return this.dialog .open(ConfirmDialog, { data, restoreFocus: true }) .afterClosed() - .pipe(filter(Boolean), take(1)); + .pipe(filter(Boolean), take(1), takeUntilDestroyed(this.destroyRef)); } private runAssignment(player: AdminPlayerSummary): void { if (this.pendingPlayerId() !== null) return; - this.pendingPlayerId.set(player.id); - this.error.set(null); + this.setPending(player.id); + this.mutationError.set(null); this.adminUsersApi .assignPlayer(this.user.id, player.id) - .pipe(finalize(() => this.pendingPlayerId.set(null))) + .pipe(finalize(() => this.setPending(null)), takeUntilDestroyed(this.destroyRef)) .subscribe({ next: () => this.refreshAfterMutation(), - error: (error: HttpErrorResponse) => this.error.set(this.errorMessage(error)), + error: (error: HttpErrorResponse) => this.mutationError.set(this.mutationErrorMessage(error)), }); } private runUnlink(player: AdminPlayerSummary): void { if (this.pendingPlayerId() !== null) return; - this.pendingPlayerId.set(player.id); - this.error.set(null); + this.setPending(player.id); + this.mutationError.set(null); this.adminUsersApi .unlinkPlayer(this.user.id, player.id) - .pipe(finalize(() => this.pendingPlayerId.set(null))) + .pipe(finalize(() => this.setPending(null)), takeUntilDestroyed(this.destroyRef)) .subscribe({ next: () => this.refreshAfterMutation(), - error: (error: HttpErrorResponse) => this.error.set(this.errorMessage(error)), + error: (error: HttpErrorResponse) => this.mutationError.set(this.mutationErrorMessage(error)), }); } @@ -257,7 +306,18 @@ export class PlayerAssignments implements OnChanges { this.loadPlayers(); } - private errorMessage(error: HttpErrorResponse): string { + private setPending(playerId: number | null): void { + this.pendingPlayerId.set(playerId); + this.busyChange.emit(playerId !== null); + } + + private loadErrorMessage(error: HttpErrorResponse): string { + const detail = typeof error.error?.message === 'string' ? error.error.message : ''; + const fallback = error.status === 403 ? 'Keine Berechtigung.' : 'Spieler konnten nicht geladen werden.'; + return detail ? `${fallback} ${detail}` : fallback; + } + + private mutationErrorMessage(error: HttpErrorResponse): string { const detail = typeof error.error?.message === 'string' ? error.error.message : ''; const fallback = error.status === 403 ? 'Keine Berechtigung.' : 'Zuordnung konnte nicht geändert werden.'; return detail ? `${fallback} ${detail}` : fallback; diff --git a/myteamwallet_frontend_modern/src/app/features/users/user-edit.ts b/myteamwallet_frontend_modern/src/app/features/users/user-edit.ts index e572592..3a1e3bd 100644 --- a/myteamwallet_frontend_modern/src/app/features/users/user-edit.ts +++ b/myteamwallet_frontend_modern/src/app/features/users/user-edit.ts @@ -1,4 +1,4 @@ -import { Component, EventEmitter, Input, OnChanges, Output } from '@angular/core'; +import { Component, EventEmitter, Input, OnChanges, Output, SimpleChanges } from '@angular/core'; import { MatButtonModule } from '@angular/material/button'; import { MatFormFieldModule } from '@angular/material/form-field'; import { MatInputModule } from '@angular/material/input'; @@ -66,10 +66,23 @@ export class UserEdit implements OnChanges { protected lastName = ''; protected role: AdminUserRoleId = 2; - ngOnChanges(): void { - this.firstName = this.user.firstName ?? ''; - this.lastName = this.user.lastName ?? ''; - this.role = this.user.role?.id === 1 ? 1 : 2; + ngOnChanges(changes: SimpleChanges): void { + const userChange = changes['user']; + if (!userChange) return; + const previous = userChange.previousValue as AdminUserDirectorySummary | undefined; + const current = userChange.currentValue as AdminUserDirectorySummary; + if ( + !userChange.firstChange && + previous?.id === current.id && + previous.firstName === current.firstName && + previous.lastName === current.lastName && + previous.role?.id === current.role?.id + ) { + return; + } + this.firstName = current.firstName ?? ''; + this.lastName = current.lastName ?? ''; + this.role = current.role?.id === 1 ? 1 : 2; } protected submit(): void { diff --git a/myteamwallet_frontend_modern/src/app/features/users/users.html b/myteamwallet_frontend_modern/src/app/features/users/users.html index 7d46401..c04da1c 100644 --- a/myteamwallet_frontend_modern/src/app/features/users/users.html +++ b/myteamwallet_frontend_modern/src/app/features/users/users.html @@ -45,7 +45,12 @@ } @else if (directory()) {
@for (user of directory()!.data; track user.id) { -
+
@@ -76,18 +81,39 @@ @if (adminDetails(user); as details) {
- - + +
+ @if (isSelf(user)) { +

+ Das eigene Konto kann nicht deaktiviert werden. +

+ } @if (editingUserId() === user.id) { } } diff --git a/myteamwallet_frontend_modern/src/app/features/users/users.scss b/myteamwallet_frontend_modern/src/app/features/users/users.scss index 730f9ce..f4b80fa 100644 --- a/myteamwallet_frontend_modern/src/app/features/users/users.scss +++ b/myteamwallet_frontend_modern/src/app/features/users/users.scss @@ -133,6 +133,13 @@ h1 { gap: 6px; } +.self-status-note { + margin: -8px 0 0; + color: var(--mat-sys-on-surface-variant); + font-size: 0.8rem; + text-align: right; +} + .page-state { min-height: 240px; display: grid; diff --git a/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts b/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts index c65211a..fa7e367 100644 --- a/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts +++ b/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts @@ -94,7 +94,7 @@ describe('Users directory', () => { http = TestBed.inject(HttpTestingController); }); - afterEach(() => http.verify()); + afterEach(() => http.verify({ ignoreCancelled: true })); function create(): void { fixture = TestBed.createComponent(Users); @@ -113,6 +113,13 @@ describe('Users directory', () => { return match as HTMLButtonElement; } + + function childButton(host: HTMLElement, label: string): HTMLButtonElement { + const match = [...host.querySelectorAll('button')].find((element) => element.textContent?.includes(label)); + if (!match) throw new Error(`Missing child button: ${label}`); + return match as HTMLButtonElement; + } + function flushDirectory(page = directoryPage()): void { http.expectOne(`${api}?page=1&limit=20`).flush(page); fixture.detectChanges(); @@ -147,6 +154,25 @@ describe('Users directory', () => { http.expectOne(`${api}?page=2&limit=20`).flush(directoryPage([admin], 2, 12, false)); }); + it('cancels an older directory load when a newer search starts', () => { + create(); + const older = http.expectOne(`${api}?page=1&limit=20`); + const search = (fixture.nativeElement as HTMLElement).querySelector('input[type="search"]')!; + search.value = 'Grace'; + search.dispatchEvent(new Event('input')); + search.closest('form')!.dispatchEvent(new Event('submit')); + const newer = http.expectOne(`${api}?page=1&limit=20&search=Grace`); + + const olderWasCancelled = older.cancelled; + newer.flush(directoryPage([admin])); + if (!olderWasCancelled) older.flush(directoryPage([ada])); + fixture.detectChanges(); + + expect(olderWasCancelled).toBe(true); + expect(text()).toContain('Grace Admin'); + expect(text()).not.toContain('Ada Lovelace'); + }); + it('redacts admin-only data and controls for a non-admin even if extra fields arrive', () => { create(); flushDirectory(directoryPage([ada])); @@ -208,6 +234,31 @@ describe('Users directory', () => { expect(text()).toContain('Augusta King'); }); + it('keeps the editor draft while saving input changes and on an initial profile failure', () => { + isAdmin.set(true); + create(); + flushDirectory(); + button('Bearbeiten').click(); + fixture.detectChanges(); + const firstName = (fixture.nativeElement as HTMLElement).querySelector('input[name="firstName"]')!; + firstName.value = 'Operator draft'; + firstName.dispatchEvent(new Event('input')); + firstName.closest('form')!.dispatchEvent(new Event('submit')); + fixture.detectChanges(); + + expect(firstName.value).toBe('Operator draft'); + http.expectOne(`${adminApi}/7/profile`).flush( + { message: 'Profile rejected' }, + { status: 400, statusText: 'Bad Request' }, + ); + http.expectNone(`${api}?page=1&limit=20`); + fixture.detectChanges(); + expect((fixture.nativeElement as HTMLElement).querySelector('input[name="firstName"]')!.value).toBe( + 'Operator draft', + ); + expect(text()).toContain('Profile rejected'); + }); + it('reloads authoritative directory state when a role change fails after the profile was saved', () => { isAdmin.set(true); create(); @@ -241,11 +292,69 @@ describe('Users directory', () => { closeDialog.next(true); const request = http.expectOne(`${adminApi}/7/status`); expect(request.request.body).toEqual({ status: 2 }); + fixture.detectChanges(); + expect(text()).toContain('Status wird geändert'); + expect((fixture.nativeElement as HTMLElement).querySelector('[data-user-id="7"]')?.getAttribute('aria-busy')).toBe('true'); request.flush(ada); http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage([{ ...ada, status: { id: 2, name: 'Inactive' } }])); }); - it('loads searchable player results and assigns an unlinked player before refreshing both lists', () => { + it('searches players from page one after paging and assigns before refreshing both lists', () => { + isAdmin.set(true); + create(); + flushDirectory(); + button('Zuordnungen verwalten').click(); + fixture.detectChanges(); + const panel = (fixture.nativeElement as HTMLElement).querySelector('app-player-assignments')!; + expect(panel.textContent).toContain('Spieler werden geladen'); + http.expectOne(`${adminApi}/players?assignment=all&page=1&limit=20`).flush(playersPage({ hasNextPage: true, total: 21 })); + fixture.detectChanges(); + expect(panel.querySelector('nav[aria-label="Spielerseiten"]')).not.toBeNull(); + childButton(panel, 'Weiter').click(); + http.expectOne(`${adminApi}/players?assignment=all&page=2&limit=20`).flush(playersPage({ page: 2 })); + fixture.detectChanges(); + const playerSearch = panel.querySelector('input[type="search"]')!; + playerSearch.value = 'Linus'; + playerSearch.dispatchEvent(new Event('input')); + playerSearch.closest('form')!.dispatchEvent(new Event('submit')); + http.expectOne(`${adminApi}/players?search=Linus&assignment=all&page=1&limit=20`).flush(playersPage()); + fixture.detectChanges(); + expect(text()).toContain('Linus Player'); + expect(text()).toContain('Second Team'); + + childButton(panel, 'Zuordnen').click(); + const assign = http.expectOne(`${adminApi}/7/players/202`); + expect(assign.request.method).toBe('PUT'); + assign.flush(ada); + http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage()); + http.expectOne(`${adminApi}/players?search=Linus&assignment=all&page=1&limit=20`).flush(playersPage()); + }); + + it('cancels an older player load when a newer player search starts', () => { + isAdmin.set(true); + create(); + flushDirectory(); + button('Zuordnungen verwalten').click(); + fixture.detectChanges(); + const older = http.expectOne(`${adminApi}/players?assignment=all&page=1&limit=20`); + const panel = (fixture.nativeElement as HTMLElement).querySelector('app-player-assignments')!; + const playerSearch = panel.querySelector('input[type="search"]')!; + playerSearch.value = 'New'; + playerSearch.dispatchEvent(new Event('input')); + playerSearch.closest('form')!.dispatchEvent(new Event('submit')); + const newer = http.expectOne(`${adminApi}/players?search=New&assignment=all&page=1&limit=20`); + + const olderWasCancelled = older.cancelled; + newer.flush(playersPage({ data: [{ ...playersPage().data[0], firstName: 'New' }] })); + if (!olderWasCancelled) older.flush(playersPage()); + fixture.detectChanges(); + + expect(olderWasCancelled).toBe(true); + expect(panel.textContent).toContain('New Player'); + expect(panel.textContent).not.toContain('Linus Player'); + }); + + it('clears stale player rows on load errors and retries with load-specific copy', () => { isAdmin.set(true); create(); flushDirectory(); @@ -253,15 +362,23 @@ describe('Users directory', () => { fixture.detectChanges(); http.expectOne(`${adminApi}/players?assignment=all&page=1&limit=20`).flush(playersPage()); fixture.detectChanges(); - expect(text()).toContain('Linus Player'); - expect(text()).toContain('Second Team'); + const panel = (fixture.nativeElement as HTMLElement).querySelector('app-player-assignments')!; + expect(panel.textContent).toContain('Linus Player'); + const playerSearch = panel.querySelector('input[type="search"]')!; + playerSearch.value = 'broken'; + playerSearch.dispatchEvent(new Event('input')); + playerSearch.closest('form')!.dispatchEvent(new Event('submit')); + http.expectOne(`${adminApi}/players?search=broken&assignment=all&page=1&limit=20`).flush( + { message: 'Search unavailable' }, + { status: 500, statusText: 'Server Error' }, + ); + fixture.detectChanges(); - button('Zuordnen').click(); - const assign = http.expectOne(`${adminApi}/7/players/202`); - expect(assign.request.method).toBe('PUT'); - assign.flush(ada); - http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage()); - http.expectOne(`${adminApi}/players?assignment=all&page=1&limit=20`).flush(playersPage()); + expect(panel.textContent).not.toContain('Linus Player'); + expect(panel.textContent).toContain('Spieler konnten nicht geladen werden'); + expect(panel.textContent).toContain('Search unavailable'); + childButton(panel, 'Erneut versuchen').click(); + http.expectOne(`${adminApi}/players?search=broken&assignment=all&page=1&limit=20`).flush(playersPage()); }); it('confirms unlinking and reassignment with the affected user names', () => { @@ -341,11 +458,37 @@ describe('Users directory', () => { const unlink = http.expectOne(`${adminApi}/7/players/101`); expect(unlink.request.method).toBe('DELETE'); expect(text()).toContain('Wird gelöst'); + const panel = (fixture.nativeElement as HTMLElement).querySelector('app-player-assignments')!; + expect(panel.querySelector('section')?.getAttribute('aria-busy')).toBe('true'); + expect(childButton(panel, 'Schließen').disabled).toBe(true); + expect(childButton(panel, 'Suchen').disabled).toBe(true); + const mainToggle = [...(fixture.nativeElement as HTMLElement).querySelectorAll('[data-user-id="7"] > .user-row__actions button')].find( + (item) => item.textContent?.includes('Zuordnungen verwalten'), + )!; + expect(mainToggle.disabled).toBe(true); + childButton(panel, 'Schließen').click(); + fixture.detectChanges(); + expect((fixture.nativeElement as HTMLElement).querySelector('app-player-assignments')).not.toBeNull(); unlink.flush(ada); http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage()); http.expectOne(`${adminApi}/players?assignment=all&page=1&limit=20`).flush(playersPage()); }); + it('shows a persistent explanation for disabled self-deactivation', () => { + isAdmin.set(true); + currentUser.set({ id: 1, firstName: 'Grace', lastName: 'Admin', role: { id: 1 } }); + create(); + http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage([admin])); + fixture.detectChanges(); + const row = (fixture.nativeElement as HTMLElement).querySelector('[data-user-id="1"]')!; + const statusButton = [...row.querySelectorAll('button')].find((item) => item.textContent?.includes('Deaktivieren'))!; + const descriptionId = statusButton.getAttribute('aria-describedby'); + expect(descriptionId).toBeTruthy(); + expect((fixture.nativeElement as HTMLElement).querySelector(`#${descriptionId}`)?.textContent).toContain( + 'eigene Konto kann nicht deaktiviert werden', + ); + }); + it('renders loading, empty, general error, and retry states', () => { create(); expect((fixture.nativeElement as HTMLElement).querySelector('[role="progressbar"]')).not.toBeNull(); diff --git a/myteamwallet_frontend_modern/src/app/features/users/users.ts b/myteamwallet_frontend_modern/src/app/features/users/users.ts index bc9f6e5..47e8278 100644 --- a/myteamwallet_frontend_modern/src/app/features/users/users.ts +++ b/myteamwallet_frontend_modern/src/app/features/users/users.ts @@ -1,5 +1,6 @@ import { HttpErrorResponse } from '@angular/common/http'; -import { Component, inject, signal } from '@angular/core'; +import { Component, DestroyRef, inject, signal } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { RouterLink } from '@angular/router'; import { MatButtonModule } from '@angular/material/button'; import { MatDialog } from '@angular/material/dialog'; @@ -7,7 +8,7 @@ import { MatFormFieldModule } from '@angular/material/form-field'; import { MatIconModule } from '@angular/material/icon'; import { MatInputModule } from '@angular/material/input'; import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; -import { filter, finalize, of, switchMap, take } from 'rxjs'; +import { EMPTY, Subject, catchError, filter, finalize, of, startWith, switchMap, take, tap } from 'rxjs'; import { AuthStore } from '../../core/auth/auth-store'; import { AdminUsersApi } from '../../core/users/admin-users-api'; import { UsersApi } from '../../core/users/users-api'; @@ -41,6 +42,8 @@ export class Users { private readonly adminUsersApi = inject(AdminUsersApi); private readonly authStore = inject(AuthStore); private readonly dialog = inject(MatDialog); + private readonly destroyRef = inject(DestroyRef); + private readonly directoryRequests = new Subject(); protected readonly isAdmin = this.authStore.isGlobalAdmin; protected readonly currentUser = this.authStore.currentUser; @@ -54,30 +57,42 @@ export class Users { protected readonly limit = 20; protected readonly editingUserId = signal(null); protected readonly assignmentUserId = signal(null); + protected readonly assignmentBusyUserId = signal(null); protected readonly pendingUserId = signal(null); constructor() { - this.loadDirectory(); + this.directoryRequests + .pipe( + startWith(true), + switchMap((showLoading) => { + if (showLoading) this.loading.set(true); + this.loadError.set(null); + const search = this.search(); + return this.usersApi + .loadDirectory({ page: this.page(), limit: this.limit, ...(search ? { search } : {}) }) + .pipe( + tap((directory) => this.directory.set(directory)), + catchError((error: HttpErrorResponse) => { + this.directory.set(null); + this.loadError.set(this.errorMessage(error, 'Benutzer konnten nicht geladen werden.')); + return EMPTY; + }), + finalize(() => this.loading.set(false)), + ); + }), + takeUntilDestroyed(this.destroyRef), + ) + .subscribe(); } - protected loadDirectory(): void { - this.loading.set(true); - this.loadError.set(null); - const search = this.search(); - this.usersApi - .loadDirectory({ page: this.page(), limit: this.limit, ...(search ? { search } : {}) }) - .pipe(finalize(() => this.loading.set(false))) - .subscribe({ - next: (directory) => this.directory.set(directory), - error: (error: HttpErrorResponse) => { - this.directory.set(null); - this.loadError.set(this.errorMessage(error, 'Benutzer konnten nicht geladen werden.')); - }, - }); + protected loadDirectory(showLoading = true): void { + this.directoryRequests.next(showLoading); } protected submitSearch(): void { - this.search.set(this.searchDraft().trim()); + const search = this.searchDraft().trim(); + if (this.loading() && search === this.search() && this.page() === 1) return; + this.search.set(search); this.page.set(1); this.loadDirectory(); } @@ -129,12 +144,14 @@ export class Users { } protected toggleEdit(userId: number): void { + if (this.assignmentBusyUserId() !== null) return; this.assignmentUserId.set(null); this.editingUserId.update((value) => (value === userId ? null : userId)); this.mutationError.set(null); } protected toggleAssignments(userId: number): void { + if (this.assignmentBusyUserId() !== null) return; this.editingUserId.set(null); this.assignmentUserId.update((value) => (value === userId ? null : userId)); this.mutationError.set(null); @@ -149,12 +166,15 @@ export class Users { lastName: value.lastName, }); const roleId = user.role?.id; + let profileSaved = false; profileRequest .pipe( + tap(() => (profileSaved = true)), switchMap(() => roleId === value.role ? of(user) : this.adminUsersApi.updateRole(user.id, { role: value.role }), ), finalize(() => this.pendingUserId.set(null)), + takeUntilDestroyed(this.destroyRef), ) .subscribe({ next: () => { @@ -163,7 +183,7 @@ export class Users { }, error: (error: HttpErrorResponse) => { this.mutationError.set(this.errorMessage(error, 'Änderung fehlgeschlagen.')); - this.loadDirectory(); + if (profileSaved) this.loadDirectory(); }, }); } @@ -182,7 +202,7 @@ export class Users { restoreFocus: true, }) .afterClosed() - .pipe(filter(Boolean), take(1)) + .pipe(filter(Boolean), take(1), takeUntilDestroyed(this.destroyRef)) .subscribe(() => this.updateStatus(user.id, status)); } @@ -191,7 +211,11 @@ export class Users { } protected assignmentsChanged(): void { - this.loadDirectory(); + this.loadDirectory(false); + } + + protected assignmentBusyChanged(userId: number, busy: boolean): void { + this.assignmentBusyUserId.set(busy ? userId : null); } private updateStatus(userId: number, status: AdminUserStatusId): void { @@ -199,7 +223,7 @@ export class Users { this.mutationError.set(null); this.adminUsersApi .updateStatus(userId, { status }) - .pipe(finalize(() => this.pendingUserId.set(null))) + .pipe(finalize(() => this.pendingUserId.set(null)), takeUntilDestroyed(this.destroyRef)) .subscribe({ next: () => this.loadDirectory(), error: (error: HttpErrorResponse) => From 9eec6268d058a6878a431bce8bedeceb99291686 Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Sat, 1 Aug 2026 10:44:17 +0200 Subject: [PATCH 12/15] fix: lock directory during assignment changes --- .../src/app/features/users/users.html | 17 ++++- .../src/app/features/users/users.spec.ts | 74 +++++++++++++++++++ .../src/app/features/users/users.ts | 5 +- 3 files changed, 91 insertions(+), 5 deletions(-) diff --git a/myteamwallet_frontend_modern/src/app/features/users/users.html b/myteamwallet_frontend_modern/src/app/features/users/users.html index c04da1c..ef1e9a2 100644 --- a/myteamwallet_frontend_modern/src/app/features/users/users.html +++ b/myteamwallet_frontend_modern/src/app/features/users/users.html @@ -15,10 +15,11 @@ type="search" name="directorySearch" [value]="searchDraft()" + [disabled]="assignmentBusyUserId() !== null" (input)="searchDraft.set($any($event.target).value)" /> - + @if (mutationError()) { @@ -138,11 +139,21 @@
diff --git a/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts b/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts index fa7e367..867217d 100644 --- a/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts +++ b/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts @@ -474,6 +474,80 @@ describe('Users directory', () => { http.expectOne(`${adminApi}/players?assignment=all&page=1&limit=20`).flush(playersPage()); }); + it('blocks directory search and paging while an assignment mutation is pending, then restores them', () => { + isAdmin.set(true); + create(); + http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage([ada], 1, 60, true)); + fixture.detectChanges(); + button('Weiter').click(); + http.expectOne(`${api}?page=2&limit=20`).flush(directoryPage([ada], 2, 60, true)); + fixture.detectChanges(); + button('Zuordnungen verwalten').click(); + fixture.detectChanges(); + http.expectOne(`${adminApi}/players?assignment=all&page=1&limit=20`).flush( + playersPage({ + data: [ + { + id: 101, + firstName: 'Ada', + lastName: 'Lovelace', + active: true, + team: { id: 5, name: 'First Team', alias: 'first' }, + currentUser: { id: 7, firstName: 'Ada', lastName: 'Lovelace', status: { id: 1, name: 'Active' } }, + }, + ], + }), + ); + fixture.detectChanges(); + childButton( + (fixture.nativeElement as HTMLElement).querySelector('app-player-assignments')!, + 'Verknüpfung lösen', + ).click(); + closeDialog.next(true); + fixture.detectChanges(); + const unlink = http.expectOne(`${adminApi}/7/players/101`); + + const directoryForm = (fixture.nativeElement as HTMLElement).querySelector('.directory-search')!; + const directorySearch = directoryForm.querySelector('input[type="search"]')!; + const directorySearchButton = directoryForm.querySelector('button[type="submit"]')!; + const paging = (fixture.nativeElement as HTMLElement).querySelector('nav.pagination')!; + const [previous, next] = [...paging.querySelectorAll('button')]; + const controlsWereBlocked = directorySearch.disabled && directorySearchButton.disabled && previous.disabled && next.disabled; + + directorySearch.value = 'Other'; + directorySearch.dispatchEvent(new Event('input')); + directoryForm.dispatchEvent(new Event('submit')); + previous.click(); + next.click(); + fixture.detectChanges(); + const unexpectedQueries = http.match((request) => request.url.startsWith(api)); + const mutationStayedActive = !unlink.cancelled; + + if (controlsWereBlocked && unexpectedQueries.length === 0 && mutationStayedActive) { + unlink.flush(ada); + http.expectOne(`${api}?page=2&limit=20`).flush(directoryPage([ada], 2, 60, true)); + http.expectOne(`${adminApi}/players?assignment=all&page=1&limit=20`).flush(playersPage()); + fixture.detectChanges(); + expect(directorySearch.disabled).toBe(false); + expect(directorySearchButton.disabled).toBe(false); + expect(previous.disabled).toBe(false); + expect(next.disabled).toBe(false); + directoryForm.dispatchEvent(new Event('submit')); + http.expectOne(`${api}?page=1&limit=20&search=Other`).flush(directoryPage([])); + } else { + for (const query of unexpectedQueries) query.flush(directoryPage([ada], 1, 60, true)); + fixture.detectChanges(); + if (!unlink.cancelled) unlink.flush(ada); + for (const playerLoad of http.match((request) => request.url.startsWith(`${adminApi}/players`))) { + playerLoad.flush(playersPage()); + } + } + + expect(controlsWereBlocked).toBe(true); + expect(unexpectedQueries).toHaveLength(0); + expect(mutationStayedActive).toBe(true); + }); + it('shows a persistent explanation for disabled self-deactivation', () => { isAdmin.set(true); currentUser.set({ id: 1, firstName: 'Grace', lastName: 'Admin', role: { id: 1 } }); diff --git a/myteamwallet_frontend_modern/src/app/features/users/users.ts b/myteamwallet_frontend_modern/src/app/features/users/users.ts index 47e8278..ab30355 100644 --- a/myteamwallet_frontend_modern/src/app/features/users/users.ts +++ b/myteamwallet_frontend_modern/src/app/features/users/users.ts @@ -90,6 +90,7 @@ export class Users { } protected submitSearch(): void { + if (this.assignmentBusyUserId() !== null) return; const search = this.searchDraft().trim(); if (this.loading() && search === this.search() && this.page() === 1) return; this.search.set(search); @@ -98,13 +99,13 @@ export class Users { } protected previousPage(): void { - if (this.page() <= 1 || this.loading()) return; + if (this.page() <= 1 || this.loading() || this.assignmentBusyUserId() !== null) return; this.page.update((value) => value - 1); this.loadDirectory(); } protected nextPage(): void { - if (!this.directory()?.hasNextPage || this.loading()) return; + if (!this.directory()?.hasNextPage || this.loading() || this.assignmentBusyUserId() !== null) return; this.page.update((value) => value + 1); this.loadDirectory(); } From d962200b79a3ed8dfd6799f8104fe115d12cb59d Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Sat, 1 Aug 2026 10:52:17 +0200 Subject: [PATCH 13/15] test: cover assignment failure recovery --- .../src/app/features/users/users.spec.ts | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts b/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts index 867217d..8b955c4 100644 --- a/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts +++ b/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts @@ -548,6 +548,71 @@ describe('Users directory', () => { expect(mutationStayedActive).toBe(true); }); + it('keeps directory controls blocked until a failed assignment mutation clears', () => { + isAdmin.set(true); + create(); + http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage([ada], 1, 60, true)); + fixture.detectChanges(); + button('Weiter').click(); + http.expectOne(`${api}?page=2&limit=20`).flush(directoryPage([ada], 2, 60, true)); + fixture.detectChanges(); + button('Zuordnungen verwalten').click(); + fixture.detectChanges(); + http.expectOne(`${adminApi}/players?assignment=all&page=1&limit=20`).flush( + playersPage({ + data: [ + { + id: 101, + firstName: 'Ada', + lastName: 'Lovelace', + active: true, + team: { id: 5, name: 'First Team', alias: 'first' }, + currentUser: { id: 7, firstName: 'Ada', lastName: 'Lovelace', status: { id: 1, name: 'Active' } }, + }, + ], + }), + ); + fixture.detectChanges(); + + const panel = (fixture.nativeElement as HTMLElement).querySelector('app-player-assignments')!; + panel.querySelector('.player-row button')!.click(); + closeDialog.next(true); + fixture.detectChanges(); + const unlink = http.expectOne(`${adminApi}/7/players/101`); + + const directoryForm = (fixture.nativeElement as HTMLElement).querySelector('.directory-search')!; + const directorySearch = directoryForm.querySelector('input[type="search"]')!; + const directorySearchButton = directoryForm.querySelector('button[type="submit"]')!; + const [previous, next] = [ + ...(fixture.nativeElement as HTMLElement).querySelectorAll('nav.pagination button'), + ]; + expect(directorySearch.disabled).toBe(true); + expect(directorySearchButton.disabled).toBe(true); + expect(previous.disabled).toBe(true); + expect(next.disabled).toBe(true); + + directorySearch.value = 'Other'; + directorySearch.dispatchEvent(new Event('input')); + directoryForm.dispatchEvent(new Event('submit')); + previous.click(); + next.click(); + fixture.detectChanges(); + expect(http.match((request) => request.url.startsWith(api))).toHaveLength(0); + expect(unlink.cancelled).toBe(false); + + unlink.flush({ message: 'Assignment failed' }, { status: 500, statusText: 'Server Error' }); + fixture.detectChanges(); + expect(text()).toContain('Zuordnung konnte nicht geändert werden. Assignment failed'); + expect(directorySearch.disabled).toBe(false); + expect(directorySearchButton.disabled).toBe(false); + expect(previous.disabled).toBe(false); + expect(next.disabled).toBe(false); + expect(http.match((request) => request.url.startsWith(api))).toHaveLength(0); + + directoryForm.dispatchEvent(new Event('submit')); + http.expectOne(`${api}?page=1&limit=20&search=Other`).flush(directoryPage([])); + }); + it('shows a persistent explanation for disabled self-deactivation', () => { isAdmin.set(true); currentUser.set({ id: 1, firstName: 'Grace', lastName: 'Admin', role: { id: 1 } }); From 431eba86e7448c4bf00e3c114aaa8985ff5c1ff7 Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Sat, 1 Aug 2026 11:20:47 +0200 Subject: [PATCH 14/15] fix: align admin profile request validation --- .../src/users/admin-users.controller.spec.ts | 16 +++++++++++++++ .../src/users/dto/admin-user.dto.ts | 9 +++++---- .../src/app/core/users/users-api.spec.ts | 20 +++++++++++++++++-- .../src/app/models/user-directory.model.ts | 4 ++-- 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/myteamwallet_backend/src/users/admin-users.controller.spec.ts b/myteamwallet_backend/src/users/admin-users.controller.spec.ts index 7cfd986..d335167 100644 --- a/myteamwallet_backend/src/users/admin-users.controller.spec.ts +++ b/myteamwallet_backend/src/users/admin-users.controller.spec.ts @@ -69,6 +69,22 @@ describe('AdminUsersController', () => { }); describe('admin user DTOs', () => { + it('accepts omitted and string profile names but rejects explicit nulls', async () => { + const omitted = plainToInstance(AdminUserProfileDto, {}); + const names = plainToInstance(AdminUserProfileDto, { + firstName: 'Ada', + lastName: 'Admin', + }); + const nulls = plainToInstance(AdminUserProfileDto, { + firstName: null, + lastName: null, + }); + + expect(await validate(omitted)).toEqual([]); + expect(await validate(names)).toEqual([]); + expect(await validate(nulls)).not.toEqual([]); + }); + it('accepts only profile names in the profile DTO', async () => { const dto = plainToInstance(AdminUserProfileDto, { firstName: 'Ada', diff --git a/myteamwallet_backend/src/users/dto/admin-user.dto.ts b/myteamwallet_backend/src/users/dto/admin-user.dto.ts index 07b654d..ed67c16 100644 --- a/myteamwallet_backend/src/users/dto/admin-user.dto.ts +++ b/myteamwallet_backend/src/users/dto/admin-user.dto.ts @@ -7,20 +7,21 @@ import { Max, MaxLength, Min, + ValidateIf, } from 'class-validator'; import { RoleEnum } from '../../roles/roles.enum'; import { StatusEnum } from '../../statuses/statuses.enum'; export class AdminUserProfileDto { - @IsOptional() + @ValidateIf((_, value) => value !== undefined) @IsString() @MaxLength(255) - firstName?: string | null; + firstName?: string; - @IsOptional() + @ValidateIf((_, value) => value !== undefined) @IsString() @MaxLength(255) - lastName?: string | null; + lastName?: string; } export class AdminUserRoleDto { diff --git a/myteamwallet_frontend_modern/src/app/core/users/users-api.spec.ts b/myteamwallet_frontend_modern/src/app/core/users/users-api.spec.ts index e838ae5..42e7db3 100644 --- a/myteamwallet_frontend_modern/src/app/core/users/users-api.spec.ts +++ b/myteamwallet_frontend_modern/src/app/core/users/users-api.spec.ts @@ -3,6 +3,7 @@ import { TestBed } from '@angular/core/testing'; import { HttpTestingController, provideHttpClientTesting } from '@angular/common/http/testing'; import { environment } from '../../../environments/environment'; import { AdminUsersApi } from './admin-users-api'; +import { AdminUserProfileRequest } from '../../models/user-directory.model'; import { UsersApi } from './users-api'; describe('UsersApi', () => { @@ -32,6 +33,21 @@ describe('AdminUsersApi', () => { let service: AdminUsersApi; let httpMock: HttpTestingController; + it('accepts profile requests with omitted or string names only', () => { + const omitted: AdminUserProfileRequest = {}; + const names: AdminUserProfileRequest = { + firstName: 'Ada', + lastName: 'Admin', + }; + + // @ts-expect-error Profile names cannot be explicitly cleared to null. + const nullName: AdminUserProfileRequest = { firstName: null }; + + expect(omitted).toEqual({}); + expect(names).toEqual({ firstName: 'Ada', lastName: 'Admin' }); + expect(nullName).toEqual({ firstName: null }); + }); + beforeEach(() => { TestBed.configureTestingModule({ providers: [provideHttpClient(), provideHttpClientTesting()] }); service = TestBed.inject(AdminUsersApi); @@ -41,10 +57,10 @@ describe('AdminUsersApi', () => { afterEach(() => httpMock.verify()); it('uses the dedicated admin profile endpoint and payload', () => { - service.updateProfile(7, { firstName: 'Alex', lastName: null }).subscribe(); + service.updateProfile(7, { firstName: 'Alex' }).subscribe(); const request = httpMock.expectOne(`${environment.apiUrl}admin/users/7/profile`); expect(request.request.method).toBe('PATCH'); - expect(request.request.body).toEqual({ firstName: 'Alex', lastName: null }); + expect(request.request.body).toEqual({ firstName: 'Alex' }); request.flush({}); }); diff --git a/myteamwallet_frontend_modern/src/app/models/user-directory.model.ts b/myteamwallet_frontend_modern/src/app/models/user-directory.model.ts index 0a91db4..f7ac630 100644 --- a/myteamwallet_frontend_modern/src/app/models/user-directory.model.ts +++ b/myteamwallet_frontend_modern/src/app/models/user-directory.model.ts @@ -48,8 +48,8 @@ export interface UserDirectoryFilters { } export interface AdminUserProfileRequest { - firstName?: string | null; - lastName?: string | null; + firstName?: string; + lastName?: string; } export type AdminUserRoleId = 1 | 2; From 945369796e8deedf3eb5197e72632b2bdd583196 Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Sat, 1 Aug 2026 11:45:03 +0200 Subject: [PATCH 15/15] fix: refresh admin self profile state --- .../src/app/features/users/users.spec.ts | 40 ++++++++++++++++++- .../src/app/features/users/users.ts | 20 ++++++++-- 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts b/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts index 8b955c4..d88228e 100644 --- a/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts +++ b/myteamwallet_frontend_modern/src/app/features/users/users.spec.ts @@ -71,12 +71,14 @@ describe('Users directory', () => { let http: HttpTestingController; let isAdmin: ReturnType>; let currentUser: ReturnType>; + let updateUser: ReturnType; let closeDialog: Subject; let dialog: { open: ReturnType }; beforeEach(async () => { isAdmin = signal(false); currentUser = signal({ id: 99, firstName: 'Nora', lastName: 'Viewer', role: { id: 2 } }); + updateUser = vi.fn(); closeDialog = new Subject(); dialog = { open: vi.fn(() => ({ afterClosed: () => closeDialog.asObservable() })) }; @@ -86,7 +88,7 @@ describe('Users directory', () => { provideHttpClient(), provideHttpClientTesting(), provideRouter([]), - { provide: AuthStore, useValue: { isGlobalAdmin: isAdmin, currentUser } }, + { provide: AuthStore, useValue: { isGlobalAdmin: isAdmin, currentUser, updateUser } }, { provide: MatDialog, useValue: dialog }, ], }).compileComponents(); @@ -113,6 +115,42 @@ describe('Users directory', () => { return match as HTMLButtonElement; } + it('renders unknown role and status values explicitly', () => { + create(); + flushDirectory(); + + expect(fixture.componentInstance['statusName']()).toBe('Unbekannt'); + expect(fixture.componentInstance['statusName'](99)).toBe('Unbekannt'); + expect(fixture.componentInstance['roleName']()).toBe('Unbekannt'); + expect(fixture.componentInstance['roleName'](99)).toBe('Unbekannt'); + }); + + it('persists returned names when an admin edits their own profile', () => { + isAdmin.set(true); + currentUser.set({ id: 7, firstName: 'Ada', lastName: 'Lovelace', role: { id: 1 } }); + create(); + flushDirectory(directoryPage([admin, ada])); + + fixture.componentInstance['saveEdit'](ada, { + firstName: 'Augusta', + lastName: 'King', + role: 2, + }); + http.expectOne(`${adminApi}/7/profile`).flush({ + ...ada, + firstName: 'Augusta', + lastName: 'King', + }); + http.expectOne(`${api}?page=1&limit=20`).flush(directoryPage()); + + expect(updateUser).toHaveBeenCalledWith({ + id: 7, + firstName: 'Augusta', + lastName: 'King', + role: { id: 1 }, + }); + }); + function childButton(host: HTMLElement, label: string): HTMLButtonElement { const match = [...host.querySelectorAll('button')].find((element) => element.textContent?.includes(label)); diff --git a/myteamwallet_frontend_modern/src/app/features/users/users.ts b/myteamwallet_frontend_modern/src/app/features/users/users.ts index ab30355..888ecfa 100644 --- a/myteamwallet_frontend_modern/src/app/features/users/users.ts +++ b/myteamwallet_frontend_modern/src/app/features/users/users.ts @@ -125,11 +125,15 @@ export class Users { } protected statusName(statusId?: number): string { - return statusId === 2 ? 'Inaktiv' : 'Aktiv'; + if (statusId === 1) return 'Aktiv'; + if (statusId === 2) return 'Inaktiv'; + return 'Unbekannt'; } protected roleName(roleId?: number): string { - return roleId === 1 ? 'Administrator' : 'Benutzer'; + if (roleId === 1) return 'Administrator'; + if (roleId === 2) return 'Benutzer'; + return 'Unbekannt'; } protected teamRoleName(name?: string): string { @@ -170,7 +174,17 @@ export class Users { let profileSaved = false; profileRequest .pipe( - tap(() => (profileSaved = true)), + tap((updatedUser) => { + profileSaved = true; + const currentUser = this.currentUser(); + if (currentUser?.id === updatedUser.id) { + this.authStore.updateUser({ + ...currentUser, + firstName: updatedUser.firstName, + lastName: updatedUser.lastName, + }); + } + }), switchMap(() => roleId === value.role ? of(user) : this.adminUsersApi.updateRole(user.id, { role: value.role }), ),