Skip to content

Commit

Permalink
add locking around storage access
Browse files Browse the repository at this point in the history
Refs: #1084
  • Loading branch information
synfinatic committed Nov 30, 2024
1 parent ffec8ea commit 43b20d4
Show file tree
Hide file tree
Showing 6 changed files with 173 additions and 10 deletions.
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
62 changes: 62 additions & 0 deletions internal/storage/flock.go
Original file line number Diff line number Diff line change
@@ -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
}
45 changes: 45 additions & 0 deletions internal/storage/flock_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
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"
"os"
"path"
"testing"

"github.com/stretchr/testify/assert"
"github.com/synfinatic/aws-sso-cli/internal/config"
)

func TestFlockFile(t *testing.T) {
cfgDir := config.ConfigDir(false)
assert.Equal(t, fmt.Sprintf("%s/storage.lock", cfgDir), FlockFile(false))

d, err := os.MkdirTemp("", "test-flockfile")
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")
assert.Equal(t, fmt.Sprintf("%s/storage.lock", d), FlockFile(false))
}

func TestFlockBlocker(t *testing.T) {
FlockBlockerReset()
assert.NoError(t, FlockBlocker())
}
56 changes: 49 additions & 7 deletions internal/storage/keyring.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}

Expand All @@ -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())
Expand Down Expand Up @@ -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
}

Expand All @@ -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...)
Expand All @@ -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
}

Expand Down
14 changes: 11 additions & 3 deletions internal/storage/keyring_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand All @@ -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)

Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 43b20d4

Please sign in to comment.