Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Azure Blob Storage to follow MS security best practices #16336

Open
ShaneCourtrille opened this issue Jun 17, 2024 · 8 comments
Open

Update Azure Blob Storage to follow MS security best practices #16336

ShaneCourtrille opened this issue Jun 17, 2024 · 8 comments
Milestone

Comments

@ShaneCourtrille
Copy link
Contributor

ShaneCourtrille commented Jun 17, 2024

Is your feature request related to a problem? Please describe.

We are currently implementing Microsoft best practices for managing access to Azure Blob Storage and we've ran into a problem with the existing OC BlobFileStore.cs due to the fact we cannot easily override how the BlobContainerClient is created.

Describe the solution you'd like

OC BlobFileStore should follow MS best practices which require the usage of either a Microsoft Entra ID or User delegation SAS as a fallback for those who aren't using Entra.

Describe alternatives you've considered

Our current solution involves adding a new interface which allows us to control how the client is being created like this. This works well enough for those who are heavily customizing OC but doesn't seem like an appropriate solution for the general usage.

public BlobFileStore(IBlobStorageOptions options, IClock clock, IContentTypeProvider contentTypeProvider, IBlobContainerClientFactory blobContainerClientFactory)
        {
            _options = options;
            _clock = clock;
            _contentTypeProvider = contentTypeProvider;
            _blobContainer = blobContainerClientFactory.Create(_options);

            if (!String.IsNullOrEmpty(_options.BasePath))
            {
                _basePrefix = NormalizePrefix(_options.BasePath);
            }
        }
@Piedone
Copy link
Member

Piedone commented Jun 17, 2024

Could you please follow the issue template? Otherwise, we'd need to have a conversation about the same questions :).

@ShaneCourtrille ShaneCourtrille changed the title Allow applications to define how BlobContainerClients are created for Azure Blob Storage Update Azure Blob Storage to follow MS security best practices Jun 18, 2024
@Piedone
Copy link
Member

Piedone commented Jun 18, 2024

Thanks! What exactly would be a solution for you? I.e. what needs to be coded? New configuration options for BlobFileStore? BTW you can override IMediaFileStore and use a custom one (if this is not what you already do in the code snippet).

Copy link
Contributor

github-actions bot commented Jul 4, 2024

It seems that this issue didn't really move for quite a while despite us asking the author for further feedback. Is this something you'd like to revisit any time soon or should we close? Please reply.

@github-actions github-actions bot added the stale label Jul 4, 2024
@ShaneCourtrille
Copy link
Contributor Author

@Piedone We are going to customize the code so that we can provide our own IBlobContainerClientFactory but it would be better if OC followed MS best practices by supporting Microsoft Entra ID or User delegation SAS token usage out of the box.

@Piedone
Copy link
Member

Piedone commented Jul 11, 2024

OK, thanks, so we're talking about adding new configuration options.

@sebastienros
Copy link
Member

This should be done for all Azure Services, ideally support all the auth schemes that are supported by each service. (Microsoft Entra ID, managed user id, connection string, ...)

@sebastienros sebastienros added this to the 2.x milestone Jul 11, 2024
Copy link
Contributor

We triaged this issue and set the milestone according to the priority we think is appropriate (see the docs on how we triage and prioritize issues).

This indicates when the core team may start working on it. However, if you'd like to contribute, we'd warmly welcome you to do that anytime. See our guide on contributions here.

@sebastienros
Copy link
Member

@MikeAlhayek really wants this to be consistent across features. Maybe we could have a common Authentication section (Azure SDK has a similar thing) that is bound for each feature, and configure the clients with these.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants