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

Azure DefaultAzureCredential() throwing exception during get_user_delegation_key() #1375

Open
andyp05 opened this issue Apr 15, 2024 · 21 comments

Comments

@andyp05
Copy link

andyp05 commented Apr 15, 2024

Running Django 5.02, python 3.12.2 on Windows Server and IIS (hypercorn) and django-storages 1.14.2.

Have managed identity setup and working fine using DefaultAzureCredential() to get secrets in settings.py
Azure Storage using DefaultAzureCredential() manually to upload files not using django models is working fine.

However, django-storages is throwing an exception in get_user_delegation_key() when reading an ImageField from a model.

AZURE_CLIENT_ID environment variable is set to client id of managed identity. Also tried putting this in call to DefaultAzureCredential()

In settings.py:

from azure.identity import DefaultAzureCredential
from azure.keyvault.secrets import SecretClient
...
AZURE_ACCOUNT_NAME = client.get_secret('AZURE-STORAGE-ACCOUNT-NAME').value
AZURE_CONTAINER = client.get_secret('DJANGO-AZURE-CONTAINER').value

STORAGES = {
    "default": {
        "BACKEND": "storages.backends.azure_storage.AzureStorage",
        "OPTIONS": {
            "token_credential": DefaultAzureCredential(),
            "account_name": AZURE_ACCOUNT_NAME,
            "azure_container": AZURE_CONTAINER,
        },
    },
}

The following IS working for Key Vault access:

KVUri = f'https://{os.getenv("AZURE_KEY_VAULT_NAME")}.vault.azure.net'
client = SecretClient(vault_url=KVUri, credential=DefaultAzureCredential())
SECRET_KEY = client.get_secret('DJANGO-SECRET-KEY').value

It is not an DefaultAzureCredential(), Access Control, or Firewall issue since I can upload and read files from the app not using django models from another section of my code. Code snippet that works:

            blob_service_client = BlobServiceClient(settings.AZURE_DOMAIN_FOR_SAS, credential=DefaultAzureCredential())
            user_delegation_key = blob_service_client.get_user_delegation_key(
                key_start_time=datetime.datetime.now(datetime.UTC),
                key_expiry_time=datetime.datetime.now(datetime.UTC) + datetime.timedelta(seconds=key_time_expiration)
            )

Any ideas would be appreciated.

Thanks

@andyp05
Copy link
Author

andyp05 commented Apr 15, 2024

Forgot to add the exception:
ValueError("Either user_delegation_key or account_key must be provided.")

@andyp05
Copy link
Author

andyp05 commented Apr 15, 2024

It seems once I switched to DefaultAzureCredential() and disabled Access Tokens in Azure, tag use of {{ imgfield.url }} stopped working and threw an exception. The workaround was to create a template tag to add the sas myself.

{{ f.doc.value|create_az_url }}  <--works
{{ f.doc.url }}   <-- stopped working

@jschneier
Copy link
Owner

@tonybaloney do you have any ideas, you contributed a bunch of docs recently? In general, for Azure I'm relying on the community as I've never used it.

@tonybaloney
Copy link
Contributor

I think it's because the code assumes you use SAS tokens and if you have an expiry it'll try and write a SAS token back to the connection object. Also the get_user_delegation_key doesn't return anything if you don't use a token credential.

https://github.com/jschneier/django-storages/blob/master/storages/backends/azure_storage.py#L319-L331
and
https://github.com/jschneier/django-storages/blob/master/storages/backends/azure_storage.py#L220-L223

Both of these should be fixed.

I've got on my todo list this coming week to convert a Django template app to System Assigned Managed Identity using DefaultAzureCredential so I'll add this property (.url) to the test as well and reproduce it so I can contribute a patch from a working demo.

@jschneier please feel free to tag me in any Azure issues like this.

@andyp05 hope you don't mind waiting a couple of days for a fix, but I can't see an obvious workaround without modifying the code.

@tonybaloney
Copy link
Contributor

tonybaloney commented Apr 21, 2024

Also @andyp05 just to check, what I'm assuming you're trying to do is:

  1. Have the Django service use DefaultAzureCredential to manage the blob storage (fetch, index, upload new blobs) with managed identity
  2. Generate URLs in the templates with or without SAS tokens in them, depending on the configuration

@andyp05
Copy link
Author

