Repository: kopia/kopia
Pull request #1065 (2021-05-12 03:57:39)

repo: refactored password persistence
* introduced `passwordpersist` package with password persistence strategies (keyring, file, none, multiple) with possibility of adding more in the future.
* moved all password persistence logic out of 'repo'
* removed global variable repo.EnableKeyRing


Patch:
diff --git a/cli/app.go b/cli/app.go
index d3be46c5..bf482cf2 100644
--- a/cli/app.go
+++ b/cli/app.go
@@ -16,6 +16,7 @@
 	"github.com/pkg/errors"
 
 	"github.com/kopia/kopia/internal/apiclient"
+	"github.com/kopia/kopia/internal/passwordpersist"
 	"github.com/kopia/kopia/repo"
 	"github.com/kopia/kopia/repo/blob"
 	"github.com/kopia/kopia/repo/logging"
@@ -95,6 +96,7 @@ type advancedAppServices interface {
 
 	maybeInitializeUpdateCheck(ctx context.Context, co *connectOptions)
 	removeUpdateState()
+	passwordPersistenceStrategy() passwordpersist.Strategy
 	getPasswordFromFlags(ctx context.Context, isNew, allowPersistent bool) (string, error)
 	optionsFromFlags(ctx context.Context) *repo.Options
 }
