diff --git a/backend/auth/acl.go b/backend/auth/acl.go index d9630336..c1dfc6e6 100644 --- a/backend/auth/acl.go +++ b/backend/auth/acl.go @@ -46,6 +46,10 @@ type AccessControlList struct { } func ParseACL(data []byte) (ACL, error) { + if len(data) == 0 { + return ACL{}, nil + } + var acl ACL if err := json.Unmarshal(data, &acl); err != nil { return acl, fmt.Errorf("parse acl: %w", err) @@ -76,7 +80,7 @@ func ParseACLOutput(data []byte) (GetBucketAclOutput, error) { }, nil } -func UpdateACL(input *s3.PutBucketAclInput, acl ACL, iam IAMConfig) error { +func UpdateACL(input *s3.PutBucketAclInput, acl ACL, iam IAMService) error { if acl.Owner != *input.AccessControlPolicy.Owner.ID { return s3err.GetAPIError(s3err.ErrAccessDenied) } @@ -141,12 +145,15 @@ func UpdateACL(input *s3.PutBucketAclInput, acl ACL, iam IAMConfig) error { return nil } -func checkIfAccountsExist(accs []string, iam IAMConfig) ([]string, error) { +func checkIfAccountsExist(accs []string, iam IAMService) ([]string, error) { result := []string{} for _, acc := range accs { - _, ok := iam.AccessAccounts[acc] - if !ok { + _, err := iam.GetUserAccount(acc) + if err != nil && err != ErrNoSuchUser { + return nil, fmt.Errorf("check user account: %w", err) + } + if err == nil { result = append(result, acc) } } diff --git a/backend/auth/iam.go b/backend/auth/iam.go index a6bf53ec..8e7c7582 100644 --- a/backend/auth/iam.go +++ b/backend/auth/iam.go @@ -16,187 +16,179 @@ package auth import ( "encoding/json" + "errors" "fmt" - "os" + "hash/crc32" "sync" - - "github.com/versity/versitygw/s3err" ) +// Account is an internal IAM account type Account struct { Secret string `json:"secret"` Role string `json:"role"` - Region string `json:"region"` } +// UpdateAcctFunc accepts the current data and returns the new data to be stored +type UpdateAcctFunc func([]byte) ([]byte, error) + +// Storer is the interface to manage the peristent IAM data for the internal +// IAM service +type Storer interface { + InitIAM() error + GetIAM() ([]byte, error) + StoreIAM(UpdateAcctFunc) error +} + +// IAMConfig stores all internal IAM accounts type IAMConfig struct { AccessAccounts map[string]Account `json:"accessAccounts"` } -type AccountsCache struct { - mu sync.Mutex - Accounts map[string]Account -} - -func (c *AccountsCache) getAccount(access string) *Account { - c.mu.Lock() - defer c.mu.Unlock() - - acc, ok := c.Accounts[access] - if !ok { - return nil - } - - return &acc -} - -func (c *AccountsCache) updateAccounts() error { - c.mu.Lock() - defer c.mu.Unlock() - - var data IAMConfig - - file, err := os.ReadFile("users.json") - if err != nil { - return fmt.Errorf("error reading config file: %w", err) - } - - if err := json.Unmarshal(file, &data); err != nil { - return fmt.Errorf("error parsing the data: %w", err) - } - - c.Accounts = data.AccessAccounts - - return nil -} - -func (c *AccountsCache) deleteAccount(access string) { - c.mu.Lock() - defer c.mu.Unlock() - delete(c.Accounts, access) -} - +// IAMService is the interface for all IAM service implementations type IAMService interface { - GetIAMConfig() (*IAMConfig, error) - CreateAccount(access string, account *Account) error - GetUserAccount(access string) *Account + CreateAccount(access string, account Account) error + GetUserAccount(access string) (Account, error) DeleteUserAccount(access string) error } -type IAMServiceUnsupported struct { - accCache *AccountsCache +// IAMServiceInternal manages the internal IAM service +type IAMServiceInternal struct { + storer Storer + + mu sync.RWMutex + accts IAMConfig + serial uint32 } -var _ IAMService = &IAMServiceUnsupported{} +var _ IAMService = &IAMServiceInternal{} -func InitIAM() (IAMService, error) { - _, err := os.ReadFile("users.json") +// NewInternal creates a new instance for the Internal IAM service +func NewInternal(s Storer) (*IAMServiceInternal, error) { + i := &IAMServiceInternal{ + storer: s, + } + + err := i.updateCache() if err != nil { - jsonData, err := json.MarshalIndent(IAMConfig{AccessAccounts: map[string]Account{}}, "", " ") + return nil, fmt.Errorf("refresh iam cache: %w", err) + } + + return i, nil +} + +// CreateAccount creates a new IAM account. Returns an error if the account +// already exists. +func (s *IAMServiceInternal) CreateAccount(access string, account Account) error { + s.mu.Lock() + defer s.mu.Unlock() + + return s.storer.StoreIAM(func(data []byte) ([]byte, error) { + var conf IAMConfig + + if len(data) > 0 { + if err := json.Unmarshal(data, &conf); err != nil { + return nil, fmt.Errorf("failed to parse iam: %w", err) + } + } else { + conf.AccessAccounts = make(map[string]Account) + } + + _, ok := conf.AccessAccounts[access] + if ok { + return nil, fmt.Errorf("account already exists") + } + conf.AccessAccounts[access] = account + + b, err := json.Marshal(s.accts) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to serialize iam: %w", err) } - if err := os.WriteFile("users.json", jsonData, 0644); err != nil { - return nil, err - } - } - return &IAMServiceUnsupported{accCache: &AccountsCache{Accounts: map[string]Account{}}}, nil + return b, nil + }) } -func (IAMServiceUnsupported) GetIAMConfig() (*IAMConfig, error) { - return nil, s3err.GetAPIError(s3err.ErrNotImplemented) -} +var ErrNoSuchUser = errors.New("user not found") -func GetIAMConfig() (*IAMConfig, error) { - var data IAMConfig +// 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.mu.RLock() + defer s.mu.RUnlock() - file, err := os.ReadFile("users.json") + data, err := s.storer.GetIAM() if err != nil { - return nil, fmt.Errorf("unable to read config file: %w", err) + return Account{}, fmt.Errorf("get iam data: %w", err) } - if err := json.Unmarshal(file, &data); err != nil { - return nil, err - } - - return &data, nil -} - -func (s IAMServiceUnsupported) CreateAccount(access string, account *Account) error { - var data IAMConfig - - file, err := os.ReadFile("users.json") - if err != nil { - return fmt.Errorf("unable to read config file: %w", err) - } - - if err := json.Unmarshal(file, &data); err != nil { - return err - } - - _, ok := data.AccessAccounts[access] - if ok { - return fmt.Errorf("user with the given access already exists") - } - - data.AccessAccounts[access] = *account - - updatedJSON, err := json.MarshalIndent(data, "", " ") - if err != nil { - return err - } - - if err := os.WriteFile("users.json", updatedJSON, 0644); err != nil { - return err - } - - return nil -} - -func (s IAMServiceUnsupported) GetUserAccount(access string) *Account { - acc := s.accCache.getAccount(access) - if acc == nil { - err := s.accCache.updateAccounts() + serial := crc32.ChecksumIEEE(data) + if serial != s.serial { + s.mu.RUnlock() + err := s.updateCache() + s.mu.RUnlock() if err != nil { - return nil + return Account{}, fmt.Errorf("refresh iam cache: %w", err) } - - return s.accCache.getAccount(access) } - return acc -} - -func (s IAMServiceUnsupported) DeleteUserAccount(access string) error { - var data IAMConfig - - file, err := os.ReadFile("users.json") - if err != nil { - return fmt.Errorf("unable to read config file: %w", err) - } - - if err := json.Unmarshal(file, &data); err != nil { - return fmt.Errorf("failed to parse the config file: %w", err) - } - - _, ok := data.AccessAccounts[access] + acct, ok := s.accts.AccessAccounts[access] if !ok { - return fmt.Errorf("invalid access for the user: user does not exist") + return Account{}, ErrNoSuchUser } - delete(data.AccessAccounts, access) + return acct, nil +} - updatedJSON, err := json.MarshalIndent(data, "", " ") +// updateCache must be called with no locks held +func (s *IAMServiceInternal) updateCache() error { + s.mu.Lock() + defer s.mu.Unlock() + + data, err := s.storer.GetIAM() if err != nil { - return fmt.Errorf("failed to parse the data: %w", err) + return fmt.Errorf("get iam data: %w", err) } - if err := os.WriteFile("users.json", updatedJSON, 0644); err != nil { - return fmt.Errorf("failed to saved the changes: %w", err) + serial := crc32.ChecksumIEEE(data) + + if len(data) > 0 { + if err := json.Unmarshal(data, &s.accts); err != nil { + return fmt.Errorf("failed to parse the config file: %w", err) + } + } else { + s.accts.AccessAccounts = make(map[string]Account) } - s.accCache.deleteAccount(access) + s.serial = serial return nil } + +// DeleteUserAccount deletes the specified user account. Does not check if +// account exists. +func (s *IAMServiceInternal) DeleteUserAccount(access string) error { + s.mu.Lock() + defer s.mu.Unlock() + + return s.storer.StoreIAM(func(data []byte) ([]byte, error) { + if len(data) == 0 { + // empty config, do nothing + return data, nil + } + + var conf IAMConfig + + if err := json.Unmarshal(data, &conf); err != nil { + return nil, fmt.Errorf("failed to parse iam: %w", err) + } + + delete(conf.AccessAccounts, access) + + b, err := json.Marshal(s.accts) + if err != nil { + return nil, fmt.Errorf("failed to serialize iam: %w", err) + } + + return b, nil + }) +} diff --git a/backend/posix/posix.go b/backend/posix/posix.go index b8ee76b1..49229eca 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -28,7 +28,9 @@ import ( "sort" "strconv" "strings" + "sync" "syscall" + "time" "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" @@ -41,19 +43,35 @@ import ( ) type Posix struct { + backend.BackendUnsupported + rootfd *os.File rootdir string - backend.BackendUnsupported + + mu sync.RWMutex + iamcache []byte + iamvalid bool + iamexpire time.Time } var _ backend.Backend = &Posix{} +var ( + cacheDuration = 5 * time.Minute +) + const ( metaTmpDir = ".sgwtmp" metaTmpMultipartDir = metaTmpDir + "/multipart" onameAttr = "user.objname" tagHdr = "X-Amz-Tagging" + contentTypeHdr = "content-type" + contentEncHdr = "content-encoding" emptyMD5 = "d41d8cd98f00b204e9800998ecf8427e" + iamFile = "users.json" + iamBackupFile = "users.json.backup" + aclkey = "user.acl" + etagkey = "user.etag" ) func New(rootdir string) (*Posix, error) { @@ -140,7 +158,7 @@ func (p *Posix) PutBucket(bucket string, owner string) error { return fmt.Errorf("marshal acl: %w", err) } - if err := xattr.Set(bucket, "user.acl", jsonACL); err != nil { + if err := xattr.Set(bucket, aclkey, jsonACL); err != nil { return fmt.Errorf("set acl: %w", err) } @@ -263,7 +281,7 @@ func (p *Posix) CompleteMultipartUpload(bucket, object, uploadID string, parts [ return nil, s3err.GetAPIError(s3err.ErrInvalidPart) } - b, err := xattr.Get(partPath, "user.etag") + b, err := xattr.Get(partPath, etagkey) etag := string(b) if err != nil { etag = "" @@ -319,7 +337,7 @@ func (p *Posix) CompleteMultipartUpload(bucket, object, uploadID string, parts [ // Calculate s3 compatible md5sum for complete multipart. s3MD5 := backend.GetMultipartMD5(parts) - err = xattr.Set(objname, "user.etag", []byte(s3MD5)) + err = xattr.Set(objname, etagkey, []byte(s3MD5)) if err != nil { // cleanup object if returning error os.Remove(objname) @@ -373,22 +391,22 @@ func loadUserMetaData(path string, m map[string]string) (contentType, contentEnc m[strings.TrimPrefix(e, "user.")] = string(b) } - b, err := xattr.Get(path, "user.content-type") + b, err := xattr.Get(path, "user."+contentTypeHdr) contentType = string(b) if err != nil { contentType = "" } if contentType != "" { - m["content-type"] = contentType + m[contentTypeHdr] = contentType } - b, err = xattr.Get(path, "user.content-encoding") + b, err = xattr.Get(path, "user."+contentEncHdr) contentEncoding = string(b) if err != nil { contentEncoding = "" } if contentEncoding != "" { - m["content-encoding"] = contentEncoding + m[contentEncHdr] = contentEncoding } return @@ -626,7 +644,7 @@ func (p *Posix) ListObjectParts(bucket, object, uploadID string, partNumberMarke } partPath := filepath.Join(objdir, uploadID, e.Name()) - b, err := xattr.Get(partPath, "user.etag") + b, err := xattr.Get(partPath, etagkey) etag := string(b) if err != nil { etag = "" @@ -713,7 +731,7 @@ func (p *Posix) PutObjectPart(bucket, object, uploadID string, part int, length dataSum := hash.Sum(nil) etag := hex.EncodeToString(dataSum) - xattr.Set(partPath, "user.etag", []byte(etag)) + xattr.Set(partPath, etagkey, []byte(etag)) return etag, nil } @@ -741,7 +759,7 @@ func (p *Posix) PutObject(po *s3.PutObjectInput) (string, error) { } // set etag attribute to signify this dir was specifically put - xattr.Set(name, "user.etag", []byte(emptyMD5)) + xattr.Set(name, etagkey, []byte(emptyMD5)) return emptyMD5, nil } @@ -779,7 +797,7 @@ func (p *Posix) PutObject(po *s3.PutObjectInput) (string, error) { dataSum := hash.Sum(nil) etag := hex.EncodeToString(dataSum[:]) - xattr.Set(name, "user.etag", []byte(etag)) + xattr.Set(name, etagkey, []byte(etag)) return etag, nil } @@ -816,11 +834,15 @@ func (p *Posix) removeParents(bucket, object string) error { parent := filepath.Dir(objPath) if filepath.Base(parent) == bucket { + // stop removing parents if we hit the bucket directory. break } - _, err := xattr.Get(parent, "user.etag") + _, err := xattr.Get(parent, etagkey) if err == nil { + // a directory with a valid etag means this was specifically + // uploaded with a put object, so stop here and leave this + // directory in place. break } @@ -893,7 +915,7 @@ func (p *Posix) GetObject(bucket, object, acceptRange string, writer io.Writer) contentType, contentEncoding := loadUserMetaData(objPath, userMetaData) - b, err := xattr.Get(objPath, "user.etag") + b, err := xattr.Get(objPath, etagkey) etag := string(b) if err != nil { etag = "" @@ -937,7 +959,7 @@ func (p *Posix) HeadObject(bucket, object string) (*s3.HeadObjectOutput, error) userMetaData := make(map[string]string) contentType, contentEncoding := loadUserMetaData(objPath, userMetaData) - b, err := xattr.Get(objPath, "user.etag") + b, err := xattr.Get(objPath, etagkey) etag := string(b) if err != nil { etag = "" @@ -1010,7 +1032,7 @@ func (p *Posix) ListObjects(bucket, prefix, marker, delim string, maxkeys int) ( fileSystem := os.DirFS(bucket) results, err := backend.Walk(fileSystem, prefix, delim, marker, maxkeys, func(path string) (bool, error) { - _, err := xattr.Get(filepath.Join(bucket, path), "user.etag") + _, err := xattr.Get(filepath.Join(bucket, path), etagkey) if isNoAttr(err) { return false, nil } @@ -1019,7 +1041,7 @@ func (p *Posix) ListObjects(bucket, prefix, marker, delim string, maxkeys int) ( } return true, nil }, func(path string) (string, error) { - etag, err := xattr.Get(filepath.Join(bucket, path), "user.etag") + etag, err := xattr.Get(filepath.Join(bucket, path), etagkey) return string(etag), err }, []string{metaTmpDir}) if err != nil { @@ -1051,7 +1073,7 @@ func (p *Posix) ListObjectsV2(bucket, prefix, marker, delim string, maxkeys int) fileSystem := os.DirFS(bucket) results, err := backend.Walk(fileSystem, prefix, delim, marker, maxkeys, func(path string) (bool, error) { - _, err := xattr.Get(filepath.Join(bucket, path), "user.etag") + _, err := xattr.Get(filepath.Join(bucket, path), etagkey) if isNoAttr(err) { return false, nil } @@ -1060,7 +1082,7 @@ func (p *Posix) ListObjectsV2(bucket, prefix, marker, delim string, maxkeys int) } return true, nil }, func(path string) (string, error) { - etag, err := xattr.Get(filepath.Join(bucket, path), "user.etag") + etag, err := xattr.Get(filepath.Join(bucket, path), etagkey) return string(etag), err }, []string{metaTmpDir}) if err != nil { @@ -1089,7 +1111,7 @@ func (p *Posix) PutBucketAcl(bucket string, data []byte) error { return fmt.Errorf("stat bucket: %w", err) } - if err := xattr.Set(bucket, "user.acl", data); err != nil { + if err := xattr.Set(bucket, aclkey, data); err != nil { return fmt.Errorf("set acl: %w", err) } @@ -1105,7 +1127,10 @@ func (p *Posix) GetBucketAcl(bucket string) ([]byte, error) { return nil, fmt.Errorf("stat bucket: %w", err) } - b, err := xattr.Get(bucket, "user.acl") + b, err := xattr.Get(bucket, aclkey) + if isNoAttr(err) { + return []byte{}, nil + } if err != nil { return nil, fmt.Errorf("get acl: %w", err) } @@ -1185,6 +1210,198 @@ func (p *Posix) RemoveTags(bucket, object string) error { return p.SetTags(bucket, object, nil) } +const ( + iamMode = 0600 +) + +func (p *Posix) InitIAM() error { + p.mu.RLock() + defer p.mu.RUnlock() + + _, err := os.ReadFile(iamFile) + if errors.Is(err, fs.ErrNotExist) { + b, err := json.Marshal(auth.IAMConfig{}) + if err != nil { + return fmt.Errorf("marshal default iam: %w", err) + } + err = os.WriteFile(iamFile, b, iamMode) + if err != nil { + return fmt.Errorf("write default iam: %w", err) + } + } + + return nil +} + +func (p *Posix) GetIAM() ([]byte, error) { + p.mu.RLock() + defer p.mu.RUnlock() + + if !p.iamvalid || !p.iamexpire.After(time.Now()) { + p.mu.RUnlock() + err := p.refreshIAM() + p.mu.RLock() + if err != nil { + return nil, err + } + } + + return p.iamcache, nil +} + +const ( + backoff = 100 * time.Millisecond + maxretry = 300 +) + +func (p *Posix) refreshIAM() error { + p.mu.Lock() + defer p.mu.Unlock() + + // We are going to be racing with other running gateways without any + // coordination. So we might find the file does not exist at times. + // For this case we need to retry for a while assuming the other gateway + // will eventually write the file. If it doesn't after the max retries, + // then we will return the error. + + retries := 0 + + for { + b, err := os.ReadFile(iamFile) + if errors.Is(err, fs.ErrNotExist) { + // racing with someone else updating + // keep retrying after backoff + retries++ + if retries < maxretry { + time.Sleep(backoff) + continue + } + return fmt.Errorf("read iam file: %w", err) + } + if err != nil { + return err + } + + p.iamcache = b + p.iamvalid = true + p.iamexpire = time.Now().Add(cacheDuration) + break + } + + return nil +} + +func (p *Posix) StoreIAM(update auth.UpdateAcctFunc) error { + p.mu.Lock() + defer p.mu.Unlock() + + // We are going to be racing with other running gateways without any + // coordination. So the strategy here is to read the current file data. + // If the file doesn't exist, then we assume someone else is currently + // updating the file. So we just need to keep retrying. We also need + // to make sure the data is consistent within a single update. So racing + // writes to a file would possibly leave this in some invalid state. + // We can get atomic updates with rename. If we read the data, update + // the data, write to a temp file, then rename the tempfile back to the + // data file. This should always result in a complete data image. + + // There is at least one unsolved failure mode here. + // If a gateway removes the data file and then crashes, all other + // gateways will retry forever thinking that the original will eventually + // write the file. + + retries := 0 + + for { + b, err := os.ReadFile(iamFile) + if errors.Is(err, fs.ErrNotExist) { + // racing with someone else updating + // keep retrying after backoff + retries++ + if retries < maxretry { + time.Sleep(backoff) + continue + } + + // we have been unsuccessful trying to read the iam file + // so this must be the case where something happened and + // the file did not get updated successfully, and probably + // isn't going to be. The recovery procedure would be to + // copy the backup file into place of the original. + return fmt.Errorf("no iam file, needs backup recovery") + } + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("read iam file: %w", err) + } + + // reset retries on successful read + retries = 0 + + err = os.Remove(iamFile) + if errors.Is(err, fs.ErrNotExist) { + // racing with someone else updating + // keep retrying after backoff + time.Sleep(backoff) + continue + } + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("remove old iam file: %w", err) + } + + // save copy of data + datacopy := make([]byte, len(b)) + copy(datacopy, b) + + // make a backup copy in case we crash before update + // this is after remove, so there is a small window something + // can go wrong, but the remove should barrier other gateways + // from trying to write backup at the same time. Only one + // gateway will successfully remove the file. + os.WriteFile(iamBackupFile, b, iamMode) + + b, err = update(b) + if err != nil { + // update failed, try to write old data back out + os.WriteFile(iamFile, datacopy, iamMode) + return fmt.Errorf("update iam data: %w", err) + } + + err = writeTempFile(b) + if err != nil { + // update failed, try to write old data back out + os.WriteFile(iamFile, datacopy, iamMode) + return err + } + + p.iamcache = b + p.iamvalid = true + p.iamexpire = time.Now().Add(cacheDuration) + break + } + + return nil +} + +func writeTempFile(b []byte) error { + f, err := os.CreateTemp(".", iamFile) + if err != nil { + return fmt.Errorf("create temp file: %w", err) + } + defer os.Remove(f.Name()) + + _, err = f.Write(b) + if err != nil { + return fmt.Errorf("write temp file: %w", err) + } + + err = os.Rename(f.Name(), iamFile) + if err != nil { + return fmt.Errorf("rename temp file: %w", err) + } + + return nil +} + func isNoAttr(err error) bool { if err == nil { return false diff --git a/cmd/versitygw/main.go b/cmd/versitygw/main.go index 1c94504a..52997c81 100644 --- a/cmd/versitygw/main.go +++ b/cmd/versitygw/main.go @@ -133,7 +133,7 @@ func initFlags() []cli.Flag { } } -func runGateway(be backend.Backend) error { +func runGateway(be backend.Backend, s auth.Storer) error { app := fiber.New(fiber.Config{ AppName: "versitygw", ServerHeader: "VERSITYGW", @@ -161,16 +161,20 @@ func runGateway(be backend.Backend) error { opts = append(opts, s3api.WithDebug()) } - iam, err := auth.InitIAM() + err := s.InitIAM() if err != nil { - return err + return fmt.Errorf("init iam: %w", err) + } + + iam, err := auth.NewInternal(s) + if err != nil { + return fmt.Errorf("setup internal iam service: %w", err) } srv, err := s3api.New(app, be, middlewares.RootUserConfig{ Access: rootUserAccess, Secret: rootUserSecret, - Region: region, - }, port, iam, opts...) + }, port, region, iam, opts...) if err != nil { return fmt.Errorf("init gateway: %v", err) } diff --git a/cmd/versitygw/posix.go b/cmd/versitygw/posix.go index a7de618f..ebd8204f 100644 --- a/cmd/versitygw/posix.go +++ b/cmd/versitygw/posix.go @@ -49,5 +49,5 @@ func runPosix(ctx *cli.Context) error { return fmt.Errorf("init posix: %v", err) } - return runGateway(be) + return runGateway(be, be) } diff --git a/cmd/versitygw/scoutfs.go b/cmd/versitygw/scoutfs.go index 1b2d5fab..dd272e95 100644 --- a/cmd/versitygw/scoutfs.go +++ b/cmd/versitygw/scoutfs.go @@ -52,5 +52,5 @@ func runScoutfs(ctx *cli.Context) error { return fmt.Errorf("init scoutfs: %v", err) } - return runGateway(be) + return runGateway(be, be) } diff --git a/s3api/controllers/admin.go b/s3api/controllers/admin.go index a9e82ddf..3a0fdc45 100644 --- a/s3api/controllers/admin.go +++ b/s3api/controllers/admin.go @@ -26,16 +26,16 @@ type AdminController struct { } func (c AdminController) CreateUser(ctx *fiber.Ctx) error { - access, secret, role, region := ctx.Query("access"), ctx.Query("secret"), ctx.Query("role"), ctx.Query("region") + access, secret, role := ctx.Query("access"), ctx.Query("secret"), ctx.Query("role") requesterRole := ctx.Locals("role") if requesterRole != "admin" { return fmt.Errorf("access denied: only admin users have access to this resource") } - user := auth.Account{Secret: secret, Role: role, Region: region} + user := auth.Account{Secret: secret, Role: role} - err := c.IAMService.CreateAccount(access, &user) + err := c.IAMService.CreateAccount(access, user) if err != nil { return fmt.Errorf("failed to create a user: %w", err) } diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 9116d1fd..17ee506f 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -36,7 +36,8 @@ import ( ) type S3ApiController struct { - be backend.Backend + be backend.Backend + iam auth.IAMService } func New(be backend.Backend) S3ApiController { @@ -257,12 +258,7 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { AccessControlPolicy: &types.AccessControlPolicy{Owner: &types.Owner{ID: &access}}, } - iam, err := auth.GetIAMConfig() - if err != nil { - return SendResponse(ctx, err) - } - - err = auth.UpdateACL(input, parsedAcl, *iam) + err = auth.UpdateACL(input, parsedAcl, c.iam) return SendResponse(ctx, err) } diff --git a/s3api/middlewares/authentication.go b/s3api/middlewares/authentication.go index 88589226..a5ab3237 100644 --- a/s3api/middlewares/authentication.go +++ b/s3api/middlewares/authentication.go @@ -38,10 +38,9 @@ const ( type RootUserConfig struct { Access string Secret string - Region string } -func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, debug bool) fiber.Handler { +func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, region string, debug bool) fiber.Handler { acct := accounts{root: root, iam: iam} return func(ctx *fiber.Ctx) error { @@ -74,10 +73,13 @@ func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, debug bool) fib } signedHdrs := strings.Split(signHdrKv[1], ";") - account := acct.getAccount(creds[0]) - if account == nil { + account, err := acct.getAccount(creds[0]) + if err == auth.ErrNoSuchUser { return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidAccessKeyID)) } + if err != nil { + return controllers.SendResponse(ctx, err) + } // Check X-Amz-Date header date := ctx.Get("X-Amz-Date") @@ -113,7 +115,7 @@ func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, debug bool) fib signErr := signer.SignHTTP(req.Context(), aws.Credentials{ AccessKeyID: creds[0], SecretAccessKey: account.Secret, - }, req, hexPayload, creds[3], account.Region, tdate, func(options *v4.SignerOptions) { + }, req, hexPayload, creds[3], region, tdate, func(options *v4.SignerOptions) { if debug { options.LogSigning = true options.Logger = logging.NewStandardLogger(os.Stderr) @@ -147,16 +149,13 @@ type accounts struct { iam auth.IAMService } -func (a accounts) getAccount(access string) *auth.Account { - var account *auth.Account +func (a accounts) getAccount(access string) (auth.Account, error) { if access == a.root.Access { - account = &auth.Account{ + return auth.Account{ Secret: a.root.Secret, Role: "admin", - Region: a.root.Region, - } - } else { - account = a.iam.GetUserAccount(access) + }, nil } - return account + + return a.iam.GetUserAccount(access) } diff --git a/s3api/router_test.go b/s3api/router_test.go index 2b14dd93..663c71fa 100644 --- a/s3api/router_test.go +++ b/s3api/router_test.go @@ -39,7 +39,7 @@ func TestS3ApiRouter_Init(t *testing.T) { args: args{ app: fiber.New(), be: backend.BackendUnsupported{}, - iam: auth.IAMServiceUnsupported{}, + iam: &auth.IAMServiceInternal{}, }, }, } diff --git a/s3api/server.go b/s3api/server.go index 1588dabf..09883192 100644 --- a/s3api/server.go +++ b/s3api/server.go @@ -33,7 +33,7 @@ type S3ApiServer struct { debug bool } -func New(app *fiber.App, be backend.Backend, root middlewares.RootUserConfig, port string, iam auth.IAMService, opts ...Option) (*S3ApiServer, error) { +func New(app *fiber.App, be backend.Backend, root middlewares.RootUserConfig, port, region string, iam auth.IAMService, opts ...Option) (*S3ApiServer, error) { server := &S3ApiServer{ app: app, backend: be, @@ -45,7 +45,7 @@ func New(app *fiber.App, be backend.Backend, root middlewares.RootUserConfig, po opt(server) } - app.Use(middlewares.VerifyV4Signature(root, iam, server.debug)) + app.Use(middlewares.VerifyV4Signature(root, iam, region, server.debug)) app.Use(logger.New()) app.Use(middlewares.VerifyMD5Body()) server.router.Init(app, be, iam) diff --git a/s3api/server_test.go b/s3api/server_test.go index ea6aa1cf..27ce0bcf 100644 --- a/s3api/server_test.go +++ b/s3api/server_test.go @@ -63,7 +63,7 @@ func TestNew(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { gotS3ApiServer, err := New(tt.args.app, tt.args.be, tt.args.root, - tt.args.port, auth.IAMServiceUnsupported{}) + tt.args.port, "us-east-1", &auth.IAMServiceInternal{}) if (err != nil) != tt.wantErr { t.Errorf("New() error = %v, wantErr %v", err, tt.wantErr) return