andyp05 commented Apr 21, 2024

  1. Yes. I am using a managed identity in production and an App in development. Both use DefaultAzureCredential() to authenticate. I am having the same issues in prod and development. Creating a new blob works. Reading a blob using a form and trying to display an image in a template does not.

  2. Generate a valid url with SAS when an image is pulled from a model using a Form. {{image.url}} used to work in a tag when using token authorization. It stopped working when I switched to DefaultAzureCredential().

I switched to DefaultAzureCredential (and Managed Identity) to turn off 'Allow storage account key access' to improve security.

Hope that is clear and helpful.

@tonybaloney
Copy link
Contributor

Can you share the full settings? That would help me verify we're checking the same scenario

Also, are your storage containers set to Private, Anonymous (Blob) or Anonymous (Container)?
Have you generated a SAS token and configured it in settings.py?

@tonybaloney
Copy link
Contributor

tonybaloney commented Apr 21, 2024

Which role have you assigned to the Managed Identity?

User Delegation Key generation is a special role "Storage Blob Delegator" (db58b8e5-c6ad-4a2a-8342-4190687cbf4a), but this control is also available in "Storage Blob Data Contributor"

I'm looking at the error you're getting and it seems to be because your Managed Identity doesn't have that role (that's the most likely cause)

@andyp05
Copy link
Author

andyp05 commented Apr 22, 2024

I do have that role. Storing images to an ImageField works. The problem is when I try to view the image in a template using a form, the <img src="{{ f.image.url }}"> throws an exception. It never did.

I created a template tag to create the SAS and that works fine as a workaround. The SAS is generating fine, so it is not an authorization issue.

The relevant code looks like:

      blob_service_client = BlobServiceClient(settings.AZURE_DOMAIN_FOR_SAS, credential=DefaultAzureCredential())

      user_delegation_key = blob_service_client.get_user_delegation_key(
          key_start_time=now,
          key_expiry_time=now + datetime.timedelta(seconds=key_time_expiration + 60)
      )
      blob_sas = generate_blob_sas(account_name=settings.AZURE_ACCOUNT_NAME,
                                   container_name=settings.AZURE_CONTAINER,
                                   blob_name=filename,
                                   user_delegation_key=user_delegation_key,
                                   permission=BlobSasPermissions(read=True, create=create),
                                   expiry=expire)
{{ f.doc.value | create_az_url }}  <--works
{{ f.doc.url }}   <-- stopped working

@tonybaloney
Copy link
Contributor

Ok, gotcha. I can't reproduce this but I'm going to configure a standalone template to try and reproduce it in another tenant.

Here's the demo app I've written to test it https://github.com/tonybaloney/django-storages/blob/demo_stire/demo/app/templates/home.html#L9

I did notice that setting an expiry value is really important to the code flow and forcing it down the path of fetching a user delegation key:
https://github.com/tonybaloney/django-storages/blob/demo_stire/demo/demo/settings.py#L135

Here are all the storage account settings I've used https://github.com/tonybaloney/django-storages/blob/demo_stire/docs/backends/_resources/azure.bicep

I'm testing this locally by using az login first then DefaultAzureCredential will pick up the Azure CLI Extension credential session when running in a debugger like VS code.

If you spot anything else different in those settings lmk

@andyp05
Copy link
Author

andyp05 commented Apr 22, 2024

In a sample template to allow user upload of an image, using the following:

<div class="col-md-9">{{ form.logo }}</div>
I see the input widget and if I add a file, it will upload correctly.
image

If I add the ".url" to display an existing image, it returns nothing.
<img src="{{ form.logo.url }}">

If in the view I construct the URL and SAS using the code snippet I showed above, it displays fine.

I did not have "expiration_secs": 3600, in settings. I added it, but it did not fix the issue.

@tonybaloney
Copy link
Contributor

tonybaloney commented Apr 22, 2024

I've tried a template with an edit form, a custom model with an image field and I still can't reproduce this error with the .url property. I tried creating a model with an ImageField and editing it via the admin portal and again via a custom form view.

I've got a full E2E demo app that's compatible with AZD (Azure Developer CLI) here that has managed identity and the same settings for the storage containers we discussed. Everything works fine.

https://github.com/tonybaloney/django-on-azure

Without seeing specifics of your setup I'm stuck trying to figure out a proper fix or documentation on how to avoid this. I'd also like to add a test to the library to make sure nobody else hits this issue.

