Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

OpenIdConnectAuthenticationMiddleware initialization issue #128

Closed
Desislav opened this issue Jan 26, 2015 · 3 comments
Closed

OpenIdConnectAuthenticationMiddleware initialization issue #128

Desislav opened this issue Jan 26, 2015 · 3 comments

Comments

@Desislav
Copy link

Hello,

I was looking at the OpenIdConnectAuthenticationMiddleware and i found some strange lines of code inside its constructor:

else if (!(string.IsNullOrWhiteSpace(Options.MetadataAddress) && string.IsNullOrWhiteSpace(Options.Authority)))
{
    //If the previous if statement is true how this condition can be ever satisfied??
   if (string.IsNullOrWhiteSpace(Options.MetadataAddress) && !string.IsNullOrWhiteSpace(Options.Authority))
   {
       Options.MetadataAddress = Options.Authority;
       if (!Options.MetadataAddress.EndsWith("/", StringComparison.Ordinal))
       {
             Options.MetadataAddress += "/";
       }
       Options.MetadataAddress += ".well-known/openid-configuration";
   }

   HttpClient httpClient = new HttpClient(ResolveHttpMessageHandler(Options));
   httpClient.Timeout = Options.BackchannelTimeout;
   httpClient.MaxResponseContentBufferSize = 1024 * 1024 * 10; // 10 MB
   Options.ConfigurationManager = new ConfigurationManager<OpenIdConnectConfiguration>(Options.MetadataAddress, httpClient);
 }
@Tratcher
Copy link
Member

I agree it's not the clearest logic, but I think it's technically correct. Let me try writing it in English to see if it makes any more sense.

First:
If the user has specified MetadataAddress and/or Authority
And then:
If the user did not specify metadata address, but did specify Authority

So specifying MetadataAddress gets you into the first if but skips the second one, where specifying Authority gets you into both.

The first statement could also be written as:

else if (!string.IsNullOrWhiteSpace(Options.MetadataAddress) || !string.IsNullOrWhiteSpace(Options.Authority))

I don't think the second if can be simplified.

@Desislav
Copy link
Author

Oh i missed the first bracket after the "!" operator. Sorry about that. Actually your second aproach is what i had in mind.

Anyway what i meant is that the second aproach that you wrote is way more better in terms of readability.

@kevinchalet
Copy link
Contributor

Mouahahaha, I told you this condition was too hard to read: #112 (comment) 😆

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants