Add logging to the automatic RequireOrganizationTenantPolicy path (#6864)

Author: Scott Bommarito

src/NuGetGallery/Services/LoginDiscontinuationConfiguration.cs

@@ -43,7 +43,7 @@ public LoginDiscontinuationConfiguration(
             DiscontinuedForDomains = new HashSet<string>(discontinuedForDomains, StringComparer.OrdinalIgnoreCase);
             ExceptionsForEmailAddresses = new HashSet<string>(exceptionsForEmailAddresses, StringComparer.OrdinalIgnoreCase);
             ForceTransformationToOrganizationForEmailAddresses = new HashSet<string>(forceTransformationToOrganizationForEmailAddresses, StringComparer.OrdinalIgnoreCase);
-            EnabledOrganizationAadTenants = new HashSet<OrganizationTenantPair>(enabledOrganizationAadTenants, new OrganizationTenantPairComparer());
+            EnabledOrganizationAadTenants = new HashSet<OrganizationTenantPair>(enabledOrganizationAadTenants);
             IsPasswordDiscontinuedForAll = isPasswordDiscontinuedForAll;
         }
 
@@ -87,6 +87,16 @@ public bool ShouldUserTransformIntoOrganization(User user)
 
         public bool IsTenantIdPolicySupportedForOrganization(string emailAddress, string tenantId)
         {
+            if (string.IsNullOrEmpty(emailAddress))
+            {
+                throw new ArgumentException(nameof(emailAddress));
+            }
+
+            if (string.IsNullOrEmpty(tenantId))
+            {
+                throw new ArgumentException(nameof(tenantId));
+            }
+
             return EnabledOrganizationAadTenants.Contains(new OrganizationTenantPair(new MailAddress(emailAddress).Host, tenantId));
         }
 
@@ -105,7 +115,7 @@ public interface ILoginDiscontinuationConfiguration
         bool IsTenantIdPolicySupportedForOrganization(string emailAddress, string tenantId);
     }
 
-    public class OrganizationTenantPair
+    public class OrganizationTenantPair : IEquatable<OrganizationTenantPair>
     {
         public string EmailDomain { get; }
         public string TenantId { get; }
@@ -116,25 +126,28 @@ public OrganizationTenantPair(string emailDomain, string tenantId)
             EmailDomain = emailDomain ?? throw new ArgumentNullException(nameof(emailDomain));
             TenantId = tenantId ?? throw new ArgumentNullException(nameof(tenantId));
         }
-    }
 
-    public class OrganizationTenantPairComparer : IEqualityComparer<OrganizationTenantPair>
-    {
-        public bool Equals(OrganizationTenantPair x, OrganizationTenantPair y)
+        public override bool Equals(object obj)
         {
-            if (x == null || y == null)
-            {
-                return x == null && y == null;
-            }
+            return Equals(obj as OrganizationTenantPair);
+        }
 
-            return
-                string.Equals(x.EmailDomain, y.EmailDomain, StringComparison.OrdinalIgnoreCase) &&
-                string.Equals(x.TenantId, y.TenantId, StringComparison.OrdinalIgnoreCase);
+        public bool Equals(OrganizationTenantPair other)
+        {
+            return other != null &&
+                string.Equals(EmailDomain, other.EmailDomain, StringComparison.OrdinalIgnoreCase) &&
+                string.Equals(TenantId, other.TenantId, StringComparison.OrdinalIgnoreCase);
         }
 
-        public int GetHashCode(OrganizationTenantPair obj)
+        /// <remarks>
+        /// Autogenerated by "Quick Actions and Refactoring" -> "Generate Equals and GetHashCode".
+        /// </remarks>
+        public override int GetHashCode()
         {
-            return obj.EmailDomain.GetHashCode() ^ obj.TenantId.GetHashCode();
+            var hashCode = -1334890813;
+            hashCode = hashCode * -1521134295 + EqualityComparer<string>.Default.GetHashCode(EmailDomain.ToLowerInvariant());
+            hashCode = hashCode * -1521134295 + EqualityComparer<string>.Default.GetHashCode(TenantId.ToLowerInvariant());
+            return hashCode;
         }
     }
 }

src/NuGetGallery/Services/UserService.cs

@@ -8,6 +8,7 @@
 using System.Linq;
 using System.Text.RegularExpressions;
 using System.Threading.Tasks;
+using Microsoft.Extensions.Logging;
 using NuGet.Services.Entities;
 using NuGetGallery.Auditing;
 using NuGetGallery.Configuration;
@@ -39,6 +40,8 @@ public class UserService : IUserService
 
         public ITelemetryService TelemetryService { get; protected set; }
 
+        public ILogger<UserService> Logger { get; protected set; }
+
         protected UserService() { }
 
         public UserService(
@@ -52,7 +55,8 @@ public UserService(
             ISecurityPolicyService securityPolicyService,
             IDateTimeProvider dateTimeProvider,
             ICredentialBuilder credentialBuilder,
-            ITelemetryService telemetryService)
+            ITelemetryService telemetryService,
+            ILogger<UserService> logger)
             : this()
         {
             Config = config;
@@ -65,6 +69,7 @@ public UserService(
             SecurityPolicyService = securityPolicyService;
             DateTimeProvider = dateTimeProvider;
             TelemetryService = telemetryService;
+            Logger = logger;
         }
 
         public async Task<MembershipRequest> AddMembershipRequestAsync(Organization organization, string memberName, bool isAdmin)
@@ -591,12 +596,21 @@ public async Task<Organization> AddOrganizationAsync(string username, string ema
         private async Task SubscribeOrganizationToTenantPolicyIfTenantIdIsSupported(User organization, User adminUser, bool commitChanges = true)
         {
             var tenantId = adminUser.Credentials.GetAzureActiveDirectoryCredential()?.TenantId;
-            if (string.IsNullOrWhiteSpace(tenantId) ||
-                !ContentObjectService.LoginDiscontinuationConfiguration.IsTenantIdPolicySupportedForOrganization(organization.EmailAddress ?? organization.UnconfirmedEmailAddress, tenantId))
+            if (string.IsNullOrEmpty(tenantId))
+            {
+                Logger.LogInformation("Will not apply tenant policy to organization because admin user does not have an AAD credential.");
+                return;
+            }
+
+            if (!ContentObjectService.LoginDiscontinuationConfiguration.IsTenantIdPolicySupportedForOrganization(
+                organization.EmailAddress ?? organization.UnconfirmedEmailAddress, 
+                tenantId))
             {
+                Logger.LogInformation("Will not apply tenant policy to organization because policy is not supported for email-tenant pair.");
                 return;
             }
 
+            Logger.LogInformation("Applying tenant policy to organization.");
             var tenantPolicy = RequireOrganizationTenantPolicy.Create(tenantId);
             await SecurityPolicyService.SubscribeAsync(organization, tenantPolicy, commitChanges);
         }

tests/NuGetGallery.Facts/Services/ContentObjectServiceFacts.cs

@@ -113,10 +113,10 @@ public async Task RefreshRefreshesObject()
                 typosquattingConfiguration = service.TyposquattingConfiguration as TyposquattingConfiguration;
 
                 // Assert
-                Assert.True(loginDiscontinuationConfiguration.DiscontinuedForEmailAddresses.SequenceEqual(emails));
-                Assert.True(loginDiscontinuationConfiguration.DiscontinuedForDomains.SequenceEqual(domains));
-                Assert.True(loginDiscontinuationConfiguration.ExceptionsForEmailAddresses.SequenceEqual(exceptions));
-                Assert.True(loginDiscontinuationConfiguration.EnabledOrganizationAadTenants.SequenceEqual(orgTenantPairs, new OrganizationTenantPairComparer()));
+                Assert.Equal(emails, loginDiscontinuationConfiguration.DiscontinuedForEmailAddresses);
+                Assert.Equal(domains, loginDiscontinuationConfiguration.DiscontinuedForDomains);
+                Assert.Equal(exceptions, loginDiscontinuationConfiguration.ExceptionsForEmailAddresses);
+                Assert.Equal(orgTenantPairs, loginDiscontinuationConfiguration.EnabledOrganizationAadTenants);
 
                 Assert.True(certificatesConfiguration.IsUIEnabledByDefault);
                 Assert.Equal(alwaysEnabledForDomains, certificatesConfiguration.AlwaysEnabledForDomains);