From 4c6725a5531fe7a813085d6312066ff1591ec04e Mon Sep 17 00:00:00 2001 From: Erick Yondon <8766776+erdembayar@users.noreply.github.com> Date: Fri, 30 Aug 2024 16:31:46 -0700 Subject: [PATCH] Introduce delegation SAS (#10159) --- .../NuGet.Jobs.Common.csproj | 1 + .../StorageAccountExtensions.cs | 69 ++++++++++ .../Extensions/UriExtensions.cs | 43 ++++++ .../CloudBlobCoreFileStorageService.cs | 28 +++- .../Services/CloudBlobWrapper.cs | 68 ++++++--- .../Services/ICoreFileStorageService.cs | 20 ++- .../Services/ISimpleCloudBlob.cs | 18 ++- .../Services/FileSystemFileStorageService.cs | 5 + .../Support/InMemoryCloudBlob.cs | 7 +- .../CloudBlobCoreFileStorageServiceFacts.cs | 130 ++++++++++++++---- 10 files changed, 335 insertions(+), 54 deletions(-) create mode 100644 src/NuGetGallery.Core/Extensions/UriExtensions.cs diff --git a/src/NuGet.Jobs.Common/NuGet.Jobs.Common.csproj b/src/NuGet.Jobs.Common/NuGet.Jobs.Common.csproj index 99fc47c49b..2acb102c93 100644 --- a/src/NuGet.Jobs.Common/NuGet.Jobs.Common.csproj +++ b/src/NuGet.Jobs.Common/NuGet.Jobs.Common.csproj @@ -13,6 +13,7 @@ + diff --git a/src/NuGet.Jobs.Common/StorageAccountExtensions.cs b/src/NuGet.Jobs.Common/StorageAccountExtensions.cs index 1671145bbd..e82aef6eef 100644 --- a/src/NuGet.Jobs.Common/StorageAccountExtensions.cs +++ b/src/NuGet.Jobs.Common/StorageAccountExtensions.cs @@ -4,6 +4,8 @@ using System; using Autofac; using Autofac.Builder; +using Azure.Data.Tables; +using Azure.Identity; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; @@ -103,6 +105,50 @@ public static IRegistrationBuilder>().Value; + return CreateTableServiceClientClient( + msiConfiguration, + storageConnectionString); + } + + public static IRegistrationBuilder RegisterTableServiceClient( + this ContainerBuilder builder, + Func getConnectionString) + where TConfiguration : class, new() + { + if (builder == null) + { + throw new ArgumentNullException(nameof(builder)); + } + if (getConnectionString == null) + { + throw new ArgumentNullException(nameof(getConnectionString)); + } + + return builder.Register(c => + { + IOptionsSnapshot options = c.Resolve>(); + string storageConnectionString = getConnectionString(options.Value); + StorageMsiConfiguration msiConfiguration = c.Resolve>().Value; + return CreateTableServiceClientClient( + msiConfiguration, + storageConnectionString); + }); + } + private static CloudBlobClientWrapper CreateCloudBlobClient( StorageMsiConfiguration msiConfiguration, string storageConnectionString, @@ -133,5 +179,28 @@ private static CloudBlobClientWrapper CreateCloudBlobClient( readAccessGeoRedundant, requestTimeout); } + + private static TableServiceClient CreateTableServiceClientClient( + StorageMsiConfiguration msiConfiguration, + string tableStorageConnectionString) + { + if (msiConfiguration.UseManagedIdentity) + { + if (string.IsNullOrWhiteSpace(msiConfiguration.ManagedIdentityClientId)) + { + return new TableServiceClient(new Uri(tableStorageConnectionString), + new DefaultAzureCredential()); + } + else + { + return new TableServiceClient(new Uri(tableStorageConnectionString), + new ManagedIdentityCredential(msiConfiguration.ManagedIdentityClientId)); + } + } + + // workaround for https://github.com/Azure/azure-sdk-for-net/issues/44373 + tableStorageConnectionString.Replace("SharedAccessSignature=?", "SharedAccessSignature="); + return new TableServiceClient(tableStorageConnectionString); + } } } diff --git a/src/NuGetGallery.Core/Extensions/UriExtensions.cs b/src/NuGetGallery.Core/Extensions/UriExtensions.cs new file mode 100644 index 0000000000..dd347c684e --- /dev/null +++ b/src/NuGetGallery.Core/Extensions/UriExtensions.cs @@ -0,0 +1,43 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace NuGetGallery.Extensions +{ + public static class UriExtensions + { + /// + /// Appends the given SAS token to the Uri, ensuring the query string is correctly formatted. + /// + /// The base Uri to which the query string will be appended. + /// The SAS string to append, which may or may not start with a '?' character. + /// A new Uri with the SAS string appended. + public static Uri BlobStorageAppendSas(this Uri uri, string sas) + { + if (uri == null) + { + throw new ArgumentNullException(nameof(uri)); + } + + if (string.IsNullOrEmpty(sas)) + { + throw new ArgumentNullException(nameof(sas)); + } + + // Trim any leading '?' from the query string to avoid double '?' + string trimmedQueryString = sas.TrimStart('?'); + + var uriBuilder = new UriBuilder(uri) + { + Query = trimmedQueryString + }; + + return uriBuilder.Uri; + } + } +} diff --git a/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs b/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs index 514aeedb40..f3797983e7 100644 --- a/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs +++ b/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -10,6 +10,7 @@ using System.Net; using System.Threading.Tasks; using NuGetGallery.Diagnostics; +using NuGetGallery.Extensions; using LogLevel = Microsoft.Extensions.Logging.LogLevel; namespace NuGetGallery @@ -371,10 +372,27 @@ public async Task GetPrivilegedFileUriAsync( throw new ArgumentOutOfRangeException(nameof(endOfAccess), $"{nameof(endOfAccess)} is in the past"); } - var blob = await GetBlobForUriAsync(folderName, fileName); + ISimpleCloudBlob blob = await GetBlobForUriAsync(folderName, fileName); string sas = await blob.GetSharedAccessSignature(permissions, endOfAccess); - return new Uri(blob.Uri, sas); + return blob.Uri.BlobStorageAppendSas(sas); + } + + public async Task GetPrivilegedFileUriWithDelegationSasAsync( + string folderName, + string fileName, + FileUriPermissions permissions, + DateTimeOffset endOfAccess) + { + if (endOfAccess < DateTimeOffset.UtcNow) + { + throw new ArgumentOutOfRangeException(nameof(endOfAccess), $"{nameof(endOfAccess)} is in the past"); + } + + ISimpleCloudBlob blob = await GetBlobForUriAsync(folderName, fileName); + string sas = await blob.GetDelegationSasAsync(permissions, endOfAccess); + + return blob.Uri.BlobStorageAppendSas(sas); } public async Task GetFileReadUriAsync(string folderName, string fileName, DateTimeOffset? endOfAccess) @@ -398,7 +416,7 @@ public async Task GetFileReadUriAsync(string folderName, string fileName, D string sas = await blob.GetSharedAccessSignature(FileUriPermissions.Read, endOfAccess.Value); - return new Uri(blob.Uri, sas); + return blob.Uri.BlobStorageAppendSas(sas); } /// @@ -599,4 +617,4 @@ public StorageResult(HttpStatusCode statusCode, Stream data, string etag) } } } -} \ No newline at end of file +} diff --git a/src/NuGetGallery.Core/Services/CloudBlobWrapper.cs b/src/NuGetGallery.Core/Services/CloudBlobWrapper.cs index 613c7fcf56..9eaa915850 100644 --- a/src/NuGetGallery.Core/Services/CloudBlobWrapper.cs +++ b/src/NuGetGallery.Core/Services/CloudBlobWrapper.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -327,30 +327,16 @@ private void ReplaceMetadata(IDictionary newMetadata) public async Task GetSharedAccessSignature(FileUriPermissions permissions, DateTimeOffset endOfAccess) { - var sasBuilder = new BlobSasBuilder - { - BlobContainerName = _blob.BlobContainerName, - BlobName = _blob.Name, - Resource = "b", - StartsOn = DateTimeOffset.UtcNow.AddMinutes(-5), - ExpiresOn = endOfAccess, - }; - sasBuilder.SetPermissions(CloudWrapperHelpers.GetSdkSharedAccessPermissions(permissions)); + BlobSasBuilder sasBuilder = CreateSasBuilderWithPermission(permissions, endOfAccess); if (_blob.CanGenerateSasUri) { // regular SAS return _blob.GenerateSasUri(sasBuilder).Query; } - else if (_container?.Account?.UsingTokenCredential == true && _container?.Account?.Client != null) + else if (IsUsingDelegationSas()) { - // user delegation SAS - var userDelegationKey = (await _container.Account.Client.GetUserDelegationKeyAsync(sasBuilder.StartsOn, sasBuilder.ExpiresOn)).Value; - var blobUriBuilder = new BlobUriBuilder(_blob.Uri) - { - Sas = sasBuilder.ToSasQueryParameters(userDelegationKey, _blob.AccountName), - }; - return blobUriBuilder.ToUri().Query; + return await GenerateDelegationSasAsync(sasBuilder); } else { @@ -358,6 +344,20 @@ public async Task GetSharedAccessSignature(FileUriPermissions permission } } + public async Task GetDelegationSasAsync(FileUriPermissions permissions, DateTimeOffset endOfAccess) + { + BlobSasBuilder sasBuilder = CreateSasBuilderWithPermission(permissions, endOfAccess); + + if (IsUsingDelegationSas()) + { + return await GenerateDelegationSasAsync(sasBuilder); + } + else + { + throw new InvalidOperationException("Unsupported blob authentication, managed identity required for this method."); + } + } + public async Task StartCopyAsync(ISimpleCloudBlob source, IAccessCondition sourceAccessCondition, IAccessCondition destAccessCondition) { // To avoid this we would need to somehow abstract away the primary and secondary storage locations. This @@ -531,5 +531,35 @@ private void UpdateEtag(BlobDownloadDetails details) // workaround for https://github.com/Azure/azure-sdk-for-net/issues/29942 private static string EtagToString(ETag etag) => etag.ToString("H"); + + private BlobSasBuilder CreateSasBuilderWithPermission(FileUriPermissions permissions, DateTimeOffset endOfAccess) + { + var sasBuilder = new BlobSasBuilder + { + BlobContainerName = _blob.BlobContainerName, + BlobName = _blob.Name, + Resource = "b", + StartsOn = DateTimeOffset.UtcNow.AddMinutes(-5), + ExpiresOn = endOfAccess, + }; + sasBuilder.SetPermissions(CloudWrapperHelpers.GetSdkSharedAccessPermissions(permissions)); + + return sasBuilder; + } + + private bool IsUsingDelegationSas() + { + return _container?.Account?.UsingTokenCredential == true && _container?.Account?.Client != null; + } + + private async Task GenerateDelegationSasAsync(BlobSasBuilder sasBuilder) + { + UserDelegationKey userDelegationKey = (await _container.Account.Client.GetUserDelegationKeyAsync(sasBuilder.StartsOn, sasBuilder.ExpiresOn)).Value; + BlobUriBuilder blobUriBuilder = new BlobUriBuilder(_blob.Uri) + { + Sas = sasBuilder.ToSasQueryParameters(userDelegationKey, _blob.AccountName), + }; + return blobUriBuilder.ToUri().Query; + } } -} \ No newline at end of file +} diff --git a/src/NuGetGallery.Core/Services/ICoreFileStorageService.cs b/src/NuGetGallery.Core/Services/ICoreFileStorageService.cs index 311ff56fd4..e9ca0108ee 100644 --- a/src/NuGetGallery.Core/Services/ICoreFileStorageService.cs +++ b/src/NuGetGallery.Core/Services/ICoreFileStorageService.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -59,6 +59,22 @@ Task GetPrivilegedFileUriAsync( FileUriPermissions permissions, DateTimeOffset endOfAccess); + /// + /// Generates a storage file URI giving certain permissions for the specific file via delegation SAS. For example, this method can + /// be used to generate a URI that allows the caller to either delete (via + /// ) or read (via ) the file. + /// + /// The folder name containing the file. + /// The file name. + /// The permissions to give to the privileged URI. + /// The time when the access ends. + /// The URI with privileged delegation SAS access. + Task GetPrivilegedFileUriWithDelegationSasAsync( + string folderName, + string fileName, + FileUriPermissions permissions, + DateTimeOffset endOfAccess); + Task SaveFileAsync(string folderName, string fileName, Stream file, bool overwrite = true); /// @@ -166,4 +182,4 @@ Task GetETagOrNullAsync( string folderName, string fileName); } -} \ No newline at end of file +} diff --git a/src/NuGetGallery.Core/Services/ISimpleCloudBlob.cs b/src/NuGetGallery.Core/Services/ISimpleCloudBlob.cs index 04272b6417..f864cbfec7 100644 --- a/src/NuGetGallery.Core/Services/ISimpleCloudBlob.cs +++ b/src/NuGetGallery.Core/Services/ISimpleCloudBlob.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -52,6 +52,20 @@ public interface ISimpleCloudBlob /// Shared access signature in form of URI query portion. Task GetSharedAccessSignature(FileUriPermissions permissions, DateTimeOffset endOfAccess); + /// + /// Generates a new delegation sas token that if appended to the blob URI + /// would allow actions matching the provided without having access to the + /// access keys of the storage account. + /// + /// The permissions to include in the SAS token. + /// + /// "End of access" timestamp. After the specified timestamp, + /// the returned signature becomes invalid if implementation supports it. + /// Null for no time limit. + /// + /// Delegation SAS in form of URI query portion. + Task GetDelegationSasAsync(FileUriPermissions permissions, DateTimeOffset endOfAccess); + /// /// Opens the seekable read stream to the file in blob storage. /// @@ -84,4 +98,4 @@ Task OpenReadStreamAsync( /// Stream if the call was successful, null if blob does not exist. Task OpenReadIfExistsAsync(); } -} \ No newline at end of file +} diff --git a/src/NuGetGallery/Services/FileSystemFileStorageService.cs b/src/NuGetGallery/Services/FileSystemFileStorageService.cs index 4c246c5f65..9aebd8c5c8 100644 --- a/src/NuGetGallery/Services/FileSystemFileStorageService.cs +++ b/src/NuGetGallery/Services/FileSystemFileStorageService.cs @@ -261,6 +261,11 @@ public Task GetPrivilegedFileUriAsync(string folderName, string fileName, F throw new NotImplementedException(); } + public Task GetPrivilegedFileUriWithDelegationSasAsync(string folderName, string fileName, FileUriPermissions permissions, DateTimeOffset endOfAccess) + { + throw new NotImplementedException(); + } + public Task SetMetadataAsync( string folderName, string fileName, diff --git a/tests/NuGet.Services.V3.Tests/Support/InMemoryCloudBlob.cs b/tests/NuGet.Services.V3.Tests/Support/InMemoryCloudBlob.cs index 52a525ddf9..8bbd14cfa8 100644 --- a/tests/NuGet.Services.V3.Tests/Support/InMemoryCloudBlob.cs +++ b/tests/NuGet.Services.V3.Tests/Support/InMemoryCloudBlob.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -106,6 +106,11 @@ public Task FetchAttributesIfExistsAsync() throw new NotImplementedException(); } + public Task GetDelegationSasAsync(FileUriPermissions permissions, DateTimeOffset endOfAccess) + { + throw new NotImplementedException(); + } + public Task GetSharedAccessSignature(FileUriPermissions permissions, DateTimeOffset endOfAccess) { throw new NotImplementedException(); diff --git a/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs b/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs index 43d4a0d2b8..8e7c4667ce 100644 --- a/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs +++ b/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -6,6 +6,7 @@ using System.IO; using System.Text; using System.Threading.Tasks; +using Azure.Storage.Sas; using Moq; using NuGetGallery.Diagnostics; using Xunit; @@ -738,43 +739,61 @@ public class TheGetPrivilegedFileUriAsyncMethod private const string fileName = "theFileName"; private const string signature = "?secret=42"; - [Fact] - public async Task WillThrowIfFolderIsNull() - { + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task WillThrowIfFolderIsNull(bool isDelegationSas) + { + // Arrange var service = CreateService(); + var permissions = FileUriPermissions.Read; + var endOfAccess = DateTimeOffset.UtcNow.AddHours(3); - var ex = await Assert.ThrowsAsync(() => service.GetPrivilegedFileUriAsync( - null, - fileName, - FileUriPermissions.Read, - DateTimeOffset.UtcNow.AddHours(3))); + Func methodToTest = isDelegationSas + ? () => service.GetPrivilegedFileUriWithDelegationSasAsync(folderName: null, fileName, permissions, endOfAccess) + : () => service.GetPrivilegedFileUriAsync(folderName:null, fileName, permissions, endOfAccess); + + // Act & Assert + var ex = await Assert.ThrowsAsync(methodToTest); Assert.Equal("folderName", ex.ParamName); } - [Fact] - public async Task WillThrowIfFilenameIsNull() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task WillThrowIfFilenameIsNull(bool isDelegationSas) { + // Arrange var service = CreateService(); + var permissions = FileUriPermissions.Read; + var endOfAccess = DateTimeOffset.UtcNow.AddHours(3); - var ex = await Assert.ThrowsAsync(() => service.GetPrivilegedFileUriAsync( - folderName, - null, - FileUriPermissions.Read, - DateTimeOffset.UtcNow.AddHours(3))); + // Define the method to test based on the flag + Func methodToTest = isDelegationSas + ? () => service.GetPrivilegedFileUriWithDelegationSasAsync(folderName, fileName:null, permissions, endOfAccess) + : () => service.GetPrivilegedFileUriAsync(folderName, fileName:null, permissions, endOfAccess); + + // Act & Assert + var ex = await Assert.ThrowsAsync(methodToTest); Assert.Equal("fileName", ex.ParamName); } - [Fact] - public async Task WillThrowIfEndOfAccessIsInThePast() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task WillThrowIfEndOfAccessIsInThePast(bool isDelegationSas) { + // Arrange var service = CreateService(); - DateTimeOffset inThePast = DateTimeOffset.UtcNow.AddSeconds(-1); - var ex = await Assert.ThrowsAsync(() => service.GetPrivilegedFileUriAsync( - folderName, - fileName, - FileUriPermissions.Read, - inThePast)); + + // Define the method to test based on the flag + Func methodToTest = isDelegationSas + ? () => service.GetPrivilegedFileUriWithDelegationSasAsync(folderName, fileName, FileUriPermissions.Read, inThePast) + : () => service.GetPrivilegedFileUriAsync(folderName, fileName, FileUriPermissions.Read, inThePast); + + // Act & Assert + var ex = await Assert.ThrowsAsync(methodToTest); Assert.Equal("endOfAccess", ex.ParamName); } @@ -802,6 +821,30 @@ public async Task WillAlwaysUseSasTokenDependingOnContainerAvailability(string c Assert.Equal(expectedUri, uri.AbsoluteUri); } + [Theory] + [InlineData(CoreConstants.Folders.ValidationFolderName, "http://example.com/" + CoreConstants.Folders.ValidationFolderName + "/" + fileName + signature)] + [InlineData(CoreConstants.Folders.PackagesFolderName, "http://example.com/" + CoreConstants.Folders.PackagesFolderName + "/" + fileName + signature)] + public async Task WillAlwaysUseDelegationSasTokenDependingOnContainerAvailability(string containerName, string expectedUri) + { + var setupResult = Setup(containerName, fileName); + var fakeBlobClient = setupResult.Item1; + var fakeBlob = setupResult.Item2; + var blobUri = setupResult.Item3; + + fakeBlob + .Setup(b => b.GetDelegationSasAsync(FileUriPermissions.Read, It.IsAny())) + .ReturnsAsync(signature); + var service = CreateService(fakeBlobClient); + + var uri = await service.GetPrivilegedFileUriWithDelegationSasAsync( + containerName, + fileName, + FileUriPermissions.Read, + DateTimeOffset.Now.AddHours(3)); + + Assert.Equal(expectedUri, uri.AbsoluteUri); + } + [Fact] public async Task WillPassTheEndOfAccessTimestampFurther() { @@ -839,6 +882,43 @@ public async Task WillPassTheEndOfAccessTimestampFurther() It.IsAny()), Times.Once); } + [Fact] + public async Task DelegationSasWillPassTheEndOfAccessTimestampFurther() + { + const string folderName = CoreConstants.Folders.ValidationFolderName; + const string fileName = "theFileName"; + const string signature = "?secret=42"; + DateTimeOffset endOfAccess = DateTimeOffset.Now.AddHours(3); + Tuple, Mock, Uri> setupResult = Setup(folderName, fileName); + Mock fakeBlobClient = setupResult.Item1; + Mock fakeBlob = setupResult.Item2; + Uri blobUri = setupResult.Item3; + + fakeBlob + .Setup(b => b.GetDelegationSasAsync( + FileUriPermissions.Read | FileUriPermissions.Delete, + endOfAccess)) + .ReturnsAsync(signature) + .Verifiable(); + + CloudBlobCoreFileStorageService service = CreateService(fakeBlobClient); + + var uri = await service.GetPrivilegedFileUriWithDelegationSasAsync( + folderName, + fileName, + FileUriPermissions.Read | FileUriPermissions.Delete, + endOfAccess); + + string expectedUri = new Uri(blobUri, signature).AbsoluteUri; + Assert.Equal(expectedUri, uri.AbsoluteUri); + fakeBlob.Verify( + b => b.GetDelegationSasAsync(FileUriPermissions.Read | FileUriPermissions.Delete, endOfAccess), + Times.Once); + fakeBlob.Verify( + b => b.GetDelegationSasAsync(It.IsAny(), + It.IsAny()), Times.Once); + } + private static Tuple, Mock, Uri> Setup(string folderName, string fileName) { var fakeBlobClient = new Mock(); @@ -1665,4 +1745,4 @@ public async Task VerifyETagIsNullWhenBlobDoesNotExist() } } } -} \ No newline at end of file +}