[MSA] Pass the error messages from AADv2 to gallery (#6581)

Author: shishirx34

src/NuGetGallery/Authentication/Providers/AzureActiveDirectoryV2/AzureActiveDirectoryV2Authenticator.cs

@@ -45,10 +45,15 @@ public static class AuthenticationType
         public static readonly string Authority = "https://login.microsoftonline.com/{0}/v2.0";
 
         private static string _callbackPath = "users/account/authenticate/return";
-        private static HashSet<string> _errorMessageList = new HashSet<string> { "access_denied", "consent_required" };
         private static HashSet<string> _alternateSiteRootList;
         private const string SELECT_ACCOUNT = "select_account";
 
+        private static class ErrorQueryKeys
+        {
+            public const string Error = "error";
+            public const string ErrorDescription = "errorDescription";
+        }
+
         /// <summary>
         /// The possible values returned by <see cref="V2Claims.ACR"/> claim, and also the possible token values to be sent
         /// for authentication to the common endpoint.
@@ -200,16 +205,23 @@ public override bool SupportsMultiFactorAuthentication()
         // error handling is done.
         private Task AuthenticationFailed(AuthenticationFailedNotification<OpenIdConnectMessage, OpenIdConnectAuthenticationOptions> notification)
         {
-            if (_errorMessageList.Contains(notification.Exception.Message))
+            // For every 'Challenge' sent to the external providers, we pass the 'State'
+            // with the redirect uri where we intend to return after successful authentication.
+            // Extract this "RedirectUri" property from this "State" object for redirecting on failed authentication as well.
+            var authenticationProperties = GetAuthenticationPropertiesFromProtocolMessage(notification.ProtocolMessage, notification.Options);
+
+            // All authentication related exceptions will be handled.
+            notification.HandleResponse();
+
+            // Pass the errors as the query string to show appropriate message to the user.
+            var queryString = new List<KeyValuePair<string, string>>()
             {
-                // For every 'Challenge' sent to the external providers, we store the 'State'
-                // with the redirect uri where we intend to return after successful authentication.
-                // Extract this "RedirectUri" property from this "State" object for redirecting on failed authentication as well.
-                var authenticationProperties = GetAuthenticationPropertiesFromProtocolMessage(notification.ProtocolMessage, notification.Options);
+                new KeyValuePair<string, string>(ErrorQueryKeys.Error, notification.ProtocolMessage.Error),
+                new KeyValuePair<string, string>(ErrorQueryKeys.ErrorDescription, notification.ProtocolMessage.ErrorDescription)
+            };
 
-                notification.HandleResponse();
-                notification.Response.Redirect(authenticationProperties.RedirectUri);
-            }
+            var redirectUri = UriExtensions.AppendQueryStringToRelativeUri(authenticationProperties.RedirectUri, queryString);
+            notification.Response.Redirect(redirectUri);
 
             return Task.FromResult(0);
         }

src/NuGetGallery/Controllers/AuthenticationController.cs

