-
Notifications
You must be signed in to change notification settings - Fork 75
feat: use walletAddress as the primary public ID #140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,224 @@ | ||
| # UUID Migration and walletAddress as Primary Identifier | ||
|
|
||
| ## Overview | ||
|
|
||
| This document outlines the migration from numeric IDs to UUIDs and the transition to using `walletAddress` as the primary identifier for all public API interactions. | ||
|
|
||
| ## Security Benefits | ||
|
|
||
| - **Prevents ID enumeration attacks**: UUIDs are not sequential and cannot be easily guessed | ||
| - **Eliminates scraping vulnerabilities**: Public endpoints no longer expose internal database IDs | ||
| - **Enhanced privacy**: Users are identified by their blockchain wallet address instead of arbitrary numbers | ||
|
|
||
| ## Changes Made | ||
|
|
||
| ### 1. Database Schema Changes | ||
|
|
||
| #### User Table | ||
| - `id` column changed from `SERIAL` to `UUID` with auto-generation | ||
| - `walletAddress` column now has a unique index for performance | ||
| - Foreign key relationships updated to use UUID | ||
|
|
||
| #### Related Tables | ||
| - `user_roles.userId` β `UUID` | ||
| - `buyer_requests.userId` β `UUID` | ||
| - `reviews.userId` β `UUID` | ||
| - `carts.user_id` β Already `UUID` (compatible) | ||
| - `orders.user_id` β Already `UUID` (compatible) | ||
|
|
||
| ### 2. API Endpoint Changes | ||
|
|
||
| #### Before (using numeric ID) | ||
| ``` | ||
| PUT /users/update/:id | ||
| GET /users/:id | ||
| ``` | ||
|
|
||
| #### After (using walletAddress) | ||
| ``` | ||
| PUT /users/update/:walletAddress | ||
| GET /users/:walletAddress | ||
| ``` | ||
|
|
||
| ### 3. Entity Updates | ||
|
|
||
| #### User Entity | ||
| ```typescript | ||
| @Entity('users') | ||
| export class User { | ||
| @PrimaryGeneratedColumn('uuid') | ||
| id: string; // Now UUID | ||
|
|
||
| @Column({ unique: true }) | ||
| @Index() | ||
| walletAddress: string; // Primary identifier for API | ||
| } | ||
| ``` | ||
|
|
||
| #### Related Entities | ||
| - `UserRole.userId`: `string` (UUID) | ||
| - `BuyerRequest.userId`: `string` (UUID) | ||
| - `Review.userId`: `string` (UUID) | ||
|
|
||
| ### 4. Service Layer Changes | ||
|
|
||
| #### UserService | ||
| - `updateUser(walletAddress: string, data)` instead of `updateUser(id: string, data)` | ||
| - `getUserByWalletAddress(walletAddress: string)` for public operations | ||
| - `getUserById(id: string)` retained for internal use only | ||
|
|
||
| #### AuthService | ||
| - JWT tokens now include `walletAddress` as primary identifier | ||
| - `updateUser(walletAddress: string, data)` method updated | ||
| - Role assignment methods updated to use `walletAddress` | ||
|
|
||
| ### 5. Controller Updates | ||
|
|
||
| #### UserController | ||
| - All public endpoints now use `walletAddress` parameter | ||
| - Response objects no longer include `id` field | ||
| - Authorization checks use `walletAddress` for user identification | ||
|
|
||
| #### Authentication Flow | ||
| - JWT strategy updated to handle both `walletAddress` and `id` (backward compatibility) | ||
| - Request objects use `walletAddress` for user identification | ||
|
|
||
| ## Migration Process | ||
|
|
||
| ### 1. Database Migration | ||
| ```bash | ||
| # Run migrations in order | ||
| npm run typeorm migration:run -- -d src/config/database.ts | ||
| ``` | ||
|
|
||
| ### 2. Data Migration | ||
| - Existing numeric IDs are converted to UUIDs | ||
| - Foreign key relationships are updated | ||
| - Data integrity is maintained throughout the process | ||
|
|
||
| ### 3. Application Updates | ||
| - All services updated to use `walletAddress` as primary identifier | ||
| - Controllers updated to handle new parameter structure | ||
| - Tests updated to verify new behavior | ||
|
|
||
| ## API Response Format | ||
|
|
||
| ### Before | ||
| ```json | ||
| { | ||
| "success": true, | ||
| "data": { | ||
| "id": 123, | ||
| "walletAddress": "GABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890123456789012345678901234567890", | ||
| "name": "John Doe", | ||
| "email": "john@example.com" | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### After | ||
| ```json | ||
| { | ||
| "success": true, | ||
| "data": { | ||
| "walletAddress": "GABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890123456789012345678901234567890", | ||
| "name": "John Doe", | ||
| "email": "john@example.com" | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ## Backward Compatibility | ||
|
|
||
| ### JWT Tokens | ||
| - Tokens with `id` field continue to work during migration | ||
| - New tokens use `walletAddress` as primary identifier | ||
| - JWT strategy handles both formats | ||
|
|
||
| ### Internal Operations | ||
| - `id` field retained for database relationships | ||
| - Internal services can still use `getUserById()` method | ||
| - External APIs exclusively use `walletAddress` | ||
|
|
||
| ## Testing | ||
|
|
||
| ### Unit Tests | ||
| - `src/modules/users/tests/user-update-api.spec.ts` - Comprehensive API testing | ||
| - Verifies all CRUD operations work with `walletAddress` | ||
| - Ensures UUID `id` is not exposed in responses | ||
|
|
||
| ### Integration Tests | ||
| - End-to-end testing of user update flows | ||
| - Authentication and authorization verification | ||
| - Database migration validation | ||
|
|
||
| ## Validation | ||
|
|
||
| ### walletAddress Format | ||
| - Stellar wallet addresses: `^G[A-Z2-7]{55}$` | ||
| - Ethereum addresses: `^0x[a-fA-F0-9]{40}$` | ||
| - Format validation in DTOs and services | ||
|
|
||
| ### Error Handling | ||
| - Invalid `walletAddress` format returns 400 Bad Request | ||
| - Duplicate `walletAddress` returns 409 Conflict | ||
| - User not found returns 404 Not Found | ||
|
|
||
| ## Performance Considerations | ||
|
|
||
| ### Indexing | ||
| - `walletAddress` column has unique index | ||
| - Foreign key relationships optimized for UUID lookups | ||
| - Query performance maintained through proper indexing | ||
|
|
||
| ### Caching | ||
| - JWT tokens include `walletAddress` for fast user resolution | ||
| - Database queries optimized for `walletAddress` lookups | ||
|
|
||
| ## Security Considerations | ||
|
|
||
| ### Access Control | ||
| - Users can only access their own profiles using `walletAddress` | ||
| - Admin users can access any profile | ||
| - Role-based access control maintained | ||
|
|
||
| ### Data Exposure | ||
| - Internal UUIDs never exposed to clients | ||
| - All public endpoints use `walletAddress` identifier | ||
| - Sensitive information properly protected | ||
|
|
||
| ## Rollback Plan | ||
|
|
||
| ### Database Rollback | ||
| ```bash | ||
| # Revert migrations if needed | ||
| npm run typeorm migration:revert -- -d src/config/database.ts | ||
| ``` | ||
|
|
||
| ### Application Rollback | ||
| - Revert entity changes | ||
| - Restore original controller methods | ||
| - Update service layer to use numeric IDs | ||
|
|
||
| ## Future Enhancements | ||
|
|
||
| ### Multi-Chain Support | ||
| - Support for different blockchain wallet formats | ||
| - Wallet address validation per blockchain type | ||
| - Cross-chain user identification | ||
|
|
||
| ### Enhanced Security | ||
| - Wallet signature verification for critical operations | ||
| - Multi-factor authentication integration | ||
| - Rate limiting per wallet address | ||
|
|
||
| ## Conclusion | ||
|
|
||
| This migration significantly enhances the security posture of the StarShop backend by: | ||
|
|
||
| 1. **Eliminating ID enumeration vulnerabilities** | ||
| 2. **Using blockchain-native identifiers** | ||
| 3. **Maintaining backward compatibility** | ||
| 4. **Improving API security** | ||
|
|
||
| The transition to `walletAddress` as the primary identifier aligns with blockchain-first architecture while maintaining all existing functionality. | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,46 @@ | ||||||||||||||||||||||||||||||||||||||||
| import { MigrationInterface, QueryRunner } from 'typeorm'; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| export class MigrateUserIdToUUID1751199237000 implements MigrationInterface { | ||||||||||||||||||||||||||||||||||||||||
| name = 'MigrateUserIdToUUID1751199237000'; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| public async up(queryRunner: QueryRunner): Promise<void> { | ||||||||||||||||||||||||||||||||||||||||
| // First, add a new UUID column | ||||||||||||||||||||||||||||||||||||||||
| await queryRunner.query(`ALTER TABLE "users" ADD COLUMN "id_new" UUID DEFAULT gen_random_uuid()`); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Update existing records to have unique UUIDs | ||||||||||||||||||||||||||||||||||||||||
| await queryRunner.query(`UPDATE "users" SET "id_new" = gen_random_uuid() WHERE "id_new" IS NULL`); | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+8
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Redundant UUID generation in UPDATE statement Line 11 attempts to update records where Remove the redundant UPDATE or modify the logic: // First, add a new UUID column
- await queryRunner.query(`ALTER TABLE "users" ADD COLUMN "id_new" UUID DEFAULT gen_random_uuid()`);
+ await queryRunner.query(`ALTER TABLE "users" ADD COLUMN "id_new" UUID`);
// Update existing records to have unique UUIDs
- await queryRunner.query(`UPDATE "users" SET "id_new" = gen_random_uuid() WHERE "id_new" IS NULL`);
+ await queryRunner.query(`UPDATE "users" SET "id_new" = gen_random_uuid()`);π Committable suggestion
Suggested change
π€ Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Drop the old id column and rename the new one | ||||||||||||||||||||||||||||||||||||||||
| await queryRunner.query(`ALTER TABLE "users" DROP CONSTRAINT "PK_a3ffb1c0c8416b9fc6f907b7433"`); | ||||||||||||||||||||||||||||||||||||||||
| await queryRunner.query(`ALTER TABLE "users" DROP COLUMN "id"`); | ||||||||||||||||||||||||||||||||||||||||
| await queryRunner.query(`ALTER TABLE "users" RENAME COLUMN "id_new" TO "id"`); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Make the new id column the primary key | ||||||||||||||||||||||||||||||||||||||||
| await queryRunner.query(`ALTER TABLE "users" ADD CONSTRAINT "PK_a3ffb1c0c8416b9fc6f907b7433" PRIMARY KEY ("id")`); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Ensure walletAddress is unique and indexed | ||||||||||||||||||||||||||||||||||||||||
| await queryRunner.query(`CREATE UNIQUE INDEX IF NOT EXISTS "IDX_users_walletAddress" ON "users" ("walletAddress")`); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Update related tables that reference user id | ||||||||||||||||||||||||||||||||||||||||
| // Note: This migration assumes other tables will be updated separately | ||||||||||||||||||||||||||||||||||||||||
| // to use UUID foreign keys | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| public async down(queryRunner: QueryRunner): Promise<void> { | ||||||||||||||||||||||||||||||||||||||||
| // Revert to SERIAL id | ||||||||||||||||||||||||||||||||||||||||
| await queryRunner.query(`ALTER TABLE "users" ADD COLUMN "id_old" SERIAL`); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Drop the UUID primary key constraint | ||||||||||||||||||||||||||||||||||||||||
| await queryRunner.query(`ALTER TABLE "users" DROP CONSTRAINT "PK_a3ffb1c0c8416b9fc6f907b7433"`); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Rename columns | ||||||||||||||||||||||||||||||||||||||||
| await queryRunner.query(`ALTER TABLE "users" DROP COLUMN "id"`); | ||||||||||||||||||||||||||||||||||||||||
| await queryRunner.query(`ALTER TABLE "users" RENAME COLUMN "id_old" TO "id"`); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Restore the SERIAL primary key | ||||||||||||||||||||||||||||||||||||||||
| await queryRunner.query(`ALTER TABLE "users" ADD CONSTRAINT "PK_a3ffb1c0c8416b9fc6f907b7433" PRIMARY KEY ("id")`); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Drop the walletAddress index | ||||||||||||||||||||||||||||||||||||||||
| await queryRunner.query(`DROP INDEX IF EXISTS "IDX_users_walletAddress"`); | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+29
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Down migration is destructive and will fail The down migration attempts to convert UUIDs back to SERIAL integers, which will fail because:
This migration should be marked as irreversible or implement a proper rollback strategy: public async down(queryRunner: QueryRunner): Promise<void> {
+ throw new Error('This migration cannot be reversed. UUID to integer conversion would result in data loss.');
- // Revert to SERIAL id
- await queryRunner.query(`ALTER TABLE "users" ADD COLUMN "id_old" SERIAL`);
- // ... rest of the down migration
}π Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| import { MigrationInterface, QueryRunner } from 'typeorm'; | ||
|
|
||
| export class UpdateForeignKeysToUUID1751199238000 implements MigrationInterface { | ||
| name = 'UpdateForeignKeysToUUID1751199238000'; | ||
|
|
||
| public async up(queryRunner: QueryRunner): Promise<void> { | ||
| // Update user_roles table | ||
| await queryRunner.query(`ALTER TABLE "user_roles" ALTER COLUMN "userId" TYPE UUID USING "userId"::uuid`); | ||
|
|
||
| // Update buyer_requests table | ||
| await queryRunner.query(`ALTER TABLE "buyer_requests" ALTER COLUMN "userId" TYPE UUID USING "userId"::uuid`); | ||
|
|
||
| // Update reviews table | ||
| await queryRunner.query(`ALTER TABLE "reviews" ALTER COLUMN "userId" TYPE UUID USING "userId"::uuid`); | ||
|
|
||
| // Note: carts and orders already use UUID for user_id | ||
|
|
||
| // Add foreign key constraints if they don't exist | ||
| await queryRunner.query(` | ||
| ALTER TABLE "user_roles" | ||
| ADD CONSTRAINT "FK_user_roles_user" | ||
| FOREIGN KEY ("userId") REFERENCES "users"("id") ON DELETE CASCADE | ||
| `); | ||
|
|
||
| await queryRunner.query(` | ||
| ALTER TABLE "buyer_requests" | ||
| ADD CONSTRAINT "FK_buyer_requests_user" | ||
| FOREIGN KEY ("userId") REFERENCES "users"("id") ON DELETE CASCADE | ||
| `); | ||
|
|
||
| await queryRunner.query(` | ||
| ALTER TABLE "reviews" | ||
| ADD CONSTRAINT "FK_reviews_user" | ||
| FOREIGN KEY ("userId") REFERENCES "users"("id") ON DELETE CASCADE | ||
| `); | ||
|
|
||
| await queryRunner.query(` | ||
| ALTER TABLE "carts" | ||
| ADD CONSTRAINT "FK_carts_user" | ||
| FOREIGN KEY ("user_id") REFERENCES "users"("id") ON DELETE CASCADE | ||
| `); | ||
|
|
||
| await queryRunner.query(` | ||
| ALTER TABLE "orders" | ||
| ADD CONSTRAINT "FK_orders_user" | ||
| FOREIGN KEY ("user_id") REFERENCES "users"("id") ON DELETE CASCADE | ||
| `); | ||
| } | ||
|
|
||
| public async down(queryRunner: QueryRunner): Promise<void> { | ||
| // Drop foreign key constraints | ||
| await queryRunner.query(`ALTER TABLE "user_roles" DROP CONSTRAINT IF EXISTS "FK_user_roles_user"`); | ||
| await queryRunner.query(`ALTER TABLE "buyer_requests" DROP CONSTRAINT IF EXISTS "FK_buyer_requests_user"`); | ||
| await queryRunner.query(`ALTER TABLE "reviews" DROP CONSTRAINT IF EXISTS "FK_reviews_user"`); | ||
| await queryRunner.query(`ALTER TABLE "carts" DROP CONSTRAINT IF EXISTS "FK_carts_user"`); | ||
| await queryRunner.query(`ALTER TABLE "orders" DROP CONSTRAINT IF EXISTS "FK_orders_user"`); | ||
|
|
||
| // Revert column types to integer (this will require data migration in a real scenario) | ||
| await queryRunner.query(`ALTER TABLE "user_roles" ALTER COLUMN "userId" TYPE INTEGER USING "userId"::integer`); | ||
| await queryRunner.query(`ALTER TABLE "buyer_requests" ALTER COLUMN "userId" TYPE INTEGER USING "userId"::integer`); | ||
| await queryRunner.query(`ALTER TABLE "reviews" ALTER COLUMN "userId" TYPE INTEGER USING "userId"::integer`); | ||
|
Comment on lines
+59
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Down migration will lose UUID data Converting UUIDs back to integers in the down migration will result in data loss since UUIDs cannot be meaningfully converted to integers. This makes the migration effectively irreversible in production. Consider documenting this as a one-way migration or implementing a mapping table approach: // Revert column types to integer (this will require data migration in a real scenario)
+ // WARNING: This is a destructive operation that will lose the UUID values
+ // Consider implementing a mapping table if rollback capability is needed
await queryRunner.query(`ALTER TABLE "user_roles" ALTER COLUMN "userId" TYPE INTEGER USING "userId"::integer`);
π€ Prompt for AI Agents |
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
π‘ Verification agent
π§© Analysis chain
Add language identifiers to fenced code blocks (markdownlint MD040).
Specify a language for the HTTP route examples to satisfy markdownlint and improve rendering.
Apply this diff:
-
+httpPUT /users/update/:walletAddress
GET /users/:walletAddress
Run markdownlint to verify:
Also applies to: 38-41
π Script executed:
Length of output: 18567
π Script executed:
Length of output: 286
π Script executed:
Length of output: 256
Add HTTP language identifiers to code fences in docs/UUID-MIGRATION.md
To satisfy markdownlint MD040 and improve readability, the unnamed fenced blocks under βBefore (using numeric ID)β and βAfter (using walletAddress)β need a language identifier.
β’ docs/UUID-MIGRATION.md, lines 32β35 (Before)
β’ docs/UUID-MIGRATION.md, lines 38β41 (After)
Apply this diff:
π§° Tools
πͺ markdownlint-cli2 (0.17.2)
32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
π€ Prompt for AI Agents