Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added token credential for queue #4673

Merged
merged 20 commits into from
Aug 13, 2019
Merged

Added token credential for queue #4673

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
9d849c8
Added token credential for queue
sima-zhu Jul 29, 2019
ea937c9
Merge branch 'master' into tokenCred
sima-zhu Jul 30, 2019
a48ecb7
Addressed feedback,
sima-zhu Jul 30, 2019
24d7400
Rename StorageClient to BlobServiceClient (#4707)
alzimmermsft Jul 30, 2019
9303c45
Add REST API Documentation Links (#4557)
alzimmermsft Jul 30, 2019
3e48912
Fixes the checkstyle error on master.? (#4717)
mitchdenny Jul 30, 2019
1f11515
Add @hemanttanwar to /sdk/identity/
joshfree Jul 30, 2019
7a14ea8
Fixed some merge issue
sima-zhu Jul 31, 2019
3e7cc7f
Resolved conflicts
sima-zhu Jul 31, 2019
80567ea
Merge branch 'sima-zhu-tokenCred' into tokenCred
sima-zhu Jul 31, 2019
98613ff
Remove extra chagnes
sima-zhu Jul 31, 2019
47452a6
Minor indentation
sima-zhu Jul 31, 2019
27ed656
Merge from mainline
sima-zhu Jul 31, 2019
13c2431
merge mainline and checkstyle fix
sima-zhu Jul 31, 2019
43763e5
Make changes to comments. Delete the duplicate README
sima-zhu Aug 2, 2019
6a5b1ac
merge from mainline
sima-zhu Aug 2, 2019
f81a310
Merge from master
sima-zhu Aug 13, 2019
762202c
API changes refactor
sima-zhu Aug 13, 2019
a5d24a4
Remove common in queue
sima-zhu Aug 13, 2019
89810ac
Remove redundant changes
sima-zhu Aug 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 27 additions & 51 deletions ...e/client/queue/src/main/java/com/azure/storage/common/credentials/SASTokenCredential.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,22 @@

import com.azure.core.implementation.util.ImplUtils;

import java.util.HashMap;
import java.util.Map;

/**
* Holds a SAS token used for authenticating requests.
*/
public final class SASTokenCredential {
// Required SAS token pieces
private static final String SIGNED_VERSION = "sv";
private static final String SIGNED_SERVICES = "ss";
private static final String SIGNED_RESOURCE_TYPES = "srt";
private static final String SIGNED_PERMISSIONS = "sp";
private static final String SIGNED_EXPIRY = "se";
private static final String SIGNATURE = "sig";

// Optional SAS token pieces
private static final String SIGNED_START = "st";
private static final String SIGNED_PROTOCOL = "spr";
private static final String SIGNED_IP = "sip";

private final String sasToken;

/**
* Creates a SAS token credential from the passed SAS token.
*
* @param sasToken SAS token used to authenticate requests with the service.
*/
public SASTokenCredential(String sasToken) {
private SASTokenCredential(String sasToken) {
this.sasToken = sasToken;
}

Expand All @@ -42,55 +32,41 @@ public String sasToken() {
}

/**
* Creates a SAS token credential from the passed URL query string
* @param query URL query used to build the SAS token
* @return a SAS token credential if the query param contains all the necessary pieces
* Creates a SAS token credential from the passed SAS token.
*
* @param sasToken SAS token
* @return a SAS token credential if {@code sasToken} is not {@code null} or empty, otherwise null.
*/
public static SASTokenCredential fromQuery(String query) {
if (ImplUtils.isNullOrEmpty(query)) {
public static SASTokenCredential fromSASTokenString(String sasToken) {
if (ImplUtils.isNullOrEmpty(sasToken)) {
return null;
}

HashMap<String, String> queryParams = new HashMap<>();
for (String queryParam : query.split("&")) {
String key = queryParam.split("=", 2)[0];
queryParams.put(key, queryParam);
}
return new SASTokenCredential(sasToken);
}

if (queryParams.size() < 6
|| !queryParams.containsKey(SIGNED_VERSION)
|| !queryParams.containsKey(SIGNED_SERVICES)
|| !queryParams.containsKey(SIGNED_RESOURCE_TYPES)
|| !queryParams.containsKey(SIGNED_PERMISSIONS)
|| !queryParams.containsKey(SIGNED_EXPIRY)
|| !queryParams.containsKey(SIGNATURE)) {
/**
* Creates a SAS token credential from the passed query string parameters.
*
* @param queryParameters URL query parameters
* @return a SAS token credential if {@code queryParameters} is not {@code null} and has
* the signature ("sig") query parameter, otherwise returns {@code null}.
*/
public static SASTokenCredential fromQueryParameters(Map<String, String> queryParameters) {
if (ImplUtils.isNullOrEmpty(queryParameters) || !queryParameters.containsKey(SIGNATURE)) {
return null;
}

StringBuilder sasTokenBuilder = new StringBuilder(queryParams.get(SIGNED_VERSION))
.append("&").append(queryParams.get(SIGNED_SERVICES))
.append("&").append(queryParams.get(SIGNED_RESOURCE_TYPES))
.append("&").append(queryParams.get(SIGNED_PERMISSIONS));
StringBuilder sb = new StringBuilder();
for (Map.Entry<String, String> kvp : queryParameters.entrySet()) {
if (sb.length() != 0) {
sb.append("&");

// SIGNED_START is optional
if (queryParams.containsKey(SIGNED_START)) {
sasTokenBuilder.append("&").append(queryParams.get(SIGNED_START));
}

sasTokenBuilder.append("&").append(queryParams.get(SIGNED_EXPIRY));
}

// SIGNED_IP is optional
if (queryParams.containsKey(SIGNED_IP)) {
sasTokenBuilder.append("&").append(queryParams.get(SIGNED_IP));
sb.append(kvp.getKey()).append("=").append(kvp.getValue());
}

// SIGNED_PROTOCOL is optional
if (queryParams.containsKey(SIGNED_PROTOCOL)) {
sasTokenBuilder.append("&").append(queryParams.get(SIGNED_PROTOCOL));
}

sasTokenBuilder.append("&").append(queryParams.get(SIGNATURE));

return new SASTokenCredential(sasTokenBuilder.toString());
return new SASTokenCredential(sb.toString());
}
}
36 changes: 30 additions & 6 deletions storage/client/queue/src/main/java/com/azure/storage/queue/QueueClientBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
// Licensed under the MIT License.
package com.azure.storage.queue;

import com.azure.core.credentials.TokenCredential;
import com.azure.core.http.HttpClient;
import com.azure.core.http.HttpPipeline;
import com.azure.core.http.policy.AddDatePolicy;
import com.azure.core.http.policy.BearerTokenAuthenticationPolicy;
import com.azure.core.http.policy.HttpLogDetailLevel;
import com.azure.core.http.policy.HttpLoggingPolicy;
import com.azure.core.http.policy.HttpPipelinePolicy;
Expand All @@ -26,6 +28,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;

/**
Expand Down Expand Up @@ -83,6 +86,7 @@ public final class QueueClientBuilder {
private HttpLogDetailLevel logLevel;
private RetryPolicy retryPolicy;
private Configuration configuration;
private TokenCredential tokenCredential;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you put this up with the other credential declarations so they're all together? Also, can we name the field bearerTokenCredential? That way, compared with the name sasTokenCredential, they both have qualifiers and it doesn't sound like one's a generic version of the other.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed the tokenCred as suggested.


/**
* Creates a builder instance that is able to configure and construct {@link QueueClient QueueClients}
Expand Down Expand Up @@ -134,7 +138,7 @@ public QueueAsyncClient buildAsyncClient() {
Objects.requireNonNull(endpoint);
Objects.requireNonNull(queueName);

if (sasTokenCredential == null && sharedKeyCredential == null) {
if (sasTokenCredential == null && sharedKeyCredential == null && tokenCredential == null) {
LOGGER.asError().log("Credentials are required for authorization");
throw new IllegalArgumentException("Credentials are required for authorization");
}
Expand All @@ -151,7 +155,9 @@ public QueueAsyncClient buildAsyncClient() {

if (sharedKeyCredential != null) {
policies.add(new SharedKeyCredentialPolicy(sharedKeyCredential));
} else {
} else if (tokenCredential != null) {
policies.add(new BearerTokenAuthenticationPolicy(tokenCredential, String.format("%s/.default", endpoint)));
} else if (sasTokenCredential != null) {
policies.add(new SASTokenCredentialPolicy(sasTokenCredential));
}

Expand All @@ -177,7 +183,7 @@ public QueueAsyncClient buildAsyncClient() {
* <p>The first path segment, if the endpoint contains path segments, will be assumed to be the name of the queue
* that the client will interact with.</p>
*
* <p>Query parameters of the endpoint will be parsed using {@link SASTokenCredential#fromQuery(String) fromQuery} in an
* <p>Query parameters of the endpoint will be parsed using {@link SASTokenCredential#fromQueryParameters(Map)} fromQuery} in an
* attempt to generate a {@link SASTokenCredential} to authenticate requests sent to the service.</p>
*
* @param endpoint The URL of the Azure Storage Queue instance to send service requests to and receive responses from.
Expand All @@ -197,9 +203,10 @@ public QueueClientBuilder endpoint(String endpoint) {
}

// Attempt to get the SAS token from the URL passed
SASTokenCredential credential = SASTokenCredential.fromQuery(fullURL.getQuery());
if (credential != null) {
this.sasTokenCredential = credential;
this.sasTokenCredential = SASTokenCredential.fromQueryParameters(Utility.parseQueryString(fullURL.getQuery()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we parsing a query string into a map just to pass it into a method that turns it back into a query string?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agreed with take the string directly. Moved it logic to SasTokenCredential for current use. Alan will have another common module PR which will take care the token as a whole.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One possible reason this was happening is for validation. We do something similar with permissions (parse the permissions string passed to validate it and then immediately toString it). If there is any sort of validation logic in either parseQueryString or fromQueryString, I'd recommend keeping this as is and adding a comment. If there's nothing of value and it's just a complete waste of cpu time, then I'm fine changing this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line has been changed from Alan's common module PR.
I don't think it is good to address this only for queue.
If it is necessary, then we need to file a new issue to address this overall.

if (this.sasTokenCredential != null) {
this.sharedKeyCredential = null;
this.tokenCredential = null;
}
} catch (MalformedURLException ex) {
LOGGER.asError().log("The Azure Storage Queue endpoint url is malformed. Endpoint: " + endpoint);
Expand Down Expand Up @@ -230,6 +237,8 @@ public QueueClientBuilder queueName(String queueName) {
*/
public QueueClientBuilder credential(SASTokenCredential credential) {
this.sasTokenCredential = Objects.requireNonNull(credential);
this.sharedKeyCredential = null;
this.tokenCredential = null;
return this;
}

Expand All @@ -242,6 +251,21 @@ public QueueClientBuilder credential(SASTokenCredential credential) {
*/
public QueueClientBuilder credential(SharedKeyCredential credential) {
this.sharedKeyCredential = Objects.requireNonNull(credential);
this.sasTokenCredential = null;
this.tokenCredential = null;
return this;
}

/**
* Sets the {@link TokenCredential} used to authenticate requests sent to the Queue service.
* @param credential authorization credential
* @return the updated QueueServiceClientBuilder object
* @throws NullPointerException If {@code credential} is {@code null}
*/
public QueueClientBuilder credential(TokenCredential credential) {
this.tokenCredential = Objects.requireNonNull(credential);
this.sharedKeyCredential = null;
this.sasTokenCredential = null;
return this;
}

Expand Down
35 changes: 29 additions & 6 deletions storage/client/queue/src/main/java/com/azure/storage/queue/QueueServiceClientBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
// Licensed under the MIT License.
package com.azure.storage.queue;

import com.azure.core.credentials.TokenCredential;
import com.azure.core.http.HttpClient;
import com.azure.core.http.HttpPipeline;
import com.azure.core.http.policy.AddDatePolicy;
import com.azure.core.http.policy.BearerTokenAuthenticationPolicy;
import com.azure.core.http.policy.HttpLogDetailLevel;
import com.azure.core.http.policy.HttpLoggingPolicy;
import com.azure.core.http.policy.HttpPipelinePolicy;
Expand All @@ -23,6 +25,7 @@
import java.net.URL;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;

/**
Expand Down Expand Up @@ -81,6 +84,7 @@ public final class QueueServiceClientBuilder {
private HttpLogDetailLevel logLevel;
private RetryPolicy retryPolicy;
private Configuration configuration;
private TokenCredential tokenCredential;

/**
* Creates a builder instance that is able to configure and construct {@link QueueServiceClient QueueServiceClients}
Expand Down Expand Up @@ -110,7 +114,7 @@ public QueueServiceClientBuilder() {
public QueueServiceAsyncClient buildAsyncClient() {
Objects.requireNonNull(endpoint);

if (sasTokenCredential == null && sharedKeyCredential == null) {
if (sasTokenCredential == null && sharedKeyCredential == null && tokenCredential == null) {
LOGGER.asError().log("Credentials are required for authorization");
throw new IllegalArgumentException("Credentials are required for authorization");
}
Expand All @@ -127,7 +131,9 @@ public QueueServiceAsyncClient buildAsyncClient() {

if (sharedKeyCredential != null) {
policies.add(new SharedKeyCredentialPolicy(sharedKeyCredential));
} else {
} else if (tokenCredential != null) {
policies.add(new BearerTokenAuthenticationPolicy(tokenCredential, String.format("%s/.default", endpoint)));
} else if (sasTokenCredential != null) {
policies.add(new SASTokenCredentialPolicy(sasTokenCredential));
}

Expand Down Expand Up @@ -169,7 +175,7 @@ public QueueServiceClient buildClient() {
/**
* Sets the endpoint for the Azure Storage Queue instance that the client will interact with.
*
* <p>Query parameters of the endpoint will be parsed using {@link SASTokenCredential#fromQuery(String) fromQuery} in an
* <p>Query parameters of the endpoint will be parsed using {@link SASTokenCredential#fromQueryParameters(Map)} fromQuery} in an
* attempt to generate a {@link SASTokenCredential} to authenticate requests sent to the service.</p>
*
* @param endpoint The URL of the Azure Storage Queue instance to send service requests to and receive responses from.
Expand All @@ -183,9 +189,10 @@ public QueueServiceClientBuilder endpoint(String endpoint) {
this.endpoint = new URL(fullURL.getProtocol() + "://" + fullURL.getHost());

// Attempt to get the SAS token from the URL passed
SASTokenCredential credential = SASTokenCredential.fromQuery(fullURL.getQuery());
if (credential != null) {
this.sasTokenCredential = credential;
this.sasTokenCredential = SASTokenCredential.fromQueryParameters(Utility.parseQueryString(fullURL.getQuery()));
if (this.sasTokenCredential != null) {
this.sharedKeyCredential = null;
this.tokenCredential = null;
}
} catch (MalformedURLException ex) {
LOGGER.asError().log("The Azure Storage Queue endpoint url is malformed.");
Expand All @@ -204,6 +211,8 @@ public QueueServiceClientBuilder endpoint(String endpoint) {
*/
public QueueServiceClientBuilder credential(SASTokenCredential credential) {
this.sasTokenCredential = Objects.requireNonNull(credential);
this.sharedKeyCredential = null;
this.tokenCredential = null;
return this;
}

Expand All @@ -216,9 +225,23 @@ public QueueServiceClientBuilder credential(SASTokenCredential credential) {
*/
public QueueServiceClientBuilder credential(SharedKeyCredential credential) {
this.sharedKeyCredential = Objects.requireNonNull(credential);
this.sasTokenCredential = null;
this.tokenCredential = null;
return this;
}

/**
* Sets the {@link TokenCredential} used to authenticate requests sent to the Queue service.
* @param credential authorization credential
* @return the updated QueueServiceClientBuilder object
* @throws NullPointerException If {@code credential} is {@code null}
*/
public QueueServiceClientBuilder credential(TokenCredential credential) {
this.tokenCredential = Objects.requireNonNull(credential);
this.sharedKeyCredential = null;
this.sasTokenCredential = null;
return this;
}

/**
* Creates a {@link SharedKeyCredential} from the {@code connectionString} used to authenticate requests sent to the
Expand Down
Loading