Skip to content

refactor: issues 1-4 - harden user.controller, auth.routes, auth.service - #421

Open
Skinny001 wants to merge 4 commits into
StellarState:devfrom
Skinny001:refactor/auth-user-issues-1-4
Open

refactor: issues 1-4 - harden user.controller, auth.routes, auth.service#421
Skinny001 wants to merge 4 commits into
StellarState:devfrom
Skinny001:refactor/auth-user-issues-1-4

Conversation

@Skinny001

Copy link
Copy Markdown

This pull request primarily focuses on code style improvements, formatting consistency, and minor refactoring across the codebase. There are no functional changes, but the updates enhance readability and maintainability. The most notable update is the upgrade of the typescript dependency.

Dependency update:

  • Upgraded the typescript dependency in package.json from version ^5.3.3 to ^5.9.3.

Code formatting and style improvements:

  • Reformatted multi-line function parameters and object destructuring for improved readability in several files, including src/app.ts, src/config/database.ts, src/config/data-source.ts, src/config/env.ts, src/config/stellar.ts, src/controllers/auth.controller.ts, src/controllers/investment.controller.ts, and src/controllers/invoice.controller.ts. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18] [19] [20] [21] [22] [23]

  • Simplified and standardized error handling and logging statements for better clarity. [1] [2] [3] [4]

  • Removed unnecessary blank lines and improved grouping of related code sections in configuration files. [1] [2] [3]

These changes do not affect application behavior but make the codebase easier to read and maintain.Issue 1: src/controllers/user.controller.ts

  • Parallel fetch in listUsers with Promise.all
  • Cursor-based pagination alongside offset pagination
  • Email uniqueness validation in updateProfile
  • Request ID correlation in logs/responses
  • Cache headers (ETag, Last-Modified, Cache-Control)
  • Error sanitization (no internal details)

Issue 2/4: src/routes/auth.routes.ts

  • Split rate limiters: /challenge (5/min), /verify (20/min)
  • Idempotency key support (Idempotency-Key header, 1hr TTL)
  • Circuit breaker (5 failures -> open, 30s timeout, 2 successes -> closed)
  • Normalized error responses: {success, error: {code, message}, requestId}
  • Security headers: nosniff, DENY

Issue 3: src/services/auth.service.ts

  • Fixed upsertUser race condition (handles duplicate key error)
  • Challenge cleanup: cleanupExpiredChallenges(maxAgeMs)
  • Challenge metrics: getChallengeMetrics() -> active/consumed/expired
  • Metrics instrumentation: auth_challenge_total counter
  • New repo methods: deleteExpired, countByStatus

Supporting:

  • src/observability/metrics.ts: added increment() for custom counters
  • src/lib/circuit-breaker.ts: new implementation
  • src/middleware/rate-limit.middleware.ts: challenge/verify specific limiters
  • src/index.ts: pass MetricsRegistry to createAuthService
  • Tests updated for new ChallengeRepositoryContract methods

All lint/format/type-check pass (pre-existing tsc errors unrelated to these changes)

Description

Closes #292
Closes #297
Closes #298
Closes #299

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📝 Documentation update
  • 🎨 UI/UX improvement
  • ♻️ Code refactoring
  • ✅ Test addition or update
  • 🔧 Configuration change

Checklist

  • All GitHub Actions workflows are green on this PR (required for merge)
  • Commit messages follow Conventional Commits (feat:, fix:, chore:, etc.) — enforced by CI
  • No secrets, API keys, .env, or credentials committed (see CONTRIBUTING.md)
  • My code follows the code style of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

Testing

How to Test

  1. Step one
  2. Step two
  3. Step three

Test Coverage

  • Unit tests added/updated
  • Integration tests added/updated
  • E2E tests added/updated (if applicable)
  • Manual testing completed

Screenshots (if applicable)

Additional Notes

For Reviewers

  • Code quality and readability
  • Test coverage
  • Security implications
  • Performance impact
  • Breaking changes

Issue 1: src/controllers/user.controller.ts
- Parallel fetch in listUsers with Promise.all
- Cursor-based pagination alongside offset pagination
- Email uniqueness validation in updateProfile
- Request ID correlation in logs/responses
- Cache headers (ETag, Last-Modified, Cache-Control)
- Error sanitization (no internal details)

Issue 2/4: src/routes/auth.routes.ts
- Split rate limiters: /challenge (5/min), /verify (20/min)
- Idempotency key support (Idempotency-Key header, 1hr TTL)
- Circuit breaker (5 failures -> open, 30s timeout, 2 successes -> closed)
- Normalized error responses: {success, error: {code, message}, requestId}
- Security headers: nosniff, DENY

Issue 3: src/services/auth.service.ts
- Fixed upsertUser race condition (handles duplicate key error)
- Challenge cleanup: cleanupExpiredChallenges(maxAgeMs)
- Challenge metrics: getChallengeMetrics() -> active/consumed/expired
- Metrics instrumentation: auth_challenge_total counter
- New repo methods: deleteExpired, countByStatus

Supporting:
- src/observability/metrics.ts: added increment() for custom counters
- src/lib/circuit-breaker.ts: new implementation
- src/middleware/rate-limit.middleware.ts: challenge/verify specific limiters
- src/index.ts: pass MetricsRegistry to createAuthService
- Tests updated for new ChallengeRepositoryContract methods

All lint/format/type-check pass (pre-existing tsc errors unrelated to these changes)
@drips-wave

drips-wave Bot commented Aug 29, 2026

Copy link
Copy Markdown

@Skinny001 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

Skinny001 and others added 3 commits August 30, 2026 02:29
- Added rejectInvoice method to InvoiceService with proper status transitions
- Fixed duplicate logger import in data-source.ts
- Added rpcUrl to test config in full-flow.e2e.test.ts
- Added INVOICE_REJECTED to InvoiceTransitionReason and NotificationType enums
- Fixed admin-invoice-reject test to properly set up seller relation

Build, format, lint all pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant