feat(ids-api): Use syslumenn api to verify delegation. #16029
Conversation
WalkthroughThe changes enhance delegation management within the Changes
Possibly related PRs
Suggested reviewers
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: 21
Outside diff range and nitpick comments (7)
apps/services/auth/ids-api/src/app/delegations/delegation-verification-result.dto.ts (1)
6-7: Enhance API documentation with property descriptionConsider adding a description to the
@ApiProperty()decorator for theverifiedproperty to improve the generated Swagger documentation and provide additional context.Apply this diff to include a description:
@IsBoolean() -@ApiProperty() +@ApiProperty({ description: 'Indicates whether the delegation has been successfully verified.' }) verified!: booleanapps/services/auth/ids-api/src/app/delegations/delegations.controller.ts (1)
117-137: Add unit and integration tests for the newverifyendpointTo ensure the new
verifyendpoint functions correctly and to prevent future regressions, please add unit and integration tests covering various scenarios, including success cases and handling of invalid inputs.Do you want me to help generate test cases or open a new GitHub issue to track this task?
apps/services/auth/ids-api/src/app/delegations/test/delegations-filters.spec.ts (2)
168-175: Ensure test case handles unexpected failures gracefullyIn the test verifying a valid legal representative, consider adding assertions to check that the response body contains all expected properties and handle potential unexpected failures to improve test robustness.
Add additional assertions:
173 expect(res.status).toEqual(200) 174 expect(res.body.verified).toEqual(true) + expect(res.body).toHaveProperty('verified') + expect(typeof res.body.verified).toBe('boolean')
177-187: Improve test description for clarityThe description of the second test could be more specific to indicate that it is testing the scenario with a non-existing legal representative. This enhances readability and understanding of the test purpose.
Update the test description:
177 - it(`GET ${path} returns non-verified response`, async () => { 177 + it(`GET ${path} returns non-verified response for non-existing legal representative`, async () => {libs/auth-api-lib/src/lib/delegations/delegation-scope.service.ts (2)
275-279: Include error details in the log messageIn the catch block of
findDistrictCommissionersRegistryScopesTo, the error is logged without including the error details. Including the error message can aid in debugging and diagnosing issues.Apply this diff to include the error details:
} catch (error) { logger.error( - `Failed checking if delegation exists at provider '${AuthDelegationProvider.DistrictCommissionersRegistry}'`, + `Failed checking if delegation exists at provider '${AuthDelegationProvider.DistrictCommissionersRegistry}': ${error.message}`, ) }
376-384: Add braces to conditional statement for clarityIn the
findAllScopesTomethod, the conditional statement checking forDistrictCommissionersRegistryprovider lacks braces around the block of code. This can lead to maintenance issues and reduce code readability.Apply this diff to add braces for better clarity:
if ( providers.includes(AuthDelegationProvider.DistrictCommissionersRegistry) ) - scopePromises.push( +{ + scopePromises.push( this.findDistrictCommissionersRegistryScopesTo( user.nationalId, fromNationalId, delegationTypes, ), ) +}libs/clients/syslumenn/src/clientConfig.json (1)
1608-1608: Inconsistent Indentation in the Parameters SectionThere's an indentation issue at line 1608, which may affect the JSON structure. Ensure consistent formatting for better readability and to prevent parsing errors.
Consider adjusting the indentation:
"name": "malsnumer", "in": "query", - "x-nullable": true + "x-nullable": true
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (13)
- apps/services/auth/delegation-api/src/app/app.module.ts (2 hunks)
- apps/services/auth/ids-api/src/app/app.module.ts (3 hunks)
- apps/services/auth/ids-api/src/app/delegations/delegation-verification-result.dto.ts (1 hunks)
- apps/services/auth/ids-api/src/app/delegations/delegations.controller.ts (3 hunks)
- apps/services/auth/ids-api/src/app/delegations/test/delegations-filters.spec.ts (2 hunks)
- apps/services/auth/ids-api/src/app/delegations/test/delegations-scopes.spec.ts (3 hunks)
- apps/services/auth/ids-api/test/setup.ts (4 hunks)
- apps/services/auth/public-api/src/app/app.module.ts (2 hunks)
- libs/auth-api-lib/src/lib/delegations/delegation-scope.service.ts (5 hunks)
- libs/auth-api-lib/src/lib/delegations/delegations-incoming.service.ts (3 hunks)
- libs/auth-api-lib/src/lib/delegations/delegations.module.ts (2 hunks)
- libs/clients/syslumenn/src/clientConfig.json (135 hunks)
- libs/clients/syslumenn/src/lib/syslumennClient.service.ts (2 hunks)
Additional context used
Path-based instructions (13)
apps/services/auth/ids-api/src/app/delegations/delegation-verification-result.dto.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/public-api/src/app/app.module.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/delegation-api/src/app/app.module.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/ids-api/src/app/app.module.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/ids-api/src/app/delegations/delegations.controller.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/auth-api-lib/src/lib/delegations/delegations.module.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/ids-api/test/setup.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/ids-api/src/app/delegations/test/delegations-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/ids-api/src/app/delegations/test/delegations-filters.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/auth-api-lib/src/lib/delegations/delegations-incoming.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/delegations/delegation-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/clients/syslumenn/src/lib/syslumennClient.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/clients/syslumenn/src/clientConfig.json (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 (16)
apps/services/auth/ids-api/src/app/delegations/delegation-verification-result.dto.ts (2)
1-2: Imports are appropriate and necessaryThe imported modules
ApiPropertyandIsBooleanare correctly included for validation and API documentation purposes.
4-8: DTO is well-defined and adheres to NestJS best practicesThe
DelegationVerificationResultclass is properly defined with the necessary validation and API documentation decorators, aligning with NestJS conventions for Data Transfer Objects.apps/services/auth/public-api/src/app/app.module.ts (1)
48-48: Ensure correct configuration loading forSyslumennClientConfig
SyslumennClientConfighas been added to theloadarray inConfigModule.forRoot. Verify that the configuration values are correctly set and that any services depending onSyslumennClientConfigare properly utilizing it.Run the following script to confirm usage of
SyslumennClientConfig:apps/services/auth/delegation-api/src/app/app.module.ts (1)
13-13: Integration ofSyslumennClientConfigis appropriateThe addition of
SyslumennClientConfigto the imports and the configuration load array correctly integrates the Syslumenn client configuration into the application module. This change aligns with the PR objective to verify delegations using the Syslumenn API.Also applies to: 54-54
apps/services/auth/ids-api/src/app/app.module.ts (3)
14-14: Addition of SyslumennClientConfig import is appropriateThe import of
SyslumennClientConfigfrom@island.is/clients/syslumenncorrectly includes the necessary configuration for the Syslumenn client integration.
32-32: Imported PasskeysModule correctlyThe import statement for
PasskeysModuleensures that the passkey functionalities are properly included in the module.
72-72: Included SyslumennClientConfig in ConfigModule's load arrayAdding
SyslumennClientConfigto theloadarray ofConfigModule.forRootensures that the configuration is loaded and available for dependency injection throughout the application.libs/auth-api-lib/src/lib/delegations/delegations.module.ts (1)
5-7: Integration of new modules and services aligns with PR objectives.The additions of
SyslumennClientModuleand the related delegation services and models enhance the module's functionality for verifying delegations via the syslumenn API, as intended. The imports and module configuration appear correct.Also applies to: 13-14, 18-37, 63-63
apps/services/auth/ids-api/test/setup.ts (1)
25-25: Appropriate use ofcreateNationalIdfor test dataImporting
createNationalIdis suitable for generating test national IDs in your test scenarios.apps/services/auth/ids-api/src/app/delegations/test/delegations-filters.spec.ts (1)
135-188: Test cases for delegation verification are well-implementedThe new test suite effectively verifies the delegation verification functionality for both valid and invalid legal representatives. The tests are correctly structured and follow best practices for integration testing in NestJS.
libs/auth-api-lib/src/lib/delegations/delegations-incoming.service.ts (1)
71-71: Integration ofsyslumennServicelooks goodThe
syslumennServicehas been properly injected, enabling communication with the Syslumenn API for delegation verification.libs/auth-api-lib/src/lib/delegations/delegation-scope.service.ts (1)
Line range hint
9-43: Verify that new dependencies are properly registered and configuredThe
SyslumennServiceandDelegationsIndexServicehave been added as dependencies. Ensure that they are properly provided and configured in the module, and that any necessary environment variables or configurations are updated.Run the following script to check if the services are properly registered:
Verification successful
Verification successful: Dependencies properly configured
The SyslumennClientModule and DelegationScopeService (which includes DelegationsIndexService) are correctly imported, provided, and configured in the auth-api-lib's DelegationsModule. This ensures that these services are available for dependency injection throughout the module and in any modules that import DelegationsModule.
- SyslumennClientModule is imported and included in the module's imports array.
- DelegationScopeService is provided and exported by the DelegationsModule.
The module structure follows NestJS best practices, indicating a well-organized approach to handling delegations and related services.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that SyslumennService and DelegationsIndexService are provided in the module. # Check for SyslumennService import and provider rg --type ts --glob '**/delegations.module.ts' -A 10 'SyslumennService' # Check for DelegationsIndexService import and provider rg --type ts --glob '**/delegations.module.ts' -A 10 'DelegationsIndexService'Length of output: 2738
Script:
#!/bin/bash # Description: Search for SyslumennService in other module files and check for a separate Syslumenn module # Search for SyslumennService in all module files within auth-api-lib rg --type ts --glob '**/auth-api-lib/**/*.module.ts' -A 10 'SyslumennService' # Check if there's a separate Syslumenn module fd -e ts -p 'syslumenn.*module' libs/auth-api-lib # If a Syslumenn module is found, check if it's imported in the DelegationsModule rg --type ts --glob '**/delegations.module.ts' 'import.*Syslumenn.*Module'Length of output: 335
Script:
#!/bin/bash # Description: Verify SyslumennClientModule configuration and exports # Check if SyslumennClientModule is in the imports array of DelegationsModule rg --type ts --glob '**/delegations.module.ts' -A 20 'imports.*SyslumennClientModule' # Check the exports of SyslumennClientModule rg --type ts --glob '**/syslumenn/syslumenn-client.module.ts' -A 10 'exports'Length of output: 314
Script:
#!/bin/bash # Description: Verify project structure and locate relevant files # List directory structure find . -type d -maxdepth 3 # Search for delegations.module.ts find . -name "delegations.module.ts" # Search for files containing SyslumennClientModule grep -r "SyslumennClientModule" . # If delegations.module.ts is found, check its contents delegations_module=$(find . -name "delegations.module.ts" | head -n 1) if [ ! -z "$delegations_module" ]; then echo "Contents of delegations.module.ts:" cat "$delegations_module" fiLength of output: 19009
Script:
#!/bin/bash # Description: Verify SyslumennClientModule import and DelegationScopeService provision in auth-api-lib # Check the contents of auth-api-lib's delegations.module.ts cat ./libs/auth-api-lib/src/lib/delegations/delegations.module.ts # Search for DelegationScopeService provision grep -n "DelegationScopeService" ./libs/auth-api-lib/src/lib/delegations/delegations.module.tsLength of output: 4185
libs/clients/syslumenn/src/lib/syslumennClient.service.ts (1)
694-695: Verify date comparison logicEnsure that the date comparison in the delegation validity check handles
gildirTilcorrectly, especially ifgildirTilmight be a string or in a different time zone. Confirm thatdelegation.gildirTilandstartOfDay(new Date())are compatible for comparison.libs/clients/syslumenn/src/clientConfig.json (3)
530-534: Potential Security Concern: Avoid Passing Sensitive Data in Query ParametersPassing sensitive information like
"kennitala"in query parameters may expose it in server logs or browser history, leading to potential PII leakage. Consider using a POST request with the"kennitala"included in the request body or ensure that proper encryption and security measures are in place to protect this data.[security]
3777-3788: Missing Required Fields inLogradamadurSvarDefinitionThe
LogradamadurSvarschema does not specify required fields. IfkennitalaandgildirTilare mandatory, they should be included in the"required"array to enforce validation.Please confirm whether these fields are required:
"LogradamadurSvar": { "type": "object", + "required": [ + "kennitala", + "gildirTil" + ], "properties": { "kennitala": { "type": "string" }, "gildirTil": { "type": "string", "format": "date-time" } } },
700-702: Typographical Error in Endpoint PathThe endpoint path
"/api/Starfsrettindi/api/Starfsrettindi/{audkenni}"appears to have a duplicated segment"api/Starfsrettindi". It should likely be"/api/Starfsrettindi/{audkenni}". Please verify and correct the path to prevent routing issues.Apply this diff to fix the endpoint path:
-"/api/Starfsrettindi/api/Starfsrettindi/{audkenni}": { +"/api/Starfsrettindi/{audkenni}": {Likely invalid or redundant comment.
apps/services/auth/ids-api/src/app/delegations/delegations.controller.ts
Outdated
Show resolved
Hide resolved
libs/auth-api-lib/src/lib/delegations/delegation-scope.service.ts
Outdated
Show resolved
Hide resolved
libs/auth-api-lib/src/lib/delegations/delegation-scope.service.ts
Outdated
Show resolved
Hide resolved
Datadog ReportAll test runs ✅ 29 Total Test Services: 0 Failed, 29 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (10)
|
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (3)
libs/clients/syslumenn/src/clientConfig.json (3)
941-957: Enhance Response Descriptions for the PUT Method of/v1/Heimagistingar/{audkenni}The response descriptions for the PUT method are currently empty, which may lead to confusion for API consumers. Providing clear descriptions improves the API documentation and developer experience.
Apply this diff to add meaningful descriptions:
"responses": { "200": { - "description": "" + "description": "Success" }, "401": { "x-nullable": false, - "description": "" + "description": "Unauthorized" "schema": { "$ref": "#/definitions/ProblemDetails" } }, "default": { "x-nullable": false, - "description": "" + "description": "Error" "schema": { "$ref": "#/definitions/ProblemDetails" } } },
3877-3879: Specify Format for Date PropertyutgefidinHeimagistingDetailThe
utgefidproperty likely represents a date. Specifying theformatas"date-time"ensures correct data parsing and validation.Apply this diff to include the format:
"utgefid": { "type": "string", + "format": "date-time" }
Line range hint
1498-1510: Avoid Using a Generic Root-Level Path Parameter/{audkenni}Defining the endpoint
/{audkenni}at the root level can lead to routing conflicts and ambiguity in the API. It's best practice to include a specific path segment to prevent collisions with other endpoints.Apply this diff to modify the endpoint path:
- "/{audkenni}": { + "/api/Erfdafjarskattur/{audkenni}": {
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- libs/clients/syslumenn/src/clientConfig.json (7 hunks)
Additional context used
Path-based instructions (1)
libs/clients/syslumenn/src/clientConfig.json (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."
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (7)
- apps/services/auth/ids-api/infra/ids-api.ts (2 hunks)
- apps/services/auth/ids-api/src/app/delegations/test/delegations-scopes.spec.ts (3 hunks)
- apps/services/auth/ids-api/test/setup.ts (5 hunks)
- charts/identity-server/values.dev.yaml (2 hunks)
- charts/identity-server/values.prod.yaml (2 hunks)
- charts/identity-server/values.staging.yaml (2 hunks)
- libs/auth-api-lib/src/lib/delegations/delegation-scope.service.ts (5 hunks)
Files skipped from review as they are similar to previous changes (3)
- apps/services/auth/ids-api/src/app/delegations/test/delegations-scopes.spec.ts
- apps/services/auth/ids-api/test/setup.ts
- libs/auth-api-lib/src/lib/delegations/delegation-scope.service.ts
Additional context used
Path-based instructions (1)
apps/services/auth/ids-api/infra/ids-api.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 not posted (8)
apps/services/auth/ids-api/infra/ids-api.ts (2)
87-87: Verify the appropriateness ofSYSLUMENN_TIMEOUTvalueThe timeout is set to
'10000'milliseconds (10 seconds). Please confirm that this duration is suitable for the SYSLUMENN API interactions and meets the application's performance requirements.
97-98: SYSLUMENN credentials added securelyThe
SYSLUMENN_USERNAMEandSYSLUMENN_PASSWORDhave been appropriately added to the secrets section.charts/identity-server/values.prod.yaml (2)
404-405: EnsureSYSLUMENN_HOSTis handled appropriatelyThe
SYSLUMENN_HOSTandSYSLUMENN_TIMEOUTvariables have been added. Confirm thatSYSLUMENN_HOSTdoes not contain sensitive information and is appropriate to include in the configuration file.
500-501: SYSLUMENN credentials added securelyThe
SYSLUMENN_USERNAMEandSYSLUMENN_PASSWORDhave been correctly added as secrets.charts/identity-server/values.staging.yaml (2)
407-408: Validate inclusion ofSYSLUMENN_HOSTThe addition of
SYSLUMENN_HOSTandSYSLUMENN_TIMEOUTis noted. Please ensure thatSYSLUMENN_HOSTis appropriate for inclusion and does not expose sensitive data.
503-504: SYSLUMENN credentials securely addedThe
SYSLUMENN_USERNAMEandSYSLUMENN_PASSWORDhave been properly added to the secrets.charts/identity-server/values.dev.yaml (2)
407-408: Check handling ofSYSLUMENN_HOSTThe variables
SYSLUMENN_HOSTandSYSLUMENN_TIMEOUThave been introduced. Verify thatSYSLUMENN_HOSTis safe to include in this configuration and does not contain sensitive information.
503-504: SYSLUMENN credentials securely addedThe
SYSLUMENN_USERNAMEandSYSLUMENN_PASSWORDhave been correctly added to the secrets.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (8)
- apps/services/auth/admin-api/infra/auth-admin-api.ts (2 hunks)
- apps/services/auth/delegation-api/infra/delegation-api.ts (2 hunks)
- apps/services/auth/personal-representative-public/infra/personal-representative-public.ts (1 hunks)
- apps/services/auth/personal-representative/infra/personal-representative.ts (1 hunks)
- apps/services/auth/public-api/infra/auth-public-api.ts (2 hunks)
- charts/identity-server/values.dev.yaml (12 hunks)
- charts/identity-server/values.prod.yaml (12 hunks)
- charts/identity-server/values.staging.yaml (12 hunks)
Files skipped from review as they are similar to previous changes (3)
- charts/identity-server/values.dev.yaml
- charts/identity-server/values.prod.yaml
- charts/identity-server/values.staging.yaml
Additional context used
Path-based instructions (5)
apps/services/auth/personal-representative-public/infra/personal-representative-public.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/personal-representative/infra/personal-representative.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/delegation-api/infra/delegation-api.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/infra/auth-admin-api.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/public-api/infra/auth-public-api.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 not posted (16)
apps/services/auth/personal-representative-public/infra/personal-representative-public.ts (2)
17-23: SYSLUMENN environment variables added correctlyThe
SYSLUMENN_HOSTandSYSLUMENN_TIMEOUTconfigurations are properly defined for all environments.
24-26: SYSLUMENN secrets configured correctlyThe secrets
SYSLUMENN_USERNAMEandSYSLUMENN_PASSWORDare correctly set up in the secrets configuration.apps/services/auth/personal-representative/infra/personal-representative.ts (3)
45-56: Consider centralizing SYSLUMENN configuration to reduce duplication
45-50: SYSLUMENN environment variables added correctlyThe
SYSLUMENN_HOSTandSYSLUMENN_TIMEOUTconfigurations are properly defined for all environments.
55-56: SYSLUMENN secrets configured correctlyThe secrets
SYSLUMENN_USERNAMEandSYSLUMENN_PASSWORDare correctly set up in the secrets configuration.apps/services/auth/delegation-api/infra/delegation-api.ts (4)
3-3: Import 'ref' added appropriatelyThe
refimport is correctly added to support dynamic references in environment variables.
57-70: Consider centralizing SYSLUMENN configuration to reduce duplication
57-62: SYSLUMENN environment variables added correctlyThe
SYSLUMENN_HOSTandSYSLUMENN_TIMEOUTconfigurations are properly defined for all environments.
69-70: SYSLUMENN secrets configured correctlyThe secrets
SYSLUMENN_USERNAMEandSYSLUMENN_PASSWORDare correctly set up in the secrets configuration.apps/services/auth/admin-api/infra/auth-admin-api.ts (3)
Line range hint
62-77: Consider centralizing SYSLUMENN configuration to reduce duplication
62-67: SYSLUMENN environment variables added correctlyThe
SYSLUMENN_HOSTandSYSLUMENN_TIMEOUTconfigurations are properly defined for all environments.
76-77: SYSLUMENN secrets configured correctlyThe secrets
SYSLUMENN_USERNAMEandSYSLUMENN_PASSWORDare correctly set up in the secrets configuration.apps/services/auth/public-api/infra/auth-public-api.ts (4)
1-1: Imports organized for better readabilityThe import statements have been appropriately consolidated for clarity.
66-79: Consider centralizing SYSLUMENN configuration to reduce duplication
66-71: SYSLUMENN environment variables added correctlyThe
SYSLUMENN_HOSTandSYSLUMENN_TIMEOUTconfigurations are properly defined for all environments.
78-79: SYSLUMENN secrets configured correctlyThe secrets
SYSLUMENN_USERNAMEandSYSLUMENN_PASSWORDare correctly set up in the secrets configuration.
apps/services/auth/personal-representative-public/infra/personal-representative-public.ts
Outdated
Show resolved
Hide resolved
apps/services/auth/ids-api/src/app/delegations/delegations.controller.ts
Show resolved
Hide resolved
apps/services/auth/personal-representative-public/infra/personal-representative-public.ts
Outdated
Show resolved
Hide resolved
libs/auth-api-lib/src/lib/delegations/delegation-scope.service.ts
Outdated
Show resolved
Hide resolved
libs/auth-api-lib/src/lib/delegations/delegation-scope.service.ts
Outdated
Show resolved
Hide resolved
apps/services/auth/ids-api/src/app/delegations/delegations.controller.ts
Outdated
Show resolved
Hide resolved
apps/services/auth/ids-api/src/app/delegations/delegations.controller.ts
Outdated
Show resolved
Hide resolved
apps/services/auth/ids-api/src/app/delegations/delegations.controller.ts
Outdated
Show resolved
Hide resolved
libs/auth-api-lib/src/lib/delegations/delegations-incoming.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
apps/services/auth/ids-api/src/app/delegations/delegations.controller.ts (1)
118-135: The new endpoint looks good and follows best practices!A few additional suggestions:
- Consider adding validation decorators to the
DelegationVerificationDTO properties to ensure data integrity.- Add unit tests to verify the endpoint behavior and error handling.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- apps/services/auth/ids-api/src/app/delegations/delegation-verification.dto.ts (1 hunks)
- apps/services/auth/ids-api/src/app/delegations/delegations.controller.ts (3 hunks)
Additional context used
Path-based instructions (2)
apps/services/auth/ids-api/src/app/delegations/delegation-verification.dto.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/ids-api/src/app/delegations/delegations.controller.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 not posted (5)
apps/services/auth/ids-api/src/app/delegations/delegation-verification.dto.ts (2)
6-8: LGTM!The
fromNationalIdproperty is correctly defined as a string and properly validated using the@IsString()decorator. The@ApiProperty()decorator ensures that the property is documented in the API schema.
10-13: LGTM!The
delegationTypesproperty is correctly defined as an array of strings and properly validated using the@IsArray()decorator. The@Type(() => String)decorator ensures that the elements of the array are transformed to strings. The@ApiProperty()decorator documents the property as an array of strings in the API schema.apps/services/auth/ids-api/src/app/delegations/delegations.controller.ts (3)
32-34: LGTM!The new DTO imports are necessary for the
verifyendpoint and follow the correct conventions.
7-7: LGTM!The
Postdecorator import is necessary and follows the existing import style.
2-2: LGTM!The
Bodydecorator import is necessary and follows the existing import style.
valurefugl
added
the
deprecated:automerge
|
This PR currently has a merge conflict. Please resolve this and then re-add the |
kodiakhq
bot
removed
the
deprecated:automerge
valurefugl
added
the
deprecated:automerge
apps/services/auth/personal-representative/infra/personal-representative.ts
Outdated
Show resolved
Hide resolved
* 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(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>
https://www.notion.so/Add-legal-representative-hard-check-when-user-chooses-delegation-848df5969a6d4c898f5ff648d386e71a?pvs=4
What
Use syslumenn api to verify delegation.
Why
To be sure that legal representative delegation still exists before authenticating.
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Tests