Skip to content

Commit 733b867

Browse files
goagainkyle-rader-msft
andauthored
AzureAuth: --resource should not be required if passing scopes (#142)
* AzureAuth: --resource should not be required if passing scopes * Add unit tests * Fix unit tests. * change method comments. * Remove unused imports. * Change log * Add a normal parameter unit test. * Update src/AzureAuth/CommandMain.cs Co-authored-by: Kyle W. Rader <kyle.rader@microsoft.com> * Unit test with right scope parameters for documentation. * Apply code suggestions. * address comments in PR. Change all scopes to <resource>/.default in unit test * Move `--resource` feature to unreleased section Co-authored-by: Kyle W. Rader <kyle.rader@microsoft.com>
1 parent 65c8f0a commit 733b867

File tree

3 files changed

+67
-9
lines changed

3 files changed

+67
-9
lines changed

CHANGELOG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
55
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
66

77
## [Unreleased]
8+
### Fixed
9+
- Option `--resource` is not needed if option `--scope` is provided.
10+
811
## [0.5.1] - 2022-09-08
912
### Fixed
1013
- Fixed a bug where we early exited before sending individual events telemetry data containing valuable `error_messages`.
@@ -14,7 +17,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1417
- Added functionality to disable Public Client Authentication using an environment variable `AZUREAUTH_NO_USER`.
1518
- Added `--timeout` functionality to provide reliable contract of allowed runtime (default: 15 minutes) and warnings as the timeout approaches.
1619

17-
### Fixed
1820
- Fixed a bug where broker auth prompt is hanging in the background and gives a false impression to the user that the console app is hung.
1921
- Fixed a bug where sometimes, when logged in with only a password (not a strong form of authentication) the broker flow could hang indefinitely, preventing fall back to another auth flow.
2022

src/AzureAuth.Test/CommandMainTest.cs

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ internal class CommandMainTest
3838
client = ""73e5793e-8f71-4da2-9f71-575cb3019b37""
3939
domain = ""contoso.com""
4040
tenant = ""a3be859b-7f9a-4955-98ed-f3602dbd954c""
41-
scopes = [ "".default"", ]
41+
scopes = [ ""67eeda51-3891-4101-a0e3-bf0c64047157/.default"", ]
4242
prompt_hint = ""sample prompt hint.""
4343
";
4444

@@ -169,7 +169,7 @@ public void TestEvaluateOptionsProvidedAliasWithoutCommandLineOptions()
169169
Client = "73e5793e-8f71-4da2-9f71-575cb3019b37",
170170
Domain = "contoso.com",
171171
Tenant = "a3be859b-7f9a-4955-98ed-f3602dbd954c",
172-
Scopes = new List<string> { ".default" },
172+
Scopes = new List<string> { "67eeda51-3891-4101-a0e3-bf0c64047157/.default" },
173173
PromptHint = "sample prompt hint.",
174174
};
175175

@@ -196,7 +196,7 @@ public void TestEvaluateOptionsProvidedAliasWithCommandLineOptions()
196196
Client = clientOverride,
197197
Domain = "contoso.com",
198198
Tenant = "a3be859b-7f9a-4955-98ed-f3602dbd954c",
199-
Scopes = new List<string> { ".default" },
199+
Scopes = new List<string> { "67eeda51-3891-4101-a0e3-bf0c64047157/.default" },
200200
PromptHint = "sample prompt hint.",
201201
};
202202

@@ -226,7 +226,7 @@ public void TestEvaluateOptionsProvidedAliasWithEnvVarConfig()
226226
Client = clientOverride,
227227
Domain = "contoso.com",
228228
Tenant = "a3be859b-7f9a-4955-98ed-f3602dbd954c",
229-
Scopes = new List<string> { ".default" },
229+
Scopes = new List<string> { "67eeda51-3891-4101-a0e3-bf0c64047157/.default" },
230230
PromptHint = "sample prompt hint.",
231231
};
232232

@@ -291,7 +291,7 @@ public void TestEvaluateOptionsWithoutAliasMissingResource()
291291
subject.Tenant = "9f6227ee-3d14-473e-8bed-1281171ef8c9";
292292

