add locking around storage access

Refs: #1084
synfinatic · Nov 30, 2024 · 7cc244c · 7cc244c
1 parent ffec8ea
commit 7cc244c
Show file tree

Hide file tree

Showing 5 changed files with 128 additions and 10 deletions.
diff --git a/go.mod b/go.mod
@@ -74,8 +74,10 @@ require (
 	github.com/aws/aws-sdk-go-v2/service/sso v1.22.1
 	github.com/aws/aws-sdk-go-v2/service/ssooidc v1.26.2
 	github.com/aws/aws-sdk-go-v2/service/sts v1.30.1
+	github.com/danjacques/gofslock v0.0.0-20240212154529-d899e02bfe22
 	github.com/docker/docker v27.2.1+incompatible
 	github.com/docker/go-connections v0.5.0
+	github.com/jpillora/backoff v1.0.0
 	golang.org/x/net v0.31.0
 )
 

diff --git a/go.sum b/go.sum
@@ -99,6 +99,8 @@ github.com/coreos/go-semver v0.3.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3Ee
 github.com/coreos/go-systemd/v22 v22.3.2/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc=
 github.com/danieljoos/wincred v1.1.2 h1:QLdCxFs1/Yl4zduvBdcHB8goaYk9RARS2SgLLRuAyr0=
 github.com/danieljoos/wincred v1.1.2/go.mod h1:GijpziifJoIBfYh+S7BbkdUTU4LfM+QnGqR5Vl2tAx0=
+github.com/danjacques/gofslock v0.0.0-20240212154529-d899e02bfe22 h1:m+Fkk9QEMuV6Z1ithqqYogOHV7Pl6rMKe34NBTJTS/c=
+github.com/danjacques/gofslock v0.0.0-20240212154529-d899e02bfe22/go.mod h1:jXqs4TJbb7Xtl0FwUgBaOXty8edb/61H37U4D9E5EQE=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -246,6 +248,7 @@ github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHW
 github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U=
 github.com/joho/godotenv v1.3.0 h1:Zjp+RcGpHhGlrMbJzXTrZZPrWj+1vfm90La1wgB6Bhc=
 github.com/joho/godotenv v1.3.0/go.mod h1:7hK45KPybAkOC6peb+G5yklZfMxEjkZhHbwpqxOKXbg=
+github.com/jpillora/backoff v1.0.0 h1:uvFg412JmmHBHw7iwprIxkPMI+sGQ4kzOWsMeHnm2EA=
 github.com/jpillora/backoff v1.0.0/go.mod h1:J/6gKK9jxlEcS3zixgDgUAsiuZ7yrSoa/FX5e0EB2j4=
 github.com/json-iterator/go v1.1.6/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU=
 github.com/json-iterator/go v1.1.10/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4=
@@ -516,6 +519,7 @@ golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.0.0-20210603081109-ebe580a85c40/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20210819135213-f52c844e1c1c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.27.0 h1:wBqf8DvsY9Y/2P8gAfPDEYNuS30J4lPHJxXSb/nJZ+s=
 golang.org/x/sys v0.27.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=

