Skip to content

Commit 079c54b

Browse files
billwertbillwert
authored
Dev credential bugfixes (Azure#37122)
* Bug fixes for developer credentials * Release prep * Tests * fix version client * fixup versions * set dep version back to 1.10.1 * port tests to junit5 * use assertThrows * unused imports
1 parent 7fa1fc2 commit 079c54b

File tree

9 files changed

+291
-16
lines changed

9 files changed

+291
-16
lines changed

‎eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,4 +590,10 @@ the main ServiceBusClientBuilder. -->
590590
<suppress checks="com.azure.tools.checkstyle.checks.GoodLoggingCheck" files="ConsoleLogger.java"/>
591591
<suppress checks="com.azure.tools.checkstyle.checks.ThrowFromClientLoggerCheck"
592592
files="com.azure.sdk.build.tool.*\.java"/>
593+
594+
<!-- Identity tests use paramaterization which requires a public field -->
595+
<suppress checks="VisibilityModifier" files="com.azure.identity.AzureCliCredentialNegativeTest.java"/>
596+
<suppress checks="VisibilityModifier" files="com.azure.identity.AzureDeveloperCliCredentialNegativeTest.java"/>
597+
<suppress checks="VisibilityModifier" files="com.azure.identity.AzureCliCredentialNegativeTest.java"/>
598+
593599
</suppressions>

‎sdk/e2e/src/main/java/com/azure/endtoend/identity/CliToolsTest.java‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package com.azure.endtoend.identity;
5+
6+
import com.azure.core.credential.AccessToken;
7+
import com.azure.core.credential.TokenRequestContext;
8+
import com.azure.core.util.Configuration;
9+
import com.azure.core.util.CoreUtils;
10+
import com.azure.core.util.logging.ClientLogger;
11+
import com.azure.identity.AzureCliCredential;
12+
import com.azure.identity.AzureCliCredentialBuilder;
13+
import com.azure.identity.AzureDeveloperCliCredential;
14+
import com.azure.identity.AzureDeveloperCliCredentialBuilder;
15+
import com.azure.identity.AzurePowerShellCredential;
16+
import com.azure.identity.AzurePowerShellCredentialBuilder;
17+
18+
import java.util.Locale;
19+
20+
public class CliToolsTest {
21+
private static final String AZURE_CLI_TOOLS_TEST_MODE = "AZURE_CLI_TOOLS_TEST_MODE";
22+
private static final Configuration CONFIGURATION = Configuration.getGlobalConfiguration().clone();
23+
private static final ClientLogger logger = new ClientLogger(CliToolsTest.class);
24+
private static final String errorString = "Test mode is not set. Set environment variable AZURE_CLI_TOOLS_TEST_MODE to azd, azcli, or azps";
25+
public void run() {
26+
27+
if (CoreUtils.isNullOrEmpty(CONFIGURATION.get(AZURE_CLI_TOOLS_TEST_MODE))) {
28+
throw logger.logExceptionAsError(new IllegalStateException(errorString));
29+
}
30+
31+
32+
String mode = CONFIGURATION.get(AZURE_CLI_TOOLS_TEST_MODE).toLowerCase(Locale.ENGLISH);
33+
switch (mode) {
34+
case "azd":
35+
testAzd();
36+
break;
37+
case "azcli":
38+
testAzCli();
39+
break;
40+
case "azps":
41+
testAzPs();
42+
break;
43+
default:
44+
throw logger.logExceptionAsError(new IllegalStateException(errorString));
45+
}
46+
}
47+
48+
private void testAzd() {
49+
AzureDeveloperCliCredential credential = new AzureDeveloperCliCredentialBuilder().build();
50+
TokenRequestContext tokenRequestContext = new TokenRequestContext().addScopes("https://graph.microsoft.com/.default");
51+
AccessToken tokenCredential = credential.getToken(tokenRequestContext).block();
52+
if (tokenCredential == null) {
53+
System.out.println("Failed to get token from AzureDeveloperCliCredential");
54+
} else {
55+
System.out.println("Successfully got token from AzureDeveloperCliCredential");
56+
}
57+
}
58+
59+
private void testAzCli() {
60+
AzureCliCredential credential = new AzureCliCredentialBuilder().build();
61+
TokenRequestContext tokenRequestContext = new TokenRequestContext().addScopes("https://graph.microsoft.com/.default");
62+
AccessToken tokenCredential = credential.getToken(tokenRequestContext).block();
63+
if (tokenCredential == null) {
64+
System.out.println("Failed to get token from AzureCliCredential");
65+
} else {
66+
System.out.println("Successfully got token from AzureCliCredential");
67+
}
68+
}
69+
70+
private void testAzPs() {
71+
AzurePowerShellCredential credential = new AzurePowerShellCredentialBuilder().build();
72+
TokenRequestContext tokenRequestContext = new TokenRequestContext().addScopes("https://graph.microsoft.com/.default");
73+
AccessToken tokenCredential = credential.getToken(tokenRequestContext).block();
74+
if (tokenCredential == null) {
75+
System.out.println("Failed to get token from AzurePowerShellCredential");
76+
} else {
77+
System.out.println("Successfully got token from AzurePowerShellCredential");
78+
}
79+
}
80+
}

‎sdk/e2e/src/main/java/com/azure/endtoend/identity/IdentityTest.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
public class IdentityTest {
1515
private static final String AZURE_IDENTITY_TEST_PLATFORM = "AZURE_IDENTITY_TEST_PLATFORM";
1616
private static final Configuration CONFIGURATION = Configuration.getGlobalConfiguration().clone();
17+
private static final String errorString = "Identity Test platform is not set. Set environment variable AZURE_IDENTITY_TEST_PLATFORM to one of:\nwebjobs\nmultitenant\nclitools";
1718

1819
/**
1920
* Runs the Web jobs identity tests
@@ -22,8 +23,7 @@ public class IdentityTest {
2223
*/
2324
public static void main(String[] args) throws IllegalStateException {
2425
if (CoreUtils.isNullOrEmpty(CONFIGURATION.get(AZURE_IDENTITY_TEST_PLATFORM))) {
25-
throw new IllegalStateException("Identity Test platform is not set. Set environment "
26-
+ "variable AZURE_IDENTITY_TEST_PLATFORM to webjobs");
26+
throw new IllegalStateException(errorString);
2727
}
2828

2929
String platform = CONFIGURATION.get(AZURE_IDENTITY_TEST_PLATFORM).toLowerCase(Locale.ENGLISH);
@@ -36,9 +36,12 @@ public static void main(String[] args) throws IllegalStateException {
3636
MultiTenantTest multiTenantTest = new MultiTenantTest();
3737
multiTenantTest.run();
3838
break;
39+
case "clitools":
40+
CliToolsTest cliToolsTest = new CliToolsTest();
41+
cliToolsTest.run();
42+
break;
3943
default:
40-
throw (new IllegalStateException("Invalid Test Platform is configured for AZURE_IDENTITY_TEST_PLATFORM."
41-
+ "Possible value is webjobs."));
44+
throw new IllegalStateException(errorString);
4245
}
4346
}
4447
}

‎sdk/identity/azure-identity/CHANGELOG.md‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Release History
22

3-
## 1.11.0-beta.2 (Unreleased)
3+
## 1.11.0-beta.2 (unreleased)
44

55
### Features Added
66

@@ -10,6 +10,11 @@
1010

1111
### Other Changes
1212

13+
## 1.10.2 (2023-10-10)
14+
15+
### Bugs Fixed
16+
- Bug fixes for developer credentials
17+
1318
## 1.11.0-beta.1 (2023-09-20)
1419

1520
### Features Added
@@ -19,9 +24,6 @@
1924
- Fixed flowing `HttpClientOptions` through credentials [#36382](https://github.com/Azure/azure-sdk-for-java/pull/36382)
2025
- Fixed edge case in Docker where 403s erronously caused CredentialUnavailableExceptions [#36747](https://github.com/Azure/azure-sdk-for-java/pull/36747)
2126

22-
23-
24-
2527
## 1.10.1 (2023-09-10)
2628

2729
### Other Changes

‎sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import com.azure.identity.implementation.util.IdentityUtil;
1717
import com.azure.identity.implementation.util.LoggingUtil;
1818
import com.azure.identity.implementation.util.ScopeUtil;
19+
import com.azure.identity.implementation.util.ValidationUtil;
1920
import com.fasterxml.jackson.databind.JsonNode;
2021
import com.microsoft.aad.msal4j.AuthorizationCodeParameters;
2122
import com.microsoft.aad.msal4j.AppTokenProviderParameters;
@@ -315,7 +316,6 @@ public Mono<MsalToken> authenticateWithIntelliJ(TokenRequestContext request) {
315316
* @return a Publisher that emits an AccessToken
316317
*/
317318
public Mono<AccessToken> authenticateWithAzureCli(TokenRequestContext request) {
318-
319319
StringBuilder azCommand = new StringBuilder("az account get-access-token --output json --resource ");
320320

321321
String scopes = ScopeUtil.scopesToResource(request.getScopes());
@@ -330,11 +330,13 @@ public Mono<AccessToken> authenticateWithAzureCli(TokenRequestContext request) {
330330

331331
try {
332332
String tenant = IdentityUtil.resolveTenantId(tenantId, request, options);
333+
ValidationUtil.validateTenantIdCharacterRange(tenant, LOGGER);
334+
333335
// The default is not correct for many cases, such as when the logged in entity is a service principal.
334336
if (!CoreUtils.isNullOrEmpty(tenant) && !tenant.equals(IdentityUtil.DEFAULT_TENANT)) {
335337
azCommand.append(" --tenant ").append(tenant);
336338
}
337-
} catch (ClientAuthenticationException e) {
339+
} catch (ClientAuthenticationException | IllegalArgumentException e) {
338340
return Mono.error(e);
339341
}
340342

@@ -356,7 +358,6 @@ public Mono<AccessToken> authenticateWithAzureCli(TokenRequestContext request) {
356358
* @return a Publisher that emits an AccessToken
357359
*/
358360
public Mono<AccessToken> authenticateWithAzureDeveloperCli(TokenRequestContext request) {
359-
360361
StringBuilder azdCommand = new StringBuilder("azd auth token --output json --scope ");
361362
List<String> scopes = request.getScopes();
362363

@@ -366,16 +367,27 @@ public Mono<AccessToken> authenticateWithAzureDeveloperCli(TokenRequestContext r
366367
return Mono.error(LOGGER.logExceptionAsError(new IllegalArgumentException("Missing scope in request")));
367368
}
368369

370+
for (String scope : scopes) {
371+
try {
372+
ScopeUtil.validateScope(scope);
373+
} catch (IllegalArgumentException ex) {
374+
return Mono.error(LOGGER.logExceptionAsError(ex));
375+
}
376+
}
377+
378+
369379
// At least one scope is appended to the azd command.
370380
// If there are more than one scope, we add `--scope` before each.
371381
azdCommand.append(String.join(" --scope ", scopes));
372382

373383
try {
374384
String tenant = IdentityUtil.resolveTenantId(tenantId, request, options);
385+
ValidationUtil.validateTenantIdCharacterRange(tenant, LOGGER);
386+
375387
if (!CoreUtils.isNullOrEmpty(tenant) && !tenant.equals(IdentityUtil.DEFAULT_TENANT)) {
376388
azdCommand.append(" --tenant-id ").append(tenant);
377389
}
378-
} catch (ClientAuthenticationException e) {
390+
} catch (ClientAuthenticationException | IllegalArgumentException e) {
379391
return Mono.error(e);
380392
}
381393

@@ -396,7 +408,7 @@ public Mono<AccessToken> authenticateWithAzureDeveloperCli(TokenRequestContext r
396408
* @return a Publisher that emits an AccessToken
397409
*/
398410
public Mono<AccessToken> authenticateWithAzurePowerShell(TokenRequestContext request) {
399-
411+
ValidationUtil.validateTenantIdCharacterRange(tenantId, LOGGER);
400412
List<CredentialUnavailableException> exceptions = new ArrayList<>(2);
401413

402414
PowershellManager defaultPowerShellManager = new PowershellManager(Platform.isWindows()
@@ -454,6 +466,12 @@ public Mono<AccessToken> authenticateWithOBO(TokenRequestContext request) {
454466

455467
private Mono<AccessToken> getAccessTokenFromPowerShell(TokenRequestContext request,
456468
PowershellManager powershellManager) {
469+
String scope = ScopeUtil.scopesToResource(request.getScopes());
470+
try {
471+
ScopeUtil.validateScope(scope);
472+
} catch (IllegalArgumentException ex) {
473+
throw LOGGER.logExceptionAsError(ex);
474+
}
457475
return Mono.using(() -> powershellManager, manager -> manager.initSession().flatMap(m -> {
458476
String azAccountsCommand = "Import-Module Az.Accounts -MinimumVersion 2.2.0 -PassThru";
459477
return m.runCommand(azAccountsCommand).flatMap(output -> {
@@ -466,8 +484,9 @@ private Mono<AccessToken> getAccessTokenFromPowerShell(TokenRequestContext reque
466484
}
467485

468486
LOGGER.verbose("Az.accounts module was found installed.");
469-
String command = "Get-AzAccessToken -ResourceUrl " + ScopeUtil.scopesToResource(request.getScopes())
470-
+ " | ConvertTo-Json";
487+
String command = "Get-AzAccessToken -ResourceUrl '"
488+
+ scope
489+
+ "' | ConvertTo-Json";
471490
LOGGER.verbose("Azure Powershell Authentication => Executing the command `{}` in Azure "
472491
+ "Powershell to retrieve the Access Token.", command);
473492

‎sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentitySyncClient.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import com.azure.identity.implementation.util.IdentityUtil;
1313
import com.azure.identity.implementation.util.LoggingUtil;
1414
import com.azure.identity.implementation.util.ScopeUtil;
15+
import com.azure.identity.implementation.util.ValidationUtil;
1516
import com.microsoft.aad.msal4j.AppTokenProviderParameters;
1617
import com.microsoft.aad.msal4j.ClaimsRequest;
1718
import com.microsoft.aad.msal4j.ClientCredentialFactory;
@@ -345,7 +346,6 @@ public MsalToken authenticateWithBrowserInteraction(TokenRequestContext request,
345346
* @return a Publisher that emits an AccessToken
346347
*/
347348
public AccessToken authenticateWithAzureCli(TokenRequestContext request) {
348-
349349
StringBuilder azCommand = new StringBuilder("az account get-access-token --output json --resource ");
350350

351351
String scopes = ScopeUtil.scopesToResource(request.getScopes());
@@ -359,9 +359,11 @@ public AccessToken authenticateWithAzureCli(TokenRequestContext request) {
359359
azCommand.append(scopes);
360360

361361
String tenant = IdentityUtil.resolveTenantId(tenantId, request, options);
362+
ValidationUtil.validateTenantIdCharacterRange(tenant, LOGGER);
362363

363364
if (!CoreUtils.isNullOrEmpty(tenant)) {
364365
azCommand.append(" --tenant ").append(tenant);
366+
365367
}
366368

367369
try {
@@ -392,11 +394,22 @@ public AccessToken authenticateWithAzureDeveloperCli(TokenRequestContext request
392394
throw LOGGER.logExceptionAsError(new IllegalArgumentException("Missing scope in request"));
393395
}
394396

397+
scopes.forEach(scope -> {
398+
try {
399+
ScopeUtil.validateScope(scope);
400+
} catch (IllegalArgumentException ex) {
401+
throw LOGGER.logExceptionAsError(ex);
402+
}
403+
});
404+
405+
395406
// At least one scope is appended to the azd command.
396407
// If there are more than one scope, we add `--scope` before each.
397408
azdCommand.append(String.join(" --scope ", scopes));
398409

399410
String tenant = IdentityUtil.resolveTenantId(tenantId, request, options);
411+
ValidationUtil.validateTenantIdCharacterRange(tenant, LOGGER);
412+
400413
if (!CoreUtils.isNullOrEmpty(tenant)) {
401414
azdCommand.append(" --tenant-id ").append(tenant);
402415
}

‎sdk/identity/azure-identity/src/test/java/com/azure/identity/AzureCliCredentialNegativeTest.java‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package com.azure.identity;
5+
6+
import com.azure.core.credential.TokenRequestContext;
7+
import org.junit.jupiter.params.ParameterizedTest;
8+
import org.junit.jupiter.params.provider.MethodSource;
9+
import reactor.test.StepVerifier;
10+
11+
import java.util.stream.Stream;
12+
13+
import static org.junit.jupiter.api.Assertions.assertThrows;
14+
15+
16+
public class AzureCliCredentialNegativeTest {
17+
18+
static Stream<String> invalidCharacters() {
19+
return Stream.of("|", "&", ";");
20+
}
21+
@ParameterizedTest
22+
@MethodSource("invalidCharacters")
23+
public void testInvalidScopeFromRequest(String invalidCharacter) {
24+
TokenRequestContext request = new TokenRequestContext().addScopes("scope" + invalidCharacter);
25+
26+
27+
AzureCliCredential credential = new AzureCliCredentialBuilder().build();
28+
29+
StepVerifier.create(credential.getToken(request))
30+
.expectErrorMatches(e -> e instanceof IllegalArgumentException)
31+
.verify();
32+
}
33+
@ParameterizedTest
34+
@MethodSource("invalidCharacters")
35+
public void testInvalidTenantFromRequest(String invalidCharacter) {
36+
TokenRequestContext request = new TokenRequestContext().addScopes("scope").setTenantId("tenant" + invalidCharacter);
37+
AzureCliCredential credential = new AzureCliCredentialBuilder().build();
38+
39+
StepVerifier.create(credential.getToken(request))
40+
.expectErrorMatches(e -> e instanceof IllegalArgumentException)
41+
.verify();
42+
}
43+
44+
@ParameterizedTest
45+
@MethodSource("invalidCharacters")
46+
public void testInvalidScopeFromRequestSync(String invalidCharacter) {
47+
TokenRequestContext request = new TokenRequestContext().addScopes("scope" + invalidCharacter);
48+
49+
AzureCliCredential credential = new AzureCliCredentialBuilder().build();
50+
assertThrows(IllegalArgumentException.class, () -> credential.getTokenSync(request));
51+
}
52+
53+
@ParameterizedTest
54+
@MethodSource("invalidCharacters")
55+
public void testInvalidTenantFromRequestSync(String invalidCharacter) {
56+
TokenRequestContext request = new TokenRequestContext().addScopes("scope").setTenantId("tenant" + invalidCharacter);
57+
AzureCliCredential credential = new AzureCliCredentialBuilder().build();
58+
assertThrows(IllegalArgumentException.class, () -> credential.getTokenSync(request));
59+
}
60+
}

0 commit comments

Comments
 (0)