Update External Account Linking Flow

src/NuGetGallery.Core/Auditing/CredentialAuditRecord.cs

@@ -31,7 +31,7 @@ public CredentialAuditRecord(Credential credential, bool removed)
             Identity = credential.Identity;
 
             // Track the value for credentials that are definitely revocable (API Key, etc.) and have been removed
-            if (removed && !CredentialTypes.IsPassword(credential.Type))
+            if (removed && !credential.IsPassword())
             {
                 Value = credential.Value;
             }

src/NuGetGallery.Core/CredentialTypes.cs

@@ -28,26 +28,31 @@ public static class ApiKey
 
         public const string ExternalPrefix = "external.";
 
-        public static bool IsPassword(string type)
+        public static bool IsPassword(this Credential c)
         {
-            if (type == null)
-            {
-                throw new ArgumentNullException(nameof(type));
-            }
+            return IsPassword(c.Type);
+        }
 
-            return type.StartsWith(Password.Prefix, StringComparison.OrdinalIgnoreCase);
+        public static bool IsExternal(this Credential c)
+        {
+            return c.IsSubType(ExternalPrefix);
         }
 
-        public static bool IsApiKey(string type)
+        public static bool IsApiKey(this Credential c)
         {
-            return type.StartsWith(ApiKey.Prefix, StringComparison.OrdinalIgnoreCase);
+            return c.IsSubType(ApiKey.Prefix);
         }
 
-        public static bool IsPackageVerificationApiKey(string type)
+        public static bool IsType(this Credential c, string type)
         {
-            return type.Equals(ApiKey.VerifyV1, StringComparison.OrdinalIgnoreCase);
+            return IsType(c.Type, type);
         }
-
+
+        private static bool IsSubType(this Credential c, string typePrefix)
+        {
+            return IsSubType(c.Type, typePrefix);
+        }
+
         internal static IReadOnlyList<string> SupportedCredentialTypes = new List<string> { Password.Sha1, Password.Pbkdf2, Password.V3, ApiKey.V1, ApiKey.V2, ApiKey.V4 };
 
         /// <summary>
@@ -69,13 +74,49 @@ public static bool IsSupportedCredential(this Credential credential)
         /// <returns></returns>
         public static bool IsViewSupportedCredential(this Credential credential)
         {
-            return SupportedCredentialTypes.Any(credType => string.Compare(credential.Type, credType, StringComparison.OrdinalIgnoreCase) == 0)
-                    || credential.Type.StartsWith(ExternalPrefix, StringComparison.OrdinalIgnoreCase);
+            return
+                SupportedCredentialTypes.Any(credType => credential.IsType(credType)) ||
+                credential.IsExternal();
         }
 
         public static bool IsScopedApiKey(this Credential credential)
         {
             return IsApiKey(credential.Type) && credential.Scopes != null && credential.Scopes.Any();
         }
+
+        public static bool IsPassword(string type)
+        {
+            return IsSubType(type, Password.Prefix);
+        }
+
+        public static bool IsApiKey(string type)
+        {
+            return IsSubType(type, ApiKey.Prefix);
+        }
+
+        public static bool IsPackageVerificationApiKey(string type)
+        {
+            return IsSubType(type, ApiKey.VerifyV1);
+        }
+
+        private static bool IsType(string actualType, string expectedType)
+        {
+            if (actualType == null)
+            {
+                throw new ArgumentNullException(nameof(actualType));
+            }
+
+            return actualType.Equals(expectedType, StringComparison.OrdinalIgnoreCase);
+        }
+
+        private static bool IsSubType(string actualType, string expectedTypePrefix)
+        {
+            if (actualType == null)
+            {
+                throw new ArgumentNullException(nameof(actualType));
+            }
+
+            return actualType.StartsWith(expectedTypePrefix, StringComparison.OrdinalIgnoreCase);
+        }
     }
 }

src/NuGetGallery/Authentication/AuthenticationService.cs

