Enable nullable on Authentication.Negotiate (#29252)

Author: JamesNK

src/Security/Authentication/Negotiate/src/Events/AuthenticationFailedContext.cs

@@ -24,6 +24,6 @@ public AuthenticationFailedContext(
         /// <summary>
         /// The exception that occurred while processing the authentication.
         /// </summary>
-        public Exception Exception { get; set; }
+        public Exception Exception { get; set; } = default!;
     }
 }

src/Security/Authentication/Negotiate/src/Internal/INegotiateState.cs

@@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Authentication.Negotiate
     // For testing
     internal interface INegotiateState : IDisposable
     {
-        string GetOutgoingBlob(string incomingBlob, out BlobErrorType status, out Exception error);
+        string? GetOutgoingBlob(string incomingBlob, out BlobErrorType status, out Exception? error);
 
         bool IsCompleted { get; }
 

src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.DirectoryServices.Protocols;
 using System.Linq;
 using System.Security.Claims;
@@ -16,7 +17,7 @@ internal static class LdapAdapter
     {
         public static async Task RetrieveClaimsAsync(LdapSettings settings, ClaimsIdentity identity, ILogger logger)
         {
-            var user = identity.Name;
+            var user = identity.Name!;
             var userAccountNameIndex = user.IndexOf('@');
             var userAccountName = userAccountNameIndex == -1 ? user : user.Substring(0, userAccountNameIndex);
 
@@ -40,6 +41,8 @@ public static async Task RetrieveClaimsAsync(LdapSettings settings, ClaimsIdenti
 
             var filter = $"(&(objectClass=user)(sAMAccountName={userAccountName}))"; // This is using ldap search query language, it is looking on the server for someUser
             var searchRequest = new SearchRequest(distinguishedName, filter, SearchScope.Subtree, null);
+
+            Debug.Assert(settings.LdapConnection != null);
             var searchResponse = (SearchResponse) await Task<DirectoryResponse>.Factory.FromAsync(
                 settings.LdapConnection.BeginSendRequest,
                 settings.LdapConnection.EndSendRequest,

src/Security/Authentication/Negotiate/src/Internal/NegotiateLoggingExtensions.cs

@@ -7,17 +7,17 @@ namespace Microsoft.Extensions.Logging
 {
     internal static class NegotiateLoggingExtensions
     {
-        private static Action<ILogger, Exception> _incompleteNegotiateChallenge;
-        private static Action<ILogger, Exception> _negotiateComplete;
-        private static Action<ILogger, Exception> _enablingCredentialPersistence;
-        private static Action<ILogger, string, Exception> _disablingCredentialPersistence;
+        private static Action<ILogger, Exception?> _incompleteNegotiateChallenge;
+        private static Action<ILogger, Exception?> _negotiateComplete;
+        private static Action<ILogger, Exception?> _enablingCredentialPersistence;
+        private static Action<ILogger, string, Exception?> _disablingCredentialPersistence;
         private static Action<ILogger, Exception> _exceptionProcessingAuth;
         private static Action<ILogger, Exception> _credentialError;
         private static Action<ILogger, Exception> _clientError;
-        private static Action<ILogger, Exception> _challengeNegotiate;
-        private static Action<ILogger, Exception> _reauthenticating;
-        private static Action<ILogger, Exception> _deferring;
-        private static Action<ILogger, string, Exception> _negotiateError;
+        private static Action<ILogger, Exception?> _challengeNegotiate;
+        private static Action<ILogger, Exception?> _reauthenticating;
+        private static Action<ILogger, Exception?> _deferring;
+        private static Action<ILogger, string, Exception?> _negotiateError;
 
         static NegotiateLoggingExtensions()
         {

src/Security/Authentication/Negotiate/src/Internal/ReflectedNegotiateState.cs

@@ -25,15 +25,15 @@ internal class ReflectedNegotiateState : INegotiateState
         private static readonly FieldInfo _statusCode;
         private static readonly FieldInfo _statusException;
         private static readonly MethodInfo _getException;
-        private static readonly FieldInfo _gssMinorStatus;
-        private static readonly Type _gssExceptionType;
+        private static readonly FieldInfo? _gssMinorStatus;
+        private static readonly Type? _gssExceptionType;
 
         private readonly object _instance;
 
         static ReflectedNegotiateState()
         {
             var secAssembly = typeof(AuthenticationException).Assembly;
-            var ntAuthType = secAssembly.GetType("System.Net.NTAuthentication", throwOnError: true);
+            var ntAuthType = secAssembly.GetType("System.Net.NTAuthentication", throwOnError: true)!;
             _constructor = ntAuthType.GetConstructors(BindingFlags.NonPublic | BindingFlags.Instance).First();
             _getOutgoingBlob = ntAuthType.GetMethods(BindingFlags.NonPublic | BindingFlags.Instance).Where(info =>
                 info.Name.Equals("GetOutgoingBlob") && info.GetParameters().Count() == 3).Single();
@@ -44,19 +44,19 @@ static ReflectedNegotiateState()
             _closeContext = ntAuthType.GetMethods(BindingFlags.NonPublic | BindingFlags.Instance).Where(info =>
                 info.Name.Equals("CloseContext")).Single();
 
-            var securityStatusType = secAssembly.GetType("System.Net.SecurityStatusPal", throwOnError: true);
-            _statusCode = securityStatusType.GetField("ErrorCode");
-            _statusException = securityStatusType.GetField("Exception");
+            var securityStatusType = secAssembly.GetType("System.Net.SecurityStatusPal", throwOnError: true)!;
+            _statusCode = securityStatusType.GetField("ErrorCode")!;
+            _statusException = securityStatusType.GetField("Exception")!;
 
             if (!OperatingSystem.IsWindows())
             {
-                var interopType = secAssembly.GetType("Interop", throwOnError: true);
-                var netNativeType = interopType.GetNestedType("NetSecurityNative", BindingFlags.NonPublic | BindingFlags.Static);
-                _gssExceptionType = netNativeType.GetNestedType("GssApiException", BindingFlags.NonPublic);
-                _gssMinorStatus = _gssExceptionType.GetField("_minorStatus", BindingFlags.Instance | BindingFlags.NonPublic);
+                var interopType = secAssembly.GetType("Interop", throwOnError: true)!;
+                var netNativeType = interopType.GetNestedType("NetSecurityNative", BindingFlags.NonPublic | BindingFlags.Static)!;
+                _gssExceptionType = netNativeType.GetNestedType("GssApiException", BindingFlags.NonPublic)!;
+                _gssMinorStatus = _gssExceptionType.GetField("_minorStatus", BindingFlags.Instance | BindingFlags.NonPublic)!;
             }
 
-            var negoStreamPalType = secAssembly.GetType("System.Net.Security.NegotiateStreamPal", throwOnError: true);
+            var negoStreamPalType = secAssembly.GetType("System.Net.Security.NegotiateStreamPal", throwOnError: true)!;
             _getIdentity = negoStreamPalType.GetMethods(BindingFlags.NonPublic | BindingFlags.Static).Where(info =>
                 info.Name.Equals("GetIdentity")).Single();
             _getException = negoStreamPalType.GetMethods(BindingFlags.NonPublic | BindingFlags.Static).Where(info =>
@@ -67,24 +67,24 @@ public ReflectedNegotiateState()
         {
             // internal NTAuthentication(bool isServer, string package, NetworkCredential credential, string spn, ContextFlagsPal requestedContextFlags, ChannelBinding channelBinding)
             var credential = CredentialCache.DefaultCredentials;
-            _instance = _constructor.Invoke(new object[] { true, "Negotiate", credential, null, 0, null });
+            _instance = _constructor.Invoke(new object?[] { true, "Negotiate", credential, null, 0, null });
         }
 
         // Copied rather than reflected to remove the IsCompleted -> CloseContext check.
         // The client doesn't need the context once auth is complete, but the server does.
         // I'm not sure why it auto-closes for the client given that the client closes it just a few lines later.
         // https://github.com/dotnet/corefx/blob/a3ab91e10045bb298f48c1d1f9bd5b0782a8ac46/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs#L134
-        public string GetOutgoingBlob(string incomingBlob, out BlobErrorType status, out Exception error)
+        public string? GetOutgoingBlob(string incomingBlob, out BlobErrorType status, out Exception? error)
         {
-            byte[] decodedIncomingBlob = null;
+            byte[]? decodedIncomingBlob = null;
             if (incomingBlob != null && incomingBlob.Length > 0)
             {
                 decodedIncomingBlob = Convert.FromBase64String(incomingBlob);
             }
 
             byte[] decodedOutgoingBlob = GetOutgoingBlob(decodedIncomingBlob, out status, out error);
 
-            string outgoingBlob = null;
+            string? outgoingBlob = null;
             if (decodedOutgoingBlob != null && decodedOutgoingBlob.Length > 0)
             {
                 outgoingBlob = Convert.ToBase64String(decodedOutgoingBlob);
@@ -93,28 +93,28 @@ public string GetOutgoingBlob(string incomingBlob, out BlobErrorType status, out
             return outgoingBlob;
         }
 
-        private byte[] GetOutgoingBlob(byte[] incomingBlob, out BlobErrorType status, out Exception error)
+        private byte[] GetOutgoingBlob(byte[]? incomingBlob, out BlobErrorType status, out Exception? error)
         {
             try
             {
                 // byte[] GetOutgoingBlob(byte[] incomingBlob, bool throwOnError, out SecurityStatusPal statusCode)
-                var parameters = new object[] { incomingBlob, false, null };
-                var blob = (byte[])_getOutgoingBlob.Invoke(_instance, parameters);
+                var parameters = new object?[] { incomingBlob, false, null };
+                var blob = (byte[])_getOutgoingBlob.Invoke(_instance, parameters)!;
 
                 var securityStatus = parameters[2];
                 // TODO: Update after corefx changes
-                error = (Exception)(_statusException.GetValue(securityStatus)
+                error = (Exception?)(_statusException.GetValue(securityStatus)
                     ?? _getException.Invoke(null, new[] { securityStatus }));
-                var errorCode = (SecurityStatusPalErrorCode)_statusCode.GetValue(securityStatus);
+                var errorCode = (SecurityStatusPalErrorCode)_statusCode.GetValue(securityStatus)!;
 
                 // TODO: Remove after corefx changes
                 // The linux implementation always uses InternalError;
                 if (errorCode == SecurityStatusPalErrorCode.InternalError
                     && !OperatingSystem.IsWindows()
-                    && _gssExceptionType.IsInstanceOfType(error))
+                    && _gssExceptionType!.IsInstanceOfType(error))
                 {
                     var majorStatus = (uint)error.HResult;
-                    var minorStatus = (uint)_gssMinorStatus.GetValue(error);
+                    var minorStatus = (uint)_gssMinorStatus!.GetValue(error)!;
 
                     // Remap specific errors
                     if (majorStatus == GSS_S_NO_CRED && minorStatus == 0)
@@ -149,24 +149,24 @@ private byte[] GetOutgoingBlob(byte[] incomingBlob, out BlobErrorType status, ou
             catch (TargetInvocationException tex)
             {
                 // Unwrap
-                ExceptionDispatchInfo.Capture(tex.InnerException).Throw();
+                ExceptionDispatchInfo.Capture(tex.InnerException!).Throw();
                 throw;
             }
         }
 
         public bool IsCompleted
         {
-            get => (bool)_isCompleted.Invoke(_instance, Array.Empty<object>());
+            get => (bool)_isCompleted.Invoke(_instance, Array.Empty<object>())!;
         }
 
         public string Protocol
         {
-            get => (string)_protocol.Invoke(_instance, Array.Empty<object>());
+            get => (string)_protocol.Invoke(_instance, Array.Empty<object>())!;
         }
 
         public IIdentity GetIdentity()
         {
-            return (IIdentity)_getIdentity.Invoke(obj: null, parameters: new object[] { _instance });
+            return (IIdentity)_getIdentity.Invoke(obj: null, parameters: new object[] { _instance })!;
         }
 
         public void Dispose()

src/Security/Authentication/Negotiate/src/LdapSettings.cs

@@ -24,20 +24,20 @@ public class LdapSettings
         /// <example>
         /// DOMAIN.com
         /// </example>
-        public string Domain { get; set; }
+        public string Domain { get; set; } = default!;
 
         /// <summary>
         /// The machine account name to use when opening the LDAP connection.
         /// If this is not provided, the machine wide credentials of the
         /// domain joined machine will be used.
         /// </summary>
-        public string MachineAccountName { get; set; }
+        public string? MachineAccountName { get; set; }
 
         /// <summary>
         /// The machine account password to use when opening the LDAP connection.
         /// This must be provided if a <see cref="MachineAccountName"/> is provided.
         /// </summary>
-        public string MachineAccountPassword { get; set; }
+        public string? MachineAccountPassword { get; set; }
 
         /// <summary>
         /// This option indicates whether nested groups should be ignored when
@@ -55,7 +55,7 @@ public class LdapSettings
         /// <see cref="MachineAccountPassword"/>  options will not be used to create
         /// the <see cref="LdapConnection"/>.
         /// </summary>
-        public LdapConnection LdapConnection { get; set; }
+        public LdapConnection? LdapConnection { get; set; }
 
         /// <summary>
         /// The sliding expiration that should be used for entries in the cache for user claims, defaults to 10 minutes.
@@ -74,7 +74,7 @@ public class LdapSettings
         /// </summary>
         public int ClaimsCacheSize { get; set; } = 100 * 1024 * 1024;
 
-        internal MemoryCache ClaimsCache { get; set; }
+        internal MemoryCache? ClaimsCache { get; set; }
 
         /// <summary>
         /// Validates the <see cref="LdapSettings"/>.

src/Security/Authentication/Negotiate/src/Microsoft.AspNetCore.Authentication.Negotiate.csproj

@@ -5,6 +5,7 @@
     <TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <PackageTags>aspnetcore;authentication;security</PackageTags>
+    <Nullable>enable</Nullable>
   </PropertyGroup>
 
   <ItemGroup>

src/Security/Authentication/Negotiate/src/NegotiateExtensions.cs

@@ -67,7 +67,7 @@ public static AuthenticationBuilder AddNegotiate(this AuthenticationBuilder buil
         /// <param name="displayName">The name displayed to users when selecting an authentication handler. The default is null to prevent this from displaying.</param>
         /// <param name="configureOptions">Allows for configuring the authentication handler.</param>
         /// <returns>The original builder.</returns>
-        public static AuthenticationBuilder AddNegotiate(this AuthenticationBuilder builder, string authenticationScheme, string displayName, Action<NegotiateOptions> configureOptions)
+        public static AuthenticationBuilder AddNegotiate(this AuthenticationBuilder builder, string authenticationScheme, string? displayName, Action<NegotiateOptions> configureOptions)
         {
             builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton<IPostConfigureOptions<NegotiateOptions>, PostConfigureNegotiateOptions>());
             builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton<IStartupFilter>(new NegotiateOptionsValidationStartupFilter(authenticationScheme)));