Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] CCA CreateWithApplicationOptions forces use of ClientSecret, won't allow just Certificate usage #1332

Closed
dilansudharaka opened this issue Aug 9, 2019 · 3 comments
Assignees
Labels
regression Behavior that worked in a previous release that no longer works in a newer release
Milestone

Comments

@dilansudharaka
Copy link

Which Version of MSAL are you using ? 4.3.0

Platform
net46

What authentication flow has the issue?
Web App
* [ ] Authorization code

**Is this a new or existing app? Existing single tenant app. We were using ADAL so far, and working on migrating to MSAL.

Repro
We use OIDC and we do not have clientsecrets, instead we use certificates. So do not pass ClientSecret in the request.

Receives the ID Token first.
Then try to get Access Token using the IDToken. However getting the below error.
{"Value cannot be null.\r\nParameter name: clientSecret"}

at Microsoft.Identity.Client.ConfidentialClientApplicationBuilder.WithClientSecret(String clientSecret)
at AadOnboarding.Core.MsalOpenIdAuthenticationOptionFactory.<>c__DisplayClass28_0.b__4(AuthorizationCodeReceivedNotification context) in E:\GIT\AD\AppModel\src\AadOnboarding\AadOnboarding\Core\MsalOpenIdAuthenticationOptionFactory.cs:line 268
at Microsoft.Owin.Security.OpenIdConnect.OpenIdConnectAuthenticationHandler.d__1a.MoveNext()

ConfidentialClientApplicationOptions confidentialClientoptions = new ConfidentialClientApplicationOptions()
                        {
                            ClientId = clientId,
                            TenantId = tenantId,
                            AadAuthorityAudience = AadAuthorityAudience.AzureAdMyOrg,
                            LogLevel = LogLevel.Verbose,
                            RedirectUri = this.GetRedirectUrl()
                        };
X509Certificate2 clientCert = this.authClient.GetCertificate(graphcertName);
IConfidentialClientApplication app = ConfidentialClientApplicationBuilder
                                .CreateWithApplicationOptions(confidentialClientoptions)
                                .WithAuthority(environmentAuthorityUrl, true)
                                .WithCertificate(clientCert)
                                .Build();

**Expected behavior**
I am expecting receive Access Token.

**Actual behavior**
A clear and concise description of what happens, e.g. exception is thrown, UI freezes  

**Possible Solution**
<!--- Only if you have suggestions on a fix for the bug -->

**Additional context/ Logs / Screenshots**
Add any other context about the problem here, such as logs and screebshots. Logging is described at https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/wiki/logging
@MarkZuber
Copy link
Contributor

This looks like it's a bug with the CreateWithApplicationOptions where we force a ClientSecret usage.

@MarkZuber MarkZuber self-assigned this Aug 9, 2019
@MarkZuber MarkZuber changed the title [Bug] [Bug] CCA CreateWithApplicationOptions forces use of ClientSecret, won't allow just Certificate usage Aug 9, 2019
@bgavrilMS
Copy link
Member

I agree, @dilansudharaka - can you try to use the Builder for all those options. There are methods on the builder for each of them (.Create(), .WithRedirectUri, .WithAudience, .WithLogging)

@MarkZuber MarkZuber added this to the 4.3.1 milestone Aug 9, 2019
@MarkZuber MarkZuber added regression Behavior that worked in a previous release that no longer works in a newer release Fixed labels Aug 9, 2019
@bgavrilMS
Copy link
Member

MSAL 4.3.1 released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Behavior that worked in a previous release that no longer works in a newer release
Projects
None yet
Development

No branches or pull requests

3 participants