diff --git a/e2e-tests/cypress/tests/integration/channels/system_console/azure_blob_storage_spec.js b/e2e-tests/cypress/tests/integration/channels/system_console/azure_blob_storage_spec.js index 187582345a1..fedd2cedf1a 100644 --- a/e2e-tests/cypress/tests/integration/channels/system_console/azure_blob_storage_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/system_console/azure_blob_storage_spec.js @@ -36,6 +36,7 @@ describe('Environment - File Storage (Azure Blob Storage)', () => { cy.findByTestId('FileSettings.AzureStorageAccountinput').should('not.be.disabled'); cy.findByTestId('FileSettings.AzureContainerinput').should('not.be.disabled'); cy.findByTestId('FileSettings.AzurePathPrefixinput').should('not.be.disabled'); + cy.findByTestId('FileSettings.AzureAuthModedropdown').should('not.be.disabled'); cy.findByTestId('FileSettings.AzureAccessKeyinput').should('not.be.disabled'); cy.findByTestId('FileSettings.AzureClouddropdown').should('not.be.disabled'); cy.findByTestId('FileSettings.AzureRequestTimeoutMillisecondsnumber').should('not.be.disabled'); @@ -82,11 +83,33 @@ describe('Environment - File Storage (Azure Blob Storage)', () => { // * Azure fields are not rendered when the driver is not Azure cy.findByTestId('FileSettings.AzureStorageAccountinput').should('not.exist'); cy.findByTestId('FileSettings.AzureContainerinput').should('not.exist'); + cy.findByTestId('FileSettings.AzureAuthModedropdown').should('not.exist'); cy.findByTestId('FileSettings.AzureAccessKeyinput').should('not.exist'); cy.findByTestId('FileSettings.AzureClouddropdown').should('not.exist'); cy.findByTestId('FileSettings.AzureEndpointinput').should('not.exist'); }); + it('hides the access key when the authentication mode is default credential', () => { + // # Select the Azure driver + cy.findByTestId('FileSettings.DriverNamedropdown').select('azureblob'); + + // * Shared key is the default and the access key is visible + cy.findByTestId('FileSettings.AzureAuthModedropdown').should('have.value', 'shared_key'); + cy.findByTestId('FileSettings.AzureAccessKeyinput').scrollIntoView().should('be.visible'); + + // # Switch to default credential + cy.findByTestId('FileSettings.AzureAuthModedropdown').select('default_credential'); + + // * The access key field is removed from the DOM + cy.findByTestId('FileSettings.AzureAccessKeyinput').should('not.exist'); + + // # Switch back to shared key + cy.findByTestId('FileSettings.AzureAuthModedropdown').select('shared_key'); + + // * The access key field reappears + cy.findByTestId('FileSettings.AzureAccessKeyinput').scrollIntoView().should('be.visible'); + }); + it('exposes the backend-agnostic Test Connection button when Azure is selected', () => { // # Select the Azure driver cy.findByTestId('FileSettings.DriverNamedropdown').select('azureblob'); diff --git a/server/channels/app/file.go b/server/channels/app/file.go index 712f2254c31..1e4750caad9 100644 --- a/server/channels/app/file.go +++ b/server/channels/app/file.go @@ -72,10 +72,12 @@ func (a *App) CheckMandatoryS3Fields(settings *model.FileSettings) *model.AppErr func (a *App) CheckMandatoryAzureFields(settings *model.FileSettings) *model.AppError { storageAccount := settings.AzureStorageAccount + authMode := settings.AzureAuthMode accessKey := settings.AzureAccessKey container := settings.AzureContainer if a.License().IsCloud() && a.Config().FeatureFlags.CloudDedicatedExportUI && a.Config().FileSettings.DedicatedExportStore != nil && *a.Config().FileSettings.DedicatedExportStore { storageAccount = settings.ExportAzureStorageAccount + authMode = settings.ExportAzureAuthMode accessKey = settings.ExportAzureAccessKey container = settings.ExportAzureContainer } @@ -85,7 +87,9 @@ func (a *App) CheckMandatoryAzureFields(settings *model.FileSettings) *model.App if container == nil || *container == "" { return model.NewAppError("CheckMandatoryAzureFields", "api.admin.test_azure.missing_azure_field", nil, "missing azure container setting", http.StatusBadRequest) } - if accessKey == nil || *accessKey == "" { + // Access key only matters under shared-key auth. Default credential pulls + // identity from the host environment. + if authMode != nil && *authMode == model.AzureAuthModeSharedKey && (accessKey == nil || *accessKey == "") { return model.NewAppError("CheckMandatoryAzureFields", "api.admin.test_azure.missing_azure_field", nil, "missing azure access key setting", http.StatusBadRequest) } return nil diff --git a/server/cmd/mattermost/commands/db.go b/server/cmd/mattermost/commands/db.go index 543a25f7b61..cf594d342d2 100644 --- a/server/cmd/mattermost/commands/db.go +++ b/server/cmd/mattermost/commands/db.go @@ -333,6 +333,7 @@ func ConfigToFileBackendSettings(s *model.FileSettings, enableComplianceFeature return filestore.FileBackendSettings{ DriverName: *s.DriverName, AzureStorageAccount: *s.AzureStorageAccount, + AzureAuthMode: *s.AzureAuthMode, AzureAccessKey: *s.AzureAccessKey, AzureContainer: *s.AzureContainer, AzurePathPrefix: *s.AzurePathPrefix, diff --git a/server/go.mod b/server/go.mod index 1f3ac124780..6494d323c17 100644 --- a/server/go.mod +++ b/server/go.mod @@ -5,6 +5,7 @@ go 1.26.3 require ( code.sajari.com/docconv/v2 v2.0.0-pre.4 github.com/Azure/azure-sdk-for-go/sdk/azcore v1.21.1 + github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.13.1 github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.6.4 github.com/Masterminds/semver/v3 v3.5.0 github.com/avct/uasurfer v0.0.0-20250915105040-a942f6fb6edc @@ -87,6 +88,7 @@ require ( require ( filippo.io/edwards25519 v1.2.0 // indirect github.com/Azure/azure-sdk-for-go/sdk/internal v1.12.0 // indirect + github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0 // indirect github.com/JalfResi/justext v0.0.0-20221106200834-be571e3e3052 // indirect github.com/PuerkitoBio/goquery v1.12.0 // indirect github.com/STARRY-S/zip v0.2.3 // indirect @@ -156,6 +158,7 @@ require ( github.com/klauspost/cpuid/v2 v2.3.0 // indirect github.com/klauspost/crc32 v1.3.0 // indirect github.com/klauspost/pgzip v1.2.6 // indirect + github.com/kylelemons/godebug v1.1.0 // indirect github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect github.com/levigross/exp-html v0.0.0-20120902181939-8df60c69a8f5 // indirect @@ -183,6 +186,7 @@ require ( github.com/pelletier/go-toml/v2 v2.3.1 // indirect github.com/philhofer/fwd v1.2.0 // indirect github.com/pierrec/lz4/v4 v4.1.26 // indirect + github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/procfs v0.20.1 // indirect github.com/redis/go-redis/v9 v9.19.0 // indirect diff --git a/server/go.sum b/server/go.sum index 005aeef37b3..7dc420559c3 100644 --- a/server/go.sum +++ b/server/go.sum @@ -16,12 +16,16 @@ github.com/Azure/azure-sdk-for-go/sdk/azcore v1.21.1 h1:jHb/wfvRikGdxMXYV3QG/SzU github.com/Azure/azure-sdk-for-go/sdk/azcore v1.21.1/go.mod h1:pzBXCYn05zvYIrwLgtK8Ap8QcjRg+0i76tMQdWN6wOk= github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.13.1 h1:Hk5QBxZQC1jb2Fwj6mpzme37xbCDdNTxU7O9eb5+LB4= github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.13.1/go.mod h1:IYus9qsFobWIc2YVwe/WPjcnyCkPKtnHAqUYeebc8z0= +github.com/Azure/azure-sdk-for-go/sdk/azidentity/cache v0.3.2 h1:yz1bePFlP5Vws5+8ez6T3HWXPmwOK7Yvq8QxDBD3SKY= +github.com/Azure/azure-sdk-for-go/sdk/azidentity/cache v0.3.2/go.mod h1:Pa9ZNPuoNu/GztvBSKk9J1cDJW6vk/n0zLtV4mgd8N8= github.com/Azure/azure-sdk-for-go/sdk/internal v1.12.0 h1:fhqpLE3UEXi9lPaBRpQ6XuRW0nU7hgg4zlmZZa+a9q4= github.com/Azure/azure-sdk-for-go/sdk/internal v1.12.0/go.mod h1:7dCRMLwisfRH3dBupKeNCioWYUZ4SS09Z14H+7i8ZoY= github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/storage/armstorage v1.8.1 h1:/Zt+cDPnpC3OVDm/JKLOs7M2DKmLRIIp3XIx9pHHiig= github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/storage/armstorage v1.8.1/go.mod h1:Ng3urmn6dYe8gnbCMoHHVl5APYz2txho3koEkV2o2HA= github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.6.4 h1:jWQK1GI+LeGGUKBADtcH2rRqPxYB1Ljwms5gFA2LqrM= github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.6.4/go.mod h1:8mwH4klAm9DUgR2EEHyEEAQlRDvLPyg5fQry3y+cDew= +github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1 h1:WJTmL004Abzc5wDB5VtZG2PJk5ndYDgVacGqfirKxjM= +github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1/go.mod h1:tCcJZ0uHAmvjsVYzEFivsRTN00oz5BEsRgQHu5JZ9WE= github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0 h1:XRzhVemXdgvJqCH0sFfrBUTnUJSBrBf7++ypk+twtRs= github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0/go.mod h1:HKpQxkWaGLJ+D/5H8QRpyQXA1eKjxkFlOMwck5+33Jk= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= @@ -339,6 +343,8 @@ github.com/json-iterator/go v1.1.11/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/ github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1:6v2b51hI/fHJwM22ozAgKL4VKDeJcHhJFhtBdhmNjmU= github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7VTCxuUUipMqKk8s4w= github.com/julienschmidt/httprouter v1.3.0/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM= +github.com/keybase/go-keychain v0.0.1 h1:way+bWYa6lDppZoZcgMbYsvC7GxljxrskdNInRtuthU= +github.com/keybase/go-keychain v0.0.1/go.mod h1:PdEILRW3i9D8JcdM+FmY6RwkHGnhHxXwkPPMeUgOK1k= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/klauspost/compress v1.4.1/go.mod h1:RyIbtBH6LamlWaDj8nUwkbUhJ87Yi3uG0guNDohfE1A= github.com/klauspost/compress v1.18.6 h1:2jupLlAwFm95+YDR+NwD2MEfFO9d4z4Prjl1XXDjuao= @@ -800,6 +806,7 @@ golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220503163025-988cb79eb6c6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= diff --git a/server/i18n/en.json b/server/i18n/en.json index 8746354ab32..c6f5cd38140 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -11374,6 +11374,10 @@ "id": "model.config.is_valid.autotranslation.workers.app_error", "translation": "Workers must be between 1 and 64." }, + { + "id": "model.config.is_valid.azure_auth_mode.app_error", + "translation": "Invalid Azure authentication mode {{.Value}}. Must be 'shared_key' or 'default_credential'." + }, { "id": "model.config.is_valid.azure_cloud.app_error", "translation": "Invalid value {{.Value}} for {{.Setting}}. Must be 'commercial', 'government', or 'custom'." @@ -11570,6 +11574,10 @@ "id": "model.config.is_valid.export.retention_days_too_low.app_error", "translation": "Invalid value for RetentionDays. Value should be greater than 0" }, + { + "id": "model.config.is_valid.export_azure_auth_mode.app_error", + "translation": "Invalid Azure authentication mode {{.Value}} for the dedicated export store. Must be 'shared_key' or 'default_credential'." + }, { "id": "model.config.is_valid.export_azure_timeout.app_error", "translation": "Invalid timeout value {{.Value}}. Should be a positive number." diff --git a/server/platform/shared/filestore/azurestore.go b/server/platform/shared/filestore/azurestore.go index 31671be2709..ee6eecb2a56 100644 --- a/server/platform/shared/filestore/azurestore.go +++ b/server/platform/shared/filestore/azurestore.go @@ -19,6 +19,7 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azidentity" "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob" "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/blob" "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/bloberror" @@ -27,7 +28,6 @@ import ( "github.com/google/uuid" "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/public/shared/mlog" - pkgerr "github.com/pkg/errors" ) // azureBlockSize is the chunk size used when staging block blob uploads. @@ -35,8 +35,11 @@ import ( // StageBlock call well under the per-block REST limit (4000 MiB). const azureBlockSize = 4 * 1024 * 1024 -// AzureFileBackend stores files in Azure Blob Storage. Connections are -// authenticated with a shared key today; Microsoft Entra ID is a follow-up. +// AzureFileBackend stores files in Azure Blob Storage. Two authentication +// modes are supported: shared key (an account access key configured by the +// admin) and Microsoft Entra ID via DefaultAzureCredential (managed identity, +// service principal, workload identity, or az login - whichever the host +// environment provides). type AzureFileBackend struct { client *azblob.Client container string @@ -49,11 +52,6 @@ func NewAzureFileBackend(settings FileBackendSettings) (*AzureFileBackend, error return nil, err } - credential, err := azblob.NewSharedKeyCredential(settings.AzureStorageAccount, settings.AzureAccessKey) - if err != nil { - return nil, pkgerr.Wrap(err, "failed to create azure shared key credential") - } - scheme := "https" if !settings.AzureSSL { scheme = "http" @@ -80,9 +78,9 @@ func NewAzureFileBackend(settings FileBackendSettings) (*AzureFileBackend, error } } - client, err := azblob.NewClientWithSharedKeyCredential(serviceURL, credential, clientOptions) + client, err := newAzureClient(settings, serviceURL, clientOptions) if err != nil { - return nil, pkgerr.Wrap(err, "failed to create azure blob client") + return nil, err } // Config.IsValid rejects non-positive timeouts before they reach this @@ -109,6 +107,38 @@ func (b *AzureFileBackend) DriverName() string { return driverAzure } +// newAzureClient builds an azblob client for the configured authentication +// mode. Shared key uses NewClientWithSharedKeyCredential; default credential +// uses NewClient with DefaultAzureCredential, which discovers managed +// identity, workload identity, service principal env vars, and az login in +// that order at runtime. +func newAzureClient(settings FileBackendSettings, serviceURL string, clientOptions *azblob.ClientOptions) (*azblob.Client, error) { + switch settings.AzureAuthMode { + case model.AzureAuthModeDefaultCredential: + cred, err := azidentity.NewDefaultAzureCredential(nil) + if err != nil { + return nil, fmt.Errorf("failed to create azure default credential: %w", err) + } + client, err := azblob.NewClient(serviceURL, cred, clientOptions) + if err != nil { + return nil, fmt.Errorf("failed to create azure blob client: %w", err) + } + return client, nil + case model.AzureAuthModeSharedKey: + cred, err := azblob.NewSharedKeyCredential(settings.AzureStorageAccount, settings.AzureAccessKey) + if err != nil { + return nil, fmt.Errorf("failed to create azure shared key credential: %w", err) + } + client, err := azblob.NewClientWithSharedKeyCredential(serviceURL, cred, clientOptions) + if err != nil { + return nil, fmt.Errorf("failed to create azure blob client: %w", err) + } + return client, nil + default: + return nil, fmt.Errorf("unknown azure auth mode %q", settings.AzureAuthMode) + } +} + // buildAzureServiceURL renders the Blob service URL that the SDK signs // requests against. The cloud value selects the topology: // @@ -206,12 +236,12 @@ func (b *AzureFileBackend) TestConnection() error { return nil } if bloberror.HasCode(err, bloberror.ContainerNotFound) { - return &FileBackendNoBucketError{Err: pkgerr.Wrapf(err, "azure container %q does not exist", b.container)} + return &FileBackendNoBucketError{Err: fmt.Errorf("azure container %q does not exist: %w", b.container, err)} } if isAzureAuthError(err) { - return &FileBackendAuthError{Err: pkgerr.Wrap(err, "unable to authenticate against azure blob storage")} + return &FileBackendAuthError{Err: fmt.Errorf("unable to authenticate against azure blob storage: %w", err)} } - return pkgerr.Wrap(err, "unable to connect to azure blob storage") + return fmt.Errorf("unable to connect to azure blob storage: %w", err) } // MakeContainer creates the configured container. Mirrors S3FileBackend.MakeBucket @@ -226,7 +256,7 @@ func (b *AzureFileBackend) MakeContainer() error { if bloberror.HasCode(err, bloberror.ContainerAlreadyExists) { return nil } - return pkgerr.Wrapf(err, "unable to create azure container %q", b.container) + return fmt.Errorf("unable to create azure container %q: %w", b.container, err) } return nil } @@ -244,12 +274,12 @@ func (b *AzureFileBackend) Reader(p string) (ReadCloseSeeker, error) { if err != nil { timer.Stop() cancel() - return nil, pkgerr.Wrapf(err, "unable to read file %q", p) + return nil, fmt.Errorf("unable to read file %q: %w", p, err) } if props.ContentLength == nil { timer.Stop() cancel() - return nil, pkgerr.Errorf("missing content length for %q", p) + return nil, fmt.Errorf("missing content length for %q", p) } return &azureRangeReader{ @@ -279,7 +309,7 @@ func (b *AzureFileBackend) FileExists(p string) (bool, error) { if bloberror.HasCode(err, bloberror.BlobNotFound) { return false, nil } - return false, pkgerr.Wrapf(err, "unable to check existence of %q", p) + return false, fmt.Errorf("unable to check existence of %q: %w", p, err) } return true, nil } @@ -290,7 +320,7 @@ func (b *AzureFileBackend) FileSize(p string) (int64, error) { props, err := b.newBlobClient(p).GetProperties(ctx, nil) if err != nil { - return 0, pkgerr.Wrapf(err, "unable to get size of %q", p) + return 0, fmt.Errorf("unable to get size of %q: %w", p, err) } return model.SafeDereference(props.ContentLength), nil @@ -302,7 +332,7 @@ func (b *AzureFileBackend) FileModTime(p string) (time.Time, error) { props, err := b.newBlobClient(p).GetProperties(ctx, nil) if err != nil { - return time.Time{}, pkgerr.Wrapf(err, "unable to get modification time of %q", p) + return time.Time{}, fmt.Errorf("unable to get modification time of %q: %w", p, err) } return model.SafeDereference(props.LastModified), nil @@ -318,7 +348,7 @@ func (b *AzureFileBackend) CopyFile(oldPath, newPath string) error { src := b.newBlobClient(oldPath).URL() dst := b.newBlockBlobClient(newPath) if _, err := dst.StartCopyFromURL(ctx, src, nil); err != nil { - return pkgerr.Wrapf(err, "unable to copy %q to %q", oldPath, newPath) + return fmt.Errorf("unable to copy %q to %q: %w", oldPath, newPath, err) } // Poll until the copy reports success. For server-to-server copies within @@ -327,7 +357,7 @@ func (b *AzureFileBackend) CopyFile(oldPath, newPath string) error { for { props, err := dst.GetProperties(ctx, nil) if err != nil { - return pkgerr.Wrapf(err, "unable to read copy status for %q", newPath) + return fmt.Errorf("unable to read copy status for %q: %w", newPath, err) } if props.CopyStatus == nil { return nil @@ -337,11 +367,11 @@ func (b *AzureFileBackend) CopyFile(oldPath, newPath string) error { return nil case blob.CopyStatusTypeFailed, blob.CopyStatusTypeAborted: desc := model.SafeDereference(props.CopyStatusDescription) - return pkgerr.Errorf("azure copy from %q to %q ended in status %q: %q", oldPath, newPath, *props.CopyStatus, desc) + return fmt.Errorf("azure copy from %q to %q ended in status %q: %q", oldPath, newPath, *props.CopyStatus, desc) } select { case <-ctx.Done(): - return pkgerr.Wrapf(ctx.Err(), "azure copy from %q to %q did not complete in time", oldPath, newPath) + return fmt.Errorf("azure copy from %q to %q did not complete in time: %w", oldPath, newPath, ctx.Err()) case <-time.After(50 * time.Millisecond): } } @@ -374,10 +404,10 @@ func (b *AzureFileBackend) stageBlocks(ctx context.Context, bb *blockblob.Client if n > 0 { id, idErr := newAzureBlockID() if idErr != nil { - return nil, 0, pkgerr.Wrap(idErr, "failed to generate azure block id") + return nil, 0, fmt.Errorf("failed to generate azure block id: %w", idErr) } if _, sbErr := bb.StageBlock(ctx, id, &readSeekNopCloser{Reader: bytes.NewReader(buf[:n])}, nil); sbErr != nil { - return nil, 0, pkgerr.Wrapf(sbErr, "unable to stage block for %q", p) + return nil, 0, fmt.Errorf("unable to stage block for %q: %w", p, sbErr) } ids = append(ids, id) total += int64(n) @@ -386,7 +416,7 @@ func (b *AzureFileBackend) stageBlocks(ctx context.Context, bb *blockblob.Client break } if err != nil { - return nil, 0, pkgerr.Wrap(err, "failed to read input") + return nil, 0, fmt.Errorf("failed to read input: %w", err) } } return ids, total, nil @@ -416,16 +446,16 @@ func (b *AzureFileBackend) WriteFileContext(ctx context.Context, fr io.Reader, p // committed block list so AppendFile can target it. id, idErr := newAzureBlockID() if idErr != nil { - return 0, pkgerr.Wrap(idErr, "failed to generate azure block id") + return 0, fmt.Errorf("failed to generate azure block id: %w", idErr) } if _, sbErr := bb.StageBlock(ctx, id, &readSeekNopCloser{Reader: bytes.NewReader(nil)}, nil); sbErr != nil { - return 0, pkgerr.Wrapf(sbErr, "unable to stage empty block for %q", p) + return 0, fmt.Errorf("unable to stage empty block for %q: %w", p, sbErr) } blockIDs = append(blockIDs, id) } if _, err := bb.CommitBlockList(ctx, blockIDs, nil); err != nil { - return 0, pkgerr.Wrapf(err, "unable to commit block list for %q", p) + return 0, fmt.Errorf("unable to commit block list for %q: %w", p, err) } return total, nil } @@ -450,7 +480,7 @@ func (b *AzureFileBackend) AppendFile(fr io.Reader, p string) (int64, error) { listResp, err := bb.GetBlockList(ctx, blockblob.BlockListTypeCommitted, nil) if err != nil { - return 0, pkgerr.Wrapf(err, "unable to find file %q to append data", p) + return 0, fmt.Errorf("unable to find file %q to append data: %w", p, err) } var existingIDs []string @@ -465,10 +495,10 @@ func (b *AzureFileBackend) AppendFile(fr io.Reader, p string) (int64, error) { if len(existingIDs) == 0 { props, propsErr := bb.GetProperties(ctx, nil) if propsErr != nil { - return 0, pkgerr.Wrapf(propsErr, "unable to inspect %q before append", p) + return 0, fmt.Errorf("unable to inspect %q before append: %w", p, propsErr) } if model.SafeDereference(props.ContentLength) > 0 { - return 0, pkgerr.Errorf("refusing to append to %q: blob has content but no committed block list (likely written via Put Blob by another tool)", p) + return 0, fmt.Errorf("refusing to append to %q: blob has content but no committed block list (likely written via Put Blob by another tool)", p) } } @@ -478,7 +508,7 @@ func (b *AzureFileBackend) AppendFile(fr io.Reader, p string) (int64, error) { } if _, err := bb.CommitBlockList(ctx, append(existingIDs, newIDs...), nil); err != nil { - return 0, pkgerr.Wrapf(err, "unable to commit block list for %q", p) + return 0, fmt.Errorf("unable to commit block list for %q: %w", p, err) } return total, nil } @@ -489,7 +519,7 @@ func (b *AzureFileBackend) RemoveFile(p string) error { _, err := b.newBlobClient(p).Delete(ctx, nil) if err != nil && !bloberror.HasCode(err, bloberror.BlobNotFound) { - return pkgerr.Wrapf(err, "unable to remove file %q", p) + return fmt.Errorf("unable to remove file %q: %w", p, err) } return nil } @@ -511,7 +541,7 @@ func (b *AzureFileBackend) ListDirectory(p string) ([]string, error) { for pager.More() { page, err := pager.NextPage(ctx) if err != nil { - return nil, pkgerr.Wrapf(err, "unable to list directory %q", p) + return nil, fmt.Errorf("unable to list directory %q: %w", p, err) } for _, item := range page.Segment.BlobItems { if item.Name == nil { @@ -551,7 +581,7 @@ func (b *AzureFileBackend) ListDirectoryRecursively(p string) ([]string, error) for pager.More() { page, err := pager.NextPage(ctx) if err != nil { - return nil, pkgerr.Wrapf(err, "unable to list directory %q recursively", p) + return nil, fmt.Errorf("unable to list directory %q recursively: %w", p, err) } for _, item := range page.Segment.BlobItems { if item.Name == nil { diff --git a/server/platform/shared/filestore/azurestore_test.go b/server/platform/shared/filestore/azurestore_test.go index 10546fa7155..c3671f2d0b7 100644 --- a/server/platform/shared/filestore/azurestore_test.go +++ b/server/platform/shared/filestore/azurestore_test.go @@ -223,6 +223,7 @@ func azuriteSettings(t *testing.T) FileBackendSettings { return FileBackendSettings{ DriverName: driverAzure, AzureStorageAccount: azuriteWellKnownAccount, + AzureAuthMode: model.AzureAuthModeSharedKey, AzureAccessKey: azuriteWellKnownKey, AzureContainer: "mattermost-test", AzureCloud: model.AzureCloudCustom, @@ -231,6 +232,82 @@ func azuriteSettings(t *testing.T) FileBackendSettings { } } +func TestNewAzureFileBackendAuthMode(t *testing.T) { + base := FileBackendSettings{ + DriverName: driverAzure, + AzureStorageAccount: "anaccount", + AzureContainer: "acontainer", + AzureEndpoint: "localhost:10000", + AzureSSL: false, + AzureRequestTimeoutMilliseconds: 30000, + } + + t.Run("shared_key constructs a client", func(t *testing.T) { + s := base + s.AzureAuthMode = model.AzureAuthModeSharedKey + s.AzureAccessKey = azuriteWellKnownKey + + be, err := NewAzureFileBackend(s) + require.NoError(t, err) + require.NotNil(t, be.client) + }) + + t.Run("default_credential constructs a client without an access key", func(t *testing.T) { + s := base + s.AzureAuthMode = model.AzureAuthModeDefaultCredential + // Intentionally no AzureAccessKey - default credential reads + // identity from the host environment, not config. + + be, err := NewAzureFileBackend(s) + require.NoError(t, err) + require.NotNil(t, be.client) + }) + + t.Run("empty AuthMode is rejected", func(t *testing.T) { + s := base + s.AzureAuthMode = "" + s.AzureAccessKey = azuriteWellKnownKey + + _, err := NewAzureFileBackend(s) + require.Error(t, err) + require.Contains(t, err.Error(), "unknown azure auth mode") + }) + + t.Run("unknown AuthMode is rejected", func(t *testing.T) { + s := base + s.AzureAuthMode = "oauth2" + s.AzureAccessKey = azuriteWellKnownKey + + _, err := NewAzureFileBackend(s) + require.Error(t, err) + require.Contains(t, err.Error(), "unknown azure auth mode") + }) +} + +func TestCheckMandatoryAzureFieldsAuthMode(t *testing.T) { + base := FileBackendSettings{ + AzureStorageAccount: "anaccount", + AzureContainer: "acontainer", + } + + t.Run("shared_key requires access key", func(t *testing.T) { + s := base + s.AzureAuthMode = model.AzureAuthModeSharedKey + s.AzureAccessKey = "" + require.Error(t, s.CheckMandatoryAzureFields()) + + s.AzureAccessKey = "somekey" + require.NoError(t, s.CheckMandatoryAzureFields()) + }) + + t.Run("default_credential does not require access key", func(t *testing.T) { + s := base + s.AzureAuthMode = model.AzureAuthModeDefaultCredential + s.AzureAccessKey = "" + require.NoError(t, s.CheckMandatoryAzureFields()) + }) +} + func TestAzureFileBackendTestSuite(t *testing.T) { suite.Run(t, &FileBackendTestSuite{settings: azuriteSettings(t)}) } diff --git a/server/platform/shared/filestore/filesstore.go b/server/platform/shared/filestore/filesstore.go index 591dec8226a..7afc57f79ad 100644 --- a/server/platform/shared/filestore/filesstore.go +++ b/server/platform/shared/filestore/filesstore.go @@ -67,6 +67,7 @@ type FileBackendSettings struct { AmazonS3UploadPartSizeBytes int64 AmazonS3StorageClass string AzureStorageAccount string + AzureAuthMode string AzureAccessKey string AzureContainer string AzurePathPrefix string @@ -87,6 +88,7 @@ func NewFileBackendSettingsFromConfig(fileSettings *model.FileSettings, enableCo return FileBackendSettings{ DriverName: *fileSettings.DriverName, AzureStorageAccount: *fileSettings.AzureStorageAccount, + AzureAuthMode: *fileSettings.AzureAuthMode, AzureAccessKey: *fileSettings.AzureAccessKey, AzureContainer: *fileSettings.AzureContainer, AzurePathPrefix: *fileSettings.AzurePathPrefix, @@ -127,6 +129,7 @@ func NewExportFileBackendSettingsFromConfig(fileSettings *model.FileSettings, en return FileBackendSettings{ DriverName: *fileSettings.ExportDriverName, AzureStorageAccount: *fileSettings.ExportAzureStorageAccount, + AzureAuthMode: *fileSettings.ExportAzureAuthMode, AzureAccessKey: *fileSettings.ExportAzureAccessKey, AzureContainer: *fileSettings.ExportAzureContainer, AzurePathPrefix: *fileSettings.ExportAzurePathPrefix, @@ -177,7 +180,11 @@ func (settings *FileBackendSettings) CheckMandatoryAzureFields() error { if settings.AzureContainer == "" { return errors.New("missing azure container setting") } - if settings.AzureAccessKey == "" { + // AzureAccessKey is only meaningful for shared-key auth. Default credential + // reads identity from the host environment (managed identity / workload + // identity / service principal env vars / az login), so an empty access key + // is the expected configuration in that mode. + if settings.AzureAuthMode == model.AzureAuthModeSharedKey && settings.AzureAccessKey == "" { return errors.New("missing azure access key setting") } return nil diff --git a/server/public/model/config.go b/server/public/model/config.go index a3c839832d6..57fd7c4cb8a 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -38,6 +38,9 @@ const ( ImageDriverS3 = "amazons3" ImageDriverAzure = "azureblob" + AzureAuthModeSharedKey = "shared_key" + AzureAuthModeDefaultCredential = "default_credential" + // AzureCloudCommercial / AzureCloudGovernment select hardcoded Azure // service endpoints so admins do not have to spell out the suffix // for the well-known clouds. AzureCloudCustom hands control to the @@ -1814,6 +1817,7 @@ type FileSettings struct { AmazonS3UploadPartSizeBytes *int64 `access:"environment_file_storage,write_restrictable,cloud_restrictable"` // telemetry: none AmazonS3StorageClass *string `access:"environment_file_storage,write_restrictable,cloud_restrictable"` // telemetry: none AzureStorageAccount *string `access:"environment_file_storage,write_restrictable,cloud_restrictable"` // telemetry: none + AzureAuthMode *string `access:"environment_file_storage,write_restrictable,cloud_restrictable"` // telemetry: none AzureAccessKey *string `access:"environment_file_storage,write_restrictable,cloud_restrictable"` // telemetry: none AzureContainer *string `access:"environment_file_storage,write_restrictable,cloud_restrictable"` // telemetry: none AzurePathPrefix *string `access:"environment_file_storage,write_restrictable,cloud_restrictable"` // telemetry: none @@ -1840,6 +1844,7 @@ type FileSettings struct { ExportAmazonS3UploadPartSizeBytes *int64 `access:"environment_file_storage,write_restrictable"` // telemetry: none ExportAmazonS3StorageClass *string `access:"environment_file_storage,write_restrictable"` // telemetry: none ExportAzureStorageAccount *string `access:"environment_file_storage,write_restrictable"` // telemetry: none + ExportAzureAuthMode *string `access:"environment_file_storage,write_restrictable"` // telemetry: none ExportAzureAccessKey *string `access:"environment_file_storage,write_restrictable"` // telemetry: none ExportAzureContainer *string `access:"environment_file_storage,write_restrictable"` // telemetry: none ExportAzurePathPrefix *string `access:"environment_file_storage,write_restrictable"` // telemetry: none @@ -1967,6 +1972,10 @@ func (s *FileSettings) SetDefaults(isUpdate bool) { s.AzureStorageAccount = NewPointer("") } + if s.AzureAuthMode == nil { + s.AzureAuthMode = NewPointer(AzureAuthModeSharedKey) + } + if s.AzureAccessKey == nil { s.AzureAccessKey = NewPointer("") } @@ -2069,6 +2078,10 @@ func (s *FileSettings) SetDefaults(isUpdate bool) { s.ExportAzureStorageAccount = NewPointer("") } + if s.ExportAzureAuthMode == nil { + s.ExportAzureAuthMode = NewPointer(AzureAuthModeSharedKey) + } + if s.ExportAzureAccessKey == nil { s.ExportAzureAccessKey = NewPointer("") } @@ -4584,6 +4597,10 @@ func (s *FileSettings) isValid() *AppError { return NewAppError("Config.IsValid", "model.config.is_valid.azure_timeout.app_error", map[string]any{"Value": *s.AzureRequestTimeoutMilliseconds}, "", http.StatusBadRequest) } + if !(*s.AzureAuthMode == AzureAuthModeSharedKey || *s.AzureAuthMode == AzureAuthModeDefaultCredential) { + return NewAppError("Config.IsValid", "model.config.is_valid.azure_auth_mode.app_error", map[string]any{"Value": *s.AzureAuthMode}, "", http.StatusBadRequest) + } + switch *s.AzureCloud { case AzureCloudCommercial, AzureCloudGovernment, AzureCloudCustom: default: @@ -4618,6 +4635,10 @@ func (s *FileSettings) isValid() *AppError { return NewAppError("Config.IsValid", "model.config.is_valid.export_azure_timeout.app_error", map[string]any{"Value": *s.ExportAzureRequestTimeoutMilliseconds}, "", http.StatusBadRequest) } + if !(*s.ExportAzureAuthMode == AzureAuthModeSharedKey || *s.ExportAzureAuthMode == AzureAuthModeDefaultCredential) { + return NewAppError("Config.IsValid", "model.config.is_valid.export_azure_auth_mode.app_error", map[string]any{"Value": *s.ExportAzureAuthMode}, "", http.StatusBadRequest) + } + switch *s.ExportAzureCloud { case AzureCloudCommercial, AzureCloudGovernment, AzureCloudCustom: default: diff --git a/server/public/model/config_test.go b/server/public/model/config_test.go index 40d2bf71189..fa34fa9cc66 100644 --- a/server/public/model/config_test.go +++ b/server/public/model/config_test.go @@ -322,6 +322,46 @@ func TestFileSettingsAzureRequestTimeoutBounds(t *testing.T) { } } +func TestFileSettingsAzureAuthMode(t *testing.T) { + t.Run("defaults to shared_key", func(t *testing.T) { + cfg := &Config{} + cfg.SetDefaults() + require.NotNil(t, cfg.FileSettings.AzureAuthMode) + require.NotNil(t, cfg.FileSettings.ExportAzureAuthMode) + assert.Equal(t, AzureAuthModeSharedKey, *cfg.FileSettings.AzureAuthMode) + assert.Equal(t, AzureAuthModeSharedKey, *cfg.FileSettings.ExportAzureAuthMode) + }) + + t.Run("default_credential is accepted", func(t *testing.T) { + cfg := &Config{} + cfg.SetDefaults() + cfg.FileSettings.AzureAuthMode = NewPointer(AzureAuthModeDefaultCredential) + cfg.FileSettings.ExportAzureAuthMode = NewPointer(AzureAuthModeDefaultCredential) + + assert.Nil(t, cfg.FileSettings.isValid()) + }) + + t.Run("unknown primary mode is rejected", func(t *testing.T) { + cfg := &Config{} + cfg.SetDefaults() + cfg.FileSettings.AzureAuthMode = NewPointer("oauth2") + + err := cfg.FileSettings.isValid() + require.NotNil(t, err) + assert.Equal(t, "model.config.is_valid.azure_auth_mode.app_error", err.Id) + }) + + t.Run("unknown export mode is rejected", func(t *testing.T) { + cfg := &Config{} + cfg.SetDefaults() + cfg.FileSettings.ExportAzureAuthMode = NewPointer("oauth2") + + err := cfg.FileSettings.isValid() + require.NotNil(t, err) + assert.Equal(t, "model.config.is_valid.export_azure_auth_mode.app_error", err.Id) + }) +} + func TestFileSettingsAzureCloudValidation(t *testing.T) { t.Run("unknown cloud values are rejected", func(t *testing.T) { cases := []struct { diff --git a/webapp/channels/src/components/admin_console/admin_definition.tsx b/webapp/channels/src/components/admin_console/admin_definition.tsx index d2d7ab8fd5a..0abdea693fd 100644 --- a/webapp/channels/src/components/admin_console/admin_definition.tsx +++ b/webapp/channels/src/components/admin_console/admin_definition.tsx @@ -138,6 +138,8 @@ export {it}; const FILE_STORAGE_DRIVER_LOCAL = 'local'; const FILE_STORAGE_DRIVER_S3 = 'amazons3'; const FILE_STORAGE_DRIVER_AZURE = 'azureblob'; +const AZURE_AUTH_MODE_SHARED_KEY = 'shared_key'; +const AZURE_AUTH_MODE_DEFAULT_CREDENTIAL = 'default_credential'; const AZURE_CLOUD_COMMERCIAL = 'commercial'; const AZURE_CLOUD_GOVERNMENT = 'government'; const AZURE_CLOUD_CUSTOM = 'custom'; @@ -1405,6 +1407,28 @@ const AdminDefinition: AdminDefinitionType = { ), isHidden: it.not(it.stateEquals('FileSettings.DriverName', FILE_STORAGE_DRIVER_AZURE)), }, + { + type: 'dropdown', + key: 'FileSettings.AzureAuthMode', + label: defineMessage({id: 'admin.image.azureAuthModeTitle', defaultMessage: 'Azure Authentication:'}), + help_text: defineMessage({id: 'admin.image.azureAuthModeDescription', defaultMessage: '"Shared key" signs requests with the Storage Account access key.\n \n"Default credential (Microsoft Entra ID)" reads the identity from the host environment - managed identity on Azure-hosted deployments, workload identity, service principal env vars, or "az login" for local development. No access key required.'}), // eslint-disable-line formatjs/no-multiple-whitespaces + help_text_markdown: true, + options: [ + { + value: AZURE_AUTH_MODE_SHARED_KEY, + display_name: defineMessage({id: 'admin.image.azureAuthModeSharedKey', defaultMessage: 'Shared key'}), + }, + { + value: AZURE_AUTH_MODE_DEFAULT_CREDENTIAL, + display_name: defineMessage({id: 'admin.image.azureAuthModeDefaultCredential', defaultMessage: 'Default credential (Microsoft Entra ID)'}), + }, + ], + isDisabled: it.any( + it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.ENVIRONMENT.FILE_STORAGE)), + it.not(it.stateEquals('FileSettings.DriverName', FILE_STORAGE_DRIVER_AZURE)), + ), + isHidden: it.not(it.stateEquals('FileSettings.DriverName', FILE_STORAGE_DRIVER_AZURE)), + }, { type: 'text', key: 'FileSettings.AzureAccessKey', @@ -1414,8 +1438,12 @@ const AdminDefinition: AdminDefinitionType = { isDisabled: it.any( it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.ENVIRONMENT.FILE_STORAGE)), it.not(it.stateEquals('FileSettings.DriverName', FILE_STORAGE_DRIVER_AZURE)), + it.not(it.stateEquals('FileSettings.AzureAuthMode', AZURE_AUTH_MODE_SHARED_KEY)), + ), + isHidden: it.any( + it.not(it.stateEquals('FileSettings.DriverName', FILE_STORAGE_DRIVER_AZURE)), + it.not(it.stateEquals('FileSettings.AzureAuthMode', AZURE_AUTH_MODE_SHARED_KEY)), ), - isHidden: it.not(it.stateEquals('FileSettings.DriverName', FILE_STORAGE_DRIVER_AZURE)), }, { type: 'text', @@ -1724,6 +1752,28 @@ const AdminDefinition: AdminDefinitionType = { ), isHidden: it.any(it.not(it.stateEquals('FileSettings.ExportDriverName', FILE_STORAGE_DRIVER_AZURE)), it.stateEquals('FileSettings.DedicatedExportStore', false)), }, + { + type: 'dropdown', + key: 'FileSettings.ExportAzureAuthMode', + label: defineMessage({id: 'admin.image.azureAuthModeTitle', defaultMessage: 'Azure Authentication:'}), + help_text: defineMessage({id: 'admin.image.azureAuthModeDescription', defaultMessage: '"Shared key" signs requests with the Storage Account access key.\n \n"Default credential (Microsoft Entra ID)" reads the identity from the host environment - managed identity on Azure-hosted deployments, workload identity, service principal env vars, or "az login" for local development. No access key required.'}), // eslint-disable-line formatjs/no-multiple-whitespaces + help_text_markdown: true, + options: [ + { + value: AZURE_AUTH_MODE_SHARED_KEY, + display_name: defineMessage({id: 'admin.image.azureAuthModeSharedKey', defaultMessage: 'Shared key'}), + }, + { + value: AZURE_AUTH_MODE_DEFAULT_CREDENTIAL, + display_name: defineMessage({id: 'admin.image.azureAuthModeDefaultCredential', defaultMessage: 'Default credential (Microsoft Entra ID)'}), + }, + ], + isDisabled: it.any( + it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.ENVIRONMENT.FILE_STORAGE)), + it.stateEquals('FileSettings.DedicatedExportStore', false), + ), + isHidden: it.any(it.not(it.stateEquals('FileSettings.ExportDriverName', FILE_STORAGE_DRIVER_AZURE)), it.stateEquals('FileSettings.DedicatedExportStore', false)), + }, { type: 'text', key: 'FileSettings.ExportAzureAccessKey', @@ -1733,8 +1783,13 @@ const AdminDefinition: AdminDefinitionType = { isDisabled: it.any( it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.ENVIRONMENT.FILE_STORAGE)), it.stateEquals('FileSettings.DedicatedExportStore', false), + it.not(it.stateEquals('FileSettings.ExportAzureAuthMode', AZURE_AUTH_MODE_SHARED_KEY)), + ), + isHidden: it.any( + it.not(it.stateEquals('FileSettings.ExportDriverName', FILE_STORAGE_DRIVER_AZURE)), + it.stateEquals('FileSettings.DedicatedExportStore', false), + it.not(it.stateEquals('FileSettings.ExportAzureAuthMode', AZURE_AUTH_MODE_SHARED_KEY)), ), - isHidden: it.any(it.not(it.stateEquals('FileSettings.ExportDriverName', FILE_STORAGE_DRIVER_AZURE)), it.stateEquals('FileSettings.DedicatedExportStore', false)), }, { type: 'text', diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index 3d675e0dad6..8105cc56f49 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -1507,6 +1507,10 @@ "admin.image.azureAccessKeyDescription": "The shared key for your Azure Storage account.", "admin.image.azureAccessKeyExample": "E.g.: \"9MZbtYgfq18PJ8PbRaJ5u91IH8izHvReTbcuQzMl+So=\"", "admin.image.azureAccessKeyTitle": "Azure Storage Account Key:", + "admin.image.azureAuthModeDefaultCredential": "Default credential (Microsoft Entra ID)", + "admin.image.azureAuthModeDescription": "\"Shared key\" signs requests with the Storage Account access key.\n \n\"Default credential (Microsoft Entra ID)\" reads the identity from the host environment - managed identity on Azure-hosted deployments, workload identity, service principal env vars, or \"az login\" for local development. No access key required.", + "admin.image.azureAuthModeSharedKey": "Shared key", + "admin.image.azureAuthModeTitle": "Azure Authentication:", "admin.image.azureCloudCommercial": "Azure Commercial", "admin.image.azureCloudCustom": "Custom Endpoint", "admin.image.azureCloudDescription": "The Azure cloud to connect to. Choose \"Azure Commercial\" or \"Azure Government\" to use the well-known endpoint for that cloud; only the storage account name is required. Choose \"Custom Endpoint\" to point at an arbitrary host such as Azurite, a reverse proxy, or any other Azure cloud (for example Azure China).",