From 39dfb1817f1e5967b9a315f844d1071758b545a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Tue, 5 Nov 2024 20:50:55 +0100 Subject: [PATCH] golang-ci: Enable lll (long lines) linter and fix issues Set the long lines linter to block lines longer than 120 chars an fix the cases in which we were not respecting this limit. This was somewhat mentioned during the sprint, and I wanted to finally tackle it :) --- .golangci.yaml | 5 + cmd/authd/daemon/config.go | 6 +- cmd/authd/daemon/daemon.go | 12 +- cmd/authd/daemon/version.go | 5 +- examplebroker/broker.go | 32 ++- examplebroker/dbus.go | 27 ++- internal/brokers/broker.go | 36 ++-- internal/brokers/broker_test.go | 118 +++++++---- internal/brokers/dbusbroker.go | 27 ++- internal/brokers/export_test.go | 3 +- internal/brokers/localbroker.go | 44 ++-- internal/brokers/manager.go | 9 +- internal/brokers/manager_test.go | 62 ++++-- internal/daemon/daemon.go | 3 +- internal/daemon/daemon_test.go | 43 ++-- .../services/errmessages/internal_test.go | 9 +- internal/services/errmessages/redactor.go | 9 +- internal/services/manager.go | 8 +- internal/services/manager_test.go | 7 +- internal/services/nss/nss.go | 3 +- internal/services/nss/nss_test.go | 137 +++++++++---- internal/services/pam/pam.go | 18 +- internal/services/pam/pam_test.go | 194 +++++++++++++----- internal/services/permissions.go | 3 +- internal/services/permissions/servercreds.go | 6 +- .../testsdetection/testsdetection_test.go | 13 +- internal/testutils/broker.go | 21 +- internal/testutils/daemon.go | 14 +- internal/testutils/dbus.go | 7 +- internal/users/cache/db_test.go | 147 +++++++++---- internal/users/cache/update.go | 27 ++- internal/users/internal_test.go | 3 +- internal/users/localgroups/localgroups.go | 3 +- .../users/localgroups/localgroups_test.go | 103 +++++++--- .../localgroups/testutils/localgroups.go | 3 +- internal/users/manager.go | 13 +- internal/users/manager_test.go | 147 ++++++++++--- nss/integration-tests/helper_test.go | 3 +- nss/integration-tests/integration_test.go | 92 ++++++--- pam/integration-tests/exec_test.go | 13 +- pam/integration-tests/gdm_test.go | 4 +- pam/integration-tests/helpers_test.go | 4 +- pam/integration-tests/modulehelpers_test.go | 12 +- pam/integration-tests/vhs-helpers_test.go | 3 +- pam/internal/adapter/authentication.go | 11 +- pam/internal/adapter/authmodeselection.go | 16 +- pam/internal/adapter/brokerselection.go | 3 +- pam/internal/adapter/commands.go | 3 +- pam/internal/adapter/gdmmodel_test.go | 53 +++-- pam/internal/adapter/model.go | 3 +- pam/internal/adapter/nativemodel.go | 3 +- pam/internal/dbusmodule/transaction.go | 3 +- pam/internal/gdm/protocol_test.go | 22 +- pam/internal/pam_test/pam-client-dummy.go | 24 ++- .../pam_test/pam-client-dummy_test.go | 3 +- pam/internal/pam_test/runner-utils.go | 6 +- pam/internal/pam_test/service-utils_test.go | 1 + pam/pam.go | 5 +- 58 files changed, 1149 insertions(+), 465 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index dba5860e1..1749d8892 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -15,6 +15,7 @@ linters: - godot - gofmt - gosec + - lll - misspell - nakedret - nolintlint @@ -61,3 +62,7 @@ linters-settings: # Never have naked return ever nakedret: max-func-lines: 1 + + lll: + line-length: 120 + tab-width: 4 diff --git a/cmd/authd/daemon/config.go b/cmd/authd/daemon/config.go index 6bacb36fb..35c78d88e 100644 --- a/cmd/authd/daemon/config.go +++ b/cmd/authd/daemon/config.go @@ -40,7 +40,8 @@ func initViperConfig(name string, cmd *cobra.Command, vip *viper.Viper) (err err vip.AddConfigPath("/etc/authd/") // Add the executable path to the config search path. if binPath, err := os.Executable(); err != nil { - log.Warningf(context.Background(), "Failed to get current executable path, not adding it as a config dir: %v", err) + log.Warningf(context.Background(), + "Failed to get current executable path, not adding it as a config dir: %v", err) } else { vip.AddConfigPath(filepath.Dir(binPath)) } @@ -49,7 +50,8 @@ func initViperConfig(name string, cmd *cobra.Command, vip *viper.Viper) (err err if err := vip.ReadInConfig(); err != nil { var e viper.ConfigFileNotFoundError if errors.As(err, &e) { - log.Infof(context.Background(), "No configuration file: %v.\nWe will only use the defaults, env variables or flags.", e) + log.Infof(context.Background(), + "No configuration file: %v.\nWe will only use the defaults, env variables or flags.", e) } else { return fmt.Errorf("invalid configuration file: %w", err) } diff --git a/cmd/authd/daemon/daemon.go b/cmd/authd/daemon/daemon.go index 5f429b0e0..e752f40a4 100644 --- a/cmd/authd/daemon/daemon.go +++ b/cmd/authd/daemon/daemon.go @@ -49,7 +49,8 @@ type daemonConfig struct { func New() *App { a := App{ready: make(chan struct{})} a.rootCmd = cobra.Command{ - Use: fmt.Sprintf("%s COMMAND", cmdName), + Use: fmt.Sprintf("%s COMMAND", cmdName), + Short:/*i18n.G(*/ "Authentication daemon", /*)*/ Long:/*i18n.G(*/ "Authentication daemon bridging the system with external brokers.", /*)*/ Args: cobra.NoArgs, @@ -138,7 +139,8 @@ func (a *App) serve(config daemonConfig) error { // installVerbosityFlag adds the -v and -vv options and returns the reference to it. func installVerbosityFlag(cmd *cobra.Command, viper *viper.Viper) *int { - r := cmd.PersistentFlags().CountP("verbosity", "v" /*i18n.G(*/, "issue INFO (-v), DEBUG (-vv) or DEBUG with caller (-vvv) output") //) + r := cmd.PersistentFlags().CountP("verbosity", "v", /*i18n.G(*/ + "issue INFO (-v), DEBUG (-vv) or DEBUG with caller (-vvv) output") //) decorate.LogOnError(viper.BindPFlag("verbosity", cmd.PersistentFlags().Lookup("verbosity"))) return r } @@ -171,12 +173,14 @@ func (a *App) Quit() { } // WaitReady signals when the daemon is ready -// Note: we need to use a pointer to not copy the App object before the daemon is ready, and thus, creates a data race. +// Note: we need to use a pointer to not copy the App object before the daemon is ready, +// and thus, creates a data race. func (a *App) WaitReady() { <-a.ready } -// RootCmd returns a copy of the root command for the app. Shouldn't be in general necessary apart when running generators. +// RootCmd returns a copy of the root command for the app. +// Shouldn't be in general necessary apart when running generators. func (a App) RootCmd() cobra.Command { return a.rootCmd } diff --git a/cmd/authd/daemon/version.go b/cmd/authd/daemon/version.go index 60507ff37..3b57a785a 100644 --- a/cmd/authd/daemon/version.go +++ b/cmd/authd/daemon/version.go @@ -11,8 +11,9 @@ func (a *App) installVersion() { cmd := &cobra.Command{ Use: "version", Short:/*i18n.G(*/ "Returns version of daemon and exits", /*)*/ - Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, args []string) error { return getVersion() }, + + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { return getVersion() }, } a.rootCmd.AddCommand(cmd) } diff --git a/examplebroker/broker.go b/examplebroker/broker.go index f6479c77b..892397760 100644 --- a/examplebroker/broker.go +++ b/examplebroker/broker.go @@ -145,7 +145,8 @@ func New(name string) (b *Broker, fullName, brandIcon string) { } // NewSession creates a new session for the specified user. -func (b *Broker) NewSession(ctx context.Context, username, lang, mode string) (sessionID, encryptionKey string, err error) { +func (b *Broker) NewSession(ctx context.Context, username, lang, mode string) ( + sessionID, encryptionKey string, err error) { sessionID = uuid.New().String() info := sessionInfo{ username: username, @@ -214,8 +215,10 @@ func (b *Broker) NewSession(ctx context.Context, username, lang, mode string) (s return sessionID, base64.StdEncoding.EncodeToString(pubASN1), nil } -// GetAuthenticationModes returns the list of supported authentication modes for the selected broker depending on session info. -func (b *Broker) GetAuthenticationModes(ctx context.Context, sessionID string, supportedUILayouts []map[string]string) (authenticationModes []map[string]string, err error) { +// GetAuthenticationModes returns the list of supported authentication modes +// for the selected broker depending on session info. +func (b *Broker) GetAuthenticationModes(ctx context.Context, sessionID string, supportedUILayouts []map[string]string) ( + authenticationModes []map[string]string, err error) { sessionInfo, err := b.sessionInfo(sessionID) if err != nil { return nil, err @@ -312,8 +315,9 @@ func getSupportedModes(sessionInfo sessionInfo, supportedUILayouts []map[string] "selection_label": fmt.Sprintf("Send URL to %s@gmail.com", sessionInfo.username), "email": fmt.Sprintf("%s@gmail.com", sessionInfo.username), "ui": mapToJSON(map[string]string{ - "type": "form", - "label": fmt.Sprintf("Click on the link received at %s@gmail.com or enter the code:", sessionInfo.username), + "type": "form", + "label": fmt.Sprintf("Click on the link received at %s@gmail.com or enter the code:", + sessionInfo.username), "entry": "chars", "wait": "true", }), @@ -463,7 +467,8 @@ func qrcodeData(sessionInfo *sessionInfo) (content string, code string) { } // SelectAuthenticationMode returns the UI layout information for the selected authentication mode. -func (b *Broker) SelectAuthenticationMode(ctx context.Context, sessionID, authenticationModeName string) (uiLayoutInfo map[string]string, err error) { +func (b *Broker) SelectAuthenticationMode(ctx context.Context, sessionID, authenticationModeName string) ( + uiLayoutInfo map[string]string, err error) { // Ensure session ID is an active one. sessionInfo, err := b.sessionInfo(sessionID) if err != nil { @@ -513,7 +518,8 @@ func (b *Broker) SelectAuthenticationMode(ctx context.Context, sessionID, authen } // IsAuthenticated evaluates the provided authenticationData and returns the authentication status for the user. -func (b *Broker) IsAuthenticated(ctx context.Context, sessionID, authenticationData string) (access, data string, err error) { +func (b *Broker) IsAuthenticated(ctx context.Context, sessionID, authenticationData string) ( + access, data string, err error) { sessionInfo, err := b.sessionInfo(sessionID) if err != nil { return "", "", err @@ -574,7 +580,8 @@ func (b *Broker) sleepDuration(in time.Duration) time.Duration { return time.Duration(math.Round(float64(in) * b.sleepMultiplier)) } -func (b *Broker) handleIsAuthenticated(ctx context.Context, sessionInfo sessionInfo, authData map[string]string) (access, data string) { +func (b *Broker) handleIsAuthenticated(ctx context.Context, sessionInfo sessionInfo, authData map[string]string) ( + access, data string) { // Decrypt challenge if present. challenge, err := decodeRawChallenge(b.privateKey, authData["challenge"]) if err != nil { @@ -597,7 +604,8 @@ func (b *Broker) handleIsAuthenticated(ctx context.Context, sessionInfo sessionI expectedChallenge := user.Password if challenge != expectedChallenge { - return AuthRetry, fmt.Sprintf(`{"message": "invalid password '%s', should be '%s'"}`, challenge, expectedChallenge) + return AuthRetry, fmt.Sprintf(`{"message": "invalid password '%s', should be '%s'"}`, + challenge, expectedChallenge) } case "pincode": @@ -650,7 +658,8 @@ func (b *Broker) handleIsAuthenticated(ctx context.Context, sessionInfo sessionI case "qrcodewithtypo", "qrcodeandcodewithtypo": if authData["wait"] != "true" { - return AuthDenied, fmt.Sprintf(`{"message": "%s should have wait set to true"}`, sessionInfo.currentAuthMode) + return AuthDenied, fmt.Sprintf(`{"message": "%s should have wait set to true"}`, + sessionInfo.currentAuthMode) } // Simulate connexion with remote server to check that the correct code was entered select { @@ -673,7 +682,8 @@ func (b *Broker) handleIsAuthenticated(ctx context.Context, sessionInfo sessionI } if challenge != expectedChallenge { - return AuthRetry, fmt.Sprintf(`{"message": "new password does not match criteria: must be '%s'"}`, expectedChallenge) + return AuthRetry, fmt.Sprintf(`{"message": "new password does not match criteria: must be '%s'"}`, + expectedChallenge) } exampleUsersMu.Lock() exampleUsers[sessionInfo.username] = userInfoBroker{Password: challenge} diff --git a/examplebroker/dbus.go b/examplebroker/dbus.go index 76df614ac..d1100cc68 100644 --- a/examplebroker/dbus.go +++ b/examplebroker/dbus.go @@ -75,7 +75,8 @@ dbus_object = %s return conn, nil } -// NewSession is the method through which the broker and the daemon will communicate once dbusInterface.NewSession is called. +// NewSession is the method through which the broker and the daemon will communicate once +// dbusInterface.NewSession is called. func (b *Bus) NewSession(username, lang, mode string) (sessionID, encryptionKey string, dbusErr *dbus.Error) { sessionID, encryptionKey, err := b.broker.NewSession(context.Background(), username, lang, mode) if err != nil { @@ -84,8 +85,10 @@ func (b *Bus) NewSession(username, lang, mode string) (sessionID, encryptionKey return sessionID, encryptionKey, nil } -// GetAuthenticationModes is the method through which the broker and the daemon will communicate once dbusInterface.GetAuthenticationModes is called. -func (b *Bus) GetAuthenticationModes(sessionID string, supportedUILayouts []map[string]string) (authenticationModes []map[string]string, dbusErr *dbus.Error) { +// GetAuthenticationModes is the method through which the broker and the daemon will communicate once +// dbusInterface.GetAuthenticationModes is called. +func (b *Bus) GetAuthenticationModes(sessionID string, supportedUILayouts []map[string]string) ( + authenticationModes []map[string]string, dbusErr *dbus.Error) { authenticationModes, err := b.broker.GetAuthenticationModes(context.Background(), sessionID, supportedUILayouts) if err != nil { return nil, dbus.MakeFailedError(err) @@ -93,8 +96,10 @@ func (b *Bus) GetAuthenticationModes(sessionID string, supportedUILayouts []map[ return authenticationModes, nil } -// SelectAuthenticationMode is the method through which the broker and the daemon will communicate once dbusInterface.SelectAuthenticationMode is called. -func (b *Bus) SelectAuthenticationMode(sessionID, authenticationModeName string) (uiLayoutInfo map[string]string, dbusErr *dbus.Error) { +// SelectAuthenticationMode is the method through which the broker and the daemon will communicate once +// dbusInterface.SelectAuthenticationMode is called. +func (b *Bus) SelectAuthenticationMode(sessionID, authenticationModeName string) ( + uiLayoutInfo map[string]string, dbusErr *dbus.Error) { uiLayoutInfo, err := b.broker.SelectAuthenticationMode(context.Background(), sessionID, authenticationModeName) if err != nil { return nil, dbus.MakeFailedError(err) @@ -102,7 +107,8 @@ func (b *Bus) SelectAuthenticationMode(sessionID, authenticationModeName string) return uiLayoutInfo, nil } -// IsAuthenticated is the method through which the broker and the daemon will communicate once dbusInterface.IsAuthenticated is called. +// IsAuthenticated is the method through which the broker and the daemon will communicate once +// dbusInterface.IsAuthenticated is called. func (b *Bus) IsAuthenticated(sessionID, authenticationData string) (access, data string, dbusErr *dbus.Error) { access, data, err := b.broker.IsAuthenticated(context.Background(), sessionID, authenticationData) if err != nil { @@ -111,7 +117,8 @@ func (b *Bus) IsAuthenticated(sessionID, authenticationData string) (access, dat return access, data, nil } -// EndSession is the method through which the broker and the daemon will communicate once dbusInterface.EndSession is called. +// EndSession is the method through which the broker and the daemon will communicate once +// dbusInterface.EndSession is called. func (b *Bus) EndSession(sessionID string) (dbusErr *dbus.Error) { err := b.broker.EndSession(context.Background(), sessionID) if err != nil { @@ -120,13 +127,15 @@ func (b *Bus) EndSession(sessionID string) (dbusErr *dbus.Error) { return nil } -// CancelIsAuthenticated is the method through which the broker and the daemon will communicate once dbusInterface.CancelIsAuthenticated is called. +// CancelIsAuthenticated is the method through which the broker and the daemon will communicate once +// dbusInterface.CancelIsAuthenticated is called. func (b *Bus) CancelIsAuthenticated(sessionID string) (dbusErr *dbus.Error) { b.broker.CancelIsAuthenticated(context.Background(), sessionID) return nil } -// UserPreCheck is the method through which the broker and the daemon will communicate once dbusInterface.UserPreCheck is called. +// UserPreCheck is the method through which the broker and the daemon will communicate once +// dbusInterface.UserPreCheck is called. func (b *Bus) UserPreCheck(username string) (userinfo string, dbusErr *dbus.Error) { userinfo, err := b.broker.UserPreCheck(context.Background(), username) if err != nil { diff --git a/internal/brokers/broker.go b/internal/brokers/broker.go index 9f71d3960..5b8dd7f32 100644 --- a/internal/brokers/broker.go +++ b/internal/brokers/broker.go @@ -37,10 +37,14 @@ const ( var AuthReplies = []string{AuthGranted, AuthDenied, AuthCancelled, AuthRetry, AuthNext} type brokerer interface { - NewSession(ctx context.Context, username, lang, mode string) (sessionID, encryptionKey string, err error) - GetAuthenticationModes(ctx context.Context, sessionID string, supportedUILayouts []map[string]string) (authenticationModes []map[string]string, err error) - SelectAuthenticationMode(ctx context.Context, sessionID, authenticationModeName string) (uiLayoutInfo map[string]string, err error) - IsAuthenticated(ctx context.Context, sessionID, authenticationData string) (access, data string, err error) + NewSession(ctx context.Context, username, lang, mode string) ( + sessionID, encryptionKey string, err error) + GetAuthenticationModes(ctx context.Context, sessionID string, supportedUILayouts []map[string]string) ( + authenticationModes []map[string]string, err error) + SelectAuthenticationMode(ctx context.Context, sessionID, authenticationModeName string) ( + uiLayoutInfo map[string]string, err error) + IsAuthenticated(ctx context.Context, sessionID, authenticationData string) ( + access, data string, err error) EndSession(ctx context.Context, sessionID string) (err error) CancelIsAuthenticated(ctx context.Context, sessionID string) @@ -101,7 +105,8 @@ func newBroker(ctx context.Context, configFile string, bus *dbus.Conn) (b Broker } // newSession calls the broker corresponding method, expanding sessionID with the broker ID prefix. -func (b Broker) newSession(ctx context.Context, username, lang, mode string) (sessionID, encryptionKey string, err error) { +func (b Broker) newSession(ctx context.Context, username, lang, mode string) ( + sessionID, encryptionKey string, err error) { sessionID, encryptionKey, err = b.brokerer.NewSession(ctx, username, lang, mode) if err != nil { return "", "", err @@ -119,7 +124,8 @@ func (b Broker) newSession(ctx context.Context, username, lang, mode string) (se } // GetAuthenticationModes calls the broker corresponding method, stripping broker ID prefix from sessionID. -func (b *Broker) GetAuthenticationModes(ctx context.Context, sessionID string, supportedUILayouts []map[string]string) (authenticationModes []map[string]string, err error) { +func (b *Broker) GetAuthenticationModes(ctx context.Context, sessionID string, + supportedUILayouts []map[string]string) (authenticationModes []map[string]string, err error) { sessionID = b.parseSessionID(sessionID) b.layoutValidatorsMu.Lock() @@ -143,7 +149,8 @@ func (b *Broker) GetAuthenticationModes(ctx context.Context, sessionID string, s } // SelectAuthenticationMode calls the broker corresponding method, stripping broker ID prefix from sessionID. -func (b Broker) SelectAuthenticationMode(ctx context.Context, sessionID, authenticationModeName string) (uiLayoutInfo map[string]string, err error) { +func (b Broker) SelectAuthenticationMode(ctx context.Context, sessionID, authenticationModeName string) ( + uiLayoutInfo map[string]string, err error) { sessionID = b.parseSessionID(sessionID) uiLayoutInfo, err = b.brokerer.SelectAuthenticationMode(ctx, sessionID, authenticationModeName) if err != nil { @@ -153,7 +160,8 @@ func (b Broker) SelectAuthenticationMode(ctx context.Context, sessionID, authent } // IsAuthenticated calls the broker corresponding method, stripping broker ID prefix from sessionID. -func (b Broker) IsAuthenticated(ctx context.Context, sessionID, authenticationData string) (access string, data string, err error) { +func (b Broker) IsAuthenticated(ctx context.Context, sessionID, authenticationData string) ( + access string, data string, err error) { sessionID = b.parseSessionID(sessionID) // monitor ctx in goroutine to call cancel @@ -254,11 +262,13 @@ func (b Broker) UserPreCheck(ctx context.Context, username string) (userinfo str // } // } // } -func generateValidators(ctx context.Context, sessionID string, supportedUILayouts []map[string]string) map[string]layoutValidator { +func generateValidators(ctx context.Context, sessionID string, + supportedUILayouts []map[string]string) map[string]layoutValidator { validators := make(map[string]layoutValidator) for _, layout := range supportedUILayouts { if _, exists := layout["type"]; !exists { - log.Errorf(ctx, "layout %v provided with missing type for session %s, it will be ignored", layout, sessionID) + log.Errorf(ctx, "layout %v provided with missing type for session %s, it will be ignored", + layout, sessionID) continue } @@ -326,7 +336,8 @@ func (b Broker) validateUILayout(sessionID string, layout map[string]string) (r continue } if validator.supportedValues != nil && !slices.Contains(validator.supportedValues, value) { - return nil, fmt.Errorf("field %q has invalid value %q, expected one of %s", key, value, strings.Join(validator.supportedValues, ",")) + return nil, fmt.Errorf("field %q has invalid value %q, expected one of %s", + key, value, strings.Join(validator.supportedValues, ",")) } } return layout, nil @@ -389,7 +400,8 @@ func validateUserInfo(uInfo userInfo) (err error) { func unmarshalAndGetKey(data, key string) (json.RawMessage, error) { var returnedData map[string]json.RawMessage if err := json.Unmarshal([]byte(data), &returnedData); err != nil { - return nil, fmt.Errorf("response returned by the broker is not a valid json: %v\nBroker returned: %v", err, data) + return nil, fmt.Errorf("response returned by the broker is not a valid json: %v\nBroker returned: %v", + err, data) } rawMsg, ok := returnedData[key] diff --git a/internal/brokers/broker_test.go b/internal/brokers/broker_test.go index ea2c04024..5c20af9a1 100644 --- a/internal/brokers/broker_test.go +++ b/internal/brokers/broker_test.go @@ -100,10 +100,18 @@ func TestGetAuthenticationModes(t *testing.T) { wantErr bool }{ - "Get authentication modes and generate validators": {sessionID: "success", supportedUILayouts: []string{"required-entry", "optional-entry"}}, - "Get authentication modes and generate validator ignoring whitespaces in supported values": {sessionID: "success", supportedUILayouts: []string{"layout-with-spaces"}}, - "Get authentication modes and ignores invalid UI layout": {sessionID: "success", supportedUILayouts: []string{"required-entry", "missing-type"}}, - "Get multiple authentication modes and generate validators": {sessionID: "GAM_multiple_modes", supportedUILayouts: []string{"required-entry", "optional-entry"}}, + "Get authentication modes and generate validators": { + sessionID: "success", supportedUILayouts: []string{"required-entry", "optional-entry"}, + }, + "Get authentication modes and generate validator ignoring whitespaces in supported values": { + sessionID: "success", supportedUILayouts: []string{"layout-with-spaces"}, + }, + "Get authentication modes and ignores invalid UI layout": { + sessionID: "success", supportedUILayouts: []string{"required-entry", "missing-type"}, + }, + "Get multiple authentication modes and generate validators": { + sessionID: "GAM_multiple_modes", supportedUILayouts: []string{"required-entry", "optional-entry"}, + }, "Does not error out when no authentication modes are returned": {sessionID: "GAM_empty"}, @@ -124,7 +132,8 @@ func TestGetAuthenticationModes(t *testing.T) { supportedUILayouts = append(supportedUILayouts, supportedLayouts[layout]) } - gotModes, err := b.GetAuthenticationModes(context.Background(), prefixID(t, tc.sessionID), supportedUILayouts) + gotModes, err := b.GetAuthenticationModes(context.Background(), + prefixID(t, tc.sessionID), supportedUILayouts) if tc.wantErr { require.Error(t, err, "GetAuthenticationModes should return an error, but did not") return @@ -134,7 +143,8 @@ func TestGetAuthenticationModes(t *testing.T) { modesStr, err := json.Marshal(gotModes) require.NoError(t, err, "Post: error when marshaling result") - got := "MODES:\n" + string(modesStr) + "\n\nVALIDATORS:\n" + b.LayoutValidatorsString(prefixID(t, tc.sessionID)) + got := "MODES:\n" + string(modesStr) + "\n\nVALIDATORS:\n" + + b.LayoutValidatorsString(prefixID(t, tc.sessionID)) want := testutils.LoadWithUpdateFromGolden(t, got) require.Equal(t, want, got, "GetAuthenticationModes should return the expected modes, but did not") }) @@ -152,23 +162,45 @@ func TestSelectAuthenticationMode(t *testing.T) { wantErr bool }{ - "Successfully select mode with required value": {sessionID: "SAM_success_required_entry"}, - "Successfully select mode with optional value": {sessionID: "SAM_success_optional_entry", supportedUILayouts: []string{"optional-entry"}}, - "Successfully select mode with missing optional value": {sessionID: "SAM_missing_optional_entry", supportedUILayouts: []string{"optional-entry"}}, + "Successfully select mode with required value": { + sessionID: "SAM_success_required_entry", + }, + "Successfully select mode with optional value": { + sessionID: "SAM_success_optional_entry", supportedUILayouts: []string{"optional-entry"}, + }, + "Successfully select mode with missing optional value": { + sessionID: "SAM_missing_optional_entry", supportedUILayouts: []string{"optional-entry"}, + }, // broker errors "Error when selecting invalid auth mode": {sessionID: "SAM_error", wantErr: true}, "Error when no validators were generated for session": {sessionID: "no-validators", wantErr: true}, /* Layout errors */ - "Error when returns no layout": {sessionID: "SAM_no_layout", wantErr: true}, - "Error when returns empty layout": {sessionID: "SAM_empty_layout", wantErr: true}, - "Error when returns layout with no type": {sessionID: "SAM_no_layout_type", wantErr: true}, - "Error when returns layout with invalid type": {sessionID: "SAM_invalid_layout_type", wantErr: true}, - "Error when returns layout without required value": {sessionID: "SAM_missing_required_entry", wantErr: true}, - "Error when returns layout with unknown field": {sessionID: "SAM_unknown_field", wantErr: true}, - "Error when returns layout with invalid required value": {sessionID: "SAM_invalid_required_entry", wantErr: true}, - "Error when returns layout with invalid optional value": {sessionID: "SAM_invalid_optional_entry", wantErr: true}, + "Error when returns no layout": { + sessionID: "SAM_no_layout", wantErr: true, + }, + "Error when returns empty layout": { + sessionID: "SAM_empty_layout", wantErr: true, + }, + "Error when returns layout with no type": { + sessionID: "SAM_no_layout_type", wantErr: true, + }, + "Error when returns layout with invalid type": { + sessionID: "SAM_invalid_layout_type", wantErr: true, + }, + "Error when returns layout without required value": { + sessionID: "SAM_missing_required_entry", wantErr: true, + }, + "Error when returns layout with unknown field": { + sessionID: "SAM_unknown_field", wantErr: true, + }, + "Error when returns layout with invalid required value": { + sessionID: "SAM_invalid_required_entry", wantErr: true, + }, + "Error when returns layout with invalid optional value": { + sessionID: "SAM_invalid_optional_entry", wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -184,7 +216,8 @@ func TestSelectAuthenticationMode(t *testing.T) { } if tc.sessionID != "no-validators" { - // This is normally done in the broker's GetAuthenticationModes method, but we need to do it here to test the SelectAuthenticationMode method. + // This is normally done in the broker's GetAuthenticationModes method, + // but we need to do it here to test the SelectAuthenticationMode method. brokers.GenerateLayoutValidators(&b, prefixID(t, tc.sessionID), supportedUILayouts) } @@ -212,31 +245,38 @@ func TestIsAuthenticated(t *testing.T) { cancelFirstCall bool }{ - "Successfully authenticate": {sessionID: "success"}, - "Successfully authenticate after cancelling first call": {sessionID: "IA_second_call", secondCall: true}, + "Successfully authenticate": {sessionID: "success"}, + "Successfully authenticate after cancelling first call": { + sessionID: "IA_second_call", + secondCall: true, + }, "Denies authentication when broker times out": {sessionID: "IA_timeout"}, "Adds default groups even if broker did not set them": {sessionID: "IA_info_empty_groups"}, "No error when auth.Next and no data": {sessionID: "IA_next"}, "No error when broker returns userinfo with empty gecos": {sessionID: "IA_info_empty_gecos"}, "No error when broker returns userinfo with group with empty UGID": {sessionID: "IA_info_empty_ugid"}, - "No error when broker returns userinfo with mismatching username": {sessionID: "IA_info_mismatching_user_name"}, + "No error when broker returns userinfo with mismatching username": { + sessionID: "IA_info_mismatching_user_name", + }, // broker errors - "Error when authenticating": {sessionID: "IA_error"}, - "Error on empty data even if granted": {sessionID: "IA_empty_data"}, - "Error when broker returns invalid data": {sessionID: "IA_invalid_data"}, - "Error when broker returns invalid access": {sessionID: "IA_invalid_access"}, - "Error when broker returns invalid userinfo": {sessionID: "IA_invalid_userinfo"}, - "Error when broker returns userinfo with empty username": {sessionID: "IA_info_empty_user_name"}, - "Error when broker returns userinfo with empty group name": {sessionID: "IA_info_empty_group_name"}, - "Error when broker returns userinfo with empty UUID": {sessionID: "IA_info_empty_uuid"}, - "Error when broker returns userinfo with invalid homedir": {sessionID: "IA_info_invalid_home"}, - "Error when broker returns userinfo with invalid shell": {sessionID: "IA_info_invalid_shell"}, - "Error when broker returns data on auth.Next": {sessionID: "IA_next_with_data"}, - "Error when broker returns data on auth.Cancelled": {sessionID: "IA_cancelled_with_data"}, - "Error when broker returns no data on auth.Denied": {sessionID: "IA_denied_without_data"}, - "Error when broker returns no data on auth.Retry": {sessionID: "IA_retry_without_data"}, - "Error when calling IsAuthenticated a second time without cancelling": {sessionID: "IA_second_call", secondCall: true, cancelFirstCall: true}, + "Error when authenticating": {sessionID: "IA_error"}, + "Error on empty data even if granted": {sessionID: "IA_empty_data"}, + "Error when broker returns invalid data": {sessionID: "IA_invalid_data"}, + "Error when broker returns invalid access": {sessionID: "IA_invalid_access"}, + "Error when broker returns invalid userinfo": {sessionID: "IA_invalid_userinfo"}, + "Error when broker returns userinfo with empty username": {sessionID: "IA_info_empty_user_name"}, + "Error when broker returns userinfo with empty group name": {sessionID: "IA_info_empty_group_name"}, + "Error when broker returns userinfo with empty UUID": {sessionID: "IA_info_empty_uuid"}, + "Error when broker returns userinfo with invalid homedir": {sessionID: "IA_info_invalid_home"}, + "Error when broker returns userinfo with invalid shell": {sessionID: "IA_info_invalid_shell"}, + "Error when broker returns data on auth.Next": {sessionID: "IA_next_with_data"}, + "Error when broker returns data on auth.Cancelled": {sessionID: "IA_cancelled_with_data"}, + "Error when broker returns no data on auth.Denied": {sessionID: "IA_denied_without_data"}, + "Error when broker returns no data on auth.Retry": {sessionID: "IA_retry_without_data"}, + "Error when calling IsAuthenticated a second time without cancelling": { + sessionID: "IA_second_call", secondCall: true, cancelFirstCall: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -257,7 +297,8 @@ func TestIsAuthenticated(t *testing.T) { go func() { defer close(done) access, gotData, err := b.IsAuthenticated(ctx, sessionID, "password") - firstCallReturn = fmt.Sprintf("FIRST CALL:\n\taccess: %s\n\tdata: %s\n\terr: %v\n", access, gotData, err) + firstCallReturn = fmt.Sprintf("FIRST CALL:\n\taccess: %s\n\tdata: %s\n\terr: %v\n", + access, gotData, err) }() // Give some time for the first call to block @@ -269,7 +310,8 @@ func TestIsAuthenticated(t *testing.T) { <-done } access, gotData, err := b.IsAuthenticated(context.Background(), sessionID, "password") - secondCallReturn = fmt.Sprintf("SECOND CALL:\n\taccess: %s\n\tdata: %s\n\terr: %v\n", access, gotData, err) + secondCallReturn = fmt.Sprintf("SECOND CALL:\n\taccess: %s\n\tdata: %s\n\terr: %v\n", + access, gotData, err) } <-done diff --git a/internal/brokers/dbusbroker.go b/internal/brokers/dbusbroker.go index b360a5f93..c45a256f3 100644 --- a/internal/brokers/dbusbroker.go +++ b/internal/brokers/dbusbroker.go @@ -22,7 +22,8 @@ type dbusBroker struct { } // newDbusBroker returns a dbus broker and broker attributes from its configuration file. -func newDbusBroker(ctx context.Context, bus *dbus.Conn, configFile string) (b dbusBroker, name, brandIcon string, err error) { +func newDbusBroker(ctx context.Context, bus *dbus.Conn, configFile string) ( + b dbusBroker, name, brandIcon string, err error) { defer decorate.OnError(&err, "dbus broker from configuration file: %q", configFile) log.Debugf(ctx, "Dbus broker configuration at %q", configFile) @@ -58,8 +59,10 @@ func newDbusBroker(ctx context.Context, bus *dbus.Conn, configFile string) (b db }, nameVal.String(), brandIconVal.String(), nil } -// NewSession calls the corresponding method on the broker bus and returns the session ID and encryption key. -func (b dbusBroker) NewSession(ctx context.Context, username, lang, mode string) (sessionID, encryptionKey string, err error) { +// NewSession calls the corresponding method on the broker bus and +// returns the session ID and encryption key. +func (b dbusBroker) NewSession(ctx context.Context, username, lang, mode string) ( + sessionID, encryptionKey string, err error) { call, err := b.call(ctx, "NewSession", username, lang, mode) if err != nil { return "", "", err @@ -71,8 +74,10 @@ func (b dbusBroker) NewSession(ctx context.Context, username, lang, mode string) return sessionID, encryptionKey, nil } -// GetAuthenticationModes calls the corresponding method on the broker bus and returns the authentication modes supported by it. -func (b dbusBroker) GetAuthenticationModes(ctx context.Context, sessionID string, supportedUILayouts []map[string]string) (authenticationModes []map[string]string, err error) { +// GetAuthenticationModes calls the corresponding method on the broker bus and +// returns the authentication modes supported by it. +func (b dbusBroker) GetAuthenticationModes(ctx context.Context, sessionID string, + supportedUILayouts []map[string]string) (authenticationModes []map[string]string, err error) { call, err := b.call(ctx, "GetAuthenticationModes", sessionID, supportedUILayouts) if err != nil { return nil, err @@ -84,8 +89,10 @@ func (b dbusBroker) GetAuthenticationModes(ctx context.Context, sessionID string return authenticationModes, nil } -// SelectAuthenticationMode calls the corresponding method on the broker bus and returns the UI layout for the selected mode. -func (b dbusBroker) SelectAuthenticationMode(ctx context.Context, sessionID, authenticationModeName string) (uiLayoutInfo map[string]string, err error) { +// SelectAuthenticationMode calls the corresponding method on the broker bus and +// returns the UI layout for the selected mode. +func (b dbusBroker) SelectAuthenticationMode(ctx context.Context, sessionID, authenticationModeName string) ( + uiLayoutInfo map[string]string, err error) { call, err := b.call(ctx, "SelectAuthenticationMode", sessionID, authenticationModeName) if err != nil { return nil, err @@ -97,8 +104,10 @@ func (b dbusBroker) SelectAuthenticationMode(ctx context.Context, sessionID, aut return uiLayoutInfo, nil } -// IsAuthenticated calls the corresponding method on the broker bus and returns the user information and access. -func (b dbusBroker) IsAuthenticated(_ context.Context, sessionID, authenticationData string) (access, data string, err error) { +// IsAuthenticated calls the corresponding method on the broker bus and +// returns the user information and access. +func (b dbusBroker) IsAuthenticated(_ context.Context, sessionID, authenticationData string) ( + access, data string, err error) { // We don’t want to cancel the context when the parent call is cancelled. call, err := b.call(context.Background(), "IsAuthenticated", sessionID, authenticationData) if err != nil { diff --git a/internal/brokers/export_test.go b/internal/brokers/export_test.go index a8f119fa4..700bb453a 100644 --- a/internal/brokers/export_test.go +++ b/internal/brokers/export_test.go @@ -55,7 +55,8 @@ func (b *Broker) LayoutValidatorsString(sessionID string) string { sort.Strings(vKeys) for _, v := range vKeys { - layoutStr += fmt.Sprintf("\t\t%s: { required: %v, supportedValues: %v }\n", v, validator[v].required, validator[v].supportedValues) + layoutStr += fmt.Sprintf("\t\t%s: { required: %v, supportedValues: %v }\n", + v, validator[v].required, validator[v].supportedValues) } s += layoutStr diff --git a/internal/brokers/localbroker.go b/internal/brokers/localbroker.go index c0730fced..8e55a1ca0 100644 --- a/internal/brokers/localbroker.go +++ b/internal/brokers/localbroker.go @@ -5,40 +5,60 @@ import ( "errors" ) -//nolint:unused // We still need localBroker to implement the brokerer interface, even though this type will not be interacted with by the daemon. +// even though this type will not be interacted with by the daemon. +// +//nolint:unused // We still need localBroker to implement the brokerer interface, type localBroker struct { } -//nolint:unused // We still need localBroker to implement the brokerer interface, even though this method should never be called on it. -func (b localBroker) NewSession(ctx context.Context, username, lang, mode string) (sessionID, encryptionKey string, err error) { +// even though this method should never be called on it. +// +//nolint:unused // We still need localBroker to implement the brokerer interface, +func (b localBroker) NewSession(ctx context.Context, username, lang, mode string) ( + sessionID, encryptionKey string, err error) { return "", "", errors.New("NewSession should never be called on local broker") } -//nolint:unused // We still need localBroker to implement the brokerer interface, even though this method should never be called on it. -func (b localBroker) GetAuthenticationModes(ctx context.Context, sessionID string, supportedUILayouts []map[string]string) (authenticationModes []map[string]string, err error) { +// even though this method should never be called on it. +// +//nolint:unused // We still need localBroker to implement the brokerer interface, +func (b localBroker) GetAuthenticationModes(ctx context.Context, sessionID string, + supportedUILayouts []map[string]string) (authenticationModes []map[string]string, err error) { return nil, errors.New("GetAuthenticationModes should never be called on local broker") } -//nolint:unused // We still need localBroker to implement the brokerer interface, even though this method should never be called on it. -func (b localBroker) SelectAuthenticationMode(ctx context.Context, sessionID, authenticationModeName string) (uiLayoutInfo map[string]string, err error) { +// even though this method should never be called on it. +// +//nolint:unused // We still need localBroker to implement the brokerer interface, +func (b localBroker) SelectAuthenticationMode(ctx context.Context, sessionID, authenticationModeName string) ( + uiLayoutInfo map[string]string, err error) { return nil, errors.New("SelectAuthenticationMode should never be called on local broker") } -//nolint:unused // We still need localBroker to implement the brokerer interface, even though this method should never be called on it. -func (b localBroker) IsAuthenticated(ctx context.Context, sessionID, authenticationData string) (access, data string, err error) { +// even though this method should never be called on it. +// +//nolint:unused // We still need localBroker to implement the brokerer interface, +func (b localBroker) IsAuthenticated(ctx context.Context, sessionID, authenticationData string) ( + access, data string, err error) { return "", "", errors.New("IsAuthenticated should never be called on local broker") } -//nolint:unused // We still need localBroker to implement the brokerer interface, even though this method should never be called on it. +// even though this method should never be called on it. +// +//nolint:unused // We still need localBroker to implement the brokerer interface, func (b localBroker) EndSession(ctx context.Context, sessionID string) (err error) { return errors.New("EndSession should never be called on local broker") } -//nolint:unused // We still need localBroker to implement the brokerer interface, even though this method should never be called on it. +// even though this method should never be called on it. +// +//nolint:unused // We still need localBroker to implement the brokerer interface, func (b localBroker) CancelIsAuthenticated(ctx context.Context, sessionID string) { } -//nolint:unused // We still need localBroker to implement the brokerer interface, even though this method should never be called on it. +// even though this method should never be called on it. +// +//nolint:unused // We still need localBroker to implement the brokerer interface, func (b localBroker) UserPreCheck(ctx context.Context, username string) (string, error) { return "", errors.New("UserPreCheck should never be called on local broker") } diff --git a/internal/brokers/manager.go b/internal/brokers/manager.go index 21a45cf8f..b7bc46e85 100644 --- a/internal/brokers/manager.go +++ b/internal/brokers/manager.go @@ -57,7 +57,8 @@ func NewManager(ctx context.Context, brokersConfPath string, configuredBrokers [ entries, err := os.ReadDir(brokersConfPath) if errors.Is(err, fs.ErrNotExist) { - log.Warningf(ctx, "Broker configuration directory %q does not exist, only local broker will be available", brokersConfPath) + log.Warningf(ctx, "Broker configuration directory %q does not exist, only local broker will be available", + brokersConfPath) } else if err != nil { return m, fmt.Errorf("could not read brokers directory to detect brokers: %v", err) } @@ -67,7 +68,8 @@ func NewManager(ctx context.Context, brokersConfPath string, configuredBrokers [ continue } if !strings.HasSuffix(e.Name(), ".conf") { - log.Infof(ctx, "Skipping file %q in brokers configuration directory, only .conf files are supported", e.Name()) + log.Infof(ctx, "Skipping file %q in brokers configuration directory, only .conf files are supported", + e.Name()) continue } configuredBrokers = append(configuredBrokers, e.Name()) @@ -152,7 +154,8 @@ func (m *Manager) BrokerFromSessionID(id string) (broker *Broker, err error) { } // NewSession create a new session for the broker and store the sesssionID on the manager. -func (m *Manager) NewSession(brokerID, username, lang, mode string) (sessionID string, encryptionKey string, err error) { +func (m *Manager) NewSession(brokerID, username, lang, mode string) ( + sessionID string, encryptionKey string, err error) { broker, err := m.brokerFromID(brokerID) if err != nil { return "", "", fmt.Errorf("invalid broker: %v", err) diff --git a/internal/brokers/manager_test.go b/internal/brokers/manager_test.go index 2fd161972..7c0635859 100644 --- a/internal/brokers/manager_test.go +++ b/internal/brokers/manager_test.go @@ -26,12 +26,24 @@ func TestNewManager(t *testing.T) { wantErr bool }{ - "Creates all brokers when config dir has only valid brokers": {brokerConfigDir: "valid_brokers"}, - "Creates without autodiscovery when configuredBrokers is set": {brokerConfigDir: "valid_brokers", configuredBrokers: []string{"valid_2.conf"}}, - "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"}, + "Creates all brokers when config dir has only valid brokers": { + brokerConfigDir: "valid_brokers", + }, + "Creates without autodiscovery when configuredBrokers is set": { + brokerConfigDir: "valid_brokers", configuredBrokers: []string{"valid_2.conf"}, + }, + "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"}, @@ -45,7 +57,8 @@ func TestNewManager(t *testing.T) { t.Setenv("DBUS_SYSTEM_BUS_ADDRESS", "/dev/null") } - got, err := brokers.NewManager(context.Background(), filepath.Join(brokerConfFixtures, tc.brokerConfigDir), tc.configuredBrokers) + got, err := brokers.NewManager(context.Background(), + filepath.Join(brokerConfFixtures, tc.brokerConfigDir), tc.configuredBrokers) if tc.wantErr { require.Error(t, err, "NewManager should return an error, but did not") return @@ -96,7 +109,8 @@ func TestSetDefaultBrokerForUser(t *testing.T) { require.NoError(t, err, "SetDefaultBrokerForUser should not return an error, but did") got := m.BrokerForUser("user") - require.Equal(t, want.ID, got.ID, "SetDefaultBrokerForUser should have assiged the expected broker, but did not") + require.Equal(t, want.ID, got.ID, + "SetDefaultBrokerForUser should have assiged the expected broker, but did not") }) } } @@ -161,7 +175,8 @@ func TestBrokerFromSessionID(t *testing.T) { return } require.NoError(t, err, "BrokerFromSessionID should not return an error, but did") - require.Equal(t, tc.wantBrokerID, got.ID, "BrokerFromSessionID should return the expected broker, but did not") + require.Equal(t, tc.wantBrokerID, got.ID, + "BrokerFromSessionID should return the expected broker, but did not") }) } } @@ -179,9 +194,11 @@ func TestNewSession(t *testing.T) { wantErr bool }{ - "Successfully start a new auth session": {username: "success"}, - "Successfully start a new passwd session": {username: "success", sessionMode: "passwd"}, - "Successfully start a new session with the correct broker": {username: "success", configuredBrokers: []string{t.Name() + "_Broker1.conf", t.Name() + "_Broker2.conf"}}, + "Successfully start a new auth session": {username: "success"}, + "Successfully start a new passwd session": {username: "success", sessionMode: "passwd"}, + "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}, @@ -243,13 +260,15 @@ func TestNewSession(t *testing.T) { require.NoError(t, err, "NewSession should not return an error, but did") // Replaces the autogenerated part of the ID with a placeholder before saving the file. - gotStr := fmt.Sprintf("ID: %s\nEncryption Key: %s\n", strings.ReplaceAll(gotID, wantBroker.ID, "BROKER_ID"), gotEKey) + gotStr := fmt.Sprintf("ID: %s\nEncryption Key: %s\n", + strings.ReplaceAll(gotID, wantBroker.ID, "BROKER_ID"), gotEKey) wantStr := testutils.LoadWithUpdateFromGolden(t, gotStr) require.Equal(t, wantStr, gotStr, "NewSession should return the expected session, but did not") gotBroker, err := m.BrokerFromSessionID(gotID) require.NoError(t, err, "NewSession should have assigned a broker for the session, but did not") - require.Equal(t, wantBroker.ID, gotBroker.ID, "BrokerFromSessionID should have assigned the expected broker for the session, but did not") + require.Equal(t, wantBroker.ID, gotBroker.ID, + "BrokerFromSessionID should have assigned the expected broker for the session, but did not") }) } } @@ -265,8 +284,10 @@ func TestEndSession(t *testing.T) { wantErr bool }{ - "Successfully end session": {sessionID: "success"}, - "Successfully end session on the correct broker": {sessionID: "success", configuredBrokers: []string{t.Name() + "_Broker1", t.Name() + "_Broker2"}}, + "Successfully end session": {sessionID: "success"}, + "Successfully end session on the correct broker": { + sessionID: "success", configuredBrokers: []string{t.Name() + "_Broker1", t.Name() + "_Broker2"}, + }, "Error when broker does not exist": {brokerID: "does not exist", sessionID: "dont matter", wantErr: true}, "Error when ending session": {sessionID: "ES_error", wantErr: true}, @@ -301,7 +322,8 @@ func TestEndSession(t *testing.T) { } require.NoError(t, err, "EndSession should not return an error, but did") _, err = m.BrokerFromSessionID(tc.sessionID) - require.Error(t, err, "EndSession should have removed the broker from the active transactions, but did not") + require.Error(t, err, + "EndSession should have removed the broker from the active transactions, but did not") }) } } @@ -357,10 +379,12 @@ func TestStartAndEndSession(t *testing.T) { assignedBroker, err := m.BrokerFromSessionID(*firstID) require.NoError(t, err, "First NewSession should have assigned a broker for the session, but did not") - require.Equal(t, b1.Name, assignedBroker.Name, "First NewSession should have assigned the expected broker for the session, but did not") + require.Equal(t, b1.Name, assignedBroker.Name, + "First NewSession should have assigned the expected broker for the session, but did not") assignedBroker, err = m.BrokerFromSessionID(*secondID) require.NoError(t, err, "Second NewSession should have assigned a broker for the session, but did not") - require.Equal(t, b2.Name, assignedBroker.Name, "Second NewSession should have assigned the expected broker for the session, but did not") + require.Equal(t, b2.Name, assignedBroker.Name, + "Second NewSession should have assigned the expected broker for the session, but did not") /* Ending the sessions */ wg.Add(1) diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 6603212ec..b18336f90 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -91,7 +91,8 @@ func New(ctx context.Context, registerGRPCService GRPCServiceRegisterer, args .. } if len(listeners) != 1 { - return nil, fmt.Errorf( /*i18n.G(*/ "unexpected number of systemd socket activation (%d != 1)" /*)*/, len(listeners)) + return nil, fmt.Errorf( /*i18n.G(*/ + "unexpected number of systemd socket activation (%d != 1)" /*)*/, len(listeners)) } lis = listeners[0] } diff --git a/internal/daemon/daemon_test.go b/internal/daemon/daemon_test.go index aaf2e6f98..dcc90de9e 100644 --- a/internal/daemon/daemon_test.go +++ b/internal/daemon/daemon_test.go @@ -39,14 +39,28 @@ func TestNew(t *testing.T) { wantSelectedSocket string wantErr bool }{ - "With socket activation": {wantSelectedSocket: "systemd.sock1"}, - "Socket provided manually is created": {socketType: manualSocket, wantSelectedSocket: "manual.sock"}, - "Socket provided manually wins over socket activation": {socketType: systemdActivationListenerAndManualSocket, wantSelectedSocket: "manual.sock"}, - - "Error when systemd provides multiple sockets": {socketType: systemdActivationListenerMultipleSockets, wantErr: true}, - "Error when systemd activation fails": {socketType: systemdActivationListenerFails, wantErr: true}, - "Error when systemd activated socket does not exists": {socketType: systemdActivationListenerSocketDoesNotExists, wantErr: true}, - "Error when manually provided socket path does not exists": {socketType: manualSocketParentDirectoryDoesNotExists, wantErr: true}, + "With socket activation": { + wantSelectedSocket: "systemd.sock1", + }, + "Socket provided manually is created": { + socketType: manualSocket, wantSelectedSocket: "manual.sock", + }, + "Socket provided manually wins over socket activation": { + socketType: systemdActivationListenerAndManualSocket, wantSelectedSocket: "manual.sock", + }, + + "Error when systemd provides multiple sockets": { + socketType: systemdActivationListenerMultipleSockets, wantErr: true, + }, + "Error when systemd activation fails": { + socketType: systemdActivationListenerFails, wantErr: true, + }, + "Error when systemd activated socket does not exists": { + socketType: systemdActivationListenerSocketDoesNotExists, wantErr: true, + }, + "Error when manually provided socket path does not exists": { + socketType: manualSocketParentDirectoryDoesNotExists, wantErr: true, + }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { @@ -271,17 +285,20 @@ func TestQuit(t *testing.T) { } if !tc.activeConnection || tc.force { - require.Eventually(t, serverHasQuit, 100*time.Millisecond, 10*time.Millisecond, "Server should quit with no active connection or force") + require.Eventually(t, serverHasQuit, 100*time.Millisecond, 10*time.Millisecond, + "Server should quit with no active connection or force") return } time.Sleep(100 * time.Millisecond) - require.False(t, serverHasQuit(), "Server should still be running because of active connection and not forced") + require.False(t, serverHasQuit(), + "Server should still be running because of active connection and not forced") // drop connection disconnectClient() - require.Eventually(t, serverHasQuit, 100*time.Millisecond, 10*time.Millisecond, "Server should quit with no more active connection") + require.Eventually(t, serverHasQuit, 100*time.Millisecond, 10*time.Millisecond, + "Server should quit with no more active connection") }) } } @@ -296,7 +313,9 @@ func createClientConnection(t *testing.T, socketPath string) (success bool, disc go func() { defer close(connected) var err error - conn, err = grpc.NewClient("unix://"+socketPath, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithUnaryInterceptor(errmessages.FormatErrorMessage)) + conn, err = grpc.NewClient( + "unix://"+socketPath, grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithUnaryInterceptor(errmessages.FormatErrorMessage)) require.NoError(t, err, "Could not connect to grpc server") // The daemon tests require an active connection, so we need to block here until the connection is ready. diff --git a/internal/services/errmessages/internal_test.go b/internal/services/errmessages/internal_test.go index 06533e928..334b3e4d6 100644 --- a/internal/services/errmessages/internal_test.go +++ b/internal/services/errmessages/internal_test.go @@ -35,7 +35,8 @@ func TestRedactErrorInterceptor(t *testing.T) { _, err := RedactErrorInterceptor(context.TODO(), testRequest{tc.inputError}, nil, testHandler) require.Error(t, err, "RedactErrorInterceptor should return an error") - require.Equal(t, tc.wantMessage, err.Error(), "RedactErrorInterceptor returned unexpected error message") + require.Equal(t, tc.wantMessage, err.Error(), + "RedactErrorInterceptor returned unexpected error message") }) } } @@ -80,7 +81,8 @@ func TestFormatErrorMessage(t *testing.T) { err := FormatErrorMessage(context.TODO(), "", testRequest{tc.inputError}, nil, nil, testInvoker) require.Error(t, err, "FormatErrorMessage should return an error") - require.Equal(t, tc.wantMessage, err.Error(), "FormatErrorMessage returned unexpected error message") + require.Equal(t, tc.wantMessage, err.Error(), + "FormatErrorMessage returned unexpected error message") }) } } @@ -94,7 +96,8 @@ func testHandler(ctx context.Context, req any) (any, error) { return nil, req.(testRequest).err } -func testInvoker(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, opts ...grpc.CallOption) error { +func testInvoker(ctx context.Context, method string, req, reply any, + cc *grpc.ClientConn, opts ...grpc.CallOption) error { //nolint:forcetypeassert // This is only used in the tests and we know the type. return req.(testRequest).err } diff --git a/internal/services/errmessages/redactor.go b/internal/services/errmessages/redactor.go index fe1b46d3a..5e502c614 100644 --- a/internal/services/errmessages/redactor.go +++ b/internal/services/errmessages/redactor.go @@ -14,8 +14,10 @@ import ( // RedactErrorInterceptor redacts some of the attached errors before sending it to the client. // -// It unwraps the error up to the first ErrToDisplay and sends it to the client. If none is found, it sends the original error. -func RedactErrorInterceptor(ctx context.Context, req any, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) { +// It unwraps the error up to the first ErrToDisplay and sends it to the client. +// If none is found, it sends the original error. +func RedactErrorInterceptor(ctx context.Context, req any, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) ( + any, error) { m, err := handler(ctx, req) if err != nil { slog.Warn(err.Error()) @@ -31,7 +33,8 @@ func RedactErrorInterceptor(ctx context.Context, req any, _ *grpc.UnaryServerInf // FormatErrorMessage formats the error message received by the client to avoid printing useless information. // // It converts the gRPC error to a more human-readable error with a better message. -func FormatErrorMessage(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { +func FormatErrorMessage(ctx context.Context, method string, req, reply any, + cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { err := invoker(ctx, method, req, reply, cc, opts...) if err == nil { return nil diff --git a/internal/services/manager.go b/internal/services/manager.go index 7e4906570..da4e183e8 100644 --- a/internal/services/manager.go +++ b/internal/services/manager.go @@ -26,7 +26,8 @@ type Manager struct { } // NewManager returns a new manager after creating all necessary items for our business logic. -func NewManager(ctx context.Context, cacheDir, brokersConfPath string, configuredBrokers []string, usersConfig users.Config) (m Manager, err error) { +func NewManager(ctx context.Context, cacheDir, brokersConfPath string, configuredBrokers []string, + usersConfig users.Config) (m Manager, err error) { defer decorate.OnError(&err /*i18n.G(*/, "can't create authd object") //) log.Debug(ctx, "Building authd object") @@ -58,7 +59,10 @@ func NewManager(ctx context.Context, cacheDir, brokersConfPath string, configure func (m Manager) RegisterGRPCServices(ctx context.Context) *grpc.Server { log.Debug(ctx, "Registering GRPC services") - opts := []grpc.ServerOption{permissions.WithUnixPeerCreds(), grpc.ChainUnaryInterceptor(m.globalPermissions, errmessages.RedactErrorInterceptor)} + opts := []grpc.ServerOption{ + permissions.WithUnixPeerCreds(), + grpc.ChainUnaryInterceptor(m.globalPermissions, errmessages.RedactErrorInterceptor), + } grpcServer := grpc.NewServer(opts...) authd.RegisterNSSServer(grpcServer, m.nssService) diff --git a/internal/services/manager_test.go b/internal/services/manager_test.go index c9516cdb5..5b6310f6f 100644 --- a/internal/services/manager_test.go +++ b/internal/services/manager_test.go @@ -42,7 +42,8 @@ func TestNewManager(t *testing.T) { t.Setenv("DBUS_SYSTEM_BUS_ADDRESS", tc.systemBusSocket) } - m, err := services.NewManager(context.Background(), tc.cacheDir, t.TempDir(), nil, users.DefaultConfig) + m, err := services.NewManager(context.Background(), + tc.cacheDir, t.TempDir(), nil, users.DefaultConfig) if tc.wantErr { require.Error(t, err, "NewManager should have returned an error, but did not") return @@ -97,7 +98,9 @@ func TestAccessAuthorization(t *testing.T) { require.NoError(t, <-serverDone, "gRPC server should not return an error from serving") }() - conn, err := grpc.NewClient("unix://"+socketPath, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithUnaryInterceptor(errmessages.FormatErrorMessage)) + conn, err := grpc.NewClient("unix://"+socketPath, + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithUnaryInterceptor(errmessages.FormatErrorMessage)) require.NoError(t, err, "Setup: could not dial the server") // Global authorization for PAM is always denied for non root user. diff --git a/internal/services/nss/nss.go b/internal/services/nss/nss.go index 38706bfe3..3562d27ac 100644 --- a/internal/services/nss/nss.go +++ b/internal/services/nss/nss.go @@ -27,7 +27,8 @@ type Service struct { } // NewService returns a new NSS GRPC service. -func NewService(ctx context.Context, userManager *users.Manager, brokerManager *brokers.Manager, permissionManager *permissions.Manager) Service { +func NewService(ctx context.Context, userManager *users.Manager, + brokerManager *brokers.Manager, permissionManager *permissions.Manager) Service { log.Debug(ctx, "Building new GRPC NSS service") return Service{ diff --git a/internal/services/nss/nss_test.go b/internal/services/nss/nss_test.go index 4c4481b05..c12c10c85 100644 --- a/internal/services/nss/nss_test.go +++ b/internal/services/nss/nss_test.go @@ -54,16 +54,33 @@ func TestGetPasswdByName(t *testing.T) { }{ "Return existing user": {username: "user1"}, - "Precheck user if not in cache": {username: "user-pre-check", shouldPreCheck: true}, - "Prechecked user with upper cases in username has same id as lower case": {username: "User-Pre-Check", shouldPreCheck: true}, - - "Error in database fetched content": {username: "user1", sourceDB: "invalid.db.yaml", wantErr: true}, - "Error with typed GRPC notfound code on unexisting user": {username: "does-not-exists", wantErr: true, wantErrNotExists: true}, - "Error on missing name": {wantErr: true}, - - "Error in database fetched content does not trigger precheck": {username: "user1", sourceDB: "invalid.db.yaml", shouldPreCheck: true, wantErr: true}, - "Error if user not in cache and precheck is disabled": {username: "user-pre-check", wantErr: true, wantErrNotExists: true}, - "Error if user not in cache and precheck fails": {username: "does-not-exist", sourceDB: "empty.db.yaml", shouldPreCheck: true, wantErr: true, wantErrNotExists: true}, + "Precheck user if not in cache": { + username: "user-pre-check", shouldPreCheck: true, + }, + "Prechecked user with upper cases in username has same id as lower case": { + username: "User-Pre-Check", shouldPreCheck: true, + }, + + "Error in database fetched content": { + username: "user1", sourceDB: "invalid.db.yaml", wantErr: true, + }, + "Error with typed GRPC notfound code on unexisting user": { + username: "does-not-exists", wantErr: true, wantErrNotExists: true, + }, + "Error on missing name": { + wantErr: true, + }, + + "Error in database fetched content does not trigger precheck": { + username: "user1", sourceDB: "invalid.db.yaml", shouldPreCheck: true, wantErr: true, + }, + "Error if user not in cache and precheck is disabled": { + username: "user-pre-check", wantErr: true, wantErrNotExists: true, + }, + "Error if user not in cache and precheck fails": { + username: "does-not-exist", sourceDB: "empty.db.yaml", shouldPreCheck: true, + wantErr: true, wantErrNotExists: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -72,7 +89,8 @@ func TestGetPasswdByName(t *testing.T) { client := newNSSClient(t, tc.sourceDB, false) - got, err := client.GetPasswdByName(context.Background(), &authd.GetPasswdByNameRequest{Name: tc.username, ShouldPreCheck: tc.shouldPreCheck}) + got, err := client.GetPasswdByName(context.Background(), + &authd.GetPasswdByNameRequest{Name: tc.username, ShouldPreCheck: tc.shouldPreCheck}) requireExpectedResult(t, "GetPasswdByName", got, err, tc.wantErr, tc.wantErrNotExists) }) } @@ -89,9 +107,15 @@ func TestGetPasswdByUID(t *testing.T) { }{ "Return existing user": {uid: 1111}, - "Error in database fetched content": {uid: 1111, sourceDB: "invalid.db.yaml", wantErr: true}, - "Error with typed GRPC notfound code on unexisting user": {uid: 4242, wantErr: true, wantErrNotExists: true}, - "Error on missing uid": {wantErr: true}, + "Error in database fetched content": { + uid: 1111, sourceDB: "invalid.db.yaml", wantErr: true, + }, + "Error with typed GRPC notfound code on unexisting user": { + uid: 4242, wantErr: true, wantErrNotExists: true, + }, + "Error on missing uid": { + wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -141,9 +165,15 @@ func TestGetGroupByName(t *testing.T) { }{ "Return existing group": {groupname: "group1"}, - "Error in database fetched content": {groupname: "group1", sourceDB: "invalid.db.yaml", wantErr: true}, - "Error with typed GRPC notfound code on unexisting user": {groupname: "does-not-exists", wantErr: true, wantErrNotExists: true}, - "Error on missing name": {wantErr: true}, + "Error in database fetched content": { + groupname: "group1", sourceDB: "invalid.db.yaml", wantErr: true, + }, + "Error with typed GRPC notfound code on unexisting user": { + groupname: "does-not-exists", wantErr: true, wantErrNotExists: true, + }, + "Error on missing name": { + wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -169,9 +199,15 @@ func TestGetGroupByGID(t *testing.T) { }{ "Return existing group": {gid: 11111}, - "Error in database fetched content": {gid: 1111, sourceDB: "invalid.db.yaml", wantErr: true}, - "Error with typed GRPC notfound code on unexisting user": {gid: 4242, wantErr: true, wantErrNotExists: true}, - "Error on missing uid": {wantErr: true}, + "Error in database fetched content": { + gid: 1111, sourceDB: "invalid.db.yaml", wantErr: true, + }, + "Error with typed GRPC notfound code on unexisting user": { + gid: 4242, wantErr: true, wantErrNotExists: true, + }, + "Error on missing uid": { + wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -222,10 +258,18 @@ func TestGetShadowByName(t *testing.T) { }{ "Return existing user": {username: "user1"}, - "Error when not root": {currentUserNotRoot: true, username: "user1", wantErr: true}, - "Error in database fetched content": {username: "user1", sourceDB: "invalid.db.yaml", wantErr: true}, - "Error with typed GRPC notfound code on unexisting user": {username: "does-not-exists", wantErr: true, wantErrNotExists: true}, - "Error on missing name": {wantErr: true}, + "Error when not root": { + currentUserNotRoot: true, username: "user1", wantErr: true, + }, + "Error in database fetched content": { + username: "user1", sourceDB: "invalid.db.yaml", wantErr: true, + }, + "Error with typed GRPC notfound code on unexisting user": { + username: "does-not-exists", wantErr: true, wantErrNotExists: true, + }, + "Error on missing name": { + wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -289,9 +333,14 @@ func newNSSClient(t *testing.T, sourceDB string, currentUserNotRoot bool) (clien } pm := permissions.New(opts...) - service := nss.NewService(context.Background(), newUserManagerForTests(t, sourceDB), newBrokersManagerForTests(t), &pm) + service := nss.NewService(context.Background(), + newUserManagerForTests(t, sourceDB), + newBrokersManagerForTests(t), + &pm) - grpcServer := grpc.NewServer(permissions.WithUnixPeerCreds(), grpc.ChainUnaryInterceptor(enableCheckGlobalAccess(service), errmessages.RedactErrorInterceptor)) + grpcServer := grpc.NewServer(permissions.WithUnixPeerCreds(), + grpc.ChainUnaryInterceptor(enableCheckGlobalAccess(service), + errmessages.RedactErrorInterceptor)) authd.RegisterNSSServer(grpcServer, service) done := make(chan struct{}) go func() { @@ -312,7 +361,7 @@ func newNSSClient(t *testing.T, sourceDB string, currentUserNotRoot bool) (clien } func enableCheckGlobalAccess(s nss.Service) grpc.UnaryServerInterceptor { - return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) { + return func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) { if err := s.CheckGlobalAccess(ctx, info.FullMethod); err != nil { return nil, err } @@ -354,50 +403,58 @@ func newBrokersManagerForTests(t *testing.T) *brokers.Manager { } // requireExpectedResult asserts expected behaviour from any get* NSS requests and can update them from golden content. -func requireExpectedResult[T authd.PasswdEntry | authd.GroupEntry | authd.ShadowEntry](t *testing.T, funcName string, got *T, err error, wantErr, wantErrNotExists bool) { +func requireExpectedResult[T authd.PasswdEntry | authd.GroupEntry | authd.ShadowEntry]( + t *testing.T, funcName string, got *T, err error, wantErr, wantErrNotExists bool) { t.Helper() if wantErr { - require.Error(t, err, fmt.Sprintf("%s should return an error but did not", funcName)) + require.Error(t, err, "%s should return an error but did not", funcName) s, ok := status.FromError(err) require.True(t, ok, "The error is always a GRPC error") if wantErrNotExists { - require.Equal(t, codes.NotFound, s.Code(), fmt.Sprintf("%s should return NotFound error", funcName)) + require.Equal(t, codes.NotFound, s.Code(), "%s should return NotFound error", funcName) } return } - require.NoError(t, err, fmt.Sprintf("%s should not return an error, but did", funcName)) + require.NoError(t, err, "%s should not return an error, but did", funcName) want := testutils.LoadWithUpdateFromGoldenYAML(t, got) - requireExportedEquals(t, want, got, fmt.Sprintf("%s should return the expected entry, but did not", funcName)) + requireExportedEquals(t, want, got, fmt.Sprintf( + "%s should return the expected entry, but did not", funcName)) } -// requireExpectedEntriesResult asserts expected behaviour from any get* NSS request returning a list and can update them from golden content. -func requireExpectedEntriesResult[T authd.PasswdEntry | authd.GroupEntry | authd.ShadowEntry](t *testing.T, funcName string, got []*T, err error, wantErr bool) { +// requireExpectedEntriesResult asserts expected behaviour from any get* NSS request returning a list +// and can update them from golden content. +func requireExpectedEntriesResult[T authd.PasswdEntry | authd.GroupEntry | authd.ShadowEntry]( + t *testing.T, funcName string, got []*T, err error, wantErr bool) { t.Helper() if wantErr { - require.Error(t, err, fmt.Sprintf("%s should return an error but did not", funcName)) + require.Error(t, err, "%s should return an error but did not", funcName) s, ok := status.FromError(err) require.True(t, ok, "The error is always a GRPC error") - require.NotEqual(t, codes.NotFound, s.Code(), fmt.Sprintf("%s should never return NotFound error even with empty list", funcName)) + require.NotEqual(t, codes.NotFound, s.Code(), + "%s should never return NotFound error even with empty list", funcName) return } - require.NoError(t, err, fmt.Sprintf("%s should not return an error, but did", funcName)) + require.NoError(t, err, "%s should not return an error, but did", funcName) want := testutils.LoadWithUpdateFromGoldenYAML(t, got) if len(want) != len(got) { - require.Equal(t, len(want), len(got), "Not the expected number of elements in the list. Wanted: %v\nGot: %v", want, got) + require.Equal(t, len(want), len(got), + "Not the expected number of elements in the list. Wanted: %v\nGot: %v", want, got) } for i, e := range got { - requireExportedEquals(t, want[i], e, fmt.Sprintf("%s should return the expected entry in the list, but did not", funcName)) + requireExportedEquals(t, want[i], e, fmt.Sprintf( + "%s should return the expected entry in the list, but did not", funcName)) } } // requireExportedEquals compare *want to *got, only using the exported fields. // It helps ensuring that we don’t end up in a lockcopy vetting warning when we directly // compare the exported fields with require.EqualExportedValues. -func requireExportedEquals[T authd.PasswdEntry | authd.GroupEntry | authd.ShadowEntry](t *testing.T, want *T, got *T, msg string) { +func requireExportedEquals[T authd.PasswdEntry | authd.GroupEntry | authd.ShadowEntry]( + t *testing.T, want *T, got *T, msg string) { t.Helper() data, err := yaml.Marshal(got) diff --git a/internal/services/pam/pam.go b/internal/services/pam/pam.go index 1e0db10af..13ede7d61 100644 --- a/internal/services/pam/pam.go +++ b/internal/services/pam/pam.go @@ -31,7 +31,8 @@ type Service struct { } // NewService returns a new PAM GRPC service. -func NewService(ctx context.Context, userManager *users.Manager, brokerManager *brokers.Manager, permissionManager *permissions.Manager) Service { +func NewService(ctx context.Context, userManager *users.Manager, + brokerManager *brokers.Manager, permissionManager *permissions.Manager) Service { log.Debug(ctx, "Building new GRPC PAM service") return Service{ @@ -72,7 +73,8 @@ func (s Service) GetPreviousBroker(ctx context.Context, req *authd.GPBRequest) ( // autoselection silently in authd. // User not in cache, if there is only the local broker available, return this one without saving it. if len(s.brokerManager.AvailableBrokers()) == 1 { - log.Debugf(ctx, "User %q is not handled by authd and only local broker: select it.", req.GetUsername()) + log.Debugf(ctx, "User %q is not handled by authd and only local broker: select it.", + req.GetUsername()) return &authd.GPBResponse{PreviousBroker: brokers.LocalBrokerName}, nil } @@ -98,7 +100,8 @@ func (s Service) GetPreviousBroker(ctx context.Context, req *authd.GPBRequest) ( // Updates manager memory to stop needing to query the database for the broker. if err = s.brokerManager.SetDefaultBrokerForUser(brokerID, req.GetUsername()); err != nil { - log.Warningf(ctx, "Last broker used by %q is not available, letting the user selecting one: %v", req.GetUsername(), err) + log.Warningf(ctx, "Last broker used by %q is not available, letting the user selecting one: %v", + req.GetUsername(), err) return &authd.GPBResponse{}, nil } @@ -147,8 +150,10 @@ func (s Service) SelectBroker(ctx context.Context, req *authd.SBRequest) (resp * }, err } -// GetAuthenticationModes fetches a list of authentication modes supported by the broker depending on the session information. -func (s Service) GetAuthenticationModes(ctx context.Context, req *authd.GAMRequest) (resp *authd.GAMResponse, err error) { +// GetAuthenticationModes fetches a list of authentication modes supported by the broker +// depending on the session information. +func (s Service) GetAuthenticationModes(ctx context.Context, req *authd.GAMRequest) ( + resp *authd.GAMResponse, err error) { defer decorate.OnError(&err, "could not get authentication modes") sessionID := req.GetSessionId() @@ -189,7 +194,8 @@ func (s Service) GetAuthenticationModes(ctx context.Context, req *authd.GAMReque } // SelectAuthenticationMode set given authentication mode as selected for this sessionID to the broker. -func (s Service) SelectAuthenticationMode(ctx context.Context, req *authd.SAMRequest) (resp *authd.SAMResponse, err error) { +func (s Service) SelectAuthenticationMode(ctx context.Context, req *authd.SAMRequest) ( + resp *authd.SAMResponse, err error) { defer decorate.OnError(&err, "can't select authentication mode") sessionID := req.GetSessionId() diff --git a/internal/services/pam/pam_test.go b/internal/services/pam/pam_test.go index d8fbb798d..b30a2afbf 100644 --- a/internal/services/pam/pam_test.go +++ b/internal/services/pam/pam_test.go @@ -129,15 +129,29 @@ func TestGetPreviousBroker(t *testing.T) { wantBroker string wantErr bool }{ - "Success getting previous broker": {user: "userwithbroker", wantBroker: mockBrokerGeneratedID}, - "For local user, get local broker": {user: currentUsername, wantBroker: brokers.LocalBrokerName}, - "For unmanaged user and only one broker, get local broker": {user: "nonexistent", onlyLocalBroker: true, wantBroker: brokers.LocalBrokerName}, - - "Returns empty when user does not exist": {user: "nonexistent", wantBroker: ""}, - "Returns empty when user does not have a broker": {user: "userwithoutbroker", wantBroker: ""}, - "Returns empty when broker is not available": {user: "userwithinactivebroker", wantBroker: ""}, - - "Error when not root": {user: "userwithbroker", currentUserNotRoot: true, wantErr: true}, + "Success getting previous broker": { + user: "userwithbroker", wantBroker: mockBrokerGeneratedID, + }, + "For local user, get local broker": { + user: currentUsername, wantBroker: brokers.LocalBrokerName, + }, + "For unmanaged user and only one broker, get local broker": { + user: "nonexistent", onlyLocalBroker: true, wantBroker: brokers.LocalBrokerName, + }, + + "Returns empty when user does not exist": { + user: "nonexistent", wantBroker: "", + }, + "Returns empty when user does not have a broker": { + user: "userwithoutbroker", wantBroker: "", + }, + "Returns empty when broker is not available": { + user: "userwithinactivebroker", wantBroker: "", + }, + + "Error when not root": { + user: "userwithbroker", currentUserNotRoot: true, wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -175,7 +189,8 @@ func TestGetPreviousBroker(t *testing.T) { } require.NoError(t, err, "GetPreviousBroker should not return an error, but did") - require.Equal(t, tc.wantBroker, gotResp.GetPreviousBroker(), "GetPreviousBroker should return expected broker") + require.Equal(t, tc.wantBroker, gotResp.GetPreviousBroker(), + "GetPreviousBroker should return expected broker") }) } } @@ -195,14 +210,30 @@ func TestSelectBroker(t *testing.T) { "Successfully select a broker and creates auth session": {username: "success"}, "Successfully select a broker and creates passwd session": {username: "success", sessionMode: "passwd"}, - "Error when not root": {username: "success", currentUserNotRoot: true, wantErr: true}, - "Error when username is empty": {wantErr: true}, - "Error when mode is empty": {sessionMode: "-", wantErr: true}, - "Error when mode does not exist": {sessionMode: "does not exist", wantErr: true}, - "Error when brokerID is empty": {username: "empty broker", brokerID: "-", wantErr: true}, - "Error when broker does not exist": {username: "no broker", brokerID: "does not exist", wantErr: true}, - "Error when broker does not provide a session ID": {username: "NS_no_id", wantErr: true}, - "Error when starting the session": {username: "NS_error", wantErr: true}, + "Error when not root": { + username: "success", currentUserNotRoot: true, wantErr: true, + }, + "Error when username is empty": { + wantErr: true, + }, + "Error when mode is empty": { + sessionMode: "-", wantErr: true, + }, + "Error when mode does not exist": { + sessionMode: "does not exist", wantErr: true, + }, + "Error when brokerID is empty": { + username: "empty broker", brokerID: "-", wantErr: true, + }, + "Error when broker does not exist": { + username: "no broker", brokerID: "does not exist", wantErr: true, + }, + "Error when broker does not provide a session ID": { + username: "NS_no_id", wantErr: true, + }, + "Error when starting the session": { + username: "NS_error", wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -332,24 +363,48 @@ func TestSelectAuthenticationMode(t *testing.T) { wantErr bool }{ - "Successfully select mode with required value": {username: "SAM_success_required_entry", supportedUILayouts: []*authd.UILayout{requiredEntry}}, - "Successfully select mode with missing optional value": {username: "SAM_missing_optional_entry", supportedUILayouts: []*authd.UILayout{optionalEntry}}, + "Successfully select mode with required value": { + username: "SAM_success_required_entry", supportedUILayouts: []*authd.UILayout{requiredEntry}, + }, + "Successfully select mode with missing optional value": { + username: "SAM_missing_optional_entry", supportedUILayouts: []*authd.UILayout{optionalEntry}, + }, // service errors - "Error when not root": {username: "SAM_success_required_entry", currentUserNotRoot: true, wantErr: true}, - "Error when sessionID is empty": {sessionID: "-", wantErr: true}, - "Error when session ID is invalid": {sessionID: "invalid-session", wantErr: true}, - "Error when no authmode is selected": {sessionID: "no auth mode", authMode: "-", wantErr: true}, + "Error when not root": { + username: "SAM_success_required_entry", currentUserNotRoot: true, wantErr: true, + }, + "Error when sessionID is empty": { + sessionID: "-", wantErr: true, + }, + "Error when session ID is invalid": { + sessionID: "invalid-session", wantErr: true, + }, + "Error when no authmode is selected": { + sessionID: "no auth mode", authMode: "-", wantErr: true, + }, // broker errors - "Error when selecting invalid auth mode": {username: "SAM_error", supportedUILayouts: []*authd.UILayout{requiredEntry}, wantErr: true}, - "Error when broker does not have validators for the session": {username: "does not matter", noValidators: true, wantErr: true}, + "Error when selecting invalid auth mode": { + username: "SAM_error", supportedUILayouts: []*authd.UILayout{requiredEntry}, wantErr: true, + }, + "Error when broker does not have validators for the session": { + username: "does not matter", noValidators: true, wantErr: true, + }, /* Layout errors */ - "Error when returns no layout": {username: "SAM_no_layout", supportedUILayouts: []*authd.UILayout{requiredEntry}, wantErr: true}, - "Error when returns layout with no type": {username: "SAM_no_layout_type", supportedUILayouts: []*authd.UILayout{requiredEntry}, wantErr: true}, - "Error when returns layout without required value": {username: "SAM_missing_required_entry", supportedUILayouts: []*authd.UILayout{requiredEntry}, wantErr: true}, - "Error when returns layout with unknown field": {username: "SAM_unknown_field", supportedUILayouts: []*authd.UILayout{requiredEntry}, wantErr: true}, + "Error when returns no layout": { + username: "SAM_no_layout", supportedUILayouts: []*authd.UILayout{requiredEntry}, wantErr: true, + }, + "Error when returns layout with no type": { + username: "SAM_no_layout_type", supportedUILayouts: []*authd.UILayout{requiredEntry}, wantErr: true, + }, + "Error when returns layout without required value": { + username: "SAM_missing_required_entry", supportedUILayouts: []*authd.UILayout{requiredEntry}, wantErr: true, + }, + "Error when returns layout with unknown field": { + username: "SAM_unknown_field", supportedUILayouts: []*authd.UILayout{requiredEntry}, wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -376,7 +431,8 @@ func TestSelectAuthenticationMode(t *testing.T) { tc.authMode = "" } - // If the username does not have a SAM_something, it means we don't care about the broker answer and we don't need the validators. + // If the username does not have a SAM_something, it means we don't care about the broker answer + // and we don't need the validators. if !tc.noValidators && strings.HasPrefix(tc.username, "SAM_") { // We need to call GetAuthenticationModes to generate the layout validators on the broker. gamReq := &authd.GAMRequest{ @@ -421,18 +477,28 @@ func TestIsAuthenticated(t *testing.T) { // There is no wantErr as it's stored in the golden file. }{ - "Successfully authenticate": {username: "success"}, - "Successfully authenticate if first call is canceled": {username: "IA_second_call", secondCall: true, cancelFirstCall: true}, - "Denies authentication when broker times out": {username: "IA_timeout"}, - "Update existing DB on success": {username: "success", existingDB: "cache-with-user.db"}, - "Update local groups": {username: "success_with_local_groups", localGroupsFile: "valid.group"}, + "Successfully authenticate": {username: "success"}, + "Successfully authenticate if first call is canceled": { + username: "IA_second_call", secondCall: true, cancelFirstCall: true, + }, + "Denies authentication when broker times out": {username: "IA_timeout"}, + "Update existing DB on success": { + username: "success", existingDB: "cache-with-user.db", + }, + "Update local groups": { + username: "success_with_local_groups", localGroupsFile: "valid.group", + }, // service errors "Error when not root": {username: "success", currentUserNotRoot: true}, - "Error when UID conflicts with existing different user": {username: "conflicting-uid", existingDB: "cache-with-conflicting-uid.db"}, - "Error when GID conflicts with existing different group": {username: "conflicting-gid", existingDB: "cache-with-conflicting-gid.db"}, - "Error when sessionID is empty": {sessionID: "-"}, - "Error when there is no broker": {sessionID: "invalid-session"}, + "Error when UID conflicts with existing different user": { + username: "conflicting-uid", existingDB: "cache-with-conflicting-uid.db", + }, + "Error when GID conflicts with existing different group": { + username: "conflicting-gid", existingDB: "cache-with-conflicting-gid.db", + }, + "Error when sessionID is empty": {sessionID: "-"}, + "Error when there is no broker": {sessionID: "invalid-session"}, // broker errors "Error when authenticating": {username: "IA_error"}, @@ -443,7 +509,9 @@ func TestIsAuthenticated(t *testing.T) { "Error when calling second time without cancelling": {username: "IA_second_call", secondCall: true}, // local group error - "Error on updating local groups with unexisting file": {username: "success_with_local_groups", localGroupsFile: "does_not_exists.group"}, + "Error on updating local groups with unexisting file": { + username: "success_with_local_groups", localGroupsFile: "does_not_exists.group", + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -453,7 +521,8 @@ func TestIsAuthenticated(t *testing.T) { var destCmdsFile string if tc.localGroupsFile != "" { - destCmdsFile = localgroupstestutils.SetupGPasswdMock(t, filepath.Join(testutils.TestFamilyPath(t), tc.localGroupsFile)) + destCmdsFile = localgroupstestutils.SetupGPasswdMock(t, + filepath.Join(testutils.TestFamilyPath(t), tc.localGroupsFile)) } cacheDir := t.TempDir() @@ -523,16 +592,19 @@ func TestIsAuthenticated(t *testing.T) { got := firstCall + secondCall got = permissionstestutils.IdempotentPermissionError(got) - want := testutils.LoadWithUpdateFromGolden(t, got, testutils.WithGoldenPath(filepath.Join(testutils.GoldenPath(t), "IsAuthenticated"))) + want := testutils.LoadWithUpdateFromGolden(t, got, + testutils.WithGoldenPath(filepath.Join(testutils.GoldenPath(t), "IsAuthenticated"))) require.Equal(t, want, got, "IsAuthenticated should return the expected combined data, but did not") // Check that cache has been updated too. gotDB, err := cachetestutils.DumpToYaml(userstestutils.GetManagerCache(m)) require.NoError(t, err, "Setup: failed to dump database for comparing") - wantDB := testutils.LoadWithUpdateFromGolden(t, gotDB, testutils.WithGoldenPath(filepath.Join(testutils.GoldenPath(t), "cache.db"))) + wantDB := testutils.LoadWithUpdateFromGolden(t, gotDB, + testutils.WithGoldenPath(filepath.Join(testutils.GoldenPath(t), "cache.db"))) require.Equal(t, wantDB, gotDB, "IsAuthenticated should update the cache database as expected") - localgroupstestutils.RequireGPasswdOutput(t, destCmdsFile, filepath.Join(testutils.GoldenPath(t), "gpasswd.output")) + localgroupstestutils.RequireGPasswdOutput(t, destCmdsFile, + filepath.Join(testutils.GoldenPath(t), "gpasswd.output")) }) } } @@ -564,13 +636,15 @@ func TestIDGeneration(t *testing.T) { }) require.NoError(t, err, "Setup: failed to create session for tests") - resp, err := client.IsAuthenticated(context.Background(), &authd.IARequest{SessionId: sbResp.GetSessionId()}) + resp, err := client.IsAuthenticated(context.Background(), + &authd.IARequest{SessionId: sbResp.GetSessionId()}) require.NoError(t, err, "Setup: could not authenticate user") require.Equal(t, "granted", resp.GetAccess(), "Setup: authentication should be granted") gotDB, err := cachetestutils.DumpToYaml(userstestutils.GetManagerCache(m)) require.NoError(t, err, "Setup: failed to dump database for comparing") - wantDB := testutils.LoadWithUpdateFromGolden(t, gotDB, testutils.WithGoldenPath(filepath.Join(testutils.GoldenPath(t), "cache.db"))) + wantDB := testutils.LoadWithUpdateFromGolden(t, gotDB, + testutils.WithGoldenPath(filepath.Join(testutils.GoldenPath(t), "cache.db"))) require.Equal(t, wantDB, gotDB, "IsAuthenticated should update the cache database as expected") }) } @@ -588,7 +662,9 @@ func TestSetDefaultBrokerForUser(t *testing.T) { }{ "Set default broker for existing user with no broker": {username: "usersetbroker"}, "Update default broker for existing user with a broker": {username: "userupdatebroker"}, - "Setting local broker as default should not save on DB": {username: "userlocalbroker", brokerID: brokers.LocalBrokerName}, + "Setting local broker as default should not save on DB": { + username: "userlocalbroker", brokerID: brokers.LocalBrokerName, + }, "Error when not root": {username: "usersetbroker", currentUserNotRoot: true, wantErr: true}, "Error when username is empty": {wantErr: true}, @@ -600,7 +676,8 @@ func TestSetDefaultBrokerForUser(t *testing.T) { t.Parallel() cacheDir := t.TempDir() - cachetestutils.CreateDBFromYAML(t, filepath.Join(testutils.TestFamilyPath(t), "set-default-broker.db"), cacheDir) + cachetestutils.CreateDBFromYAML(t, + filepath.Join(testutils.TestFamilyPath(t), "set-default-broker.db"), cacheDir) m, err := users.NewManager(users.DefaultConfig, cacheDir) require.NoError(t, err, "Setup: could not create user manager") @@ -625,12 +702,14 @@ func TestSetDefaultBrokerForUser(t *testing.T) { gpbResp, err := client.GetPreviousBroker(context.Background(), &authd.GPBRequest{Username: tc.username}) require.NoError(t, err, "GetPreviousBroker should not return an error") - require.Equal(t, tc.brokerID, gpbResp.GetPreviousBroker(), "SetDefaultBrokerForUser should set the default broker as expected") + require.Equal(t, tc.brokerID, gpbResp.GetPreviousBroker(), + "SetDefaultBrokerForUser should set the default broker as expected") // Check that cache has been updated too. gotDB, err := cachetestutils.DumpToYaml(userstestutils.GetManagerCache(m)) require.NoError(t, err, "Setup: failed to dump database for comparing") - wantDB := testutils.LoadWithUpdateFromGolden(t, gotDB, testutils.WithGoldenPath(filepath.Join(testutils.GoldenPath(t), "cache.db"))) + wantDB := testutils.LoadWithUpdateFromGolden(t, gotDB, + testutils.WithGoldenPath(filepath.Join(testutils.GoldenPath(t), "cache.db"))) require.Equal(t, wantDB, gotDB, "SetDefaultBrokerForUser should update the cache database as expected") }) } @@ -719,7 +798,8 @@ func initBrokers() (brokerConfigPath string, cleanup func(), err error) { // newPAMClient returns a new GRPC PAM client for tests connected to brokerManager with the given cache and // permissionmanager. // If the one passed is nil, this function will create the cache and close it upon test teardown. -func newPamClient(t *testing.T, m *users.Manager, brokerManager *brokers.Manager, pm *permissions.Manager) (client authd.PAMClient) { +func newPamClient( + t *testing.T, m *users.Manager, brokerManager *brokers.Manager, pm *permissions.Manager) (client authd.PAMClient) { t.Helper() // socket path is limited in length. @@ -739,7 +819,9 @@ func newPamClient(t *testing.T, m *users.Manager, brokerManager *brokers.Manager service := pam.NewService(context.Background(), m, brokerManager, pm) - grpcServer := grpc.NewServer(permissions.WithUnixPeerCreds(), grpc.ChainUnaryInterceptor(enableCheckGlobalAccess(service), errmessages.RedactErrorInterceptor)) + grpcServer := grpc.NewServer(permissions.WithUnixPeerCreds(), + grpc.ChainUnaryInterceptor(enableCheckGlobalAccess(service), + errmessages.RedactErrorInterceptor)) authd.RegisterPAMServer(grpcServer, service) done := make(chan struct{}) go func() { @@ -751,7 +833,9 @@ func newPamClient(t *testing.T, m *users.Manager, brokerManager *brokers.Manager <-done }) - conn, err := grpc.NewClient("unix://"+socketPath, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithUnaryInterceptor(errmessages.FormatErrorMessage)) + conn, err := grpc.NewClient("unix://"+socketPath, + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithUnaryInterceptor(errmessages.FormatErrorMessage)) require.NoError(t, err, "Setup: Could not connect to GRPC server") t.Cleanup(func() { _ = conn.Close() }) // We don't care about the error on cleanup @@ -773,7 +857,7 @@ func newPermissionManager(t *testing.T, currentUserNotRoot bool) permissions.Man // enableCheckGlobalAccess returns the middleware hooking up in CheckGlobalAccess for the given service. func enableCheckGlobalAccess(s pam.Service) grpc.UnaryServerInterceptor { - return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) { + return func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) { if err := s.CheckGlobalAccess(ctx, info.FullMethod); err != nil { return nil, err } diff --git a/internal/services/permissions.go b/internal/services/permissions.go index 96364fa61..e77a5ac7a 100644 --- a/internal/services/permissions.go +++ b/internal/services/permissions.go @@ -7,7 +7,8 @@ import ( "google.golang.org/grpc" ) -func (m Manager) globalPermissions(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) { +func (m Manager) globalPermissions( + ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) { if strings.HasPrefix(info.FullMethod, "/authd.PAM/") { if err := m.pamService.CheckGlobalAccess(ctx, info.FullMethod); err != nil { return nil, err diff --git a/internal/services/permissions/servercreds.go b/internal/services/permissions/servercreds.go index 99010891d..d82c17573 100644 --- a/internal/services/permissions/servercreds.go +++ b/internal/services/permissions/servercreds.go @@ -18,7 +18,8 @@ func WithUnixPeerCreds() grpc.ServerOption { return grpc.Creds(serverPeerCreds{}) } -// serverPeerCreds encapsulates a TransportCredentials which extracts uid and pid of caller via Unix Socket SO_PEERCRED. +// serverPeerCreds encapsulates a TransportCredentials which extracts the +// uid and the pid of caller via Unix Socket SO_PEERCRED. type serverPeerCreds struct{} func (serverPeerCreds) ServerHandshake(conn net.Conn) (n net.Conn, c credentials.AuthInfo, err error) { @@ -59,7 +60,8 @@ func (serverPeerCreds) ServerHandshake(conn net.Conn) (n net.Conn, c credentials return conn, peerCredsInfo{uid: cred.Uid, pid: cred.Pid}, nil } -func (serverPeerCreds) ClientHandshake(_ context.Context, _ string, conn net.Conn) (net.Conn, credentials.AuthInfo, error) { +func (serverPeerCreds) ClientHandshake(_ context.Context, _ string, conn net.Conn) ( + net.Conn, credentials.AuthInfo, error) { return conn, nil, nil } func (serverPeerCreds) Info() credentials.ProtocolInfo { return credentials.ProtocolInfo{} } diff --git a/internal/testsdetection/testsdetection_test.go b/internal/testsdetection/testsdetection_test.go index c8d9e41f5..39549a705 100644 --- a/internal/testsdetection/testsdetection_test.go +++ b/internal/testsdetection/testsdetection_test.go @@ -29,9 +29,15 @@ func TestMustBeTestingInProcess(t *testing.T) { wantPanic bool }{ - "Pass when called in an integration tests build": {integrationtestsTag: true, wantPanic: false}, + "Pass when called in an integration tests build": { + integrationtestsTag: true, + wantPanic: false, + }, - "Error (panics) when called in non tests and no integration tests": {integrationtestsTag: false, wantPanic: true}, + "Error (panics) when called in non tests and no integration tests": { + integrationtestsTag: false, + wantPanic: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -58,7 +64,8 @@ func TestMustBeTestingInProcess(t *testing.T) { require.Errorf(t, err, "MustBeTesting should have panicked the subprocess: %s", out) return } - require.NoErrorf(t, err, "MustBeTesting should have returned without panicking the subprocess: %s", out) + require.NoErrorf(t, err, "MustBeTesting should have returned without panicking the subprocess: %s", + out) }) } } diff --git a/internal/testutils/broker.go b/internal/testutils/broker.go index 75bf3371c..8deacab41 100644 --- a/internal/testutils/broker.go +++ b/internal/testutils/broker.go @@ -133,7 +133,8 @@ func (b *BrokerBusMock) NewSession(username, lang, mode string) (sessionID, encr } // GetAuthenticationModes returns default values to be used in tests or an error if requested. -func (b *BrokerBusMock) GetAuthenticationModes(sessionID string, supportedUILayouts []map[string]string) (authenticationModes []map[string]string, dbusErr *dbus.Error) { +func (b *BrokerBusMock) GetAuthenticationModes(sessionID string, supportedUILayouts []map[string]string) ( + authenticationModes []map[string]string, dbusErr *dbus.Error) { sessionID = parseSessionID(sessionID) switch sessionID { case "GAM_invalid": @@ -157,7 +158,8 @@ func (b *BrokerBusMock) GetAuthenticationModes(sessionID string, supportedUILayo } // SelectAuthenticationMode returns default values to be used in tests or an error if requested. -func (b *BrokerBusMock) SelectAuthenticationMode(sessionID, authenticationModeName string) (uiLayoutInfo map[string]string, dbusErr *dbus.Error) { +func (b *BrokerBusMock) SelectAuthenticationMode(sessionID, authenticationModeName string) ( + uiLayoutInfo map[string]string, dbusErr *dbus.Error) { sessionID = parseSessionID(sessionID) switch sessionID { case "SAM_success_required_entry": @@ -210,20 +212,24 @@ func (b *BrokerBusMock) SelectAuthenticationMode(sessionID, authenticationModeNa } // IsAuthenticated returns default values to be used in tests or an error if requested. -func (b *BrokerBusMock) IsAuthenticated(sessionID, authenticationData string) (access, data string, dbusErr *dbus.Error) { +func (b *BrokerBusMock) IsAuthenticated(sessionID, authenticationData string) ( + access, data string, dbusErr *dbus.Error) { // The IsAuthenticated needs to function a bit differently to still allow tests to be executed in parallel. // We have to use both the prefixed sessionID and the parsed one in order to differentiate between test cases. parsedID := parseSessionID(sessionID) if parsedID == "IA_error" { - return "", "", dbus.MakeFailedError(fmt.Errorf("broker %q: IsAuthenticated errored out", b.name)) + return "", "", dbus.MakeFailedError( + fmt.Errorf("broker %q: IsAuthenticated errored out", b.name)) } // Handles the context that will be assigned for the IsAuthenticated handler b.isAuthenticatedCallsMu.RLock() if _, exists := b.isAuthenticatedCalls[sessionID]; exists { b.isAuthenticatedCallsMu.RUnlock() - return "", "", dbus.MakeFailedError(fmt.Errorf("broker %q: IsAuthenticated already running for session %q", b.name, sessionID)) + return "", "", dbus.MakeFailedError( + fmt.Errorf("broker %q: IsAuthenticated already running for session %q", + b.name, sessionID)) } b.isAuthenticatedCallsMu.RUnlock() @@ -334,8 +340,9 @@ func (b *BrokerBusMock) UserPreCheck(username string) (userinfo string, dbusErr // parseSessionID is wrapper around the sessionID to remove some values appended during the tests. // -// The sessionID can have multiple values appended to differentiate between subtests and avoid concurrency conflicts, -// and only the last value (i.e. "..._separator_ID-session_id") will be considered. +// The sessionID can have multiple values appended to differentiate between subtests and +// avoid concurrency conflicts, and only the last value (i.e. "..._separator_ID-session_id") +// will be considered. func parseSessionID(sessionID string) string { cut := strings.Split(sessionID, IDSeparator) if len(cut) == 0 { diff --git a/internal/testutils/daemon.go b/internal/testutils/daemon.go index e319a85f1..fde61caf4 100644 --- a/internal/testutils/daemon.go +++ b/internal/testutils/daemon.go @@ -56,9 +56,10 @@ func WithEnvironment(env ...string) DaemonOption { } } -// RunDaemon runs the daemon in a separate process and returns the socket path and a channel that will be closed when -// the daemon stops. -func RunDaemon(ctx context.Context, t *testing.T, execPath string, args ...DaemonOption) (socketPath string, stopped chan struct{}) { +// RunDaemon runs the daemon in a separate process and returns the socket path and a channel +// that will be closed when the daemon stops. +func RunDaemon(ctx context.Context, t *testing.T, execPath string, args ...DaemonOption) ( + socketPath string, stopped chan struct{}) { t.Helper() opts := &daemonOptions{} @@ -92,7 +93,8 @@ paths: `, opts.cachePath, opts.socketPath) configPath := filepath.Join(tempDir, "testconfig.yaml") - require.NoError(t, os.WriteFile(configPath, []byte(config), 0600), "Setup: failed to create config file for tests") + require.NoError(t, os.WriteFile(configPath, []byte(config), 0600), + "Setup: failed to create config file %q for tests", configPath) // #nosec:G204 - we control the command arguments in tests cmd := exec.CommandContext(ctx, execPath, "-c", configPath) @@ -114,7 +116,9 @@ paths: t.Logf("Daemon stopped (%v)\n ##### STDOUT #####\n %s \n ##### END #####", err, out) }() - conn, err := grpc.NewClient("unix://"+opts.socketPath, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithUnaryInterceptor(errmessages.FormatErrorMessage)) + conn, err := grpc.NewClient("unix://"+opts.socketPath, + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithUnaryInterceptor(errmessages.FormatErrorMessage)) require.NoError(t, err, "Setup: could not connect to the daemon on %s", opts.socketPath) defer conn.Close() diff --git a/internal/testutils/dbus.go b/internal/testutils/dbus.go index 116a3c706..1a875bf41 100644 --- a/internal/testutils/dbus.go +++ b/internal/testutils/dbus.go @@ -134,12 +134,13 @@ func StartSystemBusMock() (func(), error) { }, nil } -// GetSystemBusConnection returns a connection to the system bus with a safety check to avoid mistakenly connecting to the -// actual system bus. +// GetSystemBusConnection returns a connection to the system bus with a safety check +// to avoid mistakenly connecting to the actual system bus. func GetSystemBusConnection(t *testing.T) (*dbus.Conn, error) { t.Helper() if !isRunning() { - return nil, errors.New("system bus mock is not running. If that's intended, manually connect to the system bus instead of using this function") + return nil, errors.New("system bus mock is not running. " + + "If that's intended, manually connect to the system bus instead of using this function") } conn, err := dbus.ConnectSystemBus() if err != nil { diff --git a/internal/users/cache/db_test.go b/internal/users/cache/db_test.go index a64d9bc63..c4dd2ab8c 100644 --- a/internal/users/cache/db_test.go +++ b/internal/users/cache/db_test.go @@ -31,10 +31,14 @@ func TestNew(t *testing.T) { "New recreates any missing buckets and delete unknowns": {dbFile: "database_with_unknown_bucket"}, "New removes orphaned user records from UserByID bucket": {dbFile: "orphaned_user_record"}, - "Error on cacheDir non existent cacheDir": {dbFile: "-", wantErr: true}, - "Error on corrupted db file": {corruptedDbFile: true, wantErr: true}, - "Error on invalid permission on database file": {dbFile: "multiple_users_and_groups", perm: &perm0644, wantErr: true}, - "Error on unreadable database file": {dbFile: "multiple_users_and_groups", perm: &perm0000, wantErr: true}, + "Error on cacheDir non existent cacheDir": {dbFile: "-", wantErr: true}, + "Error on corrupted db file": {corruptedDbFile: true, wantErr: true}, + "Error on invalid permission on database file": { + dbFile: "multiple_users_and_groups", perm: &perm0644, wantErr: true, + }, + "Error on unreadable database file": { + dbFile: "multiple_users_and_groups", perm: &perm0000, wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -100,7 +104,8 @@ func TestUpdateUserEntry(t *testing.T) { Gecos: "User1 gecos\nOn multiple lines", Dir: "/home/user1", Shell: "/bin/bash", - // These values don't matter. We just want to make sure they are the same as the ones provided by the manager. + // These values don't matter: + // we just want to make sure they are the same as the ones provided by the manager. LastPwdChange: -1, MaxPwdAge: -1, PwdWarnPeriod: -1, PwdInactivity: -1, MinPwdAge: -1, ExpirationDate: -1, }, "user1-new-attributes": { @@ -109,7 +114,8 @@ func TestUpdateUserEntry(t *testing.T) { Gecos: "New user1 gecos", Dir: "/home/user1", Shell: "/bin/dash", - // These values don't matter. We just want to make sure they are the same as the ones provided by the manager. + // These values don't matter: + // we just want to make sure they are the same as the ones provided by the manager. LastPwdChange: -1, MaxPwdAge: -1, PwdWarnPeriod: -1, PwdInactivity: -1, MinPwdAge: -1, ExpirationDate: -1, }, "user1-new-name": { @@ -118,7 +124,8 @@ func TestUpdateUserEntry(t *testing.T) { Gecos: "User1 gecos\nOn multiple lines", Dir: "/home/user1", Shell: "/bin/bash", - // These values don't matter. We just want to make sure they are the same as the ones provided by the manager. + // These values don't matter: + // we just want to make sure they are the same as the ones provided by the manager. LastPwdChange: -1, MaxPwdAge: -1, PwdWarnPeriod: -1, PwdInactivity: -1, MinPwdAge: -1, ExpirationDate: -1, }, "user1-new-homedir": { @@ -127,7 +134,8 @@ func TestUpdateUserEntry(t *testing.T) { Gecos: "User1 gecos\nOn multiple lines", Dir: "/new/home/user1", Shell: "/bin/bash", - // These values don't matter. We just want to make sure they are the same as the ones provided by the manager. + // These values don't matter: + // we just want to make sure they are the same as the ones provided by the manager. LastPwdChange: -1, MaxPwdAge: -1, PwdWarnPeriod: -1, PwdInactivity: -1, MinPwdAge: -1, ExpirationDate: -1, }, "user1-without-gecos": { @@ -135,7 +143,8 @@ func TestUpdateUserEntry(t *testing.T) { UID: 1111, Dir: "/home/user1", Shell: "/bin/bash", - // These values don't matter. We just want to make sure they are the same as the ones provided by the manager. + // These values don't matter: + // we just want to make sure they are the same as the ones provided by the manager. LastPwdChange: -1, MaxPwdAge: -1, PwdWarnPeriod: -1, PwdInactivity: -1, MinPwdAge: -1, ExpirationDate: -1, }, "user3": { @@ -144,7 +153,8 @@ func TestUpdateUserEntry(t *testing.T) { Gecos: "User3 gecos", Dir: "/home/user3", Shell: "/bin/zsh", - // These values don't matter. We just want to make sure they are the same as the ones provided by the manager. + // These values don't matter: + // we just want to make sure they are the same as the ones provided by the manager. LastPwdChange: -1, MaxPwdAge: -1, PwdWarnPeriod: -1, PwdInactivity: -1, MinPwdAge: -1, ExpirationDate: -1, }, } @@ -163,41 +173,83 @@ func TestUpdateUserEntry(t *testing.T) { wantErr bool }{ // New user - "Insert new user": {}, - "Update last login time for user": {dbFile: "one_user_and_group"}, - "Insert new user without optional gecos field": {userCase: "user1-without-gecos"}, + "Insert new user": {}, + "Update last login time for user": {dbFile: "one_user_and_group"}, + "Insert new user without optional gecos field": { + userCase: "user1-without-gecos", + }, // User and Group updates - "Update user by changing attributes": {userCase: "user1-new-attributes", dbFile: "one_user_and_group"}, - "Update user does not change homedir if it exists": {userCase: "user1-new-homedir", dbFile: "one_user_and_group"}, - "Update user by removing optional gecos field if not set": {userCase: "user1-without-gecos", dbFile: "one_user_and_group"}, + "Update user by changing attributes": { + userCase: "user1-new-attributes", dbFile: "one_user_and_group", + }, + "Update user does not change homedir if it exists": { + userCase: "user1-new-homedir", dbFile: "one_user_and_group", + }, + "Update user by removing optional gecos field if not set": { + userCase: "user1-without-gecos", dbFile: "one_user_and_group", + }, // Group updates - "Update user by adding a new group": {groupCases: []string{"group1", "group2"}, dbFile: "one_user_and_group"}, - "Update user by adding a new default group": {groupCases: []string{"group2", "group1"}, dbFile: "one_user_and_group"}, - "Remove group from user": {groupCases: []string{"group2"}, dbFile: "one_user_and_group"}, + "Update user by adding a new group": { + groupCases: []string{"group1", "group2"}, dbFile: "one_user_and_group", + }, + "Update user by adding a new default group": { + groupCases: []string{"group2", "group1"}, dbFile: "one_user_and_group", + }, + "Remove group from user": { + groupCases: []string{"group2"}, dbFile: "one_user_and_group", + }, // Multi users handling - "Update only user even if we have multiple of them": {dbFile: "multiple_users_and_groups"}, - "Add user to group from another user": {groupCases: []string{"group1", "group2"}, dbFile: "multiple_users_and_groups"}, - "Remove user from a group still part from another user": {userCase: "user3", groupCases: []string{"group3"}, dbFile: "multiple_users_and_groups"}, + "Update only user even if we have multiple of them": { + dbFile: "multiple_users_and_groups", + }, + "Add user to group from another user": { + groupCases: []string{"group1", "group2"}, dbFile: "multiple_users_and_groups", + }, + "Remove user from a group still part from another user": { + userCase: "user3", groupCases: []string{"group3"}, dbFile: "multiple_users_and_groups", + }, // Allowed inconsistent cases - "Invalid value entry in groupByName recreates entries": {dbFile: "invalid_entry_in_groupByName"}, - "Invalid value entry in userByName recreates entries": {dbFile: "invalid_entry_in_userByName"}, - "Invalid value entries in other user and groups don't impact current request": {dbFile: "invalid_entries_but_user_and_group1"}, + "Invalid value entry in groupByName recreates entries": { + dbFile: "invalid_entry_in_groupByName", + }, + "Invalid value entry in userByName recreates entries": { + dbFile: "invalid_entry_in_userByName", + }, + "Invalid value entries in other user and groups don't impact current request": { + dbFile: "invalid_entries_but_user_and_group1", + }, // Renaming errors - "Error when user has conflicting uid": {userCase: "user1-new-name", dbFile: "one_user_and_group", wantErr: true}, - "Error when group has conflicting gid": {groupCases: []string{"newgroup1"}, dbFile: "one_user_and_group", wantErr: true}, + "Error when user has conflicting uid": { + userCase: "user1-new-name", dbFile: "one_user_and_group", wantErr: true, + }, + "Error when group has conflicting gid": { + groupCases: []string{"newgroup1"}, dbFile: "one_user_and_group", wantErr: true, + }, // Error cases - "Error on invalid value entry in groupByID": {dbFile: "invalid_entry_in_groupByID", wantErr: true}, - "Error on invalid value entry in userByID": {dbFile: "invalid_entry_in_userByID", wantErr: true}, - "Error on invalid value entry in userToGroups": {dbFile: "invalid_entry_in_userToGroups", wantErr: true}, - "Error on invalid value entry in groupToUsers": {dbFile: "invalid_entry_in_groupToUsers", wantErr: true}, - "Error on invalid value entry in groupToUsers for user dropping from group": {dbFile: "invalid_entry_in_groupToUsers_secondary_group", wantErr: true}, - "Error on invalid value entry in groupByID for user dropping from group": {dbFile: "invalid_entry_in_groupByID_secondary_group", wantErr: true}, + "Error on invalid value entry in groupByID": { + dbFile: "invalid_entry_in_groupByID", wantErr: true, + }, + "Error on invalid value entry in userByID": { + dbFile: "invalid_entry_in_userByID", wantErr: true, + }, + "Error on invalid value entry in userToGroups": { + dbFile: "invalid_entry_in_userToGroups", wantErr: true, + }, + "Error on invalid value entry in groupToUsers": { + dbFile: "invalid_entry_in_groupToUsers", wantErr: true, + }, + "Error on invalid value entry in groupToUsers for user dropping from group": { + dbFile: "invalid_entry_in_groupToUsers_secondary_group", wantErr: true, + }, + "Error on invalid value entry in groupByID for user dropping from group": { + dbFile: "invalid_entry_in_groupByID_secondary_group", wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -327,7 +379,10 @@ func TestGroupByID(t *testing.T) { "Error on missing group": {wantErrType: cache.NoDataFoundError{}}, "Error on invalid database entry": {dbFile: "invalid_entry_in_groupByID", wantErr: true}, - "Error as missing userByID": {dbFile: "partially_valid_multiple_users_and_groups_groupByID_groupToUsers", wantErr: true}, + "Error as missing userByID": { + dbFile: "partially_valid_multiple_users_and_groups_groupByID_groupToUsers", + wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -354,7 +409,10 @@ func TestGroupByName(t *testing.T) { "Error on missing group": {wantErrType: cache.NoDataFoundError{}}, "Error on invalid database entry": {dbFile: "invalid_entry_in_groupByName", wantErr: true}, - "Error as missing userByID": {dbFile: "partially_valid_multiple_users_and_groups_groupByID_groupToUsers", wantErr: true}, + "Error as missing userByID": { + dbFile: "partially_valid_multiple_users_and_groups_groupByID_groupToUsers", + wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -380,11 +438,19 @@ func TestAllGroups(t *testing.T) { "Get one group": {dbFile: "one_user_and_group"}, "Get multiple groups": {dbFile: "multiple_users_and_groups"}, - "Get groups rely on groupByID, groupToUsers, UserByID": {dbFile: "partially_valid_multiple_users_and_groups_groupByID_groupToUsers_UserByID"}, + "Get groups rely on groupByID, groupToUsers, UserByID": { + dbFile: "partially_valid_multiple_users_and_groups_groupByID_groupToUsers_UserByID", + }, - "Error on some invalid groups entry": {dbFile: "invalid_entries_but_user_and_group1", wantErr: true}, - "Error as not only relying on groupByID": {dbFile: "partially_valid_multiple_users_and_groups_only_groupByID", wantErr: true}, - "Error as missing userByID": {dbFile: "partially_valid_multiple_users_and_groups_groupByID_groupToUsers", wantErr: true}, + "Error on some invalid groups entry": { + dbFile: "invalid_entries_but_user_and_group1", wantErr: true, + }, + "Error as not only relying on groupByID": { + dbFile: "partially_valid_multiple_users_and_groups_only_groupByID", wantErr: true, + }, + "Error as missing userByID": { + dbFile: "partially_valid_multiple_users_and_groups_groupByID_groupToUsers", wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -445,7 +511,8 @@ func TestRemoveDb(t *testing.T) { require.NoFileExists(t, cacheDir, "RemoveDb should remove the database file") // Second call should return ErrNotExist as the database file was already removed. - require.ErrorIs(t, cache.RemoveDb(cacheDir), fs.ErrNotExist, "RemoveDb should return os.ErrNotExist on the second call") + require.ErrorIs(t, cache.RemoveDb(cacheDir), fs.ErrNotExist, + "RemoveDb should return os.ErrNotExist on the second call") } func TestDeleteUser(t *testing.T) { diff --git a/internal/users/cache/update.go b/internal/users/cache/update.go index f6b2d35e7..3d6e43cda 100644 --- a/internal/users/cache/update.go +++ b/internal/users/cache/update.go @@ -47,7 +47,8 @@ func (c *Cache) UpdateUserEntry(usr UserDB, groupContents []GroupDB) error { } /* 3. Users and groups mapping buckets */ - if err := updateUsersAndGroups(buckets, userDB.UID, groupContents, previousGroupsForCurrentUser.GIDs); err != nil { + if err := updateUsersAndGroups(buckets, userDB.UID, groupContents, + previousGroupsForCurrentUser.GIDs); err != nil { return err } @@ -72,12 +73,14 @@ func updateUser(buckets map[string]bucketWithName, userContent userDB) error { // Ensure that we use the same homedir as the one we have in cache. if existingUser.Dir != "" && existingUser.Dir != userContent.Dir { - slog.Warn(fmt.Sprintf("User %q already has a homedir. The existing %q one will be kept instead of %q", userContent.Name, existingUser.Dir, userContent.Dir)) + slog.Warn(fmt.Sprintf("User %q already has a homedir. The existing %q one will be kept instead of %q", + userContent.Name, existingUser.Dir, userContent.Dir)) userContent.Dir = existingUser.Dir } // Update user buckets - log.Debug(context.Background(), fmt.Sprintf("Updating entry of user %q (UID: %d)", userContent.Name, userContent.UID)) + log.Debug(context.Background(), fmt.Sprintf("Updating entry of user %q (UID: %d)", + userContent.Name, userContent.UID)) updateBucket(buckets[userByIDBucketName], userContent.UID, userContent) updateBucket(buckets[userByNameBucketName], userContent.Name, userContent) @@ -94,21 +97,27 @@ func updateGroups(buckets map[string]bucketWithName, groupContents []GroupDB) er // If a group with the same GID exists, we need to ensure that it's the same group or fail the update otherwise. if existingGroup.Name != "" && existingGroup.Name != groupContent.Name { - slog.Error(fmt.Sprintf("GID %d for group %q already in use by group %q", groupContent.GID, groupContent.Name, existingGroup.Name)) + slog.Error(fmt.Sprintf("GID %d for group %q already in use by group %q", + groupContent.GID, groupContent.Name, existingGroup.Name)) return fmt.Errorf("GID for group %q already in use by a different group", groupContent.Name) } // Update group buckets - updateBucket(buckets[groupByIDBucketName], groupContent.GID, groupDB{Name: groupContent.Name, GID: groupContent.GID}) - updateBucket(buckets[groupByNameBucketName], groupContent.Name, groupDB{Name: groupContent.Name, GID: groupContent.GID}) + updateBucket(buckets[groupByIDBucketName], groupContent.GID, groupDB{ + Name: groupContent.Name, GID: groupContent.GID, + }) + updateBucket(buckets[groupByNameBucketName], groupContent.Name, groupDB{ + Name: groupContent.Name, GID: groupContent.GID, + }) } return nil } -// updateUserAndGroups updates the pivot table for user to groups and group to users. It handles any update -// to groups uid is not part of anymore. -func updateUsersAndGroups(buckets map[string]bucketWithName, uid uint32, groupContents []GroupDB, previousGIDs []uint32) error { +// updateUserAndGroups updates the pivot table for user to groups and group to users. +// It handles any update to groups uid is not part of anymore. +func updateUsersAndGroups(buckets map[string]bucketWithName, uid uint32, + groupContents []GroupDB, previousGIDs []uint32) error { var currentGIDs []uint32 for _, groupContent := range groupContents { currentGIDs = append(currentGIDs, groupContent.GID) diff --git a/internal/users/internal_test.go b/internal/users/internal_test.go index 9f07b8a6a..85822c5bb 100644 --- a/internal/users/internal_test.go +++ b/internal/users/internal_test.go @@ -34,7 +34,8 @@ func TestGenerateID(t *testing.T) { tc.idMax = DefaultConfig.UIDMax } - require.Equal(t, tc.wantID, fmt.Sprint(generateID(tc.input, tc.idMin, tc.idMax)), "GenerateID did not return expected value") + require.Equal(t, tc.wantID, fmt.Sprint(generateID(tc.input, tc.idMin, tc.idMax)), + "GenerateID did not return expected value") }) } } diff --git a/internal/users/localgroups/localgroups.go b/internal/users/localgroups/localgroups.go index 708e120a5..1b0193089 100644 --- a/internal/users/localgroups/localgroups.go +++ b/internal/users/localgroups/localgroups.go @@ -111,7 +111,8 @@ func existingLocalGroups(user, groupPath string) (groups []string, err error) { // computeGroupOperation returns which local groups to add and which to remove comparing with the existing group state. // Only local groups (with no GID) are considered from GroupInfo. -func computeGroupOperation(newGroupsInfo []string, currentLocalGroups []string) (groupsToAdd []string, groupsToRemove []string) { +func computeGroupOperation(newGroupsInfo []string, currentLocalGroups []string) ( + groupsToAdd []string, groupsToRemove []string) { newGroups := make(map[string]struct{}) for _, grp := range newGroupsInfo { newGroups[grp] = struct{}{} diff --git a/internal/users/localgroups/localgroups_test.go b/internal/users/localgroups/localgroups_test.go index 47d6026c1..6dd14767f 100644 --- a/internal/users/localgroups/localgroups_test.go +++ b/internal/users/localgroups/localgroups_test.go @@ -23,32 +23,74 @@ func TestUpdateLocalGroups(t *testing.T) { wantErr bool }{ // First insertion cases - "Insert new user in existing files with no users in our group": {groupFilePath: "no_users_in_our_groups.group"}, - "Insert new user when no users in any group": {groupFilePath: "no_users.group"}, - "Insert new user in existing files with other users in our group": {groupFilePath: "users_in_our_groups.group"}, - "Insert new user in existing files with multiple other users in our group": {groupFilePath: "multiple_users_in_our_groups.group"}, + "Insert new user in existing files with no users in our group": { + groupFilePath: "no_users_in_our_groups.group", + }, + "Insert new user when no users in any group": { + groupFilePath: "no_users.group", + }, + "Insert new user in existing files with other users in our group": { + groupFilePath: "users_in_our_groups.group", + }, + "Insert new user in existing files with multiple other users in our group": { + groupFilePath: "multiple_users_in_our_groups.group", + }, // Users in existing groups - "No-Op for user is already present in both local groups": {groupFilePath: "user_in_both_groups.group"}, - "Insert user in the only local group when not present": {groupFilePath: "user_in_one_group.group"}, - "Insert user in the only local group when not present even with multiple": {groupFilePath: "user_and_others_in_one_groups.group"}, - "Remove user from an additional group, being alone": {groupFilePath: "user_in_second_local_group.group"}, - "Remove user from an additional group, multiple users in group": {groupFilePath: "user_in_second_local_group_with_others.group"}, - "Add and remove user from multiple groups, one remaining": {groupFilePath: "user_in_many_groups.group"}, + "No-Op for user is already present in both local groups": { + groupFilePath: "user_in_both_groups.group", + }, + "Insert user in the only local group when not present": { + groupFilePath: "user_in_one_group.group", + }, + "Insert user in the only local group when not present even with multiple": { + groupFilePath: "user_and_others_in_one_groups.group", + }, + "Remove user from an additional group, being alone": { + groupFilePath: "user_in_second_local_group.group", + }, + "Remove user from an additional group, multiple users in group": { + groupFilePath: "user_in_second_local_group_with_others.group", + }, + "Add and remove user from multiple groups, one remaining": { + groupFilePath: "user_in_many_groups.group", + }, // Flexible accepted cases - "Missing group is ignored": {groupFilePath: "missing_group.group"}, - "Group file with empty line is ignored": {groupFilePath: "empty_line.group"}, + "Missing group is ignored": { + groupFilePath: "missing_group.group", + }, + "Group file with empty line is ignored": { + groupFilePath: "empty_line.group", + }, // No group - "No-Op for user with no groups and was in none": {groups: []string{}, groupFilePath: "no_users_in_our_groups.group"}, - "Remove user with no groups from existing ones": {groups: []string{}, groupFilePath: "user_in_both_groups.group"}, + "No-Op for user with no groups and was in none": { + groups: []string{}, groupFilePath: "no_users_in_our_groups.group", + }, + "Remove user with no groups from existing ones": { + groups: []string{}, groupFilePath: "user_in_both_groups.group", + }, // Error cases - "Error on missing groups file": {groupFilePath: "does_not_exists.group", wantErr: true}, - "Error when groups file is malformed": {groupFilePath: "malformed_file.group", wantErr: true}, - "Error on any unignored add gpasswd error": {username: "gpasswdfail", groupFilePath: "no_users.group", wantErr: true}, - "Error on any unignored delete gpasswd error": {username: "gpasswdfail", groupFilePath: "gpasswdfail_in_deleted_group.group", wantErr: true}, + "Error on missing groups file": { + groupFilePath: "does_not_exists.group", + wantErr: true, + }, + "Error when groups file is malformed": { + groupFilePath: "malformed_file.group", + wantErr: true, + }, + "Error on any unignored add gpasswd error": { + username: "gpasswdfail", + groupFilePath: "no_users.group", + wantErr: true, + }, + "Error on any unignored delete gpasswd error": { + username: "gpasswdfail", + groupFilePath: "gpasswdfail_in_deleted_group.group", + wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -73,7 +115,9 @@ func TestUpdateLocalGroups(t *testing.T) { groupFilePath, destCmdsFile, } - err := localgroups.Update(tc.username, tc.groups, localgroups.WithGroupPath(groupFilePath), localgroups.WithGpasswdCmd(cmdArgs)) + err := localgroups.Update(tc.username, tc.groups, + localgroups.WithGroupPath(groupFilePath), + localgroups.WithGpasswdCmd(cmdArgs)) if tc.wantErr { require.Error(t, err, "UpdateLocalGroups should have failed") } else { @@ -101,10 +145,23 @@ func TestCleanLocalGroups(t *testing.T) { "Cleans up multiple users from group": {groupFilePath: "inactive_users_in_one_group.group"}, "Cleans up multiple users from multiple groups": {groupFilePath: "inactive_users_in_many_groups.group"}, - "Error if there's no active user": {groupFilePath: "user_in_many_groups.group", getUsersReturn: []string{}, wantErr: true}, - "Error on missing groups file": {groupFilePath: "does_not_exists.group", wantErr: true}, - "Error when groups file is malformed": {groupFilePath: "malformed_file.group", wantErr: true}, - "Error on any unignored delete gpasswd error": {groupFilePath: "gpasswdfail_in_deleted_group.group", wantErr: true}, + "Error if there's no active user": { + groupFilePath: "user_in_many_groups.group", + getUsersReturn: []string{}, + wantErr: true, + }, + "Error on missing groups file": { + groupFilePath: "does_not_exists.group", + wantErr: true, + }, + "Error when groups file is malformed": { + groupFilePath: "malformed_file.group", + wantErr: true, + }, + "Error on any unignored delete gpasswd error": { + groupFilePath: "gpasswdfail_in_deleted_group.group", + wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { diff --git a/internal/users/localgroups/testutils/localgroups.go b/internal/users/localgroups/testutils/localgroups.go index 3201a4770..6e8a1a792 100644 --- a/internal/users/localgroups/testutils/localgroups.go +++ b/internal/users/localgroups/testutils/localgroups.go @@ -1,4 +1,5 @@ -// Package localgrouptestutils export users test functionalities used by other packages to change cmdline and group file. +// Package localgrouptestutils export users test functionalities used by other packages +// to change cmdline and group file. package localgrouptestutils //nolint:gci // We import unsafe as it is needed for go:linkname, but the nolint comment confuses gofmt and it adds diff --git a/internal/users/manager.go b/internal/users/manager.go index 1ec4e470d..9073da0d9 100644 --- a/internal/users/manager.go +++ b/internal/users/manager.go @@ -173,10 +173,14 @@ func checkHomeDirOwnership(u UserInfo) error { // Check if the home directory is owned by the user. if oldUID != newUID { - slog.Warn(fmt.Sprintf("Home directory %q is not owned by UID %d. To fix this, run `sudo chown -R --from=%d %d %s`.", u.Dir, oldUID, oldUID, newUID, u.Dir)) + slog.Warn(fmt.Sprintf("Home directory %q is not owned by UID %d. "+ + "To fix this, run `sudo chown -R --from=%d %d %s`.", + u.Dir, oldUID, oldUID, newUID, u.Dir)) } if oldGID != newGID { - slog.Warn(fmt.Sprintf("Home directory %q is not owned by GID %d. To fix this, run `sudo chown -R --from=:%d :%d %s`.", u.Dir, oldGID, oldGID, newGID, u.Dir)) + slog.Warn(fmt.Sprintf("Home directory %q is not owned by GID %d. "+ + "To fix this, run `sudo chown -R --from=:%d :%d %s`.", + u.Dir, oldGID, oldGID, newGID, u.Dir)) } return nil @@ -312,8 +316,9 @@ func generateID(str string, minID, maxID uint32) uint32 { // Convert the first 4 bytes of the hash into an integer number := binary.BigEndian.Uint32(hash[:4]) % (maxID + 1) - // Repeat hashing until we get a number in the desired range. This ensures that the generated IDs are uniformly - // distributed in the range, opposed to a simple modulo operation. + // Repeat hashing until we get a number in the desired range. + // This ensures that the generated IDs are uniformly distributed in the range, + // opposed to a simple modulo operation. for number < minID { hash = sha256.Sum256(hash[:]) number = binary.BigEndian.Uint32(hash[:4]) % (maxID + 1) diff --git a/internal/users/manager_test.go b/internal/users/manager_test.go index 26c0b842c..670727ab7 100644 --- a/internal/users/manager_test.go +++ b/internal/users/manager_test.go @@ -38,7 +38,8 @@ func TestNewManager(t *testing.T) { } for name, tc := range tests { t.Run(name, func(t *testing.T) { - destCmdsFile := localgroupstestutils.SetupGPasswdMock(t, filepath.Join("testdata", "groups", "users_in_groups.group")) + destCmdsFile := localgroupstestutils.SetupGPasswdMock(t, + filepath.Join("testdata", "groups", "users_in_groups.group")) cacheDir := t.TempDir() if tc.dbFile == "" { @@ -169,20 +170,55 @@ func TestUpdateUser(t *testing.T) { noOutput bool wantSameUID bool }{ - "Successfully update user": {groupsCase: "cloud-group"}, - "Successfully update user updating local groups": {groupsCase: "mixed-groups-cloud-first", localGroupsFile: "users_in_groups.group"}, - "UID does not change if user already exists": {userCase: "same-name-different-uid", dbFile: "one_user_and_group", wantSameUID: true}, + "Successfully update user": { + groupsCase: "cloud-group", + }, + "Successfully update user updating local groups": { + groupsCase: "mixed-groups-cloud-first", localGroupsFile: "users_in_groups.group", + }, + "UID does not change if user already exists": { + userCase: "same-name-different-uid", dbFile: "one_user_and_group", wantSameUID: true, + }, - "Error if user has no username": {userCase: "nameless", wantErr: true, noOutput: true}, - "Error if user has conflicting uid": {userCase: "different-name-same-uid", dbFile: "one_user_and_group", wantErr: true, noOutput: true}, - "Error if group has no name": {groupsCase: "nameless-group", wantErr: true, noOutput: true}, - "Error if group has conflicting gid": {groupsCase: "different-name-same-gid", dbFile: "one_user_and_group", wantErr: true, noOutput: true}, + "Error if user has no username": { + userCase: "nameless", wantErr: true, noOutput: true, + }, + "Error if user has conflicting uid": { + userCase: "different-name-same-uid", dbFile: "one_user_and_group", wantErr: true, noOutput: true, + }, + "Error if group has no name": { + groupsCase: "nameless-group", wantErr: true, noOutput: true, + }, + "Error if group has conflicting gid": { + groupsCase: "different-name-same-gid", dbFile: "one_user_and_group", wantErr: true, noOutput: true, + }, - "Error when updating local groups remove user from db": {groupsCase: "mixed-groups-gpasswd-fail", localGroupsFile: "gpasswdfail_in_deleted_group.group", wantErr: true}, - "Error when updating local groups remove user from db without touching other users": {dbFile: "multiple_users_and_groups", groupsCase: "mixed-groups-gpasswd-fail", localGroupsFile: "gpasswdfail_in_deleted_group.group", wantErr: true}, - "Error when updating local groups remove user from db even if already existed": {userCase: "user2", dbFile: "multiple_users_and_groups", groupsCase: "mixed-groups-gpasswd-fail", localGroupsFile: "gpasswdfail_in_deleted_group.group", wantErr: true}, + "Error when updating local groups remove user from db": { + groupsCase: "mixed-groups-gpasswd-fail", + localGroupsFile: "gpasswdfail_in_deleted_group.group", + wantErr: true, + }, + "Error when updating local groups remove user from db without touching other users": { + dbFile: "multiple_users_and_groups", + groupsCase: "mixed-groups-gpasswd-fail", + localGroupsFile: "gpasswdfail_in_deleted_group.group", + wantErr: true, + }, + "Error when updating local groups remove user from db even if already existed": { + userCase: "user2", + dbFile: "multiple_users_and_groups", + groupsCase: "mixed-groups-gpasswd-fail", + localGroupsFile: "gpasswdfail_in_deleted_group.group", + wantErr: true, + }, - "Error on invalid entry": {groupsCase: "cloud-group", dbFile: "invalid_entry_in_userToGroups", localGroupsFile: "users_in_groups.group", wantErr: true, noOutput: true}, + "Error on invalid entry": { + groupsCase: "cloud-group", + dbFile: "invalid_entry_in_userToGroups", + localGroupsFile: "users_in_groups.group", + wantErr: true, + noOutput: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -192,7 +228,8 @@ func TestUpdateUser(t *testing.T) { var destCmdsFile string if tc.localGroupsFile != "" { - destCmdsFile = localgroupstestutils.SetupGPasswdMock(t, filepath.Join("testdata", "groups", tc.localGroupsFile)) + destCmdsFile = localgroupstestutils.SetupGPasswdMock(t, + filepath.Join("testdata", "groups", tc.localGroupsFile)) } if tc.userCase == "" { @@ -207,7 +244,8 @@ func TestUpdateUser(t *testing.T) { cacheDir := t.TempDir() if tc.dbFile != "" { - cachetestutils.CreateDBFromYAML(t, filepath.Join("testdata", "db", tc.dbFile+".db.yaml"), cacheDir) + cachetestutils.CreateDBFromYAML(t, + filepath.Join("testdata", "db", tc.dbFile+".db.yaml"), cacheDir) } m := newManagerForTests(t, cacheDir) @@ -237,7 +275,8 @@ func TestUpdateUser(t *testing.T) { want := testutils.LoadWithUpdateFromGoldenYAML(t, got) require.Equal(t, want, got, "Did not get expected database content") - localgroupstestutils.RequireGPasswdOutput(t, destCmdsFile, testutils.GoldenPath(t)+".gpasswd.output") + localgroupstestutils.RequireGPasswdOutput(t, destCmdsFile, + testutils.GoldenPath(t)+".gpasswd.output") }) } } @@ -251,11 +290,19 @@ func TestBrokerForUser(t *testing.T) { wantErr bool wantErrType error }{ - "Successfully get broker for user": {username: "user1", dbFile: "multiple_users_and_groups", wantBrokerID: "broker-id"}, - "Return no broker but in cache if user has no broker yet": {username: "userwithoutbroker", dbFile: "multiple_users_and_groups", wantBrokerID: ""}, + "Successfully get broker for user": { + username: "user1", dbFile: "multiple_users_and_groups", wantBrokerID: "broker-id", + }, + "Return no broker but in cache if user has no broker yet": { + username: "userwithoutbroker", dbFile: "multiple_users_and_groups", wantBrokerID: "", + }, - "Error if user does not exist": {username: "doesnotexist", dbFile: "multiple_users_and_groups", wantErrType: cache.NoDataFoundError{}}, - "Error if db has invalid entry": {username: "user1", dbFile: "invalid_entry_in_userByName", wantErr: true}, + "Error if user does not exist": { + username: "doesnotexist", dbFile: "multiple_users_and_groups", wantErrType: cache.NoDataFoundError{}, + }, + "Error if db has invalid entry": { + username: "user1", dbFile: "invalid_entry_in_userByName", wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -273,7 +320,8 @@ func TestBrokerForUser(t *testing.T) { return } - require.Equal(t, tc.wantBrokerID, brokerID, "BrokerForUser should return the expected brokerID, but did not") + require.Equal(t, tc.wantBrokerID, brokerID, + "BrokerForUser should return the expected brokerID, but did not") }) } } @@ -333,10 +381,21 @@ func TestUserByName(t *testing.T) { wantErr bool wantErrType error }{ - "Successfully get user by name": {username: "user1", dbFile: "multiple_users_and_groups"}, + "Successfully get user by name": { + username: "user1", + dbFile: "multiple_users_and_groups", + }, - "Error if user does not exist": {username: "doesnotexist", dbFile: "multiple_users_and_groups", wantErrType: cache.NoDataFoundError{}}, - "Error if db has invalid entry": {username: "user1", dbFile: "invalid_entry_in_userByName", wantErr: true}, + "Error if user does not exist": { + username: "doesnotexist", + dbFile: "multiple_users_and_groups", + wantErrType: cache.NoDataFoundError{}, + }, + "Error if db has invalid entry": { + username: "user1", + dbFile: "invalid_entry_in_userByName", + wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -371,8 +430,15 @@ func TestUserByID(t *testing.T) { }{ "Successfully get user by ID": {uid: 1111, dbFile: "multiple_users_and_groups"}, - "Error if user does not exist": {uid: 0, dbFile: "multiple_users_and_groups", wantErrType: cache.NoDataFoundError{}}, - "Error if db has invalid entry": {uid: 1111, dbFile: "invalid_entry_in_userByID", wantErr: true}, + "Error if user does not exist": { + uid: 0, + dbFile: "multiple_users_and_groups", + wantErrType: cache.NoDataFoundError{}, + }, + "Error if db has invalid entry": {uid: 1111, + dbFile: "invalid_entry_in_userByID", + wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -439,10 +505,17 @@ func TestGroupByName(t *testing.T) { wantErr bool wantErrType error }{ - "Successfully get group by name": {groupname: "group1", dbFile: "multiple_users_and_groups"}, + "Successfully get group by name": { + groupname: "group1", dbFile: "multiple_users_and_groups"}, - "Error if group does not exist": {groupname: "doesnotexist", dbFile: "multiple_users_and_groups", wantErrType: cache.NoDataFoundError{}}, - "Error if db has invalid entry": {groupname: "group1", dbFile: "invalid_entry_in_groupByName", wantErr: true}, + "Error if group does not exist": { + groupname: "doesnotexist", dbFile: "multiple_users_and_groups", + wantErrType: cache.NoDataFoundError{}, + }, + "Error if db has invalid entry": { + groupname: "group1", dbFile: "invalid_entry_in_groupByName", + wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -477,7 +550,11 @@ func TestGroupByID(t *testing.T) { }{ "Successfully get group by ID": {gid: 11111, dbFile: "multiple_users_and_groups"}, - "Error if group does not exist": {gid: 0, dbFile: "multiple_users_and_groups", wantErrType: cache.NoDataFoundError{}}, + "Error if group does not exist": { + gid: 0, + dbFile: "multiple_users_and_groups", + wantErrType: cache.NoDataFoundError{}, + }, "Error if db has invalid entry": {gid: 11111, dbFile: "invalid_entry_in_groupByID", wantErr: true}, } for name, tc := range tests { @@ -547,8 +624,16 @@ func TestShadowByName(t *testing.T) { }{ "Successfully get shadow by name": {username: "user1", dbFile: "multiple_users_and_groups"}, - "Error if shadow does not exist": {username: "doesnotexist", dbFile: "multiple_users_and_groups", wantErrType: cache.NoDataFoundError{}}, - "Error if db has invalid entry": {username: "user1", dbFile: "invalid_entry_in_userByName", wantErr: true}, + "Error if shadow does not exist": { + username: "doesnotexist", + dbFile: "multiple_users_and_groups", + wantErrType: cache.NoDataFoundError{}, + }, + "Error if db has invalid entry": { + username: "user1", + dbFile: "invalid_entry_in_userByName", + wantErr: true, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { diff --git a/nss/integration-tests/helper_test.go b/nss/integration-tests/helper_test.go index bec0dd4c1..5706e01b5 100644 --- a/nss/integration-tests/helper_test.go +++ b/nss/integration-tests/helper_test.go @@ -55,7 +55,8 @@ func buildRustNSSLib(t *testing.T) (libPath string, rustCovEnv []string) { // getentOutputForLib returns the specific part for the nss command for the authd service. // It uses the locally build authd nss module for the integration tests. -func getentOutputForLib(t *testing.T, libPath, socketPath string, rustCovEnv []string, shouldPreCheck bool, cmds ...string) (got string, exitCode int) { +func getentOutputForLib(t *testing.T, libPath, socketPath string, rustCovEnv []string, shouldPreCheck bool, + cmds ...string) (got string, exitCode int) { t.Helper() // #nosec:G204 - we control the command arguments in tests diff --git a/nss/integration-tests/integration_test.go b/nss/integration-tests/integration_test.go index 7b5c3ceb3..00188dacf 100644 --- a/nss/integration-tests/integration_test.go +++ b/nss/integration-tests/integration_test.go @@ -28,7 +28,10 @@ func TestIntegration(t *testing.T) { defaultOutputPath := filepath.Join(filepath.Dir(daemonPath), "gpasswd.output") defaultGroupsFilePath := filepath.Join(testutils.TestFamilyPath(t), "gpasswd.group") - env := append(localgroupstestutils.AuthdIntegrationTestsEnvWithGpasswdMock(t, defaultOutputPath, defaultGroupsFilePath), "AUTHD_INTEGRATIONTESTS_CURRENT_USER_AS_ROOT=1") + env := append( + localgroupstestutils.AuthdIntegrationTestsEnvWithGpasswdMock(t, defaultOutputPath, defaultGroupsFilePath), + "AUTHD_INTEGRATIONTESTS_CURRENT_USER_AS_ROOT=1", + ) ctx, cancel := context.WithCancel(context.Background()) _, stopped := testutils.RunDaemon(ctx, t, daemonPath, testutils.WithSocketPath(defaultSocket), @@ -69,35 +72,72 @@ func TestIntegration(t *testing.T) { // Even though those are "error" cases, the getent command won't fail when trying to list content of a service. "Returns empty when getting all entries from shadow if regular user": {db: "shadow", currentUserNotRoot: true}, - "Returns empty when getting all entries from passwd and daemon is not available": {db: "passwd", noDaemon: true}, - "Returns empty when getting all entries from group and daemon is not available": {db: "group", noDaemon: true}, - "Returns empty when getting all entries from shadow and daemon is not available": {db: "shadow", noDaemon: true}, + "Returns empty when getting all entries from passwd and daemon is not available": { + db: "passwd", noDaemon: true, + }, + "Returns empty when getting all entries from group and daemon is not available": {db: "group", noDaemon: true}, + "Returns empty when getting all entries from shadow and daemon is not available": { + db: "shadow", noDaemon: true, + }, /* Error cases */ - // We can't assert on the returned error type since the error returned by getent will always be 2 (i.e. Not Found), even though the library returns other types. - "Error when getting all entries from passwd and database is corrupted": {db: "passwd", cacheDB: "invalid_entry_in_userByID", wantSecondCall: true}, - "Error when getting all entries from group and database is corrupted": {db: "group", cacheDB: "invalid_entry_in_groupByID", wantSecondCall: true}, - "Error when getting all entries from shadow and database is corrupted": {db: "shadow", cacheDB: "invalid_entry_in_userByID", wantSecondCall: true}, - - "Error when getting shadow by name if regular user": {db: "shadow", key: "user1", currentUserNotRoot: true, wantStatus: codeNotFound}, - - "Error when getting passwd by name and entry does not exist": {db: "passwd", key: "doesnotexit", wantStatus: codeNotFound}, - "Error when getting passwd by name entry exists in broker but precheck is disabled": {db: "passwd", key: "user-pre-check", wantStatus: codeNotFound}, - "Error when getting group by name and entry does not exist": {db: "group", key: "doesnotexit", wantStatus: codeNotFound}, - "Error when getting shadow by name and entry does not exist": {db: "shadow", key: "doesnotexit", wantStatus: codeNotFound}, - - "Error when getting passwd by id and entry does not exist": {db: "passwd", key: "404", wantStatus: codeNotFound}, - "Error when getting group by id and entry does not exist": {db: "group", key: "404", wantStatus: codeNotFound}, - - "Error when getting passwd by name and daemon is not available": {db: "passwd", key: "user1", noDaemon: true, wantStatus: codeNotFound}, - "Error when getting group by name and daemon is not available": {db: "group", key: "group1", noDaemon: true, wantStatus: codeNotFound}, - "Error when getting shadow by name and daemon is not available": {db: "shadow", key: "user1", noDaemon: true, wantStatus: codeNotFound}, - - "Error when getting passwd by id and daemon is not available": {db: "passwd", key: "1111", noDaemon: true, wantStatus: codeNotFound}, - "Error when getting group by id and daemon is not available": {db: "group", key: "11111", noDaemon: true, wantStatus: codeNotFound}, + // We can't assert on the returned error type since the error returned by getent will always be 2 + // (i.e. Not Found), even though the library returns other types. + "Error when getting all entries from passwd and database is corrupted": { + db: "passwd", cacheDB: "invalid_entry_in_userByID", wantSecondCall: true, + }, + "Error when getting all entries from group and database is corrupted": { + db: "group", cacheDB: "invalid_entry_in_groupByID", wantSecondCall: true, + }, + "Error when getting all entries from shadow and database is corrupted": { + db: "shadow", cacheDB: "invalid_entry_in_userByID", wantSecondCall: true, + }, + + "Error when getting shadow by name if regular user": { + db: "shadow", key: "user1", currentUserNotRoot: true, wantStatus: codeNotFound, + }, + + "Error when getting passwd by name and entry does not exist": { + db: "passwd", key: "doesnotexit", wantStatus: codeNotFound, + }, + "Error when getting passwd by name entry exists in broker but precheck is disabled": { + db: "passwd", key: "user-pre-check", wantStatus: codeNotFound, + }, + "Error when getting group by name and entry does not exist": { + db: "group", key: "doesnotexit", wantStatus: codeNotFound, + }, + "Error when getting shadow by name and entry does not exist": { + db: "shadow", key: "doesnotexit", wantStatus: codeNotFound, + }, + + "Error when getting passwd by id and entry does not exist": { + db: "passwd", key: "404", wantStatus: codeNotFound, + }, + "Error when getting group by id and entry does not exist": { + db: "group", key: "404", wantStatus: codeNotFound, + }, + + "Error when getting passwd by name and daemon is not available": { + db: "passwd", key: "user1", noDaemon: true, wantStatus: codeNotFound, + }, + "Error when getting group by name and daemon is not available": { + db: "group", key: "group1", noDaemon: true, wantStatus: codeNotFound, + }, + "Error when getting shadow by name and daemon is not available": { + db: "shadow", key: "user1", noDaemon: true, wantStatus: codeNotFound, + }, + + "Error when getting passwd by id and daemon is not available": { + db: "passwd", key: "1111", noDaemon: true, wantStatus: codeNotFound, + }, + "Error when getting group by id and daemon is not available": { + db: "group", key: "11111", noDaemon: true, wantStatus: codeNotFound, + }, /* Special cases */ - "Do not query the cache when user is pam_unix_non_existent": {db: "passwd", key: "pam_unix_non_existent:", cacheDB: "pam_unix_non_existent", wantStatus: codeNotFound}, + "Do not query the cache when user is pam_unix_non_existent": { + db: "passwd", key: "pam_unix_non_existent:", cacheDB: "pam_unix_non_existent", wantStatus: codeNotFound, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { diff --git a/pam/integration-tests/exec_test.go b/pam/integration-tests/exec_test.go index f4638f2b7..63848e23d 100644 --- a/pam/integration-tests/exec_test.go +++ b/pam/integration-tests/exec_test.go @@ -864,20 +864,24 @@ func getModuleArgs(t *testing.T, clientPath string, args []string) []string { return append(moduleArgs, args...) } -func preparePamTransaction(t *testing.T, libPath string, clientPath string, args []string, user string) *pam.Transaction { +func preparePamTransaction( + t *testing.T, libPath string, clientPath string, args []string, user string) *pam.Transaction { t.Helper() return preparePamTransactionWithConv(t, libPath, clientPath, args, user, nil) } -func preparePamTransactionWithConv(t *testing.T, libPath string, clientPath string, args []string, user string, conv pam.ConversationHandler) *pam.Transaction { +func preparePamTransactionWithConv( + t *testing.T, libPath string, clientPath string, args []string, user string, + conv pam.ConversationHandler) *pam.Transaction { t.Helper() serviceFile := createServiceFile(t, execServiceName, libPath, getModuleArgs(t, clientPath, args)) return preparePamTransactionForServiceFile(t, serviceFile, user, conv) } -func preparePamTransactionWithActionArgs(t *testing.T, libPath string, clientPath string, actionArgs actionArgsMap, user string) *pam.Transaction { +func preparePamTransactionWithActionArgs( + t *testing.T, libPath string, clientPath string, actionArgs actionArgsMap, user string) *pam.Transaction { t.Helper() actionArgs = maps.Clone(actionArgs) @@ -889,7 +893,8 @@ func preparePamTransactionWithActionArgs(t *testing.T, libPath string, clientPat return preparePamTransactionForServiceFile(t, serviceFile, user, nil) } -func preparePamTransactionForServiceFile(t *testing.T, serviceFile string, user string, conv pam.ConversationHandler) *pam.Transaction { +func preparePamTransactionForServiceFile( + t *testing.T, serviceFile string, user string, conv pam.ConversationHandler) *pam.Transaction { t.Helper() var tx *pam.Transaction diff --git a/pam/integration-tests/gdm_test.go b/pam/integration-tests/gdm_test.go index 5f64d3e27..3331c3e26 100644 --- a/pam/integration-tests/gdm_test.go +++ b/pam/integration-tests/gdm_test.go @@ -509,7 +509,9 @@ func TestGdmModule(t *testing.T) { "Error on missing user": { pamUser: ptrValue(""), wantPamErrorMessages: []string{ - "can't select broker: error InvalidArgument from server: can't start authentication transaction: rpc error: code = InvalidArgument desc = no user name provided", + "can't select broker: error InvalidArgument from server: " + + "can't start authentication transaction: " + + "rpc error: code = InvalidArgument desc = no user name provided", }, wantError: pam.ErrSystem, wantAcctMgmtErr: pam_test.ErrIgnore, diff --git a/pam/integration-tests/helpers_test.go b/pam/integration-tests/helpers_test.go index aabaeb287..17c9a4eb7 100644 --- a/pam/integration-tests/helpers_test.go +++ b/pam/integration-tests/helpers_test.go @@ -134,7 +134,9 @@ func prepareFileLogging(t *testing.T, fileName string) string { func requirePreviousBrokerForUser(t *testing.T, socketPath string, brokerName string, user string) { t.Helper() - conn, err := grpc.NewClient("unix://"+socketPath, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithUnaryInterceptor(errmessages.FormatErrorMessage)) + conn, err := grpc.NewClient("unix://"+socketPath, + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithUnaryInterceptor(errmessages.FormatErrorMessage)) require.NoError(t, err, "Can't connect to authd socket") t.Cleanup(func() { conn.Close() }) diff --git a/pam/integration-tests/modulehelpers_test.go b/pam/integration-tests/modulehelpers_test.go index 910a2f14e..534da7206 100644 --- a/pam/integration-tests/modulehelpers_test.go +++ b/pam/integration-tests/modulehelpers_test.go @@ -146,13 +146,17 @@ func createServiceFileWithActionArgs(t *testing.T, name string, libPath string, t.Helper() serviceFile, err := pam_test.CreateService(t.TempDir(), name, []pam_test.ServiceLine{ - {Action: pam_test.Auth, Control: pam_test.SufficientRequisite, Module: libPath, Args: actionArgs[pam_test.Auth]}, + {Action: pam_test.Auth, Control: pam_test.SufficientRequisite, + Module: libPath, Args: actionArgs[pam_test.Auth]}, {Action: pam_test.Auth, Control: pam_test.Requisite, Module: pam_test.Ignore.String()}, - {Action: pam_test.Account, Control: pam_test.SufficientRequisite, Module: libPath, Args: actionArgs[pam_test.Account]}, + {Action: pam_test.Account, Control: pam_test.SufficientRequisite, + Module: libPath, Args: actionArgs[pam_test.Account]}, {Action: pam_test.Account, Control: pam_test.Requisite, Module: pam_test.Ignore.String()}, - {Action: pam_test.Password, Control: pam_test.SufficientRequisite, Module: libPath, Args: actionArgs[pam_test.Password]}, + {Action: pam_test.Password, Control: pam_test.SufficientRequisite, + Module: libPath, Args: actionArgs[pam_test.Password]}, {Action: pam_test.Password, Control: pam_test.Requisite, Module: pam_test.Ignore.String()}, - {Action: pam_test.Session, Control: pam_test.SufficientRequisite, Module: libPath, Args: actionArgs[pam_test.Session]}, + {Action: pam_test.Session, Control: pam_test.SufficientRequisite, + Module: libPath, Args: actionArgs[pam_test.Session]}, {Action: pam_test.Session, Control: pam_test.Requisite, Module: pam_test.Ignore.String()}, }) require.NoError(t, err, "Setup: Can't create service file %s", serviceFile) diff --git a/pam/integration-tests/vhs-helpers_test.go b/pam/integration-tests/vhs-helpers_test.go index ffdce45b8..98e5c8e4a 100644 --- a/pam/integration-tests/vhs-helpers_test.go +++ b/pam/integration-tests/vhs-helpers_test.go @@ -58,7 +58,8 @@ func newTapeData(tapeName string, settings ...tapeSetting) tapeData { m := map[string]any{ vhsWidth: 800, vhsHeight: 500, - // TODO: Ideally, we should use Ubuntu Mono. However, the github runner is still on Jammy, which does not have it. + // TODO: Ideally, we should use Ubuntu Mono. However, the github runner is still on Jammy, + // which does not have it. // We should update this to use Ubuntu Mono once the runner is updated. vhsFontFamily: "Monospace", vhsFontSize: 13, diff --git a/pam/internal/adapter/authentication.go b/pam/internal/adapter/authentication.go index a9376673e..e527e2c39 100644 --- a/pam/internal/adapter/authentication.go +++ b/pam/internal/adapter/authentication.go @@ -262,10 +262,14 @@ func (m *authenticationModel) Update(msg tea.Msg) (authModel authenticationModel // no challenge value, pass it as is plainTextChallenge, err := msg.encryptChallengeIfPresent(m.encryptionKey) if err != nil { - return *m, sendEvent(pamError{status: pam.ErrSystem, msg: fmt.Sprintf("could not encrypt challenge payload: %v", err)}) + return *m, sendEvent(pamError{ + status: pam.ErrSystem, + msg: fmt.Sprintf("could not encrypt challenge payload: %v", err)}, + ) } - return *m, sendIsAuthenticated(msg.ctx, m.client, m.currentSessionID, &authd.IARequest_AuthenticationData{Item: msg.item}, plainTextChallenge) + return *m, sendIsAuthenticated(msg.ctx, m.client, m.currentSessionID, + &authd.IARequest_AuthenticationData{Item: msg.item}, plainTextChallenge) case isAuthenticatedCancelled: log.Debugf(context.TODO(), "%#v", msg) @@ -373,7 +377,8 @@ func (m *authenticationModel) Blur() { // Compose initialize the authentication model to be used. // It creates and attaches the sub layout models based on UILayout. -func (m *authenticationModel) Compose(brokerID, sessionID string, encryptionKey *rsa.PublicKey, layout *authd.UILayout) tea.Cmd { +func (m *authenticationModel) Compose(brokerID, sessionID string, encryptionKey *rsa.PublicKey, + layout *authd.UILayout) tea.Cmd { m.currentBrokerID = brokerID m.currentSessionID = sessionID m.encryptionKey = encryptionKey diff --git a/pam/internal/adapter/authmodeselection.go b/pam/internal/adapter/authmodeselection.go index 630c0c833..0a86238c5 100644 --- a/pam/internal/adapter/authmodeselection.go +++ b/pam/internal/adapter/authmodeselection.go @@ -24,20 +24,24 @@ type authModeSelectionModel struct { currentAuthModeSelectedID string } -// supportedUILayoutsReceived is the internal event signalling that the current supported ui layout in the context have been received. +// supportedUILayoutsReceived is the internal event signalling that the current supported +// UI layout in the context have been received. type supportedUILayoutsReceived struct { layouts []*authd.UILayout } -// supportedUILayoutsSet is the event signalling that the current supported ui layout in the context have been set. +// supportedUILayoutsSet is the event signalling that the current supported ui layout in the +// context have been set. type supportedUILayoutsSet struct{} -// authModesReceived is the internal event signalling that the supported authentication modes have been received. +// authModesReceived is the internal event signalling that the supported authentication modes +// have been received. type authModesReceived struct { authModes []*authd.GAMResponse_AuthenticationMode } -// authModeSelected is the internal event signalling that the an authentication mode has been selected. +// authModeSelected is the internal event signalling that the an authentication mode +// has been selected. type authModeSelected struct { id string } @@ -159,7 +163,9 @@ func (m authModeSelectionModel) Update(msg tea.Msg) (authModeSelectionModel, tea log.Debugf(context.TODO(), "%#v", msg) // Ensure auth mode id is valid if !validAuthModeID(msg.id, m.availableAuthModes) { - log.Infof(context.TODO(), "authentication mode %q is not part of currently available authentication mode", msg.id) + log.Infof(context.TODO(), + "authentication mode %q is not part of currently available authentication mode", + msg.id) return m, nil } m.currentAuthModeSelectedID = msg.id diff --git a/pam/internal/adapter/brokerselection.go b/pam/internal/adapter/brokerselection.go index b65229167..77d7e8309 100644 --- a/pam/internal/adapter/brokerselection.go +++ b/pam/internal/adapter/brokerselection.go @@ -244,7 +244,8 @@ func (d itemLayout) Render(w io.Writer, m list.Model, index int, item list.Item) line := fmt.Sprintf("%d. %s", index+1, label) if index == m.Index() { - line = lipgloss.NewStyle().Bold(true).Foreground(lipgloss.AdaptiveColor{Light: "#000000", Dark: "#FFFFFF"}).Render("> " + line) + line = lipgloss.NewStyle().Bold(true).Foreground( + lipgloss.AdaptiveColor{Light: "#000000", Dark: "#FFFFFF"}).Render("> " + line) } else { line = lipgloss.NewStyle().PaddingLeft(2).Render(line) } diff --git a/pam/internal/adapter/commands.go b/pam/internal/adapter/commands.go index 46a7142d2..63bc2e4de 100644 --- a/pam/internal/adapter/commands.go +++ b/pam/internal/adapter/commands.go @@ -115,7 +115,8 @@ func endSession(client authd.PAMClient, currentSession *sessionInfo) tea.Cmd { SessionId: currentSession.sessionID, }) if err != nil { - log.Infof(context.Background(), "Could not end session %q. Considering already done", currentSession.sessionID) + log.Infof(context.Background(), "Could not end session %q. Considering already done", + currentSession.sessionID) return nil } return SessionEnded{} diff --git a/pam/internal/adapter/gdmmodel_test.go b/pam/internal/adapter/gdmmodel_test.go index d75af62e3..9916ab8d1 100644 --- a/pam/internal/adapter/gdmmodel_test.go +++ b/pam/internal/adapter/gdmmodel_test.go @@ -464,7 +464,8 @@ func TestGdmModel(t *testing.T) { msg: "Hi GDM, it's a pleasure to change your password!", }, }, - "New password can't change because not respecting rules after server-side user, broker and authMode selection": { + "New password can't change because not respecting rules after server-side " + + "user, broker and authMode selection": { clientOptions: append(slices.Clone(singleBrokerNewPasswordClientOptions), pam_test.WithGetPreviousBrokerReturn(firstBrokerInfo.Id, nil), pam_test.WithIsAuthenticatedReturn(&authd.IAResponse{ @@ -564,9 +565,11 @@ func TestGdmModel(t *testing.T) { gdm_test.AuthModeSelectedEvent(newPasswordUILayoutID), }, commands: []tea.Cmd{ - sendEvent(gdmTestSendAuthDataWhenReady{&authd.IARequest_AuthenticationData_Challenge{ - Challenge: "gdm-repeated-password", - }}), + sendEvent(gdmTestSendAuthDataWhenReady{ + &authd.IARequest_AuthenticationData_Challenge{ + Challenge: "gdm-repeated-password", + }, + }), }, }), }, @@ -910,7 +913,8 @@ func TestGdmModel(t *testing.T) { wantStage: pam_proto.Stage_challenge, wantExitStatus: gdmTestEarlyStopExitStatus, }, - "AuthMode selection stage from client after server-side broker and auth mode selection if there is only one auth mode": { + "AuthMode selection stage from client after server-side broker and auth mode selection " + + "if there is only one auth mode": { clientOptions: append(slices.Clone(singleBrokerClientOptions), pam_test.WithGetPreviousBrokerReturn(firstBrokerInfo.Id, nil), ), @@ -954,7 +958,8 @@ func TestGdmModel(t *testing.T) { wantStage: pam_proto.Stage_authModeSelection, wantExitStatus: gdmTestEarlyStopExitStatus, }, - "AuthMode selection stage from client after server-side broker and auth mode selection with multiple auth modes": { + "AuthMode selection stage from client after server-side broker and auth mode selection " + + "with multiple auth modes": { clientOptions: append(slices.Clone(singleBrokerClientOptions), pam_test.WithGetPreviousBrokerReturn(firstBrokerInfo.Id, nil), pam_test.WithUILayout("pincode", "Pin Code", pam_test.FormUILayout()), @@ -994,7 +999,8 @@ func TestGdmModel(t *testing.T) { wantStage: pam_proto.Stage_authModeSelection, wantExitStatus: gdmTestEarlyStopExitStatus, }, - "AuthMode selection stage from client after client-side broker and auth mode selection if there is only one auth mode": { + "AuthMode selection stage from client after client-side broker and auth mode selection " + + "if there is only one auth mode": { gdmEvents: []*gdm.EventData{ gdm_test.SelectUserEvent("gdm-selected-user-broker-and-auth-mode"), }, @@ -1041,7 +1047,8 @@ func TestGdmModel(t *testing.T) { wantStage: pam_proto.Stage_authModeSelection, wantExitStatus: gdmTestEarlyStopExitStatus, }, - "Authenticated after auth selection stage from client after client-side broker and auth mode selection if there is only one auth mode": { + "Authenticated after auth selection stage from client after client-side broker " + + "and auth mode selection if there is only one auth mode": { clientOptions: append(slices.Clone(singleBrokerClientOptions), pam_test.WithIsAuthenticatedWantChallenge("gdm-good-password"), ), @@ -1108,7 +1115,8 @@ func TestGdmModel(t *testing.T) { }, wantExitStatus: PamSuccess{BrokerID: firstBrokerInfo.Id}, }, - "Authenticated after auth selection stage from client after client-side broker and auth mode selection with multiple auth modes": { + "Authenticated after auth selection stage from client after client-side broker " + + "and auth mode selection with multiple auth modes": { clientOptions: append(slices.Clone(singleBrokerClientOptions), pam_test.WithUILayout("pincode", "Write the pin Code", pam_test.FormUILayout()), pam_test.WithIsAuthenticatedWantChallenge("1234"), @@ -1177,7 +1185,8 @@ func TestGdmModel(t *testing.T) { }, wantExitStatus: PamSuccess{BrokerID: firstBrokerInfo.Id}, }, - "Authenticated with qrcode after auth selection stage from client after client-side broker and auth mode selection": { + "Authenticated with qrcode after auth selection stage from client after client-side broker" + + " and auth mode selection": { supportedLayouts: []*authd.UILayout{ pam_test.FormUILayout(), pam_test.QrCodeUILayout(), @@ -1250,7 +1259,8 @@ func TestGdmModel(t *testing.T) { }, wantExitStatus: PamSuccess{BrokerID: firstBrokerInfo.Id}, }, - "Authenticated with qrcode regenerated after auth selection stage from client after client-side broker and auth mode selection": { + "Authenticated with qrcode regenerated after auth selection stage from client " + + "after client-side broker and auth mode selection": { timeout: 10 * time.Second, supportedLayouts: []*authd.UILayout{ pam_test.FormUILayout(), @@ -1332,7 +1342,8 @@ func TestGdmModel(t *testing.T) { }, wantExitStatus: PamSuccess{BrokerID: firstBrokerInfo.Id}, }, - "Authenticated with qrcode regenerated after wait started at auth selection stage from client after client-side broker and auth mode selection": { + "Authenticated with qrcode regenerated after wait started at auth selection stage from client" + + "after client-side broker and auth mode selection": { timeout: 10 * time.Second, supportedLayouts: []*authd.UILayout{ pam_test.FormUILayout(), @@ -1419,7 +1430,8 @@ func TestGdmModel(t *testing.T) { }, wantExitStatus: PamSuccess{BrokerID: firstBrokerInfo.Id}, }, - "Broker selection stage from client after client-side broker and auth mode selection if there is only one auth mode": { + "Broker selection stage from client after client-side broker and auth mode selection " + + "if there is only one auth mode": { gdmEvents: []*gdm.EventData{ gdm_test.SelectUserEvent("gdm-selected-user-broker-and-auth-mode"), }, @@ -1471,7 +1483,8 @@ func TestGdmModel(t *testing.T) { wantStage: pam_proto.Stage_brokerSelection, wantExitStatus: gdmTestEarlyStopExitStatus, }, - "User selection stage from client after client-side broker and auth mode selection if there is only one auth mode": { + "User selection stage from client after client-side broker and auth mode selection " + + "if there is only one auth mode": { gdmEvents: []*gdm.EventData{ gdm_test.SelectUserEvent("gdm-selected-user-broker-and-auth-mode"), }, @@ -1593,7 +1606,8 @@ func TestGdmModel(t *testing.T) { }, wantExitStatus: pamError{ status: pam.ErrSystem, - msg: "Sending GDM poll failed: Conversation error: poll response data member 0 invalid: missing event data", + msg: "Sending GDM poll failed: Conversation error: " + + "poll response data member 0 invalid: missing event data", }, }, "Error on invalid poll data response for missing data": { @@ -1609,7 +1623,8 @@ func TestGdmModel(t *testing.T) { }, wantExitStatus: pamError{ status: pam.ErrSystem, - msg: "Sending GDM poll failed: Conversation error: poll response data member 0 invalid: missing event type", + msg: "Sending GDM poll failed: Conversation error: " + + "poll response data member 0 invalid: missing event type", }, }, "Error on no brokers": { @@ -1719,7 +1734,8 @@ func TestGdmModel(t *testing.T) { }, wantExitStatus: pamError{ status: pam.ErrSystem, - msg: "encryption key sent by broker is not a valid base64 encoded string: illegal base64 data at input byte 2", + msg: "encryption key sent by broker is not a valid base64 encoded string: " + + "illegal base64 data at input byte 2", }, }, "Error during broker selection if encryption key is not valid key": { @@ -2281,7 +2297,8 @@ func TestGdmModel(t *testing.T) { wantNoBrokers: true, wantExitStatus: pamError{ status: pam.ErrSystem, - msg: "Sending GDM UI capabilities Request failed: Conversation error: this is an UI capabilities request error", + msg: "Sending GDM UI capabilities Request failed: " + + "Conversation error: this is an UI capabilities request error", }, }, "Error on selecting user name after PAM provided already one": { diff --git a/pam/internal/adapter/model.go b/pam/internal/adapter/model.go index 94915b745..a6f1df3c2 100644 --- a/pam/internal/adapter/model.go +++ b/pam/internal/adapter/model.go @@ -65,7 +65,8 @@ type UIModel struct { /* global events */ -// UsernameOrBrokerListReceived is received either when the user name is filled (pam or manually) and we got the broker list. +// UsernameOrBrokerListReceived is received either when the user name is filled (pam or manually) +// and we got the broker list. type UsernameOrBrokerListReceived struct{} // BrokerSelected signifies that the broker has been chosen. diff --git a/pam/internal/adapter/nativemodel.go b/pam/internal/adapter/nativemodel.go index 36419fde6..b6a2869a7 100644 --- a/pam/internal/adapter/nativemodel.go +++ b/pam/internal/adapter/nativemodel.go @@ -795,7 +795,8 @@ func (m nativeModel) newPasswordChallenge(previousChallenge *string) tea.Cmd { return cmd } } else { - if cmd := maybeSendPamError(m.sendInfo("Repeat the previously inserted password or insert '%[1]s' to cancel the request and go back", + if cmd := maybeSendPamError(m.sendInfo( + "Repeat the previously inserted password or insert '%[1]s' to cancel the request and go back", nativeCancelKey)); cmd != nil { return cmd } diff --git a/pam/internal/dbusmodule/transaction.go b/pam/internal/dbusmodule/transaction.go index ee1ad2bcc..c80f46c9e 100644 --- a/pam/internal/dbusmodule/transaction.go +++ b/pam/internal/dbusmodule/transaction.go @@ -41,7 +41,8 @@ const variantNothing = "<@mv nothing>" // NewTransaction creates a new [dbusmodule.Transaction] with the provided connection. // A [pam.ModuleTransaction] implementation is returned together with a cleanup function that // should be called to release the connection. -func NewTransaction(ctx context.Context, address string, o ...TransactionOptions) (tx pam.ModuleTransaction, cleanup func(), err error) { +func NewTransaction(ctx context.Context, address string, o ...TransactionOptions) ( + tx pam.ModuleTransaction, cleanup func(), err error) { opts := options{} for _, f := range o { f(&opts) diff --git a/pam/internal/gdm/protocol_test.go b/pam/internal/gdm/protocol_test.go index 582a26abf..62f930680 100644 --- a/pam/internal/gdm/protocol_test.go +++ b/pam/internal/gdm/protocol_test.go @@ -224,14 +224,20 @@ func TestGdmStructsMarshal(t *testing.T) { wantErrMsg: "missing event type", }, "Error event packet with missing type": { - gdmData: &gdm.Data{Type: gdm.DataType_event, Event: &gdm.EventData{Data: &gdm.EventData_AuthModeSelected{}}}, + gdmData: &gdm.Data{ + Type: gdm.DataType_event, + Event: &gdm.EventData{Data: &gdm.EventData_AuthModeSelected{}}, + }, wantErrMsg: "missing event type", }, "Error event packet with unexpected data": { gdmData: &gdm.Data{ - Type: gdm.DataType_event, - Event: &gdm.EventData{Type: gdm.EventType_authEvent, Data: &gdm.EventData_AuthModeSelected{}}, + Type: gdm.DataType_event, + Event: &gdm.EventData{ + Type: gdm.EventType_authEvent, + Data: &gdm.EventData_AuthModeSelected{}, + }, Hello: &gdm.HelloData{}, }, @@ -243,12 +249,18 @@ func TestGdmStructsMarshal(t *testing.T) { wantErrMsg: "field Event should not be defined", }, "Error request packet with unknown type": { - gdmData: &gdm.Data{Type: gdm.DataType_request, Request: &gdm.RequestData{Data: &gdm.RequestData_ChangeStage{}}}, + gdmData: &gdm.Data{ + Type: gdm.DataType_request, + Request: &gdm.RequestData{Data: &gdm.RequestData_ChangeStage{}}, + }, wantErrMsg: "missing request type", }, "Error request packet with invalid type": { - gdmData: &gdm.Data{Type: gdm.DataType_request, Request: &gdm.RequestData{Type: gdm.RequestType(-1)}}, + gdmData: &gdm.Data{ + Type: gdm.DataType_request, + Request: &gdm.RequestData{Type: gdm.RequestType(-1)}, + }, wantErrMsg: "unexpected request type", }, diff --git a/pam/internal/pam_test/pam-client-dummy.go b/pam/internal/pam_test/pam-client-dummy.go index a10f7b22b..a3ee9bf6e 100644 --- a/pam/internal/pam_test/pam-client-dummy.go +++ b/pam/internal/pam_test/pam-client-dummy.go @@ -232,7 +232,8 @@ func NewDummyClient(privateKey *rsa.PrivateKey, args ...DummyClientOptions) *Dum } // AvailableBrokers simulates AvailableBrokers using the provided parameters. -func (dc *DummyClient) AvailableBrokers(ctx context.Context, in *authd.Empty, opts ...grpc.CallOption) (*authd.ABResponse, error) { +func (dc *DummyClient) AvailableBrokers(ctx context.Context, in *authd.Empty, opts ...grpc.CallOption) ( + *authd.ABResponse, error) { log.Debugf(ctx, "AvailableBrokers Called: %#v", in) dc.mu.Lock() defer dc.mu.Unlock() @@ -247,7 +248,8 @@ func (dc *DummyClient) availableBrokers() (*authd.ABResponse, error) { } // GetPreviousBroker simulates GetPreviousBroker using the provided parameters. -func (dc *DummyClient) GetPreviousBroker(ctx context.Context, in *authd.GPBRequest, opts ...grpc.CallOption) (*authd.GPBResponse, error) { +func (dc *DummyClient) GetPreviousBroker(ctx context.Context, in *authd.GPBRequest, opts ...grpc.CallOption) ( + *authd.GPBResponse, error) { log.Debugf(ctx, "GetPreviousBroker Called: %#v", in) dc.mu.Lock() defer dc.mu.Unlock() @@ -268,7 +270,8 @@ func (dc *DummyClient) GetPreviousBroker(ctx context.Context, in *authd.GPBReque } // SelectBroker simulates SelectBroker using the provided parameters. -func (dc *DummyClient) SelectBroker(ctx context.Context, in *authd.SBRequest, opts ...grpc.CallOption) (*authd.SBResponse, error) { +func (dc *DummyClient) SelectBroker(ctx context.Context, in *authd.SBRequest, opts ...grpc.CallOption) ( + *authd.SBResponse, error) { log.Debugf(ctx, "SelectBroker Called: %#v", in) dc.mu.Lock() defer dc.mu.Unlock() @@ -326,7 +329,8 @@ func (dc *DummyClient) SelectBroker(ctx context.Context, in *authd.SBRequest, op } // GetAuthenticationModes simulates GetAuthenticationModes using the provided parameters. -func (dc *DummyClient) GetAuthenticationModes(ctx context.Context, in *authd.GAMRequest, opts ...grpc.CallOption) (*authd.GAMResponse, error) { +func (dc *DummyClient) GetAuthenticationModes(ctx context.Context, in *authd.GAMRequest, opts ...grpc.CallOption) ( + *authd.GAMResponse, error) { log.Debugf(ctx, "GetAuthenticationModes Called: %#v", in) dc.mu.Lock() defer dc.mu.Unlock() @@ -358,7 +362,8 @@ func (dc *DummyClient) GetAuthenticationModes(ctx context.Context, in *authd.GAM } // SelectAuthenticationMode simulates SelectAuthenticationMode using the provided parameters. -func (dc *DummyClient) SelectAuthenticationMode(ctx context.Context, in *authd.SAMRequest, opts ...grpc.CallOption) (*authd.SAMResponse, error) { +func (dc *DummyClient) SelectAuthenticationMode(ctx context.Context, in *authd.SAMRequest, opts ...grpc.CallOption) ( + *authd.SAMResponse, error) { log.Debugf(ctx, "SelectAuthenticationMode Called: %#v", in) dc.mu.Lock() defer dc.mu.Unlock() @@ -390,7 +395,8 @@ func (dc *DummyClient) SelectAuthenticationMode(ctx context.Context, in *authd.S } // IsAuthenticated simulates IsAuthenticated using the provided parameters. -func (dc *DummyClient) IsAuthenticated(ctx context.Context, in *authd.IARequest, opts ...grpc.CallOption) (*authd.IAResponse, error) { +func (dc *DummyClient) IsAuthenticated(ctx context.Context, in *authd.IARequest, opts ...grpc.CallOption) ( + *authd.IAResponse, error) { dc.mu.Lock() defer dc.mu.Unlock() log.Debugf(ctx, "IsAuthenticated Called: %#v", in) @@ -488,7 +494,8 @@ func (dc *DummyClient) handleChallenge(challenge string, msg string) (*authd.IAR } // EndSession simulates EndSession using the provided parameters. -func (dc *DummyClient) EndSession(ctx context.Context, in *authd.ESRequest, opts ...grpc.CallOption) (*authd.Empty, error) { +func (dc *DummyClient) EndSession(ctx context.Context, in *authd.ESRequest, opts ...grpc.CallOption) ( + *authd.Empty, error) { log.Debugf(ctx, "EndSession Called: %#v", in) dc.mu.Lock() defer dc.mu.Unlock() @@ -510,7 +517,8 @@ func (dc *DummyClient) EndSession(ctx context.Context, in *authd.ESRequest, opts } // SetDefaultBrokerForUser simulates SetDefaultBrokerForUser using the provided parameters. -func (dc *DummyClient) SetDefaultBrokerForUser(ctx context.Context, in *authd.SDBFURequest, opts ...grpc.CallOption) (*authd.Empty, error) { +func (dc *DummyClient) SetDefaultBrokerForUser(ctx context.Context, in *authd.SDBFURequest, opts ...grpc.CallOption) ( + *authd.Empty, error) { log.Debugf(ctx, "SetDefaultBrokerForUser Called: %#v", in) dc.mu.Lock() defer dc.mu.Unlock() diff --git a/pam/internal/pam_test/pam-client-dummy_test.go b/pam/internal/pam_test/pam-client-dummy_test.go index 86b372eb2..e61216782 100644 --- a/pam/internal/pam_test/pam-client-dummy_test.go +++ b/pam/internal/pam_test/pam-client-dummy_test.go @@ -596,7 +596,8 @@ func TestSelectAuthenticationModes(t *testing.T) { client: NewDummyClient(nil), args: &authd.SAMRequest{SessionId: "session-id"}, skipBrokerSelection: true, - wantError: errors.New(`impossible to select authentication mode, session ID "session-id" not found`), + wantError: errors.New("impossible to select authentication mode, " + + `session ID "session-id" not found`), }, "Error with no authentication mode ID": { client: NewDummyClient(nil, diff --git a/pam/internal/pam_test/runner-utils.go b/pam/internal/pam_test/runner-utils.go index fd6a907d1..4382cac6e 100644 --- a/pam/internal/pam_test/runner-utils.go +++ b/pam/internal/pam_test/runner-utils.go @@ -3,11 +3,13 @@ package pam_test const ( // RunnerEnvLogFile is the environment variable used by the test client to set the log file. RunnerEnvLogFile = "AUTHD_PAM_RUNNER_LOG_FILE" - // RunnerEnvSupportsConversation is the environment variable used by the test client to set whether it supports PAM conversations. + // RunnerEnvSupportsConversation is the environment variable used by the test client to set whether + // it supports PAM conversations. RunnerEnvSupportsConversation = "AUTHD_PAM_RUNNER_SUPPORTS_CONVERSATION" // RunnerEnvExecModule is the environment variable used by the test client to set the exec module library path. RunnerEnvExecModule = "AUTHD_PAM_RUNNER_EXEC_MODULE" - // RunnerEnvExecChildPath is the environment variable used by the test client to get the PAM exec child client application. + // RunnerEnvExecChildPath is the environment variable used by the test client to get the + // PAM exec child client application. RunnerEnvExecChildPath = "AUTHD_PAM_RUNNER_EXEC_CHILD_PATH" // RunnerEnvTestName is the environment variable used by the test client to set the test name. RunnerEnvTestName = "AUTHD_PAM_RUNNER_TEST_NAME" diff --git a/pam/internal/pam_test/service-utils_test.go b/pam/internal/pam_test/service-utils_test.go index ce9ec1358..5aadc335b 100644 --- a/pam/internal/pam_test/service-utils_test.go +++ b/pam/internal/pam_test/service-utils_test.go @@ -47,6 +47,7 @@ func TestCreateService(t *testing.T) { {Session, Optional, "pam_session_module.so", []string{""}}, {Session, Optional, Ignore.String(), []string{}}, }, + //nolint:lll wantContent: `account required pam_account_module.so a b c [d e] [f [g h\]] account required pam_deny.so auth requisite pam_auth_module.so diff --git a/pam/pam.go b/pam/pam.go index bc163abae..7c5662aca 100644 --- a/pam/pam.go +++ b/pam/pam.go @@ -54,7 +54,7 @@ var supportedArgs = []string{ "disable_journal", // Disable logging on systemd journal (this is implicit when `logfile` is set). "socket", // The authd socket to connect to. "force_native_client", // Use native PAM client instead of custom UIs. - "force_reauth", // Whether the authentication should be performed again even if it has been already completed. + "force_reauth", // Whether the authentication should be performed again when it's been already completed. } // parseArgs parses the PAM arguments and returns a map of them and a function that logs the parsing issues. @@ -220,7 +220,8 @@ func (h *pamModule) ChangeAuthTok(mTx pam.ModuleTransaction, flags pam.Flags, ar return err } -func (h *pamModule) handleAuthRequest(mode authd.SessionMode, mTx pam.ModuleTransaction, flags pam.Flags, parsedArgs map[string]string, logArgsIssues func()) (err error) { +func (h *pamModule) handleAuthRequest(mode authd.SessionMode, mTx pam.ModuleTransaction, flags pam.Flags, + parsedArgs map[string]string, logArgsIssues func()) (err error) { // Initialize localization // TODO