diff --git a/api4/system_test.go b/api4/system_test.go index dfe0379892342..c327a5955e54c 100644 --- a/api4/system_test.go +++ b/api4/system_test.go @@ -507,12 +507,20 @@ func TestS3TestConnection(t *testing.T) { config.FileSettings.AmazonS3Bucket = model.NewString("Wrong_bucket") _, resp = th.SystemAdminClient.TestS3Connection(&config) CheckInternalErrorStatus(t, resp) - assert.Equal(t, "api.file.test_connection.app_error", resp.Error.Id) + assert.Equal(t, "api.file.test_connection_s3_bucket_does_not_exist.app_error", resp.Error.Id) *config.FileSettings.AmazonS3Bucket = "shouldnotcreatenewbucket" _, resp = th.SystemAdminClient.TestS3Connection(&config) CheckInternalErrorStatus(t, resp) - assert.Equal(t, "api.file.test_connection.app_error", resp.Error.Id) + assert.Equal(t, "api.file.test_connection_s3_bucket_does_not_exist.app_error", resp.Error.Id) + }) + + t.Run("with incorrect credentials", func(t *testing.T) { + configCopy := config + *configCopy.FileSettings.AmazonS3AccessKeyId = "invalidaccesskey" + _, resp := th.SystemAdminClient.TestS3Connection(&configCopy) + CheckInternalErrorStatus(t, resp) + assert.Equal(t, "api.file.test_connection_s3_auth.app_error", resp.Error.Id) }) t.Run("as restricted system admin", func(t *testing.T) { @@ -521,7 +529,6 @@ func TestS3TestConnection(t *testing.T) { _, resp := th.SystemAdminClient.TestS3Connection(&config) CheckForbiddenStatus(t, resp) }) - } func TestSupportedTimezones(t *testing.T) { diff --git a/app/file.go b/app/file.go index 721a9eb7b4be6..bdff81ff7af0b 100644 --- a/app/file.go +++ b/app/file.go @@ -91,6 +91,17 @@ func (a *App) CheckMandatoryS3Fields(settings *model.FileSettings) *model.AppErr return nil } +func connectionTestErrorToAppError(connTestErr error) *model.AppError { + switch err := connTestErr.(type) { + case *filestore.S3FileBackendAuthError: + return model.NewAppError("TestConnection", "api.file.test_connection_s3_auth.app_error", nil, err.Error(), http.StatusInternalServerError) + case *filestore.S3FileBackendNoBucketError: + return model.NewAppError("TestConnection", "api.file.test_connection_s3_bucket_does_not_exist.app_error", nil, err.Error(), http.StatusInternalServerError) + default: + return model.NewAppError("TestConnection", "api.file.test_connection.app_error", nil, connTestErr.Error(), http.StatusInternalServerError) + } +} + func (a *App) TestFileStoreConnection() *model.AppError { backend, err := a.FileBackend() if err != nil { @@ -98,7 +109,7 @@ func (a *App) TestFileStoreConnection() *model.AppError { } nErr := backend.TestConnection() if nErr != nil { - return model.NewAppError("TestConnection", "api.file.test_connection.app_error", nil, nErr.Error(), http.StatusInternalServerError) + return connectionTestErrorToAppError(nErr) } return nil } @@ -111,7 +122,7 @@ func (a *App) TestFileStoreConnectionWithConfig(cfg *model.FileSettings) *model. } nErr := backend.TestConnection() if nErr != nil { - return model.NewAppError("TestConnection", "api.file.test_connection.app_error", nil, nErr.Error(), http.StatusInternalServerError) + return connectionTestErrorToAppError(nErr) } return nil } diff --git a/app/server.go b/app/server.go index c91d614778ffe..79fe4b888a233 100644 --- a/app/server.go +++ b/app/server.go @@ -522,7 +522,7 @@ func NewServer(options ...Option) (*Server, error) { } else { nErr := backend.TestConnection() if nErr != nil { - if errors.Is(nErr, filestore.ErrNoS3Bucket) { + if _, ok := nErr.(*filestore.S3FileBackendNoBucketError); ok { nErr = backend.(*filestore.S3FileBackend).MakeBucket() } if nErr != nil { diff --git a/i18n/en.json b/i18n/en.json index 8d68374ae80dc..03248126703c0 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1832,6 +1832,14 @@ "id": "api.file.test_connection.app_error", "translation": "Unable to access the file storage." }, + { + "id": "api.file.test_connection_s3_auth.app_error", + "translation": "Unable to connect to S3. Verify your Amazon S3 connection authorization parameters and authentication settings." + }, + { + "id": "api.file.test_connection_s3_bucket_does_not_exist.app_error", + "translation": "Ensure your Amazon S3 bucket is available, and verify your bucket permissions." + }, { "id": "api.file.upload_file.incorrect_channelId.app_error", "translation": "Unable to upload the file. Incorrect channel ID: {{.channelId}}" diff --git a/shared/filestore/filesstore_test.go b/shared/filestore/filesstore_test.go index a62c173c26477..d9fbb3e127e0e 100644 --- a/shared/filestore/filesstore_test.go +++ b/shared/filestore/filesstore_test.go @@ -94,9 +94,9 @@ func (s *FileBackendTestSuite) SetupTest() { s.backend = backend // This is needed to create the bucket if it doesn't exist. - if err := s.backend.TestConnection(); err == ErrNoS3Bucket { - s3Backend, ok := s.backend.(*S3FileBackend) - s.True(ok) + err = s.backend.TestConnection() + if _, ok := err.(*S3FileBackendNoBucketError); ok { + s3Backend := s.backend.(*S3FileBackend) s.NoError(s3Backend.MakeBucket()) } else { s.NoError(err) diff --git a/shared/filestore/s3store.go b/shared/filestore/s3store.go index 6e7c49f0cdc64..5d0bf38e650aa 100644 --- a/shared/filestore/s3store.go +++ b/shared/filestore/s3store.go @@ -36,13 +36,18 @@ type S3FileBackend struct { client *s3.Client } +type S3FileBackendAuthError struct { + DetailedError string +} + +// S3FileBackendNoBucketError is returned when testing a connection and no S3 bucket is found +type S3FileBackendNoBucketError struct{} + const ( // This is not exported by minio. See: https://github.com/minio/minio-go/issues/1339 bucketNotFound = "NoSuchBucket" ) -// ErrNoS3Bucket is returned when testing a connection and no S3 bucket is found -var ErrNoS3Bucket = errors.New("no such bucket") var ( imageExtensions = map[string]bool{".jpg": true, ".jpeg": true, ".gif": true, ".bmp": true, ".png": true, ".tiff": true, "tif": true} imageMimeTypes = map[string]string{".jpg": "image/jpeg", ".jpeg": "image/jpeg", ".gif": "image/gif", ".bmp": "image/bmp", ".png": "image/png", ".tiff": "image/tiff", ".tif": "image/tif"} @@ -61,6 +66,14 @@ func getImageMimeType(ext string) string { return imageMimeTypes[ext] } +func (s *S3FileBackendAuthError) Error() string { + return s.DetailedError +} + +func (s *S3FileBackendNoBucketError) Error() string { + return "no such bucket" +} + // NewS3FileBackend returns an instance of an S3FileBackend. func NewS3FileBackend(settings FileBackendSettings) (*S3FileBackend, error) { backend := &S3FileBackend{ @@ -148,19 +161,19 @@ func (b *S3FileBackend) TestConnection() error { if obj.Err != nil { typedErr := s3.ToErrorResponse(obj.Err) if typedErr.Code != bucketNotFound { - return errors.Wrap(err, "unable to list objects in the s3 bucket") + return &S3FileBackendAuthError{DetailedError: "unable to list objects in the S3 bucket"} } exists = false } } else { exists, err = b.client.BucketExists(context.Background(), b.bucket) if err != nil { - return errors.Wrap(err, "unable to check if the s3 bucket exists") + return &S3FileBackendAuthError{DetailedError: "unable to check if the S3 bucket exists"} } } if !exists { - return ErrNoS3Bucket + return &S3FileBackendNoBucketError{} } mlog.Debug("Connection to S3 or minio is good. Bucket exists.") return nil