Deny login early when offline and force_provider_authentication set

authd-oidc-brokers/internal/broker/broker.go

@@ -30,7 +30,6 @@ import (
 	"github.com/canonical/authd/log"
 	"github.com/coreos/go-oidc/v3/oidc"
 	"github.com/google/uuid"
-	"github.com/ubuntu/decorate"
 	"golang.org/x/oauth2"
 )
 
@@ -158,8 +157,6 @@ func New(cfg Config, args ...Option) (b *Broker, err error) {
 
 // NewSession creates a new session for the user.
 func (b *Broker) NewSession(username, lang, mode string) (sessionID, encryptionKey string, err error) {
-	defer decorate.OnError(&err, "could not create new session for user %q", username)
-
 	sessionID = uuid.New().String()
 	s := session{
 		username: username,
@@ -171,7 +168,7 @@ func (b *Broker) NewSession(username, lang, mode string) (sessionID, encryptionK
 
 	pubASN1, err := x509.MarshalPKIXPublicKey(&b.privateKey.PublicKey)
 	if err != nil {
-		return "", "", err
+		return "", "", fmt.Errorf("failed to marshal broker public key: %v", err)
 	}
 
 	_, issuer, _ := strings.Cut(b.cfg.issuerURL, "://")
@@ -185,8 +182,13 @@ func (b *Broker) NewSession(username, lang, mode string) (sessionID, encryptionK
 
 	// Construct an OIDC provider via OIDC discovery.
 	s.oidcServer, err = b.connectToOIDCServer(context.Background())
+	if err != nil && b.cfg.forceProviderAuthentication {
+		log.Errorf(context.Background(), "Could not connect to the provider and force_provider_authentication is set, denying authentication: %v", err)
+		//nolint:staticcheck,revive // ST1005 This error is displayed as is to the user, so it should be capitalized
+		return "", "", errors.New("Error connecting to provider. Check your network connection.")
+	}
 	if err != nil {
-		log.Noticef(context.Background(), "Could not connect to the provider: %v. Starting session in offline mode.", err)
+		log.Noticef(context.Background(), "Could not connect to the provider, starting session in offline mode: %v", err)
 		s.isOffline = true
 		s.providerConnectionError = err
 	}
@@ -743,11 +745,6 @@ func (b *Broker) passwordAuth(ctx context.Context, session *session, secret stri
 		return AuthNext, nil
 	}
 
-	if b.cfg.forceProviderAuthentication && session.isOffline {
-		log.Error(context.Background(), "Remote authentication failed: force_provider_authentication is enabled, but the identity provider is not reachable")
-		return AuthDenied, errorMessage{Message: "Remote authentication failed: identity provider is not reachable"}
-	}
-
 	if authInfo.UserIsDisabled && session.isOffline {
 		log.Errorf(context.Background(), "Login denied: user %q is disabled in %s and session is offline", session.username, b.provider.DisplayName())
 		return AuthDenied, errorMessage{Message: fmt.Sprintf("Your user account is disabled in %s. Please contact your administrator or try again with a working network connection.", b.provider.DisplayName())}

authd-oidc-brokers/internal/broker/broker_test.go

@@ -83,9 +83,11 @@ func TestNewSession(t *testing.T) {
 	t.Parallel()
 
 	tests := map[string]struct {
-		customHandlers map[string]testutils.EndpointHandler
+		customHandlers              map[string]testutils.EndpointHandler
+		forceProviderAuthentication bool
 
 		wantOffline bool
+		wantErr     bool
 	}{
 		"Successfully_create_new_session": {},
 		"Creates_new_session_in_offline_mode_if_provider_is_not_available": {
@@ -100,16 +102,29 @@ func TestNewSession(t *testing.T) {
 			},
 			wantOffline: true,
 		},
+
+		"Error_when_provider_authentication_is_forced_and_provider_is_not_available": {
+			customHandlers: map[string]testutils.EndpointHandler{
+				"/.well-known/openid-configuration": testutils.UnavailableHandler(),
+			},
+			forceProviderAuthentication: true,
+			wantErr:                     true,
+		},
 	}
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
 			b := newBrokerForTests(t, &brokerForTestConfig{
-				customHandlers: tc.customHandlers,
+				customHandlers:              tc.customHandlers,
+				forceProviderAuthentication: tc.forceProviderAuthentication,
 			})
 
 			id, _, err := b.NewSession("test-user", "lang", sessionmode.Login)
+			if tc.wantErr {
+				require.Error(t, err, "NewSession should have returned an error")
+				return
+			}
 			require.NoError(t, err, "NewSession should not have returned an error")
 
 			gotOffline, err := b.IsOffline(id)
@@ -770,12 +785,6 @@ func TestIsAuthenticated(t *testing.T) {
 		"Error_when_mode_is_newpassword_and_session_has_no_token": {firstMode: authmodes.NewPassword},
 		// This test case also tests that errors with double quotes are marshaled to JSON correctly.
 		"Error_when_selected_username_does_not_match_the_provider_one": {username: "not-matching", firstSecret: "-"},
-		"Error_when_provider_authentication_is_forced_and_session_is_offline": {
-			firstMode:                   authmodes.Password,
-			token:                       &tokenOptions{},
-			forceProviderAuthentication: true,
-			sessionOffline:              true,
-		},
 		"Error_when_user_is_disabled_and_session_is_offline": {
 			firstMode:      authmodes.Password,
 			token:          &tokenOptions{userIsDisabled: true},