Skip to content

fix: Respect the next link in UIF Entra backend requests#859

Merged
lfrancke merged 3 commits into
mainfrom
fix/uif-entra-fetch-all-groups
Jul 17, 2026
Merged

fix: Respect the next link in UIF Entra backend requests#859
lfrancke merged 3 commits into
mainfrom
fix/uif-entra-fetch-all-groups

Conversation

@maltesander

@maltesander maltesander commented Jul 17, 2026

Copy link
Copy Markdown
Member

Description

fixes #858

Cherry-picked into release-26.7 here: #860

  • GroupMembershipResponse now has #[serde(rename = "@odata.nextLink")] next_link: Option<String>, so the link is no longer silently discarded.
  • The single group request became a loop that follows next_link until it's absent, accumulating groups across pages.
  • group_info() now requests /memberOf/microsoft.graph.group?$select=displayName&$top=999, filtering to actual groups, and cutting round-trips by taking the largest page size Graph accepts.
  • Added wiremock tests

Definition of Done Checklist

  • Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant
  • Please make sure all these things are done and tick the boxes

Author

  • Changes are OpenShift compatible
  • CRD changes approved
  • CRD documentation for all fields, following the style guide.
  • Helm chart can be installed and deployed operator works
  • Integration tests passed (for non trivial changes)
  • Changes need to be "offline" compatible
  • Links to generated (nightly) docs added
  • Release note snippet added

Reviewer

  • Code contains useful comments
  • Code contains useful logging statements
  • (Integration-)Test cases added
  • Documentation added or updated. Follows the style guide.
  • Changelog updated
  • Cargo.toml only contains references to git tags (not specific commits or branches)

Acceptance

  • Feature Tracker has been updated
  • Proper release label has been added
  • Links to generated (nightly) docs added
  • Release note snippet added
  • Add type/deprecation label & add to the deprecation schedule
  • Add type/experimental label & add to the experimental features tracker

…eouts

Follow-ups to the @odata.nextLink pagination fix in #859:

- Add a 60s request / 15s connect timeout to every user-info-fetcher
  outbound HTTP backend (Keycloak, Entra, XFSC AAS) via a shared
  utils::http::client_builder(); reqwest otherwise waits indefinitely.
- Keep every directory membership by requesting /memberOf instead of
  restricting to microsoft.graph.group, so directory roles and
  administrative units are still returned.
- Cap @odata.nextLink following at 100 pages as a backstop against a
  cyclic or runaway link, warning if the group list may be truncated.
- Bump the yanked spin 0.9.8 to 0.9.9 (matching #857, which only landed
  on release-26.7) so cargo-deny passes, and regenerate Cargo.nix for
  the new wiremock dev-dependency.
@NickLarsenNZ NickLarsenNZ changed the title fix: respect next link in uif entra backend requests fix: Respect the next link in UIF entra backend requests Jul 17, 2026
@NickLarsenNZ NickLarsenNZ added scheduled-for/26.7.0 release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jul 17, 2026
@NickLarsenNZ NickLarsenNZ changed the title fix: Respect the next link in UIF entra backend requests fix: Respect the next link in UIF Entra backend requests Jul 17, 2026
Comment thread rust/user-info-fetcher/src/backend/entra.rs
@lfrancke
lfrancke marked this pull request as ready for review July 17, 2026 09:20
@lfrancke lfrancke moved this to Development: Waiting for Review in Stackable Engineering Jul 17, 2026
@lfrancke

Copy link
Copy Markdown
Member

I added release notes to the issue: #858 (comment)

@Techassi Techassi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly lgtm, just two minor comments.

Comment thread rust/user-info-fetcher/src/backend/entra.rs Outdated
Comment thread CHANGELOG.md Outdated
@Techassi Techassi moved this from Development: Waiting for Review to Development: In Review in Stackable Engineering Jul 17, 2026
- Restructure the Entra group pagination loop guard-first: check the page
  cap and warn/break before doing the request, per review.
- Reword the changelog entry for the pagination fix.
@lfrancke
lfrancke added this pull request to the merge queue Jul 17, 2026
Merged via the queue into main with commit 5b3aa4c Jul 17, 2026
18 checks passed
@lfrancke
lfrancke deleted the fix/uif-entra-fetch-all-groups branch July 17, 2026 11:31
@NickLarsenNZ NickLarsenNZ moved this from Development: In Review to Development: Done in Stackable Engineering Jul 17, 2026
NickLarsenNZ added a commit that referenced this pull request Jul 17, 2026
* fix: respect next link in uif entra backend requests

* fix: bound Entra group pagination, keep all memberships, add HTTP timeouts

Follow-ups to the @odata.nextLink pagination fix in #859:

- Add a 60s request / 15s connect timeout to every user-info-fetcher
  outbound HTTP backend (Keycloak, Entra, XFSC AAS) via a shared
  utils::http::client_builder(); reqwest otherwise waits indefinitely.
- Keep every directory membership by requesting /memberOf instead of
  restricting to microsoft.graph.group, so directory roles and
  administrative units are still returned.
- Cap @odata.nextLink following at 100 pages as a backstop against a
  cyclic or runaway link, warning if the group list may be truncated.
- Bump the yanked spin 0.9.8 to 0.9.9 (matching #857, which only landed
  on release-26.7) so cargo-deny passes, and regenerate Cargo.nix for
  the new wiremock dev-dependency.

* fix: address review feedback

- Restructure the Entra group pagination loop guard-first: check the page
  cap and warn/break before doing the request, per review.
- Reword the changelog entry for the pagination fix.

---------

Co-authored-by: maltesander <contact@maltesander.com>
Co-authored-by: Lars Francke <jujutsu@lars-francke.de>
@lfrancke lfrancke moved this from Development: Done to Done in Stackable Engineering Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

UserInfoFetcher (OPA) does not collect all available Entra-Groups for policy checks

4 participants