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

Should RemoteAuthenticationOptions validate SignInScheme != null? #1378

Closed
HaoK opened this issue Aug 20, 2017 · 6 comments
Closed

Should RemoteAuthenticationOptions validate SignInScheme != null? #1378

HaoK opened this issue Aug 20, 2017 · 6 comments
Assignees

Comments

@HaoK
Copy link
Member

HaoK commented Aug 20, 2017

This should validation was lost at some point, SignInScheme is required for remote auth.

@HaoK
Copy link
Member Author

HaoK commented Aug 20, 2017

Or maybe we removed this validation to enable calling the default sign in scheme...

@HaoK HaoK changed the title RemoteAuthenticationOptions should validate SignInScheme != null Should RemoteAuthenticationOptions validate SignInScheme != null? Aug 20, 2017
@davidfowl
Copy link
Member

Can we also validate and SignInScheme != Scheme, are the schemes stored on the instances of the handlers themselves?

@HaoK
Copy link
Member Author

HaoK commented Aug 20, 2017

We will likely have to validate that outside of the normal validation (not AuthenticationSchemeOptions.Validate, but inside of the base RemoteAuthenticationHandler itself to have access to the scheme name, and we'd likely have to also explicitly check for when its the default that fallback to itself as well.

@kevinchalet
Copy link
Contributor

Can we also validate and SignInScheme != Scheme, are the schemes stored on the instances of the handlers themselves?

Exactly what I had recommended here: #1264 (comment)

We will likely have to validate that outside of the normal validation

You can easily do that in EnsureSignInScheme as you have access to both names:

public void PostConfigure(string name, TOptions options)
{
options.SignInScheme = options.SignInScheme ?? _authOptions.DefaultSignInScheme;
}

@Tratcher
Copy link
Member

Related issue to address:

public void PostConfigure(string name, TOptions options)
{
options.SignInScheme = options.SignInScheme ?? _authOptions.DefaultSignInScheme;
}

There should be an additional ?? _authOptions.DefaultScheme; for when DefaultSignInScheme isn't set. That's what was starting some of the stack overlfows by passing nulls.

@HaoK HaoK added 3 - Done and removed 2 - Working labels Sep 15, 2017
@HaoK
Copy link
Member Author

HaoK commented Sep 15, 2017

b9d9418

@HaoK HaoK closed this as completed Sep 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants