Update External Account Linking Flow

src/NuGetGallery.Core/Auditing/CredentialAuditRecord.cs

@@ -31,10 +31,19 @@ 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)
             {
-                Value = credential.Value;
+                if (Type == null)
+                {
+                    throw new ArgumentNullException(nameof(credential.Type));
+                }
+
+                if (!credential.IsPassword())
+                {
+                    Value = credential.Value;
+                }
             }
+
             Created = credential.Created;
             Expires = credential.Expires;
             LastUsed = credential.LastUsed;

src/NuGetGallery.Core/CredentialTypes.cs

@@ -29,36 +29,36 @@ 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 c?.Type?.StartsWith(Password.Prefix, StringComparison.OrdinalIgnoreCase) ?? false;
+        }
 
-            return type.StartsWith(Password.Prefix, StringComparison.OrdinalIgnoreCase);
+        public static bool IsExternal(this Credential c)
+        {
+            return c?.Type?.StartsWith(ExternalPrefix, StringComparison.OrdinalIgnoreCase) ?? false;
         }
 
-        public static bool IsApiKey(string type)
+        public static bool IsApiKey(this Credential c)
         {
-            return type.StartsWith(ApiKey.Prefix, StringComparison.OrdinalIgnoreCase);
+            return c?.Type?.StartsWith(ApiKey.Prefix, StringComparison.OrdinalIgnoreCase) ?? false;
         }
 
-        public static bool IsPackageVerificationApiKey(string type)
+        public static bool IsType(this Credential c, string type)
         {
-            return type.Equals(ApiKey.VerifyV1, StringComparison.OrdinalIgnoreCase);
+            return c?.Type?.Equals(type, StringComparison.OrdinalIgnoreCase) ?? false;
         }
-        
+
         internal static IReadOnlyList<string> SupportedCredentialTypes = new List<string>
-            {
-                Password.Sha1,
-                Password.Pbkdf2,
-                Password.V3,
-                ApiKey.V1,
-                ApiKey.V2,
-                ApiKey.V3,
-                ApiKey.V4
-            };
+        {
+            Password.Sha1,
+            Password.Pbkdf2,
+            Password.V3,
+            ApiKey.V1,
+            ApiKey.V2,
+            ApiKey.V3,
+            ApiKey.V4
+        };
 
         /// <summary>
         /// Determines whether a credential is supported (internal or from the UI). For forward compatibility,
@@ -79,13 +79,29 @@ 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 type?.StartsWith(Password.Prefix, StringComparison.OrdinalIgnoreCase) ?? false;
+        }
+
+        public static bool IsApiKey(string type)
+        {
+            return type?.StartsWith(ApiKey.Prefix, StringComparison.OrdinalIgnoreCase) ?? false;
+        }
+
+        public static bool IsPackageVerificationApiKey(string type)
+        {
+            return type?.Equals(ApiKey.VerifyV1, StringComparison.OrdinalIgnoreCase) ?? false;
+        }
     }
 }

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

@@ -17,6 +17,7 @@ namespace NuGetGallery.Authentication.Providers.AzureActiveDirectory
     public class AzureActiveDirectoryAuthenticator : Authenticator<AzureActiveDirectoryAuthenticatorConfiguration>
     {
         public static readonly string DefaultAuthenticationType = "AzureActiveDirectory";
+        public static readonly string ClaimTypeName = "name";
 
         protected override void AttachToOwinApp(IGalleryConfigurationService config, IAppBuilder app)
         {
@@ -83,7 +84,7 @@ public override ActionResult Challenge(string redirectUrl)
 
         public override IdentityInformation GetIdentityInformation(ClaimsIdentity claimsIdentity)
         {
-            return ClaimsExtentions.GetIdentityInformation(claimsIdentity, DefaultAuthenticationType);
+            return ClaimsExtentions.GetIdentityInformation(claimsIdentity, DefaultAuthenticationType, ClaimTypes.NameIdentifier, ClaimTypeName, ClaimTypes.Name);
         }
     }
 }

src/NuGetGallery/Authentication/Providers/Utils/ClaimsExtentions.cs

