feat: dispute ownership check, risk-assessment filtering, verification constraint, pagination limit - #1225
Conversation
…n constraint, pagination limit
|
@amberly-d is attempting to deploy a commit to the Mftee's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@amberly-d 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! 🚀 |
mftee
left a comment
There was a problem hiding this comment.
Reviewed. Two genuine broken-access-control (IDOR) fixes on sensitive dispute records:
fileDispute: previously any authenticated user could file a dispute against any document regardless of ownership. Now checksdocument.ownerId !== user.id && user.role !== 'admin'before allowing it.getDispute: previously any authenticated user could read any dispute by ID with no ownership check at all. Now checksdispute.filedBy !== user.id && user.role !== 'admin'.
BothDocument.ownerIdandDispute.filedByverified as real entity fields.
Also good: a DB-level unique constraint (documentId + status) plus a service-level ConflictException preventing a second CONFIRMED verification record for the same document, and risk-score/date-range filtering with pagination for the risk-assessment list endpoint.
Approving.
…tion Resolve add/add conflict in verification.service.spec.ts against CodeGirlsInc#1221: both versions are compatible with the merged service (verified create()'s duplicate-check only fires when payload.status === CONFIRMED, which neither of CodeGirlsInc#1221's existing tests trigger), so combined into one file with all unique coverage including this PR's new duplicate-confirmed-rejection test.
Resolve conflicts: - brute-force.guard.ts: merged this PR's configurable BRUTE_FORCE_LOCK_MINUTES into CodeGirlsInc#1224's already-merged Redis-backed guard (kept Redis implementation, made its lock TTL configurable instead of hardcoded). - brute-force.guard.spec.ts (add/add): this PR's version tested the old synchronous in-memory guard, now obsolete after CodeGirlsInc#1224's Redis rewrite. Kept CodeGirlsInc#1224's Redis-mocked version. - risk-assessment.service.spec.ts (add/add): critical fix here - CodeGirlsInc#1225's already-merged risk-scoring test suite asserted expect(result.flags).toContain(RiskFlag.X) directly, but this PR changes RiskResult.flags from RiskFlag[] to RiskFlagResult[] ({flag, description} objects). Left as-is, all 17 of those assertions would have started failing the moment this PR merged. Rewrote them to expect(result.flags.map(f => f.flag)).toContain(...) and combined with this PR's localization tests into one file.
…ty-reliability Merging per repo maintainer review. Resolved conflicts against #1224 (brute-force guard) and #1225 (risk-assessment tests - fixed 17 assertions that would have broken against this PR's flags shape change from RiskFlag[] to RiskFlagResult[]). Pre-existing CI failures otherwise predate this PR.
Closes #1137
Closes #1136
Closes #1135
Closes #1134