feat(auth-admin): Allow non super users to publish super user fields #17290
Conversation
WalkthroughThe pull request introduces modifications to the authentication service's client management functionality. The changes primarily focus on test suite updates for the Changes
Sequence DiagramsequenceDiagram
participant User
participant MeClientsController
participant AdminClientsService
participant Database
User->>MeClientsController: Create/Update Client Request
MeClientsController->>AdminClientsService: Process Client
AdminClientsService->>AdminClientsService: Validate Client Parameters
AdminClientsService->>Database: Save/Update Client
Database-->>AdminClientsService: Confirmation
AdminClientsService-->>MeClientsController: Client Response
MeClientsController-->>User: Return Client Details
Possibly related PRs
Suggested labels
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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: 1
🧹 Nitpick comments (1)
apps/services/auth/admin-api/src/app/v2/clients/test/me-clients.spec.ts (1)
474-476: Duplicate Access Token Lifetime Handling
Similar fallback logic is duplicated but remains consistent with how it's handled above.Consider extracting this fallback into a helper function to standardize and reduce repetition.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/services/auth/admin-api/src/app/v2/clients/test/me-clients.spec.ts(2 hunks)libs/auth-api-lib/src/lib/clients/admin/admin-clients.service.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
libs/auth-api-lib/src/lib/clients/admin/admin-clients.service.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."
apps/services/auth/admin-api/src/app/v2/clients/test/me-clients.spec.ts (2)
Pattern apps/services/**/*: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (6)
apps/services/auth/admin-api/src/app/v2/clients/test/me-clients.spec.ts (6)
434-436: Use of Nullish Coalescing Operator for Access Token Lifetime Looks Good
This fallback pattern correctly assigns the property, defaulting to base attributes when custom values are absent.
442-444: Consistent Fallback for requirePkce
Using "??" follows the existing pattern, ensuring a graceful fallback to clientBaseAttributes.
450-450: Appropriate Fallback for customClaims
Defaulting to an empty array helps avoid potential null reference errors.
453-453: Fallback for allowedAcr
The default value "[defaultAcrValue]" ensures allowedAcr is never undefined.
477-479: Inline Fallback for allowOfflineAccess
Maintains code clarity and consistency. No issues observed.
480-481: Same Fallback Pattern for requirePkce
This mirrors the logic used in the earlier segment, ensuring uniform handling.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17290 +/- ##
=======================================
Coverage 35.69% 35.69%
=======================================
Files 6928 6928
Lines 148733 148730 -3
Branches 42498 42496 -2
=======================================
Hits 53087 53087
+ Misses 95646 95643 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 8 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (2) |
What
Remove restrictions where non super users could only publish to another environment with default values for super admin fields
Why
So publish functionality will result in identical client
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation