diff --git a/nuget.config b/nuget.config index 6338e895..a3246f9f 100644 --- a/nuget.config +++ b/nuget.config @@ -4,4 +4,12 @@ + + + + + + + \ No newline at end of file diff --git a/src/AzureAuth.Test/Commands/Ado/CommandTokenTest.cs b/src/AzureAuth.Test/Commands/Ado/CommandTokenTest.cs index f13f8a8c..e92e0176 100644 --- a/src/AzureAuth.Test/Commands/Ado/CommandTokenTest.cs +++ b/src/AzureAuth.Test/Commands/Ado/CommandTokenTest.cs @@ -3,19 +3,68 @@ namespace AzureAuth.Test.Commands.Ado { + using System; + using System.Collections.Generic; + using FluentAssertions; - using Microsoft.Authentication.AzureAuth.Ado; + using AzureAuth.Test; + using Microsoft.Authentication.AzureAuth; using Microsoft.Authentication.AzureAuth.Commands.Ado; + using Microsoft.Authentication.MSALWrapper; + using Microsoft.Extensions.DependencyInjection; + using Microsoft.Extensions.Logging; + using Microsoft.IdentityModel.JsonWebTokens; + using Microsoft.Office.Lasso.Interfaces; + using Microsoft.Office.Lasso.Telemetry; + using Moq; + using NLog.Extensions.Logging; + using NLog.Targets; using NUnit.Framework; internal class CommandTokenTest { + private Mock mockEnv; + private Mock mockTelemetry; + private Mock mockPublicClientAuth; + private IServiceProvider serviceProvider; + private MemoryTarget logTarget; + private CommandExecuteEventData eventData; + + [SetUp] + public void SetUp() + { + this.mockEnv = new Mock(); + this.mockEnv.Setup(e => e.Get(It.IsAny())).Returns((string)null); + this.mockTelemetry = new Mock(); + this.mockPublicClientAuth = new Mock(); + this.eventData = new CommandExecuteEventData(); + + // Setup in memory logging target with NLog - allows making assertions against what has been logged. + var loggingConfig = new NLog.Config.LoggingConfiguration(); + this.logTarget = new MemoryTarget("memory_target") + { + Layout = "${message}" // Define a simple layout so we don't get timestamps in messages. + }; + loggingConfig.AddTarget(this.logTarget); + loggingConfig.AddRuleForAllLevels(this.logTarget); + + // Setup Dependency Injection container to provide logger. + this.serviceProvider = new ServiceCollection() + .AddLogging(loggingBuilder => + { + loggingBuilder.ClearProviders(); + loggingBuilder.SetMinimumLevel(LogLevel.Trace); + loggingBuilder.AddNLog(loggingConfig); + }) + .BuildServiceProvider(); + } + [TestCase("foobar", CommandToken.OutputMode.Token, "foobar")] [TestCase("foobar", CommandToken.OutputMode.HeaderValue, "Basic OmZvb2Jhcg==")] [TestCase("foobar", CommandToken.OutputMode.Header, "Authorization: Basic OmZvb2Jhcg==")] public void FormatToken_Basic(string input, CommandToken.OutputMode mode, string expected) { - CommandToken.FormatToken(input, mode, Authorization.Basic).Should().Be(expected); + CommandToken.FormatToken(input, mode, Microsoft.Authentication.AzureAuth.Ado.Authorization.Basic).Should().Be(expected); } [TestCase("foobar", CommandToken.OutputMode.Token, "foobar")] @@ -23,7 +72,108 @@ public void FormatToken_Basic(string input, CommandToken.OutputMode mode, string [TestCase("foobar", CommandToken.OutputMode.Header, "Authorization: Bearer foobar")] public void FormatToken_Bearer(string input, CommandToken.OutputMode mode, string expected) { - CommandToken.FormatToken(input, mode, Authorization.Bearer).Should().Be(expected); + CommandToken.FormatToken(input, mode, Microsoft.Authentication.AzureAuth.Ado.Authorization.Bearer).Should().Be(expected); + } + + [Test] + public void OnExecute_AzureAuthAdoPat_AlwaysUsed() + { + this.mockEnv.Setup(e => e.Get(EnvVars.AdoPat)).Returns("my-explicit-pat"); + + var command = new CommandToken(); + var result = command.OnExecute( + this.serviceProvider.GetService>(), + this.mockEnv.Object, + this.mockTelemetry.Object, + this.mockPublicClientAuth.Object, + this.eventData); + + result.Should().Be(0); + this.mockPublicClientAuth.Verify( + p => p.Token(It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + } + + [Test] + public void OnExecute_AdoPipeline_UsesSystemAccessToken() + { + this.mockEnv.Setup(e => e.Get(EnvVars.TfBuild)).Returns("True"); + this.mockEnv.Setup(e => e.Get(EnvVars.SystemAccessToken)).Returns("pipeline-token"); + + var command = new CommandToken(); + var result = command.OnExecute( + this.serviceProvider.GetService>(), + this.mockEnv.Object, + this.mockTelemetry.Object, + this.mockPublicClientAuth.Object, + this.eventData); + + result.Should().Be(0); + this.mockPublicClientAuth.Verify( + p => p.Token(It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + } + + [Test] + public void OnExecute_AdoPipeline_NoSystemAccessToken_ReturnsError() + { + this.mockEnv.Setup(e => e.Get(EnvVars.TfBuild)).Returns("True"); + + var command = new CommandToken(); + var result = command.OnExecute( + this.serviceProvider.GetService>(), + this.mockEnv.Object, + this.mockTelemetry.Object, + this.mockPublicClientAuth.Object, + this.eventData); + + result.Should().Be(1); + this.logTarget.Logs.Should().Contain(l => l.Contains($"{EnvVars.SystemAccessToken} is not set")); + } + + [Test] + public void OnExecute_NotAdoPipeline_SystemAccessTokenSet_WarnsAndContinues() + { + this.mockEnv.Setup(e => e.Get(EnvVars.SystemAccessToken)).Returns("stale-token"); + var fakeTokenResult = new TokenResult(new JsonWebToken(Fake.Token), Guid.NewGuid()); + this.mockPublicClientAuth + .Setup(p => p.Token(It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(fakeTokenResult); + + var command = new CommandToken(); + var result = command.OnExecute( + this.serviceProvider.GetService>(), + this.mockEnv.Object, + this.mockTelemetry.Object, + this.mockPublicClientAuth.Object, + this.eventData); + + result.Should().Be(0); + this.logTarget.Logs.Should().Contain(l => l.Contains("does not appear to be an Azure DevOps Pipeline environment")); + + // Verify it fell through to AAD auth (ignored the SYSTEM_ACCESSTOKEN) + this.mockPublicClientAuth.Verify( + p => p.Token(It.IsAny(), It.IsAny>(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), + Times.Once); + } + + [Test] + public void OnExecute_AdoPipeline_AzureAuthAdoPat_TakesPriority() + { + this.mockEnv.Setup(e => e.Get(EnvVars.AdoPat)).Returns("my-explicit-pat"); + this.mockEnv.Setup(e => e.Get(EnvVars.TfBuild)).Returns("True"); + this.mockEnv.Setup(e => e.Get(EnvVars.SystemAccessToken)).Returns("pipeline-token"); + + var command = new CommandToken(); + var result = command.OnExecute( + this.serviceProvider.GetService>(), + this.mockEnv.Object, + this.mockTelemetry.Object, + this.mockPublicClientAuth.Object, + this.eventData); + + result.Should().Be(0); + this.logTarget.Logs.Should().Contain(l => l.Contains(EnvVars.AdoPat)); } } } diff --git a/src/AzureAuth.Test/IEnvExtensionsTest.cs b/src/AzureAuth.Test/IEnvExtensionsTest.cs index 3acb16ab..86ce993b 100644 --- a/src/AzureAuth.Test/IEnvExtensionsTest.cs +++ b/src/AzureAuth.Test/IEnvExtensionsTest.cs @@ -10,7 +10,6 @@ namespace AzureAuth.Test using Microsoft.Authentication.TestHelper; using Microsoft.Extensions.Logging; using Microsoft.Office.Lasso.Interfaces; - using Microsoft.Office.Lasso.Telemetry; using Moq; using NLog.Targets; using NUnit.Framework; @@ -58,6 +57,20 @@ public void InteractiveAuth_IsEnabledIfEnvVarsAreNotSet() IEnvExtensions.InteractiveAuthDisabled(this.envMock.Object).Should().BeFalse(); } + [TestCase("True", true)] + [TestCase("true", true)] + [TestCase("TRUE", true)] + [TestCase("False", false)] + [TestCase("", false)] + [TestCase(null, false)] + public void IsAdoPipeline_DetectsTfBuildEnvVar(string tfBuild, bool expected) + { + this.envMock.Setup(env => env.Get(It.IsAny())).Returns((string)null); + this.envMock.Setup(e => e.Get(EnvVars.TfBuild)).Returns(tfBuild); + + IEnvExtensions.IsAdoPipeline(this.envMock.Object).Should().Be(expected); + } + [Test] public void ReadAuthModeFromEnvOrSetDefault_ReturnsDefault_WhenEnvVarIsEmpty() { diff --git a/src/AzureAuth/Ado/PatFromEnv.cs b/src/AzureAuth/Ado/PatFromEnv.cs index f02c2d48..4be4b78e 100644 --- a/src/AzureAuth/Ado/PatFromEnv.cs +++ b/src/AzureAuth/Ado/PatFromEnv.cs @@ -3,18 +3,12 @@ namespace Microsoft.Authentication.AzureAuth.Ado { - using System; using System.Collections.Generic; - using System.Linq; - using System.Threading.Tasks; - using Microsoft.Authentication.MSALWrapper; - using Microsoft.Authentication.MSALWrapper.AuthFlow; - using Microsoft.Extensions.Logging; using Microsoft.Office.Lasso.Interfaces; /// - /// A class for getting an ADO PAT from an or an AAD access token through MSAL. + /// A class for getting an ADO PAT from an . /// public static class PatFromEnv { diff --git a/src/AzureAuth/Commands/Ado/CommandToken.cs b/src/AzureAuth/Commands/Ado/CommandToken.cs index 463bad73..2244cb00 100644 --- a/src/AzureAuth/Commands/Ado/CommandToken.cs +++ b/src/AzureAuth/Commands/Ado/CommandToken.cs @@ -89,13 +89,32 @@ public enum OutputMode /// An integer status code. 0 for success and non-zero for failure. public int OnExecute(ILogger logger, IEnv env, ITelemetryService telemetryService, IPublicClientAuth publicClientAuth, CommandExecuteEventData eventData) { - // First attempt using a PAT. + // First attempt using a PAT from the environment. var pat = PatFromEnv.Get(env); if (pat.Exists) { - logger.LogDebug($"Using PAT from env var {pat.EnvVarSource}"); - logger.LogInformation(FormatToken(pat.Value, this.Output, Authorization.Basic)); - return 0; + // SYSTEM_ACCESSTOKEN should only be used inside an ADO Pipeline. + if (pat.EnvVarSource == EnvVars.SystemAccessToken && !env.IsAdoPipeline()) + { + logger.LogWarning( + $"{EnvVars.SystemAccessToken} is set but this does not appear to be an Azure DevOps Pipeline environment. " + + "Having this variable set on a developer machine is unusual. It will be ignored."); + } + else + { + logger.LogDebug($"Using PAT from env var {pat.EnvVarSource}"); + logger.LogInformation(FormatToken(pat.Value, this.Output, Authorization.Basic)); + return 0; + } + } + else if (env.IsAdoPipeline()) + { + // In a pipeline but no token was found at all. + logger.LogError( + $"Running in an Azure DevOps Pipeline environment but {EnvVars.SystemAccessToken} is not set. " + + "Interactive authentication is not possible in a pipeline. " + + "Ensure the pipeline has access to the system token."); + return 1; } // If command line options for mode are not specified, then use the environment variables. diff --git a/src/AzureAuth/Commands/Ado/Pat/CommandScopes.cs b/src/AzureAuth/Commands/Ado/Pat/CommandScopes.cs index bbf0171a..aa9bdb69 100644 --- a/src/AzureAuth/Commands/Ado/Pat/CommandScopes.cs +++ b/src/AzureAuth/Commands/Ado/Pat/CommandScopes.cs @@ -10,8 +10,7 @@ namespace Microsoft.Authentication.AzureAuth.Commands.Ado.Pat { /// - /// Command to print the list of available scopes - + /// Command to print the list of available scopes for ADO PATs. /// [Command("scopes", Description = "List the valid ado pat scopes")] public class CommandScopes diff --git a/src/AzureAuth/EnvVars.cs b/src/AzureAuth/EnvVars.cs index 4b439e0b..3842c84f 100644 --- a/src/AzureAuth/EnvVars.cs +++ b/src/AzureAuth/EnvVars.cs @@ -36,6 +36,12 @@ public static class EnvVars /// public static readonly string NoUser = $"{EnvVarPrefix}_NO_USER"; + /// + /// Name of the env var set by Azure DevOps Pipelines to indicate a pipeline environment. + /// Value is "True" when running in an ADO Pipeline. + /// + public const string TfBuild = "TF_BUILD"; + /// /// Name of the env var for the Azure DevOps pipelines default personal access token. /// diff --git a/src/AzureAuth/IEnvExtensions.cs b/src/AzureAuth/IEnvExtensions.cs index f67f4143..ad214a6c 100644 --- a/src/AzureAuth/IEnvExtensions.cs +++ b/src/AzureAuth/IEnvExtensions.cs @@ -5,7 +5,6 @@ namespace Microsoft.Authentication.AzureAuth { using Microsoft.Authentication.MSALWrapper; using Microsoft.Office.Lasso.Interfaces; - using Microsoft.Office.Lasso.Telemetry; using System.Collections.Generic; using System; @@ -16,6 +15,16 @@ public static class IEnvExtensions { private const string CorextPositiveValue = "1"; + /// + /// Determines whether we are running in an Azure DevOps Pipeline environment. + /// + /// The to use to get environment variables. + /// True if running in an Azure DevOps Pipeline. + public static bool IsAdoPipeline(this IEnv env) + { + return string.Equals("True", env.Get(EnvVars.TfBuild), StringComparison.OrdinalIgnoreCase); + } + /// /// Determines whether interactive auth is disabled or not. /// @@ -31,7 +40,6 @@ public static bool InteractiveAuthDisabled(this IEnv env) /// Get the auth modes from the environment or set the default. /// /// The to use. - /// Event data to add the auth mode to. /// AuthModes. public static IEnumerable ReadAuthModeFromEnvOrSetDefault(this IEnv env) {