293293
subject.EvaluateOptions().Should().BeFalse();
294-
this.logTarget.Logs.Should().Contain("The --resource field is required.");
294+
this.logTarget.Logs.Should().Contain("The --resource field or the --scope field is required.");
295295
}
296296

297297
/// <summary>
@@ -338,12 +338,60 @@ public void TestEvaluateOptionsWithoutAliasMissingRequiredOptions()
338338
subject.EvaluateOptions().Should().BeFalse();
339339
this.logTarget.Logs.Should().Contain(new[]
340340
{
341-
"The --resource field is required.",
341+
"The --resource field or the --scope field is required.",
342342
"The --client field is required.",
343343
"The --tenant field is required.",
344344
});
345345
}
346346

347+
/// <summary>
348+
/// The test to evaluate options without resource but with scopes.
349+
/// </summary>
350+
[Test]
351+
public void TestEvaluateOptionsWithOverridedScopes()
352+
{
353+
CommandMain subject = this.serviceProvider.GetService<CommandMain>();
354+
subject.Resource = null;
355+
subject.Client = "e19f71ed-3b14-448d-9346-9eff9753646b";
356+
subject.Tenant = "9f6227ee-3d14-473e-8bed-1281171ef8c9";
357+
subject.Scopes = new string[] { "f0e8d801-3a50-48fd-b2da-6476d6e832a2/.default" };
358+
359+
subject.EvaluateOptions().Should().BeTrue();
360+
}
361+
362+
/// <summary>
363+
/// The test to evaluate options with normal parameters.
364+
/// </summary>
365+
[Test]
366+
public void TestEvaluateOptionsWithNormalParameters()
367+
{
368+
CommandMain subject = this.serviceProvider.GetService<CommandMain>();
369+
subject.Resource = "f0e8d801-3a50-48fd-b2da-6476d6e832a2";
370+
subject.Client = "e19f71ed-3b14-448d-9346-9eff9753646b";
371+
subject.Tenant = "9f6227ee-3d14-473e-8bed-1281171ef8c9";
372+
373+
subject.EvaluateOptions().Should().BeTrue();
374+
}
375+
376+
/// <summary>
377+
/// The test to evaluate options with both resource and scopes.
378+
/// </summary>
379+
[Test]
380+
public void TestEvaluateOptionsWithResourceAndScopes()
381+
{
382+
CommandMain subject = this.serviceProvider.GetService<CommandMain>();
383+
subject.Resource = "f0e8d801-3a50-48fd-b2da-6476d6e832a2";
384+
subject.Client = "e19f71ed-3b14-448d-9346-9eff9753646b";
385+
subject.Tenant = "9f6227ee-3d14-473e-8bed-1281171ef8c9";
386+
subject.Scopes = new string[] { "f0e8d801-3a50-48fd-b2da-6476d6e832a2/.default" };
387+
388+
subject.EvaluateOptions().Should().BeTrue();
389+
this.logTarget.Logs.Should().Contain(new[]
390+
{
391+
"The --scope option was provided with the --resource option. Only --scope will be used.",
392+
});
393+
}
394+
347395
/// <summary>
348396
/// The test to evaluate options without alias valid command line options.
349397
/// </summary>

src/AzureAuth/CommandMain.cs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -382,12 +382,20 @@ public bool PCADisabled()
382382
private bool ValidateOptions()
383383
{
384384
bool validOptions = true;
385-
if (string.IsNullOrEmpty(this.authSettings.Resource))
385+
386+
int scopesCount = this.authSettings.Scopes?.Count ?? 0;
387+
388+
if (string.IsNullOrEmpty(this.authSettings.Resource) && scopesCount == 0)
386389
{
387-
this.logger.LogError($"The {ResourceOption} field is required.");
390+
this.logger.LogError($"The {ResourceOption} field or the {ScopeOption} field is required.");
388391
validOptions = false;
389392
}
390393

394+
if (!string.IsNullOrEmpty(this.authSettings.Resource) && scopesCount > 0)
395+
{
396+
this.logger.LogWarning($"The {ScopeOption} option was provided with the {ResourceOption} option. Only {ScopeOption} will be used.");
397+
}
398+
391399
if (string.IsNullOrEmpty(this.authSettings.Client))
392400
{
393401
this.logger.LogError($"The {ClientOption} field is required.");

0 commit comments

Comments
 (0)