Use sas token for attachment downloads (#1153)

* Get limited life attachment download URL

This change limits url download to a 1min lifetime.
This requires moving to a new container to allow for non-public blob
access.

Clients will have to call GetAttachmentData api function to receive the download
URL. For backwards compatibility, attachment URLs are still present, but will not
work for attachments stored in non-public access blobs.

* Make GlobalSettings interface for testing

* Test LocalAttachmentStorageService equivalence

* Remove comment

* Add missing globalSettings using

* Simplify default attachment container

* Default to attachments containe for existing methods

A new upload method will be made for uploading to attachments-v2.
For compatibility for clients which don't use these new methods, we need
to still use the old container. The new container will be used only for
new uploads

* Remove Default MetaData fixture.

* Keep attachments container blob-level security for all instances

* Close unclosed FileStream

* Favor default value for noop services

test/Api.Test/Controllers/AccountsControllerTests.cs

@@ -7,6 +7,7 @@ using Bit.Core.Models.Data;
 using Bit.Core.Models.Table;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using Microsoft.AspNetCore.Identity;
 using NSubstitute;
 using System;

test/Core.Test/AutoFixture/CipherAttachmentMetaDataFixtures.cs

@@ -0,0 +1,33 @@
+using AutoFixture;
+using AutoFixture.Dsl;
+using Bit.Core.Models.Data;
+
+namespace Bit.Core.Test.AutoFixture.CipherAttachmentMetaData
+{
+    public class MetaData : ICustomization
+    {
+        protected virtual IPostprocessComposer<CipherAttachment.MetaData> ComposerAction(IFixture fixture,
+            ICustomizationComposer<CipherAttachment.MetaData> composer)
+        {
+            return composer.With(d => d.Size, fixture.Create<long>()).Without(d => d.SizeString);
+        }
+        public void Customize(IFixture fixture)
+        {
+            fixture.Customize<CipherAttachment.MetaData>(composer => ComposerAction(fixture, composer));
+        }
+    }
+
+    public class MetaDataWithoutContainer : MetaData
+    {
+        protected override IPostprocessComposer<CipherAttachment.MetaData> ComposerAction(IFixture fixture,
+            ICustomizationComposer<CipherAttachment.MetaData> composer) =>
+            base.ComposerAction(fixture, composer).With(d => d.ContainerName, (string)null);
+    }
+
+    public class MetaDataWithoutKey : MetaDataWithoutContainer
+    {
+        protected override IPostprocessComposer<CipherAttachment.MetaData> ComposerAction(IFixture fixture,
+            ICustomizationComposer<CipherAttachment.MetaData> composer) =>
+            base.ComposerAction(fixture, composer).Without(d => d.Key);
+    }
+}

test/Core.Test/Services/AmazonSesMailDeliveryServiceTests.cs

@@ -6,6 +6,7 @@ using Amazon.SimpleEmail;
 using Amazon.SimpleEmail.Model;
 using Bit.Core.Models.Mail;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.Extensions.Logging;
 using NSubstitute;

test/Core.Test/Services/AmazonSqsBlockIpServiceTests.cs

@@ -2,6 +2,7 @@ using System;
 using System.Threading.Tasks;
 using Amazon.SQS;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using NSubstitute;
 using Xunit;
 

test/Core.Test/Services/AzureAttachmentStorageServiceTests.cs

@@ -1,5 +1,6 @@
 using System;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using NSubstitute;
 using Xunit;
 

test/Core.Test/Services/AzureQueueBlockIpServiceTests.cs

@@ -1,5 +1,6 @@
 using System;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using NSubstitute;
 using Xunit;
 

test/Core.Test/Services/AzureQueueEventWriteServiceTests.cs

@@ -1,6 +1,7 @@
 using System;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using NSubstitute;
 using Xunit;
 

test/Core.Test/Services/AzureQueuePushNotificationServiceTests.cs

@@ -1,5 +1,6 @@
 using System;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using Microsoft.AspNetCore.Http;
 using NSubstitute;
 using Xunit;

test/Core.Test/Services/EventServiceTests.cs

@@ -2,6 +2,7 @@ using System;
 using Bit.Core.Context;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using NSubstitute;
 using Xunit;
 

test/Core.Test/Services/HandlebarsMailServiceTests.cs

@@ -1,5 +1,6 @@
 using System;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using NSubstitute;
 using Xunit;
 

test/Core.Test/Services/InMemoryServiceBusApplicationCacheServiceTests.cs

@@ -1,6 +1,7 @@
 using System;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using NSubstitute;
 using Xunit;
 

test/Core.Test/Services/LicensingServiceTests.cs

@@ -1,6 +1,7 @@
 using System;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.Extensions.Logging;
 using NSubstitute;

test/Core.Test/Services/LocalAttachmentStorageServiceTests.cs

@@ -1,29 +1,233 @@
 using System;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using NSubstitute;
 using Xunit;
+using System.IO;
+using Bit.Core.Test.AutoFixture.Attributes;
+using Bit.Core.Test.AutoFixture;
+using Bit.Core.Test.AutoFixture.CipherFixtures;
+using Bit.Core.Models.Data;
+using System.Threading.Tasks;
+using Bit.Core.Models.Table;
+using U2F.Core.Utils;
+using Bit.Core.Test.AutoFixture.CipherAttachmentMetaData;
+using AutoFixture;
 
 namespace Bit.Core.Test.Services
 {
     public class LocalAttachmentStorageServiceTests
     {
-        private readonly LocalAttachmentStorageService _sut;
 
-        private readonly GlobalSettings _globalSettings;
-
-        public LocalAttachmentStorageServiceTests()
+        private void AssertFileCreation(string expectedPath, string expectedFileContents)
         {
-            _globalSettings = new GlobalSettings();
-
-            _sut = new LocalAttachmentStorageService(_globalSettings);
+            Assert.True(File.Exists(expectedPath));
+            Assert.Equal(expectedFileContents, File.ReadAllText(expectedPath));
         }
 
-        // Remove this test when we add actual tests. It only proves that
-        // we've properly constructed the system under test.
-        [Fact]
-        public void ServiceExists()
+        [Theory]
+        [InlineCustomAutoData(new[] { typeof(UserCipher), typeof(MetaData) })]
+        [InlineCustomAutoData(new[] { typeof(UserCipher), typeof(MetaDataWithoutContainer) })]
+        [InlineCustomAutoData(new[] { typeof(UserCipher), typeof(MetaDataWithoutKey) })]
+        public async Task UploadNewAttachmentAsync_Success(string stream, Cipher cipher, CipherAttachment.MetaData attachmentData)
         {
-            Assert.NotNull(_sut);
+            using (var tempDirectory = new TempDirectory())
+            {
+                var sutProvider = GetSutProvider(tempDirectory);
+
+                await sutProvider.Sut.UploadNewAttachmentAsync(new MemoryStream(stream.GetBytes()), cipher, attachmentData);
+
+                AssertFileCreation($"{tempDirectory}/{cipher.Id}/{attachmentData.AttachmentId}", stream);
+            }
+        }
+
+        [Theory]
+        [InlineCustomAutoData(new[] { typeof(OrganizationCipher), typeof(MetaData) })]
+        [InlineCustomAutoData(new[] { typeof(OrganizationCipher), typeof(MetaDataWithoutContainer) })]
+        [InlineCustomAutoData(new[] { typeof(OrganizationCipher), typeof(MetaDataWithoutKey) })]
+        public async Task UploadShareAttachmentAsync_Success(string stream, Cipher cipher, CipherAttachment.MetaData attachmentData)
+        {
+            using (var tempDirectory = new TempDirectory())
+            {
+                var sutProvider = GetSutProvider(tempDirectory);
+
+                await sutProvider.Sut.UploadShareAttachmentAsync(new MemoryStream(stream.GetBytes()), cipher.Id,
+                    cipher.OrganizationId.Value, attachmentData);
+
+                AssertFileCreation($"{tempDirectory}/temp/{cipher.Id}/{cipher.OrganizationId}/{attachmentData.AttachmentId}", stream);
+            }
+        }
+
+        [Theory]
+        [InlineCustomAutoData(new[] { typeof(OrganizationCipher), typeof(MetaData) })]
+        [InlineCustomAutoData(new[] { typeof(OrganizationCipher), typeof(MetaDataWithoutContainer) })]
+        [InlineCustomAutoData(new[] { typeof(OrganizationCipher), typeof(MetaDataWithoutKey) })]
+        public async Task StartShareAttachmentAsync_NoSource_NoWork(Cipher cipher, CipherAttachment.MetaData attachmentData)
+        {
+            using (var tempDirectory = new TempDirectory())
+            {
+                var sutProvider = GetSutProvider(tempDirectory);
+
+                await sutProvider.Sut.StartShareAttachmentAsync(cipher.Id, cipher.OrganizationId.Value, attachmentData);
+
+                Assert.False(File.Exists($"{tempDirectory}/{cipher.Id}/{attachmentData.AttachmentId}"));
+                Assert.False(File.Exists($"{tempDirectory}/{cipher.Id}/{attachmentData.AttachmentId}"));
+            }
+        }
+
+        [Theory]
+        [InlineCustomAutoData(new[] { typeof(OrganizationCipher), typeof(MetaData) })]
+        [InlineCustomAutoData(new[] { typeof(OrganizationCipher), typeof(MetaDataWithoutContainer) })]
+        [InlineCustomAutoData(new[] { typeof(OrganizationCipher), typeof(MetaDataWithoutKey) })]
+        public async Task StartShareAttachmentAsync_NoDest_NoWork(string source, Cipher cipher, CipherAttachment.MetaData attachmentData)
+        {
+            using (var tempDirectory = new TempDirectory())
+            {
+                var sutProvider = GetSutProvider(tempDirectory);
+
+                var sourcePath = $"{tempDirectory}/temp/{cipher.Id}/{cipher.OrganizationId}/{attachmentData.AttachmentId}";
+                var destPath = $"{tempDirectory}/{cipher.Id}/{attachmentData.AttachmentId}";
+                var rollBackPath = $"{tempDirectory}/temp/{cipher.Id}/{attachmentData.AttachmentId}";
+                Directory.CreateDirectory(Path.GetDirectoryName(sourcePath));
+                File.WriteAllText(sourcePath, source);
+
+                await sutProvider.Sut.StartShareAttachmentAsync(cipher.Id, cipher.OrganizationId.Value, attachmentData);
+
+                Assert.True(File.Exists(sourcePath));
+                Assert.Equal(source, File.ReadAllText(sourcePath));
+                Assert.False(File.Exists(destPath));
+                Assert.False(File.Exists(rollBackPath));
+            }
+        }
+
+
+        [Theory]
+        [InlineCustomAutoData(new[] { typeof(OrganizationCipher), typeof(MetaData) })]
+        [InlineCustomAutoData(new[] { typeof(OrganizationCipher), typeof(MetaDataWithoutContainer) })]
+        [InlineCustomAutoData(new[] { typeof(OrganizationCipher), typeof(MetaDataWithoutKey) })]
+        public async Task StartShareAttachmentAsync_Success(string source, string destOriginal, Cipher cipher, CipherAttachment.MetaData attachmentData)
+        {
+            using (var tempDirectory = new TempDirectory())
+            {
+                await StartShareAttachmentAsync(source, destOriginal, cipher, attachmentData, tempDirectory);
+            }
+        }
+
+        [Theory]
+        [InlineCustomAutoData(new[] { typeof(OrganizationCipher), typeof(MetaData) })]
+        [InlineCustomAutoData(new[] { typeof(OrganizationCipher), typeof(MetaDataWithoutContainer) })]
+        [InlineCustomAutoData(new[] { typeof(OrganizationCipher), typeof(MetaDataWithoutKey) })]
+        public async Task RollbackShareAttachmentAsync_Success(string source, string destOriginal, Cipher cipher, CipherAttachment.MetaData attachmentData)
+        {
+            using (var tempDirectory = new TempDirectory())
+            {
+                var sutProvider = GetSutProvider(tempDirectory);
+
+                var sourcePath = $"{tempDirectory}/temp/{cipher.Id}/{cipher.OrganizationId}/{attachmentData.AttachmentId}";
+                var destPath = $"{tempDirectory}/{cipher.Id}/{attachmentData.AttachmentId}";
+                var rollBackPath = $"{tempDirectory}/temp/{cipher.Id}/{attachmentData.AttachmentId}";
+
+                await StartShareAttachmentAsync(source, destOriginal, cipher, attachmentData, tempDirectory);
+                await sutProvider.Sut.RollbackShareAttachmentAsync(cipher.Id, cipher.OrganizationId.Value, attachmentData, "Not Used Here");
+
+                Assert.True(File.Exists(destPath));
+                Assert.Equal(destOriginal, File.ReadAllText(destPath));
+                Assert.False(File.Exists(sourcePath));
+                Assert.False(File.Exists(rollBackPath));
+            }
+        }
+
+        [Theory]
+        [InlineCustomAutoData(new[] { typeof(UserCipher), typeof(MetaData) })]
+        [InlineCustomAutoData(new[] { typeof(UserCipher), typeof(MetaDataWithoutContainer) })]
+        [InlineCustomAutoData(new[] { typeof(UserCipher), typeof(MetaDataWithoutKey) })]
+        public async Task DeleteAttachmentAsync_Success(Cipher cipher, CipherAttachment.MetaData attachmentData)
+        {
+            using (var tempDirectory = new TempDirectory())
+            {
+                var sutProvider = GetSutProvider(tempDirectory);
+
+                var expectedPath = $"{tempDirectory}/{cipher.Id}/{attachmentData.AttachmentId}";
+                Directory.CreateDirectory(Path.GetDirectoryName(expectedPath));
+                File.Create(expectedPath).Close();
+
+                await sutProvider.Sut.DeleteAttachmentAsync(cipher.Id, attachmentData);
+
+                Assert.False(File.Exists(expectedPath));
+            }
+        }
+
+        [Theory]
+        [InlineUserCipherAutoData]
+        [InlineOrganizationCipherAutoData]
+        public async Task CleanupAsync_Succes(Cipher cipher)
+        {
+            using (var tempDirectory = new TempDirectory())
+            {
+                var sutProvider = GetSutProvider(tempDirectory);
+
+                var tempPath = $"{tempDirectory}/temp/{cipher.Id}";
+                var permPath = $"{tempDirectory}/{cipher.Id}";
+                Directory.CreateDirectory(tempPath);
+                Directory.CreateDirectory(permPath);
+
+                await sutProvider.Sut.CleanupAsync(cipher.Id);
+
+                Assert.False(Directory.Exists(tempPath));
+                Assert.True(Directory.Exists(permPath));
+            }
+        }
+
+        [Theory]
+        [InlineUserCipherAutoData]
+        [InlineOrganizationCipherAutoData]
+        public async Task DeleteAttachmentsForCipherAsync_Succes(Cipher cipher)
+        {
+            using (var tempDirectory = new TempDirectory())
+            {
+                var sutProvider = GetSutProvider(tempDirectory);
+
+                var tempPath = $"{tempDirectory}/temp/{cipher.Id}";
+                var permPath = $"{tempDirectory}/{cipher.Id}";
+                Directory.CreateDirectory(tempPath);
+                Directory.CreateDirectory(permPath);
+
+                await sutProvider.Sut.DeleteAttachmentsForCipherAsync(cipher.Id);
+
+                Assert.True(Directory.Exists(tempPath));
+                Assert.False(Directory.Exists(permPath));
+            }
+        }
+
+        private async Task StartShareAttachmentAsync(string source, string destOriginal, Cipher cipher,
+            CipherAttachment.MetaData attachmentData, TempDirectory tempDirectory)
+        {
+            var sutProvider = GetSutProvider(tempDirectory);
+
+            var sourcePath = $"{tempDirectory}/temp/{cipher.Id}/{cipher.OrganizationId}/{attachmentData.AttachmentId}";
+            var destPath = $"{tempDirectory}/{cipher.Id}/{attachmentData.AttachmentId}";
+            var rollBackPath = $"{tempDirectory}/temp/{cipher.Id}/{attachmentData.AttachmentId}";
+            Directory.CreateDirectory(Path.GetDirectoryName(sourcePath));
+            Directory.CreateDirectory(Path.GetDirectoryName(destPath));
+            File.WriteAllText(sourcePath, source);
+            File.WriteAllText(destPath, destOriginal);
+
+            await sutProvider.Sut.StartShareAttachmentAsync(cipher.Id, cipher.OrganizationId.Value, attachmentData);
+
+            Assert.False(File.Exists(sourcePath));
+            Assert.True(File.Exists(destPath));
+            Assert.Equal(source, File.ReadAllText(destPath));
+            Assert.True(File.Exists(rollBackPath));
+            Assert.Equal(destOriginal, File.ReadAllText(rollBackPath));
+        }
+
+        private SutProvider<LocalAttachmentStorageService> GetSutProvider(TempDirectory tempDirectory)
+        {
+            var fixture = new Fixture().WithAutoNSubstitutions();
+            fixture.Freeze<IGlobalSettings>().Attachment.BaseDirectory.Returns(tempDirectory.Directory);
+            fixture.Freeze<IGlobalSettings>().Attachment.BaseUrl.Returns(Guid.NewGuid().ToString());
+
+            return new SutProvider<LocalAttachmentStorageService>(fixture).Create();
         }
     }
 }

test/Core.Test/Services/MailKitSmtpMailDeliveryServiceTests.cs

@@ -1,5 +1,6 @@
 using System;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using Microsoft.Extensions.Logging;
 using NSubstitute;
 using Xunit;

test/Core.Test/Services/MultiServicePushNotificationServiceTests.cs

@@ -1,6 +1,7 @@
 using System;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.Logging;
 using NSubstitute;

test/Core.Test/Services/NotificationHubPushNotificationServiceTests.cs

@@ -1,6 +1,7 @@
 using System;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using Microsoft.AspNetCore.Http;
 using NSubstitute;
 using Xunit;

test/Core.Test/Services/NotificationHubPushRegistrationServiceTests.cs

@@ -1,6 +1,7 @@
 using System;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using NSubstitute;
 using Xunit;
 

test/Core.Test/Services/NotificationsApiPushNotificationServiceTests.cs

@@ -1,5 +1,6 @@
 using System;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.Logging;
 using NSubstitute;

test/Core.Test/Services/OrganizationServiceTests.cs

@@ -6,6 +6,7 @@ using Bit.Core.Models.Table;
 using Bit.Core.Models.Business;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using Microsoft.AspNetCore.DataProtection;
 using NSubstitute;
 using Xunit;

test/Core.Test/Services/RelayPushNotificationServiceTests.cs

@@ -1,6 +1,7 @@
 using System;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.Logging;
 using NSubstitute;

test/Core.Test/Services/RelayPushRegistrationServiceTests.cs

@@ -1,5 +1,6 @@
 using System;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using Microsoft.Extensions.Logging;
 using NSubstitute;
 using Xunit;

test/Core.Test/Services/StripePaymentServiceTests.cs

@@ -1,6 +1,7 @@
 using System;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using Microsoft.Extensions.Logging;
 using NSubstitute;
 using Xunit;

test/Core.Test/Services/UserServiceTests.cs

@@ -3,6 +3,7 @@ using System.Collections.Generic;
 using Bit.Core.Models.Table;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
+using Bit.Core.Settings;
 using Microsoft.AspNetCore.DataProtection;
 using Microsoft.AspNetCore.Identity;
 using Microsoft.Extensions.Options;

test/Core.Test/TempDirectory.cs

@@ -0,0 +1,42 @@
+using System;
+using System.IO;
+
+namespace Bit.Core.Test
+{
+    public class TempDirectory : IDisposable
+    {
+        public string Directory { get; private set; }
+
+        public TempDirectory()
+        {
+            Directory = Path.Combine(Path.GetTempPath(), $"bitwarden_{Guid.NewGuid().ToString().Replace("-", "")}");
+        }
+
+        public override string ToString() => Directory;
+
+        #region IDisposable implementation
+        ~TempDirectory()
+        {
+            Dispose(false);
+        }
+
+        public void Dispose()
+        {
+            Dispose(true);
+            GC.SuppressFinalize(this);
+        }
+
+        public void Dispose(bool disposing)
+        {
+            if (disposing)
+            {
+                try
+                {
+                    System.IO.Directory.Delete(Directory, true);
+                }
+                catch { }
+            }
+        }
+        # endregion
+    }
+}

test/Core.Test/Utilities/SelfHostedAttributeTests.cs

@@ -1,5 +1,6 @@
 using System.Collections.Generic;
 using Bit.Core.Exceptions;
+using Bit.Core.Settings;
 using Bit.Core.Utilities;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;