Support Microsoft Entra ID authentication to Azure. #4126

teo-tsirpanis · 2023-06-12T16:26:15Z

To authenticate to Azure, we currently support shared keys and SAS tokens. The former have the disadvantage of being discouraged by Microsoft and the latter have the disadvantage of having a usually short expiry time (edit: and being impossible to individually revoke).

This PR adds support for Microsoft Entra ID authentication (formerly known as Azure Active Directory), which is what Microsoft recommends. It will be used for HTTPS endpoints if neither a shared key or SAS token are specified in the config. We use the ChainedTokenCredential class from the azure-identity-cpp package, and try to get credentials from the following sources in order:

Environment variables
The Azure CLI (for local development)
Managed identities (apps hosted on Azure can get automatically authenticated)
Workload identities (for apps hosted in Kubernetes)

At initialization time we try to fetch an Active Directory token. If all three credential sources fail, we fall back to anonymous authentication.

TODO

Verify that it works locally.
Verify that accessing a publicly available blob with Entra ID credentials that do not grant access to it works.
- If it doesn't, we have to remove the anonymous access fallback and add a config option (whether it will be opt-in or out is TBD).

TYPE: FEATURE
DESC: Support Microsoft Entra ID authentication to Azure.

shortcut-integration · 2023-06-12T16:26:19Z

This pull request has been linked to Shortcut Story #25944: Support Active Directory authentication to Azure..

…ails.

