From b94d7eebdc0bae767417b937674a03545fe0e2ca Mon Sep 17 00:00:00 2001
From: Ben McClelland <ben.mcclelland@versity.com>
Date: Wed, 12 Jun 2024 16:17:26 -0700
Subject: [PATCH] fix: internal iam racing account updates causing
 inconsistencies

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.
---
 auth/iam_internal.go  | 20 ++++++++++++++++++++
 auth/iam_s3_object.go | 21 +++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/auth/iam_internal.go b/auth/iam_internal.go
index 73e7637a..6588b642 100644
--- a/auth/iam_internal.go
+++ b/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)
diff --git a/auth/iam_s3_object.go b/auth/iam_s3_object.go
index f916cb1d..0b7637c7 100644
--- a/auth/iam_s3_object.go
+++ b/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