Skip to content

Commit

Permalink
fix: internal iam racing account updates causing inconsistencies
Browse files Browse the repository at this point in the history
Add a mutex to prevent reacing accounts updates from multiple
simultaneous account update requests.

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.
  • Loading branch information
benmcclelland committed Jun 12, 2024
1 parent 1b922ca commit b94d7ee
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 0 deletions.
20 changes: 20 additions & 0 deletions auth/iam_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"os"
"path/filepath"
"sort"
"sync"
"time"
)

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

Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand All @@ -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 {
Expand All @@ -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)
Expand Down
21 changes: 21 additions & 0 deletions auth/iam_s3_object.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit b94d7ee

Please sign in to comment.