With the conda feedstock being ready to get the Azure SDK from conda-forge (conda-forge/tiledb-feedstock#205), there is little value in adding yet another external project for `azure-identity-cpp`, to keep supporting Azure with the legacy external projects system.

…ntity credentials as well.

KiterLuc · 2024-02-28T08:37:06Z

Closing for now. We don't have time to test the authentication properly at the moment. It's still a good change and we can re-open this PR when we prioritize the shortcut story.

teo-tsirpanis · 2024-03-12T22:07:39Z

Validated successfully on the real Azure. I followed these steps:

Create an Azure Blob Storage account.
Add a role assignment of the "Storage Blob Data Contributor" role on the account to your user.
Log in to Azure by running az login and select your subscription.
Configure the tests to use your account name here, here and here¹. Remove the custom endpoint and storage key options.
Run tiledb_unit [vfs][uri][file_io].

~~Please don't review yet; I will do some additional changes tomorrow.~~

Why is the VFS configuration split in three places? --vfs azure is also broken; I had to change this line to run only Azure tests. ↩

KiterLuc

I would like to see the same concerns addressed as in the GCS PR here.

In addition, I see that this is changing a lot on the build, probably because we now need more components and it's easier to get those from VCPKG. I need confirmation that this will not end up breaking dependencies like SOMA, mariaBD before we merge this change.

teo-tsirpanis · 2024-03-13T12:38:06Z

tiledb/sm/filesystem/azure.cc

+      try {
+        ::Azure::Core::Credentials::TokenCredentialOptions cred_options;
+        cred_options.Retry = options.Retry;
+        cred_options.Transport = options.Transport;


This line will protect us from bugs similar to the AssumeRole bugs that were fixed by #4616 and #4641. options.Transport is created some lines earlier with the user-provided SSL config.

teo-tsirpanis · 2024-03-13T12:49:23Z

tiledb/sm/filesystem/azure.cc

+        // If a token is not available we wouldn't know it until we make a
+        // request and it would be too late. Try getting a token, and if it
+        // fails fall back to anonymous authentication.


For simplicity, instead of trying to fetch a token during initialization, I am thinking of enabling Entra ID authentication by default unless:

A storage key or SAS token is specified.

A custom endpoint is specified with the http protocol.

A new config option vfs.azure.disable_token_auth is enabled.

What do you think?

Most existing use cases would not be broken since they already specify a shared key.

@robertbindar @KiterLuc thoughts on this? And if we don't add the config option to disable it now, can we do it in a subsequent PR/release?

@teo-tsirpanis I think it's better if we move this work in incremental steps. The current implementation you have here keeps the behavior we currently have intact. I'd go with this, then try to do config+no token in a PR past this release.

+1 on Robert's comment. @teo-tsirpanis, we can file a follow up story.

teo-tsirpanis · 2024-03-13T13:37:39Z

tiledb/CMakeLists.txt

-    find_package(AzureCore_EP REQUIRED)
-    find_package(AzureStorageCommon_EP REQUIRED)
-    find_package(AzureStorageBlobs_EP REQUIRED)
-    if (NOT WIN32)
-      if (NOT TARGET LibXml2::LibXml2)
-        find_package(LibXml2 REQUIRED)
-      endif()
-    endif()
-    target_link_libraries(TILEDB_CORE_OBJECTS_ILIB
-            INTERFACE
-            Azure::azure-storage-blobs
-            Azure::azure-storage-common
-            Azure::azure-core)
-    # Link xml after Azure to allow symbol resolution on all platforms
-    if (NOT WIN32)
-      target_link_libraries(TILEDB_CORE_OBJECTS_ILIB INTERFACE LibXml2::LibXml2)
-    endif()
-  endif()


This part is used only if vcpkg is disabled, which no longer happens in first-party code.

Regarding the new dependency, Conda will need to be updated to add azure-identity-cpp in the requirements, and an empty vcpkg port overlay.

robertbindar

This PR looks good to me, minor code structure suggestions.
I would be more comfortable with all the build changes here (removal of non-vcpkg cmake code) if #4570 would already be in and things would scream if we'd still have any builds with -DTILEDB_VCPKG=OFF.
I'll reproduce asap Theo's steps for manually testing this, in the meantime it'd be lovely to get a confirmation on this statement, @ihnorton can you help out?

Regarding the new dependency, Conda will need to be updated to add azure-identity-cpp in the requirements, and an empty vcpkg port overlay.

robertbindar · 2024-03-21T14:54:46Z

tiledb/sm/filesystem/azure.cc

@@ -79,9 +80,12 @@ std::string get_config_with_env_fallback(
 }

 std::string get_blob_endpoint(
-    const Config& config, const std::string& account_name) {


definitely a new function for checking if there is a sas token.

robertbindar · 2024-03-21T15:35:59Z

tiledb/sm/filesystem/azure.cc

@@ -192,15 +196,57 @@ Azure::AzureClientSingleton::get(const AzureParameters& params) {
              make_shared<::Azure::Storage::StorageSharedKeyCredential>(
                  HERE(), params.account_name_, params.account_key_),
              options));
-    } else {


we should return early here the client if not nullptr, it'll help us get rid of the some if levels.

KiterLuc

We'll let that one go through for now as we need to validate the build changes early. Testing will still be required to ship this.

KiterLuc · 2024-03-22T13:51:29Z

Merging so that we can test the build changes early. @robertbindar will validate further and make fixes if required before we ship.

teo-tsirpanis added the blocked label Jun 14, 2023

teo-tsirpanis force-pushed the azure-ad branch 2 times, most recently from 41f112b to c80b8bd Compare June 16, 2023 14:33

teo-tsirpanis removed the blocked label Jun 16, 2023

teo-tsirpanis force-pushed the azure-ad branch from c80b8bd to 06e9e82 Compare June 19, 2023 13:58

teo-tsirpanis mentioned this pull request Aug 18, 2023

Install Azure SDK C++ libraries from conda conda-forge/tiledb-feedstock#205

Merged

4 tasks

jdblischak mentioned this pull request Aug 23, 2023

Build for more openssl variants conda-forge/azure-identity-cpp-feedstock#2

Merged

4 tasks

KiterLuc added the pending-2.19 label Oct 6, 2023

teo-tsirpanis force-pushed the azure-ad branch 2 times, most recently from 6c4b189 to 91ae487 Compare October 23, 2023 20:48

teo-tsirpanis changed the title ~~Support Active Directory authentication to Azure.~~ Support Microsoft Entra ID authentication to Azure. Nov 16, 2023

KiterLuc removed the pending-2.19 label Dec 19, 2023

teo-tsirpanis added 11 commits February 8, 2024 14:50

Install azure-identity-cpp.

fbe1846

Support Active Directory authentication to Azure Blob Storage.

4d0bf76

Try getting a token and fall back to anonymous authentication if it f…

ca60f0c

…ails.

Correctly set the AD scopes when trying to get a token.

9c46fb3

Remove some duplicate code.

82f54fa

Explicitly use ChainedTokenCredential.

38a49fa

Fix missing return.

91c5224

Update azure-identity-cpp to version 1.6.0 and try using workload ide…

2828ff6

…ntity credentials as well.

Azure Active Directory has been renamed to Microsoft Entra ID.

b6fcaa1

Add a find_dependency(azure-identity-cpp).

b79a945

teo-tsirpanis force-pushed the azure-ad branch from 14ed0b3 to b79a945 Compare February 8, 2024 12:57

Pass transport and retry options when creating the credentials.

b5533c0

KiterLuc closed this Feb 28, 2024

teo-tsirpanis reopened this Mar 12, 2024

KiterLuc requested a review from robertbindar March 12, 2024 21:04

Merge branch 'dev' into azure-ad

4af2a82

teo-tsirpanis force-pushed the azure-ad branch from 28bd0c7 to 4af2a82 Compare March 12, 2024 22:07

KiterLuc suggested changes Mar 13, 2024

View reviewed changes

teo-tsirpanis commented Mar 13, 2024

View reviewed changes

teo-tsirpanis marked this pull request as ready for review March 13, 2024 13:38

robertbindar mentioned this pull request Mar 18, 2024

Support GCS service account impersonation et. al. #4799

Merged

robertbindar approved these changes Mar 21, 2024

View reviewed changes

Review comments from @robertbindar.

705bfe0

KiterLuc force-pushed the azure-ad branch from daf8d8f to 705bfe0 Compare March 22, 2024 10:35

KiterLuc approved these changes Mar 22, 2024

View reviewed changes

KiterLuc merged commit ceee95e into TileDB-Inc:dev Mar 22, 2024
54 checks passed

teo-tsirpanis deleted the azure-ad branch March 22, 2024 14:30

This was referenced Mar 25, 2024

Nighly feedstock build failed TileDB-Inc/conda-forge-nightly-controller#69

Closed

To-do list for 2.22 release conda-forge/tiledb-feedstock#268

Closed

teo-tsirpanis mentioned this pull request May 13, 2024

faketime tests stall when S3 support is enabled #4956

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Microsoft Entra ID authentication to Azure. #4126

Support Microsoft Entra ID authentication to Azure. #4126

teo-tsirpanis commented Jun 12, 2023 •

edited

Loading

shortcut-integration bot commented Jun 12, 2023

KiterLuc commented Feb 28, 2024

teo-tsirpanis commented Mar 12, 2024 •

edited

Loading

KiterLuc left a comment

teo-tsirpanis Mar 13, 2024

teo-tsirpanis Mar 13, 2024

teo-tsirpanis Mar 13, 2024

teo-tsirpanis Mar 21, 2024

robertbindar Mar 22, 2024

KiterLuc Mar 22, 2024

teo-tsirpanis Mar 13, 2024

robertbindar left a comment

robertbindar Mar 21, 2024

robertbindar Mar 21, 2024

KiterLuc left a comment

KiterLuc commented Mar 22, 2024

Support Microsoft Entra ID authentication to Azure. #4126

Support Microsoft Entra ID authentication to Azure. #4126

Conversation

teo-tsirpanis commented Jun 12, 2023 • edited Loading

TODO

shortcut-integration bot commented Jun 12, 2023

KiterLuc commented Feb 28, 2024

teo-tsirpanis commented Mar 12, 2024 • edited Loading

Footnotes

KiterLuc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robertbindar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KiterLuc left a comment

Choose a reason for hiding this comment

KiterLuc commented Mar 22, 2024

teo-tsirpanis commented Jun 12, 2023 •

edited

Loading

teo-tsirpanis commented Mar 12, 2024 •

edited

Loading