From e128697d3df295642f713b3ac2313c129e5f7c1b Mon Sep 17 00:00:00 2001 From: Noor Eldeen Mansour Date: Thu, 2 Apr 2026 17:53:24 +0200 Subject: [PATCH] fix: allow logins for token refresh failures due to unstable connections --- authd-oidc-brokers/internal/broker/broker.go | 15 +++++++++-- .../internal/broker/broker_test.go | 27 ++++++++++++++++++- .../provider_url/test-user@email.com/password | 0 .../test-user@email.com/token.json | 0 .../first_call | 3 +++ .../provider_url/test-user@email.com/password | 1 + .../test-user@email.com/token.json | 1 + .../first_call | 0 8 files changed, 44 insertions(+), 3 deletions(-) rename authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/{Error_when_mode_is_password_and_token_refresh_times_out => Authenticating_with_password_skips_token_refresh_network_error}/data/provider_url/test-user@email.com/password (100%) rename authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/{Error_when_mode_is_password_and_token_refresh_times_out => Authenticating_with_password_skips_token_refresh_network_error}/data/provider_url/test-user@email.com/token.json (100%) create mode 100644 authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Authenticating_with_password_skips_token_refresh_network_error/first_call create mode 100644 authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_mode_is_password_and_token_refresh_times_out_with_forced_provider_auth/data/provider_url/test-user@email.com/password create mode 100644 authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_mode_is_password_and_token_refresh_times_out_with_forced_provider_auth/data/provider_url/test-user@email.com/token.json rename authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/{Error_when_mode_is_password_and_token_refresh_times_out => Error_when_mode_is_password_and_token_refresh_times_out_with_forced_provider_auth}/first_call (100%) diff --git a/authd-oidc-brokers/internal/broker/broker.go b/authd-oidc-brokers/internal/broker/broker.go index 2dee4139b5..a9d16df7c4 100644 --- a/authd-oidc-brokers/internal/broker/broker.go +++ b/authd-oidc-brokers/internal/broker/broker.go @@ -10,6 +10,7 @@ import ( "encoding/json" "errors" "fmt" + "net" "path/filepath" "slices" "strings" @@ -792,7 +793,17 @@ func (b *Broker) passwordAuth(ctx context.Context, session *session, secret stri } if err != nil { log.Errorf(context.Background(), "Failed to refresh token: %s", err) - return AuthDenied, errorMessage{Message: "Failed to refresh token"} + + // Fall back to offline mode for transient network failures (e.g. timeout, DNS, + // connection refused). Unless provider authentication is forced. + var netErr net.Error + if errors.As(err, &netErr) && !b.cfg.forceProviderAuthentication { + log.Warningf(context.Background(), "Network error during token refresh for user %q, skipping token refresh", session.username) + authInfo = oldAuthInfo + session.isOffline = true + } else { + return AuthDenied, errorMessage{Message: "Failed to refresh token"} + } } } @@ -1095,7 +1106,7 @@ func (b *Broker) refreshToken(ctx context.Context, session *session, oldToken *t func (b *Broker) userInfoFromIDToken(ctx context.Context, session *session, rawIDToken string) (info.User, error) { idToken, err := session.oidcServer.Verifier(&b.oidcCfg).Verify(ctx, rawIDToken) if err != nil { - return info.User{}, fmt.Errorf("could not verify token: %v", err) + return info.User{}, fmt.Errorf("could not verify token: %w", err) } userInfo, err := b.provider.GetUserInfo(idToken) diff --git a/authd-oidc-brokers/internal/broker/broker_test.go b/authd-oidc-brokers/internal/broker/broker_test.go index 3cc46b7cbe..2c6ee8111e 100644 --- a/authd-oidc-brokers/internal/broker/broker_test.go +++ b/authd-oidc-brokers/internal/broker/broker_test.go @@ -564,6 +564,7 @@ func TestIsAuthenticated(t *testing.T) { readOnlyDataDir bool wantGroups []info.Group wantNextAuthModes []string + wantOffline bool }{ "Successfully_authenticate_user_with_device_auth_and_newpassword": {firstSecret: "-", wantSecondCall: true}, "Successfully_authenticate_user_with_password": {firstMode: authmodes.Password, token: &tokenOptions{}}, @@ -718,7 +719,16 @@ func TestIsAuthenticated(t *testing.T) { "Error_when_mode_is_password_and_token_is_invalid": {firstMode: authmodes.Password, token: &tokenOptions{invalid: true}}, "Error_when_mode_is_password_and_no_refresh_token": {firstMode: authmodes.Password, token: &tokenOptions{noRefreshToken: true}}, "Error_when_token_is_expired_and_refreshing_token_fails": {firstMode: authmodes.Password, token: &tokenOptions{expired: true, noRefreshToken: true}}, - "Error_when_mode_is_password_and_token_refresh_times_out": {firstMode: authmodes.Password, token: &tokenOptions{expired: true}, + "Authenticating_with_password_skips_token_refresh_network_error": {firstMode: authmodes.Password, token: &tokenOptions{expired: true}, + customHandlers: map[string]testutils.EndpointHandler{ + "/token": testutils.HangingHandler(broker.MaxRequestDuration + 1), + }, + wantOffline: true, + }, + "Error_when_mode_is_password_and_token_refresh_times_out_with_forced_provider_auth": { + firstMode: authmodes.Password, + token: &tokenOptions{expired: true}, + forceProviderAuthentication: true, customHandlers: map[string]testutils.EndpointHandler{ "/token": testutils.HangingHandler(broker.MaxRequestDuration + 1), }, @@ -906,6 +916,21 @@ func TestIsAuthenticated(t *testing.T) { <-firstCallDone } + if tc.wantOffline { + gotOffline, err := b.IsOffline(sessionID) + require.NoError(t, err, "IsOffline should not have returned an error") + require.True(t, gotOffline, "Session should be offline after token refresh network error") + } + + // When forceProviderAuthentication is set, offline fallback must never happen, + // even for transient network errors. Verify the session was not flipped to offline. + // (Skip if the session was already offline at creation, which is a separate scenario.) + if tc.forceProviderAuthentication && !tc.sessionOffline { + gotOffline, err := b.IsOffline(sessionID) + require.NoError(t, err, "IsOffline should not have returned an error") + require.False(t, gotOffline, "Session should not be offline when forceProviderAuthentication is true") + } + if tc.wantSecondCall { // Give some time for the first call time.Sleep(10 * time.Millisecond) diff --git a/authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_mode_is_password_and_token_refresh_times_out/data/provider_url/test-user@email.com/password b/authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Authenticating_with_password_skips_token_refresh_network_error/data/provider_url/test-user@email.com/password similarity index 100% rename from authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_mode_is_password_and_token_refresh_times_out/data/provider_url/test-user@email.com/password rename to authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Authenticating_with_password_skips_token_refresh_network_error/data/provider_url/test-user@email.com/password diff --git a/authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_mode_is_password_and_token_refresh_times_out/data/provider_url/test-user@email.com/token.json b/authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Authenticating_with_password_skips_token_refresh_network_error/data/provider_url/test-user@email.com/token.json similarity index 100% rename from authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_mode_is_password_and_token_refresh_times_out/data/provider_url/test-user@email.com/token.json rename to authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Authenticating_with_password_skips_token_refresh_network_error/data/provider_url/test-user@email.com/token.json diff --git a/authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Authenticating_with_password_skips_token_refresh_network_error/first_call b/authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Authenticating_with_password_skips_token_refresh_network_error/first_call new file mode 100644 index 0000000000..2928c3a331 --- /dev/null +++ b/authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Authenticating_with_password_skips_token_refresh_network_error/first_call @@ -0,0 +1,3 @@ +access: granted +data: '{"userinfo":{"name":"test-user@email.com","uuid":"saved-user-id","dir":"/home/test-user@email.com","shell":"/usr/bin/bash","gecos":"test-user@email.com","groups":[{"name":"saved-remote-group","ugid":"12345"},{"name":"saved-local-group","ugid":""}]}}' +err: diff --git a/authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_mode_is_password_and_token_refresh_times_out_with_forced_provider_auth/data/provider_url/test-user@email.com/password b/authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_mode_is_password_and_token_refresh_times_out_with_forced_provider_auth/data/provider_url/test-user@email.com/password new file mode 100644 index 0000000000..119947240f --- /dev/null +++ b/authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_mode_is_password_and_token_refresh_times_out_with_forced_provider_auth/data/provider_url/test-user@email.com/password @@ -0,0 +1 @@ +Definitely a hashed password \ No newline at end of file diff --git a/authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_mode_is_password_and_token_refresh_times_out_with_forced_provider_auth/data/provider_url/test-user@email.com/token.json b/authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_mode_is_password_and_token_refresh_times_out_with_forced_provider_auth/data/provider_url/test-user@email.com/token.json new file mode 100644 index 0000000000..ecaed7cd75 --- /dev/null +++ b/authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_mode_is_password_and_token_refresh_times_out_with_forced_provider_auth/data/provider_url/test-user@email.com/token.json @@ -0,0 +1 @@ +Definitely a token \ No newline at end of file diff --git a/authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_mode_is_password_and_token_refresh_times_out/first_call b/authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_mode_is_password_and_token_refresh_times_out_with_forced_provider_auth/first_call similarity index 100% rename from authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_mode_is_password_and_token_refresh_times_out/first_call rename to authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Error_when_mode_is_password_and_token_refresh_times_out_with_forced_provider_auth/first_call