Merged
Conversation
Reviewer's GuideImplements full authorization logic for document access based on document sensitivity and parent entity, significantly expands tests around canAccessDocument, wires new mortgage/obligation/onboardingRequest seed modules into the API, tightens auth usage with authedQuery, and improves error handling and future permission-gate documentation in document engine modules. Class diagram for document access helpers and related entitiesclassDiagram
class Viewer {
authId: string
isFairLendAdmin: boolean
permissions: Set_string
}
class GeneratedDocument {
_id: Id_generatedDocuments
entityType: string
entityId: string
sensitivityTier: string
}
class DealAccessRecord {
_id: Id_dealAccess
userId: Id_users
dealId: Id_deals
status: string
}
class Deal {
_id: Id_deals
mortgageId: Id_mortgages
}
class Mortgage {
_id: Id_mortgages
}
class ApplicationPackage {
_id: Id_applicationPackages
}
class ProvisionalApplication {
_id: Id_provisionalApplications
brokerId: Id_brokers
borrowerId: Id_borrowers
}
class ResourceChecks {
+canAccessDocument(ctx_db, viewer, documentId)
+hasActiveDealAccess(ctx_db, viewer, entityType, entityId)
+canAccessMortgage(ctx_db, viewer, mortgageId)
+canAccessDeal(ctx_db, viewer, dealId)
+canAccessApplicationPackage(ctx_db, viewer, applicationPackageId)
+getBrokerByAuthId(ctx_db, authId)
+getBorrowerByAuthId(ctx_db, authId)
}
class AuthContext {
db: QueryDb
viewer: Viewer
}
ResourceChecks --> GeneratedDocument : reads
ResourceChecks --> DealAccessRecord : queries
ResourceChecks --> Deal : queries
ResourceChecks --> Mortgage : queries
ResourceChecks --> ApplicationPackage : queries
ResourceChecks --> ProvisionalApplication : queries
AuthContext *-- Viewer
AuthContext *-- QueryDb
GeneratedDocument --> Deal : may_link_via_entityType_deal
GeneratedDocument --> Mortgage : may_link_via_entityType_mortgage
GeneratedDocument --> ApplicationPackage : may_link_via_entityType_applicationPackage
GeneratedDocument --> ProvisionalApplication : may_link_via_entityType_provisionalApplication
DealAccessRecord --> Deal : many_to_one
DealAccessRecord --> Viewer : many_to_one_via_userId
ProvisionalApplication --> Viewer : linked_via_broker_or_borrower
Flow diagram for hasActiveDealAccess helperflowchart TD
B_start[Start hasActiveDealAccess]
B1{entityType}
B2[Query dealAccess using by_user_and_deal index
filter userId == viewer.authId and dealId == entityId as Id_deals]
B3[Collect records]
B4{Any record.status == active?}
B5[Return true]
B6[Return false]
B7[Query deals using by_mortgage index
filter mortgageId == entityId as Id_mortgages]
B8[Collect deals]
B9[For each deal]
B10[Query dealAccess using by_user_and_deal index
filter userId == viewer.authId and dealId == deal._id]
B11[Collect records for deal]
B12{Any record.status == active?}
B13[Return true]
B14[After loop, return false]
B15[Return true for pre_deal entities
applicationPackage and provisionalApplication]
B_start --> B1
B1 -->|deal| B2 --> B3 --> B4
B4 -->|yes| B5
B4 -->|no| B6
B1 -->|mortgage| B7 --> B8 --> B9 --> B10 --> B11 --> B12
B12 -->|yes| B13
B12 -->|no| B9
B9 --> B14
B1 -->|applicationPackage or provisionalApplication or other| B15
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThis pull request refactors authentication in auth.ts by replacing Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant canAccessDocument as canAccessDocument Handler
participant DB as Database
participant EntityHandler as Entity-Type Handler<br/>(Mortgage/Deal/etc.)
participant DealAccess as Deal Access Validator
Caller->>canAccessDocument: Request document access<br/>with documentId
canAccessDocument->>DB: Fetch generatedDocument
DB-->>canAccessDocument: Document with entityType & sensitivity
canAccessDocument->>canAccessDocument: Check entity-level access<br/>via type dispatcher
canAccessDocument->>EntityHandler: canAccessMortgage<br/>canAccessDeal<br/>canAccessApplicationPackage<br/>canAccessProvisionalApplication
EntityHandler-->>canAccessDocument: Entity access granted?
alt Entity Access Denied
canAccessDocument-->>Caller: Access Denied
end
alt Public Sensitivity
canAccessDocument-->>Caller: Access Granted
end
alt Private or Sensitive
canAccessDocument->>DealAccess: hasActiveDealAccess<br/>(entityType, entityId)
DealAccess->>DB: Query deal state & permissions
DB-->>DealAccess: Deal access records
DealAccess-->>canAccessDocument: Active deal access?
alt No Active Deal Access
canAccessDocument-->>Caller: Access Denied
end
alt Sensitive Tier
canAccessDocument->>canAccessDocument: Check permission<br/>documents:sensitive_access
alt Permission Denied
canAccessDocument-->>Caller: Access Denied
end
end
end
canAccessDocument-->>Caller: Access Granted
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
|
@greptile please review |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 5 issues, and left some high level feedback:
- In
canAccessDocument/hasActiveDealAccess, the comment says applicationPackage/provisionalApplication only require entity-level access for all tiers, but sensitive documents still go through thedocuments:sensitive_accesspermission check; consider either special-casing these entity types in the sensitive tier branch or updating the comment/tests to match the actual behavior. - The mortgage branch of
hasActiveDealAccessissues onedealAccessquery per related deal, which can become N+1 queries for mortgages with many deals; consider adding a more suitable index or restructuring the query to check all relevantdealAccessrecords in a single query if this path is performance-sensitive.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `canAccessDocument` / `hasActiveDealAccess`, the comment says applicationPackage/provisionalApplication only require entity-level access for all tiers, but sensitive documents still go through the `documents:sensitive_access` permission check; consider either special-casing these entity types in the sensitive tier branch or updating the comment/tests to match the actual behavior.
- The mortgage branch of `hasActiveDealAccess` issues one `dealAccess` query per related deal, which can become N+1 queries for mortgages with many deals; consider adding a more suitable index or restructuring the query to check all relevant `dealAccess` records in a single query if this path is performance-sensitive.
## Individual Comments
### Comment 1
<location path="convex/auth/resourceChecks.ts" line_range="318-327" />
<code_context>
+// private → entity access + dealAccess record required
+// sensitive → entity access + dealAccess + documents:sensitive_access permission
+
+async function hasActiveDealAccess(
+ ctx: { db: QueryCtx["db"] },
+ viewer: Viewer,
+ entityType: string,
+ entityId: string
+): Promise<boolean> {
+ if (entityType === "deal") {
+ const records = await ctx.db
+ .query("dealAccess")
+ .withIndex("by_user_and_deal", (q) =>
+ q.eq("userId", viewer.authId).eq("dealId", entityId as Id<"deals">)
+ )
+ .collect();
+ return records.some((r) => r.status === "active");
+ }
+
+ if (entityType === "mortgage") {
+ const deals = await ctx.db
+ .query("deals")
</code_context>
<issue_to_address>
**suggestion:** Consider tightening the `entityType` parameter type in `hasActiveDealAccess` to a union of known entity types instead of `string`.
Since this function only has specific logic for "deal" and "mortgage" and otherwise defaults to `true` (for applicationPackage/provisionalApplication), keeping `entityType` as `string` makes it easy to pass an unsupported value and silently get the default branch. A union of the supported values (or reusing `doc.entityType`’s type) would prevent that at compile time.
Suggested implementation:
```typescript
async function hasActiveDealAccess(
ctx: { db: QueryCtx["db"] },
viewer: Viewer,
entityType: "deal" | "mortgage" | "applicationPackage" | "provisionalApplication",
entityId: string
): Promise<boolean> {
```
If there is an existing type for the `entityType` field on the underlying document (e.g. a `GeneratedDocument["entityType"]` union or similar), it would be preferable to:
1. Import that type into this file, and
2. Replace the inline union with that type alias for better reuse and single-source-of-truth for supported entity types.
Call sites of `hasActiveDealAccess` will also need to be adjusted if they currently pass arbitrary strings or values that are not part of this union.
</issue_to_address>
### Comment 2
<location path="convex/auth.ts" line_range="296" />
<code_context>
-export const getCurrentUser = convex
- .query()
+export const getCurrentUser = authedQuery
.handler(async (ctx) => {
const user = await authKit.getAuthUser(ctx);
</code_context>
<issue_to_address>
**question (bug_risk):** Switching `getCurrentUser` from a generic query to `authedQuery` changes its behavior for unauthenticated callers.
Using `authedQuery` means unauthenticated callers will now be rejected at the middleware layer instead of being able to invoke `getCurrentUser`. If any existing flows call this before login (e.g., to probe for an existing session), those clients may now break. If this change is intentional, no action needed; otherwise, please verify the call sites and adjust expectations or behavior accordingly.
</issue_to_address>
### Comment 3
<location path="convex/auth/__tests__/resourceChecks.test.ts" line_range="1018" />
<code_context>
+ describe("canAccessDocument", () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test case for admin access when the document does not exist
Currently admin access is only tested when the document exists. Since `canAccessDocument` short-circuits on `viewer.isFairLendAdmin` before any DB lookup, please add a case like `"admin — non-existent document — true"` that passes a deleted/non-existent `generatedDocuments` id and asserts `true` to protect against regressions where admin access starts depending on document existence.
</issue_to_address>
### Comment 4
<location path="convex/auth/__tests__/resourceChecks.test.ts" line_range="1363" />
<code_context>
+ });
+ });
+
+ it("private + applicationPackage — entity access sufficient (pre-deal)", async () => {
+ const t = convexTest(schema, modules);
+ await t.run(async (ctx) => {
</code_context>
<issue_to_address>
**suggestion (testing):** Add coverage for sensitive-tier applicationPackage access with and without permission
`applicationPackage` comments and `hasActiveDealAccess` imply entity-level access should be enough for all tiers, but the core logic still enforces `documents:sensitive_access` for `sensitive` docs. Right now only `public` and `private` applicationPackage flows are tested. Please add tests for the sensitive tier, e.g.: (1) `sensitive + applicationPackage` as `sr_underwriter` with `documents:sensitive_access` → `true`, and (2) the same without `documents:sensitive_access` → `false`, to verify pre-deal behavior matches the intended security model.
Suggested implementation:
```typescript
// Broker has entity access to mortgage but no dealAccess
const viewer = makeViewer({ authId: "broker-auth" });
const result = await canAccessDocument(ctx, viewer, docId);
expect(result).toBe(false);
});
});
it("sensitive + applicationPackage — entity access sufficient with documents:sensitive_access (pre-deal)", async () => {
const t = convexTest(schema, modules);
await t.run(async (ctx) => {
const { templateId, storageId } = await insertDocumentPrereqs(ctx);
const underwriterUserId = await insertUser(ctx, {
authId: "sr-underwriter-auth",
role: "sr_underwriter",
});
const brokerId = await insertBroker(ctx, underwriterUserId);
const propId = await insertProperty(ctx);
const mortgageId = await insertMortgage(ctx, propId, brokerId);
const docId = await insertGeneratedDocument(
ctx,
templateId,
storageId,
"mortgage",
"sensitive"
);
const viewer = makeViewer({
authId: "sr-underwriter-auth",
role: "sr_underwriter",
permissions: ["documents:sensitive_access"],
applicationPackage: {
propertyId: propId,
},
});
const result = await canAccessDocument(ctx, viewer, docId);
expect(result).toBe(true);
});
});
it("sensitive + applicationPackage — entity access insufficient without documents:sensitive_access (pre-deal)", async () => {
const t = convexTest(schema, modules);
await t.run(async (ctx) => {
const { templateId, storageId } = await insertDocumentPrereqs(ctx);
const underwriterUserId = await insertUser(ctx, {
authId: "sr-underwriter-no-sensitive-auth",
role: "sr_underwriter",
});
const brokerId = await insertBroker(ctx, underwriterUserId);
const propId = await insertProperty(ctx);
const mortgageId = await insertMortgage(ctx, propId, brokerId);
const docId = await insertGeneratedDocument(
ctx,
templateId,
storageId,
"mortgage",
"sensitive"
);
const viewer = makeViewer({
authId: "sr-underwriter-no-sensitive-auth",
role: "sr_underwriter",
// NOTE: no documents:sensitive_access here
permissions: [],
applicationPackage: {
propertyId: propId,
},
});
const result = await canAccessDocument(ctx, viewer, docId);
expect(result).toBe(false);
});
});
```
You may need to align the helper usage with the rest of the file:
1. **Viewer shape**:
- If `makeViewer` expects different keys (e.g. `roles` instead of `role`, `scopes` instead of `permissions`, or a different shape for `applicationPackage`), adjust the viewer objects accordingly to match how the existing `public/private + applicationPackage` tests construct viewers.
2. **User / broker relationship**:
- If underwriters aren’t modeled via `insertBroker`, replace that with the appropriate helper used for `sr_underwriter` in other tests (or drop the broker entirely if not needed for entity access in the application package flow).
3. **Tier argument**:
- Ensure `insertGeneratedDocument`’s final `"sensitive"` argument matches the actual enum/string used elsewhere for sensitive-tier documents.
4. **Auth IDs and roles**:
- If the role identifier is different (e.g. `"sr_underwriter_role"` or comes from a constant), substitute the correct value so the tests reflect how `sr_underwriter` is represented in your auth model.
</issue_to_address>
### Comment 5
<location path="convex/auth/__tests__/resourceChecks.test.ts" line_range="1406" />
<code_context>
+
+ // ── Sensitive tier: entity + dealAccess + permission ──────────
+
+ it("sensitive + deal — dealAccess + permission — true", async () => {
+ const t = convexTest(schema, modules);
+ await t.run(async (ctx) => {
</code_context>
<issue_to_address>
**suggestion (testing):** Extend sensitive-tier coverage to mortgage-backed documents as well
Current tests only cover sensitive-tier logic for `deal` docs. Since mortgages can also be sensitive and depend on `hasActiveDealAccess` via related deals, please add coverage for at least: (a) `sensitive + mortgage` with related-deal access and permission → `true`, (b) same but without permission → `false`, and ideally (c) entity access but no active deal access → `false`, to validate the combined entity/dealAccess/permission checks for mortgages as well.
Suggested implementation:
```typescript
// ── Sensitive tier: entity + dealAccess + permission ──────────
it("sensitive + mortgage — dealAccess + permission — true", async () => {
const t = convexTest(schema, modules);
await t.run(async (ctx) => {
const { templateId, storageId } = await insertDocumentPrereqs(ctx);
const brokerUserId = await insertUser(ctx, { authId: "broker-auth" });
const brokerId = await insertBroker(ctx, brokerUserId);
const propId = await insertProperty(ctx);
const mortgageId = await insertMortgage(ctx, propId, brokerId);
const docId = await insertGeneratedDocument(
ctx,
templateId,
storageId,
"mortgage",
mortgageId,
{
sensitivityTier: "sensitive",
},
);
const viewer = await insertViewer(ctx, {
authId: "sr-uw-auth",
roles: new Set(["sr_underwriter"]),
});
const result = await canAccessDocument(ctx, viewer, docId);
expect(result).toBe(true);
});
});
it("sensitive + mortgage — dealAccess + no permission — false", async () => {
const t = convexTest(schema, modules);
await t.run(async (ctx) => {
const { templateId, storageId } = await insertDocumentPrereqs(ctx);
const brokerUserId = await insertUser(ctx, { authId: "broker-auth" });
const brokerId = await insertBroker(ctx, brokerUserId);
const propId = await insertProperty(ctx);
const mortgageId = await insertMortgage(ctx, propId, brokerId);
const docId = await insertGeneratedDocument(
ctx,
templateId,
storageId,
"mortgage",
mortgageId,
{
sensitivityTier: "sensitive",
},
);
const viewer = await insertViewer(ctx, {
authId: "no-sensitive-auth",
roles: new Set(), // viewer without sensitive-docs permission
});
const result = await canAccessDocument(ctx, viewer, docId);
expect(result).toBe(false);
});
});
it("sensitive + mortgage — entity access only, no active deal access — false", async () => {
const t = convexTest(schema, modules);
await t.run(async (ctx) => {
const { templateId, storageId } = await insertDocumentPrereqs(ctx);
const brokerUserId = await insertUser(ctx, { authId: "broker-auth" });
const brokerId = await insertBroker(ctx, brokerUserId);
const propId = await insertProperty(ctx);
const mortgageId = await insertMortgage(ctx, propId, brokerId);
const docId = await insertGeneratedDocument(
ctx,
templateId,
storageId,
"mortgage",
mortgageId,
{
sensitivityTier: "sensitive",
},
);
// Viewer associated with the same entity but without active deal access
const viewer = await insertViewer(ctx, {
authId: "entity-only-auth",
roles: new Set(["sr_underwriter"]),
});
const result = await canAccessDocument(ctx, viewer, docId);
expect(result).toBe(false);
});
});
```
Because only a fragment of the test file is visible, you should:
1. Align the viewer/role setup with whatever the existing “sensitive + deal” tests use:
- If there is a dedicated helper for seeding permissions (e.g. `grantPermission` or similar), swap out the simple role changes above for that helper so that “with permission”/“without permission” exactly mirror the deal tests.
2. Ensure that mortgages are actually tied to deals for the `hasActiveDealAccess` logic:
- If the existing deal tests explicitly create a deal and call a helper like `insertDealAccess`/`insertActiveDealAccess`, replicate that pattern here so that `mortgageId` is associated with the same deal and the viewer’s deal access matches the intended scenario in each test.
3. If `insertViewer` has a different name or the viewer factory takes additional required fields, update the viewer creation blocks to match the existing tests’ convention.
4. If `sensitivityTier` is set differently in your existing tests (constant, enum, or nested field), update the `insertGeneratedDocument` calls to follow that same shape so the tests correctly exercise the sensitive-tier path.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR implements the canAccessDocument resource check (T-009 / ENG-144), replacing a stub that blocked all non-admin access with a full three-tier sensitivity model (public, private, sensitive). It also refactors getCurrentUser to use authedQuery, adds TODO comments for production permission gates in the document engine, cleans up an unused type constraint in requireFairLendAdminAction, and registers three new seed modules.
Changes:
- Implemented
canAccessDocumentwith entity-level access checks, deal-access checks, and sensitive-tier permission checks, along with comprehensive tests covering all tiers and entity types. - Refactored
getCurrentUserfromconvex.query()toauthedQuery, adding authentication enforcement. - Added TODO comments across document engine modules for future production permission gates, removed unused
runMutationtype fromrequireFairLendAdminAction, and registered new seed modules (seedMortgage,seedObligation,seedOnboardingRequest).
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
convex/auth/resourceChecks.ts |
Replaced canAccessDocument stub with full three-tier sensitivity implementation and hasActiveDealAccess helper |
convex/auth/__tests__/resourceChecks.test.ts |
Added ~530 lines of tests covering admin, non-existent doc, public/private/sensitive tiers across all entity types |
convex/auth.ts |
Changed getCurrentUser from convex.query() to authedQuery |
convex/fluent.ts |
Removed unused runMutation from requireFairLendAdminAction context type |
convex/seed/seedObligation.ts |
Wrapped error template literals with String() for type safety |
convex/documentEngine/*.ts |
Added TODO comments for production permission gates |
convex/_generated/api.d.ts |
Registered three new seed modules |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryThis PR implements the Key changes:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[canAccessDocument called] --> B{viewer.isFairLendAdmin?}
B -- Yes --> Z[return true]
B -- No --> C{doc exists?}
C -- No --> F[return false]
C -- Yes --> D[Step 1: Entity-level access check]
D --> D1{entityType?}
D1 -- mortgage --> E1[canAccessMortgage]
D1 -- deal --> E2[canAccessDeal]
D1 -- applicationPackage --> E3[canAccessApplicationPackage]
D1 -- provisionalApplication --> E4[check broker/borrower ownership]
E1 --> G{hasEntityAccess?}
E2 --> G
E3 --> G
E4 --> G
G -- No --> F
G -- Yes --> H{sensitivityTier = public?}
H -- Yes --> Z
H -- No --> I[Step 3: hasActiveDealAccess check]
I --> I1{entityType?}
I1 -- deal --> I2[query dealAccess by userId + dealId]
I1 -- mortgage --> I3[find related deals, check any active dealAccess]
I1 -- applicationPackage / provisionalApplication --> I4[return true bypass]
I2 --> J{hasDealAccess?}
I3 --> J
I4 --> J
J -- No --> F
J -- Yes --> K{sensitivityTier = sensitive?}
K -- No --> Z
K -- Yes --> L{has documents:sensitive_access permission?}
L -- Yes --> Z
L -- No --> F
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+355
to
+357
| // applicationPackage / provisionalApplication are pre-deal entities — | ||
| // entity-level access (underwriting RBAC) is sufficient for all tiers. | ||
| return true; |
There was a problem hiding this comment.
hasActiveDealAccess always returns true for applicationPackage / provisionalApplication — bypasses the deal access gate for private tier
The comment states "entity-level access (underwriting RBAC) is sufficient for all tiers." For the sensitive tier this still works correctly because Step 4 independently checks viewer.permissions.has("documents:sensitive_access"). However, for the private tier the entire hasDealAccessResult gate is bypassed — any user who passes the entity-level check (e.g. any sr_underwriter) automatically satisfies the private-tier requirement with no dealAccess record needed.
If this is intentional, it should be clearly documented that private docs on pre-deal entities intentionally have no deal access gate. If it is not intentional and private documents on an applicationPackage should still require a deal access record when a related deal exists, the fallback should return false (or look up the linked deal) rather than true.
There is also no test covering sensitive + applicationPackage denied when documents:sensitive_access is missing, which would help verify the permission gate is still active for this path.
Connorbelez
force-pushed
the
Connorbelez/auth-audit
branch
from
ae60b5b to
20a9861
Compare
This was referenced Mar 18, 2026
Merged
Connorbelez
added a commit
that referenced
this pull request
Apr 20, 2026
### TL;DR Added new seed modules for mortgage, obligation, and onboarding request entities, improved authentication patterns, and enhanced error handling with type safety improvements. ### What changed? - Added three new seed modules: `seedMortgage`, `seedObligation`, and `seedOnboardingRequest` to the API - Refactored `getCurrentUser` in auth.ts to use `authedQuery` instead of the generic `convex.query()` - Added TODO comments across document engine modules indicating plans to add permission gates for production - Removed unused `runMutation` parameter from `requireFairLendAdminAction` context type - Enhanced error handling in `seedObligation.ts` by using `String()` conversion for better type safety in error messages ### How to test? - Verify the new seed modules are accessible through the API - Test authentication flows to ensure `getCurrentUser` still works correctly - Run existing document engine operations to confirm functionality remains intact - Test obligation seeding with various states to verify improved error handling ### Why make this change? The changes expand seeding capabilities for core business entities while improving code organization and type safety. The authentication refactoring provides cleaner patterns, and the TODO comments establish a clear roadmap for production security hardening. The error handling improvements prevent potential runtime issues with undefined values in error messages. ## Summary by Sourcery Implement document access control for generated documents with sensitivity tiers and update seeding, auth, and document engine scaffolding. New Features: - Expose new mortgage, obligation, and onboarding request seed modules through the API. - Introduce document access control for generated documents based on entity relationships and sensitivity tiers. Bug Fixes: - Improve type safety of obligation seeding errors by safely stringifying unsupported states. Enhancements: - Implement full canAccessDocument logic that resolves generated documents to their parent entities and enforces entity, deal access, and permission checks. - Refine getCurrentUser to use the authenticated query helper for consistent auth handling. - Annotate document engine queries and actions with TODOs describing future permission gates for production hardening. - Simplify the FairLend admin action middleware context by removing an unused runMutation parameter. Tests: - Add comprehensive tests covering document access for admins, missing documents, and public, private, and sensitive tiers across mortgages, deals, application packages, and provisional applications. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Implemented document access control with three sensitivity tiers (public, private, sensitive) and role-based permission checks based on user roles and entity relationships. * **Tests** * Expanded document access control test coverage with comprehensive scenarios across multiple entity types, roles, and permission combinations. * **Chores** * Added notes for future permission gate implementations and improved error message formatting. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
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.
TL;DR
Added new seed modules for mortgage, obligation, and onboarding request entities, improved authentication patterns, and enhanced error handling with type safety improvements.
What changed?
seedMortgage,seedObligation, andseedOnboardingRequestto the APIgetCurrentUserin auth.ts to useauthedQueryinstead of the genericconvex.query()runMutationparameter fromrequireFairLendAdminActioncontext typeseedObligation.tsby usingString()conversion for better type safety in error messagesHow to test?
getCurrentUserstill works correctlyWhy make this change?
The changes expand seeding capabilities for core business entities while improving code organization and type safety. The authentication refactoring provides cleaner patterns, and the TODO comments establish a clear roadmap for production security hardening. The error handling improvements prevent potential runtime issues with undefined values in error messages.
Summary by Sourcery
Implement document access control for generated documents with sensitivity tiers and update seeding, auth, and document engine scaffolding.
New Features:
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit
Release Notes
New Features
Tests
Refactor