Support versioning in Broker API and authd

Adds a com.ubuntu.authd.Broker2 D-Bus interface in accordance with
https://dbus.freedesktop.org/doc/dbus-api-design.html#api-versioning

Also authd now properly considers the possibility of multiple interfaces
and, for now, chooses the latest one available.

Author: adombeck

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
 	}

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

@@ -35,6 +35,10 @@ 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.
+	LatestAPIVersion = 2
+
 	maxAuthAttempts    = 3
 	maxRequestDuration = 5 * time.Second
 )
@@ -49,7 +53,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 +103,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 +151,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,

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

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
 }

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

@@ -11,8 +11,9 @@ import (
 	"github.com/godbus/dbus/v5/introspect"
 )
 
-const intro = `
-<node>
+const (
+	introspectableHeader    = `<node>`
+	introspectableInterface = `
 	<interface name="%s">
 		<method name="NewSession">
 			<arg type="s" direction="in" name="username"/>
@@ -50,51 +51,86 @@ const intro = `
 		<method name="DeleteUser">
 			<arg type="s" direction="in" name="username"/>
 		</method>
-	</interface>` + introspect.IntrospectDataString + `</node> `
+	</interface>`
+	introspectableFooter = introspect.IntrospectDataString + `</node> `
+)
 
-// 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
 }

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"
 )
 

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)