[OTE-626]: Add support for whitelisted addresses #1966
Conversation
WalkthroughThe changes introduce a comprehensive enhancement to the compliance functionality by incorporating a new address whitelisting feature. A new function, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ComplianceController
participant ComplianceCheck
User->>ComplianceController: Request with address
ComplianceController->>ComplianceCheck: Check if address is whitelisted
alt Address is whitelisted
ComplianceCheck-->>ComplianceController: Return compliant status
ComplianceController-->>User: Respond with compliance status
else Address is not whitelisted
ComplianceCheck-->>ComplianceController: Continue with checks
ComplianceController-->>User: Respond with compliance status
end
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- indexer/packages/compliance/tests/geoblocking/restrict-countries.test.ts (2 hunks)
- indexer/packages/compliance/src/config.ts (1 hunks)
- indexer/packages/compliance/src/geoblocking/restrict-countries.ts (1 hunks)
- indexer/services/comlink/tests/controllers/api/v4/compliance-v2-controller.test.ts (7 hunks)
- indexer/services/comlink/tests/lib/compliance-and-geo-check.test.ts (5 hunks)
- indexer/services/comlink/src/controllers/api/v4/compliance-v2-controller.ts (3 hunks)
- indexer/services/comlink/src/lib/compliance-and-geo-check.ts (2 hunks)
Additional comments not posted (11)
indexer/packages/compliance/src/config.ts (1)
25-28: LGTM!The addition of the
WHITELISTED_ADDRESSESconfiguration parameter is straightforward and well-integrated.indexer/packages/compliance/__tests__/geoblocking/restrict-countries.test.ts (1)
52-64: LGTM!The tests for the
isWhitelistedAddressfunction are comprehensive and ensure proper functionality.indexer/services/comlink/src/lib/compliance-and-geo-check.ts (1)
44-46: LGTM! Verify the reliability ofisWhitelistedAddress.The code changes are approved. The new check optimizes the control flow by bypassing compliance checks for whitelisted addresses.
However, ensure that the
isWhitelistedAddressfunction is reliable and does not introduce security vulnerabilities.indexer/services/comlink/__tests__/lib/compliance-and-geo-check.test.ts (1)
241-256: LGTM! The test case is comprehensive.The new test case correctly verifies that if an address is whitelisted, the application does not return a 403 status even if the request comes from a restricted country.
indexer/services/comlink/src/controllers/api/v4/compliance-v2-controller.ts (2)
148-152: LGTM! Verify the reliability ofisWhitelistedAddress.The code changes are approved. The new check optimizes the control flow by returning a compliant status for whitelisted addresses.
However, ensure that the
isWhitelistedAddressfunction is reliable and does not introduce security vulnerabilities.Verification successful
The
isWhitelistedAddressfunction is reliable.The function
isWhitelistedAddressis implemented inindexer/packages/compliance/src/geoblocking/restrict-countries.tsand checks if an address is in a comma-separated list from the configuration. TheWHITELISTED_ADDRESSESconfiguration is securely managed and tested, ensuring the function's reliability.
indexer/packages/compliance/src/geoblocking/restrict-countries.tsindexer/packages/compliance/src/config.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `isWhitelistedAddress` function. # Test: Search for the function implementation. Expect: The function should be implemented correctly. rg --type python -A 5 $'def isWhitelistedAddress'Length of output: 81
Script:
#!/bin/bash # Description: Verify the implementation of `isWhitelistedAddress` function. # Test: Search for the function implementation across all file types. rg 'function isWhitelistedAddress' -A 10Length of output: 375
Script:
#!/bin/bash # Description: Verify the source and handling of `config.WHITELISTED_ADDRESSES`. # Test: Search for the definition and handling of `config.WHITELISTED_ADDRESSES`. rg 'WHITELISTED_ADDRESSES' -A 5Length of output: 2462
260-265: LGTM! Verify the reliability ofisWhitelistedAddress.The code changes are approved. The new check optimizes the control flow by returning a compliant status for whitelisted addresses.
However, ensure that the
isWhitelistedAddressfunction is reliable and does not introduce security vulnerabilities.indexer/services/comlink/__tests__/controllers/api/v4/compliance-v2-controller.test.ts (5)
22-22: Import statement looks good.The import of
isWhitelistedAddressis correct and aligns with the new functionality being tested.
63-67: Spy instance setup forisWhitelistedAddresslooks good.The
isWhitelistedAddressSpyis correctly set up and mocked for use in the GET request tests.
164-187: New test case for whitelisted address in GET request looks good.The test case correctly verifies that a restricted address with an existing
CLOSE_ONLYcompliance status returnsCOMPLIANTwhen whitelisted.
Line range hint
330-344: Spy instance setup forisWhitelistedAddressin POST request tests looks good.The
isWhitelistedAddressSpyis correctly set up and mocked for use in the POST request tests.
426-448: New test case for whitelisted address in POST request looks good.The test case correctly verifies that a restricted address from a restricted country returns
COMPLIANTwhen whitelisted.
| export function isWhitelistedAddress(address: string): boolean { | ||
| return config.WHITELISTED_ADDRESSES.split(',').includes(address); | ||
| } |
There was a problem hiding this comment.
Optimize the whitelisted address check.
Consider caching the split whitelist addresses to avoid splitting the string each time the function is called.
export function isWhitelistedAddress(address: string): boolean {
- return config.WHITELISTED_ADDRESSES.split(',').includes(address);
+ const whitelistedAddresses = config.WHITELISTED_ADDRESSES.split(',');
+ return whitelistedAddresses.includes(address);
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function isWhitelistedAddress(address: string): boolean { | |
| return config.WHITELISTED_ADDRESSES.split(',').includes(address); | |
| } | |
| export function isWhitelistedAddress(address: string): boolean { | |
| const whitelistedAddresses = config.WHITELISTED_ADDRESSES.split(','); | |
| return whitelistedAddresses.includes(address); | |
| } |
|
@Mergifyio backport release/indexer/v5.x |
✅ Backports have been created
|
(cherry picked from commit f162a11)
) Co-authored-by: Christopher-Li <[email protected]>
Changelist
[Describe or list the changes made in this PR]
Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
New Features
Bug Fixes
Tests