From 9f4aa98ee24533d32e30c41013e193c387ace6ac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micka=C3=ABl=20Derriey?= <mderriey@gmail.com>
Date: Thu, 16 May 2019 06:48:15 +1000
Subject: [PATCH] Handle `null`s in the `JsonDocumentAuthExtensions.GetString`
 extension method (#10252)

---
 .../Core/src/JsonDocumentAuthExtensions.cs    | 12 ++++++---
 .../src/MicrosoftAccountOptions.cs            | 10 +------
 .../test/JsonDocumentAuthExtensionsTests.cs   | 26 +++++++++++++++++++
 .../test/MicrosoftAccountTests.cs             |  3 +--
 4 files changed, 37 insertions(+), 14 deletions(-)
 create mode 100644 src/Security/Authentication/test/JsonDocumentAuthExtensionsTests.cs

diff --git a/src/Security/Authentication/Core/src/JsonDocumentAuthExtensions.cs b/src/Security/Authentication/Core/src/JsonDocumentAuthExtensions.cs
index 54b8f8e42f..71ddea922f 100644
--- a/src/Security/Authentication/Core/src/JsonDocumentAuthExtensions.cs
+++ b/src/Security/Authentication/Core/src/JsonDocumentAuthExtensions.cs
@@ -7,8 +7,14 @@ namespace Microsoft.AspNetCore.Authentication
 {
     public static class JsonDocumentAuthExtensions
     {
-        public static string GetString(this JsonElement element, string key) =>
-            element.TryGetProperty(key, out var property)
-                ? property.ToString() : null;
+        public static string GetString(this JsonElement element, string key)
+        {
+            if (element.TryGetProperty(key, out var property) && property.Type != JsonValueType.Null)
+            {
+                return property.ToString();
+            }
+
+            return null;
+        }
     }
 }
diff --git a/src/Security/Authentication/MicrosoftAccount/src/MicrosoftAccountOptions.cs b/src/Security/Authentication/MicrosoftAccount/src/MicrosoftAccountOptions.cs
index 36abae5be7..8462913a3d 100644
--- a/src/Security/Authentication/MicrosoftAccount/src/MicrosoftAccountOptions.cs
+++ b/src/Security/Authentication/MicrosoftAccount/src/MicrosoftAccountOptions.cs
@@ -27,15 +27,7 @@ namespace Microsoft.AspNetCore.Authentication.MicrosoftAccount
             ClaimActions.MapJsonKey(ClaimTypes.Name, "displayName");
             ClaimActions.MapJsonKey(ClaimTypes.GivenName, "givenName");
             ClaimActions.MapJsonKey(ClaimTypes.Surname, "surname");
-            ClaimActions.MapCustomJson(ClaimTypes.Email, user =>
-            {
-                var mail = user.GetString("mail");
-                if (string.IsNullOrEmpty(mail))
-                {
-                    mail = user.GetString("userPrincipalName");
-                }
-                return mail;
-            });
+            ClaimActions.MapCustomJson(ClaimTypes.Email, user => user.GetString("mail") ?? user.GetString("userPrincipalName"));
         }
     }
 }
diff --git a/src/Security/Authentication/test/JsonDocumentAuthExtensionsTests.cs b/src/Security/Authentication/test/JsonDocumentAuthExtensionsTests.cs
new file mode 100644
index 0000000000..3b73929db5
--- /dev/null
+++ b/src/Security/Authentication/test/JsonDocumentAuthExtensionsTests.cs
@@ -0,0 +1,26 @@
+// 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.Text.Json;
+using Xunit;
+
+namespace Microsoft.AspNetCore.Authentication.Test
+{
+    public class JsonDocumentAuthExtensionsTests
+    {
+        [Theory]
+        [InlineData("{ \"foo\": null }", null)]
+        [InlineData("{ \"foo\": \"\" }", "")]
+        [InlineData("{ \"foo\": \"bar\" }", "bar")]
+        [InlineData("{ \"foo\": 1 }", "1")]
+        [InlineData("{ \"bar\": \"baz\" }", null)]
+        public void GetStringReturnsCorrectValue(string json, string expected)
+        {
+            using (var document = JsonDocument.Parse(json))
+            {
+                var value = document.RootElement.GetString("foo");
+                Assert.Equal(expected, value);
+            }
+        }
+    }
+}
diff --git a/src/Security/Authentication/test/MicrosoftAccountTests.cs b/src/Security/Authentication/test/MicrosoftAccountTests.cs
index 5e6021ebab..c13f4bf51b 100644
--- a/src/Security/Authentication/test/MicrosoftAccountTests.cs
+++ b/src/Security/Authentication/test/MicrosoftAccountTests.cs
@@ -203,8 +203,7 @@ namespace Microsoft.AspNetCore.Authentication.Tests.MicrosoftAccount
                                 displayName = "Test Name",
                                 givenName = "Test Given Name",
                                 surname = "Test Family Name",
-                                mail = "",
-                                userPrincipalName = "Test email"
+                                mail = "Test email"
                             });
                         }