Skip to content

Commit c60bbaa

Browse files
dggsaxdggsax
authored
Merge pull request #450 from AzureAD/users/danigon/system-access-token-surprises
Make ado token command environment-aware for SYSTEM_ACCESSTOKEN
2 parents c4c1bb0 + a536bfd commit c60bbaa

8 files changed

Lines changed: 221 additions & 19 deletions

File tree

‎nuget.config‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,12 @@
44
<clear />
55
<add key="Office" value="https://pkgs.dev.azure.com/office/_packaging/Office/nuget/v3/index.json"/>
66
</packageSources>
7+
<!-- Provide ADO_TOKEN for authenticated package restore:
8+
ADO_TOKEN=$(azureauth ado token) dotnet restore -->
9+
<packageSourceCredentials>
10+
<Office>
11+
<add key="Username" value="UnusedWithToken" />
12+
<add key="ClearTextPassword" value="%ADO_TOKEN%" />
13+
</Office>
14+
</packageSourceCredentials>
715
</configuration>

‎src/AzureAuth.Test/Commands/Ado/CommandTokenTest.cs‎

Lines changed: 153 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,27 +3,177 @@
33

44
namespace AzureAuth.Test.Commands.Ado
55
{
6+
using System;
7+
using System.Collections.Generic;
8+
69
using FluentAssertions;
7-
using Microsoft.Authentication.AzureAuth.Ado;
10+
using AzureAuth.Test;
11+
using Microsoft.Authentication.AzureAuth;
812
using Microsoft.Authentication.AzureAuth.Commands.Ado;
13+
using Microsoft.Authentication.MSALWrapper;
14+
using Microsoft.Extensions.DependencyInjection;
15+
using Microsoft.Extensions.Logging;
16+
using Microsoft.IdentityModel.JsonWebTokens;
17+
using Microsoft.Office.Lasso.Interfaces;
18+
using Microsoft.Office.Lasso.Telemetry;
19+
using Moq;
20+
using NLog.Extensions.Logging;
21+
using NLog.Targets;
922
using NUnit.Framework;
1023

1124
internal class CommandTokenTest
1225
{
26+
private Mock<IEnv> mockEnv;
27+
private Mock<ITelemetryService> mockTelemetry;
28+
private Mock<IPublicClientAuth> mockPublicClientAuth;
29+
private IServiceProvider serviceProvider;
30+
private MemoryTarget logTarget;
31+
private CommandExecuteEventData eventData;
32+
33+
[SetUp]
34+
public void SetUp()
35+
{
36+
this.mockEnv = new Mock<IEnv>();
37+
this.mockEnv.Setup(e => e.Get(It.IsAny<string>())).Returns((string)null);
38+
this.mockTelemetry = new Mock<ITelemetryService>();
39+
this.mockPublicClientAuth = new Mock<IPublicClientAuth>();
40+
this.eventData = new CommandExecuteEventData();
41+
42+
// Setup in memory logging target with NLog - allows making assertions against what has been logged.
43+
var loggingConfig = new NLog.Config.LoggingConfiguration();
44+
this.logTarget = new MemoryTarget("memory_target")
45+
{
46+
Layout = "${message}" // Define a simple layout so we don't get timestamps in messages.
47+
};
48+
loggingConfig.AddTarget(this.logTarget);
49+
loggingConfig.AddRuleForAllLevels(this.logTarget);
50+
51+
// Setup Dependency Injection container to provide logger.
52+
this.serviceProvider = new ServiceCollection()
53+
.AddLogging(loggingBuilder =>
54+
{
55+
loggingBuilder.ClearProviders();
56+
loggingBuilder.SetMinimumLevel(LogLevel.Trace);
57+
loggingBuilder.AddNLog(loggingConfig);
58+
})
59+
.BuildServiceProvider();
60+
}
61+
1362
[TestCase("foobar", CommandToken.OutputMode.Token, "foobar")]
1463
[TestCase("foobar", CommandToken.OutputMode.HeaderValue, "Basic OmZvb2Jhcg==")]
1564
[TestCase("foobar", CommandToken.OutputMode.Header, "Authorization: Basic OmZvb2Jhcg==")]
1665
public void FormatToken_Basic(string input, CommandToken.OutputMode mode, string expected)
1766
{
18-
CommandToken.FormatToken(input, mode, Authorization.Basic).Should().Be(expected);
67+
CommandToken.FormatToken(input, mode, Microsoft.Authentication.AzureAuth.Ado.Authorization.Basic).Should().Be(expected);
1968
}
2069

2170
[TestCase("foobar", CommandToken.OutputMode.Token, "foobar")]
2271
[TestCase("foobar", CommandToken.OutputMode.HeaderValue, "Bearer foobar")]
2372
[TestCase("foobar", CommandToken.OutputMode.Header, "Authorization: Bearer foobar")]
2473
public void FormatToken_Bearer(string input, CommandToken.OutputMode mode, string expected)
2574
{
26-
CommandToken.FormatToken(input, mode, Authorization.Bearer).Should().Be(expected);
75+
CommandToken.FormatToken(input, mode, Microsoft.Authentication.AzureAuth.Ado.Authorization.Bearer).Should().Be(expected);
76+
}
77+
78+
[Test]
79+
public void OnExecute_AzureAuthAdoPat_AlwaysUsed()
80+
{
81+
this.mockEnv.Setup(e => e.Get(EnvVars.AdoPat)).Returns("my-explicit-pat");
82+
83+
var command = new CommandToken();
84+
var result = command.OnExecute(
85+
this.serviceProvider.GetService<ILogger<CommandToken>>(),
86+
this.mockEnv.Object,
87+
this.mockTelemetry.Object,
88+
this.mockPublicClientAuth.Object,
89+
this.eventData);
90+
91+
result.Should().Be(0);
92+
this.mockPublicClientAuth.Verify(
93+
p => p.Token(It.IsAny<AuthParameters>(), It.IsAny<IEnumerable<AuthMode>>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<TimeSpan>(), It.IsAny<EventData>()),
94+
Times.Never);
95+
}
96+
97+
[Test]
98+
public void OnExecute_AdoPipeline_UsesSystemAccessToken()
99+
{
100+
this.mockEnv.Setup(e => e.Get(EnvVars.TfBuild)).Returns("True");
101+
this.mockEnv.Setup(e => e.Get(EnvVars.SystemAccessToken)).Returns("pipeline-token");
102+
103+
var command = new CommandToken();
104+
var result = command.OnExecute(
105+
this.serviceProvider.GetService<ILogger<CommandToken>>(),
106+
this.mockEnv.Object,
107+
this.mockTelemetry.Object,
108+
this.mockPublicClientAuth.Object,
109+
this.eventData);
110+
111+
result.Should().Be(0);
112+
this.mockPublicClientAuth.Verify(
113+
p => p.Token(It.IsAny<AuthParameters>(), It.IsAny<IEnumerable<AuthMode>>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<TimeSpan>(), It.IsAny<EventData>()),
114+
Times.Never);
115+
}
116+
117+
[Test]
118+
public void OnExecute_AdoPipeline_NoSystemAccessToken_ReturnsError()
119+
{
120+
this.mockEnv.Setup(e => e.Get(EnvVars.TfBuild)).Returns("True");
121+
122+
var command = new CommandToken();
123+
var result = command.OnExecute(
124+
this.serviceProvider.GetService<ILogger<CommandToken>>(),
125+
this.mockEnv.Object,
126+
this.mockTelemetry.Object,
127+
this.mockPublicClientAuth.Object,
128+
this.eventData);
129+
130+
result.Should().Be(1);
131+
this.logTarget.Logs.Should().Contain(l => l.Contains($"{EnvVars.SystemAccessToken} is not set"));
132+
}
133+
134+
[Test]
135+
public void OnExecute_NotAdoPipeline_SystemAccessTokenSet_WarnsAndContinues()
136+
{
137+
this.mockEnv.Setup(e => e.Get(EnvVars.SystemAccessToken)).Returns("stale-token");
138+
var fakeTokenResult = new TokenResult(new JsonWebToken(Fake.Token), Guid.NewGuid());
139+
this.mockPublicClientAuth
140+
.Setup(p => p.Token(It.IsAny<AuthParameters>(), It.IsAny<IEnumerable<AuthMode>>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<TimeSpan>(), It.IsAny<EventData>()))
141+
.Returns(fakeTokenResult);
142+
143+
var command = new CommandToken();
144+
var result = command.OnExecute(
145+
this.serviceProvider.GetService<ILogger<CommandToken>>(),
146+
this.mockEnv.Object,
147+
this.mockTelemetry.Object,
148+
this.mockPublicClientAuth.Object,
149+
this.eventData);
150+
151+
result.Should().Be(0);
152+
this.logTarget.Logs.Should().Contain(l => l.Contains("does not appear to be an Azure DevOps Pipeline environment"));
153+
154+
// Verify it fell through to AAD auth (ignored the SYSTEM_ACCESSTOKEN)
155+
this.mockPublicClientAuth.Verify(
156+
p => p.Token(It.IsAny<AuthParameters>(), It.IsAny<IEnumerable<AuthMode>>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<TimeSpan>(), It.IsAny<EventData>()),
157+
Times.Once);
158+
}
159+
160+
[Test]
161+
public void OnExecute_AdoPipeline_AzureAuthAdoPat_TakesPriority()
162+
{
163+
this.mockEnv.Setup(e => e.Get(EnvVars.AdoPat)).Returns("my-explicit-pat");
164+
this.mockEnv.Setup(e => e.Get(EnvVars.TfBuild)).Returns("True");
165+
this.mockEnv.Setup(e => e.Get(EnvVars.SystemAccessToken)).Returns("pipeline-token");
166+
167+
var command = new CommandToken();
168+
var result = command.OnExecute(
169+
this.serviceProvider.GetService<ILogger<CommandToken>>(),
170+
this.mockEnv.Object,
171+
this.mockTelemetry.Object,
172+
this.mockPublicClientAuth.Object,
173+
this.eventData);
174+
175+
result.Should().Be(0);
176+
this.logTarget.Logs.Should().Contain(l => l.Contains(EnvVars.AdoPat));
27177
}
28178
}
29179
}