diff --git a/internal/storage/flock.go b/internal/storage/flock.go
@@ -0,0 +1,62 @@
+package storage
+
+/*
+ * AWS SSO CLI
+ * Copyright (c) 2021-2024 Aaron Turner  <synfinatic at gmail dot com>
+ *
+ * This program is free software: you can redistribute it
+ * and/or modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or with the authors permission any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import (
+	"fmt"
+	"strings"
+	"time"
+
+	"github.com/jpillora/backoff"
+	"github.com/synfinatic/aws-sso-cli/internal/config"
+)
+
+const (
+	FLOCK_FILE = "%s/storage.lock"
+)
+
+func FlockFile(expand bool) string {
+	if strings.HasPrefix(flockFile, "%s") {
+		return fmt.Sprintf(flockFile, config.ConfigDir(expand))
+	} else {
+		return flockFile
+	}
+}
+
+var sleeper = &backoff.Backoff{}
+var flockFile string = FLOCK_FILE
+
+func init() {
+	sleeper = &backoff.Backoff{
+		Min:    10 * time.Millisecond,
+		Max:    1 * time.Second,
+		Factor: 2,
+		Jitter: true,
+	}
+}
+
+func FlockBlockerReset() {
+	sleeper.Reset()
+}
+
+// Implments fslock.Blocker
+func FlockBlocker() error {
+	time.Sleep(sleeper.Duration())
+	return nil
+}
diff --git a/internal/storage/keyring.go b/internal/storage/keyring.go
@@ -28,6 +28,8 @@ import (
 
 	"github.com/99designs/keyring"
 	// "github.com/davecgh/go-spew/spew"
+	"github.com/danjacques/gofslock/fslock"
+
 	"github.com/synfinatic/aws-sso-cli/internal/utils"
 	"golang.org/x/term"
 )
@@ -171,7 +173,13 @@ func OpenKeyring(cfg *keyring.Config) (*KeyringStore, error) {
 		cache:   NewStorageData(),
 	}
 
-	if err = kr.getStorageData(&kr.cache); err != nil {
+	err = fslock.WithSharedBlocking(FlockFile(true), FlockBlocker,
+		func() error {
+			return kr.getStorageData(&kr.cache)
+		},
+	)
+	FlockBlockerReset()
+	if err != nil {
 		return nil, err
 	}
 
@@ -193,10 +201,22 @@ func (kr *KeyringStore) getStorageData(s *StorageData) error {
 
 	switch keyringGOOS {
 	case "windows":
-		data, err = kr.joinAndGetKeyringData(RECORD_KEY)
+		err = fslock.WithSharedBlocking(FlockFile(true), FlockBlocker,
+			func() error {
+				data, err = kr.joinAndGetKeyringData(RECORD_KEY)
+				return err
+			},
+		)
+
 	default:
-		data, err = kr.getKeyringData(RECORD_KEY)
+		err = fslock.WithSharedBlocking(FlockFile(true), FlockBlocker,
+			func() error {
+				data, err = kr.getKeyringData(RECORD_KEY)
+				return err
+			},
+		)
 	}
+	FlockBlockerReset()
 
 	if err != nil {
 		log.Warn("unable to load keyring data", "error", err.Error())
@@ -225,7 +245,14 @@ func (kr *KeyringStore) joinAndGetKeyringData(key string) ([]byte, error) {
 	var err error
 	var chunk []byte
 
-	if chunk, err = kr.getKeyringData(fmt.Sprintf("%s_%d", key, 0)); err != nil {
+	err = fslock.WithSharedBlocking(FlockFile(true), FlockBlocker,
+		func() error {
+			chunk, err = kr.getKeyringData(fmt.Sprintf("%s_%d", key, 0))
+			return err
+		},
+	)
+	FlockBlockerReset()
+	if err != nil {
 		return nil, err
 	}
 
@@ -238,7 +265,13 @@ func (kr *KeyringStore) joinAndGetKeyringData(key string) ([]byte, error) {
 
 	for i := 1; readBytes < totalBytes; i++ {
 		k := fmt.Sprintf("%s_%d", key, i)
-		if chunk, err = kr.getKeyringData(k); err != nil {
+		err = fslock.WithSharedBlocking(FlockFile(true), FlockBlocker,
+			func() error {
+				chunk, err = kr.getKeyringData(k)
+				return err
+			},
+		)
+		if err != nil {
 			return nil, fmt.Errorf("unable to fetch %s: %s", k, err.Error())
 		}
 		data = append(data, chunk...)
@@ -259,10 +292,19 @@ func (kr *KeyringStore) saveStorageData() error {
 
 	switch keyringGOOS {
 	case "windows":
-		err = kr.splitAndSetStorageData(jdata, RECORD_KEY, KEYRING_ID)
+		err = fslock.WithBlocking(FlockFile(true), FlockBlocker,
+			func() error {
+				return kr.splitAndSetStorageData(jdata, RECORD_KEY, KEYRING_ID)
+			},
+		)
 	default:
-		err = kr.setStorageData(jdata, RECORD_KEY, KEYRING_ID)
+		err = fslock.WithBlocking(FlockFile(true), FlockBlocker,
+			func() error {
+				return kr.setStorageData(jdata, RECORD_KEY, KEYRING_ID)
+			},
+		)
 	}
+	FlockBlockerReset()
 	return err
 }
 

diff --git a/internal/storage/keyring_test.go b/internal/storage/keyring_test.go
@@ -41,6 +41,9 @@ type KeyringSuite struct {
 func TestKeyringSuite(t *testing.T) {
 	d, err := os.MkdirTemp("", "test-keyring")
 	assert.NoError(t, err)
+	// need to set this here as we're not using the normal location during tests
+	flockFile = path.Join(d, "storage.lock")
+
 	defer func() {
 		os.RemoveAll(d)
 		os.Unsetenv(ENV_SSO_FILE_PASSWORD)
@@ -256,6 +259,7 @@ func (suite *KeyringSuite) TestJoinAndGetKeyringData() {
 func TestGetStorageData(t *testing.T) {
 	d, err := os.MkdirTemp("", "test-keyring")
 	assert.NoError(t, err)
+	flockFile = path.Join(d, "storage.lock")
 	defer os.RemoveAll(d)
 
 	os.Setenv(ENV_SSO_FILE_PASSWORD, "justapassword")
@@ -285,6 +289,7 @@ func (m *mockKeyringAPI) Remove(key string) error {
 func TestKeyringErrors(t *testing.T) {
 	d, err := os.MkdirTemp("", "test-keyring")
 	assert.NoError(t, err)
+	flockFile = path.Join(d, "storage.lock")
 	defer os.RemoveAll(d)
 
 	os.Setenv(ENV_SSO_FILE_PASSWORD, "justapassword")
@@ -417,15 +422,16 @@ func getPasswordErrorDifferentFunc(p string) (string, error) {
 func TestNewKeyringConfig(t *testing.T) {
 	d, err := os.MkdirTemp("", "test-keyring")
 	assert.NoError(t, err)
-
-	err = os.WriteFile(path.Join(d, "aws-sso-cli-records"), []byte("INVALID DATA"), 0600)
-	assert.NoError(t, err)
+	flockFile = path.Join(d, "storage.lock")
 
 	defer func() {
 		getPasswordFunc = fileKeyringPassword
 		os.RemoveAll(d)
 	}()
 
+	err = os.WriteFile(path.Join(d, "aws-sso-cli-records"), []byte("INVALID DATA"), 0600)
+	assert.NoError(t, err)
+
 	getPasswordFunc = getPasswordErrorFunc
 	_, err = NewKeyringConfig("file", d)
 	assert.Error(t, err)
@@ -448,6 +454,7 @@ func TestNewKeyringConfig(t *testing.T) {
 func TestKeyringSuiteFails(t *testing.T) {
 	d, err := os.MkdirTemp("", "test-keyring")
 	assert.NoError(t, err)
+	flockFile = path.Join(d, "storage.lock")
 	err = os.MkdirAll(path.Join(d, "secure"), 0755)
 	assert.NoError(t, err)
 
@@ -490,6 +497,7 @@ func TestKeyringSuiteFails(t *testing.T) {
 func TestSplitCredentials(t *testing.T) {
 	d, err := os.MkdirTemp("", "test-keyring")
 	assert.NoError(t, err)
+	flockFile = path.Join(d, "storage.lock")
 
 	// setup logger for testing
 	oldLogger := log.Copy()