@@ -128,7 +128,7 @@ await Auditing.SaveAuditRecordAsync(
 
                 var passwordCredentials = user
                     .Credentials
-                    .Where(c => CredentialTypes.IsPassword(c.Type))
+                    .Where(c => c.IsPassword())
                     .ToList();
 
                 if (passwordCredentials.Count > 1 ||
@@ -160,7 +160,7 @@ public virtual async Task<AuthenticatedUser> Authenticate(Credential credential)
 
         private async Task<AuthenticatedUser> AuthenticateInternal(Func<Credential, Credential> matchCredential, Credential credential)
         {
-            if (credential.Type.StartsWith(CredentialTypes.Password.Prefix, StringComparison.OrdinalIgnoreCase))
+            if (credential.IsPassword())
             {
                 // Password credentials cannot be used this way.
                 throw new ArgumentException(Strings.PasswordCredentialsCannotBeUsedHere, nameof(credential));
@@ -201,7 +201,7 @@ await Auditing.SaveAuditRecordAsync(
                     return null;
                 }
 
-                if (CredentialTypes.IsApiKey(matched.Type) &&
+                if (matched.IsApiKey() &&
                     !matched.IsScopedApiKey() &&
                     !matched.HasBeenUsedInLastDays(_config.ExpirationInDaysForApiKeyV1))
                 {

src/NuGetGallery/Authentication/Providers/AzureActiveDirectory/AzureActiveDirectoryAuthenticator.cs

@@ -83,7 +83,16 @@ public override ActionResult Challenge(string redirectUrl)
 
         public override IdentityInformation GetIdentityInformation(ClaimsIdentity claimsIdentity)
         {
-            return ClaimsExtentions.GetIdentityInformation(claimsIdentity, DefaultAuthenticationType);
+            var identityInfo = ClaimsExtentions.GetIdentityInformation(claimsIdentity, DefaultAuthenticationType);
+
+            // The claims returned by AzureActiveDirectory have the email as the name claim are missing the email claim.
+            // Copy the object returned by the method but set the email as the name.
+            return new IdentityInformation(
+                identityInfo.Identifier, 
+                identityInfo.Name, 
+                identityInfo.Name, 
+                identityInfo.AuthenticationType, 
+                identityInfo.TenantId);
         }
     }
 }

src/NuGetGallery/Controllers/AuthenticationController.cs

@@ -144,18 +144,22 @@ public virtual async Task<ActionResult> SignIn(LogOnViewModel model, string retu
                     modelErrorMessage = string.Format(CultureInfo.CurrentCulture, Strings.UserAccountLocked, timeRemaining);
                 }
 
-                ModelState.AddModelError("SignIn", modelErrorMessage);
-
-                return SignInOrExternalLinkView(model, linkingAccount);
+                return SignInFailure(model, linkingAccount, modelErrorMessage);
             }
 
-            var user = authenticationResult.AuthenticatedUser;
+            var authenticatedUser = authenticationResult.AuthenticatedUser;
 
             if (linkingAccount)
             {
+                // Verify account has no other external accounts
+                if (authenticatedUser.User.Credentials.Any(c => c.IsExternal()) && !authenticatedUser.User.IsAdministrator())
+                {
+                    return SignInFailure(model, linkingAccount, Strings.LinkingMultipleExternalAccountsUnsupported);
+                }
+
                 // Link with an external account
-                user = await AssociateCredential(user);
-                if (user == null)
+                authenticatedUser = await AssociateCredential(authenticatedUser);
+                if (authenticatedUser == null)
                 {
                     return ExternalLinkExpired();
                 }
@@ -165,16 +169,23 @@ public virtual async Task<ActionResult> SignIn(LogOnViewModel model, string retu
             // to require a specific authentication provider, challenge that provider if needed.
             ActionResult challenge;
             if (ShouldChallengeEnforcedProvider(
-                NuGetContext.Config.Current.EnforcedAuthProviderForAdmin, user, returnUrl, out challenge))
+                NuGetContext.Config.Current.EnforcedAuthProviderForAdmin, authenticatedUser, returnUrl, out challenge))
             {
                 return challenge;
             }
 
             // Create session
-            await _authService.CreateSessionAsync(OwinContext, user);
+            await _authService.CreateSessionAsync(OwinContext, authenticatedUser);
             return SafeRedirect(returnUrl);
         }
 
+        private ActionResult SignInFailure(LogOnViewModel model, bool linkingAccount, string modelErrorMessage)
+        {
+            ModelState.AddModelError("SignIn", modelErrorMessage);
+
+            return SignInOrExternalLinkView(model, linkingAccount);
+        }
+
         internal bool ShouldChallengeEnforcedProvider(string enforcedProviders, AuthenticatedUser authenticatedUser, string returnUrl, out ActionResult challenge)
         {
             if (!string.IsNullOrEmpty(enforcedProviders)
@@ -362,9 +373,7 @@ public virtual async Task<ActionResult> LinkExternalAccount(string returnUrl)
                 {
                     // Consume the exception for now, for backwards compatibility to previous MSA provider.
                     email = result.ExternalIdentity.GetClaimOrDefault(ClaimTypes.Email);
-                    name = result
-                        .ExternalIdentity 
-                        .GetClaimOrDefault(ClaimTypes.Name);
+                    name = result.ExternalIdentity.GetClaimOrDefault(ClaimTypes.Name);
                 }
 
                 // Check for a user with this email address
@@ -374,11 +383,33 @@ public virtual async Task<ActionResult> LinkExternalAccount(string returnUrl)
                     existingUser = _userService.FindByEmailAddress(email);
                 }
 
+                var foundExistingUser = existingUser != null;
+                string existingUserLinkingError = null;
+
+                if (foundExistingUser)
+                {
+                    if (existingUser is Organization)
+                    {
+                        existingUserLinkingError = string.Format(
+                               CultureInfo.CurrentCulture,
+                               Strings.LinkingOrganizationUnsupported,
+                               email);
+                    }
+                    else if (existingUser.Credentials.Any(c => c.IsExternal()) && !existingUser.IsAdministrator())
+                    {
+                        existingUserLinkingError = string.Format(
+                               CultureInfo.CurrentCulture,
+                               Strings.AccountIsLinkedToAnotherExternalAccount,
+                               email);
+                    }
+                }
+
                 var external = new AssociateExternalAccountViewModel()
                 {
                     ProviderAccountNoun = authUI.AccountNoun,
                     AccountName = name,
-                    FoundExistingUser = existingUser != null
+                    FoundExistingUser = foundExistingUser,
+                    ExistingUserLinkingError = existingUserLinkingError
                 };
 
                 var model = new LogOnViewModel

src/NuGetGallery/Controllers/UsersController.cs

@@ -1000,7 +1000,7 @@ private async Task<ActionResult> RemoveCredentialInternal(User user, Credential
             }
 
             // Count credentials and make sure the user can always login
-            if (!CredentialTypes.IsApiKey(cred.Type) && CountLoginCredentials(user) <= 1)
+            if (!cred.IsApiKey() && CountLoginCredentials(user) <= 1)
             {
                 TempData["Message"] = Strings.CannotRemoveOnlyLoginCredential;
             }

src/NuGetGallery/Extensions/UserExtensions.cs

@@ -1,7 +1,6 @@
 // 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;
 using System.Linq;
 using System.Security.Claims;

src/NuGetGallery/Security/SecurePushSubscription.cs

@@ -66,7 +66,7 @@ public SecurePushSubscription(IAuditingService auditing, IDiagnosticsService dia
         public async Task OnSubscribeAsync(UserSecurityPolicySubscriptionContext context)
         {
             var pushKeys = context.User.Credentials.Where(c =>
-                CredentialTypes.IsApiKey(c.Type) &&
+                c.IsApiKey() &&
                 (
                     c.Scopes.Count == 0 ||
                     c.Scopes.Any(s =>

src/NuGetGallery/Strings.resx

@@ -736,4 +736,13 @@ For more information, please contact '{2}'.</value>
   <data name="TransformAccount_RequestExists" xml:space="preserve">
     <value>Another tranform request was created on {0} with organization admin '{1}'.</value>
   </data>
+  <data name="AccountIsLinkedToAnotherExternalAccount" xml:space="preserve">
+    <value>We found an account with the email address associated with this external account ({0}), but linked to a different external account.</value>
+  </data>
+  <data name="LinkingMultipleExternalAccountsUnsupported" xml:space="preserve">
+    <value>You cannot link your NuGet.org account to multiple external accounts.</value>
+  </data>
+  <data name="LinkingOrganizationUnsupported" xml:space="preserve">
+    <value>We found an organization with the email address associated with this external account ({0}), but organizations cannot be linked to external accounts.</value>
+  </data>
 </root>

src/NuGetGallery/ViewModels/LogOnViewModel.cs

@@ -33,6 +33,8 @@ public class AssociateExternalAccountViewModel
         public string ProviderAccountNoun { get; set; }
         public string AccountName { get; set; }
         public bool FoundExistingUser { get; set; }
+        public bool ExistingUserCanBeLinked => string.IsNullOrEmpty(ExistingUserLinkingError);
+        public string ExistingUserLinkingError { get; set; }
     }
 
     public class SignInViewModel