Sustainsys / Saml2

Saml2 Authentication services for ASP.NET
Other
960 stars 602 forks source link

Start application without any registered IDPs, fail #486

Open maxim-s opened 8 years ago

maxim-s commented 8 years ago

if start application using OWIN middleware without any registered IDPs, will get error

Sequence contains no elements

Description: An unhandled exception occurred during the execution of the current web request. Please review the stack trace for more information about the error and where it originated in the code. 

Exception Details: System.InvalidOperationException: Sequence contains no elements

Source Error: 

Line 113:                lock(dictionary)
Line 114:                {
Line 115:                    return dictionary.Values.Skip(i).First();
Line 116:                }
Line 117:            }

If you can dynamic switch between IDPs, and also can dynamic add IDPs, why it's impossible to start without any registered IDP ?

AndersAbel commented 8 years ago

Because we never thought about that scenario. Can you provide a more complete stack trace? Do you have any suggestions on what to do instead?

maxim-s commented 8 years ago

Actually I thing it's not necessary to run SignInCommand if we have null idp

Exception Details: System.InvalidOperationException: Sequence contains no elements

Source Error: 

An unhandled exception was generated during the execution of the current web request. Information regarding the origin and location of the exception can be identified using the exception stack trace below.

Stack Trace: 

[InvalidOperationException: Sequence contains no elements]
   System.Linq.Enumerable.First(IEnumerable`1 source) +264
   Kentor.AuthServices.Configuration.IdentityProviderDictionary.get_Item(Int32 i) +126
   Kentor.AuthServices.WebSso.SignInCommand.Run(EntityId idpEntityId, String returnPath, HttpRequestData request, IOptions options, IDictionary`2 relayData) +154
   Kentor.AuthServices.Owin.<ApplyResponseChallengeAsync>d__1.MoveNext() +511
   System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) +92
   System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) +58
   Microsoft.Owin.Security.Infrastructure.<ApplyResponseCoreAsync>d__b.MoveNext() +292
   System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) +92
   System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) +58
   Microsoft.Owin.Security.Infrastructure.<ApplyResponseAsync>d__8.MoveNext() +278
   System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) +92
   System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) +58
   Microsoft.Owin.Security.Infrastructure.<TeardownAsync>d__5.MoveNext() +165
   System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) +92
   System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) +58
   Microsoft.Owin.Security.Infrastructure.<Invoke>d__0.MoveNext() +716
   System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) +92
   System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) +58
   Microsoft.Owin.Host.SystemWeb.IntegratedPipeline.<RunApp>d__5.MoveNext() +187
   System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) +92
   System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) +58
   Microsoft.Owin.Security.Infrastructure.<Invoke>d__0.MoveNext() +561
   System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) +92
   System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) +58
   Microsoft.Owin.Security.Infrastructure.<Invoke>d__0.MoveNext() +561
   System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) +92
   System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) +58
   Microsoft.Owin.Host.SystemWeb.IntegratedPipeline.<RunApp>d__5.MoveNext() +187
   System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) +92
   System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) +58
   Microsoft.Owin.Host.SystemWeb.IntegratedPipeline.<DoFinalWork>d__2.MoveNext() +185
   Microsoft.Owin.Host.SystemWeb.IntegratedPipeline.StageAsyncResult.End(IAsyncResult ar) +69
   Microsoft.Owin.Host.SystemWeb.IntegratedPipeline.IntegratedPipelineContext.EndFinalWork(IAsyncResult ar) +64
   System.Web.AsyncEventExecutionStep.System.Web.HttpApplication.IExecutionStep.Execute() +380
   System.Web.HttpApplication.ExecuteStep(IExecutionStep step, Boolean& completedSynchronously) +155
maxim-s commented 8 years ago

Problem here https://github.com/KentorIT/authservices/blob/master/Kentor.AuthServices/Configuration/IdentityProviderDictionary.cs#L115

AndersAbel commented 8 years ago

Looks a bit fishy - the comment on that method indicates that it should only be used by tests...

Looks like some stuff is lost from the stack trace in optimization. The problem is that SignInCommand tries to access IdentityProviderDictionary.Default during sign in.

What do you think it should do instead when there is no known identityprovider?

maxim-s commented 8 years ago

I think just don't run SignInCommand , and return the same challenge result to OWIN pipeline

AndersAbel commented 8 years ago

Yes, that makes sense - if the AuthServices middleware is configured as active and the auth type in the challenge is blank. A challenge with a blank auth type is basically a request that anyone that can handle it should. And in the case of no IDPs AuthServices can't handle it.

If the auth type in the challenge matches the one in the AuthServices options I still think it should throw an exception, because then it's a direct request to AuthServices that cannot be fulfilled.

You're welcome to add this and submit a PR, or if you want us to do it as a commercial assignment you can mail me for details.

dazinator commented 4 years ago

Just hit this by accident because I forgot to add the identity provider to the collection on startup. I guess this could be a legit issue that would then end up masked?