feat(auth): handle EntraID group overage via Graph fallback with configurable limits#2985
Merged
crivetimihai merged 6 commits intomainfrom Feb 20, 2026
Merged
feat(auth): handle EntraID group overage via Graph fallback with configurable limits#2985crivetimihai merged 6 commits intomainfrom
crivetimihai merged 6 commits intomainfrom
Conversation
Merged
10 tasks
Member
Author
|
Just rebased on top of: fix: enhance OAuth callback handling for error responses and missing authorization code #2899 |
e4471fd to
594b8f7
Compare
jonpspri
previously approved these changes
Feb 17, 2026
Collaborator
jonpspri
left a comment
There was a problem hiding this comment.
This PR adds EntraID group overage Graph API fallback with configurable limits, OAuth callback error handling with XSS remediation, and Entra v2 resource parameter omission — all backed by thorough unit tests, E2E tests, and documentation updates. The review identified strong security practices (HTML escaping, exact hostname validation, graceful degradation) and found two minor code quality issues: an f-string concatenated with a string literal in a logger.warning call (replaced with %s lazy formatting) and unnecessary getattr() defensive access on settings fields that are defined with defaults (replaced with direct attribute access). Both fixes were applied and committed as 695ec8d62. No additional E2E test cases were needed, as the Graph fallback paths are impractical to test against real Azure infrastructure and are already well-covered by unit tests.
Implement Microsoft Graph fallback for EntraID group overage in SSO flow, with configurable enable/timeout/max-group cap, expanded overage marker detection, bootstrap wiring, docs/config schema updates, and unit coverage.\n\nCloses #2201 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
… Graph fallback - Change getMemberObjects securityEnabledOnly from false to true so only security-enabled groups and directory roles are returned (excludes administrative units that could cause unintended role mappings) - Enforce 120s upper bound on graph_api_timeout from provider_metadata overrides, matching the config schema constraint (ge=1, le=120) Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
…aph fallback Replace f-string concatenation with %s lazy formatting in the overage warning log and drop unnecessary getattr() calls in sso_bootstrap since the settings fields are defined with defaults on the Settings class. Also improve overage troubleshooting docs and fix stale references. Signed-off-by: Jonathan Springer <jps@s390x.com>
…erride Replace blind bool() coercion with explicit int handling and a warning for unrecognised types so that provider_metadata.graph_api_enabled=null no longer silently disables the Graph overage fallback. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
695ec8d to
a150511
Compare
Member
Author
Review ChangesRebased onto Fixes
Commit HistoryAll 119 unit tests pass. |
vishu-bh
pushed a commit
that referenced
this pull request
Feb 24, 2026
…igurable limits (#2985) * feat(auth): add EntraID group overage Graph fallback limits Implement Microsoft Graph fallback for EntraID group overage in SSO flow, with configurable enable/timeout/max-group cap, expanded overage marker detection, bootstrap wiring, docs/config schema updates, and unit coverage.\n\nCloses #2201 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(auth): resolve pylint R1716 in Entra group cap logic Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * test(auth): cover Entra Graph fallback edge branches Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(auth): use securityEnabledOnly and enforce timeout upper bound in Graph fallback - Change getMemberObjects securityEnabledOnly from false to true so only security-enabled groups and directory roles are returned (excludes administrative units that could cause unintended role mappings) - Enforce 120s upper bound on graph_api_timeout from provider_metadata overrides, matching the config schema constraint (ge=1, le=120) Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(auth): use lazy log formatting and remove defensive getattr in Graph fallback Replace f-string concatenation with %s lazy formatting in the overage warning log and drop unnecessary getattr() calls in sso_bootstrap since the settings fields are defined with defaults on the Settings class. Also improve overage troubleshooting docs and fix stale references. Signed-off-by: Jonathan Springer <jps@s390x.com> * fix(auth): reject null/unsupported types in Graph fallback enabled override Replace blind bool() coercion with explicit int handling and a warning for unrecognised types so that provider_metadata.graph_api_enabled=null no longer silently disables the Graph overage fallback. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Signed-off-by: Jonathan Springer <jps@s390x.com> Co-authored-by: Jonathan Springer <jps@s390x.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Issue
Closes #2201