auth0 / auth0-aspnet-owin

Auth0 ASP.NET 4.5 Owin/Katana Authentication Handler
MIT License
46 stars 50 forks source link

Relative paths are not supported properly #50

Closed arieradle closed 7 years ago

arieradle commented 7 years ago

At auth0-aspnet-owin/src/Auth0.Owin/Auth0AuthenticationHandler.cs (line 295):

string redirectUri = context.RedirectUri ?? Options.RedirectPath.ToString();

At this point, the redirect URI is determined by the value configured in Options.RedirectPath. I think this value should be relative to RequestPathBase.

For example:

my redirect path is Options.RedirectPath = "/redirect".

In IIS, my application configured to reside under "/app", bound to www.acme.com, meaning RequestPathBase will be "/app", and the redirectUri should be /app/redirect, and thus when the redirect is applied, it should be "www.acme.com/app/redirect", but instead it is "www.acme.com/redirect", which is wrong,

Is it possible that line 295 should be changed to something like: string redirectUri = context.RedirectUri ?? RequestPathBase + Options.RedirectPath.ToString() ?

jerriep commented 7 years ago

@arieradle I pushed v2.2.1 to NuGet to resolve this issue for you

arieradle commented 7 years ago

Thanks, I'll test. By the way, we use Auth0-ASPNET-Owin-Libs nuget, I need it updated as well.

jerriep commented 7 years ago

@arieradle Since V2 of Auth0-ASPNET-Owin, we have remove all the extra content (such as the Auth0AccountController) which was added. There is therefore no more need for using the libs-only package. As a matter of fact I should actually deprecate that package.

I would appreciate if you could upgrade to the normal Auth0-ASPNET-Owin package.

If there is another reason why you still would require the libs-only package?

arieradle commented 7 years ago

Here is the list of dependencies for the package

.NETFramework 4.5 Microsoft.AspNet.Mvc (>= 5.0.0) Microsoft.Owin (>= 3.0.1) Microsoft.Owin.Host.SystemWeb (>= 3.0.1) Microsoft.Owin.Security (>= 3.0.1) Newtonsoft.Json (>= 7.0.1) Owin (>= 1.0.0)

We don't want the marked dependencies in our project - the project is a pure OWIN solution, without any AspNet.Mvc (we use Nancy instead). The reason for that is that we want to decouple the code from IIS. AspNet.Mvc has a dependency on System.Web, which is coupled to IIS.

In addition, we run our project on a host of our choosing, which can be a Console application as well as IIS, thus we can't have hardcoded dependency on Microsoft.Owin.Host.SystemWeb, and frankly there is no reason why there will be.

In fact, I was the person who asked to create a separate Auth0-ASPNET-Owin-Libs Nuget for the reasons listed above.

If all the extra content was removed, can you please remove the dependencies Microsoft.AspNet.Mvc and Microsoft.Owin.Host.SystemWeb from the package? Or is there a reason to keep them?

jerriep commented 7 years ago

My apologies for not fixing those dependencies earlier. They are now fixed in v2.2.2

It may take a few minutes to be indexed on NuGet

arieradle commented 7 years ago

Wow! Great. Thanks for the quick response. In this case, the second library can be deprecated in deed.