fix(api): add correct scope #16874
fix(api): add correct scope #16874
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
libs/api/domains/health-directorate/src/lib/health-directorate.resolver.ts (2)
Line range hint
83-92: Consider documenting error casesThe scope addition looks good, but since the method can return null, consider adding JSDoc comments to document when null is returned versus when an error is thrown.
+ /** + * Retrieves vaccinations for the current user + * @param locale - The locale for the response + * @param user - The authenticated user + * @returns Vaccinations data or null if no records found + * @throws Unauthorized if user lacks required scope + */ @Audit() @Scopes(ApiScope.healthVaccinations)
Line range hint
1-92: Well-structured security implementation with defense in depthThe resolver implements multiple security layers effectively:
- IdsUserGuard for authentication
- ScopesGuard for authorization
- FeatureFlagGuard for feature control
- Audit decorators for logging
This follows security best practices by implementing defense in depth.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
libs/api/domains/health-directorate/src/lib/health-directorate.resolver.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/api/domains/health-directorate/src/lib/health-directorate.resolver.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (2)
libs/api/domains/health-directorate/src/lib/health-directorate.resolver.ts (2)
Line range hint 41-51: LGTM! Appropriate scope added for organ donation data access
The addition of @Scopes(ApiScope.healthOrganDonation) properly secures the organ donation status endpoint while maintaining existing feature flag controls.
Line range hint 67-77: Verify if write operations need a separate scope
Currently using the same scope (healthOrganDonation) for both reading and updating donor status. While this might be intentional, consider if write operations should have a more specific scope for finer-grained access control.
✅ Verification successful
Based on the search results, I can now provide a definitive response about the scope granularity for health-related mutations in the codebase:
Using the same scope for read/write operations follows established pattern
Looking at other health-related mutations in the codebase:
healthDentistsscope is used for both registering dentists and querying dentist informationhealthMedicinesscope is used for both drug calculations and certificate querieshealthOrganDonationscope follows this same pattern for both reading and updating donor status
This consistent pattern across the health domain indicates that using the same scope for both read and write operations is an intentional design choice in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other mutation endpoints in the codebase to verify scope granularity patterns
ast-grep --pattern 'class $_ {
$$$
@Mutation($_)
@Scopes($_)
$$$
}'
Length of output: 77
Script:
#!/bin/bash
# Let's try a different approach to find mutations with scopes
rg "@Mutation.*\n.*@Scopes" -A 5
# Also search for any usage of healthOrganDonation scope
rg "healthOrganDonation" -B 2 -A 2
Length of output: 3401
Script:
#!/bin/bash
# Let's try one more search to find mutations with multiline support
rg -U "@Mutation[\s\S]*?@Scopes" -A 10
# Also search for any other health-related mutations to see the scope pattern
rg -U "@Mutation[\s\S]*?health" -A 10
Length of output: 69394
Datadog ReportAll test runs ✅ 99 Total Test Services: 0 Failed, 97 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (3) |
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* fix(my-pages): health vaccination tags & locale (#16567) * fix: tags * feat: add locale to service WIP * feat: add locale from service * feat: add locale to query call * fix(api): add ff to resolver (#16865) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(api): add correct scope (#16874) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Api - Health Directorate
What
Adding correct scope
Checklist:
Summary by CodeRabbit