-
Notifications
You must be signed in to change notification settings - Fork 61
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(portals-admin-ids-admin): Add LegalRepresentative delegation type #16069
Conversation
…account for LegalRepresentative as superuser only.
WalkthroughThe pull request introduces significant enhancements to the handling of delegation types within the authorization framework of the application. Key changes include the addition of the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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
|
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: 5
Outside diff range and nitpick comments (4)
libs/auth-api-lib/src/lib/resources/utils/filters.ts (1)
1-19
: Add unit tests for the new utility functionsTo ensure the correctness and reliability of the newly added
SUPER_USER_DELEGATION_TYPES
constant anddelegationTypeSuperUserFilter
function, consider adding unit tests. This will help verify their behavior and prevent future regressions.libs/auth-api-lib/src/lib/clients/admin/dto/admin-patch-client.dto.ts (2)
76-78
: Addtype
andexample
to@ApiPropertyOptional
foraddedDelegationTypes
For better API documentation and consistency, consider adding
type
,enum
,isArray
, andexample
to the@ApiPropertyOptional
decorator foraddedDelegationTypes
.Apply this diff:
@ApiPropertyOptional({ description: 'Only super users can update this value.', + type: [AuthDelegationType], + enum: AuthDelegationType, + isArray: true, + example: [AuthDelegationType.Custom], }) addedDelegationTypes?: AuthDelegationType[]
83-85
: Addtype
andexample
to@ApiPropertyOptional
forremovedDelegationTypes
Similarly, for
removedDelegationTypes
, consider addingtype
,enum
,isArray
, andexample
to the@ApiPropertyOptional
decorator for completeness.Apply this diff:
@ApiPropertyOptional({ description: 'Only super users can update this value.', + type: [AuthDelegationType], + enum: AuthDelegationType, + isArray: true, + example: [AuthDelegationType.Custom], }) removedDelegationTypes?: AuthDelegationType[]libs/auth-api-lib/src/index.ts (1)
102-102
: Consider exportingdelegationTypeSuperUserFilter
Currently, only
SUPER_USER_DELEGATION_TYPES
is exported from./lib/resources/utils/filters
. IfdelegationTypeSuperUserFilter
is intended for use outside of its module, consider exporting it inindex.ts
for better reusability.Apply this diff:
-export { SUPER_USER_DELEGATION_TYPES } from './lib/resources/utils/filters' +export { SUPER_USER_DELEGATION_TYPES, delegationTypeSuperUserFilter } from './lib/resources/utils/filters'
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (12)
- apps/services/auth/admin-api/src/app/v2/clients/test/me-clients.spec.ts (10 hunks)
- apps/services/auth/admin-api/src/app/v2/scopes/test/me-scopes.spec.ts (12 hunks)
- libs/auth-api-lib/src/index.ts (1 hunks)
- libs/auth-api-lib/src/lib/clients/admin/admin-clients.service.ts (3 hunks)
- libs/auth-api-lib/src/lib/clients/admin/dto/admin-create-client.dto.ts (2 hunks)
- libs/auth-api-lib/src/lib/clients/admin/dto/admin-patch-client.dto.ts (2 hunks)
- libs/auth-api-lib/src/lib/resources/admin/admin-scope.service.ts (3 hunks)
- libs/auth-api-lib/src/lib/resources/admin/dto/admin-create-scope.dto.ts (2 hunks)
- libs/auth-api-lib/src/lib/resources/admin/dto/admin-patch-scope.dto.ts (2 hunks)
- libs/auth-api-lib/src/lib/resources/utils/filters.ts (1 hunks)
- libs/auth-api-lib/src/lib/resources/utils/personalRepresentativeFilter.ts (0 hunks)
- libs/portals/admin/ids-admin/src/lib/messages.ts (3 hunks)
Files not reviewed due to no reviewable changes (1)
- libs/auth-api-lib/src/lib/resources/utils/personalRepresentativeFilter.ts
Additional context used
Path-based instructions (11)
libs/auth-api-lib/src/lib/resources/utils/filters.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."
libs/auth-api-lib/src/lib/resources/admin/dto/admin-create-scope.dto.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."
libs/auth-api-lib/src/lib/clients/admin/dto/admin-create-client.dto.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."
libs/auth-api-lib/src/lib/resources/admin/dto/admin-patch-scope.dto.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."
libs/auth-api-lib/src/lib/clients/admin/dto/admin-patch-client.dto.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."
libs/auth-api-lib/src/index.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."
libs/auth-api-lib/src/lib/resources/admin/admin-scope.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."
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/scopes/test/me-scopes.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."
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."
libs/portals/admin/ids-admin/src/lib/messages.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 not posted (19)
libs/auth-api-lib/src/lib/resources/admin/admin-scope.service.ts (3)
30-33
: Importing delegation utilities for super user handlingThe addition of
delegationTypeSuperUserFilter
andSUPER_USER_DELEGATION_TYPES
imports ensures that the service has access to the necessary utilities for managing super user delegation types.
171-173
: Filtering out super user delegation types for non-super adminsBy applying
delegationTypeSuperUserFilter
toinput.supportedDelegationTypes
, non-super admin users are prevented from assigning super user delegation types when creating scopes. This enforces proper access control.
415-416
: Validating user permissions when updating delegation typesThe logic correctly checks if the user is a super admin before allowing updates to super user delegation types. If a non-super admin attempts to update these types, the function returns
false
, preventing unauthorized changes.Also applies to: 419-420
libs/auth-api-lib/src/lib/clients/admin/admin-clients.service.ts (3)
44-47
: Importing delegation type filters for client managementIncluding
delegationTypeSuperUserFilter
andSUPER_USER_DELEGATION_TYPES
ensures that the client service can correctly handle delegation types related to super users.
185-187
: Filtering super user delegation types during client creationThe use of
delegationTypeSuperUserFilter
filters out super user delegation types fromclientDto.supportedDelegationTypes
when the user is not a super admin, maintaining proper access restrictions.
665-676
: Checking for unauthorized updates to super user delegation typesThe validation accurately identifies if a non-super admin is attempting to add or remove super user delegation types and blocks the action by returning
false
. This protects against unauthorized modifications.apps/services/auth/admin-api/src/app/v2/scopes/test/me-scopes.spec.ts (4)
15-15
: AddingSUPER_USER_DELEGATION_TYPES
for comprehensive testingImporting
SUPER_USER_DELEGATION_TYPES
allows the test suite to include scenarios involving super user delegation types, enhancing test coverage.
131-134
: IncludingAuthDelegationType.LegalRepresentative
in test dataAdding
AuthDelegationType.LegalRepresentative
to the delegation types ensures that this new type is properly tested within the scope of delegation type management.
382-383
: ExtendingPatchTestCase
to support delegation type modificationsIntroducing
addedDelegationTypes
andremovedDelegationTypes
properties allows test cases to simulate adding and removing delegation types, including super user types.
523-560
: Testing forbidden access for non-super admins modifying super user delegation typesThe added test cases correctly verify that non-super admin users receive a
403 Forbidden
response when attempting to add or remove super user delegation types, ensuring access control is enforced.apps/services/auth/admin-api/src/app/v2/clients/test/me-clients.spec.ts (5)
10-15
: Importing delegation-related utilities for client testsIncluding
SUPER_USER_DELEGATION_TYPES
and related utilities ensures that the client tests can validate behavior concerning super user delegation types.
56-59
: AddingAuthDelegationType.LegalRepresentative
to client test setupIncluding
LegalRepresentative
in the test data ensures that the new delegation type is considered in client-related tests, enhancing test coverage.
Line range hint
629-646
: Testing client creation with all delegation types for super usersThe test confirms that super admin users can create clients with all delegation types, including
LegalRepresentative
, ensuring that super users have the necessary permissions.
Line range hint
997-1055
: Verifying addition and removal of delegation types by super usersThe tests correctly validate that super users can add and remove delegation types, and that the client's delegation settings are updated accordingly, including super user delegation types.
1084-1142
: Ensuring non-super admins cannot modify super user delegation typesThe test cases appropriately assert that normal users receive a
403 Forbidden
response when attempting to add or remove super user delegation types, enforcing proper access control.libs/portals/admin/ids-admin/src/lib/messages.ts (4)
114-117
: Consistent Message ID NamingThe
id
fordisplayNameDescription
has been updated to include theap.ids-admin:
prefix, enhancing consistency across the codebase.
923-931
: Addition of Syslumenn Delegation Provider MessagesThe new messages
clientDelegationProviderSyslumennName
andclientDelegationProviderSyslumennDescription
are correctly added and provide clear descriptions for the District Commissioner delegation provider.
988-991
: Addition of Syslumenn Delegation Provider Messages for API ScopesThe messages
apiScopeDelegationProviderSyslumennName
are appropriately added, enhancing the messaging for API scope delegation providers.
992-999
: Consistent Naming for Legal Representative Delegation TypeThe additions of
apiScopeDelegationTypeLegalRepresentativeName
andapiScopeDelegationTypeLegalRepresentativeDescription
are correct and improve clarity for the legal representative delegation type.
Datadog ReportAll test runs ✅ 43 Total Test Services: 0 Failed, 42 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
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.
LGTM 🚀
#16069) * Add missing translation for legal representation delegation type in IDS Admin * Refactor delegation type filtering from client and scope creation to account for LegalRepresentative as superuser only. * Update patch super user delegation type check. Add tests. * Handle legal representative in scope patch. Add tests. * Hide it from the UI. * Fix duplicated message id --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* feat(ids-api): Use syslumenn api to verify delegation. (#16029) * Use syslumenn api to verify delegation. * Fix api error case. * Fix config. * chore: nx format:write update dirty files * Handle error in api. * Fix typo in name. * Refactor mock. * Update infra. * Update host in infra. * Add syslumenn infra to other auth apis. * Also return empty array if error. * Use post. * Fix tests. * Single delegation type. * Remove infra config from pr public. * Fix type. * Refactor error handling in check for scopes. * Openapi fix. * Refactor verification error handling. * Fix status code. * Decrease syslumenn api timeout to 3s. --------- Co-authored-by: andes-it <[email protected]> Co-authored-by: Valur Einarsson <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(auth-api): Filter delegation indexing by provider (#16141) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(portals-admin-ids-admin): Add LegalRepresentative delegation type (#16069) * Add missing translation for legal representation delegation type in IDS Admin * Refactor delegation type filtering from client and scope creation to account for LegalRepresentative as superuser only. * Update patch super user delegation type check. Add tests. * Handle legal representative in scope patch. Add tests. * Hide it from the UI. * Fix duplicated message id --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * chore: nx format:write update dirty files --------- Co-authored-by: andes-it <[email protected]> Co-authored-by: Valur Einarsson <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
Add LegalRepresentative delegation type to the IDS Admin UI for super user.
Why
To manage this setting in IDS admin.
Screenshots / Gifs
On a client
On a scope
Checklist:
Summary by CodeRabbit
Release Notes
New Features
LegalRepresentative
, enhancing delegation management.Bug Fixes
Documentation
Chores