Skip to content

Commit

Permalink
mgmt, make storage account more secure by setting better default (#19649
Browse files Browse the repository at this point in the history
)

* private access for container

* private access in readme

* storage account default to TLS 1.2

* test case for defaults

* options for public blob access and shared key access

* improve PagedIterableImpl
  • Loading branch information
weidongxu-microsoft committed Mar 8, 2021
1 parent 4ec4b2a commit e9d211e
Show file tree
Hide file tree
Showing 11 changed files with 559 additions and 21 deletions.
2 changes: 1 addition & 1 deletion sdk/resourcemanager/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ azure.storageAccounts().define("<storage-account-name>")
.flatMap(storageAccount -> azure.storageBlobContainers()
.defineContainer("container")
.withExistingBlobService(rgName, storageAccount.name())
.withPublicAccess(PublicAccess.BLOB)
.withPublicAccess(PublicAccess.NONE)
.createAsync()
)
//...
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ private Mono<Indexable> createContainerIfNotExistsAsync(final StorageAccount sto
.onErrorResume(throwable -> storageManager.blobContainers()
.defineContainer(containerName)
.withExistingBlobService(storageAccount.resourceGroupName(), storageAccount.name())
.withPublicAccess(PublicAccess.CONTAINER)
.withPublicAccess(PublicAccess.NONE)
.createAsync());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,15 +173,20 @@ public static <T> PagedFlux<T> convertListToPagedFlux(Mono<Response<List<T>>> re

private static final class PagedIterableImpl<T, S> extends PagedIterable<S> {

private final PagedIterable<T> pageIterable;
private final PagedIterable<T> pagedIterable;
private final Function<T, S> mapper;
private final Function<PagedResponse<T>, PagedResponse<S>> pageMapper;

private PagedIterableImpl(PagedIterable<T> pageIterable, Function<T, S> mapper) {
super(new PagedFlux<S>(Mono::empty));
this.pageIterable = pageIterable;
private PagedIterableImpl(PagedIterable<T> pagedIterable, Function<T, S> mapper) {
super(PagedFlux.create(() -> (continuationToken, pageSize)
-> Flux.fromStream(pagedIterable.streamByPage().map(getPageMapper(mapper)))));
this.pagedIterable = pagedIterable;
this.mapper = mapper;
this.pageMapper = page -> new PagedResponseBase<Void, S>(
this.pageMapper = getPageMapper(mapper);
}

private static <T, S> Function<PagedResponse<T>, PagedResponse<S>> getPageMapper(Function<T, S> mapper) {
return page -> new PagedResponseBase<Void, S>(
page.getRequest(),
page.getStatusCode(),
page.getHeaders(),
Expand All @@ -192,56 +197,56 @@ private PagedIterableImpl(PagedIterable<T> pageIterable, Function<T, S> mapper)

@Override
public Stream<S> stream() {
return pageIterable.stream().map(mapper);
return pagedIterable.stream().map(mapper);
}

@Override
public Stream<PagedResponse<S>> streamByPage() {
return pageIterable.streamByPage().map(pageMapper);
return pagedIterable.streamByPage().map(pageMapper);
}

@Override
public Stream<PagedResponse<S>> streamByPage(String continuationToken) {
return pageIterable.streamByPage(continuationToken).map(pageMapper);
return pagedIterable.streamByPage(continuationToken).map(pageMapper);
}

@Override
public Stream<PagedResponse<S>> streamByPage(int preferredPageSize) {
return pageIterable.streamByPage(preferredPageSize).map(pageMapper);
return pagedIterable.streamByPage(preferredPageSize).map(pageMapper);
}

@Override
public Stream<PagedResponse<S>> streamByPage(String continuationToken, int preferredPageSize) {
return pageIterable.streamByPage(continuationToken, preferredPageSize).map(pageMapper);
return pagedIterable.streamByPage(continuationToken, preferredPageSize).map(pageMapper);
}

@Override
public Iterator<S> iterator() {
return new IteratorImpl<T, S>(pageIterable.iterator(), mapper);
return new IteratorImpl<T, S>(pagedIterable.iterator(), mapper);
}

@Override
public Iterable<PagedResponse<S>> iterableByPage() {
return new IterableImpl<PagedResponse<T>, PagedResponse<S>>(
pageIterable.iterableByPage(), pageMapper);
pagedIterable.iterableByPage(), pageMapper);
}

@Override
public Iterable<PagedResponse<S>> iterableByPage(String continuationToken) {
return new IterableImpl<PagedResponse<T>, PagedResponse<S>>(
pageIterable.iterableByPage(continuationToken), pageMapper);
pagedIterable.iterableByPage(continuationToken), pageMapper);
}

@Override
public Iterable<PagedResponse<S>> iterableByPage(int preferredPageSize) {
return new IterableImpl<PagedResponse<T>, PagedResponse<S>>(
pageIterable.iterableByPage(preferredPageSize), pageMapper);
pagedIterable.iterableByPage(preferredPageSize), pageMapper);
}

@Override
public Iterable<PagedResponse<S>> iterableByPage(String continuationToken, int preferredPageSize) {
return new IterableImpl<PagedResponse<T>, PagedResponse<S>>(
pageIterable.iterableByPage(continuationToken, preferredPageSize), pageMapper);
pagedIterable.iterableByPage(continuationToken, preferredPageSize), pageMapper);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,17 @@ public void testMapPage() {
Assertions.assertFalse(iterator.hasNext());
}

@Test
public void testMapPageWithPagedIterable() {
PagedFlux<String> pagedFlux = mockPagedFlux("base", 0, 10, 4);
PagedIterable<String> pagedIterable = new PagedIterable<>(pagedFlux);

PagedIterable<String> convertedPagedIterable = PagedConverter.mapPage(pagedIterable, item -> item + "#");

PagedIterable<String> afterMapPage = convertedPagedIterable.mapPage(item -> item + "#");
Assertions.assertEquals(10, afterMapPage.stream().count());
}

@Test
public void testMapPageIterator() {
PagedFlux<String> pagedFlux = mockPagedFlux("base", 0, 10, 4);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## 2.3.0-beta.1 (Unreleased)

- Storage account default to Transport Layer Security (TLS) 1.2 for HTTPS

## 2.2.0 (2021-02-24)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import com.azure.resourcemanager.storage.models.IdentityType;
import com.azure.resourcemanager.storage.models.Kind;
import com.azure.resourcemanager.storage.models.LargeFileSharesState;
import com.azure.resourcemanager.storage.models.MinimumTlsVersion;
import com.azure.resourcemanager.storage.models.ProvisioningState;
import com.azure.resourcemanager.storage.models.PublicEndpoints;
import com.azure.resourcemanager.storage.models.Sku;
Expand Down Expand Up @@ -189,6 +190,35 @@ public boolean isLargeFileSharesEnabled() {
return this.innerModel().largeFileSharesState() == LargeFileSharesState.ENABLED;
}

@Override
public MinimumTlsVersion minimumTlsVersion() {
return this.innerModel().minimumTlsVersion();
}

@Override
public boolean isHttpsTrafficOnly() {
if (this.innerModel().enableHttpsTrafficOnly() == null) {
return true;
}
return this.innerModel().enableHttpsTrafficOnly();
}

@Override
public boolean isBlobPublicAccessAllowed() {
if (this.innerModel().allowBlobPublicAccess() == null) {
return true;
}
return this.innerModel().allowBlobPublicAccess();
}

@Override
public boolean isSharedKeyAccessAllowed() {
if (this.innerModel().allowSharedKeyAccess() == null) {
return true;
}
return this.innerModel().allowSharedKeyAccess();
}

@Override
public List<StorageAccountKey> getKeys() {
return this.getKeysAsync().block();
Expand Down Expand Up @@ -378,7 +408,61 @@ public StorageAccountImpl withOnlyHttpsTraffic() {

@Override
public StorageAccountImpl withHttpAndHttpsTraffic() {
updateParameters.withEnableHttpsTrafficOnly(false);
if (isInCreateMode()) {
createParameters.withEnableHttpsTrafficOnly(false);
} else {
updateParameters.withEnableHttpsTrafficOnly(false);
}
return this;
}

@Override
public StorageAccountImpl withMinimumTlsVersion(MinimumTlsVersion minimumTlsVersion) {
if (isInCreateMode()) {
createParameters.withMinimumTlsVersion(minimumTlsVersion);
} else {
updateParameters.withMinimumTlsVersion(minimumTlsVersion);
}
return this;
}

@Override
public StorageAccountImpl enableBlobPublicAccess() {
if (isInCreateMode()) {
createParameters.withAllowBlobPublicAccess(true);
} else {
updateParameters.withAllowBlobPublicAccess(true);
}
return this;
}

@Override
public StorageAccountImpl disableBlobPublicAccess() {
if (isInCreateMode()) {
createParameters.withAllowBlobPublicAccess(false);
} else {
updateParameters.withAllowBlobPublicAccess(false);
}
return this;
}

@Override
public StorageAccountImpl enableSharedKeyAccess() {
if (isInCreateMode()) {
createParameters.withAllowSharedKeyAccess(true);
} else {
updateParameters.withAllowSharedKeyAccess(true);
}
return this;
}

@Override
public StorageAccountImpl disableSharedKeyAccess() {
if (isInCreateMode()) {
createParameters.withAllowSharedKeyAccess(false);
} else {
updateParameters.withAllowSharedKeyAccess(false);
}
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.azure.resourcemanager.storage.fluent.StorageAccountsClient;
import com.azure.resourcemanager.storage.fluent.models.ListServiceSasResponseInner;
import com.azure.resourcemanager.storage.models.CheckNameAvailabilityResult;
import com.azure.resourcemanager.storage.models.MinimumTlsVersion;
import com.azure.resourcemanager.storage.models.ServiceSasParameters;
import com.azure.resourcemanager.storage.models.StorageAccount;
import com.azure.resourcemanager.storage.models.StorageAccountSkuType;
Expand Down Expand Up @@ -40,7 +41,11 @@ public Mono<CheckNameAvailabilityResult> checkNameAvailabilityAsync(String name)

@Override
public StorageAccountImpl define(String name) {
return wrapModel(name).withSku(StorageAccountSkuType.STANDARD_RAGRS).withGeneralPurposeAccountKindV2();
return wrapModel(name)
.withSku(StorageAccountSkuType.STANDARD_RAGRS)
.withGeneralPurposeAccountKindV2()
.withOnlyHttpsTraffic()
.withMinimumTlsVersion(MinimumTlsVersion.TLS1_2);
}

@Override
Expand Down
Loading

0 comments on commit e9d211e

Please sign in to comment.