diff --git a/authd-oidc-brokers/internal/broker/broker.go b/authd-oidc-brokers/internal/broker/broker.go
index 82d00a1217..78317a1ad3 100644
--- a/authd-oidc-brokers/internal/broker/broker.go
+++ b/authd-oidc-brokers/internal/broker/broker.go
@@ -11,6 +11,7 @@ import (
"errors"
"fmt"
"net"
+ "os"
"path/filepath"
"slices"
"strings"
@@ -155,6 +156,45 @@ func New(cfg Config, args ...Option) (b *Broker, err error) {
return b, nil
}
+// normalizedIssuer converts an issuer URL into a filesystem-safe directory name
+// by stripping the scheme and replacing path/port separators with underscores.
+func normalizedIssuer(issuerURL string) string {
+ _, issuer, found := strings.Cut(issuerURL, "://")
+ if !found {
+ // If the issuer URL does not contain a scheme, use the whole issuer URL as the issuer.
+ issuer = issuerURL
+ }
+ issuer = strings.ReplaceAll(issuer, "/", "_")
+ issuer = strings.ReplaceAll(issuer, ":", "_")
+
+ return issuer
+}
+
+// userDataDir returns the path to the broker's data directory for the given user.
+// If the issuer URL or the username contains path traversal characters, an error is returned.
+func (b *Broker) userDataDir(username string) (string, error) {
+ if username == "" {
+ return "", errors.New("username cannot be empty")
+ }
+
+ issuer := normalizedIssuer(b.cfg.issuerURL)
+ issuerDataDir := filepath.Join(b.cfg.DataDir, issuer)
+ // Check that the issuer does not contain path traversal characters by verifying that the resulting path is within
+ // the data directory and the basename matches the issuer.
+ if !strings.HasPrefix(issuerDataDir, b.cfg.DataDir) || filepath.Base(issuerDataDir) != issuer {
+ return "", fmt.Errorf("invalid issuer URL %q: path traversal detected", b.cfg.issuerURL)
+ }
+
+ dir := filepath.Join(issuerDataDir, username)
+ // Check that the username does not contain path traversal characters by verifying that the resulting path is within
+ // the issuer data directory and the basename matches the username.
+ if !strings.HasPrefix(dir, issuerDataDir) || filepath.Base(dir) != username {
+ return "", fmt.Errorf("invalid username %q: path traversal detected", username)
+ }
+
+ return dir, nil
+}
+
// NewSession creates a new session for the user.
func (b *Broker) NewSession(username, lang, mode string) (sessionID, encryptionKey string, err error) {
if username == "" {
@@ -175,27 +215,11 @@ func (b *Broker) NewSession(username, lang, mode string) (sessionID, encryptionK
return "", "", fmt.Errorf("failed to marshal broker public key: %v", err)
}
- _, issuer, found := strings.Cut(b.cfg.issuerURL, "://")
- if !found {
- // If the issuer URL does not contain a scheme, use the whole issuer URL as the issuer.
- issuer = b.cfg.issuerURL
- }
- issuer = strings.ReplaceAll(issuer, "/", "_")
- issuer = strings.ReplaceAll(issuer, ":", "_")
-
- issuerDataDir := filepath.Join(b.cfg.DataDir, issuer)
- // Check that the issuer does not contain path traversal characters by verifying that the resulting path is within
- // the data directory and the basename matches the issuer.
- if !strings.HasPrefix(issuerDataDir, b.cfg.DataDir) || filepath.Base(issuerDataDir) != issuer {
- return "", "", fmt.Errorf("invalid issuer URL %q: path traversal detected", b.cfg.issuerURL)
+ s.userDataDir, err = b.userDataDir(username)
+ if err != nil {
+ return "", "", err
}
- s.userDataDir = filepath.Join(issuerDataDir, username)
- // Check that the username does not contain path traversal characters by verifying that the resulting path is within
- // the issuer data directory and the basename matches the username.
- if !strings.HasPrefix(s.userDataDir, issuerDataDir) || filepath.Base(s.userDataDir) != username {
- return "", "", fmt.Errorf("invalid username %q: path traversal detected", username)
- }
// The token is stored in $DATA_DIR/$ISSUER/$USERNAME/token.json.
s.tokenPath = filepath.Join(s.userDataDir, "token.json")
// The password is stored in $DATA_DIR/$ISSUER/$USERNAME/password.
@@ -1032,6 +1056,21 @@ func (b *Broker) CancelIsAuthenticated(sessionID string) {
}
}
+// DeleteUser removes all broker side data stored for the given user
+// from the broker's data directory.
+func (b *Broker) DeleteUser(username string) error {
+ userDataDir, err := b.userDataDir(username)
+ if err != nil {
+ return err
+ }
+
+ if err := os.RemoveAll(userDataDir); err != nil {
+ return fmt.Errorf("could not remove user data directory %q: %w", userDataDir, err)
+ }
+ log.Infof(context.Background(), "Deleted broker data for user %q at %q", username, userDataDir)
+ return nil
+}
+
// UserPreCheck checks if the user is valid and can be allowed to authenticate.
// It returns the user info in JSON format if the user is valid, or an empty string if the user is not allowed.
func (b *Broker) UserPreCheck(username string) (string, error) {
diff --git a/authd-oidc-brokers/internal/broker/broker_test.go b/authd-oidc-brokers/internal/broker/broker_test.go
index 4da2301597..21908b4ec1 100644
--- a/authd-oidc-brokers/internal/broker/broker_test.go
+++ b/authd-oidc-brokers/internal/broker/broker_test.go
@@ -121,18 +121,10 @@ func TestNewSession(t *testing.T) {
emptyUsername: true,
wantErr: true,
},
- "Error_when_username_contains_path_traversal": {
- username: "../test",
- wantErr: true,
- },
- "Error_when_username_contains_path_traversal_but_does_not_leave_the_parent_directory": {
- username: "test/../other-user",
+ "Error_when_user_directory_path_could_not_be_derived": {
+ username: "invalid/../user",
wantErr: true,
},
- "Error_when_issuer_contains_path_traversal": {
- issuerURL: "https://..",
- wantErr: true,
- },
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
@@ -1446,6 +1438,158 @@ func TestUserPreCheck(t *testing.T) {
}
}
+func TestNormalizedIssuer(t *testing.T) {
+ t.Parallel()
+
+ tests := map[string]struct {
+ issuerURL string
+ want string
+ }{
+ "HTTP_issuerURL": {issuerURL: "http://example.com", want: "example.com"},
+ "HTTPS_issuerURL": {issuerURL: "https://example.com", want: "example.com"},
+ "IssuerURL_with_path": {issuerURL: "https://example.com/tenant/v2.0", want: "example.com_tenant_v2.0"},
+ "IssuerURL_with_port": {issuerURL: "https://example.com:8080", want: "example.com_8080"},
+ "IssuerURL_with_port_and_path": {issuerURL: "https://example.com:8080/path", want: "example.com_8080_path"},
+ "IssuerURL_with_IP_address": {issuerURL: "https://127.0.0.1", want: "127.0.0.1"},
+ "IssuerURL_with_IP_address_and_port": {issuerURL: "https://127.0.0.1:8080", want: "127.0.0.1_8080"},
+ "IssuerURL_without_scheme": {issuerURL: "example.com", want: "example.com"},
+ }
+
+ for name, tc := range tests {
+ t.Run(name, func(t *testing.T) {
+ t.Parallel()
+
+ b := newBrokerForTests(t, &brokerForTestConfig{
+ issuerURL: tc.issuerURL,
+ })
+
+ got := b.NormalizedIssuer(tc.issuerURL)
+ require.Equal(t, tc.want, got, "NormalizedIssuer returned unexpected result")
+ })
+ }
+}
+
+func TestUserDataDir(t *testing.T) {
+ t.Parallel()
+
+ tests := map[string]struct {
+ issuerURL string
+ username string
+ want string
+ wantErr bool
+ }{
+ "Successfully_return_user_data_dir_for_simple_username_and_issuer": {
+ issuerURL: "https://example.com",
+ username: "user@example.com",
+ want: "example.com/user@example.com",
+ },
+ "Successfully_return_user_data_dir_for_issuer_url_without_scheme": {
+ issuerURL: "example.com",
+ username: "user@example.com",
+ want: "example.com/user@example.com",
+ },
+ "Error_when_username_is_empty": {
+ issuerURL: "https://example.com",
+ username: "",
+ wantErr: true,
+ },
+ "Error_when_username_contains_path_traversal": {
+ issuerURL: "https://example.com",
+ username: "../test",
+ wantErr: true,
+ },
+ "Error_when_username_contains_path_traversal_but_does_not_leave_the_parent_directory": {
+ issuerURL: "https://example.com",
+ username: "test/../other-user",
+ wantErr: true,
+ },
+ "Error_when_issuer_contains_path_traversal": {
+ issuerURL: "https://..",
+ username: "validuser",
+ wantErr: true,
+ },
+ }
+ for name, tc := range tests {
+ t.Run(name, func(t *testing.T) {
+ t.Parallel()
+
+ b := newBrokerForTests(t, &brokerForTestConfig{
+ issuerURL: tc.issuerURL,
+ })
+
+ got, err := b.UserDataDir(tc.username)
+ if tc.wantErr {
+ require.Error(t, err, "UserDataDir should return an error, but did not")
+ return
+ }
+ require.NoError(t, err, "UserDataDir should not return an error")
+ require.Equal(t, filepath.Join(b.DataDir(), tc.want), got, "UserDataDir returned unexpected result")
+ })
+ }
+}
+
+func TestDeleteUser(t *testing.T) {
+ t.Parallel()
+
+ tests := map[string]struct {
+ username string
+ createUserDir bool
+ readOnlyDataDir bool
+
+ wantErr bool
+ }{
+ "Successfully_delete_existing_user": {username: "user@example.com", createUserDir: true},
+ "Successfully_delete_unknown_user_is_noop": {username: "unknown@example.com"},
+
+ "Error_when_user_data_dir_cannot_be_removed": {username: "user@example.com", createUserDir: true, readOnlyDataDir: true, wantErr: true},
+ "Error_when_userDataDir_could_not_be_determined": {username: "", wantErr: true},
+ }
+ for name, tc := range tests {
+ t.Run(name, func(t *testing.T) {
+ t.Parallel()
+
+ b := newBrokerForTests(t, &brokerForTestConfig{
+ issuerURL: defaultIssuerURL,
+ })
+
+ // Derive the path where DeleteUser will look for the user's data
+ userDataDir, err := b.UserDataDir(tc.username)
+ if tc.username == "" {
+ require.Error(t, err, "Setup: UserDataDir should have returned an error for empty username")
+ return
+ }
+ require.NoError(t, err, "Setup: UserDataDir should not have returned an error for valid username")
+
+ if tc.createUserDir {
+ err := os.MkdirAll(userDataDir, 0700)
+ require.NoError(t, err, "Setup: could not create user data directory")
+
+ // Write a dummy token file so the directory is non-empty
+ err = os.WriteFile(filepath.Join(userDataDir, "token.json"), []byte(`{}`), 0600)
+ require.NoError(t, err, "Setup: could not write dummy token file")
+ }
+
+ if tc.readOnlyDataDir {
+ // Make the issuer directory read-only so RemoveAll fails on the user subdir
+ issuerDir := filepath.Dir(userDataDir)
+ err := os.Chmod(issuerDir, 0500) //nolint:gosec // Intentional read-only permission for testing
+ require.NoError(t, err, "Setup: could not make issuer directory read-only")
+ t.Cleanup(func() { _ = os.Chmod(issuerDir, 0700) }) //nolint:gosec // Restore full permissions after test
+ }
+
+ err = b.DeleteUser(tc.username)
+ if tc.wantErr {
+ require.Error(t, err, "DeleteUser should return an error, but did not")
+ return
+ }
+ require.NoError(t, err, "DeleteUser should not return an error, but did")
+
+ // Verify the user data directory no longer exists
+ require.NoDirExists(t, userDataDir, "User data directory should have been removed")
+ })
+ }
+}
+
func TestMain(m *testing.M) {
log.SetLevel(log.DebugLevel)
diff --git a/authd-oidc-brokers/internal/broker/export_test.go b/authd-oidc-brokers/internal/broker/export_test.go
index dbc4c802ce..f7cf651507 100644
--- a/authd-oidc-brokers/internal/broker/export_test.go
+++ b/authd-oidc-brokers/internal/broker/export_test.go
@@ -125,6 +125,16 @@ func (b *Broker) DataDir() string {
return b.cfg.DataDir
}
+// UserDataDir exposes the broker's userDataDir method for tests.
+func (b *Broker) UserDataDir(username string) (string, error) {
+ return b.userDataDir(username)
+}
+
+// NormalizedIssuer exposes the broker's normalizedIssuer method for tests.
+func (b *Broker) NormalizedIssuer(issuerURL string) string {
+ return normalizedIssuer(issuerURL)
+}
+
// GetNextAuthModes returns the next auth mode of the specified session.
func (b *Broker) GetNextAuthModes(sessionID string) []string {
b.currentSessionsMu.Lock()
diff --git a/authd-oidc-brokers/internal/dbusservice/dbusservice.go b/authd-oidc-brokers/internal/dbusservice/dbusservice.go
index d9b1c52648..7016ec6e38 100644
--- a/authd-oidc-brokers/internal/dbusservice/dbusservice.go
+++ b/authd-oidc-brokers/internal/dbusservice/dbusservice.go
@@ -47,6 +47,9 @@ const intro = `
+
+
+
` + introspect.IntrospectDataString + ` `
// Service is the handler exposing our broker methods on the system bus.
diff --git a/authd-oidc-brokers/internal/dbusservice/methods.go b/authd-oidc-brokers/internal/dbusservice/methods.go
index a26b490a06..b9d71c44cf 100644
--- a/authd-oidc-brokers/internal/dbusservice/methods.go
+++ b/authd-oidc-brokers/internal/dbusservice/methods.go
@@ -86,6 +86,15 @@ func (s *Service) UserPreCheck(username string) (userinfo string, dbusErr *dbus.
return userinfo, nil
}
+// 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) {
+ log.Debugf(context.Background(), "DeleteUser: %s", username)
+ if err := s.broker.DeleteUser(username); err != nil {
+ return dbus.MakeFailedError(err)
+ }
+ return nil
+}
+
// makeCanceledError creates a dbus.Error for a canceled operation.
func makeCanceledError() *dbus.Error {
return &dbus.Error{Name: "com.ubuntu.authd.Canceled"}
diff --git a/examplebroker/broker.go b/examplebroker/broker.go
index 517b7ac279..4c3bd5e1e1 100644
--- a/examplebroker/broker.go
+++ b/examplebroker/broker.go
@@ -877,6 +877,21 @@ func (b *Broker) UserPreCheck(ctx context.Context, username string) (string, err
return userInfoFromName(username), nil
}
+// DeleteUser removes any broker side data associated with the user.
+func (b *Broker) DeleteUser(ctx context.Context, username string) error {
+ exampleUsersMu.Lock()
+ defer exampleUsersMu.Unlock()
+
+ if _, exists := exampleUsers[username]; !exists {
+ // Nothing to delete.
+ return nil
+ }
+
+ delete(exampleUsers, username)
+ log.Infof(ctx, "Broker: deleted user data for %q", username)
+ return nil
+}
+
// decryptAES is just here to illustrate the encryption and decryption
// and in no way the right way to perform a secure encryption
//
diff --git a/examplebroker/com.ubuntu.auth.ExampleBroker.xml b/examplebroker/com.ubuntu.auth.ExampleBroker.xml
index 86b1d78ed7..82c702809d 100644
--- a/examplebroker/com.ubuntu.auth.ExampleBroker.xml
+++ b/examplebroker/com.ubuntu.auth.ExampleBroker.xml
@@ -41,6 +41,9 @@
+
+
+
diff --git a/examplebroker/dbus.go b/examplebroker/dbus.go
index e9517d6cb9..e18342e06f 100644
--- a/examplebroker/dbus.go
+++ b/examplebroker/dbus.go
@@ -134,3 +134,11 @@ func (b *Bus) UserPreCheck(username string) (userinfo string, dbusErr *dbus.Erro
}
return userinfo, nil
}
+
+// DeleteUser is the method through which the broker and the daemon will communicate once dbusInterface.DeleteUser is called.
+func (b *Bus) DeleteUser(username string) (dbusErr *dbus.Error) {
+ if err := b.broker.DeleteUser(context.Background(), username); err != nil {
+ return dbus.MakeFailedError(err)
+ }
+ return nil
+}