rymghosn opened a new pull request, #6648: URL: https://github.com/apache/fineract/pull/6648
## Description JIRA: [FINERACT-2947](https://issues.apache.org/jira/browse/FINERACT-2947) **Root cause.** `GET /users` and `GET /users/{id}` resolve each user's linked staff through `StaffReadService.retrieveStaff`, which applies the *caller's* office-hierarchy filter to the *staff's* office and throws `StaffNotFoundException` when it doesn't match. The user itself is visible (its office is in the caller's tree), but as soon as one such user is linked to a staff of another office (the parent office, or the user's old office after an office change), the whole users list fails with 404 "Staff with identifier N does not exist" for every non-head-office administrator. Separately, `updateUser` only checks staff against office when `staffId` changes, so `PUT /users/{id} {"officeId": X}` keeps a linked staff that does not belong to office X. **Fix.** - `AppUserReadPlatformServiceImpl`: the list query left-joins the linked staff and its office and maps it from the same row (also removes one `retrieveStaff` query per user); `retrieveUser` reads the linked staff by id. The `staff` object returned keeps the same fields. `StaffReadService` is no longer a dependency (`UserAdministrationConfiguration` updated). - `retrieveUser` now requires the target user's office hierarchy to start with the caller's (`UserNotFoundException` otherwise), as `retrieveAllUsers` already does. It never scoped the target user before: the staff lookup's hierarchy filter was the only guard, and only for users with a linked staff. Without this, removing the staff filter would let any `READ_USER` holder read every user in the tenant. On current `develop` a branch user can already read a head-office user that has no linked staff. - `AppUserWritePlatformServiceJpaRepositoryImpl.updateUser`: when `officeId` changes and `staffId` doesn't, the existing linked staff must belong to the new office (`staffRepository.findByOffice`), otherwise `StaffNotFoundException`, the same rule a combined `staffId` + `officeId` change already applies. **Tests.** `AppUserRetrieveUserHierarchyTest` and `AppUserUpdateOfficeLinkedStaffTest` (4 tests): all 4 fail on `develop`, all 4 pass with the fix. `spotlessJavaCheck` clean. **Live check on a local stock instance** (`develop` `d4ef406954`, PostgreSQL). Offices B and C under head office; a staff in B; user U in B linked to it; user A in B whose role has only `READ_USER`; then `PUT /staff/{id} {"officeId": 1}` moves the staff to head office. | request | develop | with this PR | |---|---|---| | `GET /users` as A | 404 Staff with identifier N does not exist | 200, U listed with its staff | | `GET /users/{U}` as A | 404 Staff with identifier N does not exist | 200 with the staff | | `GET /users/1` (head-office user) as A | 200 | 404 `error.msg.user.id.invalid` | | `PUT /users/{U} {"officeId": C}` as admin | 200, U in C linked to a head-office staff | 404 `error.msg.staff.id.invalid`, U unchanged | | `PUT /users/{U} {"officeId": 1}` as admin (the staff's office) | 200 | 200 | No API change: same endpoints, same response shape. ## Checklist Please make sure these boxes are checked before submitting your pull request - thanks! - [x] Write the commit message as per [our guidelines](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#pull-requests) - [x] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers. - [x] Create/update [unit or integration tests](https://fineract.apache.org/docs/current/#_testing) for verifying the changes made. - [x] Follow our [coding conventions](https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions). - [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes (no API change) - [x] [This PR must not be a "code dump"](https://cwiki.apache.org/confluence/display/FINERACT/Pull+Request+Size+Limit). Large changes can be made in a branch, with assistance. Ask for help on the [developer mailing list](https://fineract.apache.org/#contribute). - [x] If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately. - [x] I followed the [AI Policy](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#ai-policy). Your assigned reviewer(s) will follow our [guidelines for code reviews](https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
