Skip to content

Commit

Permalink
Fixed issue where account SAS does not work on clients (#6601)
Browse files Browse the repository at this point in the history
  • Loading branch information
gapra-msft authored Dec 6, 2019
1 parent 2b24156 commit a7775d6
Show file tree
Hide file tree
Showing 29 changed files with 1,062 additions and 92 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ public EncryptedBlobClientBuilder endpoint(String endpoint) {
this.blobName = Utility.urlEncode(parts.getBlobName());
this.snapshot = parts.getSnapshot();

String sasToken = parts.getSasQueryParameters().encode();
String sasToken = parts.getCommonSasQueryParameters().encode();
if (!CoreUtils.isNullOrEmpty(sasToken)) {
this.sasToken(sasToken);
}
Expand Down
1 change: 1 addition & 0 deletions sdk/storage/azure-storage-blob/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@
--add-exports com.azure.core/com.azure.core.implementation.serializer.jackson=ALL-UNNAMED
--add-exports com.azure.core/com.azure.core.implementation.util=ALL-UNNAMED
--add-opens com.azure.storage.common/com.azure.storage.common.implementation=ALL-UNNAMED
--add-opens com.azure.storage.common/com.azure.storage.common.sas=ALL-UNNAMED
--add-opens com.azure.storage.blob/com.azure.storage.blob=ALL-UNNAMED
--add-opens com.azure.storage.blob/com.azure.storage.blob.implementation=ALL-UNNAMED
--add-opens com.azure.storage.blob/com.azure.storage.blob.specialized=ALL-UNNAMED
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ public BlobClientBuilder endpoint(String endpoint) {
this.blobName = Utility.urlEncode(parts.getBlobName());
this.snapshot = parts.getSnapshot();

String sasToken = parts.getSasQueryParameters().encode();
String sasToken = parts.getCommonSasQueryParameters().encode();
if (!CoreUtils.isNullOrEmpty(sasToken)) {
this.sasToken(sasToken);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public BlobContainerClientBuilder endpoint(String endpoint) {
this.containerName = parts.getBlobContainerName();
this.endpoint = BuilderHelper.getEndpoint(parts);

String sasToken = parts.getSasQueryParameters().encode();
String sasToken = parts.getCommonSasQueryParameters().encode();
if (!CoreUtils.isNullOrEmpty(sasToken)) {
this.sasToken(sasToken);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public BlobServiceClientBuilder endpoint(String endpoint) {
this.accountName = parts.getAccountName();
this.endpoint = BuilderHelper.getEndpoint(parts);

String sasToken = parts.getSasQueryParameters().encode();
String sasToken = parts.getCommonSasQueryParameters().encode();
if (!CoreUtils.isNullOrEmpty(sasToken)) {
this.sasToken(sasToken);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.azure.storage.blob.implementation.util.ModelHelper;
import com.azure.storage.blob.sas.BlobServiceSasQueryParameters;
import com.azure.storage.common.Utility;
import com.azure.storage.common.sas.CommonSasQueryParameters;
import com.azure.storage.common.implementation.Constants;

import java.net.MalformedURLException;
Expand All @@ -34,7 +35,7 @@ public final class BlobUrlParts {
private String snapshot;
private String accountName;
private boolean isIpUrl;
private BlobServiceSasQueryParameters blobServiceSasQueryParameters;
private CommonSasQueryParameters commonSasQueryParameters;
private Map<String, String[]> unparsedParameters;

/**
Expand Down Expand Up @@ -166,24 +167,62 @@ public BlobUrlParts setSnapshot(String snapshot) {
}

/**
* Gets the {@link BlobServiceSasQueryParameters} representing the SAS query parameters that will be used to
* generate the SAS token for this URL.
* Gets the {@link BlobServiceSasQueryParameters} representing the SAS query parameters
*
* @return the {@link BlobServiceSasQueryParameters} of the URL
* @deprecated Please use {@link #getCommonSasQueryParameters()}
*/
@Deprecated
public BlobServiceSasQueryParameters getSasQueryParameters() {
return blobServiceSasQueryParameters;
String encodedSas = commonSasQueryParameters.encode();
return new BlobServiceSasQueryParameters(parseQueryString(encodedSas), true);
}

/**
* Sets the {@link BlobServiceSasQueryParameters} representing the SAS query parameters that will be used to
* generate the SAS token for this URL.
* Sets the {@link BlobServiceSasQueryParameters} representing the SAS query parameters.
*
* @param blobServiceSasQueryParameters The SAS query parameters.
* @return the updated BlobUrlParts object.
* @deprecated Please use {@link #setCommonSasQueryParameters(CommonSasQueryParameters)}
*/
@Deprecated
public BlobUrlParts setSasQueryParameters(BlobServiceSasQueryParameters blobServiceSasQueryParameters) {
this.blobServiceSasQueryParameters = blobServiceSasQueryParameters;
String encodedBlobSas = blobServiceSasQueryParameters.encode();
this.commonSasQueryParameters = new CommonSasQueryParameters(parseQueryString(encodedBlobSas), true);
return this;
}

/**
* Gets the {@link CommonSasQueryParameters} representing the SAS query parameters that will be used to
* generate the SAS token for this URL.
*
* @return the {@link CommonSasQueryParameters} of the URL
*/
public CommonSasQueryParameters getCommonSasQueryParameters() {
return commonSasQueryParameters;
}

/**
* Sets the {@link CommonSasQueryParameters} representing the SAS query parameters that will be used to
* generate the SAS token for this URL.
*
* @param commonSasQueryParameters The SAS query parameters.
* @return the updated BlobUrlParts object.
*/
public BlobUrlParts setCommonSasQueryParameters(CommonSasQueryParameters commonSasQueryParameters) {
this.commonSasQueryParameters = commonSasQueryParameters;
return this;
}

/**
* Sets the {@link CommonSasQueryParameters} representing the SAS query parameters that will be used to
* generate the SAS token for this URL.
*
* @param queryParams The SAS query parameter string.
* @return the updated BlobUrlParts object.
*/
public BlobUrlParts parseSasQueryParameters(String queryParams) {
this.commonSasQueryParameters = new CommonSasQueryParameters(parseQueryString(queryParams), true);
return this;
}

Expand Down Expand Up @@ -239,8 +278,8 @@ public URL toUrl() {
if (this.snapshot != null) {
url.setQueryParameter(Constants.UrlConstants.SNAPSHOT_QUERY_PARAMETER, this.snapshot);
}
if (this.blobServiceSasQueryParameters != null) {
String encodedSAS = this.blobServiceSasQueryParameters.encode();
if (this.commonSasQueryParameters != null) {
String encodedSAS = this.commonSasQueryParameters.encode();
if (encodedSAS.length() != 0) {
url.setQuery(encodedSAS);
}
Expand Down Expand Up @@ -312,10 +351,9 @@ public static BlobUrlParts parse(URL url) {
parts.setSnapshot(snapshotArray[0]);
}

BlobServiceSasQueryParameters blobServiceSasQueryParameters =
new BlobServiceSasQueryParameters(queryParamsMap, true);
CommonSasQueryParameters commonSasQueryParameters = new CommonSasQueryParameters(queryParamsMap, true);

return parts.setSasQueryParameters(blobServiceSasQueryParameters)
return parts.setCommonSasQueryParameters(commonSasQueryParameters)
.setUnparsedParameters(queryParamsMap);
}

Expand Down Expand Up @@ -386,7 +424,7 @@ private static void parseNonIpUrl(URL url, BlobUrlParts parts) {
}

/**
* Parses a query string into a one to many hashmap.
* Parses a query string into a one to many TreeMap.
*
* @param queryParams The string of query params to parse.
* @return A {@code HashMap<String, String[]>} of the key values.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ public SpecializedBlobClientBuilder endpoint(String endpoint) {
this.blobName = Utility.urlEncode(parts.getBlobName());
this.snapshot = parts.getSnapshot();

String sasToken = parts.getSasQueryParameters().encode();
String sasToken = parts.getCommonSasQueryParameters().encode();
if (!CoreUtils.isNullOrEmpty(sasToken)) {
this.sasToken(sasToken);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,10 @@ class APISpec extends Specification {
}

BlobContainerClient getContainerClient(String sasToken, String endpoint) {
getContainerClientBuilder(endpoint).sasToken(sasToken).buildClient()
}

BlobContainerClientBuilder getContainerClientBuilder(String endpoint) {
BlobContainerClientBuilder builder = new BlobContainerClientBuilder()
.endpoint(endpoint)
.httpClient(getHttpClient())
Expand All @@ -311,7 +315,7 @@ class APISpec extends Specification {
builder.addPolicy(interceptorManager.getRecordPolicy())
}

builder.sasToken(sasToken).buildClient()
return builder
}

BlobAsyncClient getBlobAsyncClient(StorageSharedKeyCredential credential, String endpoint, String blobName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import com.azure.storage.common.StorageSharedKeyCredential

import com.azure.storage.common.implementation.Constants
import com.azure.storage.common.sas.SasIpRange
import spock.lang.Ignore
import spock.lang.Unroll

import java.time.LocalDateTime
Expand Down Expand Up @@ -646,6 +647,45 @@ class SASTest extends APISpec {
notThrown(BlobStorageException)
}

def "accountSAS network account sas token on endpoint"() {
setup:
def service = new AccountSasService()
.setBlobAccess(true)
def resourceType = new AccountSasResourceType()
.setContainer(true)
.setService(true)
.setObject(true)
def permissions = new AccountSasPermission()
.setReadPermission(true)
.setCreatePermission(true)
def expiryTime = getUTCNow().plusDays(1)

def sas = new AccountSasSignatureValues()
.setServices(service.toString())
.setResourceTypes(resourceType.toString())
.setPermissions(permissions)
.setExpiryTime(expiryTime)
.generateSasQueryParameters(primaryCredential)
.encode()
def containerName = generateContainerName()
def blobName = generateBlobName()

when:
def sc = getServiceClientBuilder(null, primaryBlobServiceClient.getAccountUrl() + "?" + sas, null).buildClient()
sc.createBlobContainer(containerName)

def cc = getContainerClientBuilder(primaryBlobServiceClient.getAccountUrl() + "/" + containerName + "?" + sas).buildClient()
cc.getProperties()

def bc = getBlobClient(primaryCredential, primaryBlobServiceClient.getAccountUrl() + "/" + containerName + "/" + blobName + "?" + sas)

def file = getRandomFile(256)
bc.uploadFromFile(file.toPath().toString())

then:
notThrown(BlobStorageException)
}

/*
This test will ensure that each field gets placed into the proper location within the string to sign and that null
values are handled correctly. We will validate the whole SAS with service calls as well as correct serialization of
Expand Down Expand Up @@ -1155,6 +1195,8 @@ class SASTest extends APISpec {
thrown(IllegalArgumentException)
}

// TODO : Figure out how to properly port this test over since I changed it to a common sas params
@Ignore
def "BlobURLParts"() {
setup:
def parts = new BlobUrlParts()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -614,35 +614,6 @@ class HelperTest extends APISpec {
thrown(IllegalArgumentException)
}

def "BlobURLParts"() {
setup:
BlobUrlParts parts = new BlobUrlParts()
.setScheme("http")
.setHost("host")
.setContainerName("container")
.setBlobName("blob")
.setSnapshot("snapshot")

BlobServiceSasSignatureValues sasValues = new BlobServiceSasSignatureValues()
.setExpiryTime(OffsetDateTime.now(ZoneOffset.UTC).plusDays(1))
.setPermissions(new BlobSasPermission().setReadPermission(true))
.setBlobName("blob")
.setContainerName("container")

parts.setSasQueryParameters(sasValues.generateSasQueryParameters(primaryCredential))

when:
String[] splitParts = parts.toUrl().toString().split("\\?")

then:
splitParts.size() == 2 // Ensure that there is only one question mark even when sas and snapshot are present
splitParts[0] == "http://host/container/blob"
splitParts[1].contains("snapshot=snapshot")
splitParts[1].contains("sp=r")
splitParts[1].contains("sig=")
splitParts[1].split("&").size() == 6 // snapshot & sv & sr & sp & sig & se
}

def "BlobURLParts implicit root"() {
when:
def bup = new BlobUrlParts()
Expand All @@ -653,28 +624,4 @@ class HelperTest extends APISpec {
then:
new BlobUrlParts().parse(bup.toUrl()).getBlobContainerName() == BlobContainerAsyncClient.ROOT_CONTAINER_NAME
}

def "URLParser"() {
when:
BlobUrlParts parts = BlobUrlParts.parse(new URL("http://host/container/" + originalBlobName + "?snapshot=snapshot&sv=" + Constants.HeaderConstants.TARGET_STORAGE_VERSION + "&sr=c&sp=r&sig=Ee%2BSodSXamKSzivSdRTqYGh7AeMVEk3wEoRZ1yzkpSc%3D"))

then:
parts.getScheme() == "http"
parts.getHost() == "host"
parts.getBlobContainerName() == "container"
parts.getBlobName() == finalBlobName
parts.getSnapshot() == "snapshot"
parts.getSasQueryParameters().getPermissions() == "r"
parts.getSasQueryParameters().getVersion() == Constants.HeaderConstants.TARGET_STORAGE_VERSION
parts.getSasQueryParameters().getResource() == "c"
parts.getSasQueryParameters().getSignature() == Utility.urlDecode("Ee%2BSodSXamKSzivSdRTqYGh7AeMVEk3wEoRZ1yzkpSc%3D")

where:
originalBlobName | finalBlobName
"blob" | "blob"
"path/to]a blob" | "path/to]a blob"
"path%2Fto%5Da%20blob" | "path/to]a blob"
"斑點" | "斑點"
"%E6%96%91%E9%BB%9E" | "斑點"
}
}
Loading

0 comments on commit a7775d6

Please sign in to comment.