@@ -112,6 +114,8 @@ type App struct {
 	configPath                    string
 	traceStorage                  bool
 	metricsListenAddr             string
+	keyRingEnabled                bool
+	persistCredentials            bool
 
 	// subcommands
 	blob        commandBlob
@@ -150,6 +154,21 @@ func (c *App) stderr() io.Writer {
 	return c.stderrWriter
 }
 
+func (c *App) passwordPersistenceStrategy() passwordpersist.Strategy {
+	if !c.persistCredentials {
+		return passwordpersist.None
+	}
+
+	if c.keyRingEnabled {
+		return passwordpersist.Multiple{
+			passwordpersist.Keyring,
+			passwordpersist.File,
+		}
+	}
+
+	return passwordpersist.File
+}
+
 func (c *App) setup(app *kingpin.Application) {
 	_ = app.Flag("help-full", "Show help for all commands, including hidden").Action(func(pc *kingpin.ParseContext) error {
 		_ = app.UsageForContextWithTemplate(pc, 0, kingpin.DefaultUsageTemplate)
@@ -168,6 +187,7 @@ func (c *App) setup(app *kingpin.Application) {
 	app.Flag("metrics-listen-addr", "Expose Prometheus metrics on a given host:port").Hidden().StringVar(&c.metricsListenAddr)
 	app.Flag("timezone", "Format time according to specified time zone (local, utc, original or time zone name)").Default("local").Hidden().StringVar(&timeZone)
 	app.Flag("password", "Repository password.").Envar("KOPIA_PASSWORD").Short('p').StringVar(&c.password)
+	app.Flag("persist-credentials", "Persist credentials").Default("true").Envar("KOPIA_PERSIST_CREDENTIALS_ON_CONNECT").BoolVar(&c.persistCredentials)
 
 	c.setupOSSpecificKeychainFlags(app)
 
diff --git a/cli/command_repository_connect.go b/cli/command_repository_connect.go
index dcf3acfc..b853a822 100644
--- a/cli/command_repository_connect.go
+++ b/cli/command_repository_connect.go
@@ -7,6 +7,7 @@
 	"github.com/alecthomas/kingpin"
 	"github.com/pkg/errors"
 
+	"github.com/kopia/kopia/internal/passwordpersist"
 	"github.com/kopia/kopia/repo"
 	"github.com/kopia/kopia/repo/blob"
 	"github.com/kopia/kopia/repo/content"
@@ -42,7 +43,6 @@ func (c *commandRepositoryConnect) setup(svc advancedAppServices, parent command
 }
 
 type connectOptions struct {
-	connectPersistCredentials     bool
 	connectCacheDirectory         string
 	connectMaxCacheSizeMB         int64
 	connectMaxMetadataCacheSizeMB int64
@@ -58,7 +58,6 @@ type connectOptions struct {
 func (c *connectOptions) setup(cmd *kingpin.CmdClause) {
 	// Set up flags shared between 'create' and 'connect'. Note that because those flags are used by both command
 	// we must use *Var() methods, otherwise one of the commands would always get default flag values.
-	cmd.Flag("persist-credentials", "Persist credentials").Default("true").Envar("KOPIA_PERSIST_CREDENTIALS_ON_CONNECT").BoolVar(&c.connectPersistCredentials)
 	cmd.Flag("cache-directory", "Cache directory").PlaceHolder("PATH").Envar("KOPIA_CACHE_DIRECTORY").StringVar(&c.connectCacheDirectory)
 	cmd.Flag("content-cache-size-mb", "Size of local content cache").PlaceHolder("MB").Default("5000").Int64Var(&c.connectMaxCacheSizeMB)
 	cmd.Flag("metadata-cache-size-mb", "Size of local metadata cache").PlaceHolder("MB").Default("5000").Int64Var(&c.connectMaxMetadataCacheSizeMB)
@@ -73,7 +72,6 @@ func (c *connectOptions) setup(cmd *kingpin.CmdClause) {
 
 func (c *connectOptions) toRepoConnectOptions() *repo.ConnectOptions {
 	return &repo.ConnectOptions{
-		PersistCredentials: c.connectPersistCredentials,
 		CachingOptions: content.CachingOptions{
 			CacheDirectory:            c.connectCacheDirectory,
 			MaxCacheSizeBytes:         c.connectMaxCacheSizeMB << 20,         //nolint:gomnd
@@ -101,7 +99,9 @@ func (c *App) runConnectCommandWithStorage(ctx context.Context, co *connectOptio
 
 func (c *App) runConnectCommandWithStorageAndPassword(ctx context.Context, co *connectOptions, st blob.Storage, password string) error {
 	configFile := c.repositoryConfigFileName()
-	if err := repo.Connect(ctx, configFile, st, password, co.toRepoConnectOptions()); err != nil {
+	if err := passwordpersist.OnSuccess(
+		ctx, repo.Connect(ctx, configFile, st, password, co.toRepoConnectOptions()),
+		c.passwordPersistenceStrategy(), configFile, password); err != nil {
 		return errors.Wrap(err, "error connecting to repository")
 	}
 
diff --git a/cli/command_repository_connect_server.go b/cli/command_repository_connect_server.go
index a4717422..cf1a22ee 100644
--- a/cli/command_repository_connect_server.go
+++ b/cli/command_repository_connect_server.go
@@ -6,6 +6,7 @@
 
 	"github.com/pkg/errors"
 
+	"github.com/kopia/kopia/internal/passwordpersist"
 	"github.com/kopia/kopia/repo"
 )
 
@@ -59,7 +60,9 @@ func (c *commandRepositoryConnectServer) run(ctx context.Context) error {
 		return errors.Wrap(err, "getting password")
 	}
 
-	if err := repo.ConnectAPIServer(ctx, configFile, as, pass, opt); err != nil {
+	if err := passwordpersist.OnSuccess(
+		ctx, repo.ConnectAPIServer(ctx, configFile, as, pass, opt),
+		c.svc.passwordPersistenceStrategy(), configFile, pass); err != nil {
 		return errors.Wrap(err, "error connecting to API server")
 	}
 
diff --git a/cli/command_repository_disconnect.go b/cli/command_repository_disconnect.go
index e3c4dc73..729e05b3 100644
--- a/cli/command_repository_disconnect.go
+++ b/cli/command_repository_disconnect.go
@@ -3,6 +3,8 @@
 import (
 	"context"
 
+	"github.com/pkg/errors"
+
 	"github.com/kopia/kopia/repo"
 )
 
@@ -20,5 +22,13 @@ func (c *commandRepositoryDisconnect) setup(svc advancedAppServices, parent comm
 func (c *commandRepositoryDisconnect) run(ctx context.Context) error {
 	c.svc.removeUpdateState()
 
-	return repo.Disconnect(ctx, c.svc.repositoryConfigFileName())
+	if err := repo.Disconnect(ctx, c.svc.repositoryConfigFileName()); err != nil {
+		return errors.Wrap(err, "unable to disconnect from repository")
+	}
+
+	if err := c.svc.passwordPersistenceStrategy().DeletePassword(ctx, c.svc.repositoryConfigFileName()); err != nil {
+		return errors.Wrap(err, "unable to remove persisted password")
+	}
+
+	return nil
 }
diff --git a/cli/command_server.go b/cli/command_server.go
index 5fb2da9b..a5b299e1 100644
--- a/cli/command_server.go
+++ b/cli/command_server.go
@@ -42,7 +42,7 @@ func (c *serverClientFlags) setup(cmd *kingpin.CmdClause) {
 	cmd.Flag("server-cert-fingerprint", "Server certificate fingerprint").StringVar(&c.serverCertFingerprint)
 }
 
-func (c *commandServer) setup(svc appServices, parent commandParent) {
+func (c *commandServer) setup(svc advancedAppServices, parent commandParent) {
 	cmd := parent.Command("server", "Commands to control HTTP API server.")
 
 	c.cancel.setup(svc, cmd)
diff --git a/cli/command_server_start.go b/cli/command_server_start.go
index 7b37d057..09d08bb2 100644
--- a/cli/command_server_start.go
+++ b/cli/command_server_start.go
@@ -56,11 +56,11 @@ type commandServerStart struct {
 	serverStartTLSPrintFullServerCert   bool
 
 	sf  serverFlags
-	svc appServices
+	svc advancedAppServices
 	out textOutput
 }
 
-func (c *commandServerStart) setup(svc appServices, parent commandParent) {
+func (c *commandServerStart) setup(svc advancedAppServices, parent commandParent) {
 	cmd := parent.Command("start", "Start Kopia server").Default()
 	cmd.Flag("html", "Server the provided HTML at the root URL").ExistingDirVar(&c.serverStartHTMLPath)
 	cmd.Flag("ui", "Start the server with HTML UI").Default("true").BoolVar(&c.serverStartUI)
@@ -114,6 +114,7 @@ func (c *commandServerStart) run(ctx context.Context, rep repo.Repository) error
 		Authorizer:           auth.DefaultAuthorizer(),
 		AuthCookieSigningKey: c.serverAuthCookieSingingKey,
 		UIUser:               c.sf.serverUsername,
+		PasswordPersist:      c.svc.passwordPersistenceStrategy(),
 	})
 	if err != nil {
 		return errors.Wrap(err, "unable to initialize server")
diff --git a/cli/command_snapshot_migrate.go b/cli/command_snapshot_migrate.go
index c7162473..e72cee99 100644
--- a/cli/command_snapshot_migrate.go
+++ b/cli/command_snapshot_migrate.go
@@ -130,13 +130,13 @@ func (c *commandSnapshotMigrate) run(ctx context.Context, destRepo repo.Reposito
 }
 
 func (c *commandSnapshotMigrate) openSourceRepo(ctx context.Context) (repo.Repository, error) {
-	pass, ok := repo.GetPersistedPassword(ctx, c.migrateSourceConfig)
-	if !ok {
-		var err error
+	pass, err := c.svc.passwordPersistenceStrategy().GetPassword(ctx, c.migrateSourceConfig)
+	if err != nil {
+		pass, err = c.svc.getPasswordFromFlags(ctx, false, false)
+	}
 
-		if pass, err = c.svc.getPasswordFromFlags(ctx, false, false); err != nil {
-			return nil, errors.Wrap(err, "source repository password")
-		}
+	if err != nil {
+		return nil, errors.Wrap(err, "source repository password")
 	}
 
 	sourceRepo, err := repo.Open(ctx, c.migrateSourceConfig, pass, c.svc.optionsFromFlags(ctx))
diff --git a/cli/password.go b/cli/password.go
index aa22e2e0..ec8a6731 100644
--- a/cli/password.go
+++ b/cli/password.go
@@ -10,7 +10,7 @@
 	"github.com/pkg/errors"
 	"golang.org/x/term"
 
-	"github.com/kopia/kopia/repo"
+	"github.com/kopia/kopia/internal/passwordpersist"
 )
 
 func askForNewRepositoryPassword(out io.Writer) (string, error) {
@@ -58,10 +58,14 @@ func (c *App) getPasswordFromFlags(ctx context.Context, isNew, allowPersistent b
 		return askForNewRepositoryPassword(c.stdoutWriter)
 	case allowPersistent:
 		// try fetching the password from persistent storage specific to the configuration file.
-		pass, ok := repo.GetPersistedPassword(ctx, c.repositoryConfigFileName())
-		if ok {
+		pass, err := c.passwordPersistenceStrategy().GetPassword(ctx, c.repositoryConfigFileName())
+		if err == nil {
 			return pass, nil
 		}
+
+		if !errors.Is(err, passwordpersist.ErrPasswordNotFound) {
+			return "", errors.Wrap(err, "error getting persistent password")
+		}
 	}
 
 	// fall back to asking for existing password
diff --git a/cli/password_darwin.go b/cli/password_darwin.go
index ff330d61..7691b674 100644
--- a/cli/password_darwin.go
+++ b/cli/password_darwin.go
@@ -2,10 +2,8 @@
 
 import (
 	"github.com/alecthomas/kingpin"
-
-	"github.com/kopia/kopia/repo"
 )
 
 func (c *App) setupOSSpecificKeychainFlags(app *kingpin.Application) {
-	app.Flag("use-keychain", "Use macOS Keychain for storing repository password.").Default("true").BoolVar(&repo.KeyRingEnabled)
+	app.Flag("use-keychain", "Use macOS Keychain for storing repository password.").Default("true").BoolVar(&c.keyRingEnabled)
 }
diff --git a/cli/password_linux.go b/cli/password_linux.go
index 02053dbb..4b73693a 100644
--- a/cli/password_linux.go
+++ b/cli/password_linux.go
@@ -2,10 +2,8 @@
 
 import (
 	"github.com/alecthomas/kingpin"
-
-	"github.com/kopia/kopia/repo"
 )
 
 func (c *App) setupOSSpecificKeychainFlags(app *kingpin.Application) {
-	app.Flag("use-keyring", "Use Gnome Keyring for storing repository password.").Default("false").BoolVar(&repo.KeyRingEnabled)
+	app.Flag("use-keyring", "Use Gnome Keyring for storing repository password.").Default("false").BoolVar(&c.keyRingEnabled)
 }
diff --git a/cli/password_windows.go b/cli/password_windows.go
index 0c9e7951..ab540cc5 100644
--- a/cli/password_windows.go
+++ b/cli/password_windows.go
@@ -2,10 +2,8 @@
 
 import (
 	"github.com/alecthomas/kingpin"
-
-	"github.com/kopia/kopia/repo"
 )
 
 func (c *App) setupOSSpecificKeychainFlags(app *kingpin.Application) {
-	app.Flag("use-credential-manager", "Use Windows Credential Manager for storing repository password.").Default("true").BoolVar(&repo.KeyRingEnabled)
+	app.Flag("use-credential-manager", "Use Windows Credential Manager for storing repository password.").Default("true").BoolVar(&c.keyRingEnabled)
 }
diff --git a/internal/passwordpersist/passwordpersist.go b/internal/passwordpersist/passwordpersist.go
new file mode 100644
index 00000000..1c84f154
--- /dev/null
+++ b/internal/passwordpersist/passwordpersist.go
@@ -0,0 +1,44 @@
+// Package passwordpersist manages password persistence.
+package passwordpersist
+
+import (
+	"context"
+
+	"github.com/pkg/errors"
+
+	"github.com/kopia/kopia/repo/logging"
+)
+
+// ErrPasswordNotFound is returned when a password cannot be found in a persistent storage.
+var ErrPasswordNotFound = errors.Errorf("password not found")
+
+// ErrUnsupported is returned when a password storage is not supported.
+var ErrUnsupported = errors.Errorf("password storage not supported")
+
+var log = logging.GetContextLoggerFunc("passwordpersist")
+
+// Strategy encapsulates persisting and fetching passwords.
+type Strategy interface {
+	// GetPassword gets persisted password, returns ErrNotFound or fatal errors.
+	GetPassword(ctx context.Context, configFile string) (string, error)
+
+	// PersistPassword persists a password, returns ErrUnsupported or fatal errors.
+	PersistPassword(ctx context.Context, configFile, password string) error
+
+	// DeletePassword deletes any persisted password, returns fatal errors.
+	DeletePassword(ctx context.Context, configFile string) error
+}
+
+// OnSuccess is a helper that persists the given (configFile,password) if the provided err is nil
+// and deletes any persisted password otherwise.
+func OnSuccess(ctx context.Context, err error, s Strategy, configFile, password string) error {
+	if err != nil {
+		if err2 := s.DeletePassword(ctx, configFile); err2 != nil {
+			log(ctx).Infof("unable to delete persistent password: %v", err2)
+		}
+
+		return err
+	}
+
+	return errors.Wrap(s.PersistPassword(ctx, configFile, password), "unable to persist password")
+}
diff --git a/internal/passwordpersist/passwordpersist_file.go b/internal/passwordpersist/passwordpersist_file.go
new file mode 100644
index 00000000..3d20dd01
--- /dev/null
+++ b/internal/passwordpersist/passwordpersist_file.go
@@ -0,0 +1,55 @@
+package passwordpersist
+
+import (
+	"context"
+	"encoding/base64"
+	"io/ioutil"
+	"os"
+
+	"github.com/pkg/errors"
+)
+
+// File is a Strategy that persists the base64-encoded password in a file next to repository config file.
+var File Strategy = filePasswordStorage{}
+
+type filePasswordStorage struct{}
+
+func (filePasswordStorage) GetPassword(ctx context.Context, configFile string) (string, error) {
+	b, err := ioutil.ReadFile(passwordFileName(configFile))
+	if os.IsNotExist(err) {
+		return "", ErrPasswordNotFound
+	}
+
+	if err != nil {
+		return "", errors.Wrap(err, "error reading persisted password")
+	}
+
+	s, err := base64.StdEncoding.DecodeString(string(b))
+	if err != nil {
+		return "", errors.Wrap(err, "error invalid persisted password")
+	}
+
+	log(ctx).Debugf("password for %v retrieved from password file", configFile)
+
+	return string(s), nil
+}
+
+func (filePasswordStorage) PersistPassword(ctx context.Context, configFile, password string) error {
+	fn := passwordFileName(configFile)
+	log(ctx).Debugf("Saving password to file %v.", fn)
+
+	return ioutil.WriteFile(fn, []byte(base64.StdEncoding.EncodeToString([]byte(password))), 0o600)
+}
+
+func (filePasswordStorage) DeletePassword(ctx context.Context, configFile string) error {
+	err := os.Remove(passwordFileName(configFile))
+	if err != nil && !os.IsNotExist(err) {
+		return errors.Wrap(err, "error deleting password file")
+	}
+
+	return nil
+}
+
+func passwordFileName(configFile string) string {
+	return configFile + ".kopia-password"
+}
diff --git a/internal/passwordpersist/passwordpersist_keyring.go b/internal/passwordpersist/passwordpersist_keyring.go
new file mode 100644
index 00000000..55d9ca71
--- /dev/null
+++ b/internal/passwordpersist/passwordpersist_keyring.go
@@ -0,0 +1,99 @@
+package passwordpersist
+
+import (
+	"context"
+	"crypto/sha256"
+	"fmt"
+	"io"
+	"os/user"
+	"path/filepath"
+	"runtime"
+	"strings"
+
+	"github.com/pkg/errors"
+	"github.com/zalando/go-keyring"
+)
+
+// Keyring is a Strategy that persists the password in OS-specific keyring.
+var Keyring Strategy = keyringStrategy{}
+
+type keyringStrategy struct{}
+
+func (keyringStrategy) GetPassword(ctx context.Context, configFile string) (string, error) {
+	kr, err := keyring.Get(getKeyringItemID(configFile), keyringUsername(ctx))
+
+	switch {
+	case err == nil:
+		log(ctx).Debugf("password for %v retrieved from OS keyring", configFile)
+		return kr, nil
+	case errors.Is(err, keyring.ErrNotFound):
+		return "", ErrPasswordNotFound
+	case errors.Is(err, keyring.ErrUnsupportedPlatform):
+		return "", ErrPasswordNotFound
+	default:
+		return "", errors.Wrap(err, "error retrieving password from OS keyring")
+	}
+}
+
+func (keyringStrategy) PersistPassword(ctx context.Context, configFile, password string) error {
+	log(ctx).Debugf("saving password to OS keyring...")
+
+	err := keyring.Set(getKeyringItemID(configFile), keyringUsername(ctx), password)
+
+	switch {
+	case err == nil:
+		log(ctx).Debugf("Saved password in OS keyring")
+		return nil
+
+	case errors.Is(err, keyring.ErrUnsupportedPlatform):
+		return ErrUnsupported
+
+	default:
+		return errors.Wrap(err, "error saving password in OS keyring")
+	}
+}
+
+func (keyringStrategy) DeletePassword(ctx context.Context, configFile string) error {
+	err := keyring.Delete(getKeyringItemID(configFile), keyringUsername(ctx))
+
+	switch {
+	case err == nil:
+		log(ctx).Infof("deleted repository password for %v.", configFile)
+		return nil
+
+	case errors.Is(err, keyring.ErrUnsupportedPlatform):
+		return ErrUnsupported
+
+	case errors.Is(err, keyring.ErrNotFound):
+		return ErrPasswordNotFound
+
+	default:
+		return errors.Wrapf(err, "unable to delete keyring item %v", getKeyringItemID(configFile))
+	}
+}
+
+func getKeyringItemID(configFile string) string {
+	h := sha256.New()
+	io.WriteString(h, configFile) //nolint:errcheck
+
+	return fmt.Sprintf("%v-%x", filepath.Base(configFile), h.Sum(nil)[0:8])
+}
+
+func keyringUsername(ctx context.Context) string {
+	currentUser, err := user.Current()
+	if err != nil {
+		log(ctx).Errorf("Cannot determine keyring username: %s", err)
+		return "nobody"
+	}
+
+	u := currentUser.Username
+
+	if runtime.GOOS == "windows" {
+		if p := strings.Index(u, "\\"); p >= 0 {
+			// On Windows ignore domain name.
+			u = u[p+1:]
+		}
+	}
+
+	return u
+}
diff --git a/internal/passwordpersist/passwordpersist_multiple.go b/internal/passwordpersist/passwordpersist_multiple.go
new file mode 100644
index 00000000..575a7406
--- /dev/null
+++ b/internal/passwordpersist/passwordpersist_multiple.go
@@ -0,0 +1,67 @@
+package passwordpersist
+
+import (
+	"context"
+
+	"github.com/pkg/errors"
+)
+
+var _ Strategy = (Multiple{})
+
+// Multiple is a Strategy that tries several underlying persistence strategies.
+type Multiple []Strategy
+
+// GetPassword retrieves the password form the first password storage that has it.
+func (m Multiple) GetPassword(ctx context.Context, configFile string) (string, error) {
+	for _, s := range m {
+		pass, err := s.GetPassword(ctx, configFile)
+		if err == nil {
+			return pass, nil
+		}
+
+		if errors.Is(err, ErrPasswordNotFound) {
+			// try next strategy one.
+			continue
+		}
+
+		return "", errors.Wrap(err, "error getting persistent password")
+	}
+
+	return "", ErrPasswordNotFound
+}
+
+// PersistPassword persists the provided password using the first method that succeeds.
+func (m Multiple) PersistPassword(ctx context.Context, configFile, password string) error {
+	for _, s := range m {
+		err := s.PersistPassword(ctx, configFile, password)
+
+		if err == nil {
+			return nil
+		}
+
+		if errors.Is(err, ErrUnsupported) {
+			continue
+		}
+
+		return errors.Wrap(err, "error persisting password")
+	}
+
+	return ErrUnsupported
+}
+
+// DeletePassword deletes the password from all persistent storages.
+func (m Multiple) DeletePassword(ctx context.Context, configFile string) error {
+	for _, s := range m {
+		err := s.DeletePassword(ctx, configFile)
+
+		switch {
+		case err == nil: // good
+		case errors.Is(err, ErrPasswordNotFound): // ignore
+		case errors.Is(err, ErrUnsupported): // ignore
+		default:
+			return errors.Wrap(err, "error removing password from persistent storage")
+		}
+	}
+
+	return nil
+}
diff --git a/internal/passwordpersist/passwordpersist_none.go b/internal/passwordpersist/passwordpersist_none.go
new file mode 100644
index 00000000..47c8a3b1
--- /dev/null
+++ b/internal/passwordpersist/passwordpersist_none.go
@@ -0,0 +1,21 @@
+package passwordpersist
+
+import "context"
+
+// None is a strategy that does not persist the password at all.
+var None Strategy = noneStrategy{}
+
+type noneStrategy struct{}
+
+func (noneStrategy) GetPassword(ctx context.Context, configFile string) (string, error) {
+	return "", ErrPasswordNotFound
+}
+
+func (noneStrategy) PersistPassword(ctx context.Context, configFile, password string) error {
+	// silently succeed
+	return nil
+}
+
+func (noneStrategy) DeletePassword(ctx context.Context, configFile string) error {
+	return nil
+}
diff --git a/internal/server/api_repo.go b/internal/server/api_repo.go
index d5fa6dfd..4614d137 100644
--- a/internal/server/api_repo.go
+++ b/internal/server/api_repo.go
@@ -8,6 +8,7 @@
 
 	"github.com/pkg/errors"
 
+	"github.com/kopia/kopia/internal/passwordpersist"
 	"github.com/kopia/kopia/internal/remoterepoapi"
 	"github.com/kopia/kopia/internal/serverapi"
 	"github.com/kopia/kopia/repo"
@@ -243,7 +244,9 @@ func (s *Server) getConnectOptions(cliOpts repo.ClientOptions) *repo.ConnectOpti
 }
 
 func (s *Server) connectAPIServerAndOpen(ctx context.Context, si *repo.APIServerInfo, password string, cliOpts repo.ClientOptions) *apiError {
-	if err := repo.ConnectAPIServer(ctx, s.options.ConfigFile, si, password, s.getConnectOptions(cliOpts)); err != nil {
+	if err := passwordpersist.OnSuccess(
+		ctx, repo.ConnectAPIServer(ctx, s.options.ConfigFile, si, password, s.getConnectOptions(cliOpts)),
+		s.options.PasswordPersist, s.options.ConfigFile, password); err != nil {
 		return repoErrorToAPIError(err)
 	}
 
@@ -257,7 +260,9 @@ func (s *Server) connectAndOpen(ctx context.Context, conn blob.ConnectionInfo, p
 	}
 	defer st.Close(ctx) //nolint:errcheck
 
-	if err = repo.Connect(ctx, s.options.ConfigFile, st, password, s.getConnectOptions(cliOpts)); err != nil {
+	if err = passwordpersist.OnSuccess(
+		ctx, repo.Connect(ctx, s.options.ConfigFile, st, password, s.getConnectOptions(cliOpts)),
+		s.options.PasswordPersist, s.options.ConfigFile, password); err != nil {
 		return repoErrorToAPIError(err)
 	}
 
@@ -297,6 +302,10 @@ func (s *Server) handleRepoDisconnect(ctx context.Context, r *http.Request, body
 		return nil, internalServerError(err)
 	}
 
+	if err := s.options.PasswordPersist.DeletePassword(ctx, s.options.ConfigFile); err != nil {
+		return nil, internalServerError(err)
+	}
+
 	return &serverapi.Empty{}, nil
 }
 
diff --git a/internal/server/server.go b/internal/server/server.go
index 447d69c9..8293a9a5 100644
--- a/internal/server/server.go
+++ b/internal/server/server.go
@@ -17,6 +17,7 @@
 
 	"github.com/kopia/kopia/internal/auth"
 	"github.com/kopia/kopia/internal/clock"
+	"github.com/kopia/kopia/internal/passwordpersist"
 	"github.com/kopia/kopia/internal/serverapi"
 	"github.com/kopia/kopia/internal/uitask"
 	"github.com/kopia/kopia/repo"
@@ -610,6 +611,7 @@ type Options struct {
 	MaxConcurrency       int
 	Authenticator        auth.Authenticator
 	Authorizer           auth.Authorizer
+	PasswordPersist      passwordpersist.Strategy
 	AuthCookieSigningKey string
 	UIUser               string // name of the user allowed to access the UI
 }
@@ -621,6 +623,10 @@ func New(ctx context.Context, options Options) (*Server, error) {
 		return nil, errors.Errorf("missing authorizer")
 	}
 
+	if options.PasswordPersist == nil {
+		return nil, errors.Errorf("missing password persistence")
+	}
+
 	if options.AuthCookieSigningKey == "" {
 		// generate random signing key
 		options.AuthCookieSigningKey = uuid.New().String()
diff --git a/repo/api_server_repository.go b/repo/api_server_repository.go
index deba3d3d..93a2590a 100644
--- a/repo/api_server_repository.go
+++ b/repo/api_server_repository.go
@@ -289,5 +289,5 @@ func ConnectAPIServer(ctx context.Context, configFile string, si *APIServerInfo,
 		return errors.Wrap(err, "unable to write config file")
 	}
 
-	return verifyConnect(ctx, configFile, password, opt.PersistCredentials)
+	return verifyConnect(ctx, configFile, password)
 }
diff --git a/repo/connect.go b/repo/connect.go
index 789dcb6e..c066564d 100644
--- a/repo/connect.go
+++ b/repo/connect.go
@@ -13,7 +13,6 @@
 
 // ConnectOptions specifies options when persisting configuration to connect to a repository.
 type ConnectOptions struct {
-	PersistCredentials bool `json:"persistCredentials"`
 	ClientOptions
 
 	content.CachingOptions
@@ -58,10 +57,10 @@ func Connect(ctx context.Context, configFile string, st blob.Storage, password s
 		return errors.Wrap(err, "unable to write config file")
 	}
 
-	return verifyConnect(ctx, configFile, password, opt.PersistCredentials)
+	return verifyConnect(ctx, configFile, password)
 }
 
-func verifyConnect(ctx context.Context, configFile, password string, persist bool) error {
+func verifyConnect(ctx context.Context, configFile, password string) error {
 	// now verify that the repository can be opened with the provided config file.
 	r, err := Open(ctx, configFile, password, nil)
 	if err != nil {
@@ -74,14 +73,6 @@ func verifyConnect(ctx context.Context, configFile, password string, persist boo
 		return err
 	}
 
-	if persist {
-		if err := persistPassword(ctx, configFile, password); err != nil {
-			return errors.Wrap(err, "unable to persist password")
-		}
-	} else {
-		deletePassword(ctx, configFile)
-	}
-
 	return r.Close(ctx)
 }
 
@@ -92,8 +83,6 @@ func Disconnect(ctx context.Context, configFile string) error {
 		return err
 	}
 
-	deletePassword(ctx, configFile)
-
 	if cfg.Caching != nil && cfg.Caching.CacheDirectory != "" {
 		if !filepath.IsAbs(cfg.Caching.CacheDirectory) {
 			return errors.Errorf("cache directory was not absolute, refusing to delete")
diff --git a/repo/password.go b/repo/password.go
deleted file mode 100644
index 59b688ff..00000000
--- a/repo/password.go
+++ /dev/null
@@ -1,110 +0,0 @@
-package repo
-
-import (
-	"context"
-	"crypto/sha256"
-	"encoding/base64"
-	"fmt"
-	"io"
-	"io/ioutil"
-	"os"
-	"os/user"
-	"path/filepath"
-	"runtime"
-	"strings"
-
-	"github.com/pkg/errors"
-	"github.com/zalando/go-keyring"
-)
-
-// KeyRingEnabled enables password persistence uses OS-specific keyring.
-var KeyRingEnabled = false
-
-// GetPersistedPassword retrieves persisted password for a given repository config.
-func GetPersistedPassword(ctx context.Context, configFile string) (string, bool) {
-	if KeyRingEnabled {
-		kr, err := keyring.Get(getKeyringItemID(configFile), keyringUsername(ctx))
-		if err == nil {
-			log(ctx).Debugf("password for %v retrieved from OS keyring", configFile)
-			return kr, true
-		}
-	}
-
-	b, err := ioutil.ReadFile(passwordFileName(configFile))
-	if err == nil {
-		s, err := base64.StdEncoding.DecodeString(string(b))
-		if err == nil {
-			log(ctx).Debugf("password for %v retrieved from password file", configFile)
-			return string(s), true
-		}
-	}
-
-	log(ctx).Debugf("could not find persisted password")
-
-	return "", false
-}
-
-// persistPassword stores password for a given repository config.
-func persistPassword(ctx context.Context, configFile, password string) error {
-	if KeyRingEnabled {
-		log(ctx).Debugf("saving password to OS keyring...")
-
-		err := keyring.Set(getKeyringItemID(configFile), keyringUsername(ctx), password)
-		if err == nil {
-			log(ctx).Debugf("Saved password in OS keyring")
-			return nil
-		}
-
-		return errors.Wrap(err, "error saving password in key ring")
-	}
-
-	fn := passwordFileName(configFile)
-	log(ctx).Debugf("Saving password to file %v.", fn)
-
-	return ioutil.WriteFile(fn, []byte(base64.StdEncoding.EncodeToString([]byte(password))), 0o600)
-}
-
-// deletePassword removes stored repository password.
-func deletePassword(ctx context.Context, configFile string) {
-	// delete from both keyring and a file
-	if KeyRingEnabled {
-		err := keyring.Delete(getKeyringItemID(configFile), keyringUsername(ctx))
-		if err == nil {
-			log(ctx).Infof("deleted repository password for %v.", configFile)
-		} else if !errors.Is(err, keyring.ErrNotFound) {
-			log(ctx).Errorf("unable to delete keyring item %v: %v", getKeyringItemID(configFile), err)
-		}
-	}
-
-	_ = os.Remove(passwordFileName(configFile))
-}
-
-func getKeyringItemID(configFile string) string {
-	h := sha256.New()
-	io.WriteString(h, configFile) //nolint:errcheck
-
-	return fmt.Sprintf("%v-%x", filepath.Base(configFile), h.Sum(nil)[0:8])
-}
-
-func passwordFileName(configFile string) string {
-	return configFile + ".kopia-password"
-}
-
-func keyringUsername(ctx context.Context) string {
-	currentUser, err := user.Current()
-	if err != nil {
-		log(ctx).Errorf("Cannot determine keyring username: %s", err)
-		return "nobody"
-	}
-
-	u := currentUser.Username
-
-	if runtime.GOOS == "windows" {
-		if p := strings.Index(u, "\\"); p >= 0 {
-			// On Windows ignore domain name.
-			u = u[p+1:]
-		}
-	}
-
-	return u
-}


Test patch:
diff --git a/internal/server/server_test.go b/internal/server/server_test.go
index 24a2d868..d83e1d30 100644
--- a/internal/server/server_test.go
+++ b/internal/server/server_test.go
@@ -16,6 +16,7 @@
 
 	"github.com/kopia/kopia/internal/apiclient"
 	"github.com/kopia/kopia/internal/auth"
+	"github.com/kopia/kopia/internal/passwordpersist"
 	"github.com/kopia/kopia/internal/repotesting"
 	"github.com/kopia/kopia/internal/server"
 	"github.com/kopia/kopia/internal/testlogging"
@@ -44,8 +45,9 @@ func startServer(ctx context.Context, t *testing.T) *repo.APIServerInfo {
 	_, env := repotesting.NewEnvironment(t)
 
 	s, err := server.New(ctx, server.Options{
-		ConfigFile: env.ConfigFile(),
-		Authorizer: auth.LegacyAuthorizer(),
+		ConfigFile:      env.ConfigFile(),
+		PasswordPersist: passwordpersist.File,
+		Authorizer:      auth.LegacyAuthorizer(),
 		Authenticator: auth.CombineAuthenticators(
 			auth.AuthenticateSingleUser(testUsername+"@"+testHostname, testPassword),
 			auth.AuthenticateSingleUser(testUIUsername, testUIPassword),
@@ -54,6 +56,8 @@ func startServer(ctx context.Context, t *testing.T) *repo.APIServerInfo {
 		UIUser:          testUIUsername,
 	})
 
+	require.NoError(t, err)
+
 	s.SetRepository(ctx, env.Repository)
 
 	// ensure we disconnect the repository before shutting down the server.