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

[FEATURE REQ] New ProtectKeysWithAzureKeyVault Extension Method #37903

Open
nquandt opened this issue Jul 28, 2023 · 10 comments
Open

[FEATURE REQ] New ProtectKeysWithAzureKeyVault Extension Method #37903

nquandt opened this issue Jul 28, 2023 · 10 comments
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Extensions ASP.NET Core extensions feature-request This issue requires a new behavior in the product in order be resolved.
Milestone

Comments

@nquandt
Copy link

nquandt commented Jul 28, 2023

Library name

Azure.Extensions.AspNetCore.DataProtection.Keys

Please describe the feature.

Requiring an extensions method to configure the keyIdentifier via ServiceProvider. I inject the key identifier into DI and require a way to retrieve it in order to setup ProtectKeysWithAzureKeyVault

ProtectKeysWithAzureKeyVault(this IDataProtectionBuilder builder,  Func<IServiceProvider, string> keyIdentifierFactory, Func<IServiceProvider, TokenCredential> tokenCredentialFactory)

public static IDataProtectionBuilder ProtectKeysWithAzureKeyVault(this IDataProtectionBuilder builder, Uri keyIdentifier, TokenCredential tokenCredential)

@github-actions github-actions bot added Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Extensions ASP.NET Core extensions needs-team-triage Workflow: This issue needs the team to triage. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Jul 28, 2023
@jsquire jsquire self-assigned this Jul 28, 2023
@jsquire
Copy link
Member

jsquire commented Jul 28, 2023

Hi @nquandt. Thank you for reaching out and for your suggestion. Can you help us understand the end-to-end scenario?

Since DI and data projection registrations both operate on the same IServicesCollection they typically are done in the same scope. I'm not sure that I follow why would you need to register the identifier and then retrieve it at some indeterminate point in the future rather than providing it to both data protection and DI when registering. Likewise, why would you need to defer creating the credential?

@jsquire jsquire added needs-author-feedback Workflow: More information is needed from author to address the issue. feature-request This issue requires a new behavior in the product in order be resolved. and removed question The issue doesn't require a change to the product in order to be resolved. Most issues start as that needs-team-triage Workflow: This issue needs the team to triage. labels Jul 28, 2023
@github-actions
Copy link

Hi @nquandt. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

@nquandt
Copy link
Author

nquandt commented Aug 3, 2023

I get my identifier from a config map in kubernetes.. and currently the way I load that setting into my app is via an "Options" object in my DI container. I currently have a work around where I BuildServiceProvider() before I do the DataProtection setup, but this seems weird.

@github-actions github-actions bot added needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed needs-author-feedback Workflow: More information is needed from author to address the issue. labels Aug 3, 2023
@jsquire
Copy link
Member

jsquire commented Aug 10, 2023

Thank you for the additional context, @nquandt. Forgive me, but I'm not sure that I'm following the flow of what that looks like. Would you be able to share a code snippet showing how you're initializing DI and registering data protection?

@jsquire jsquire added needs-author-feedback Workflow: More information is needed from author to address the issue. and removed needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team labels Aug 10, 2023
@github-actions
Copy link

Hi @nquandt. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

@github-actions
Copy link

Hi @nquandt, we're sending this friendly reminder because we haven't heard back from you in 7 days. We need more information about this issue to help address it. Please be sure to give us your input. If we don't hear back from you within 14 days of this comment the issue will be automatically closed. Thank you!

@github-actions github-actions bot added the no-recent-activity There has been no recent activity on this issue. label Aug 17, 2023
@nquandt
Copy link
Author

nquandt commented Aug 31, 2023

Sorry I've been slow to respond.

I have an object injected into my DI container that holds my key vault information. (this gets injected based on some environment variables set in a kube deploy)..

So basically I am just looking for a simple way of setting the keyIdentifier based on data in the di container.

services.AddDataProtection()
                .ProtectKeysWithAzureKeyVault((sp) => $"{sp.GetRequiredService<MyKVOptions>().Url}/keys/dataprotection/", 
                (sp) => {
                    var kvOptions = sp.GetRequiredService<MyKVOptions>();
                    return new ClientSecretCredential(kvOptions.TenantId, kvOptions.ClientId, kvOptions.ClientSecret);
                });            

Also some extra context, I use AzureKeyVault in other ways throughout my application, just not for DataProtection.. so thats why I have that object with my settings I can pass around to other services.

@github-actions github-actions bot added needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed needs-author-feedback Workflow: More information is needed from author to address the issue. no-recent-activity There has been no recent activity on this issue. labels Aug 31, 2023
@jsquire jsquire removed the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Dec 13, 2023
@github-actions github-actions bot added the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Dec 13, 2023
@jsquire jsquire assigned JoshLove-msft and unassigned jsquire Dec 16, 2023
@jsquire
Copy link
Member

jsquire commented Dec 16, 2023

@JoshLove-msft : Please include this in the discussion of the use of factories in our extensions packages. Thanks!

@jsquire jsquire added this to the Backlog milestone Mar 4, 2024
@jsquire
Copy link
Member

jsquire commented Mar 4, 2024

@AlexanderSher: Since you're looking into Extension package feature requests, please include this one as well.

@annelo-msft
Copy link
Member

Evaluating whether this can be addressed as part of #46671

@annelo-msft annelo-msft removed the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Extensions ASP.NET Core extensions feature-request This issue requires a new behavior in the product in order be resolved.
Projects
None yet
Development

No branches or pull requests

5 participants