Sas time encoding roundtrip (Azure#25524)

* Preserved date-time format when parsing values from a query string for better round tripping

* Updated test string constant

* Ci fixes

* Changelogs and ci fix

* PR feedback

* Moved TimeAndFormat to implementation

Author: rickle-msft

sdk/storage/azure-storage-blob/CHANGELOG.md

@@ -12,6 +12,7 @@
 ### Breaking Changes
 
 ### Bugs Fixed
+- Fixed a bug that would cause authenticating with a sas token to fail if the timestamps in the token were formatted differently.
 
 ### Other Changes
 - Deprecated BlobClient.uploadWithResponse that does not return a response.

sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BlobSasImplUtil.java

@@ -15,6 +15,7 @@
 import com.azure.storage.common.StorageSharedKeyCredential;
 import com.azure.storage.common.implementation.Constants;
 import com.azure.storage.common.implementation.StorageImplUtils;
+import com.azure.storage.common.implementation.TimeAndFormat;
 import com.azure.storage.common.sas.SasIpRange;
 import com.azure.storage.common.sas.SasProtocol;
 
@@ -199,8 +200,10 @@ private String encode(UserDelegationKey userDelegationKey, String signature) {
 
         tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_SERVICE_VERSION, VERSION);
         tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_PROTOCOL, this.protocol);
-        tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_START_TIME, formatQueryParameterDate(this.startTime));
-        tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_EXPIRY_TIME, formatQueryParameterDate(this.expiryTime));
+        tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_START_TIME, formatQueryParameterDate(
+            new TimeAndFormat(this.startTime, null)));
+        tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_EXPIRY_TIME, formatQueryParameterDate(
+            new TimeAndFormat(this.expiryTime, null)));
         tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_IP_RANGE, this.sasIpRange);
         tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_SIGNED_IDENTIFIER, this.identifier);
         if (userDelegationKey != null) {
@@ -209,9 +212,9 @@ private String encode(UserDelegationKey userDelegationKey, String signature) {
             tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_SIGNED_TENANT_ID,
                 userDelegationKey.getSignedTenantId());
             tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_SIGNED_KEY_START,
-                formatQueryParameterDate(userDelegationKey.getSignedStart()));
+                formatQueryParameterDate(new TimeAndFormat(userDelegationKey.getSignedStart(), null)));
             tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_SIGNED_KEY_EXPIRY,
-                formatQueryParameterDate(userDelegationKey.getSignedExpiry()));
+                formatQueryParameterDate(new TimeAndFormat(userDelegationKey.getSignedExpiry(), null)));
             tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_SIGNED_KEY_SERVICE,
                 userDelegationKey.getSignedService());
             tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_SIGNED_KEY_VERSION,

sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/sas/BlobServiceSasQueryParameters.java

@@ -5,11 +5,11 @@
 
 import com.azure.storage.blob.models.UserDelegationKey;
 import com.azure.storage.blob.BlobClientBuilder;
+import com.azure.storage.common.implementation.StorageImplUtils;
 import com.azure.storage.common.sas.BaseSasQueryParameters;
 import com.azure.storage.common.sas.SasProtocol;
 import com.azure.storage.common.implementation.Constants;
 import com.azure.storage.common.sas.SasIpRange;
-import com.azure.storage.common.Utility;
 
 import java.time.OffsetDateTime;
 import java.util.Map;
