-
Notifications
You must be signed in to change notification settings - Fork 1
jasper-214 test new users for PCSS guid and sync groups if found. #508
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
base: master
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR implements PCSS authorization integration for new users by checking for PCSS GUID and syncing groups during user creation. The implementation adds group alias functionality to map PCSS role names to internal groups, introduces authorization services for PCSS API communication, and enhances the authentication flow to automatically assign groups to new users based on their PCSS roles.
- Adds PCSS authorization client integration with caching and user role retrieval
- Implements GroupAlias entity and service for mapping PCSS role names to internal groups
- Enhances user authentication flow to automatically sync groups for new users with PCSS GUID
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/api/Services/GroupServiceTests.cs | Adds unit tests for the new GetGroupsByAliases functionality |
| tests/api/Services/AuthorizationServiceTests.cs | Adds comprehensive test coverage for the new AuthorizationService |
| pcss-client/Clients/AuthorizationServicesClient.cs | Generated PCSS authorization client for API communication |
| db/Seeders/GroupAliasesSeeder.cs | Database seeder for mapping group aliases to internal groups |
| db/Models/GroupAlias.cs | New entity model for storing group alias mappings |
| db/Contexts/JasperDbContext.cs | Adds GroupAlias entity to database context |
| db/Contants/CollectionNameConstants.cs | Adds collection name constant for group aliases |
| api/appsettings.json | Adds user caching configuration |
| api/Services/GroupService.cs | Implements GetGroupsByAliases method for alias-based group lookup |
| api/Services/AuthorizationService.cs | New service for PCSS authorization integration with caching |
| api/Models/AccessControlManagement/GroupDto.cs | Adds GroupAliasIds property to group DTO |
| api/Models/AccessControlManagement/GroupAliasDto.cs | New DTO for group alias representation |
| api/Infrastructure/ServiceCollectionExtensions.cs | Registers new services and PCSS authorization client |
| api/Infrastructure/Authentication/AuthenticationServiceCollectionExtension.cs | Enhanced authentication flow with PCSS integration and group synchronization |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -0,0 +1,99 @@ | |||
| using DocumentFormat.OpenXml.InkML; | |||
Copilot
AI
Sep 15, 2025
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.
This using statement appears to be unused and should be removed to keep the code clean.
| using DocumentFormat.OpenXml.InkML; |
| using System.Security.Claims; | ||
| using System.Text; | ||
| using System.Threading.Tasks; | ||
| using Hangfire.Mongo.Dto; |
Copilot
AI
Sep 15, 2025
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.
This using statement appears to be unused and should be removed to keep the code clean.
| using Hangfire.Mongo.Dto; |
| if (context.ProtocolMessage.RequestType == OpenIdConnectRequestType.Authentication && !context.Request.Path.StartsWithSegments("/api/auth/login")) | ||
| { | ||
| if (!context.Request.Path.StartsWithSegments("/api/auth/login")) | ||
| { | ||
| context.Response.StatusCode = (int)HttpStatusCode.Unauthorized; | ||
| context.HandleResponse(); | ||
| return Task.CompletedTask; | ||
| } | ||
| context.Response.StatusCode = (int)HttpStatusCode.Unauthorized; | ||
| context.HandleResponse(); | ||
| return Task.CompletedTask; |
Copilot
AI
Sep 15, 2025
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.
[nitpick] This change removes the readability of the original if-statement structure. The original nested if statements were clearer and easier to understand.
| UserItem matchingUser = null; | ||
| try | ||
| { | ||
| logger.LogInformation("Requesting user information from PCSS."); //TODO: only do this for ProvJud users. |
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.
Working on this - keeping this PR in draft for now.
|
|
||
| var groupAliases = new Dictionary<string, string> | ||
| { | ||
| [GroupAlias.JUDGE] = Group.JUDICIARY, |
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.
This is a best guess - I'm going to ask Longine to review when he gets back.
|



Pull Request for JIRA Ticket: ----put ticket number here----
Issue ticket number and link
Include the JIRA ticket # and link here
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
If applicable
Checklist:
Documentation References
Put any doc references here