Skip to content

Commit

Permalink
Redact error output (#36232)
Browse files Browse the repository at this point in the history
* Redact error output

A bug was reported to me in which I observed that the exception message
for the CLI process timing out can include a warning and then the access
token. Azure CLI defaults to printing warnings. The fact that the output
is not pure JSON causes us to fail parsing it, and the exception
includes the token.

* add test
  • Loading branch information
billwert committed Aug 3, 2023
1 parent 3255f7f commit 4b74642
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,9 @@ AccessToken getTokenFromAzureCLIAuthentication(StringBuilder azCommand) {
.toOffsetDateTime().withOffsetSameInstant(ZoneOffset.UTC);
token = new AccessToken(accessToken, expiresOn);
} catch (IOException | InterruptedException e) {
throw LOGGER.logExceptionAsError(new IllegalStateException(e));
IllegalStateException ex = new IllegalStateException(redactInfo(e.getMessage()));
ex.setStackTrace(e.getStackTrace());
throw LOGGER.logExceptionAsError(ex);
}
return token;
}
Expand Down Expand Up @@ -681,7 +683,9 @@ AccessToken getTokenFromAzureDeveloperCLIAuthentication(StringBuilder azdCommand
.withOffsetSameInstant(ZoneOffset.UTC);
token = new AccessToken(accessToken, expiresOn);
} catch (IOException | InterruptedException e) {
throw LOGGER.logExceptionAsError(new IllegalStateException(e));
IllegalStateException ex = new IllegalStateException(redactInfo(e.getMessage()));
ex.setStackTrace(e.getStackTrace());
throw LOGGER.logExceptionAsError(ex);
}

return token;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
import java.util.UUID;
import java.util.concurrent.CompletableFuture;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
Expand Down Expand Up @@ -582,6 +584,22 @@ private void mockForClientCertificate(TokenRequestContext request, String access
}
}

@Test
public void validateRedaction() {
String s = " WARNING: Could not retrieve credential from local cache for service principal *** under tenant organizations. Trying credential under tenant 72f988bf-86f1-41af-91ab-2d7cd011db47, assuming that is an app credential.\n"
+ " {\n"
+ " \"accessToken\": \"ANACCESSTOKEN\",\n"
+ " \"expiresOn\": \"2023-08-03 12:29:07.000000\",\n"
+ " \"subscription\": \"subscription\",\n"
+ " \"tenant\": \"tenant\",\n"
+ " \"tokenType\": \"Bearer\"\n"
+ " }";
IdentityClient client = new IdentityClientBuilder().clientId("dummy").build();
String redacted = client.redactInfo(s);
assertTrue(redacted.contains("****"));
assertFalse(redacted.contains("accessToken"));
}

private void mockForDeviceCodeFlow(TokenRequestContext request, String accessToken, OffsetDateTime expiresOn, Runnable test) {
try (MockedConstruction<PublicClientApplication.Builder> publicClientApplicationMock = mockConstruction(PublicClientApplication.Builder.class, (builder, context) -> {
when(builder.authority(any())).thenReturn(builder);
Expand Down

0 comments on commit 4b74642

Please sign in to comment.