Responding to Comments.

Author: harshgMSFT

src/Microsoft.AspNet.Mvc.Core/AntiForgery/AntiForgery.cs

@@ -1,9 +1,8 @@
-
-using System.ComponentModel;
+using System.ComponentModel;
+using System.Threading.Tasks;
 using Microsoft.AspNet.Abstractions;
 using Microsoft.AspNet.Mvc.Rendering;
 using Microsoft.AspNet.Security.DataProtection;
-using System.Threading.Tasks;
 
 namespace Microsoft.AspNet.Mvc
 {
@@ -16,9 +15,9 @@ public sealed class AntiForgery
         private static readonly string _purpose = "Microsoft.AspNet.Mvc.AntiXsrf.AntiForgeryToken.v1";
         private readonly AntiForgeryWorker _worker;
 
-        public AntiForgery(IClaimUidExtractor claimUidExtractor,
-                           IDataProtectionProvider dataProtectionProvider,
-                           IAntiForgeryAdditionalDataProvider additionalDataProvider)
+        public AntiForgery([NotNull] IClaimUidExtractor claimUidExtractor,
+                           [NotNull] IDataProtectionProvider dataProtectionProvider,
+                           [NotNull] IAntiForgeryAdditionalDataProvider additionalDataProvider)
         {
             // TODO: This is temporary till we figure out how to flow configs using DI.
             var config = new AntiForgeryConfigWrapper();
@@ -32,16 +31,16 @@ public AntiForgery(IClaimUidExtractor claimUidExtractor,
         /// Generates an anti-forgery token for this request. This token can
         /// be validated by calling the Validate() method.
         /// </summary>
-        /// <param name="context">The http context associated with the current call.</param>
+        /// <param name="context">The HTTP context associated with the current call.</param>
         /// <returns>An HTML string corresponding to an &lt;input type="hidden"&gt;
         /// element. This element should be put inside a &lt;form&gt;.</returns>
         /// <remarks>
-        /// This method has a side effect: it may set a response cookie.
+        /// This method has a side effect: A response cookie is set if there is no valid cookie associated with the request.
         /// </remarks>
-        public HtmlString GetHtml(HttpContext context)
+        public HtmlString GetHtml([NotNull] HttpContext context)
         {
-            TagBuilder retVal = _worker.GetFormInputElement(context);
-            return retVal.ToHtmlString(TagRenderMode.SelfClosing);
+            TagBuilder builder = _worker.GetFormInputElement(context);
+            return builder.ToHtmlString(TagRenderMode.SelfClosing);
         }
 
         /// <summary>
@@ -50,49 +49,45 @@ public HtmlString GetHtml(HttpContext context)
         /// over how to persist the returned values. To validate these tokens, call the
         /// appropriate overload of Validate.
         /// </summary>
-        /// <param name="context">The http context associated with the current call.</param>
+        /// <param name="context">The HTTP context associated with the current call.</param>
         /// <param name="oldCookieToken">The anti-forgery token - if any - that already existed
         /// for this request. May be null. The anti-forgery system will try to reuse this cookie
         /// value when generating a matching form token.</param>
         /// </remarks>
-        public AntiForgeryTokenSet GetTokens(HttpContext context, string oldCookieToken)
+        public AntiForgeryTokenSet GetTokens([NotNull] HttpContext context, string oldCookieToken)
         {
             // Will contain a new cookie value if the old cookie token
             // was null or invalid. If this value is non-null when the method completes, the caller
             // must persist this value in the form of a response cookie, and the existing cookie value
             // should be discarded. If this value is null when the method completes, the existing
             // cookie value was valid and needn't be modified.
-            string newCookieToken;
-            string formToken;
-            _worker.GetTokens(context, oldCookieToken, out newCookieToken, out formToken);
-            return new AntiForgeryTokenSet(formToken, newCookieToken);
+            return _worker.GetTokens(context, oldCookieToken);
         }
 
         /// <summary>
         /// Validates an anti-forgery token that was supplied for this request.
         /// The anti-forgery token may be generated by calling GetHtml().
         /// </summary>
-        /// <param name="context">The http context associated with the current call.</param>
-        public async Task ValidateAsync(HttpContext context)
+        /// <param name="context">The HTTP context associated with the current call.</param>
+        public async Task ValidateAsync([NotNull] HttpContext context)
         {
            await _worker.ValidateAsync(context);
         }
 
         /// <summary>
         /// Validates an anti-forgery token pair that was generated by the GetTokens method.
         /// </summary>
-        /// <param name="context">The http context associated with the current call.</param>
+        /// <param name="context">The HTTP context associated with the current call.</param>
         /// <param name="cookieToken">The token that was supplied in the request cookie.</param>
         /// <param name="formToken">The token that was supplied in the request form body.</param>
-        [EditorBrowsable(EditorBrowsableState.Advanced)]
-        public void Validate(HttpContext context, string cookieToken, string formToken)
+        public void Validate([NotNull] HttpContext context, string cookieToken, string formToken)
         {
             _worker.Validate(context, cookieToken, formToken);
         }
 
-        public void Validate(HttpContext context, AntiForgeryTokenSet antiforgeryTokenSet)
+        public void Validate([NotNull] HttpContext context, AntiForgeryTokenSet antiForgeryTokenSet)
         {
-            Validate(context, antiforgeryTokenSet.CookieToken, antiforgeryTokenSet.FormToken);
+            Validate(context, antiForgeryTokenSet.CookieToken, antiForgeryTokenSet.FormToken);
         }
     }
 }

src/Microsoft.AspNet.Mvc.Core/AntiForgery/AntiForgeryConfig.cs

@@ -6,7 +6,6 @@
     public static class AntiForgeryConfig
     {
         internal const string AntiForgeryTokenFieldName = "__RequestVerificationToken";
-        private const string AntiForgeryCookieTokenName = "__RequestVerificationCookieToken";
         private static string _cookieName;
 
         /// <summary>
@@ -59,7 +58,7 @@ public static bool SuppressXFrameOptionsHeader
         // TODO: Replace the stub. 
         private static string GetAntiForgeryCookieName()
         {
-            return AntiForgeryCookieTokenName;
+            return AntiForgeryTokenFieldName;
         }
     }
 }

