Fix/4 security issues - #585
Merged
Merged
Conversation
Add TokenRevocationService (Redis set + per-JTI TTL), RevokeController exposing POST /auth/revoke (admin), JwtRevocationGuard to reject requests bearing a revoked jti, a jti-payload helper, and an e2e test covering revoke -> subsequent 401.
Add a dependency-free StrKey checksum validator + reusable Angular reactive-forms directive (stellarAddress) so invalid "transfer to" addresses are caught before hitting the API, plus a matching class-validator @IsStellarAddress() decorator for backend DTOs. Jasmine spec covers valid/garbage/corrupted-checksum/wrong-length cases.
Add contractCallTotal counter (contract, method, status) and contractCallDurationSeconds histogram (contract, method), plus a withContractMetrics() wrapper to instrument StellarService.invokeContract call sites so per-method failure rates (e.g. approve_and_mint) become visible instead of requiring manual log analysis.
Add scripts/verify-backup-restore.sh (download latest S3 pg_dump, restore into a throwaway postgres:16-alpine container via pg_restore, run smoke-test queries for row counts + expected indexes, alert Slack on failure) and a weekly GitHub Actions scheduled workflow to run it, so incomplete/corrupted backups are caught before they're needed.
|
@martinzhames 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! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #528
closes #529
closes #530
closes #527
Why this matters now: If an admin JWT is leaked (e.g., logged to third-party service), there's no way to revoke it. The attacker can issue unlimited credits for 7 days. Token revocation is a mandatory security control for mainnet.
Problem / What: JwtStrategy.validate checks signature and expiration but doesn't consult a revocation list. Need a Redis-backed blocklist for revoked tokens.
Key Challenges:
Add POST /api/v1/auth/revoke endpoint (admin only) that adds token JTI to Redis set with TTL matching token expiration.
JwtStrategy.validate checks RedisService.sismember('revoked_tokens', jti) before allowing request.
Generate unique JTI for every token (use uuid()).
Add jti to JWT payload in AuthService.generateToken.
E2E test: generate token, revoke it, verify subsequent requests return 401.
Why this matters now: Users can manually edit the "transfer to" address field and enter garbage like "abc123". The API returns 400, but the frontend shows a generic "Transfer failed" error with no guidance. Client-side validation would prevent the API call and show "Invalid Stellar address (must start with G and be 56 chars)".
Problem / What: frontend/src/app/credits/credit-detail.component.ts — transfer method sends raw user input to API with no validation. TransferCreditDto has @IsString() but not @matches(/^G[A-Z0-9]{55}$/).
Key Challenges:
Add stellar-sdk import to frontend and use StrKey.isValidEd25519PublicKey(address).
Create a reusable StellarAddressValidator directive for reactive forms.
Update all forms that accept addresses (transfer, retire, project registration).
Show validation error immediately (red border + error message) when user blurs the input field.
Unit test with Jasmine verifies invalid addresses are rejected.