From 8b05a5f630cbc0dcccf13ddf2b0b351403e0bf1a Mon Sep 17 00:00:00 2001 From: Nabendu Maiti Date: Thu, 13 Aug 2026 14:08:30 +0530 Subject: [PATCH 1/2] fix(auth): store admin credentials from config into keystore Migrate the admin username/password out of config.yml and env vars into the OS keyring on startup, with .env and config.yml as fallback sources, per the centralize-secret-storage ADR (standalone mode). Signed-off-by: Nabendu Maiti --- .env.example | 8 +- cmd/app/main.go | 11 +- cmd/app/main_test.go | 155 +++++++++++++-- cmd/app/secret_store.go | 413 ++++++++++++++++++++++++++++++++++++++++ config/config.go | 77 +++++++- config/config.yml | 14 +- config/config_test.go | 35 ++++ go.mod | 1 + go.sum | 2 + 9 files changed, 692 insertions(+), 24 deletions(-) create mode 100644 cmd/app/secret_store.go diff --git a/.env.example b/.env.example index 5a9c0e6e7..8ef0ddfd9 100644 --- a/.env.example +++ b/.env.example @@ -47,10 +47,12 @@ EA_PASSWORD= # Auth AUTH_DISABLED=false -AUTH_ADMIN_USERNAME=standalone -# AUTH_ADMIN_PASSWORD: If not set, a random password is generated +# AUTH_ADMIN_USERNAME: resolved from keystore first; set here only as last fallback +AUTH_ADMIN_USERNAME= +# AUTH_ADMIN_PASSWORD: resolved from keystore first; set here only as last fallback AUTH_ADMIN_PASSWORD= -AUTH_JWT_KEY=your_secret_jwt_key +# AUTH_JWT_KEY: set for shared multi-pod deployments; leave empty for standalone runtime-only key +AUTH_JWT_KEY= AUTH_JWT_EXPIRATION=24h AUTH_REDIRECTION_JWT_EXPIRATION=5m # Ignored when AUTH_CLIENT_ID is set (OIDC). diff --git a/cmd/app/main.go b/cmd/app/main.go index 3bbaa563c..9700415d8 100644 --- a/cmd/app/main.go +++ b/cmd/app/main.go @@ -44,6 +44,15 @@ func main() { runHealthCheck() } + handled, err := handleAdminCLI(os.Args[1:], newKeyringStorageFunc(), os.Stdout) + if err != nil { + log.Fatalf("Admin command error: %v", err) + } + + if handled { + return + } + cfg, err := initializeConfigFunc() if err != nil { log.Fatalf("Config error: %s", err) @@ -66,7 +75,7 @@ func main() { l := logger.New(cfg.Level) handleEncryptionKey(cfg) - handleAdminPassword(cfg) + handleAdminCredentials(cfg) // Run with system tray (if built with tray tag and --tray flag) or standard mode if config.TrayMode && !trayBuildEnabled { diff --git a/cmd/app/main_test.go b/cmd/app/main_test.go index be738baa1..cc40cf770 100644 --- a/cmd/app/main_test.go +++ b/cmd/app/main_test.go @@ -1,9 +1,13 @@ package main import ( + "bufio" + "bytes" "crypto/rsa" "crypto/x509" + "errors" "os" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -16,16 +20,54 @@ import ( "github.com/device-management-toolkit/console/pkg/logger" ) +type mockCredentialStore struct { + values map[string]string + errMap map[string]error + deletedKeys []string +} + +func (m *mockCredentialStore) GetKeyValue(key string) (string, error) { + if err, ok := m.errMap[key]; ok { + return "", err + } + + if v, ok := m.values[key]; ok { + return v, nil + } + + return "", security.ErrKeyNotFound +} + +func (m *mockCredentialStore) SetKeyValue(key, value string) error { + if m.values == nil { + m.values = map[string]string{} + } + + m.values[key] = value + + return nil +} + +func (m *mockCredentialStore) DeleteKeyValue(key string) error { + if err, ok := m.errMap[key+":delete"]; ok { + return err + } + + m.deletedKeys = append(m.deletedKeys, key) + delete(m.values, key) + + return nil +} + func TestMainFunction(_ *testing.T) { //nolint:paralleltest // cannot have simultaneous tests modifying env variables. os.Setenv("GIN_MODE", "debug") - // Mock functions initializeConfigFunc = func() (*config.Config, error) { return &config.Config{ HTTP: config.HTTP{Port: "8080"}, App: config.App{EncryptionKey: "test"}, Log: config.Log{Level: "info"}, - Auth: config.Auth{AdminPassword: "test"}, + Auth: config.Auth{AdminUsername: "admin", AdminPassword: "test"}, }, nil } @@ -35,7 +77,6 @@ func TestMainFunction(_ *testing.T) { //nolint:paralleltest // cannot have simul runAppFunc = func(_ *config.Config, _ logger.Interface) {} - // Mock certificate functions loadOrGenerateRootCertFunc = func(_ security.Storager, _ bool, _, _, _ string, _ bool) (*x509.Certificate, *rsa.PrivateKey, error) { return &x509.Certificate{}, &rsa.PrivateKey{}, nil } @@ -44,11 +85,9 @@ func TestMainFunction(_ *testing.T) { //nolint:paralleltest // cannot have simul return &x509.Certificate{}, &rsa.PrivateKey{}, nil } - // Call the main function main() } -// TestGenerateRandomPassword tests the password generation function. func TestGenerateRandomPassword(t *testing.T) { t.Parallel() @@ -72,7 +111,6 @@ func TestGenerateRandomPassword(t *testing.T) { } } -// TestGenerateRandomPassword_Uniqueness ensures generated passwords are unique. func TestGenerateRandomPassword_Uniqueness(t *testing.T) { t.Parallel() @@ -82,22 +120,117 @@ func TestGenerateRandomPassword_Uniqueness(t *testing.T) { password, err := generateRandomPassword(16) require.NoError(t, err) assert.False(t, passwords[password], "generated duplicate password") - passwords[password] = true } } -// TestHandleAdminPassword_AlreadyConfigured tests when password is already set. func TestHandleAdminPassword_AlreadyConfigured(t *testing.T) { t.Parallel() cfg := &config.Config{ - Auth: config.Auth{ - AdminPassword: "already-set", - }, + Auth: config.Auth{AdminPassword: "already-set"}, } handleAdminPassword(cfg) assert.Equal(t, "already-set", cfg.AdminPassword) } + +func TestResolveAdminCredentialsFromSources_PriorityOrder(t *testing.T) { + t.Setenv(authAdminUsernameEnv, "env-user") + t.Setenv(authAdminSecretEnv, "env-pass") + + cfg := &config.Config{Auth: config.Auth{AdminUsername: "cfg-user", AdminPassword: "cfg-pass"}} + store := &mockCredentialStore{values: map[string]string{ + keyringAdminUsername: "keyring-user", + keyringAdminPassword: "keyring-pass", + }} + + username, password := resolveAdminCredentialsFromSources(cfg, store, map[string]string{ + authAdminUsernameEnv: "dotenv-user", + authAdminSecretEnv: "dotenv-pass", + }) + + assert.Equal(t, "keyring-user", username) + assert.Equal(t, "keyring-pass", password) +} + +func TestResolveAdminCredentialsFromSources_FallbackToDotEnvThenConfig(t *testing.T) { + t.Setenv(authAdminUsernameEnv, "") + t.Setenv(authAdminSecretEnv, "") + + cfg := &config.Config{Auth: config.Auth{AdminUsername: "cfg-user", AdminPassword: "cfg-pass"}} + store := &mockCredentialStore{errMap: map[string]error{ + keyringAdminUsername: security.ErrKeyNotFound, + keyringAdminPassword: security.ErrKeyNotFound, + }} + + username, password := resolveAdminCredentialsFromSources(cfg, store, map[string]string{ + authAdminUsernameEnv: "dotenv-user", + }) + + assert.Equal(t, "dotenv-user", username) + assert.Equal(t, "cfg-pass", password) +} + +func TestResolveAdminCredentialsFromSources_KeyringReadErrorFallsBack(t *testing.T) { + t.Parallel() + + cfg := &config.Config{Auth: config.Auth{AdminUsername: "cfg-user", AdminPassword: "cfg-pass"}} + store := &mockCredentialStore{errMap: map[string]error{ + keyringAdminUsername: errors.New("keyring unavailable"), + keyringAdminPassword: errors.New("keyring unavailable"), + }} + + username, password := resolveAdminCredentialsFromSources(cfg, store, map[string]string{}) + + assert.Equal(t, "cfg-user", username) + assert.Equal(t, "cfg-pass", password) +} + +func TestConfirmPersistCredentialsToConfig_Yes(t *testing.T) { + t.Parallel() + + reader := bufio.NewReader(strings.NewReader("Y\n")) + assert.True(t, confirmPersistCredentialsToConfig(reader)) +} + +func TestConfirmPersistCredentialsToConfig_No(t *testing.T) { + t.Parallel() + + reader := bufio.NewReader(strings.NewReader("n\n")) + assert.False(t, confirmPersistCredentialsToConfig(reader)) +} + +func TestHandleAdminCLI_ShowAdmin_HidesPassword(t *testing.T) { + t.Parallel() + + store := &mockCredentialStore{values: map[string]string{ + keyringAdminUsername: "alice", + keyringAdminPassword: "hash-value", + }} + buf := &bytes.Buffer{} + + handled, err := handleAdminCLI([]string{"--show-admin"}, store, buf) + require.NoError(t, err) + assert.True(t, handled) + assert.Contains(t, buf.String(), "admin username: alice") + assert.NotContains(t, buf.String(), "hash-value") +} + +func TestHandleAdminCLI_RemoveAdmin(t *testing.T) { + t.Parallel() + + store := &mockCredentialStore{values: map[string]string{ + keyringAdminUsername: "alice", + keyringAdminPassword: "super-secret", + }} + buf := &bytes.Buffer{} + + handled, err := handleAdminCLI([]string{"--remove-admin"}, store, buf) + require.NoError(t, err) + assert.True(t, handled) + assert.Contains(t, store.deletedKeys, keyringAdminUsername) + assert.Contains(t, store.deletedKeys, keyringAdminPassword) + assert.Contains(t, buf.String(), "Admin credentials removed from keystore.") +} diff --git a/cmd/app/secret_store.go b/cmd/app/secret_store.go new file mode 100644 index 000000000..72e135ee4 --- /dev/null +++ b/cmd/app/secret_store.go @@ -0,0 +1,413 @@ +package main + +import ( + "bufio" + "errors" + "fmt" + "io" + "log" + "os" + "strings" + + "golang.org/x/term" + + "github.com/device-management-toolkit/go-wsman-messages/v2/pkg/security" + + "github.com/device-management-toolkit/console/config" +) + +const ( + keyringServiceName = "device-management-toolkit" + keyringAdminUsername = "console-admin-username" + keyringAdminPassword = "console-admin-password" + dotEnvFile = ".env" + authAdminUsernameEnv = "AUTH_ADMIN_USERNAME" + authAdminSecretEnv = "AUTH_ADMIN_PASSWORD" // #nosec G101 -- environment variable name, not a credential value + dotEnvSplitParts = 2 +) + +var ( + errAdminCLIExclusiveFlags = errors.New("use only one of --remove-admin or --show-admin") + errKeystoreUnavailable = errors.New("keystore is unavailable") +) + +// credentialStore is intentionally narrow so alternate secret backends can be +// plugged in later without changing startup or CLI behavior. +type credentialStore interface { + GetKeyValue(key string) (string, error) + SetKeyValue(key, value string) error + DeleteKeyValue(key string) error +} + +// newKeyringStorageFunc is injectable for tests and can be replaced with other +// secret-store factories in future integrations. +var newKeyringStorageFunc = func() credentialStore { return security.NewKeyRingStorage(keyringServiceName) } + +func handleAdminCLI(args []string, keyringStore credentialStore, out io.Writer) (bool, error) { + command := selectedAdminCommand(args) + if command == "" { + return false, nil + } + + if command == "conflict" { + return true, errAdminCLIExclusiveFlags + } + + if keyringStore == nil { + return true, errKeystoreUnavailable + } + + switch command { + case "remove": + return true, handleRemoveAdminCLI(keyringStore, out) + case "show": + return true, handleShowAdminCLI(keyringStore, out) + default: + return true, errAdminCLIExclusiveFlags + } +} + +func selectedAdminCommand(args []string) string { + selected := 0 + command := "" + + if hasArg(args, "--remove-admin") { + selected++ + command = "remove" + } + + if hasArg(args, "--show-admin") { + selected++ + command = "show" + } + + if selected == 0 { + return "" + } + + if selected > 1 { + return "conflict" + } + + return command +} + +func handleRemoveAdminCLI(keyringStore credentialStore, out io.Writer) error { + if err := keyringStore.DeleteKeyValue(keyringAdminUsername); err != nil && !errors.Is(err, security.ErrKeyNotFound) { + return fmt.Errorf("failed to remove admin username from keystore: %w", err) + } + + if err := keyringStore.DeleteKeyValue(keyringAdminPassword); err != nil && !errors.Is(err, security.ErrKeyNotFound) { + return fmt.Errorf("failed to remove admin password from keystore: %w", err) + } + + if err := config.ClearAdminCredentials(); err != nil { + fmt.Fprintf(out, "Warning: failed to clear admin credentials from config.yml: %v\n", err) + } + + fmt.Fprintln(out, "Admin credentials removed from keystore.") + + return nil +} + +func handleShowAdminCLI(keyringStore credentialStore, out io.Writer) error { + username, err := keyringStore.GetKeyValue(keyringAdminUsername) + if err != nil { + return fmt.Errorf("failed to read admin username from keystore: %w", err) + } + + fmt.Fprintf(out, "admin username: %s\n", username) + + return nil +} + +func hasArg(args []string, target string) bool { + for _, arg := range args { + if arg == target { + return true + } + } + + return false +} + +func handleAdminCredentials(cfg *config.Config) { + if cfg.Disabled { + log.Print("Auth is disabled; skipping admin credential resolution.") + + return + } + + dotEnvValues := readDotEnvFile(dotEnvFile) + keyringStore := newKeyringStorageFunc() + + username, password := resolveAdminCredentialsFromSources(cfg, keyringStore, dotEnvValues) + + reader := bufio.NewReader(os.Stdin) + promptedForUsername := false + promptedForPassword := false + + if username == "" { + promptedForUsername = true + username = promptForCredential(reader, "Enter Console admin username: ") + } + + if password == "" { + promptedForPassword = true + password = promptForSecret(reader, "Enter Console admin password: ") + } + + cfg.AdminUsername = username + cfg.AdminPassword = password + + persistedToKeyring, usernameSaveErr, passwordSaveErr := saveAdminCredentialsToKeyring(keyringStore, cfg.AdminUsername, cfg.AdminPassword) + if persistedToKeyring { + clearAdminCredentialsInConfigAfterKeyringStore() + + return + } + + logKeyringSaveAndRollbackWarnings(keyringStore, usernameSaveErr, passwordSaveErr) + + if !promptedForUsername && !promptedForPassword { + return + } + + log.Print("Warning: keyring storage is unavailable; entered admin credentials may be lost after restart.") + + if !confirmPersistCredentialsToConfig(reader) { + log.Print("Admin credentials were not written to config.yml. Provide them again on next startup or configure keyring/.env.") + + return + } + + if err := config.SaveAdminCredentials(cfg.AdminUsername, cfg.AdminPassword); err != nil { + log.Printf("Warning: failed to persist admin credentials to config.yml: %v", err) + } else { + log.Print("Admin credentials persisted to config.yml.") + } +} + +func saveAdminCredentialsToKeyring(keyringStore credentialStore, username, passwordHash string) (persisted bool, usernameErr, passwordErr error) { + usernameErr = keyringStore.SetKeyValue(keyringAdminUsername, username) + passwordErr = keyringStore.SetKeyValue(keyringAdminPassword, passwordHash) + + if usernameErr == nil && passwordErr == nil { + return true, nil, nil + } + + return false, usernameErr, passwordErr +} + +func clearAdminCredentialsInConfigAfterKeyringStore() { + if err := config.ClearAdminCredentials(); err != nil { + log.Printf("Warning: failed to clear admin credentials from config.yml: %v", err) + } else { + log.Print("Admin credentials cleared from config.yml (stored in keystore).") + } +} + +func logKeyringSaveAndRollbackWarnings(keyringStore credentialStore, usernameSaveErr, passwordSaveErr error) { + if usernameSaveErr != nil { + log.Printf("Warning: failed to save admin username to keyring: %v", usernameSaveErr) + } + + if passwordSaveErr != nil { + if usernameSaveErr == nil { + if rollbackErr := keyringStore.DeleteKeyValue(keyringAdminUsername); rollbackErr != nil && !errors.Is(rollbackErr, security.ErrKeyNotFound) { + log.Printf("Warning: failed to rollback admin username from keyring: %v", rollbackErr) + } + } + + log.Printf("Warning: failed to save admin password to keyring: %v", passwordSaveErr) + } + + if usernameSaveErr == nil || passwordSaveErr != nil { + return + } + + if rollbackErr := keyringStore.DeleteKeyValue(keyringAdminPassword); rollbackErr != nil && !errors.Is(rollbackErr, security.ErrKeyNotFound) { + log.Printf("Warning: failed to rollback admin password from keyring: %v", rollbackErr) + } +} + +func resolveAdminCredentialsFromSources(cfg *config.Config, keyringStore credentialStore, dotEnvValues map[string]string) (username, password string) { + username = "" + password = "" + + if keyringStore != nil { + username, password, ok := readAdminCredentialsFromKeyring(keyringStore) + if ok { + return username, password + } + } + + if username == "" { + username = strings.TrimSpace(firstNonEmpty(dotEnvValues[authAdminUsernameEnv], os.Getenv(authAdminUsernameEnv))) + } + + if password == "" { + password = strings.TrimSpace(firstNonEmpty(dotEnvValues[authAdminSecretEnv], os.Getenv(authAdminSecretEnv))) + } + + if username == "" { + username = strings.TrimSpace(cfg.AdminUsername) + } + + if password == "" { + password = strings.TrimSpace(cfg.AdminPassword) + } + + return username, password +} + +func readAdminCredentialsFromKeyring(keyringStore credentialStore) (username, password string, ok bool) { + usernameVal, usernameErr := keyringStore.GetKeyValue(keyringAdminUsername) + passwordVal, passwordErr := keyringStore.GetKeyValue(keyringAdminPassword) + + if usernameErr == nil && passwordErr == nil { + username = strings.TrimSpace(usernameVal) + password = strings.TrimSpace(passwordVal) + + if username == "" || password == "" { + log.Print("Warning: incomplete admin credentials in keyring; falling back to next source.") + + return "", "", false + } + + return username, password, true + } + + if usernameErr != nil && !errors.Is(usernameErr, security.ErrKeyNotFound) { + log.Printf("Warning: failed to read admin username from keyring: %v", usernameErr) + } + + if passwordErr != nil && !errors.Is(passwordErr, security.ErrKeyNotFound) { + log.Printf("Warning: failed to read admin password from keyring: %v", passwordErr) + } + + if (usernameErr == nil && errors.Is(passwordErr, security.ErrKeyNotFound)) || + (passwordErr == nil && errors.Is(usernameErr, security.ErrKeyNotFound)) { + log.Print("Warning: partial admin credentials found in keyring; falling back to next source.") + } + + return "", "", false +} + +func promptForCredential(reader *bufio.Reader, prompt string) string { + for { + fmt.Fprint(os.Stdout, prompt) + + input, err := reader.ReadString('\n') + if err != nil { + if errors.Is(err, io.EOF) { + log.Fatal("failed to read credential from console: EOF (non-interactive startup). Set AUTH_ADMIN_USERNAME and AUTH_ADMIN_PASSWORD, or enable AUTH_DISABLED=true.") + } + + log.Fatalf("failed to read credential from console: %v", err) + } + + value := strings.TrimSpace(input) + if value != "" { + return value + } + + log.Println("Value cannot be empty.") + } +} + +func promptForSecret(reader *bufio.Reader, prompt string) string { + for { + fmt.Fprint(os.Stdout, prompt) + + if term.IsTerminal(int(os.Stdin.Fd())) { + input, err := term.ReadPassword(int(os.Stdin.Fd())) + + fmt.Fprintln(os.Stdout) + + if err != nil { + log.Fatalf("failed to read credential from console: %v", err) + } + + value := strings.TrimSpace(string(input)) + if value != "" { + return value + } + + log.Println("Value cannot be empty.") + + continue + } + + input, err := reader.ReadString('\n') + if err != nil { + if errors.Is(err, io.EOF) { + log.Fatal("failed to read credential from console: EOF (non-interactive startup). Set AUTH_ADMIN_USERNAME and AUTH_ADMIN_PASSWORD, or enable AUTH_DISABLED=true.") + } + + log.Fatalf("failed to read credential from console: %v", err) + } + + value := strings.TrimSpace(input) + if value != "" { + return value + } + + log.Println("Value cannot be empty.") + } +} + +func confirmPersistCredentialsToConfig(reader *bufio.Reader) bool { + log.Print("Store admin username/password in config.yml as fallback? Y/N: ") + + input, err := reader.ReadString('\n') + if err != nil { + log.Fatalf("failed to read confirmation from console: %v", err) + } + + response := strings.TrimSpace(input) + + return response == "Y" || response == "y" +} + +func readDotEnvFile(path string) map[string]string { + values := map[string]string{} + + data, err := os.ReadFile(path) + if err != nil { + return values + } + + lines := strings.Split(string(data), "\n") + for _, line := range lines { + trimmed := strings.TrimSpace(line) + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + + parts := strings.SplitN(trimmed, "=", dotEnvSplitParts) + if len(parts) != dotEnvSplitParts { + continue + } + + key := strings.TrimSpace(parts[0]) + value := strings.TrimSpace(parts[1]) + value = strings.Trim(value, "\"'") + + if key != "" { + values[key] = value + } + } + + return values +} + +func firstNonEmpty(values ...string) string { + for _, v := range values { + if strings.TrimSpace(v) != "" { + return v + } + } + + return "" +} diff --git a/config/config.go b/config/config.go index cf85a8ad3..57e9b9a28 100644 --- a/config/config.go +++ b/config/config.go @@ -1,6 +1,8 @@ package config import ( + "crypto/rand" + "encoding/base64" "errors" "flag" "net" @@ -22,6 +24,8 @@ const defaultHost = "localhost" // DefaultSessionCookieName names the HttpOnly cookie holding the session JWT. const DefaultSessionCookieName = "console_session" +const runtimeJWTKeyByteSize = 32 + type ( // Config -. Config struct { @@ -103,7 +107,7 @@ type ( Disabled bool `yaml:"disabled" env:"AUTH_DISABLED"` AdminUsername string `yaml:"adminUsername" env:"AUTH_ADMIN_USERNAME"` AdminPassword string `yaml:"adminPassword" env:"AUTH_ADMIN_PASSWORD"` - JWTKey string `env-required:"true" yaml:"jwtKey" env:"AUTH_JWT_KEY"` + JWTKey string `yaml:"jwtKey" env:"AUTH_JWT_KEY"` JWTExpiration time.Duration `yaml:"jwtExpiration" env:"AUTH_JWT_EXPIRATION"` RedirectionJWTExpiration time.Duration `yaml:"redirectionJWTExpiration" env:"AUTH_REDIRECTION_JWT_EXPIRATION"` ClientID string `yaml:"clientId" env:"AUTH_CLIENT_ID"` @@ -205,9 +209,9 @@ func defaultConfig() *Config { Password: "", }, Auth: Auth{ - AdminUsername: "standalone", - AdminPassword: "", // Generated and stored in config on first run if not provided - JWTKey: "your_secret_jwt_key", + AdminUsername: "", // Resolved at startup: keystore > .env > env var > config.yml > prompt + AdminPassword: "", // Resolved at startup: keystore > .env > env var > config.yml > prompt + JWTKey: "", JWTExpiration: 24 * time.Hour, RedirectionJWTExpiration: 5 * time.Minute, CookieEnabled: true, @@ -233,6 +237,31 @@ func defaultConfig() *Config { } } +func generateRuntimeJWTKey() (string, error) { + bytes := make([]byte, runtimeJWTKeyByteSize) + + if _, err := rand.Read(bytes); err != nil { + return "", err + } + + return base64.RawURLEncoding.EncodeToString(bytes), nil +} + +func ensureRuntimeJWTKey(cfg *Config) error { + if cfg.JWTKey != "" { + return nil + } + + jwtKey, err := generateRuntimeJWTKey() + if err != nil { + return err + } + + cfg.JWTKey = jwtKey + + return nil +} + // resolveConfigPath determines the effective config file path based on a flag value or default location. func resolveConfigPath(configPathFlag string) (string, error) { if configPathFlag != "" { @@ -332,6 +361,42 @@ func SaveAdminPassword(adminPassword string) error { return writeConfig(configPath, fileCfg) } +// ClearAdminCredentials removes adminUsername and adminPassword from config.yml +// so plaintext credentials do not remain on disk after being saved to the keystore. +func ClearAdminCredentials() error { + return SaveAdminCredentials("", "") +} + +// SaveAdminCredentials persists adminUsername/adminPassword to auth.* fields in +// config.yml without touching any other field. It re-reads the file directly +// (bypassing env overlays) so env-only secrets do not leak into disk config. +func SaveAdminCredentials(adminUsername, adminPassword string) error { + var configPathFlag string + if f := flag.Lookup("config"); f != nil { + configPathFlag = f.Value.String() + } + + configPath, err := resolveConfigPath(configPathFlag) + if err != nil { + return err + } + + data, err := os.ReadFile(configPath) + if err != nil { + return err + } + + fileCfg := defaultConfig() + if err := yaml.Unmarshal(data, fileCfg); err != nil { + return err + } + + fileCfg.AdminUsername = adminUsername + fileCfg.AdminPassword = adminPassword + + return writeConfig(configPath, fileCfg) +} + // NewConfig returns app config. func NewConfig() (*Config, error) { // set defaults @@ -365,5 +430,9 @@ func NewConfig() (*Config, error) { return nil, err } + if err := ensureRuntimeJWTKey(ConsoleConfig); err != nil { + return nil, err + } + return ConsoleConfig, nil } diff --git a/config/config.yml b/config/config.yml index 782bd309e..3834f4322 100644 --- a/config/config.yml +++ b/config/config.yml @@ -20,7 +20,7 @@ http: - "*" logger: log_level: info -secrets: +secrets: address: http://localhost:8200 token: "" postgres: @@ -37,14 +37,18 @@ ea: password: "" auth: disabled: false - adminUsername: standalone - adminPassword: - jwtKey: your_secret_jwt_key + # adminUsername: prefer keystore > .env (AUTH_ADMIN_USERNAME) > here + adminUsername: "" + # adminPassword: prefer keystore > .env (AUTH_ADMIN_PASSWORD) > here + # WARNING: storing plaintext passwords here is a fallback only; use keystore when possible. + adminPassword: "" + # jwtKey: set for shared multi-pod deployments; if empty, standalone generates a runtime-only key + jwtKey: "" jwtExpiration: 24h0m0s redirectionJWTExpiration: 5m0s clientId: "" issuer: "" - ui: + ui: clientId: "" issuer: "" scope: "" diff --git a/config/config_test.go b/config/config_test.go index 24e4360e7..65aeae08a 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func clearEnv() { @@ -107,3 +108,37 @@ postgres: assert.Equal(t, 10, cfg.PoolMax) assert.Equal(t, "postgres://envuser:envpassword@localhost:5432/envdb", cfg.DB.URL) } + +func TestDefaultConfig_JWTKeyEmptyByDefault(t *testing.T) { + t.Parallel() + + cfg := defaultConfig() + assert.Empty(t, cfg.JWTKey) +} + +func TestEnsureRuntimeJWTKey_GeneratesWhenMissing(t *testing.T) { + t.Parallel() + + cfg := defaultConfig() + require.Empty(t, cfg.JWTKey) + + err := ensureRuntimeJWTKey(cfg) + require.NoError(t, err) + assert.NotEmpty(t, cfg.JWTKey) + + firstKey := cfg.JWTKey + err = ensureRuntimeJWTKey(cfg) + require.NoError(t, err) + assert.Equal(t, firstKey, cfg.JWTKey) +} + +func TestEnsureRuntimeJWTKey_PreservesConfiguredValue(t *testing.T) { + t.Parallel() + + cfg := defaultConfig() + cfg.JWTKey = "configured-jwt-key" + + err := ensureRuntimeJWTKey(cfg) + require.NoError(t, err) + assert.Equal(t, "configured-jwt-key", cfg.JWTKey) +} diff --git a/go.mod b/go.mod index b984af959..a8f2ecf72 100644 --- a/go.mod +++ b/go.mod @@ -71,6 +71,7 @@ require ( go.opentelemetry.io/otel/metric v1.41.0 // indirect go.opentelemetry.io/otel/trace v1.41.0 // indirect golang.org/x/oauth2 v0.36.0 // indirect + golang.org/x/term v0.45.0 // indirect golang.org/x/time v0.12.0 // indirect modernc.org/libc v1.74.4 // indirect ) diff --git a/go.sum b/go.sum index 21f0b416c..83d24ddfc 100644 --- a/go.sum +++ b/go.sum @@ -316,6 +316,8 @@ golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= +golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= +golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= From d27d09d7eebdd3331763c27ceac1d358752a470d Mon Sep 17 00:00:00 2001 From: Nabendu Maiti Date: Thu, 13 Aug 2026 14:24:04 +0530 Subject: [PATCH 2/2] fix(auth): use bcrypt to hash cli input password before keystore write Hash admin passwords with bcrypt before they are written to the keystore, and compare bcrypt hashes on login instead of plaintext. Signed-off-by: Nabendu Maiti --- cmd/app/main_test.go | 22 +++++++++++++++ cmd/app/secret_store.go | 28 +++++++++++++++++++- internal/controller/httpapi/v1/login.go | 4 ++- internal/controller/httpapi/v1/login_test.go | 27 ++++++++++++------- 4 files changed, 70 insertions(+), 11 deletions(-) diff --git a/cmd/app/main_test.go b/cmd/app/main_test.go index cc40cf770..3a7d4e0cb 100644 --- a/cmd/app/main_test.go +++ b/cmd/app/main_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/crypto/bcrypt" "github.com/device-management-toolkit/go-wsman-messages/v2/pkg/security" @@ -136,6 +137,27 @@ func TestHandleAdminPassword_AlreadyConfigured(t *testing.T) { assert.Equal(t, "already-set", cfg.AdminPassword) } +func TestNormalizeAdminPasswordHash_PlainTextInput(t *testing.T) { + t.Parallel() + + hash, converted, err := normalizeAdminPasswordHash("plain-password") + require.NoError(t, err) + assert.True(t, converted) + require.NoError(t, bcrypt.CompareHashAndPassword([]byte(hash), []byte("plain-password"))) +} + +func TestNormalizeAdminPasswordHash_AlreadyHashed(t *testing.T) { + t.Parallel() + + existingHash, err := bcrypt.GenerateFromPassword([]byte("secret"), bcrypt.DefaultCost) + require.NoError(t, err) + + hash, converted, err := normalizeAdminPasswordHash(string(existingHash)) + require.NoError(t, err) + assert.False(t, converted) + assert.Equal(t, string(existingHash), hash) +} + func TestResolveAdminCredentialsFromSources_PriorityOrder(t *testing.T) { t.Setenv(authAdminUsernameEnv, "env-user") t.Setenv(authAdminSecretEnv, "env-pass") diff --git a/cmd/app/secret_store.go b/cmd/app/secret_store.go index 72e135ee4..0cc44f714 100644 --- a/cmd/app/secret_store.go +++ b/cmd/app/secret_store.go @@ -9,6 +9,7 @@ import ( "os" "strings" + "golang.org/x/crypto/bcrypt" "golang.org/x/term" "github.com/device-management-toolkit/go-wsman-messages/v2/pkg/security" @@ -23,6 +24,9 @@ const ( dotEnvFile = ".env" authAdminUsernameEnv = "AUTH_ADMIN_USERNAME" authAdminSecretEnv = "AUTH_ADMIN_PASSWORD" // #nosec G101 -- environment variable name, not a credential value + bcryptPrefix2A = "$2a$" + bcryptPrefix2B = "$2b$" + bcryptPrefix2Y = "$2y$" dotEnvSplitParts = 2 ) @@ -157,8 +161,13 @@ func handleAdminCredentials(cfg *config.Config) { password = promptForSecret(reader, "Enter Console admin password: ") } + hashedPassword, _, err := normalizeAdminPasswordHash(password) + if err != nil { + log.Fatalf("failed to hash admin password: %v", err) + } + cfg.AdminUsername = username - cfg.AdminPassword = password + cfg.AdminPassword = hashedPassword persistedToKeyring, usernameSaveErr, passwordSaveErr := saveAdminCredentialsToKeyring(keyringStore, cfg.AdminUsername, cfg.AdminPassword) if persistedToKeyring { @@ -411,3 +420,20 @@ func firstNonEmpty(values ...string) string { return "" } + +func normalizeAdminPasswordHash(password string) (hash string, converted bool, err error) { + if isBcryptHash(password) { + return password, false, nil + } + + bcryptHash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) + if err != nil { + return "", false, err + } + + return string(bcryptHash), true, nil +} + +func isBcryptHash(value string) bool { + return strings.HasPrefix(value, bcryptPrefix2A) || strings.HasPrefix(value, bcryptPrefix2B) || strings.HasPrefix(value, bcryptPrefix2Y) +} diff --git a/internal/controller/httpapi/v1/login.go b/internal/controller/httpapi/v1/login.go index c246d9299..ce29d92b8 100644 --- a/internal/controller/httpapi/v1/login.go +++ b/internal/controller/httpapi/v1/login.go @@ -12,6 +12,7 @@ import ( "github.com/coreos/go-oidc/v3/oidc" "github.com/gin-gonic/gin" "github.com/golang-jwt/jwt/v5" + "golang.org/x/crypto/bcrypt" "github.com/device-management-toolkit/console/config" "github.com/device-management-toolkit/console/internal/entity/dto/v1" @@ -83,7 +84,8 @@ func (lr LoginRoute) Login(c *gin.Context) { } func (lr LoginRoute) handleBasicAuth(creds dto.Credentials, c *gin.Context) { - if creds.Username != lr.Config.AdminUsername || creds.Password != lr.Config.AdminPassword { + passwordErr := bcrypt.CompareHashAndPassword([]byte(lr.Config.AdminPassword), []byte(creds.Password)) + if passwordErr != nil || creds.Username != lr.Config.AdminUsername { c.JSON(http.StatusUnauthorized, gin.H{errorKey: "invalid credentials", messageKey: "Incorrect Username and/or Password!"}) return diff --git a/internal/controller/httpapi/v1/login_test.go b/internal/controller/httpapi/v1/login_test.go index 38e3a2af6..49d4de05e 100644 --- a/internal/controller/httpapi/v1/login_test.go +++ b/internal/controller/httpapi/v1/login_test.go @@ -10,6 +10,7 @@ import ( "github.com/gin-gonic/gin" "github.com/stretchr/testify/require" + "golang.org/x/crypto/bcrypt" "github.com/device-management-toolkit/console/config" ) @@ -25,10 +26,15 @@ const ( ) // cookieAuthTestConfig is a basic-auth (non-OIDC) config with cookies enabled. -func cookieAuthTestConfig() *config.Config { +func cookieAuthTestConfig(t *testing.T) *config.Config { + t.Helper() + + hash, err := bcrypt.GenerateFromPassword([]byte(testAdminPass), bcrypt.DefaultCost) + require.NoError(t, err) + cfg := &config.Config{} cfg.AdminUsername = testAdminUser - cfg.AdminPassword = testAdminPass + cfg.AdminPassword = string(hash) cfg.JWTKey = testJWTKey cfg.JWTExpiration = time.Hour cfg.CookieEnabled = true @@ -114,7 +120,7 @@ func withCookie(cookie *http.Cookie) func(*http.Request) { // //nolint:paralleltest // shared global config.ConsoleConfig func TestAuthorizeIssuesSessionCookies(t *testing.T) { - engine := newAuthTestEngine(t, cookieAuthTestConfig()) + engine := newAuthTestEngine(t, cookieAuthTestConfig(t)) token, cookies := login(t, engine) require.NotEmpty(t, token, "token must remain in the response body for bearer clients") @@ -136,7 +142,7 @@ func TestAuthorizeIssuesSessionCookies(t *testing.T) { // //nolint:paralleltest // shared global config.ConsoleConfig func TestBearerAuthUnchanged(t *testing.T) { - engine := newAuthTestEngine(t, cookieAuthTestConfig()) + engine := newAuthTestEngine(t, cookieAuthTestConfig(t)) token, cookies := login(t, engine) @@ -160,7 +166,7 @@ func TestBearerAuthUnchanged(t *testing.T) { // //nolint:paralleltest // shared global config.ConsoleConfig func TestCookieAuthAcceptsSessionCookie(t *testing.T) { - engine := newAuthTestEngine(t, cookieAuthTestConfig()) + engine := newAuthTestEngine(t, cookieAuthTestConfig(t)) _, cookies := login(t, engine) session := cookies[config.DefaultSessionCookieName] @@ -180,11 +186,11 @@ func TestCookieAuthAcceptsSessionCookie(t *testing.T) { // //nolint:paralleltest // shared global config.ConsoleConfig func TestCookieAuthDisabled(t *testing.T) { - enabled := newAuthTestEngine(t, cookieAuthTestConfig()) + enabled := newAuthTestEngine(t, cookieAuthTestConfig(t)) _, cookies := login(t, enabled) session := cookies[config.DefaultSessionCookieName] - cfg := cookieAuthTestConfig() + cfg := cookieAuthTestConfig(t) cfg.CookieEnabled = false disabled := newAuthTestEngine(t, cfg) @@ -228,7 +234,7 @@ func TestLogoutWithoutConfig(t *testing.T) { // //nolint:paralleltest // shared global config.ConsoleConfig func TestLogoutExpiresSessionCookie(t *testing.T) { - engine := newAuthTestEngine(t, cookieAuthTestConfig()) + engine := newAuthTestEngine(t, cookieAuthTestConfig(t)) req, err := http.NewRequest(http.MethodPost, testLogoutURL, http.NoBody) require.NoError(t, err) @@ -252,8 +258,11 @@ func TestLogoutExpiresSessionCookie(t *testing.T) { func TestLogin_InvalidCredentialsReturnsMessage(t *testing.T) { t.Parallel() + hash, err := bcrypt.GenerateFromPassword([]byte("secret"), bcrypt.DefaultCost) + require.NoError(t, err) + engine := gin.New() - route := LoginRoute{Config: &config.Config{Auth: config.Auth{AdminUsername: "admin", AdminPassword: "secret"}}} + route := LoginRoute{Config: &config.Config{Auth: config.Auth{AdminUsername: "admin", AdminPassword: string(hash)}}} engine.POST("/api/v1/authorize", route.Login) req, err := http.NewRequest(http.MethodPost, "/api/v1/authorize", bytes.NewBufferString(`{"username":"admin","password":"wrong"}`))