Merge pull request #628 from versity/ben/iam_internal_race

fix: internal iam racing account updates causing inconsistencies

auth/iam_internal.go

@@ -22,6 +22,7 @@ import (
 	"os"
 	"path/filepath"
 	"sort"
+	"sync"
 	"time"
 )
 
@@ -32,6 +33,13 @@ const (
 
 // IAMServiceInternal manages the internal IAM service
 type IAMServiceInternal struct {
+	// This mutex will help with racing updates to the IAM data
+	// from multiple requests to this gateway instance, but
+	// will not help with racing updates to multiple load balanced
+	// gateway instances. This is a limitation of the internal
+	// IAM service. All account updates should be sent to a single
+	// gateway instance if possible.
+	sync.RWMutex
 	dir string
 }
 
@@ -62,6 +70,9 @@ func NewInternal(dir string) (*IAMServiceInternal, error) {
 // CreateAccount creates a new IAM account. Returns an error if the account
 // already exists.
 func (s *IAMServiceInternal) CreateAccount(account Account) error {
+	s.Lock()
+	defer s.Unlock()
+
 	return s.storeIAM(func(data []byte) ([]byte, error) {
 		conf, err := parseIAM(data)
 		if err != nil {
@@ -86,6 +97,9 @@ func (s *IAMServiceInternal) CreateAccount(account Account) error {
 // GetUserAccount retrieves account info for the requested user. Returns
 // ErrNoSuchUser if the account does not exist.
 func (s *IAMServiceInternal) GetUserAccount(access string) (Account, error) {
+	s.RLock()
+	defer s.RUnlock()
+
 	conf, err := s.getIAM()
 	if err != nil {
 		return Account{}, fmt.Errorf("get iam data: %w", err)
@@ -102,6 +116,9 @@ func (s *IAMServiceInternal) GetUserAccount(access string) (Account, error) {
 // DeleteUserAccount deletes the specified user account. Does not check if
 // account exists.
 func (s *IAMServiceInternal) DeleteUserAccount(access string) error {
+	s.Lock()
+	defer s.Unlock()
+
 	return s.storeIAM(func(data []byte) ([]byte, error) {
 		conf, err := parseIAM(data)
 		if err != nil {
@@ -121,6 +138,9 @@ func (s *IAMServiceInternal) DeleteUserAccount(access string) error {
 
 // ListUserAccounts lists all the user accounts stored.
 func (s *IAMServiceInternal) ListUserAccounts() ([]Account, error) {
+	s.RLock()
+	defer s.RUnlock()
+
 	conf, err := s.getIAM()
 	if err != nil {
 		return []Account{}, fmt.Errorf("get iam data: %w", err)

auth/iam_s3_object.go

@@ -24,6 +24,7 @@ import (
 	"io"
 	"net/http"
 	"sort"
+	"sync"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/config"
@@ -41,6 +42,14 @@ import (
 // coming from iAMConfig and iamFile in iam_internal.
 
 type IAMServiceS3 struct {
+	// This mutex will help with racing updates to the IAM data
+	// from multiple requests to this gateway instance, but
+	// will not help with racing updates to multiple load balanced
+	// gateway instances. This is a limitation of the internal
+	// IAM service. All account updates should be sent to a single
+	// gateway instance if possible.
+	sync.RWMutex
+
 	access        string
 	secret        string
 	region        string
@@ -97,6 +106,9 @@ func NewS3(access, secret, region, bucket, endpoint string, sslSkipVerify, debug
 }
 
 func (s *IAMServiceS3) CreateAccount(account Account) error {
+	s.Lock()
+	defer s.Unlock()
+
 	conf, err := s.getAccounts()
 	if err != nil {
 		return err
@@ -112,6 +124,9 @@ func (s *IAMServiceS3) CreateAccount(account Account) error {
 }
 
 func (s *IAMServiceS3) GetUserAccount(access string) (Account, error) {
+	s.RLock()
+	defer s.RUnlock()
+
 	conf, err := s.getAccounts()
 	if err != nil {
 		return Account{}, err
@@ -126,6 +141,9 @@ func (s *IAMServiceS3) GetUserAccount(access string) (Account, error) {
 }
 
 func (s *IAMServiceS3) DeleteUserAccount(access string) error {
+	s.Lock()
+	defer s.Unlock()
+
 	conf, err := s.getAccounts()
 	if err != nil {
 		return err
@@ -141,6 +159,9 @@ func (s *IAMServiceS3) DeleteUserAccount(access string) error {
 }
 
 func (s *IAMServiceS3) ListUserAccounts() ([]Account, error) {
+	s.RLock()
+	defer s.RUnlock()
+
 	conf, err := s.getAccounts()
 	if err != nil {
 		return nil, err