src/Microsoft.AspNet.Mvc.Core/AntiForgery/AntiForgeryTokenSerializer.cs

@@ -12,7 +12,7 @@ internal sealed class AntiForgeryTokenSerializer : IAntiForgeryTokenSerializer
         private readonly IDataProtector _cryptoSystem;
         private const byte TokenVersion = 0x01;
 
-        internal AntiForgeryTokenSerializer(IDataProtector cryptoSystem)
+        internal AntiForgeryTokenSerializer([NotNull] IDataProtector cryptoSystem)
         {
             _cryptoSystem = cryptoSystem;
         }

src/Microsoft.AspNet.Mvc.Core/AntiForgery/AntiForgeryTokenStore.cs

@@ -11,7 +11,8 @@ internal sealed class AntiForgeryTokenStore : ITokenStore
         private readonly IAntiForgeryConfig _config;
         private readonly IAntiForgeryTokenSerializer _serializer;
 
-        internal AntiForgeryTokenStore(IAntiForgeryConfig config, IAntiForgeryTokenSerializer serializer)
+        internal AntiForgeryTokenStore([NotNull] IAntiForgeryConfig config, 
+                                       [NotNull] IAntiForgeryTokenSerializer serializer)
         {
             _config = config;
             _serializer = serializer;

src/Microsoft.AspNet.Mvc.Core/AntiForgery/AntiForgeryWorker.cs

@@ -2,12 +2,11 @@
 using System;
 using System.Diagnostics.CodeAnalysis;
 using System.Diagnostics.Contracts;
-using System.Security.Principal;
+using System.Security.Claims;
+using System.Threading.Tasks;
 using Microsoft.AspNet.Abstractions;
 using Microsoft.AspNet.Mvc.Core;
 using Microsoft.AspNet.Mvc.Rendering;
-using System.Security.Claims;
-using System.Threading.Tasks;
 
 namespace Microsoft.AspNet.Mvc
 {
@@ -19,7 +18,11 @@ internal sealed class AntiForgeryWorker
         private readonly ITokenValidator _validator;
         private readonly ITokenGenerator _generator;
 
-        internal AntiForgeryWorker(IAntiForgeryTokenSerializer serializer, IAntiForgeryConfig config, ITokenStore tokenStore, ITokenGenerator generator, ITokenValidator validator)
+        internal AntiForgeryWorker([NotNull] IAntiForgeryTokenSerializer serializer,
+                                   [NotNull] IAntiForgeryConfig config,
+                                   [NotNull] ITokenStore tokenStore,
+                                   [NotNull] ITokenGenerator generator,
+                                   [NotNull] ITokenValidator validator)
         {
             _serializer = serializer;
             _config = config;
@@ -62,12 +65,12 @@ private static ClaimsIdentity ExtractIdentity(HttpContext httpContext)
             if (httpContext != null)
             {
                 ClaimsPrincipal user = httpContext.User;
-             
+
                 if (user != null)
                 {
                     // We only support ClaimsIdentity.
                     // Todo remove this once httpContext.User moves to ClaimsIdentity.
-                   return user.Identity as ClaimsIdentity;
+                    return user.Identity as ClaimsIdentity;
                 }
             }
 
@@ -93,14 +96,14 @@ private AntiForgeryToken GetCookieTokenNoThrow(HttpContext httpContext)
         // value is the hidden input form element that should be rendered in
         // the <form>. This method has a side effect: it may set a response
         // cookie.
-        public TagBuilder GetFormInputElement(HttpContext httpContext)
+        public TagBuilder GetFormInputElement([NotNull] HttpContext httpContext)
         {
             CheckSSLConfig(httpContext);
 
-            AntiForgeryToken oldCookieToken = GetCookieTokenNoThrow(httpContext);
-            AntiForgeryToken newCookieToken, formToken;
-            GetTokens(httpContext, oldCookieToken, out newCookieToken, out formToken);
-
+            var oldCookieToken = GetCookieTokenNoThrow(httpContext);
+            var tokenSet = GetTokens(httpContext, oldCookieToken);
+            var newCookieToken = tokenSet.CookieToken;
+            var formToken = tokenSet.FormToken;
             if (newCookieToken != null)
             {
                 // If a new cookie was generated, persist it.
@@ -129,29 +132,39 @@ public TagBuilder GetFormInputElement(HttpContext httpContext)
         // 'new cookie value' out param is non-null, the caller *must* persist
         // the new value to cookie storage since the original value was null or
         // invalid. This method is side-effect free.
-        public void GetTokens(HttpContext httpContext, string serializedOldCookieToken, out string serializedNewCookieToken, out string serializedFormToken)
+        public AntiForgeryTokenSet GetTokens([NotNull] HttpContext httpContext, string serializedOldCookieToken)
         {
             CheckSSLConfig(httpContext);
-
             AntiForgeryToken oldCookieToken = DeserializeTokenNoThrow(serializedOldCookieToken);
             AntiForgeryToken newCookieToken, formToken;
-            GetTokens(httpContext, oldCookieToken, out newCookieToken, out formToken);
+            var tokenSet = GetTokens(httpContext, oldCookieToken);
 
-            serializedNewCookieToken = Serialize(newCookieToken);
-            serializedFormToken = Serialize(formToken);
+            var serializedNewCookieToken = Serialize(tokenSet.CookieToken);
+            var serializedFormToken = Serialize(tokenSet.FormToken);
+            return new AntiForgeryTokenSet(serializedFormToken, serializedNewCookieToken);
         }
 
-        private void GetTokens(HttpContext httpContext, AntiForgeryToken oldCookieToken, out AntiForgeryToken newCookieToken, out AntiForgeryToken formToken)
+        private AntiForgeryTokenSetInternal GetTokens(HttpContext httpContext, AntiForgeryToken oldCookieToken)
         {
-            newCookieToken = null;
+            AntiForgeryToken newCookieToken = null;
             if (!_validator.IsCookieTokenValid(oldCookieToken))
             {
                 // Need to make sure we're always operating with a good cookie token.
                 oldCookieToken = newCookieToken = _generator.GenerateCookieToken();
             }
 
             Contract.Assert(_validator.IsCookieTokenValid(oldCookieToken));
-            formToken = _generator.GenerateFormToken(httpContext, ExtractIdentity(httpContext), oldCookieToken);
+
+            AntiForgeryToken formToken = _generator.
+                                            GenerateFormToken(httpContext,
+                                                    ExtractIdentity(httpContext),
+                                                    oldCookieToken);
+
+            return new AntiForgeryTokenSetInternal()
+                            {
+                                CookieToken = newCookieToken,
+                                FormToken = formToken
+                            };
         }
 
         private string Serialize(AntiForgeryToken token)
@@ -162,7 +175,7 @@ private string Serialize(AntiForgeryToken token)
         // [ ENTRY POINT ]
         // Given an HttpContext, validates that the anti-XSRF tokens contained
         // in the cookies & form are OK for this request.
-        public async Task ValidateAsync(HttpContext httpContext)
+        public async Task ValidateAsync([NotNull] HttpContext httpContext)
         {
             CheckSSLConfig(httpContext);
 
@@ -177,7 +190,7 @@ public async Task ValidateAsync(HttpContext httpContext)
         // [ ENTRY POINT ]
         // Given the serialized string representations of a cookie & form token,
         // validates that the pair is OK for this request.
-        public void Validate(HttpContext httpContext, string cookieToken, string formToken)
+        public void Validate([NotNull] HttpContext httpContext, string cookieToken, string formToken)
         {
             CheckSSLConfig(httpContext);
 
@@ -188,5 +201,12 @@ public void Validate(HttpContext httpContext, string cookieToken, string formTok
             // Validate
             _validator.ValidateTokens(httpContext, ExtractIdentity(httpContext), deserializedCookieToken, deserializedFormToken);
         }
+
+        private class AntiForgeryTokenSetInternal
+        {
+            public AntiForgeryToken FormToken { get; set; }
+
+            public AntiForgeryToken CookieToken { get; set; }
+        }
     }
 }

src/Microsoft.AspNet.Mvc.Core/AntiForgery/TokenProvider.cs

@@ -13,7 +13,9 @@ internal sealed class TokenProvider : ITokenValidator, ITokenGenerator
         private readonly IAntiForgeryConfig _config;
         private readonly IAntiForgeryAdditionalDataProvider _additionalDataProvider;
 
-        internal TokenProvider(IAntiForgeryConfig config, IClaimUidExtractor claimUidExtractor, IAntiForgeryAdditionalDataProvider additionalDataProvider)
+        internal TokenProvider(IAntiForgeryConfig config, 
+                               IClaimUidExtractor claimUidExtractor,
+                               IAntiForgeryAdditionalDataProvider additionalDataProvider)
         {
             _config = config;
             _claimUidExtractor = claimUidExtractor;
@@ -29,7 +31,9 @@ public AntiForgeryToken GenerateCookieToken()
             };
         }
 
-        public AntiForgeryToken GenerateFormToken(HttpContext httpContext, ClaimsIdentity identity, AntiForgeryToken cookieToken)
+        public AntiForgeryToken GenerateFormToken(HttpContext httpContext, 
+                                                  ClaimsIdentity identity,
+                                                  AntiForgeryToken cookieToken)
         {
             Contract.Assert(IsCookieTokenValid(cookieToken));
 
@@ -38,10 +42,13 @@ public AntiForgeryToken GenerateFormToken(HttpContext httpContext, ClaimsIdentit
                 SecurityToken = cookieToken.SecurityToken,
                 IsSessionToken = false
             };
-            
+
+            bool isIdentityAuthenticated = false;
+
             // populate Username and ClaimUid
             if (identity != null && identity.IsAuthenticated)
             {
+                isIdentityAuthenticated = true;
                 formToken.ClaimUid = GetClaimUidBlob(_claimUidExtractor.ExtractClaimUid(identity));
                 if (formToken.ClaimUid == null)
                 {
@@ -55,12 +62,15 @@ public AntiForgeryToken GenerateFormToken(HttpContext httpContext, ClaimsIdentit
                 formToken.AdditionalData = _additionalDataProvider.GetAdditionalData(httpContext);
             }
 
-            if (string.IsNullOrEmpty(formToken.Username)
+            if (isIdentityAuthenticated
+                && string.IsNullOrEmpty(formToken.Username)
                 && formToken.ClaimUid == null
                 && string.IsNullOrEmpty(formToken.AdditionalData))
             {
                 // Application says user is authenticated, but we have no identifier for the user.
-                throw new InvalidOperationException(Resources.FormatTokenValidator_AuthenticatedUserWithoutUsername(identity.GetType()));
+                throw new InvalidOperationException(
+                                        Resources.
+                                            FormatTokenValidator_AuthenticatedUserWithoutUsername(identity.GetType()));
             }
 
             return formToken;
@@ -138,6 +148,11 @@ public void ValidateTokens(HttpContext httpContext, ClaimsIdentity identity, Ant
 
         private static BinaryBlob GetClaimUidBlob(string base64ClaimUid)
         {
+            if (base64ClaimUid == null)
+            {
+                return null;
+            }
+
             return new BinaryBlob(256, Convert.FromBase64String(base64ClaimUid));
         }
     }