‎src/AzureAuth.Test/IEnvExtensionsTest.cs‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ namespace AzureAuth.Test
1010
using Microsoft.Authentication.TestHelper;
1111
using Microsoft.Extensions.Logging;
1212
using Microsoft.Office.Lasso.Interfaces;
13-
using Microsoft.Office.Lasso.Telemetry;
1413
using Moq;
1514
using NLog.Targets;
1615
using NUnit.Framework;
@@ -58,6 +57,22 @@ public void InteractiveAuth_IsEnabledIfEnvVarsAreNotSet()
5857
IEnvExtensions.InteractiveAuthDisabled(this.envMock.Object).Should().BeFalse();
5958
}
6059

60+
[TestCase("True", true)]
61+
[TestCase("true", true)]
62+
[TestCase("TRUE", true)]
63+
[TestCase("1", true)]
64+
[TestCase("0", false)]
65+
[TestCase("False", false)]
66+
[TestCase("", false)]
67+
[TestCase(null, false)]
68+
public void IsAdoPipeline_DetectsTfBuildEnvVar(string tfBuild, bool expected)
69+
{
70+
this.envMock.Setup(env => env.Get(It.IsAny<string>())).Returns((string)null);
71+
this.envMock.Setup(e => e.Get(EnvVars.TfBuild)).Returns(tfBuild);
72+
73+
IEnvExtensions.IsAdoPipeline(this.envMock.Object).Should().Be(expected);
74+
}
75+
6176
[Test]
6277
public void ReadAuthModeFromEnvOrSetDefault_ReturnsDefault_WhenEnvVarIsEmpty()
6378
{

‎src/AzureAuth/Ado/PatFromEnv.cs‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,12 @@
33

44
namespace Microsoft.Authentication.AzureAuth.Ado
55
{
6-
using System;
76
using System.Collections.Generic;
8-
using System.Linq;
9-
using System.Threading.Tasks;
107

11-
using Microsoft.Authentication.MSALWrapper;
12-
using Microsoft.Authentication.MSALWrapper.AuthFlow;
13-
using Microsoft.Extensions.Logging;
148
using Microsoft.Office.Lasso.Interfaces;
159

1610
/// <summary>
17-
/// A class for getting an ADO PAT from an <see cref="IEnv"/> or an AAD access token through MSAL.
11+
/// A class for getting an ADO PAT from an <see cref="IEnv"/>.
1812
/// </summary>
1913
public static class PatFromEnv
2014
{

‎src/AzureAuth/Commands/Ado/CommandToken.cs‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,32 @@ public enum OutputMode
8989
/// <returns>An integer status code. 0 for success and non-zero for failure.</returns>
9090
public int OnExecute(ILogger<CommandToken> logger, IEnv env, ITelemetryService telemetryService, IPublicClientAuth publicClientAuth, CommandExecuteEventData eventData)
9191
{
92-
// First attempt using a PAT.
92+
// First attempt using a PAT from the environment.
9393
var pat = PatFromEnv.Get(env);
9494
if (pat.Exists)
9595
{
96-
logger.LogDebug($"Using PAT from env var {pat.EnvVarSource}");
97-
logger.LogInformation(FormatToken(pat.Value, this.Output, Authorization.Basic));
98-
return 0;
96+
// SYSTEM_ACCESSTOKEN should only be used inside an ADO Pipeline.
97+
if (pat.EnvVarSource == EnvVars.SystemAccessToken && !env.IsAdoPipeline())
98+
{
99+
logger.LogWarning(
100+
$"{EnvVars.SystemAccessToken} is set but this does not appear to be an Azure DevOps Pipeline environment. "
101+
+ "Having this variable set on a developer machine is unusual. It will be ignored.");
102+
}
103+
else
104+
{
105+
logger.LogDebug($"Using PAT from env var {pat.EnvVarSource}");
106+
logger.LogInformation(FormatToken(pat.Value, this.Output, Authorization.Basic));
107+
return 0;
108+
}
109+
}
110+
else if (env.IsAdoPipeline())
111+
{
112+
// In a pipeline but no token was found at all.
113+
logger.LogError(
114+
$"Running in an Azure DevOps Pipeline environment but {EnvVars.SystemAccessToken} is not set. "
115+
+ "Interactive authentication is not possible in a pipeline. "
116+
+ "Ensure the pipeline has access to the system token.");
117+
return 1;
99118
}
100119

101120
// If command line options for mode are not specified, then use the environment variables.

‎src/AzureAuth/Commands/Ado/Pat/CommandScopes.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@
1010
namespace Microsoft.Authentication.AzureAuth.Commands.Ado.Pat
1111
{
1212
/// <summary>
13-
/// Command to print the list of available scopes
14-
13+
/// Command to print the list of available scopes for ADO PATs.
1514
/// </summary>
1615
[Command("scopes", Description = "List the valid ado pat scopes")]
1716
public class CommandScopes

‎src/AzureAuth/EnvVars.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ public static class EnvVars
3636
/// </summary>
3737
public static readonly string NoUser = $"{EnvVarPrefix}_NO_USER";
3838

39+
/// <summary>
40+
/// Name of the env var set by Azure DevOps Pipelines to indicate a pipeline environment.
41+
/// Value is "True" when running in an ADO Pipeline.
42+
/// </summary>
43+
public const string TfBuild = "TF_BUILD";
44+
3945
/// <summary>
4046
/// Name of the env var for the Azure DevOps pipelines default personal access token.
4147
/// </summary>

‎src/AzureAuth/IEnvExtensions.cs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ namespace Microsoft.Authentication.AzureAuth
55
{
66
using Microsoft.Authentication.MSALWrapper;
77
using Microsoft.Office.Lasso.Interfaces;
8-
using Microsoft.Office.Lasso.Telemetry;
98
using System.Collections.Generic;
109
using System;
1110

@@ -14,8 +13,21 @@ namespace Microsoft.Authentication.AzureAuth
1413
/// </summary>
1514
public static class IEnvExtensions
1615
{
16+
private const string AdoPositiveValue = "True";
1717
private const string CorextPositiveValue = "1";
1818

19+
/// <summary>
20+
/// Determines whether we are running in an Azure DevOps Pipeline environment.
21+
/// </summary>
22+
/// <param name="env">The <see cref="IEnv"/> to use to get environment variables.</param>
23+
/// <returns>True if running in an Azure DevOps Pipeline.</returns>
24+
public static bool IsAdoPipeline(this IEnv env)
25+
{
26+
string tfBuild = env.Get(EnvVars.TfBuild);
27+
return string.Equals(AdoPositiveValue, tfBuild, StringComparison.OrdinalIgnoreCase)
28+
|| string.Equals(CorextPositiveValue, tfBuild, StringComparison.Ordinal);
29+
}
30+
1931
/// <summary>
2032
/// Determines whether interactive auth is disabled or not.
2133
/// </summary>
@@ -31,7 +43,6 @@ public static bool InteractiveAuthDisabled(this IEnv env)
3143
/// Get the auth modes from the environment or set the default.
3244
/// </summary>
3345
/// <param name="env">The <see cref="IEnv"/> to use.</param>
34-
/// <param name="eventData">Event data to add the auth mode to.</param>
3546
/// <returns>AuthModes.</returns>
3647
public static IEnumerable<AuthMode> ReadAuthModeFromEnvOrSetDefault(this IEnv env)
3748
{

0 commit comments

Comments
 (0)