-
Notifications
You must be signed in to change notification settings - Fork 7
Feat/validate api responses #1129
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
Feat/validate api responses #1129
Conversation
WalkthroughAdds multiple Marshmallow response schemas and re-exports, applies schema.load validation in several handlers, removes a provider-profile sanitization helper, updates OpenAPI and Postman artifacts, renames an attestation property, and adds a submittingUser field to adverse-action response schema. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant H as Handler
participant S as Schema
rect rgb(250,255,250)
note right of H: Handler composes response dict
C->>H: HTTP request
H->>H: Build response payload (dict)
H->>S: schema.load(payload)
alt Validation OK
S-->>H: Validated/serialized response
H-->>C: 200 OK + validated response
else ValidationError
S-->>H: ValidationError
H-->>C: 400/422 + error details
end
end
sequenceDiagram
autonumber
participant C as Client
participant P as Purchases Handler
participant Pay as Payment Processor
participant S as TransactionSchema
C->>P: POST /v1/purchase/privileges
P->>Pay: Charge request
alt Charge success
Pay-->>P: Charge result
P->>P: Create privileges
P->>S: schema.load(transaction_response)
S-->>P: Validated transaction
P-->>C: 200 OK + transaction
else Charge failure
Pay-->>P: Error
P-->>C: 4xx/5xx + error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
backend/compact-connect/docs/api-specification/latest-oas30.json(12 hunks)backend/compact-connect/docs/postman/postman-collection.json(19 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py(3 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.py(1 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/attestation/__init__.py(1 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/attestation/api.py(1 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/api.py(2 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/purchase/api.py(1 hunks)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/user/api.py(2 hunks)backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_purchase.py(1 hunks)backend/compact-connect/lambdas/python/compact-configuration/handlers/attestations.py(2 hunks)backend/compact-connect/lambdas/python/provider-data-v1/handlers/provider_users.py(2 hunks)backend/compact-connect/lambdas/python/provider-data-v1/handlers/providers.py(2 hunks)backend/compact-connect/lambdas/python/purchases/handlers/privileges.py(3 hunks)backend/compact-connect/lambdas/python/staff-users/handlers/me.py(2 hunks)backend/compact-connect/stacks/api_stack/api.py(0 hunks)backend/compact-connect/stacks/api_stack/v1_api/api_model.py(1 hunks)
💤 Files with no reviewable changes (1)
- backend/compact-connect/stacks/api_stack/api.py
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-09-04T17:55:02.692Z
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#1058
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:785-786
Timestamp: 2025-09-04T17:55:02.692Z
Learning: In backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py, the sanitize_provider_account_profile_fields_from_response method is called within generate_api_response_object, which serves as the central method that all endpoints use to construct provider response objects. This ensures that sensitive profile fields are sanitized at the source, eliminating the need for additional sanitization calls in downstream methods like those in utils.py.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py
📚 Learning: 2025-09-04T17:55:02.692Z
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#1058
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:785-786
Timestamp: 2025-09-04T17:55:02.692Z
Learning: In the CompactConnect backend, provider data sanitization follows a centralized architecture: the ProviderRecordUtility.generate_api_response_object() method serves as the single source for constructing provider response objects and includes sanitization of sensitive profile fields. Downstream functions like sanitize_provider_data_based_on_caller_scopes() in utils.py receive already-sanitized data, making additional sanitization calls redundant. This ensures sensitive fields are removed at the source rather than requiring multiple sanitization points throughout the codebase.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py
📚 Learning: 2025-09-04T17:55:02.692Z
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#1058
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:785-786
Timestamp: 2025-09-04T17:55:02.692Z
Learning: In CompactConnect's provider data architecture, sanitization of sensitive profile fields (emailVerificationExpiry, emailVerificationCode, pendingEmailAddress, recoveryToken, recoveryExpiry) occurs centrally in ProviderRecordUtility.generate_api_response_object(). All endpoints use this method either directly or through get_provider_information(), ensuring that downstream functions like sanitize_provider_data_based_on_caller_scopes() receive already-sanitized data. This eliminates the need for additional sanitization calls at individual return points throughout the codebase.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py
📚 Learning: 2025-09-04T17:55:02.692Z
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#1058
File: backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py:785-786
Timestamp: 2025-09-04T17:55:02.692Z
Learning: The CompactConnect provider data sanitization architecture follows this flow: endpoints call get_provider_information() → which calls provider_user_records.generate_api_response_object() → which includes sanitization of sensitive profile fields (emailVerificationExpiry, emailVerificationCode, pendingEmailAddress, recoveryToken, recoveryExpiry) → sanitized data then flows to sanitize_provider_data_based_on_caller_scopes() for additional scope-based filtering. This centralized approach ensures sensitive fields are removed at the source rather than requiring sanitization at multiple return points.
Applied to files:
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py
📚 Learning: 2025-08-22T21:20:35.260Z
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#1029
File: backend/compact-connect/docs/api-specification/latest-oas30.json:468-471
Timestamp: 2025-08-22T21:20:35.260Z
Learning: The file backend/compact-connect/docs/api-specification/latest-oas30.json is auto-generated by API Gateway and should not be modified inline. Any schema changes would need to be addressed at the source in the CDK/CloudFormation definitions.
Applied to files:
backend/compact-connect/docs/api-specification/latest-oas30.json
🧬 Code graph analysis (13)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/__init__.py (2)
submittingUser(101-102)submittingUser(105-106)
backend/compact-connect/lambdas/python/purchases/handlers/privileges.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/purchase/api.py (2)
PurchasePrivilegeOptionsResponseSchema(12-73)TransactionResponseSchema(76-88)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/attestation/__init__.py (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/attestation/api.py (1)
AttestationResponseSchema(9-30)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/attestation/record.py (1)
AttestationRecordSchema(15-42)
backend/compact-connect/lambdas/python/staff-users/handlers/me.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/user/api.py (1)
UserMergedResponseSchema(101-120)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/purchase/api.py (3)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/base_record.py (1)
ForgivingSchema(21-25)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/api.py (1)
CompactOptionsResponseSchema(26-35)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/jurisdiction/api.py (1)
JurisdictionOptionsResponseSchema(23-36)
backend/compact-connect/lambdas/python/provider-data-v1/handlers/provider_users.py (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/api.py (1)
ProviderReadPrivateResponseSchema(49-101)backend/compact-connect/lambdas/python/provider-data-v1/handlers/__init__.py (1)
get_provider_information(6-17)
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
UpdateCategory(287-299)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/attestation/api.py (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/base_record.py (1)
ForgivingSchema(21-25)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
dateOfUpdate(149-153)
backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_purchase.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/purchase/api.py (2)
PurchasePrivilegeOptionsResponseSchema(12-73)TransactionResponseSchema(76-88)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/api.py (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py (1)
SocialSecurityNumber(26-28)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/base_record.py (1)
ForgivingSchema(21-25)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/user/api.py (3)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/base_record.py (1)
ForgivingSchema(21-25)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/common.py (1)
StaffUserStatus(348-350)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/fields.py (1)
Compact(48-50)
backend/compact-connect/lambdas/python/compact-configuration/handlers/attestations.py (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/attestation/api.py (1)
AttestationResponseSchema(9-30)backend/compact-connect/lambdas/python/common/cc_common/data_model/compact_configuration_client.py (1)
get_attestation(24-49)
backend/compact-connect/lambdas/python/provider-data-v1/handlers/providers.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/api.py (1)
ProviderSSNResponseSchema(35-46)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: TestPython
🔇 Additional comments (15)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.py (1)
81-81: submittingUser requirement aligns with record schema
The record schema and data model guarantee this field is always set, so marking it required in the response schema poses no breaking change.backend/compact-connect/lambdas/python/provider-data-v1/handlers/providers.py (1)
182-185: LGTM! Schema validation properly applied.The schema validation step ensures only the
ssnfield is returned in the response, preventing accidental data leakage. This is correctly placed after all authorization and rate-limiting checks.backend/compact-connect/lambdas/python/staff-users/handlers/me.py (1)
49-52: LGTM! Schema validation correctly applied to merged user data.The schema validation ensures the merged user response only contains expected fields, preventing unintended data exposure. The placement at the end of the merge logic is appropriate.
backend/compact-connect/lambdas/python/compact-configuration/handlers/attestations.py (1)
45-47: LGTM! Schema validation appropriately applied.The schema validation step ensures the attestation response conforms to the expected structure, filtering out any unintended fields before returning to the client.
backend/compact-connect/lambdas/python/provider-data-v1/handlers/provider_users.py (1)
71-76: LGTM! Schema validation correctly filters provider data.The schema validation ensures provider-users only receive their authorized private data fields, preventing accidental exposure of additional sensitive information. The validation is properly placed after data retrieval and before the return statement.
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/provider/api.py (1)
35-46: LGTM! Well-defined schema for SSN responses.The
ProviderSSNResponseSchemais properly designed:
- Inherits from
ForgivingSchemato exclude unknown fields- Single
ssnfield with proper type validation- Required field with no null values allowed
- Clear documentation of serialization direction
This ensures only the SSN is returned with proper format validation.
backend/compact-connect/lambdas/python/purchases/handlers/privileges.py (2)
117-119: LGTM! Schema validation ensures purchase options structure integrity.The schema validation step validates both the overall response structure and individual items (compact and jurisdiction options), ensuring only properly formatted purchase options are returned to clients.
443-445: LGTM! Transaction response properly validated.The schema validation ensures the transaction response from the payment processor conforms to the expected structure before returning to the client, filtering out any unexpected fields.
backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_purchase.py (1)
1-258: LGTM! Comprehensive test coverage for purchase schemas.The test suite provides excellent coverage:
- Happy path validation for both schemas
- Invalid item type handling (strings instead of dicts)
- Unsupported type rejection
- Missing field validation
- Nested validation error propagation
- Edge cases (empty lists, minimal payloads)
- Clear, descriptive test names
This ensures the schemas behave correctly in production.
backend/compact-connect/docs/postman/postman-collection.json (1)
1-1095: LGTM! Postman collection updated to reflect API changes.The collection updates include:
- New UUIDs for collection items
- Updated sample payloads aligned with new schemas
- Authorization headers for authenticated endpoints
These changes maintain consistency with the API schema updates throughout this PR.
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/user/api.py (2)
7-7: LGTM!The imports support the new
UserMergedResponseSchemaclass and are correctly sourced.Also applies to: 9-9
101-120: LGTM!The schema correctly models merged user data across multiple compacts. Using the
Compact()field for permission keys is consistent with the field's purpose and cleaner than inline validation. The docstring clearly documents the serialization direction.backend/compact-connect/docs/api-specification/latest-oas30.json (3)
5-5: LGTM!The version bump reflects the schema changes introduced in this PR.
45-45: LGTM!The schema references correctly point to the new schema definitions introduced in the components section.
Also applies to: 57-57, 67-67, 147-147, 225-225, 237-237, 296-296
316-627: LGTM!The new schema definitions are well-structured with appropriate required fields, types, patterns, and enums. They correctly reflect the API response validation schemas added in this PR.
Also applies to: 628-687, 688-974, 975-998, 999-1010, 1011-1153, 1154-1179
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/purchase/api.py
Show resolved
Hide resolved
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/purchase/api.py(1 hunks)backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_purchase.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/purchase/api.py (3)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/base_record.py (1)
ForgivingSchema(21-25)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/compact/api.py (1)
CompactOptionsResponseSchema(26-35)backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/jurisdiction/api.py (1)
JurisdictionOptionsResponseSchema(23-36)
backend/compact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_purchase.py (1)
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/purchase/api.py (2)
PurchasePrivilegeOptionsResponseSchema(12-69)TransactionResponseSchema(72-84)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: TestPython
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/purchase/api.py
Outdated
Show resolved
Hide resolved
...ompact-connect/lambdas/python/common/tests/unit/test_data_model/test_schema/test_purchase.py
Show resolved
Hide resolved
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/compact-connect/tests/resources/snapshots/GET_ATTESTATION_BY_ID_RESPONSE_SCHEMA.json (1)
9-11: Approve rename; no legacyattestationTypefound.
attestationIdis used consistently across code and documentation.
Optional: if IDs follow UUID/ULID format, addformat: uuidor a regex pattern for stronger validation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/compact-connect/tests/resources/snapshots/GET_ATTESTATION_BY_ID_RESPONSE_SCHEMA.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: TestPython
landonshumway-ia
left a comment
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.
Looks great. Thanks for improving the safety of these endpoints
backend/compact-connect/lambdas/python/common/cc_common/data_model/provider_record_util.py
Show resolved
Hide resolved
backend/compact-connect/lambdas/python/provider-data-v1/handlers/provider_users.py
Show resolved
Hide resolved
|
@jlkravitz , ready for you! |
jlkravitz
left a comment
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.
one question, but otherwise looks good!
backend/compact-connect/lambdas/python/common/cc_common/data_model/schema/adverse_action/api.py
Show resolved
Hide resolved
jlkravitz
left a comment
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.
@isabeleliassen good to merge
Description List
Testing List
run compact-connect/bin/download_oas30.pyCloses #1057
Summary by CodeRabbit
New Features
Refactor
Documentation
Tests