Skip to content

Commit

Permalink
Move some CLI output parsing to stream style (#39008)
Browse files Browse the repository at this point in the history
* Move some parsing to stream style

I noticed a bug in how parsing `az` output was working after adding support for the new `expires_on` field. The path to fixing that required fixing how parsing the json worked, so I took the opportunity to start moving to stream-style deserialization.

* revert some string updates and fix a variable name that failed checkstyle

* fix test failure, improve parsing.

* PR feedback
  • Loading branch information
billwert committed Mar 1, 2024
1 parent 94e2276 commit 6506c0a
Show file tree
Hide file tree
Showing 6 changed files with 253 additions and 51 deletions.
5 changes: 5 additions & 0 deletions sdk/identity/azure-identity/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@
<artifactId>azure-core-http-netty</artifactId>
<version>1.14.0</version> <!-- {x-version-update;com.azure:azure-core-http-netty;dependency} -->
</dependency>
<dependency>
<groupId>com.azure</groupId>
<artifactId>azure-json</artifactId>
<version>1.1.0</version> <!-- {x-version-update;com.azure:azure-json;dependency} -->
</dependency>
<dependency>
<groupId>com.microsoft.azure</groupId>
<artifactId>msal4j</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,12 @@
import com.azure.identity.CredentialUnavailableException;
import com.azure.identity.DeviceCodeInfo;
import com.azure.identity.TokenCachePersistenceOptions;
import com.azure.identity.implementation.models.AzureCliToken;
import com.azure.identity.implementation.util.CertificateUtil;
import com.azure.identity.implementation.util.IdentityUtil;
import com.azure.identity.implementation.util.LoggingUtil;
import com.azure.json.JsonProviders;
import com.azure.json.JsonReader;
import com.microsoft.aad.msal4j.AppTokenProviderParameters;
import com.microsoft.aad.msal4j.ClaimsRequest;
import com.microsoft.aad.msal4j.ClientCredentialFactory;
Expand Down Expand Up @@ -121,7 +124,6 @@ public abstract class IdentityClientBase {
private static final String SDK_VERSION = "version";
private static final ClientOptions DEFAULT_CLIENT_OPTIONS = new ClientOptions();

private static final OffsetDateTime EPOCH = OffsetDateTime.of(1970, 1, 1, 0, 0, 0, 0, ZoneOffset.UTC);

private final Map<String, String> properties;

Expand Down Expand Up @@ -640,13 +642,13 @@ AccessToken getTokenFromAzureCLIAuthentication(StringBuilder azCommand) {

LOGGER.verbose("Azure CLI Authentication => A token response was received from Azure CLI, deserializing the"
+ " response into an Access Token.");
Map<String, String> objectMap = SERIALIZER_ADAPTER.deserialize(processOutput, Map.class,
SerializerEncoding.JSON);
String accessToken = objectMap.get("accessToken");
OffsetDateTime tokenExpiry;
try (JsonReader reader = JsonProviders.createReader(processOutput)) {
AzureCliToken tokenHolder = AzureCliToken.fromJson(reader);
String accessToken = tokenHolder.getAccessToken();
OffsetDateTime tokenExpiration = tokenHolder.getTokenExpiration();
token = new AccessToken(accessToken, tokenExpiration);
}

tokenExpiry = getTokenExpiryOffsetDateTime(objectMap);
token = new AccessToken(accessToken, tokenExpiry);
} catch (IOException | InterruptedException e) {
IllegalStateException ex = new IllegalStateException(redactInfo(e.getMessage()));
ex.setStackTrace(e.getStackTrace());
Expand All @@ -655,28 +657,6 @@ AccessToken getTokenFromAzureCLIAuthentication(StringBuilder azCommand) {
return token;
}

static OffsetDateTime getTokenExpiryOffsetDateTime(Map<String, String> objectMap) {
OffsetDateTime tokenExpiry;
if (objectMap.containsKey("expires_on")) {
Long seconds = Long.parseLong(objectMap.get("expires_on"));
tokenExpiry = EPOCH.plusSeconds(seconds);
} else {
String time = objectMap.get("expiresOn");
tokenExpiry = parseExpiresOnTime(time);
}
return tokenExpiry;
}

static OffsetDateTime parseExpiresOnTime(String time) {
OffsetDateTime tokenExpiry;
String timeToSecond = time.substring(0, time.indexOf("."));
String timeJoinedWithT = String.join("T", timeToSecond.split(" "));
tokenExpiry = LocalDateTime.parse(timeJoinedWithT, DateTimeFormatter.ISO_LOCAL_DATE_TIME)
.atZone(ZoneId.systemDefault())
.toOffsetDateTime().withOffsetSameInstant(ZoneOffset.UTC);
return tokenExpiry;
}

AccessToken getTokenFromAzureDeveloperCLIAuthentication(StringBuilder azdCommand) {
AccessToken token;
try {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.identity.implementation.models;

import com.azure.json.JsonReader;
import com.azure.json.JsonSerializable;
import com.azure.json.JsonToken;
import com.azure.json.JsonWriter;

import java.io.IOException;
import java.time.Instant;
import java.time.LocalDateTime;
import java.time.OffsetDateTime;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.format.DateTimeFormatter;

/**
* A wrapper class for deserializing a token payload returned from the Azure CLI.
*/
public final class AzureCliToken implements JsonSerializable<AzureCliToken> {
private String accessToken;
private String expiresOn;
private Long expiresOnUnixTime;
private String subscription;
private String tenant;
private String tokenType;
private OffsetDateTime tokenExpiry;

public String getAccessToken() {
return accessToken;
}

public String getExpiresOn() {
return expiresOn;
}

public Long getExpiresOnUnixTime() {
return expiresOnUnixTime;
}

public String getSubscription() {
return subscription;
}

public String getTenant() {
return tenant;
}

public String getTokenType() {
return tokenType;
}

public OffsetDateTime getTokenExpiration() {
return tokenExpiry;
}

private static OffsetDateTime parseExpiresOnTime(String time) {
OffsetDateTime tokenExpiry;
// parse the incoming date: 2024-02-28 12:05:53.000000
tokenExpiry = LocalDateTime.parse(time, DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss.SSSSSS"))
.atZone(ZoneId.systemDefault())
.toOffsetDateTime().withOffsetSameInstant(ZoneOffset.UTC);
return tokenExpiry;
}

public JsonWriter toJson(JsonWriter jsonWriter) throws IOException {
jsonWriter.writeStartObject();
jsonWriter.writeStringField("accessToken", accessToken);
jsonWriter.writeStringField("expiresOn", expiresOn);
jsonWriter.writeNumberField("expires_on", expiresOnUnixTime);
jsonWriter.writeStringField("subscription", subscription);
jsonWriter.writeStringField("tenant", tenant);
jsonWriter.writeStringField("tokenType", tokenType);
jsonWriter.writeEndObject();
return jsonWriter;
}

public static AzureCliToken fromJson(JsonReader jsonReader) throws IOException {
return jsonReader.readObject(reader -> {
AzureCliToken tokenHolder = new AzureCliToken();
while (reader.nextToken() != JsonToken.END_OBJECT) {
String fieldName = reader.getFieldName();
reader.nextToken();
if ("accessToken".equals(fieldName)) {
tokenHolder.accessToken = reader.getString();
} else if ("expiresOn".equals(fieldName)) {
tokenHolder.expiresOn = reader.getString();
} else if ("expires_on".equals(fieldName)) {
tokenHolder.expiresOnUnixTime = reader.getLong();
} else if ("subscription".equals(fieldName)) {
tokenHolder.subscription = reader.getString();
} else if ("tenant".equals(fieldName)) {
tokenHolder.tenant = reader.getString();
} else if ("tokenType".equals(fieldName)) {
tokenHolder.tokenType = reader.getString();
} else {
reader.skipChildren();
}
}

if (tokenHolder.expiresOnUnixTime != null) {
tokenHolder.tokenExpiry = Instant.ofEpochSecond(tokenHolder.getExpiresOnUnixTime()).atOffset(ZoneOffset.UTC);
} else {
tokenHolder.tokenExpiry = parseExpiresOnTime(tokenHolder.getExpiresOn());
}

return tokenHolder;
});
}
}
1 change: 1 addition & 0 deletions sdk/identity/azure-identity/src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

module com.azure.identity {
requires transitive com.azure.core;
requires com.azure.json;

requires com.microsoft.aad.msal4j;
requires msal4j.persistence.extension;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import com.microsoft.aad.msal4j.PublicClientApplication;
import com.microsoft.aad.msal4j.SilentParameters;
import com.microsoft.aad.msal4j.UserNamePasswordParameters;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.mockito.AdditionalMatchers;
Expand All @@ -47,12 +46,12 @@
import java.time.OffsetDateTime;
import java.time.ZoneOffset;
import java.time.format.DateTimeFormatter;
import java.util.HashMap;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;

import static org.junit.jupiter.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.anyBoolean;
Expand Down Expand Up @@ -93,24 +92,6 @@ public void testValidSecret() {
});
}

@Test
public void testExpiresOnParsingAzureCli() {
// setup
Map<String, String> tokenDetails = new HashMap<>();

//Epoch equivalent of "2023-10-31 21:59:10.000000" is 1698814750;
String expiresOn = "2023-10-31 21:59:10.000000";
tokenDetails.put("expiresOn", expiresOn);
OffsetDateTime offsetDateTime = IdentityClientBase.getTokenExpiryOffsetDateTime(tokenDetails);
Assertions.assertEquals(offsetDateTime.toEpochSecond(),
IdentityClientBase.parseExpiresOnTime(expiresOn).toEpochSecond());

// Test the scenario with expires_on present, it should be given priority.
tokenDetails.put("expires_on", "1572371520");
offsetDateTime = IdentityClientBase.getTokenExpiryOffsetDateTime(tokenDetails);
Assertions.assertEquals(offsetDateTime.toEpochSecond(), 1572371520);
}

@Test
public void testInvalidSecret() {
// setup
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.identity.implementation.models;

import com.azure.json.JsonProviders;
import com.azure.json.JsonReader;
import com.azure.json.JsonWriter;
import org.junit.jupiter.api.Test;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.time.Clock;
import java.time.Instant;
import java.time.LocalDateTime;
import java.time.OffsetDateTime;
import java.time.ZoneId;
import java.time.format.DateTimeFormatter;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;

public class AzureCliTokenTests {

String jsonWithExpiresOnUnixTime = "{\n"
+ " \"accessToken\": \"tokenValue\",\n"
+ " \"expiresOn\": \"2024-02-28 12:05:53.000000\",\n"
+ " \"expires_on\": 1709150753,\n"
+ " \"subscription\": \"subscriptionValue\",\n"
+ " \"tenant\": \"tenantValue\",\n"
+ " \"tokenType\": \"Bearer\"\n"
+ "}";

// This is the payload that gets parsed in the fallback case. It does not have time zone information.
// For test purposes, we need to inject the current time here, so the test works in different regions.
String jsonWithoutExpiresOnUnixTime = "{\n"
+ " \"accessToken\": \"tokenValue\",\n"
+ " \"expiresOn\": \"%s\",\n"
+ " \"subscription\": \"subscriptionValue\",\n"
+ " \"tenant\": \"tenantValue\",\n"
+ " \"tokenType\": \"Bearer\"\n"
+ "}";
@Test
public void testRoundTripWithoutExpiresOnUnixTime() {
ByteArrayOutputStream stream = new ByteArrayOutputStream();
// This test is largely testing the round trip and conversion of the local time returned from az
// to a UTC time. Set up the current time to allow this to work in all time zones.
Clock clock = Clock.fixed(Instant.parse("2024-02-28T20:05:53.123456Z"), ZoneId.of("Z"));
OffsetDateTime expected = OffsetDateTime.now(clock);
LocalDateTime localNow = expected.atZoneSameInstant(ZoneId.systemDefault()).toLocalDateTime();
expected = expected.atZoneSameInstant(ZoneId.of("Z")).toOffsetDateTime();
// recreate the incorrect date format from az
String nowString = localNow.format(DateTimeFormatter.ISO_DATE) + " " + localNow.format(DateTimeFormatter.ISO_TIME);
String localJson = String.format(jsonWithoutExpiresOnUnixTime, nowString);

try {
try (JsonReader reader = JsonProviders.createReader(localJson)) {
AzureCliToken token = AzureCliToken.fromJson(reader);
JsonWriter writer = JsonProviders.createWriter(stream);
token.toJson(writer);
assertNull(token.getExpiresOnUnixTime());
assertEquals("tokenValue", token.getAccessToken());
assertEquals(nowString, token.getExpiresOn());
assertEquals("subscriptionValue", token.getSubscription());
assertEquals("tenantValue", token.getTenant());
assertEquals("Bearer", token.getTokenType());
assertEquals(expected, token.getTokenExpiration());
}
} catch (IOException e) {
throw new RuntimeException(e);
}
}

@Test
public void testRoundTripWithExpiresOnUnixTime() {
ByteArrayOutputStream stream = new ByteArrayOutputStream();

try {
try (JsonReader reader = JsonProviders.createReader(jsonWithExpiresOnUnixTime)) {
AzureCliToken token = AzureCliToken.fromJson(reader);
JsonWriter writer = JsonProviders.createWriter(stream);
token.toJson(writer);
assertEquals("tokenValue", token.getAccessToken());
assertEquals("2024-02-28 12:05:53.000000", token.getExpiresOn());
assertEquals(1709150753, token.getExpiresOnUnixTime());
assertEquals("subscriptionValue", token.getSubscription());
assertEquals("tenantValue", token.getTenant());
assertEquals("Bearer", token.getTokenType());
// this test works fine with the hardcoded values since we don't care about the conversion through
// local time.
assertEquals(OffsetDateTime.parse("2024-02-28T20:05:53Z"), token.getTokenExpiration());
}
} catch (IOException e) {
throw new RuntimeException(e);
}
}

// This test validates the json parsing works both ways.
@Test
public void testDoubleRoundTrip() {
ByteArrayOutputStream stream = new ByteArrayOutputStream();

try (JsonReader reader = JsonProviders.createReader(jsonWithExpiresOnUnixTime)) {
AzureCliToken token = AzureCliToken.fromJson(reader);
JsonWriter writer = JsonProviders.createWriter(stream);
token.toJson(writer);
writer.close();

try (JsonReader reader2 = JsonProviders.createReader(stream.toByteArray())) {
AzureCliToken token2 = AzureCliToken.fromJson(reader2);
assertEquals("tokenValue", token2.getAccessToken());
assertEquals("2024-02-28 12:05:53.000000", token2.getExpiresOn());
assertEquals(1709150753, token2.getExpiresOnUnixTime());
assertEquals("subscriptionValue", token2.getSubscription());
assertEquals("tenantValue", token2.getTenant());
assertEquals("Bearer", token2.getTokenType());
assertEquals(OffsetDateTime.parse("2024-02-28T20:05:53Z"), token2.getTokenExpiration());
}
} catch (IOException e) {
throw new RuntimeException(e);
}
}
}

0 comments on commit 6506c0a

Please sign in to comment.