Fix disabled-user bypass after online-to-offline transition#1583
Conversation
There was a problem hiding this comment.
Pull request overview
This PR closes a gap in the OIDC broker password-auth flow where a session that transitions to offline mode during token refresh (e.g., due to a network timeout) could previously proceed using cached credentials without re-applying cached “user/device disabled” flags.
Changes:
- Re-check cached
UserIsDisabled/DeviceIsDisabledflags after token-refresh fallback sets the session to offline. - Add regression test cases for online → offline transition during token refresh when user/device is disabled.
- Add golden test fixtures for the new regression scenarios.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| authd-oidc-brokers/internal/broker/broker.go | Adds a post-refresh offline disabled-status re-check to prevent offline login when cached flags indicate user/device is disabled. |
| authd-oidc-brokers/internal/broker/broker_test.go | Adds regression tests covering network-error token refresh causing an online→offline transition with disabled user/device. |
| authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_user_is_disabled_and_session_transitions_to_offline_due_to_network_error/first_call | Golden output asserting access denied + user-disabled messaging on offline transition. |
| authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_user_is_disabled_and_session_transitions_to_offline_due_to_network_error/data/provider_url/test-user@email.com/token.json | Golden cached token fixture for the new user-disabled offline-transition scenario. |
| authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_user_is_disabled_and_session_transitions_to_offline_due_to_network_error/data/provider_url/test-user@email.com/password | Golden cached password fixture for the new user-disabled offline-transition scenario. |
| authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_device_is_disabled_and_session_transitions_to_offline_due_to_network_error/first_call | Golden output asserting access denied + device-disabled messaging on offline transition. |
| authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_device_is_disabled_and_session_transitions_to_offline_due_to_network_error/data/provider_url/test-user@email.com/token.json | Golden cached token fixture for the new device-disabled offline-transition scenario. |
| authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_device_is_disabled_and_session_transitions_to_offline_due_to_network_error/data/provider_url/test-user@email.com/password | Golden cached password fixture for the new device-disabled offline-transition scenario. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
76cd656 to
ccfb31c
Compare
ccfb31c to
d814240
Compare
da6de45 to
d814240
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1583 +/- ##
=======================================
Coverage 84.25% 84.25%
=======================================
Files 21 21
Lines 1162 1162
=======================================
Hits 979 979
Misses 183 183 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Disabled-account checks were applied only at session start. When token refresh fails with a network error, the session falls back to offline without re-evaluating the cached disabled flags, allowing a disabled user to log in if the network drops mid-auth. Consolidate checks to the post-refresh block where all authentication paths converge. This catches both offline-at-start and online→offline-fallback cases. Removes duplicate logic and establishes a single canonical gate before access grant.
97eb239 to
0771f19
Compare
Disabled-account checks were applied only at session start. When token
refresh fails with a network error, the session falls back to offline
without re-evaluating the cached disabled flags, allowing a disabled user
to log in if the network drops mid-auth.
Consolidate checks to the post-refresh block where all authentication paths
converge. This catches both offline-at-start and online→offline-fallback
cases. Removes duplicate logic and establishes a single canonical gate
before access grant.
UDENG-10645