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

Simple way to remove ReturnUrl From Custom (Cookie and maybe other types of) Authentication. #20403

Closed
SomeProgrammerGuy opened this issue Mar 31, 2020 · 22 comments
Labels
area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved
Milestone

Comments

@SomeProgrammerGuy
Copy link

Asp.Net Core 3.1xxxx custom cookie authentication.

I answered a question on StackOverflow

https://stackoverflow.com/questions/3716153/how-to-remove-returnurl-from-url/9796693#9796693

asked over nine years ago and yet you still don't seem to have implemented a simple way to do this without "going around the houses".

I've have Googled much this evening and after all this time I am surprised this is still an issue. I apologise in advance if I didn't pick up a new simple way.

Many folk want this.

Yes we can write all sorts of workarounds but we shouldn't really have to.

Adding to this (noting I couldn't find anything on Google about this either) a return to "(maybe sign in)" page setting, even if the page being accessed doesn't exist, without the return actually showing the page (didn't authenticate or didn't exist.)

services.AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme)
    .AddCookie(options =>
    {
        options.AccessDeniedPath = "/Foo";
    });

(though I'm sure it's useful to somebody) to me just says "yes this resource actually exists".

I don't want automated "hacker" tools knowing that an authenticated folder page / authorised folder page / unauthenticated folder page exists or that it doesn't.

For note using tools like DIRB and DirBuster etc.

@scalablecory scalablecory transferred this issue from dotnet/core Apr 1, 2020
@javiercn javiercn added the area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer label Apr 1, 2020
@blowdart
Copy link
Contributor

blowdart commented Apr 1, 2020

To summarise this do you want into actual results do you want

  • No authentication to happen when someone browses to a URL that requires authentication, you just want a 404 to be returned?
  • Authentication to happen, but no returnURL parameter appended to the login page URL so when someone logins in they're just returned to a fixed URL you decide?

Or something else entirely?

@SomeProgrammerGuy
Copy link
Author

SomeProgrammerGuy commented Apr 1, 2020

My apologies if I've made the question confusing.

I have created a web app (CORE 3.1 / Razor Pages) using the built in (Identity parts) custom Cookie Authentication / Authorisation.

All working.

Lets say configured like so:

services.AddRazorPages()
                .AddRazorPagesOptions(options => {
                    options.Conventions.AuthorizeFolder("/Secure/");
                    options.Conventions.AllowAnonymousToPage("/SignIn");
                });

I haven't yet signed in.

Firstly if I attempt to go to the page: [SITE]/Secure/I-Exist WHICH EXISTS

