PublicClientApplicationBuilder - allow non-GUID client id for Authori…

…tyType.Generic (#4748)

* PublicClientApplicationBuilder - allow non-GUID client id for AuthorityType.Generic

* Completely removed ClientIdMustBeAGuid validation + related test and error messages.

* Moved generic authority interactive test from GenericAuthorityTests to InteractiveFlowTests.

src/client/Microsoft.Identity.Client/AppConfig/PublicClientApplicationBuilder.cs

@@ -377,12 +377,6 @@ internal override void Validate()
         {
             base.Validate();
 
-            //ADFS does not require client id to be in the form of a GUID.
-            if (Config.Authority.AuthorityInfo?.AuthorityType != AuthorityType.Adfs && !Guid.TryParse(Config.ClientId, out _))
-            {
-                throw new MsalClientException(MsalError.ClientIdMustBeAGuid, MsalErrorMessage.ClientIdMustBeAGuid);
-            }
-
 #if __MOBILE__ 
             if (Config.IsBrokerEnabled && Config.MultiCloudSupportEnabled)
             {

src/client/Microsoft.Identity.Client/MsalError.cs

@@ -763,12 +763,6 @@ public static class MsalError
         /// </summary>
         public const string NoClientId = "no_client_id";
 
-        /// <summary>
-        /// <para>What happens?</para>You've specified a client ID that is not a <see cref="Guid"/>
-        /// <para>Mitigation</para>Use the application ID (a GUID) from the application portal as client ID in this SDK
-        /// </summary>
-        public const string ClientIdMustBeAGuid = "client_id_must_be_guid";
-
         /// <summary>
         /// <para>What happens?</para>You have configured both a telemetry callback and a telemetry config. 
         /// <para>Mitigation</para>Only one telemetry mechanism can be configured.

src/client/Microsoft.Identity.Client/MsalErrorMessage.cs

@@ -245,7 +245,6 @@ public static string iOSBrokerKeySaveFailed(string keyChainResult)
 
         public const string ClientCredentialAuthenticationTypesAreMutuallyExclusive = "ClientSecret, Certificate and ClientAssertion are mutually exclusive properties. Only specify one. See https://aka.ms/msal-net-client-credentials. ";
         public const string ClientCredentialAuthenticationTypeMustBeDefined = "One client credential type required either: ClientSecret, Certificate, ClientAssertion or AppTokenProvider must be defined when creating a Confidential Client. Only specify one. See https://aka.ms/msal-net-client-credentials. ";
-        public const string ClientIdMustBeAGuid = "Error: ClientId is not a GUID. ";
 
         public static string InvalidRedirectUriReceived(string invalidRedirectUri)
         {

tests/Microsoft.Identity.Test.Integration.netcore/SeleniumTests/InteractiveFlowTests.NetFwk.cs

@@ -189,6 +189,36 @@ public async Task ValidateCcsHeadersForInteractiveAuthCodeFlowAsync()
             Assert.IsNotNull(authResult.AccessToken);
         }
 
+        /// Based on the publicly available https://demo.duendesoftware.com/
+        [RunOn(TargetFrameworks.NetCore)]
+        public async Task Interactive_GenericAuthority_DuendeDemoInstanceAsync()
+        {
+            string[] scopes = new[] { "openid profile email api offline_access" };
+            const string username = "bob", password = "bob";
+            const string demoDuendeSoftwareDotCom = "https://demo.duendesoftware.com";
+
+            var pca = PublicClientApplicationBuilder
+                .Create("interactive.public")
+                .WithRedirectUri(SeleniumWebUI.FindFreeLocalhostRedirectUri())
+                .WithTestLogging()
+                .WithExperimentalFeatures()
+                .WithOidcAuthority(demoDuendeSoftwareDotCom)
+                .Build();
+
+            AuthenticationResult authResult = await pca
+                .AcquireTokenInteractive(scopes)
+                .WithCustomWebUi(CreateSeleniumCustomWebUIForDuende(username, password))
+                .ExecuteAsync(new CancellationTokenSource(_interactiveAuthTimeout).Token)
+                .ConfigureAwait(false);
+
+            Assert.IsNotNull(authResult);
+            Assert.IsNotNull(authResult.Scopes);
+            Assert.IsNotNull(authResult.AccessToken);
+            Assert.IsNotNull(authResult.IdToken);
+            Assert.AreEqual(5, authResult.Scopes.Count());
+            Assert.AreEqual("Bearer", authResult.TokenType);
+        }
+
         private async Task<AuthenticationResult> RunTestForUserAsync(LabResponse labResponse, bool directToAdfs = false)
         {
             HttpSnifferClientFactory factory = null;
@@ -353,6 +383,20 @@ private SeleniumWebUI CreateSeleniumCustomWebUI(LabUser user, Prompt prompt, boo
             }, TestContext);
         }
 
+        private SeleniumWebUI CreateSeleniumCustomWebUIForDuende(string username, string password)
+        {
+            return new SeleniumWebUI((driver) =>
+            {
+                Trace.WriteLine("Starting Selenium automation");
+
+                driver.FindElementById("Input_Username").SendKeys(username);
+                driver.FindElementById("Input_Password").SendKeys(password);
+
+                var loginBtn = driver.WaitForElementToBeVisibleAndEnabled(OpenQA.Selenium.By.Name("Input.Button"));
+                loginBtn?.Click();
+            }, TestContext);
+        }
+
         #region Azure AD Kerberos Feature Tests
         [IgnoreOnOneBranch]
         public async Task Kerberos_Interactive_AADAsync()

tests/Microsoft.Identity.Test.Unit/AppConfigTests/PublicClientApplicationBuilderTests.cs

@@ -50,30 +50,6 @@ public void TestWithDifferentClientId()
             Assert.AreEqual(ClientId, pca.AppConfig.ClientId);
         }
 
-        [TestMethod]
-        public void ClientIdMustBeAGuid()
-        {
-            var ex = AssertException.Throws<MsalClientException>(
-                () => PublicClientApplicationBuilder.Create("http://my.app")
-                        .WithAuthority(TestConstants.AadAuthorityWithTestTenantId)
-                        .Build());
-
-            Assert.AreEqual(MsalError.ClientIdMustBeAGuid, ex.ErrorCode);
-
-            ex = AssertException.Throws<MsalClientException>(
-              () => PublicClientApplicationBuilder.Create("http://my.app")
-                      .WithAuthority(TestConstants.B2CAuthority)
-                      .Build());
-
-            Assert.AreEqual(MsalError.ClientIdMustBeAGuid, ex.ErrorCode);
-
-            // ADFS does not have this constraint
-            PublicClientApplicationBuilder.Create("http://my.app")
-                        .WithAuthority(new Uri(TestConstants.ADFSAuthority))
-                        .Build();
-
-        }
-
         [TestMethod]
         public void TestConstructor_ClientIdOverride()
         {