@@ -71,9 +71,9 @@ public BlobServiceSasQueryParameters(Map<String, String[]> queryParamsMap, boole
         this.keyTenantId = getQueryParameter(queryParamsMap, Constants.UrlConstants.SAS_SIGNED_TENANT_ID,
             removeSasParametersFromMap);
         this.keyStart = getQueryParameter(queryParamsMap, Constants.UrlConstants.SAS_SIGNED_KEY_START,
-            removeSasParametersFromMap, Utility::parseDate);
+            removeSasParametersFromMap, StorageImplUtils::parseDateAndFormat).getDateTime();
         this.keyExpiry = getQueryParameter(queryParamsMap, Constants.UrlConstants.SAS_SIGNED_KEY_EXPIRY,
-            removeSasParametersFromMap, Utility::parseDate);
+            removeSasParametersFromMap, StorageImplUtils::parseDateAndFormat).getDateTime();
         this.keyService = getQueryParameter(queryParamsMap, Constants.UrlConstants.SAS_SIGNED_KEY_SERVICE,
             removeSasParametersFromMap);
         this.keyVersion = getQueryParameter(queryParamsMap, Constants.UrlConstants.SAS_SIGNED_KEY_VERSION,

sdk/storage/azure-storage-common/CHANGELOG.md

@@ -7,6 +7,7 @@
 ### Breaking Changes
 
 ### Bugs Fixed
+- Fixed a bug that would cause authenticating with a sas token to fail if the timestamps in the token were formatted differently.
 
 ### Other Changes
 

sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/Utility.java

@@ -7,6 +7,7 @@
 import com.azure.core.util.CoreUtils;
 import com.azure.core.util.UrlBuilder;
 import com.azure.core.util.logging.ClientLogger;
+import com.azure.storage.common.implementation.StorageImplUtils;
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 
@@ -21,45 +22,20 @@
 import java.time.ZoneOffset;
 import java.time.format.DateTimeFormatter;
 import java.util.Locale;
-import java.util.regex.Pattern;
 
 /**
  * Utility methods for storage client libraries.
  */
 public final class Utility {
     private static final ClientLogger LOGGER = new ClientLogger(Utility.class);
     private static final String UTF8_CHARSET = "UTF-8";
-    private static final String INVALID_DATE_STRING = "Invalid Date String: %s.";
 
     /**
      * Please see <a href=https://docs.microsoft.com/en-us/azure/azure-resource-manager/management/azure-services-resource-providers>here</a>
      * for more information on Azure resource provider namespaces.
       */
     public static final String STORAGE_TRACING_NAMESPACE_VALUE = "Microsoft.Storage";
 
-    /**
-     * Stores a reference to the date/time pattern with the greatest precision Java.util.Date is capable of expressing.
-     */
-    private static final String MAX_PRECISION_PATTERN = "yyyy-MM-dd'T'HH:mm:ss.SSS";
-    /**
-     * Stores a reference to the ISO8601 date/time pattern.
-     */
-    private static final String ISO8601_PATTERN = "yyyy-MM-dd'T'HH:mm:ss'Z'";
-    /**
-     * Stores a reference to the ISO8601 date/time pattern.
-     */
-    private static final String ISO8601_PATTERN_NO_SECONDS = "yyyy-MM-dd'T'HH:mm'Z'";
-    /**
-     * The length of a datestring that matches the MAX_PRECISION_PATTERN.
-     */
-    private static final int MAX_PRECISION_DATESTRING_LENGTH = MAX_PRECISION_PATTERN.replaceAll("'", "")
-            .length();
-    /**
-     * A compiled Pattern that finds 'Z'. This is used as Java 8's String.replace method uses Pattern.compile
-     * internally without simple case opt-outs.
-     */
-    private static final Pattern Z_PATTERN = Pattern.compile("Z");
-
 
     /**
      * Performs a safe decoding of the passed string, taking care to preserve each {@code +} character rather than
@@ -188,33 +164,35 @@ public static String encodeUrlPath(String url) {
      * @param dateString the {@code String} to be interpreted as a <code>Date</code>
      * @return the corresponding <code>Date</code> object
      * @throws IllegalArgumentException If {@code dateString} doesn't match an ISO8601 pattern
+     * @deprecated Use {@link StorageImplUtils#parseDateAndFormat(String)}
      */
+    @Deprecated
     public static OffsetDateTime parseDate(String dateString) {
-        String pattern = MAX_PRECISION_PATTERN;
+        String pattern = StorageImplUtils.MAX_PRECISION_PATTERN;
         switch (dateString.length()) {
             case 28: // "yyyy-MM-dd'T'HH:mm:ss.SSSSSSS'Z'"-> [2012-01-04T23:21:59.1234567Z] length = 28
             case 27: // "yyyy-MM-dd'T'HH:mm:ss.SSSSSS'Z'"-> [2012-01-04T23:21:59.123456Z] length = 27
             case 26: // "yyyy-MM-dd'T'HH:mm:ss.SSSSS'Z'"-> [2012-01-04T23:21:59.12345Z] length = 26
             case 25: // "yyyy-MM-dd'T'HH:mm:ss.SSSS'Z'"-> [2012-01-04T23:21:59.1234Z] length = 25
             case 24: // "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'"-> [2012-01-04T23:21:59.123Z] length = 24
-                dateString = dateString.substring(0, MAX_PRECISION_DATESTRING_LENGTH);
+                dateString = dateString.substring(0, StorageImplUtils.MAX_PRECISION_DATESTRING_LENGTH);
                 break;
             case 23: // "yyyy-MM-dd'T'HH:mm:ss.SS'Z'"-> [2012-01-04T23:21:59.12Z] length = 23
                 // SS is assumed to be milliseconds, so a trailing 0 is necessary
-                dateString = Z_PATTERN.matcher(dateString).replaceAll("0");
+                dateString = StorageImplUtils.Z_PATTERN.matcher(dateString).replaceAll("0");
                 break;
             case 22: // "yyyy-MM-dd'T'HH:mm:ss.S'Z'"-> [2012-01-04T23:21:59.1Z] length = 22
                 // S is assumed to be milliseconds, so trailing 0's are necessary
-                dateString = Z_PATTERN.matcher(dateString).replaceAll("00");
+                dateString = StorageImplUtils.Z_PATTERN.matcher(dateString).replaceAll("00");
                 break;
             case 20: // "yyyy-MM-dd'T'HH:mm:ss'Z'"-> [2012-01-04T23:21:59Z] length = 20
-                pattern = Utility.ISO8601_PATTERN;
+                pattern = StorageImplUtils.ISO8601_PATTERN;
                 break;
             case 17: // "yyyy-MM-dd'T'HH:mm'Z'"-> [2012-01-04T23:21Z] length = 17
-                pattern = Utility.ISO8601_PATTERN_NO_SECONDS;
+                pattern = StorageImplUtils.ISO8601_PATTERN_NO_SECONDS;
                 break;
             default:
-                throw new IllegalArgumentException(String.format(Locale.ROOT, INVALID_DATE_STRING, dateString));
+                throw new IllegalArgumentException(String.format(Locale.ROOT, StorageImplUtils.INVALID_DATE_STRING, dateString));
         }
 
         DateTimeFormatter formatter = DateTimeFormatter.ofPattern(pattern, Locale.ROOT);

sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/implementation/AccountSasImplUtil.java

@@ -125,8 +125,10 @@ private String encode(String signature) {
         tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_SERVICES, this.services);
         tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_RESOURCES_TYPES, this.resourceTypes);
         tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_PROTOCOL, this.protocol);
-        tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_START_TIME, formatQueryParameterDate(this.startTime));
-        tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_EXPIRY_TIME, formatQueryParameterDate(this.expiryTime));
+        tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_START_TIME, formatQueryParameterDate(
+            new TimeAndFormat(this.startTime, null)));
+        tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_EXPIRY_TIME, formatQueryParameterDate(
+            new TimeAndFormat(this.expiryTime, null)));
         tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_IP_RANGE, this.sasIpRange);
         tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_SIGNED_PERMISSIONS, this.permissions);
         tryAppendQueryParameter(sb, Constants.UrlConstants.SAS_ENCRYPTION_SCOPE, this.encryptionScope);

sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/implementation/SasImplUtils.java

@@ -9,7 +9,6 @@
 import com.azure.storage.common.Utility;
 import com.azure.storage.common.policy.StorageSharedKeyCredentialPolicy;
 
-import java.time.OffsetDateTime;
 import java.util.Comparator;
 import java.util.Locale;
 import java.util.Map;
@@ -55,14 +54,18 @@ public static void tryAppendQueryParameter(StringBuilder sb, String param, Objec
     /**
      * Formats date time SAS query parameters.
      *
-     * @param dateTime The SAS date time.
+     * @param timeAndFormat The SAS date time.
      * @return A String representing the SAS date time.
      */
-    public static String formatQueryParameterDate(OffsetDateTime dateTime) {
-        if (dateTime == null) {
+    public static String formatQueryParameterDate(TimeAndFormat timeAndFormat) {
+        if (timeAndFormat == null || timeAndFormat.getDateTime() == null) {
             return null;
         } else {
-            return Constants.ISO_8601_UTC_DATE_FORMATTER.format(dateTime);
+            if (timeAndFormat.getFormatter() == null) {
+                return Constants.ISO_8601_UTC_DATE_FORMATTER.format(timeAndFormat.getDateTime());
+            } else {
+                return timeAndFormat.getFormatter().format(timeAndFormat.getDateTime());
+            }
         }
     }
 

sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/implementation/StorageImplUtils.java

@@ -17,11 +17,15 @@
 import java.security.InvalidKeyException;
 import java.security.NoSuchAlgorithmException;
 import java.time.Duration;
+import java.time.LocalDateTime;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
 import java.util.Base64;
 import java.util.Locale;
 import java.util.Map;
 import java.util.TreeMap;
 import java.util.function.Function;
+import java.util.regex.Pattern;
 
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
@@ -46,8 +50,10 @@ public class StorageImplUtils {
     private static final String NO_PATH_SEGMENTS = "URL %s does not contain path segments.";
 
     private static final String STRING_TO_SIGN_LOG_INFO_MESSAGE = "The string to sign computed by the SDK is: {}{}";
+
     private static final String STRING_TO_SIGN_LOG_WARNING_MESSAGE = "Please remember to disable '{}' before going "
         + "to production as this string can potentially contain PII.";
+
     private static final String STORAGE_EXCEPTION_LOG_STRING_TO_SIGN_MESSAGE = String.format(
         "If you are using a StorageSharedKeyCredential, and the server returned an "
             + "error message that says 'Signature did not match', you can compare the string to sign with"
@@ -62,6 +68,49 @@ public class StorageImplUtils {
         Constants.STORAGE_LOG_STRING_TO_SIGN, Constants.STORAGE_LOG_STRING_TO_SIGN,
         Constants.STORAGE_LOG_STRING_TO_SIGN);
 
+    /**
+     * @deprecated See value in {@link StorageImplUtils}
+     */
+    @Deprecated
+    public static final String INVALID_DATE_STRING = "Invalid Date String: %s.";
+
+    /**
+     * Stores a reference to the date/time pattern with the greatest precision Java.util.Date is capable of expressing.
+     * @deprecated See value in {@link StorageImplUtils}
+     */
+    @Deprecated
+    public static final String MAX_PRECISION_PATTERN = "yyyy-MM-dd'T'HH:mm:ss.SSS";
+
+    /**
+     * The length of a datestring that matches the MAX_PRECISION_PATTERN.
+     * @deprecated See value in {@link StorageImplUtils}
+     */
+    @Deprecated
+    public static final int MAX_PRECISION_DATESTRING_LENGTH = MAX_PRECISION_PATTERN.replaceAll("'", "")
+            .length();
+
+    /**
+     * Stores a reference to the ISO8601 date/time pattern.
+     * @deprecated See value in {@link StorageImplUtils}
+     */
+    @Deprecated
+    public static final String ISO8601_PATTERN = "yyyy-MM-dd'T'HH:mm:ss'Z'";
+
+    /**
+     * Stores a reference to the ISO8601 date/time pattern.
+     * @deprecated See value in {@link StorageImplUtils}
+     */
+    @Deprecated
+    public static final String ISO8601_PATTERN_NO_SECONDS = "yyyy-MM-dd'T'HH:mm'Z'";
+
+    /**
+     * A compiled Pattern that finds 'Z'. This is used as Java 8's String.replace method uses Pattern.compile
+     * internally without simple case opt-outs.
+     * @deprecated See value in {@link StorageImplUtils}
+     */
+    @Deprecated
+    public static final Pattern Z_PATTERN = Pattern.compile("Z");
+
     /**
      * Parses the query string into a key-value pair map that maintains key, query parameter key, order. The value is
      * stored as a string (ex. key=val1,val2,val3 instead of key=[val1, val2, val3]).
@@ -327,4 +376,45 @@ public static String convertStorageExceptionMessage(String message, HttpResponse
         }
         return message;
     }
+
+    /**
+     * Given a String representing a date in a form of the ISO8601 pattern, generates a Date representing it with up to
+     * millisecond precision.
+     *
+     * @param dateString the {@code String} to be interpreted as a <code>Date</code>
+     * @return the corresponding <code>Date</code> object
+     * @throws IllegalArgumentException If {@code dateString} doesn't match an ISO8601 pattern
+     */
+    public static TimeAndFormat parseDateAndFormat(String dateString) {
+        String pattern = MAX_PRECISION_PATTERN;
+        switch (dateString.length()) {
+            case 28: // "yyyy-MM-dd'T'HH:mm:ss.SSSSSSS'Z'"-> [2012-01-04T23:21:59.1234567Z] length = 28
+            case 27: // "yyyy-MM-dd'T'HH:mm:ss.SSSSSS'Z'"-> [2012-01-04T23:21:59.123456Z] length = 27
+            case 26: // "yyyy-MM-dd'T'HH:mm:ss.SSSSS'Z'"-> [2012-01-04T23:21:59.12345Z] length = 26
+            case 25: // "yyyy-MM-dd'T'HH:mm:ss.SSSS'Z'"-> [2012-01-04T23:21:59.1234Z] length = 25
+            case 24: // "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'"-> [2012-01-04T23:21:59.123Z] length = 24
+                dateString = dateString.substring(0, MAX_PRECISION_DATESTRING_LENGTH);
+                break;
+            case 23: // "yyyy-MM-dd'T'HH:mm:ss.SS'Z'"-> [2012-01-04T23:21:59.12Z] length = 23
+                // SS is assumed to be milliseconds, so a trailing 0 is necessary
+                dateString = Z_PATTERN.matcher(dateString).replaceAll("0");
+                break;
+            case 22: // "yyyy-MM-dd'T'HH:mm:ss.S'Z'"-> [2012-01-04T23:21:59.1Z] length = 22
+                // S is assumed to be milliseconds, so trailing 0's are necessary
+                dateString = Z_PATTERN.matcher(dateString).replaceAll("00");
+                break;
+            case 20: // "yyyy-MM-dd'T'HH:mm:ss'Z'"-> [2012-01-04T23:21:59Z] length = 20
+                pattern = ISO8601_PATTERN;
+                break;
+            case 17: // "yyyy-MM-dd'T'HH:mm'Z'"-> [2012-01-04T23:21Z] length = 17
+                pattern = ISO8601_PATTERN_NO_SECONDS;
+                break;
+            default:
+                throw new IllegalArgumentException(String.format(Locale.ROOT, INVALID_DATE_STRING, dateString));
+        }
+
+        DateTimeFormatter formatter = DateTimeFormatter.ofPattern(pattern, Locale.ROOT);
+        return new TimeAndFormat(LocalDateTime.parse(dateString, formatter).atZone(ZoneOffset.UTC).toOffsetDateTime(),
+            formatter);
+    }
 }

sdk/storage/azure-storage-common/src/main/java/com/azure/storage/common/implementation/TimeAndFormat.java

@@ -0,0 +1,49 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package com.azure.storage.common.implementation;
+
+import java.time.OffsetDateTime;
+import java.time.format.DateTimeFormatter;
+
+/**
+ * A class to hold an {@code OffsetDateTime} object and the formatter from which it was parsed for effective
+ * roundtripping.
+ * Without this, we always format date-times the same way, but if we parsed a value from a sas token in on an endpoint
+ * or in a connection string, we have to preserve the formatting, especially the precision, so the signature still
+ * matches.
+ */
+public final class TimeAndFormat {
+    private final OffsetDateTime dateTime;
+    private final DateTimeFormatter format;
+
+    /**
+     * Constructs a new {@code TimeAndFormat object}.
+     *
+     * @param dateTime The date-time object.
+     * @param formatter The format it was parsed from. Null if it was not parsed from a {@code DateTimeFormatter}.
+     */
+    public TimeAndFormat(OffsetDateTime dateTime, DateTimeFormatter formatter) {
+        // If the format is null, it means we didn't parse it and we can just use our default formatter
+        this.dateTime = dateTime;
+        this.format = formatter;
+    }
+
+    /**
+     * Gets the date-time.
+     *
+     * @return The date-time
+     */
+    public OffsetDateTime getDateTime() {
+        return dateTime;
+    }
+
+    /**
+     * Gets the format the time was parsed from.
+     *
+     * @return The format. May be null if time was not parsed.
+     */
+    public DateTimeFormatter getFormatter() {
+        return format;
+    }
+}