diff --git a/authd-oidc-brokers/cmd/authd-oidc/daemon/daemon.go b/authd-oidc-brokers/cmd/authd-oidc/daemon/daemon.go index 65f4bf7f7e..90c0ebac0b 100644 --- a/authd-oidc-brokers/cmd/authd-oidc/daemon/daemon.go +++ b/authd-oidc-brokers/cmd/authd-oidc/daemon/daemon.go @@ -163,15 +163,12 @@ func (a *App) serve(config daemonConfig) error { } } - b, err := broker.New(broker.Config{ + brokerConfig := broker.Config{ ConfigFile: config.Paths.BrokerConf, DataDir: config.Paths.DataDir, - }) - if err != nil { - return err } - s, err := dbusservice.New(ctx, b) + s, err := dbusservice.New(ctx, brokerConfig) if err != nil { return err } diff --git a/authd-oidc-brokers/internal/broker/broker.go b/authd-oidc-brokers/internal/broker/broker.go index 78317a1ad3..f0e4a3f7b7 100644 --- a/authd-oidc-brokers/internal/broker/broker.go +++ b/authd-oidc-brokers/internal/broker/broker.go @@ -35,6 +35,11 @@ import ( ) const ( + // LatestAPIVersion is the latest API version supported by the broker. It should be incremented when a non backward + // compatible change is made to the API. + // Note: Remember to also bump the LatestAPIVersion in internal/brokers/dbusbroker.go. + LatestAPIVersion = 2 + maxAuthAttempts = 3 maxRequestDuration = 5 * time.Second ) @@ -49,7 +54,8 @@ type Config struct { // Broker is the real implementation of the broker to track sessions and process oidc calls. type Broker struct { - cfg Config + cfg Config + apiVersion uint provider providers.Provider oidcCfg oidc.Config @@ -98,7 +104,7 @@ type option struct { type Option func(*option) // New returns a new oidc Broker with the providers listed in the configuration file. -func New(cfg Config, args ...Option) (b *Broker, err error) { +func New(cfg Config, apiVersion uint, args ...Option) (b *Broker, err error) { p := providers.CurrentProvider() if cfg.ConfigFile != "" { @@ -146,6 +152,7 @@ func New(cfg Config, args ...Option) (b *Broker, err error) { b = &Broker{ cfg: cfg, + apiVersion: apiVersion, provider: opts.provider, oidcCfg: oidc.Config{ClientID: clientID}, privateKey: privateKey, diff --git a/authd-oidc-brokers/internal/broker/broker_test.go b/authd-oidc-brokers/internal/broker/broker_test.go index 21908b4ec1..17a4528da2 100644 --- a/authd-oidc-brokers/internal/broker/broker_test.go +++ b/authd-oidc-brokers/internal/broker/broker_test.go @@ -68,7 +68,7 @@ func TestNew(t *testing.T) { bCfg := &broker.Config{DataDir: tc.dataDir} bCfg.SetIssuerURL(tc.issuer) bCfg.SetClientID(tc.clientID) - b, err := broker.New(*bCfg) + b, err := broker.New(*bCfg, broker.LatestAPIVersion) if tc.wantErr { require.Error(t, err, "New should have returned an error") return diff --git a/authd-oidc-brokers/internal/broker/helper_test.go b/authd-oidc-brokers/internal/broker/helper_test.go index 20f8d2747e..33578d1c52 100644 --- a/authd-oidc-brokers/internal/broker/helper_test.go +++ b/authd-oidc-brokers/internal/broker/helper_test.go @@ -123,7 +123,7 @@ func newBrokerForTests(t *testing.T, cfg *brokerForTestConfig) (b *broker.Broker cfg.SetIssuerURL(issuerURL) } - b, err := broker.New(cfg.Config, broker.WithCustomProvider(provider)) + b, err := broker.New(cfg.Config, broker.LatestAPIVersion, broker.WithCustomProvider(provider)) require.NoError(t, err, "Setup: New should not have returned an error") return b } diff --git a/authd-oidc-brokers/internal/dbusservice/dbusservice.go b/authd-oidc-brokers/internal/dbusservice/dbusservice.go index 7016ec6e38..3df245b6e8 100644 --- a/authd-oidc-brokers/internal/dbusservice/dbusservice.go +++ b/authd-oidc-brokers/internal/dbusservice/dbusservice.go @@ -11,8 +11,9 @@ import ( "github.com/godbus/dbus/v5/introspect" ) -const intro = ` - +const ( + introspectableHeader = `` + introspectableInterface = ` @@ -50,51 +51,86 @@ const intro = ` - ` + introspect.IntrospectDataString + ` ` + ` + introspectableFooter = introspect.IntrospectDataString + ` ` +) -// Service is the handler exposing our broker methods on the system bus. +// Service is the object representing the dbus service, which contains the exported interfaces and the necessary +// information to disconnect from the bus and stop the service. type Service struct { - name string + name string + interfaces []*Interface + disconnect func() + serve chan struct{} +} + +// Interface is the object representing a dbus interface, which contains the broker to which delegate the calls and the +// name of the interface itself. +type Interface struct { + iface string broker *broker.Broker +} - serve chan struct{} - disconnect func() +var interfaceNames = []string{ + "com.ubuntu.authd.Broker", + "com.ubuntu.authd.Broker2", } // New returns a new dbus service after exporting to the system bus our name. -func New(_ context.Context, broker *broker.Broker) (s *Service, err error) { +func New(_ context.Context, brokerConfig broker.Config) (*Service, error) { name := consts.DbusName object := dbus.ObjectPath(consts.DbusObject) - iface := "com.ubuntu.authd.Broker" - s = &Service{ - name: name, - broker: broker, - serve: make(chan struct{}), + + service := &Service{ + name: name, + serve: make(chan struct{}), } - conn, err := s.getBus() + conn, err := service.getBus() if err != nil { return nil, err } - if err := conn.Export(s, object, iface); err != nil { - return nil, err + var introspectableBody string + for i, iface := range interfaceNames { + b, err := broker.New(brokerConfig, uint(i)+1) // There's no 0 version, so we start from 1. + if err != nil { + service.disconnect() + return nil, fmt.Errorf("error initializing broker for %q: %v", iface, err) + } + + s := &Interface{ + iface: iface, + broker: b, + } + + if err := conn.Export(s, object, iface); err != nil { + service.disconnect() + return nil, err + } + + service.interfaces = append(service.interfaces, s) + introspectableBody = introspectableBody + fmt.Sprintf(introspectableInterface, iface) } - if err := conn.Export(introspect.Introspectable(fmt.Sprintf(intro, iface)), object, "org.freedesktop.DBus.Introspectable"); err != nil { + + // Build combined introspection XML for all versioned interfaces and export once. + introspectable := introspect.Introspectable(introspectableHeader + introspectableBody + introspectableFooter) + if err := conn.Export(introspectable, object, "org.freedesktop.DBus.Introspectable"); err != nil { + service.disconnect() return nil, err } reply, err := conn.RequestName(consts.DbusName, dbus.NameFlagDoNotQueue) if err != nil { - s.disconnect() + service.disconnect() return nil, err } if reply != dbus.RequestNameReplyPrimaryOwner { - s.disconnect() + service.disconnect() return nil, fmt.Errorf("%q is already taken in the bus", name) } - return s, nil + return service, nil } // Addr returns the address of the service. @@ -117,5 +153,6 @@ func (s *Service) Stop() error { close(s.serve) s.disconnect() } + return nil } diff --git a/authd-oidc-brokers/internal/dbusservice/localbus.go b/authd-oidc-brokers/internal/dbusservice/localbus.go index f496f7ae5a..afb68d7dd6 100644 --- a/authd-oidc-brokers/internal/dbusservice/localbus.go +++ b/authd-oidc-brokers/internal/dbusservice/localbus.go @@ -5,9 +5,11 @@ package dbusservice import ( + "context" "os" "github.com/canonical/authd/authd-oidc-brokers/internal/testutils" + "github.com/canonical/authd/log" "github.com/godbus/dbus/v5" ) diff --git a/authd-oidc-brokers/internal/dbusservice/methods.go b/authd-oidc-brokers/internal/dbusservice/methods.go index b9d71c44cf..03f2991a6e 100644 --- a/authd-oidc-brokers/internal/dbusservice/methods.go +++ b/authd-oidc-brokers/internal/dbusservice/methods.go @@ -10,7 +10,7 @@ import ( ) // NewSession is the method through which the broker and the daemon will communicate once dbusInterface.NewSession is called. -func (s *Service) NewSession(username, lang, mode string) (sessionID, encryptionKey string, dbusErr *dbus.Error) { +func (s *Interface) NewSession(username, lang, mode string) (sessionID, encryptionKey string, dbusErr *dbus.Error) { log.Debugf(context.Background(), "Creating new session (username=%s, lang=%s, mode=%s)", username, lang, mode) sessionID, encryptionKey, err := s.broker.NewSession(username, lang, mode) if err != nil { @@ -21,7 +21,7 @@ func (s *Service) NewSession(username, lang, mode string) (sessionID, encryption } // GetAuthenticationModes is the method through which the broker and the daemon will communicate once dbusInterface.GetAuthenticationModes is called. -func (s *Service) GetAuthenticationModes(sessionID string, supportedUILayouts []map[string]string) (authenticationModes []map[string]string, dbusErr *dbus.Error) { +func (s *Interface) GetAuthenticationModes(sessionID string, supportedUILayouts []map[string]string) (authenticationModes []map[string]string, dbusErr *dbus.Error) { log.Debugf(context.Background(), "Getting authentication modes for session %s", sessionID) authenticationModes, err := s.broker.GetAuthenticationModes(sessionID, supportedUILayouts) if err != nil { @@ -32,7 +32,7 @@ func (s *Service) GetAuthenticationModes(sessionID string, supportedUILayouts [] } // SelectAuthenticationMode is the method through which the broker and the daemon will communicate once dbusInterface.SelectAuthenticationMode is called. -func (s *Service) SelectAuthenticationMode(sessionID, authenticationModeName string) (uiLayoutInfo map[string]string, dbusErr *dbus.Error) { +func (s *Interface) SelectAuthenticationMode(sessionID, authenticationModeName string) (uiLayoutInfo map[string]string, dbusErr *dbus.Error) { log.Debugf(context.Background(), "Selecting authentication mode %s for session %s", authenticationModeName, sessionID) uiLayoutInfo, err := s.broker.SelectAuthenticationMode(sessionID, authenticationModeName) if err != nil { @@ -43,7 +43,7 @@ func (s *Service) SelectAuthenticationMode(sessionID, authenticationModeName str } // IsAuthenticated is the method through which the broker and the daemon will communicate once dbusInterface.IsAuthenticated is called. -func (s *Service) IsAuthenticated(sessionID, authenticationData string) (access, data string, dbusErr *dbus.Error) { +func (s *Interface) IsAuthenticated(sessionID, authenticationData string) (access, data string, dbusErr *dbus.Error) { // Do *not* log authenticationData here, because it may contain the user's password in cleartext. log.Debugf(context.Background(), "Handling IsAuthenticated call for session %s", sessionID) access, data, err := s.broker.IsAuthenticated(sessionID, authenticationData) @@ -59,7 +59,7 @@ func (s *Service) IsAuthenticated(sessionID, authenticationData string) (access, } // EndSession is the method through which the broker and the daemon will communicate once dbusInterface.EndSession is called. -func (s *Service) EndSession(sessionID string) (dbusErr *dbus.Error) { +func (s *Interface) EndSession(sessionID string) (dbusErr *dbus.Error) { log.Debugf(context.Background(), "Ending session %s", sessionID) err := s.broker.EndSession(sessionID) if err != nil { @@ -69,14 +69,14 @@ func (s *Service) EndSession(sessionID string) (dbusErr *dbus.Error) { } // CancelIsAuthenticated is the method through which the broker and the daemon will communicate once dbusInterface.CancelIsAuthenticated is called. -func (s *Service) CancelIsAuthenticated(sessionID string) (dbusErr *dbus.Error) { +func (s *Interface) CancelIsAuthenticated(sessionID string) (dbusErr *dbus.Error) { log.Debugf(context.Background(), "Cancelling IsAuthenticated call for session %s", sessionID) s.broker.CancelIsAuthenticated(sessionID) return nil } // UserPreCheck is the method through which the broker and the daemon will communicate once dbusInterface.UserPreCheck is called. -func (s *Service) UserPreCheck(username string) (userinfo string, dbusErr *dbus.Error) { +func (s *Interface) UserPreCheck(username string) (userinfo string, dbusErr *dbus.Error) { log.Debugf(context.Background(), "UserPreCheck: %s", username) userinfo, err := s.broker.UserPreCheck(username) if err != nil { @@ -87,7 +87,7 @@ func (s *Service) UserPreCheck(username string) (userinfo string, dbusErr *dbus. } // DeleteUser is the method through which the broker and the daemon will communicate once dbusInterface.DeleteUser is called. -func (s *Service) DeleteUser(username string) (dbusErr *dbus.Error) { +func (s *Interface) DeleteUser(username string) (dbusErr *dbus.Error) { log.Debugf(context.Background(), "DeleteUser: %s", username) if err := s.broker.DeleteUser(username); err != nil { return dbus.MakeFailedError(err) diff --git a/e2e-tests/resources/browser_login/browser_window.py b/e2e-tests/resources/browser_login/browser_window.py index 51d4adeed8..5e3fa89b0c 100644 --- a/e2e-tests/resources/browser_login/browser_window.py +++ b/e2e-tests/resources/browser_login/browser_window.py @@ -223,7 +223,7 @@ def on_draw_event(): GLib.source_remove(stable_timeout_id) logger.info("Page is stable now") - def wait_for_pattern(self, pattern, timeout_ms=5000, + def wait_for_pattern(self, pattern, timeout_ms=10000, poll_interval_ms=100) -> list[str]: """Wait until `pattern` is present in the page's visible text and return all matched substrings.""" logger.info(f"Waiting for pattern '{pattern}'...") diff --git a/internal/brokers/dbusbroker.go b/internal/brokers/dbusbroker.go index 759ad247fa..e715439d0f 100644 --- a/internal/brokers/dbusbroker.go +++ b/internal/brokers/dbusbroker.go @@ -4,19 +4,29 @@ import ( "context" "errors" "fmt" + "strconv" + "strings" "github.com/canonical/authd/internal/decorate" "github.com/canonical/authd/internal/services/errmessages" "github.com/canonical/authd/log" "github.com/godbus/dbus/v5" + "github.com/godbus/dbus/v5/introspect" + "golang.org/x/exp/slices" "gopkg.in/ini.v1" ) -// DbusInterface is the expected interface that should be implemented by the brokers. -const DbusInterface string = "com.ubuntu.authd.Broker" +const ( + // DbusBaseInterface is the expected interface that should be implemented by the brokers. + DbusBaseInterface string = "com.ubuntu.authd.Broker" + + // LatestAPIVersion is the latest API version supported by authd. + LatestAPIVersion = 2 +) type dbusBroker struct { - name string + name string + iface string dbusObject dbus.BusObject } @@ -52,10 +62,83 @@ func newDbusBroker(ctx context.Context, bus *dbus.Conn, configFile string) (b db return b, "", "", fmt.Errorf("missing field for broker: %v", err) } - return dbusBroker{ + dBroker := dbusBroker{ name: nameVal.String(), dbusObject: bus.Object(dbusName.String(), dbus.ObjectPath(objectName.String())), - }, nameVal.String(), brandIconVal.String(), nil + } + + dBroker.iface, err = getInterface(dBroker.dbusObject) + if err != nil { + return b, "", "", fmt.Errorf("could not detect broker interfaces: %v", err) + } + + return dBroker, nameVal.String(), brandIconVal.String(), nil +} + +// getInterface introspects the broker's D-Bus object and returns the interface with the highest version supported both +// by the broker and authd. +func getInterface(obj dbus.BusObject) (string, error) { + node, err := introspect.Call(obj) + if err != nil { + return "", fmt.Errorf("could not introspect broker: %v", err) + } + + var supportedInterfaces []string + for _, iface := range node.Interfaces { + // Ignore interfaces that do not satisfy the expected format, as they are not relevant for selecting the broker + // interface version. + // The expected format is com.ubuntu.authd.BrokerX, where X is the version number (or empty for the first + // version). + suffix := strings.TrimPrefix(iface.Name, DbusBaseInterface) + if suffix == iface.Name { + // The interface name doesn't start with com.ubuntu.authd.Broker + continue + } + + // The suffix should be either empty (for the first version) or a number (for later versions). + if suffix != "" { + v, err := strconv.Atoi(suffix) + if err != nil { + continue + } + + if v > LatestAPIVersion { + // The interface version is higher than the latest API version supported by authd, ignore it. + continue + } + } + + supportedInterfaces = append(supportedInterfaces, iface.Name) + } + + slices.SortFunc(supportedInterfaces, func(a, b string) int { + suffixA := strings.TrimPrefix(a, DbusBaseInterface) + suffixB := strings.TrimPrefix(b, DbusBaseInterface) + + // We know from the filtering above that the suffix is either empty (for the base + // interface, which is version 1 per D-Bus API versioning) or a valid number. + versionA := 1 + if suffixA != "" { + versionA, _ = strconv.Atoi(suffixA) + } + versionB := 1 + if suffixB != "" { + versionB, _ = strconv.Atoi(suffixB) + } + + if versionA < versionB { + return -1 + } else if versionA > versionB { + return 1 + } + return 0 + }) + + if len(supportedInterfaces) == 0 { + return "", errors.New("no supported interfaces found") + } + + return supportedInterfaces[len(supportedInterfaces)-1], nil } // NewSession calls the corresponding method on the broker bus and returns the session ID and encryption key. @@ -143,7 +226,7 @@ func (b dbusBroker) UserPreCheck(ctx context.Context, username string) (userinfo // call is an abstraction over dbus calls to ensure we wrap the returned error to an ErrorToDisplay. // All wrapped errors will be logged, but not returned to the UI. func (b dbusBroker) call(ctx context.Context, method string, args ...interface{}) (*dbus.Call, error) { - dbusMethod := DbusInterface + "." + method + dbusMethod := b.iface + "." + method call := b.dbusObject.CallWithContext(ctx, dbusMethod, 0, args...) if err := call.Err; err != nil { var dbusError dbus.Error diff --git a/internal/brokers/dbusbroker_test.go b/internal/brokers/dbusbroker_test.go new file mode 100644 index 0000000000..b7264abcea --- /dev/null +++ b/internal/brokers/dbusbroker_test.go @@ -0,0 +1,161 @@ +package brokers + +import ( + "context" + "encoding/xml" + "fmt" + "testing" + + "github.com/godbus/dbus/v5" + "github.com/godbus/dbus/v5/introspect" + "github.com/stretchr/testify/require" +) + +// mockBusObject implements dbus.BusObject for testing dbusBroker internals. +type mockBusObject struct { + introspectXML string + callErr error + + lastCalledMethod string +} + +func (m *mockBusObject) Call(method string, flags dbus.Flags, args ...interface{}) *dbus.Call { + m.lastCalledMethod = method + if m.callErr != nil { + return &dbus.Call{Err: m.callErr} + } + return &dbus.Call{Body: []interface{}{m.introspectXML}} +} +func (m *mockBusObject) CallWithContext(_ context.Context, method string, flags dbus.Flags, args ...interface{}) *dbus.Call { + return m.Call(method, flags, args...) +} + +// The following methods are not used in tests but are required to satisfy the dbus.BusObject interface. +func (m *mockBusObject) Go(method string, flags dbus.Flags, ch chan *dbus.Call, args ...interface{}) *dbus.Call { + return nil +} +func (m *mockBusObject) GoWithContext(_ context.Context, method string, flags dbus.Flags, ch chan *dbus.Call, args ...interface{}) *dbus.Call { + return nil +} +func (m *mockBusObject) AddMatchSignal(iface, member string, options ...dbus.MatchOption) *dbus.Call { + return nil +} +func (m *mockBusObject) RemoveMatchSignal(iface, member string, options ...dbus.MatchOption) *dbus.Call { + return nil +} +func (m *mockBusObject) GetProperty(p string) (dbus.Variant, error) { + return dbus.Variant{}, nil +} +func (m *mockBusObject) StoreProperty(p string, value interface{}) error { return nil } +func (m *mockBusObject) SetProperty(p string, v interface{}) error { return nil } +func (m *mockBusObject) Destination() string { return "" } +func (m *mockBusObject) Path() dbus.ObjectPath { return "/" } + +// introspectionXML generates introspection XML for the given interface names. +func introspectionXML(interfaces ...string) string { + node := introspect.Node{Name: "/test"} + for _, iface := range interfaces { + node.Interfaces = append(node.Interfaces, introspect.Interface{Name: iface}) + } + data, _ := xml.Marshal(node) + return string(data) +} + +func TestGetInterface(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + interfaces []string + callErr error + + wantInterface string + wantErr bool + }{ + "Single_base_interface": { + interfaces: []string{"com.ubuntu.authd.Broker"}, + wantInterface: "com.ubuntu.authd.Broker", + }, + "Returns_highest_supported_version": { + interfaces: []string{"com.ubuntu.authd.Broker1", "com.ubuntu.authd.Broker2", "com.ubuntu.authd.Broker3"}, + wantInterface: "com.ubuntu.authd.Broker2", + }, + "Versioned_interfaces_with_unversioned": { + interfaces: []string{"com.ubuntu.authd.Broker2", "com.ubuntu.authd.Broker", "com.ubuntu.authd.Broker1"}, + wantInterface: "com.ubuntu.authd.Broker2", + }, + "Unrelated_interfaces_are_excluded": { + interfaces: []string{"com.ubuntu.authd.Broker2", "org.freedesktop.DBus.Introspectable", + "com.ubuntu.authd.Broker1", "com.ubuntu.authd.BrokerUnrelated"}, + wantInterface: "com.ubuntu.authd.Broker2", + }, + "Single_versioned_interface": { + interfaces: []string{"com.ubuntu.authd.Broker1"}, + wantInterface: "com.ubuntu.authd.Broker1", + }, + + "Error_when_no_supported_interfaces": { + interfaces: []string{}, + wantErr: true, + }, + "Error_when_all_interfaces_above_latest_version": { + interfaces: []string{"com.ubuntu.authd.Broker3", "com.ubuntu.authd.Broker4"}, + wantErr: true, + }, + "Error_when_introspect_fails": { + callErr: fmt.Errorf("connection refused"), + wantErr: true, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + mock := &mockBusObject{callErr: tc.callErr} + if tc.callErr == nil { + mock.introspectXML = introspectionXML(tc.interfaces...) + } + + got, err := getInterface(mock) + if tc.wantErr { + require.Error(t, err, "getInterface should return an error, but did not") + return + } + require.NoError(t, err, "getInterface should not return an error, but did") + require.Equal(t, tc.wantInterface, got, "getInterface returned unexpected interface") + }) + } +} + +func TestDbusBrokerCallUsesInterface(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + iface string + wantMethodCalled string + }{ + "Uses_base_interface": { + iface: "com.ubuntu.authd.Broker", + wantMethodCalled: "com.ubuntu.authd.Broker.TestMethod", + }, + "Uses_versioned_interface": { + iface: "com.ubuntu.authd.Broker2", + wantMethodCalled: "com.ubuntu.authd.Broker2.TestMethod", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + mock := &mockBusObject{} + b := dbusBroker{ + name: "test", + iface: tc.iface, + dbusObject: mock, + } + + _, err := b.call(context.Background(), "TestMethod") + require.NoError(t, err, "call should not return a D-Bus error") + require.Equal(t, tc.wantMethodCalled, mock.lastCalledMethod) + }) + } +} diff --git a/internal/brokers/manager_test.go b/internal/brokers/manager_test.go index fa916689d4..fe1a114432 100644 --- a/internal/brokers/manager_test.go +++ b/internal/brokers/manager_test.go @@ -34,10 +34,10 @@ func TestNewManager(t *testing.T) { "Creates_only_correct_brokers_when_config_dir_has_valid_and_invalid_brokers": {brokerConfigDir: "mixed_brokers"}, "Creates_only_local_broker_when_config_dir_has_only_invalid_ones": {brokerConfigDir: "invalid_brokers"}, "Creates_only_local_broker_when_config_dir_does_not_exist": {brokerConfigDir: "does/not/exist"}, - "Creates_manager_even_if_broker_is_not_exported_on_dbus": {brokerConfigDir: "not_on_bus"}, "Ignores_broker_configuration_file_not_ending_with_.conf": {brokerConfigDir: "some_ignored_brokers"}, "Ignores_any_unknown_sections_and_fields": {brokerConfigDir: "extra_fields"}, + "Ignores_brokers_not_available_on_dbus": {brokerConfigDir: "not_on_bus"}, "Error_when_can't_connect_to_system_bus": {brokerConfigDir: "valid_brokers", noBus: true, wantErr: true}, "Error_when_broker_config_dir_is_a_file": {brokerConfigDir: "file_config_dir", wantErr: true}, @@ -177,7 +177,6 @@ func TestNewSession(t *testing.T) { sessionMode string configuredBrokers []string - unavailableBroker bool wantErr bool }{ @@ -185,10 +184,9 @@ func TestNewSession(t *testing.T) { "Successfully_start_a_new_passwd_session": {username: "success", sessionMode: auth.SessionModeChangePassword}, "Successfully_start_a_new_session_with_the_correct_broker": {username: "success", configuredBrokers: []string{t.Name() + "_Broker1.conf", t.Name() + "_Broker2.conf"}}, - "Error_when_broker_does_not_exist": {brokerID: "does_not_exist", wantErr: true}, - "Error_when_broker_does_not_provide_an_ID": {username: "ns_no_id", wantErr: true}, - "Error_when_starting_a_new_session": {username: "ns_error", wantErr: true}, - "Error_when_broker_is_not_available_on_dbus": {unavailableBroker: true, wantErr: true}, + "Error_when_broker_does_not_exist": {brokerID: "does_not_exist", wantErr: true}, + "Error_when_broker_does_not_provide_an_ID": {username: "ns_no_id", wantErr: true}, + "Error_when_starting_a_new_session": {username: "ns_error", wantErr: true}, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -206,16 +204,6 @@ func TestNewSession(t *testing.T) { } } - if tc.unavailableBroker { - // We need to manually configure the broker without exporting it on the bus. - content, err := os.ReadFile(filepath.Join(brokerConfFixtures, "not_on_bus", "not_on_bus.conf")) - require.NoError(t, err, "Setup: could not read broker configuration file") - err = os.WriteFile(filepath.Join(brokersConfPath, "not_on_bus.conf"), content, 0600) - require.NoError(t, err, "Setup: could not write broker configuration file") - wantBroker = brokers.Broker{Name: "OfflineBroker"} - tc.configuredBrokers = nil - } - m, err := brokers.NewManager(context.Background(), brokersConfPath, tc.configuredBrokers) require.NoError(t, err, "Setup: could not create manager") @@ -398,5 +386,22 @@ func TestMain(m *testing.M) { } defer cleanup() + tmpDir, err := os.MkdirTemp("", "authd-test-") + if err != nil { + fmt.Fprintf(os.Stderr, "could not create temporary directory for tests: %v\n", err) + os.Exit(1) + } + defer os.RemoveAll(tmpDir) + + // Export mock brokers matching the names used in testdata config fixtures. + for _, name := range []string{"Broker", "Broker2"} { + _, cleanup, err := testutils.StartBusBrokerMock(tmpDir, name) + if err != nil { + fmt.Fprintf(os.Stderr, "%v\n", err) + os.Exit(1) + } + defer cleanup() + } + m.Run() } diff --git a/internal/brokers/testdata/golden/TestNewManager/Creates_manager_even_if_broker_is_not_exported_on_dbus b/internal/brokers/testdata/golden/TestNewManager/Creates_manager_even_if_broker_is_not_exported_on_dbus deleted file mode 100644 index a1d67c1422..0000000000 --- a/internal/brokers/testdata/golden/TestNewManager/Creates_manager_even_if_broker_is_not_exported_on_dbus +++ /dev/null @@ -1,2 +0,0 @@ -- local -- OfflineBroker diff --git a/internal/brokers/testdata/golden/TestNewManager/Ignores_brokers_not_available_on_dbus b/internal/brokers/testdata/golden/TestNewManager/Ignores_brokers_not_available_on_dbus new file mode 100644 index 0000000000..70420ab2b5 --- /dev/null +++ b/internal/brokers/testdata/golden/TestNewManager/Ignores_brokers_not_available_on_dbus @@ -0,0 +1 @@ +- local