Add OIDC auth with role-based app permissions#94
Merged
Conversation
Introduces optional Microsoft Entra OIDC sign-in (`AUTH_TYPE=oidc`) with reader/privileged/admin group mapping, callback handling, session versioning, and role-aware auth helpers. It also adds per-capability gates (reports, sync, restricted XLSX), updates `/api/auth-check` to return capability flags/user identity, and applies those permissions across protected pages and APIs. The UI and config flow were updated to reflect capability-based access: OIDC group visibility and privileged permission toggles in Configure, neutral XLSX labeling (“Restricted”/“XLSX”), OIDC-aware login/logout behavior, and a new “Find Me” button for signed-in OIDC users. Tests were moved under `.tests`, expanded heavily for auth/OIDC/permission behavior, and test/runtime paths were isolated via env-driven data/session directories. Documentation, env template, CI test paths, and dependencies were updated accordingly.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
… remote source' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds an optional Microsoft Entra OIDC authentication mode (AUTH_TYPE=oidc) with group-based roles and capability-style permissions, then propagates those permissions through the Flask routes/APIs and the vanilla JS UI. It also reorganizes and expands the test suite (moving it under .tests) and introduces env-driven runtime paths to isolate test state.
Changes:
- Introduces OIDC auth flow (MSAL auth-code flow + Graph
/meandcheckMemberGroups) and role resolution (reader/privileged/admin). - Implements role/capability-aware authorization for pages and APIs (reports, sync, restricted XLSX columns) and updates
/api/auth-checkto return identity + capability flags. - Updates UI/i18n for “Find Me”, capability-neutral XLSX labeling, and OIDC visibility/permission controls on Configure; moves/expands tests and updates CI/test discovery paths.
Reviewed changes
Copilot reviewed 24 out of 32 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_auth.py | Removes old test location (tests moved under .tests). |
| templates/index.html | Adds “Find Me” button and changes XLSX label to capability-neutral text. |
| templates/configure.html | Adds OIDC access visibility + privileged-permission toggles; updates export column wording; hides logout in OIDC mode. |
| static/styles.css | Styles for the new search-controls layout and “Find Me” button. |
| static/locales/en-US.json | Adds i18n strings for “Find Me” and updates XLSX/export wording and OIDC section labels. |
| static/configure.js | Loads/saves oidcPrivilegedPermissions toggles in settings UI. |
| static/app.js | Consumes enhanced /api/auth-check payload; gates UI actions by capabilities; implements “Find Me” resolution. |
| simple_org_chart/settings.py | Adds default oidcPrivilegedPermissions settings. |
| simple_org_chart/oidc_auth.py | New module for MSAL OIDC flow + Graph calls + role resolution. |
| simple_org_chart/config.py | Makes DATA_DIR configurable via SIMPLE_ORG_CHART_DATA_DIR (supports test/runtime isolation). |
| simple_org_chart/auth.py | Adds role concepts + role-aware decorators and redirects; keeps sanitize-next-path logic. |
| simple_org_chart/app_main.py | Adds OIDC mode enforcement, callback handling, capability decorators, session-dir overrides, and capability-aware route protection. |
| requirements.txt | Adds msal dependency. |
| README.md | Documents OIDC setup, group mapping, and capability model. |
| pyproject.toml | Updates pytest discovery to .tests. |
| AGENTS.md | Updates/renumbers repository guidance sections. |
| .tests/test_templates.py | Adds template-level assertions for new UI ordering and neutral XLSX wording. |
| .tests/test_settings.py | Adds settings parsing/persistence tests (including defaults and normalization). |
| .tests/test_reports.py | Adds report/filter logic tests. |
| .tests/test_report_configs.py | Validates REPORT_CONFIGS i18n keys + route dataPath consistency. |
| .tests/test_oidc_auth.py | Adds unit tests for OIDC helper functions and Graph request behavior. |
| .tests/test_i18n.py | Ensures template/JS i18n keys exist in en-US.json. |
| .tests/test_hierarchy.py | Adds hierarchy-building and missing-manager detection tests. |
| .tests/test_exports.py | Adds export metadata/filter formatting tests. |
| .tests/test_email_schedule.py | Adds email scheduling logic tests and config persistence behavior tests. |
| .tests/test_config.py | Updates config path tests for env-configured data dir. |
| .tests/test_auth.py | Adds auth/role/capability integration tests (including OIDC callback scenarios). |
| .tests/conftest.py | Creates/cleans a temp runtime dir; sets env vars for isolated data/session paths. |
| .github/workflows/tests.yml | Updates CI to run pytest against .tests/. |
| .github/copilot-instructions.md | Removes outdated Copilot instruction file (guidance consolidated elsewhere). |
| .env.template | Adds OIDC env configuration and clarifies ADMIN_PASSWORD requirement in simple mode only. |
| .dockerignore | Excludes .tests/ from Docker build context. |
Comment on lines
+181
to
+188
| def has_app_permission(permission): | ||
| """Return whether the current user has an application capability.""" | ||
| if has_role(ROLE_ADMIN): | ||
| return True | ||
| if current_role() != ROLE_PRIVILEGED: | ||
| return False | ||
| configured = load_settings().get('oidcPrivilegedPermissions', {}) or {} | ||
| return bool(configured.get(permission, PRIVILEGED_PERMISSION_DEFAULTS[permission])) |
Contributor
There was a problem hiding this comment.
Fixed in the latest commit. Changed to PRIVILEGED_PERMISSION_DEFAULTS.get(permission, False) so any unknown permission string returns False (denied) instead of raising KeyError.
| 2. Under **Users and groups**, assign the reader, privileged, and admin security groups used below. | ||
| 3. Do not rely on tenant membership alone; the application also verifies membership in one of these three groups during every new login. | ||
|
|
||
| Microsoft Entra rejects an unassigned user before redirecting back to the org chart. Users assigned to the Enterprise Application but absent from the two access groups receive the base reader role. |
Contributor
There was a problem hiding this comment.
Fixed in the latest commit. Updated the README to accurately reflect that users absent from all three access groups receive a 403 denial, not a base reader role.
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
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.
Introduces optional Microsoft Entra OIDC sign-in (
AUTH_TYPE=oidc) with reader/privileged/admin group mapping, callback handling, session versioning, and role-aware auth helpers. It also adds per-capability gates (reports, sync, restricted XLSX), updates/api/auth-checkto return capability flags/user identity, and applies those permissions across protected pages and APIs.The UI and config flow were updated to reflect capability-based access: OIDC group visibility and privileged permission toggles in Configure, neutral XLSX labeling (“Restricted”/“XLSX”), OIDC-aware login/logout behavior, and a new “Find Me” button for signed-in OIDC users. Tests were moved under
.tests, expanded heavily for auth/OIDC/permission behavior, and test/runtime paths were isolated via env-driven data/session directories. Documentation, env template, CI test paths, and dependencies were updated accordingly.