This will fail (as I havn't signed in) and return me to the page: //[SITE]/SignIn?ReturnUrl=/Secure/I-Exist

What I would like is a simple configuration change (not lots of extra coding) for it to return me to //[SITE]/SignIn instead. Noting without the "?ReturnUrl=/Secure/I-Exist" parameter.

Secondly if I attempt to go to the page: [SITE]/Secure/I-Dont-Exist WHICH DOESN'T EXISTS

Then also a simple configuration change to return me to //[SITE]/SignIn without identifying if the [SITE]/Secure/I-Dont-Exist page existed or not. Maybe a 401 rather than a 404.

@blowdart blowdart added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Apr 1, 2020
@blowdart
Copy link
Contributor

blowdart commented Apr 1, 2020

Ok got it. This are rather specialist requests. Honestly if you're worried about a scanning tool discovering URIs I tend to lean on you're aiming for security by obscurity here, which isn't security at all.

However:

The first, not sure, @Tratcher any ideas? Is there an event?
The second is a routing issue, @pranavkm any ideas?

@pranavkm
Copy link
Contributor

pranavkm commented Apr 1, 2020

You might be able to get away with putting adding a Secure/Index.cshtml with a catch-all parameter. It will end up handling routes that do not match, but start with /Secure:

@* Secure/Index.cshtml *@
@page "{**extra}"

@SomeProgrammerGuy
Copy link
Author

SomeProgrammerGuy commented Apr 1, 2020

"I tend to lean on you're aiming for security by obscurity here, which isn't security at all"

I really hate it when folk pull this nonsensical statement up.

"security by obscurity" is a large topic but it is important.

Cutting that topic very short.

Things like algorithms (encryption etc.) it should certainly never be used for.

Attempting to keep a *GOOD security professional at bay then its also barely a hindrance.

As to the millions of Chinese, Russian, North Korean bots running simple automated scripts that are looking for something very specific (generally) before they move on can be the difference between a site being compromised or not.

Noting a lot of compromised sites I have audited over many years as a (38+ years security professional), this was exactly the case.

It could be as simple as changing SITE/wp-admin to SITE/MyLogin to stop simple bots knowing your site is Wordpress. (A simplified example I know).

"security by obscurity" is just an additional tool on top of many others but it can have a huge impact in some cases.

Just because a military Tank has 100 Cannons and 20 Inches of armour doesn't mean you want to paint it orange with a target on top!!

Ignoring all that (as this isn't really the place for long discussion) its not really a specialist request.

Folk have been asking for this simple feature for the last approx. ten years:

https://www.google.co.uk/search?source=hp&ei=KaKEXtq-KsWVkwXB8IuwDg&q=asp+how+to+remove+returnurl+from+url&oq=asp+how+to+remove+returnurl+from+url&gs_lcp=CgZwc3ktYWIQAzoECAAQHjoECCEQClDcClj5sAFgp7sBaABwAHgAgAF9iAGhFJIBBDI0LjaYAQCgAQKgAQGqAQdnd3Mtd2l6&sclient=psy-ab&ved=0ahUKEwia96KbtcfoAhXFyqQKHUH4AuYQ4dUDCAc&uact=5

@mkArtakMSFT mkArtakMSFT removed the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Apr 1, 2020
@Tratcher
Copy link
Member

Tratcher commented Apr 1, 2020

Those redirects are generated by CookieAuth and can be customized in the events. Here's an example that removes the query from the redirect to the login page.

            .AddCookie(options =>
            {
                options.Events = new CookieAuthenticationEvents()
                {
                    OnRedirectToLogin = redirectContext =>
                    {
                        var uri = redirectContext.RedirectUri;
                        UriHelper.FromAbsolute(uri, out var scheme, out var host, out var path, out var query, out var fragment);
                        uri = UriHelper.BuildAbsolute(scheme, host, path);
                        redirectContext.Response.Redirect(uri);
                        return Task.CompletedTask;
                    }
                };
            });

@SomeProgrammerGuy
Copy link
Author

Hi Tratcher,

That looks pretty good is it possible to add some basic explanation for myself and any future users reading this post.

@Tratcher
Copy link
Member

Tratcher commented Apr 1, 2020

protected override async Task HandleChallengeAsync(AuthenticationProperties properties)
{
var redirectUri = properties.RedirectUri;
if (string.IsNullOrEmpty(redirectUri))
{
redirectUri = OriginalPathBase + OriginalPath + Request.QueryString;
}
var loginUri = Options.LoginPath + QueryString.Create(Options.ReturnUrlParameter, redirectUri);
var redirectContext = new RedirectContext<CookieAuthenticationOptions>(Context, Scheme, Options, properties, BuildRedirectUri(loginUri));
await Events.RedirectToLogin(redirectContext);
}

Challenge is called when an anonymous users hits a page requiring auth. ReturnUrl is generated based on the current request so you can come back after logging in. It's then appended to the query string of the login page. OnRedirectToLogin is then called to let you customize the value.

@SomeProgrammerGuy
Copy link
Author

I do still think this should be a simple config change but thank you, that amount of additional code isn't terrible on the scale of things.

@Tratcher
Copy link
Member

Tratcher commented Apr 1, 2020

Perhaps, the trouble is that removing this value breaks the end-to-end scenario. The login page can't return you to the original resource.

@SomeProgrammerGuy
Copy link
Author

SomeProgrammerGuy commented Apr 1, 2020

I would just like some sort of (include the return URL / don't return the return URL) type of setting. I understand why both groups of people might require one of each.

Especially now as a lot of applications rely on various JavaScript client libraries that don't play well with returning to a page from an unrelated (sign in) page. I suppose as many folk use a hybrid of ASP.NET core server side and third party JavaScript client libraries like myself.

I personally and many others want an app to start from the "beginning" so to speak after a sign in. And absolutely the original setting that I'm sure may folk want as well.

It should always be about "reasonable" easy configurable choice.

The problem especially with DOTNET is that it is moving so quickly these "more complicated" additions to do what should be simple get a little confusing as the source keeps changing.

Maybe something as simple as:

options.Conventions.AuthorizeFolder("/Secure/", bool DO_OR_DONT_SHOW_RETURN_URL, bool RETURN_401_OR_404); type of thing (or whatever :-) .)

@blowdart blowdart added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Apr 1, 2020
@blowdart
Copy link
Contributor

blowdart commented Apr 1, 2020

It should always be about "reasonable" easy configurable choice

Kind of. It's about a reasonable solution to the scenario we're trying to solve, and that scenario and ease of use is based on ReturnURL, and returning the user to the page they requested before having to login. Bumping a user to a fixed page, or indeed returning 403 against pages that don't exist aren't in our scenarios, and thus require work.

For Javascript clients, well, login should probably be via OIDC for SPAs and their ilk (although Safari is arguably making SPAs very difficult now in their latest privacy update), and a client side ODIC would still involve a redirect away, but the return URL would be to a fixed URL.

@javiercn javiercn removed the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Apr 2, 2020
@blowdart blowdart added this to the Discussions milestone Apr 2, 2020
@blowdart blowdart added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Apr 2, 2020
@blowdart
Copy link
Contributor

blowdart commented Apr 2, 2020

Does Pranav's suggestion work for your first issue? It seems the MVC folks are anxious to remove their label, but I want confirmation that it works before that happens.

@blowdart blowdart added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Apr 3, 2020
@SomeProgrammerGuy
Copy link
Author

My apologies for the late reply.

I'm currently tied up with some emergency development relating to this pandemic in the UK.

I'll try and test this if I get a moment this weekend.

Be safe all. It's starting to get a little crazy over here...

@ghost ghost added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Apr 4, 2020
@SomeProgrammerGuy
Copy link
Author

SomeProgrammerGuy commented Apr 5, 2020

I've had a spare hour this evening to try the code @Tratcher posted and it works great.

As to the intercepting the 404's I created a simple custom middleware that though I havn't exactly tested in 'anger' seems to do the job.

As below:

public class CustomMiddlewareIntercept404
    {
        private readonly RequestDelegate _next;

        public CustomMiddlewareIntercept404(RequestDelegate next)
        {
            _next = next;
        }

        async public Task Invoke(HttpContext httpContext)
        {
            await _next(httpContext);

            if (httpContext.Response.StatusCode == 404)
            {
                // Does the sign in page exist? If not then don't create an infinite loop. 
                if (httpContext.Request.Path == "/SignIn")
                {
                    await httpContext.Response.WriteAsync("ERROR: Page doesn't exist.");
                }
                else
                {
                    httpContext.Response.Redirect("/SignIn", true);
                }
            }
        }
    }

    // Extension method used to add the middleware to the HTTP request pipeline.
    public static class CustomMiddlewareIntercept404Extensions
    {
        public static IApplicationBuilder UseCustomMiddlewareIntercept404(this IApplicationBuilder builder)
        {
            return builder.UseMiddleware<CustomMiddlewareIntercept404>();
        }
    }

Although I think these should be an easy config option I must admit the fixes where very clean and well within the spirit of ASP.NET Core, nor did they require any over convoluted complicated code as in the past.

@Tratcher
Copy link
Member

Tratcher commented Apr 6, 2020

@SomeProgrammerGuy design guideline: avoid modifying the response after calling next. What code is generating the 404 in this case? Something you added, or the request naturally reaching the end of the pipeline? If it's the latter then it's best to put your middleware at the end of the pipeline and generate the response (or 404) yourself rather than trying to react to a 404 generated elsewhere (e.g. remove the call to next).

@SomeProgrammerGuy
Copy link
Author

SomeProgrammerGuy commented Apr 6, 2020

Thankyou for the feedback @Tratcher. I havn't used .NET Core in anger yet but before I go and do some reading on the above this evening can I just verify what you are saying with the following code.

Core 3.1x

Razor Pages and API (probably).

Cookie authentication (as built in) (Custom database stuff not shown).

Everything from the root \ up Authenticated (Authorised) except the sign in page itself.

Ignoring error pages, CORS and all the Security added extras for simplicity.

Unauthenticated pages >> go back to the SignIn page.

Missing pages (404) >> go back to the SignIn page.

This is the partial code from the CustomMiddlewareIntercept404:

async public Task Invoke(HttpContext httpContext)
{
    // **** AS YOU SAY ABOVE REMOVE THIS ****
    // await _next(httpContext);

    if (httpContext.Response.StatusCode == 404)
    {
        // Does the sign in page exist? If not then don't create an infinite loop. 
        if (httpContext.Request.Path == "/SignIn")
        {
            await httpContext.Response.WriteAsync("ERROR: Page doesn't exist.");
        }
        else
        {
            httpContext.Response.Redirect("/SignIn", true);
        }
    }
}

Services.cs Partial Code:

 public void ConfigureServices(IServiceCollection services)
{
    _connectionString = Configuration["Foo:ConnectionString"];

    services.AddControllers();

    services.AddRazorPages()
        .AddRazorPagesOptions(options => {
            options.Conventions.AuthorizeFolder("/");
            options.Conventions.AllowAnonymousToPage("/SignIn");
            options.Conventions.AddPageRoute("/SignIn", "");
        });

    // Configure basic cookie authentication.
    services.AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme)
        .AddCookie(options =>
        {
            options.LoginPath        = "/SignIn";
            options.LogoutPath       = "/SignIn";
            options.AccessDeniedPath = "/SignIn";

            // Remove the ReturnUrl GET parameter from the sign in page.
            options.Events = new CookieAuthenticationEvents()
            {
                OnRedirectToLogin = redirectContext =>
                {
                    string redirectUri = redirectContext.RedirectUri;

                    UriHelper.FromAbsolute(
                        redirectUri, 
                        out string scheme, 
                        out HostString host, 
                        out PathString path, 
                        out QueryString query, 
                        out FragmentString fragment);

                    redirectUri = UriHelper.BuildAbsolute(scheme, host, path);
                    
                    redirectContext.Response.Redirect(redirectUri);
                    
                    return Task.CompletedTask;
                }
            };
        });


}

// This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
{
    if (env.IsDevelopment())
    {
        app.UseDeveloperExceptionPage();
    }
    else
    {
        app.UseExceptionHandler("/Error");
    }

    // **** IS THIS WHERE YOU ARE SAYING PUT THE MIDDLEWARE ? IT'S WHERE I CURRENTLY HAVE IT****
    app.UseCustomMiddlewareIntercept404();

    app.UseStaticFiles();

    app.UseRouting();

    app.UseAuthentication();
    app.UseAuthorization();

    app.UseEndpoints(endpoints =>
    {
        endpoints.MapControllers();
    });

    app.UseEndpoints(endpoints =>
    {
        endpoints.MapRazorPages();
        
    });
}

@Tratcher
Copy link
Member

Tratcher commented Apr 7, 2020

Try this:

async public Task Invoke(HttpContext httpContext)
{
        // Does the sign in page exist? If not then don't create an infinite loop. 
        if (httpContext.Request.Path == "/SignIn")
        {
            httpContext.Response.StatusCode = 404;
            await httpContext.Response.WriteAsync("ERROR: Page doesn't exist.");
        }
        else
        {
            httpContext.Response.Redirect("/SignIn", true);
        }
}
public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
{
    if (env.IsDevelopment())
    {
        app.UseDeveloperExceptionPage();
    }
    else
    {
        app.UseExceptionHandler("/Error");
    }

    app.UseStaticFiles();

    app.UseRouting();

    app.UseAuthentication();
    app.UseAuthorization();

    app.UseEndpoints(endpoints =>
    {
        endpoints.MapControllers();
        endpoints.MapRazorPages();
    });

    // Requests only reach here if they were not handled by another component. There's an implicit component at the end of the pipeline that would set a 404, let's replace it.
    app.UseCustomMiddlewareIntercept404();
}

@SomeProgrammerGuy
Copy link
Author

SomeProgrammerGuy commented Apr 7, 2020

@Tratcher Sorry my bad I missed something in your code.

That's starting to make more sense now.

Actually now I understand them a little better this middleware stuff is pretty good.

I'd say that works well now and I think there is plenty of info there for other and myself.

In fact I think these small snippets of code are probably better than lots of config options.

Thank you I think you can mark this as resolved.

@Tratcher

This comment has been minimized.

@SomeProgrammerGuy
Copy link
Author

See Above.

@Tratcher Tratcher added ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. and removed Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. labels Apr 7, 2020
@ghost ghost added the Status: Resolved label Apr 7, 2020
@ghost
Copy link

ghost commented Apr 8, 2020

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

@ghost ghost closed this as completed Apr 8, 2020
@ghost ghost locked as resolved and limited conversation to collaborators May 8, 2020
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved
Projects
None yet
Development

No branches or pull requests

6 participants