@@ -22,6 +22,12 @@
 
 namespace NuGetGallery
 {
+    public static class AuthenticationFailureErrors
+    {
+        public const string ACCESSS_DENIED = "access_denied";
+        public const string CONSENT_REQUIRED = "consent_required";
+    }
+
     public partial class AuthenticationController
         : AppController
     {
@@ -172,7 +178,7 @@ public virtual async Task<ActionResult> SignIn(LogOnViewModel model, string retu
                 authenticatedUser = loginUserDetails?.AuthenticatedUser;
                 if (authenticatedUser == null)
                 {
-                    return ExternalLinkExpired();
+                    return AuthenticationFailureOrExternalLinkExpired();
                 }
 
                 usedMultiFactorAuthentication = loginUserDetails.UsedMultiFactorAuthentication;
@@ -264,7 +270,7 @@ public virtual async Task<ActionResult> Register(LogOnViewModel model, string re
                     var result = await _authService.ReadExternalLoginCredential(OwinContext);
                     if (result.ExternalIdentity == null)
                     {
-                        return ExternalLinkExpired();
+                        return AuthenticationFailureOrExternalLinkExpired();
                     }
 
                     usedMultiFactorAuthentication = result.LoginDetails?.WasMultiFactorAuthenticated ?? false;
@@ -497,15 +503,16 @@ public virtual async Task<ActionResult> LinkOrChangeExternalCredential(string re
             return SafeRedirect(returnUrl);
         }
 
-        public virtual async Task<ActionResult> LinkExternalAccount(string returnUrl)
+        public virtual async Task<ActionResult> LinkExternalAccount(string returnUrl, string error = null, string errorDescription = null)
         {
             // Extract the external login info
             var result = await _authService.AuthenticateExternalLogin(OwinContext);
             if (result.ExternalIdentity == null)
             {
                 // User got here without an external login cookie (or an expired one)
                 // Send them to the logon action
-                return ExternalLinkExpired();
+                string errorMessage = GetAuthenticationFailureMessage(error, errorDescription);
+                return AuthenticationFailureOrExternalLinkExpired(errorMessage);
             }
 
             if (result.Authentication != null)
@@ -749,11 +756,11 @@ private string GetEmailAddressFromExternalLoginResult(AuthenticateExternalLoginR
             }
         }
 
-        private ActionResult ExternalLinkExpired()
+        private ActionResult AuthenticationFailureOrExternalLinkExpired(string errorMessage = null)
         {
             // User got here without an external login cookie (or an expired one)
             // Send them to the logon action with a message
-            TempData["Message"] = Strings.ExternalAccountLinkExpired;
+            TempData["Message"] = string.IsNullOrEmpty(errorMessage) ? Strings.ExternalAccountLinkExpired : errorMessage;
             return Redirect(Url.LogOn(null, relativeUrl: false));
         }
 
@@ -815,5 +822,24 @@ private ActionResult AuthenticationView(string viewName, LogOnViewModel existing
 
             return View(viewName, existingModel);
         }
+
+        private string GetAuthenticationFailureMessage(string error, string errorDescription)
+        {
+            if (string.IsNullOrEmpty(error))
+            {
+                return Strings.AuthenticationFailure_UnkownError;
+            }
+
+            switch (error)
+            {
+                case AuthenticationFailureErrors.ACCESSS_DENIED:
+                case AuthenticationFailureErrors.CONSENT_REQUIRED:
+                    return Strings.ExternalAccountLinkExpired;
+                default:
+                    return string.IsNullOrEmpty(errorDescription)
+                        ? error
+                        : errorDescription;
+            }
+        }
     }
 }

src/NuGetGallery/Helpers/UriExtensions.cs

@@ -2,6 +2,8 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
+using System.Web;
 
 namespace NuGetGallery
 {
@@ -57,5 +59,19 @@ public static Uri ToHttps(this Uri uri)
 
             return uriBuilder.Uri;
         }
+
+        public static string AppendQueryStringToRelativeUri(string relativeUrl, IReadOnlyCollection<KeyValuePair<string, string>> queryStringCollection)
+        {
+            var tempUri = new Uri("http://www.nuget.org/");
+            var builder = new UriBuilder(new Uri(tempUri, relativeUrl));
+            var query = HttpUtility.ParseQueryString(builder.Query);
+            foreach (var pair in queryStringCollection)
+            {
+                query[pair.Key] = pair.Value;
+            }
+
+            builder.Query = query.ToString();
+            return builder.Uri.PathAndQuery;
+        }
     }
 }

src/NuGetGallery/Strings.Designer.cs

@@ -1002,4 +1002,7 @@ The {1} Team</value>
   <data name="UploadPackage_NotAcceptingPackagesWithLicense" xml:space="preserve">
     <value>This package contains a &lt;license&gt; metadata which is currently not supported.</value>
   </data>
+  <data name="AuthenticationFailure_UnkownError" xml:space="preserve">
+    <value>Unknown error!</value>
+  </data>
 </root>

src/NuGetGallery/Strings.resx

