PSP-11373 Correct PIMS file/project access#5285
PSP-11373 Correct PIMS file/project access#5285eddherrera wants to merge 4 commits intobcgov:devfrom
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| /// <returns></returns> | ||
| public static short[] GetShortArrayValue(this IDictionary<string, Microsoft.Extensions.Primitives.StringValues> dict, string key, string separator = ",") | ||
| { | ||
| return dict.TryGetValue(key, out Microsoft.Extensions.Primitives.StringValues value) ? value.ToString().Split(separator).Select(v => { return short.TryParse(v, out short iv) ? (short?)iv : null; }).Where(v => v != null).Select(v => (short)v).ToArray() : Array.Empty<short>(); |
There was a problem hiding this comment.
please address this codeql warning
|
See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/5285 |
1 similar comment
|
See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/5285 |
|
|
See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/5285 |
1 similar comment
|
See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/5285 |
asanchezr
left a comment
asanchezr
left a comment
There was a problem hiding this comment.
Please see comments. There are a few places where region restriction is still in place (exports mainly)
| var userRegions = pimsUser.PimsRegionUsers.Select(r => r.RegionCode).ToHashSet(); | ||
| long? contractorPersonId = pimsUser.IsContractor ? pimsUser.PersonId : null; | ||
|
|
||
| var acqFiles = _acqFileRepository.GetAcquisitionFileExportDeep(filter, userRegions, contractorPersonId); |
There was a problem hiding this comment.
This needs to be updated as well. Exports are included in the story ACs:
- Make sure this behaviour is applied consistently on dependent excel reports.
| var userRegions = pimsUser.PimsRegionUsers.Select(r => r.RegionCode).ToHashSet(); | ||
| long? contractorPersonId = pimsUser.IsContractor ? pimsUser.PersonId : null; | ||
|
|
||
| var teamMembers = _acqFileRepository.GetTeamMembers(userRegions, contractorPersonId); |
There was a problem hiding this comment.
The endpoint to return "all file team-members" is also being filtered by the current user regions. I think the region restriction should be lifted as well here. I understand this was not explicitly called for in the ACs so this could be logged as a follow-up story/fix. Please reach out to Devin for feedback.
If not fixed with this PR we will need to track the change in another story
| var userRegions = pimsUser.PimsRegionUsers.Select(r => r.RegionCode).ToHashSet(); | ||
| long? contractorPersonId = pimsUser.IsContractor ? pimsUser.PersonId : null; | ||
|
|
||
| var teamMembers = _leaseRepository.GetTeamMembers(userRegions, contractorPersonId); |
There was a problem hiding this comment.
Same comment re: team-members region restriction
| var pimsUser = _userRepository.GetUserInfoByKeycloakUserId(_user.GetUserKey()); | ||
| long? contractorPersonId = pimsUser.IsContractor ? pimsUser.PersonId : null; | ||
|
|
||
| var leases = _leaseRepository.GetAllByIds(leaseIds, pimsUser.PimsRegionUsers.Select(u => u.RegionCode).ToHashSet(), contractorPersonId).ToList(); |
There was a problem hiding this comment.
this method is called in one of the lease exports. please remove the region restriction here.
| var pimsUser = _userRepository.GetUserInfoByKeycloakUserId(_user.GetUserKey()); | ||
| var userRegions = pimsUser.PimsRegionUsers.Select(r => r.RegionCode).ToHashSet(); | ||
|
|
||
| return _projectRepository.SearchProjects(filter, userRegions, maxResult); |
There was a problem hiding this comment.
This correspond to the endpoints that serve the project "auto-complete". Not called explicitly in ACs so it would be okay to bundle with the other changes already mentioned that could go to a separate story. Check with Devin anyways please
| acquisitionTeamMemberPersonId: acquisitionTeamPersonId ? acquisitionTeamPersonId : null, | ||
| acquisitionTeamMemberOrganizationId: acquistionTeamOrganizationId | ||
| ? acquistionTeamOrganizationId |
There was a problem hiding this comment.
use isValidId to check here
| leaseStatusTypes: MultiSelectOption[] = []; | ||
| tenantName = ''; | ||
| expiryEndDate = ''; | ||
| regionType = ''; |
There was a problem hiding this comment.
I think this single-select region is not needed
| searchBy = 'address'; | ||
| pin = ''; | ||
| pid = ''; | ||
| regionCode = ''; |
| export interface MultiSelectOption { | ||
| id: string; | ||
| text: string; | ||
| } |
There was a problem hiding this comment.
Thanks for moving this to a shared folder
| searchBy: string; | ||
| pin: string; | ||
| pid: string; | ||
| regionCode: string; |
3 participants
No description provided.