|
| 1 | +## Topcoder Identity Service: updating Authorization, Group, User Resources with TypeScript and Postgres |
| 2 | + |
| 3 | +This zip contains submission for challenge `Topcoder Identity Service: updating User Resources with TypeScript and Postgres` |
| 4 | + |
| 5 | +## Verification |
| 6 | + |
| 7 | +- Please follow the `Readme.md` to setup databases and start the application. |
| 8 | +- Please see there is a new postman collection as `users.postman_collection.json`. Refer to relevant section in `Readme.md` |
| 9 | + |
| 10 | +Please use correct variables after copying `.env.example` |
| 11 | + |
| 12 | +``` |
| 13 | +AUTH_SECRET="test" |
| 14 | +AUTH0_CLIENT_SECRET="..." # Client Secret of M2M App |
| 15 | +DICEAUTH_DICE_API_URL="https://console-api-uat.diceid.com/v1" |
| 16 | +DICEAUTH_DICE_API_KEY="..." |
| 17 | +DICEAUTH_ORG_ID="4f541723-f581-44de-b61c-5f83e8b8ef1e" |
| 18 | +DICEAUTH_USER_ID="a5e7e72a-fa5e-4acf-9eca-741d1443279b" |
| 19 | +DICEAUTH_TC_API_KEY="..." |
| 20 | +DICEAUTH_SCHEMA_NAME="Topcoder" |
| 21 | +DICEAUTH_SCHEMA_VERSION="1.4" |
| 22 | +DICEAUTH_OTP_DURATION="10" |
| 23 | +SLACK_BOT_KEY="..." |
| 24 | +SLACK_CHANNEL_ID="C04ENKCU4TZ" |
| 25 | +JWT_SECRET="just-a-random-string" |
| 26 | +``` |
| 27 | + |
| 28 | +Change this to any random string for local, and use actual key in dev/prod environments |
| 29 | + |
| 30 | +``` |
| 31 | +# Legacy Blowfish Encryption Key (Base64 Encoded - !!! REPLACE WITH ACTUAL KEY FROM OLD SYSTEM !!!) |
| 32 | +# Used for compatibility with the old password encoding scheme. |
| 33 | +LEGACY_BLOWFISH_KEY=!!!_REPLACE_WITH_BASE64_ENCODED_KEY_!!! |
| 34 | +``` |
| 35 | +Here is an example |
| 36 | + |
| 37 | +``` |
| 38 | +LEGACY_BLOWFISH_KEY=dGhpc2lzRGVmYXVmZlZhbHVl |
| 39 | +``` |
| 40 | +`.env.example has only sample values`. |
| 41 | + |
| 42 | +## Notes |
| 43 | + |
| 44 | +The document `resource_migration.md` explains the migration approach for each endpoint. |
| 45 | + |
| 46 | +**Important**: User endpoints cannot be fully automated as it requires tokens and a speific order for testing reactivation flows etc.. via postman. Please use the video `doc/users-endpoint.mp4` for a step-by-step guide on how to test all APIs. Steps needs to be followed to be able to |
| 47 | +test all endpoints. (you can play at 1.5x, the postman UI is a bit slow) |
| 48 | + |
| 49 | +**Event Notifications**: There are some open points about notifications and email templates at the moment this submission sent which can be seen in the forum. Please be aware of them, but all functionality works. |
| 50 | + |
| 51 | +## Addressed Issues |
| 52 | + |
| 53 | +This section details how the issues fixed for given excel list: |
| 54 | + |
| 55 | +1. **Potential Race Condition in `role.service.ts` (update method, Line 146):** |
| 56 | + * **Status: FIXED.** |
| 57 | + * **Details:** The `update` method in `src/api/role/role.service.ts` now utilizes `this.prismaAuth.$transaction`. This ensures that checking for duplicate role names and the actual update operation are performed atomically, preventing race conditions. |
| 58 | + |
| 59 | +2. **Error Handling in Role Assignment (`role.service.ts` Line 220 - `assignRoleToSubject`):** |
| 60 | + * **Status: FIXED.** |
| 61 | + * **Details:** Previously, when a duplicate role assignment was attempted (Prisma error P2002), the error was silently ignored. The `assignRoleToSubject` method in `src/api/role/role.service.ts` has been updated to explicitly catch the `P2002` error and throw a `ConflictException` with the message "Role {roleId} is already assigned to subject {subjectId}." This provides clear feedback to the client. |
| 62 | + |
| 63 | +3. **Missing Input Validation:** |
| 64 | + * **`roleName` length validation in `createRoleDto` and `updateRoleDto`:** |
| 65 | + * **Status: ADDRESSED.** |
| 66 | + * **Details:** The `RoleParamDto` (used within `CreateRoleBodyDto` and `UpdateRoleBodyDto` in `src/dto/role/role.dto.ts`) includes `@MinLength(3)` and `@MaxLength(45)` decorators for the `roleName` field, ensuring its length is validated. |
| 67 | + * **Validation for `subjectId` and `roleId` being positive numbers:** |
| 68 | + * **Status: ADDRESSED.** |
| 69 | + * **Details:** In `src/api/role/role.controller.ts`, the role assignment routes (`assignRoleToSubject`, `deassignRoleFromSubject`, `checkSubjectHasRole`) now include explicit checks to ensure that `roleId` and `subjectId` (when parsed from the query filter) are positive numbers (e.g., `if (roleId <= 0) { throw new BadRequestException(...); }`). |
| 70 | + * **Validation for `subjectType` being a valid value:** |
| 71 | + * **Status: ADDRESSED (Implicitly / Not Applicable for current assignment flow).** |
| 72 | + * **Details:** For the current role assignment/deassignment/checking flows, `subjectType` is hardcoded to `1` (User) within the `src/api/role/role.service.ts` methods. As the client does not provide `subjectType` for these operations, direct input validation in the controller for this specific flow is not applicable. The database schema and DTOs (like `RoleAssignmentResponseDto`) can enforce `subjectType` if it were to be user-provided in other contexts. |
| 73 | + |
| 74 | +4. **Postman tests don't test `deassign`:** |
| 75 | + * **Status: ADDRESSED.** |
| 76 | + * **Details:** The Postman collection `doc/roles api.postman_collection.json` includes a request named `"/roles deassign role (cleanup)"` which specifically tests the `DELETE /roles/{roleId}/deassign?filter=subjectID={subjectId}` endpoint. The test checks for a 200 status code, which is the expected response for a successful deassignment. |
| 77 | + |
| 78 | +5. **Postman tests don't test `hasrole`:** |
| 79 | + * **Status: ADDRESSED.** |
| 80 | + * **Details:** The Postman collection `doc/roles api.postman_collection.json` now includes a request named `"Check Subject Has Role (After Assign)"`. This test is strategically placed after a role assignment and verifies that the `GET /roles/{roleId}/hasrole?filter=subjectID={subjectId}` endpoint correctly returns a 200 status and the expected role ID for an assigned role. |
| 81 | + |
| 82 | +6. **Postman environment variables:** |
| 83 | + * **Status: ADDRESSED.** |
| 84 | + * **Details:** There is now a environment export which has the access token or other global parameters `doc/postman_environment.json`. |
0 commit comments