@@ -10,19 +10,24 @@ public static class ClaimsExtentions
     {
         public static IdentityInformation GetIdentityInformation(ClaimsIdentity claimsIdentity, string authType)
         {
-            var identifierClaim = claimsIdentity.FindFirst(ClaimTypes.NameIdentifier);
+            return GetIdentityInformation(claimsIdentity, authType, ClaimTypes.NameIdentifier, ClaimTypes.Name, ClaimTypes.Email);
+        }
+
+        public static IdentityInformation GetIdentityInformation(ClaimsIdentity claimsIdentity, string authType, string nameIdentifierClaimType, string nameClaimType, string emailClaimType)
+        {
+            var identifierClaim = claimsIdentity.FindFirst(nameIdentifierClaimType);
             if (identifierClaim == null)
             {
-                throw new ArgumentException($"External Authentication is missing required claim: {ClaimTypes.NameIdentifier}");
+                throw new ArgumentException($"External Authentication is missing required claim: {nameIdentifierClaimType}");
             }
 
-            var nameClaim = claimsIdentity.FindFirst(ClaimTypes.Name);
+            var nameClaim = claimsIdentity.FindFirst(nameClaimType);
             if (nameClaim == null)
             {
-                throw new ArgumentException($"External Authentication is missing required claim: {ClaimTypes.Name}");
+                throw new ArgumentException($"External Authentication is missing required claim: {nameClaimType}");
             }
 
-            var emailClaim = claimsIdentity.FindFirst(ClaimTypes.Email);
+            var emailClaim = claimsIdentity.FindFirst(emailClaimType);
             return new IdentityInformation(identifierClaim.Value, nameClaim.Value, emailClaim?.Value, authType, tenantId: null);
         }
     }

src/NuGetGallery/Controllers/AuthenticationController.cs

@@ -148,18 +148,26 @@ 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())
+                {
+                    var message = string.Format(
+                           CultureInfo.CurrentCulture,
+                           Strings.AccountIsLinkedToAnotherExternalAccount,
+                           authenticatedUser.User.EmailAddress);
+                    return SignInFailure(model, linkingAccount, message);
+                }
+
                 // Link with an external account
-                user = await AssociateCredential(user);
-                if (user == null)
+                authenticatedUser = await AssociateCredential(authenticatedUser);
+                if (authenticatedUser == null)
                 {
                     return ExternalLinkExpired();
                 }
@@ -169,16 +177,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)
@@ -366,9 +381,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
@@ -378,11 +391,27 @@ public virtual async Task<ActionResult> LinkExternalAccount(string returnUrl)
                     existingUser = _userService.FindByEmailAddress(email);
                 }
 
+                var foundExistingUser = existingUser != null;
+                var existingUserLinkingError = AssociateExternalAccountViewModel.ExistingUserLinkingErrorType.None;
+
+                if (foundExistingUser)
+                {
+                    if (existingUser is Organization)
+                    {
+                        existingUserLinkingError = AssociateExternalAccountViewModel.ExistingUserLinkingErrorType.AccountIsOrganization;
+                    }
+                    else if (existingUser.Credentials.Any(c => c.IsExternal()) && !existingUser.IsAdministrator())
+                    {
+                        existingUserLinkingError = AssociateExternalAccountViewModel.ExistingUserLinkingErrorType.AccountIsAlreadyLinked;
+                    }
+                }
+
                 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

@@ -987,7 +987,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

@@ -742,4 +742,8 @@ For more information, please contact '{2}'.</value>
   <data name="TransformAccount_AdminMustBeDifferentAccount" xml:space="preserve">
     <value>Administrator account '{0}' cannot be the same account as the one being transformed.</value>
   </data>
+  <data name="AccountIsLinkedToAnotherExternalAccount" xml:space="preserve">
+    <value>The account with the email {0} is linked to another Microsoft account.
+If you wish to update the linked Microsoft account you can do so from the account settings page.</value>
+  </data>
 </root>

src/NuGetGallery/ViewModels/LogOnViewModel.cs

@@ -33,6 +33,15 @@ public class AssociateExternalAccountViewModel
         public string ProviderAccountNoun { get; set; }
         public string AccountName { get; set; }
         public bool FoundExistingUser { get; set; }
+        public bool ExistingUserCanBeLinked => ExistingUserLinkingError == ExistingUserLinkingErrorType.None;
+        public ExistingUserLinkingErrorType ExistingUserLinkingError { get; set; }
+
+        public enum ExistingUserLinkingErrorType
+        {
+            None = (int)default(ExistingUserLinkingErrorType),
+            AccountIsOrganization,
+            AccountIsAlreadyLinked
+        }
     }
 
     public class SignInViewModel