@@ -1457,8 +1457,10 @@ public void WillCallChallengeAuthenticationForAADv2Provider()
 
         public class TheLinkExternalAccountAction : TestContainer
         {
-            [Fact]
-            public async Task GivenExpiredExternalAuth_ItRedirectsBackToLogOnWithExternalAuthExpiredMessage()
+            [Theory]
+            [InlineData("access_denied")]
+            [InlineData("consent_required")]
+            public async Task GivenExpiredExternalAuth_ItRedirectsBackToLogOnWithExternalAuthExpiredMessage(string error)
             {
                 // Arrange
                 GetMock<AuthenticationService>(); // Force a mock to be created
@@ -1468,12 +1470,33 @@ public async Task GivenExpiredExternalAuth_ItRedirectsBackToLogOnWithExternalAut
                     .CompletesWith(new AuthenticateExternalLoginResult());
 
                 // Act
-                var result = await controller.LinkExternalAccount("theReturnUrl");
+                var result = await controller.LinkExternalAccount("theReturnUrl", error);
 
                 // Assert
                 VerifyExternalLinkExpiredResult(controller, result);
             }
 
+            [Theory]
+            [InlineData("server_error", "The server encountered an unexpected error.")]
+            [InlineData("temporarily_unavailable", "The server is temporarily too busy to handle the request.")]
+            [InlineData("invalid_resource", "The target resource is invalid because either it does not exist, Azure AD cannot find it, or it is not correctly configured.")]
+            [InlineData("invalid_request", "Protocol error, such as a missing, required parameter.")]
+            public async Task GivenExpiredExternalAuth_ItRedirectsBackToLogOnWithPassedErrorMessage(string error, string errorDescription)
+            {
+                // Arrange
+                GetMock<AuthenticationService>(); // Force a mock to be created
+                var controller = GetController<AuthenticationController>();
+                GetMock<AuthenticationService>()
+                    .Setup(x => x.AuthenticateExternalLogin(controller.OwinContext))
+                    .CompletesWith(new AuthenticateExternalLoginResult());
+
+                // Act
+                var result = await controller.LinkExternalAccount("theReturnUrl", error, errorDescription);
+
+                // Assert
+                VerifyExternalLinkExpiredResult(controller, result, errorDescription);
+            }
+
             [Fact]
             public async Task GivenAssociatedLocalUser_ItCreatesASessionAndSafeRedirectsToReturnUrl()
             {
@@ -2213,10 +2236,11 @@ public void VerifyShouldChallenge(string providerUsedForLogin, bool shouldChalle
             }
         }
 
-        public static void VerifyExternalLinkExpiredResult(AuthenticationController controller, ActionResult result)
+        public static void VerifyExternalLinkExpiredResult(AuthenticationController controller, ActionResult result, string expectedMessage = null)
         {
+            expectedMessage = expectedMessage ?? Strings.ExternalAccountLinkExpired;
             ResultAssert.IsRedirect(result, permanent: false, url: controller.Url.LogOn(relativeUrl: false));
-            Assert.Equal(Strings.ExternalAccountLinkExpired, controller.TempData["Message"]);
+            Assert.Equal(expectedMessage, controller.TempData["Message"]);
         }
 
         private static void EnableAllAuthenticators(AuthenticationService authService)

tests/NuGetGallery.Facts/Controllers/AuthenticationControllerFacts.cs

@@ -261,6 +261,7 @@
     <Compile Include="TestUtils\TestServiceUtility.cs" />
     <Compile Include="TestUtils\TestDataUtility.cs" />
     <Compile Include="TestUtils\TestUtility.cs" />
+    <Compile Include="UriExtensionsFacts.cs" />
     <Compile Include="UrlHelperExtensionsFacts.cs" />
     <Compile Include="ViewModels\DependencySetsViewModelFacts.cs" />
     <Compile Include="ViewModels\DisplayPackageViewModelFacts.cs" />

tests/NuGetGallery.Facts/NuGetGallery.Facts.csproj

@@ -0,0 +1,38 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Collections.Generic;
+using System.Linq;
+using Xunit;
+
+namespace NuGetGallery
+{
+    public class UriExtensionsFacts
+    {
+        public class TheAppendQueryStringToRelativeUriMethod
+        {
+            [Theory]
+            [InlineData("/abac/something?q1=123", "q2=ads&q3=4324", "/abac/something?q1=123&q2=ads&q3=4324")]
+            [InlineData("/abac/something?q1=123", "q2=ads", "/abac/something?q1=123&q2=ads")]
+            [InlineData("/abac/something", "q2=ads", "/abac/something?q2=ads")]
+            [InlineData("/abac/something/", "", "/abac/something/")]
+            public void ReturnsExpectedUrl(string url, string queryParameters, string expectedUrl)
+            {
+                // Arrange
+                var queryString = string.IsNullOrEmpty(queryParameters)
+                    ? new List<KeyValuePair<string, string>>()
+                    : queryParameters
+                        .Split('&')
+                        .Select(qv => qv.Split('='))
+                        .Select(qv => new KeyValuePair<string, string>(qv[0], qv[1]))
+                        .ToList();
+
+                // Act
+                var returnUrl = UriExtensions.AppendQueryStringToRelativeUri(url, queryString);
+
+                // Assert
+                Assert.Equal(expectedUrl, returnUrl);
+            }
+        }
+    }
+}