I have office hours if you can email me at anthonyshaw at microsoft dot com

@andyp05
Copy link
Author

andyp05 commented Apr 22, 2024

I think I found the issue.
I am using a custom domain and have AZURE_CUSTOM_DOMAIN set in settings.py.
If I remove this setting, the full url is created with the SAS token.

@andyp05
Copy link
Author

andyp05 commented Apr 24, 2024

I found the existing issues opened on this. I had not put together the custom domain and the exception when I first reported the problem. As a workaround of the custom domain and proxy issue, I have implemented another settings variable called AZURE_URL_FOR_SAS for the purpose of getting the get_user_delegation_key and sas. However, I am returning the full URL with AZURE_CUSTOM_DOMAIN. This way, the user sees the custom domain and it is proxied by the CDN. Is this something that could be added to this package or am I missing something.

1 similar comment
@andyp05
Copy link
Author

andyp05 commented Apr 24, 2024

I found the existing issues opened on this. I had not put together the custom domain and the exception when I first reported the problem. As a workaround of the custom domain and proxy issue, I have implemented another settings variable called AZURE_URL_FOR_SAS for the purpose of getting the get_user_delegation_key and sas. However, I am returning the full URL with AZURE_CUSTOM_DOMAIN. This way, the user sees the custom domain and it is proxied by the CDN. Is this something that could be added to this package or am I missing something.

@tonybaloney
Copy link
Contributor

Are you using Azure Front Door, Azure CDN or something else that requires the custom domain

@andyp05
Copy link
Author

andyp05 commented Apr 25, 2024

Was using Cloudflare as a proxy since all my domains are there. I tried turning off the proxy service (and with it all caching) and doing straight DNS and the problem remained.

I am using a custom domain for consistency of the user interface when a user clicks on an image, I want them to see my url. Another benefit is I get caching and DDOS protections,

I saw an issue where DNS providers convert HEAD calls to GET calls at the edge. Some say this is the problem. But turning off all Cloudflare performance and caching by not using the proxy should have disabled all conversions. Still did not work.

Could a solution be that in _get_service_client() use the base url and for the actual https://.... url formation use the custom domain.
Two settings:
AZURE_SAS_DOMAIN
AZURE_CUSTOM_DOMAIN
If using a custom domain, both settings are required.

I am doing this manually using template tags and it seems to work fine.
Thanks

@dimbleby
Copy link
Contributor

We already have AZURE_ACCOUNT_NAME and methods for getting clients that use that, surely there is no need for additional configuration AZURE_SAS_DOMAIN?

@andyp05
Copy link
Author

andyp05 commented Apr 27, 2024

I must be missing something. I have AZURE_ACCOUNT_NAME setup with the storage account name. Everything worked fine until I setup a custom domain and defined AZURE_CUSTOM_DOMAIN

It seems the code recognizes the custom domain and uses it in the call to _get_service_client()

You mention you already have methods to use AZURE_ACCOUNT_NAME. How do I get django_storages to use them.

@dimbleby
Copy link
Contributor

get_user_delegation_key uses the custom service client, isn't what you want that it simply doesn't do that?

@tonybaloney
Copy link
Contributor

Tested with a custom domain by deploying an Azure Frontdoor CDN profile:

STORAGES = {
    "default": {
        "BACKEND": "storages.backends.azure_storage.AzureStorage",
        "OPTIONS": {
            "token_credential": DefaultAzureCredential(),
            "account_name": "xxxxx",
            "azure_container": "media",
            "expiration_secs": 3600,
            "custom_domain":  "antdjangocdntest-xxx.b02.azurefd.net"
        }
    },
    "staticfiles": {
        "BACKEND": "storages.backends.azure_storage.AzureStorage",
        "OPTIONS": {
            "token_credential": DefaultAzureCredential(),
            "account_name": "xxxxx",
            "azure_container": "static",
            "expiration_secs": 3600,
            "custom_domain": "antdjangocdntest-xxx.b02.azurefd.net"
        },
    },
}

Everything works as expected, including the .url attributes on image fields and a custom upload. Still not able to reproduce this bug, I wonder if SAS domains detect Azure FD URLs/Azure CDN since those are the only configurations I've ever tried.

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

No branches or pull requests

4 participants