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

HttpClient HttpXsrfInterceptor does not set xsrf token for absolute urls? #18859

Closed
sant0shg opened this Issue Aug 24, 2017 · 17 comments

Comments

Projects
None yet
@sant0shg

sant0shg commented Aug 24, 2017

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[X ] Bug report  
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

In the HttpXsrfInterceptor xsrf.ts code, it is written in comments

// Skip both non-mutating requests and absolute URLs.
// Non-mutating requests don't require a token, and absolute URLs require special handling
// anyway as the cookie set
// on our origin is not the same as the token expected by another origin.
if (req.method === 'GET' || req.method === 'HEAD' || lcUrl.startsWith('http://') ||
      lcUrl.startsWith('https://')) {
      return next.handle(req);
}

So if the req.url starts with http or https, it does not set the header. What is the reason behind this?
Both the wikipedia article on csrf or the owasp guide do not mention any special handling of absolute urls

Expected behavior

I think expected behavior should be the token should be added for all requests (except non-mutating ones) irrespective of being absolute urls

Environment


Angular version: 4.X
@MrBlaise

This comment has been minimized.

MrBlaise commented Sep 13, 2017

Same here, after a long debug I found this to be the cause. I am using collection-json with links in my API, and the links are absolute. Trying to migrate to HttpClient from Http but this prevents it.

@MrBlaise

This comment has been minimized.

MrBlaise commented Sep 13, 2017

@netdeamon I suggest you add to the title that this is connected to HttpClient

@sant0shg sant0shg changed the title from XSRF token not set for absolute urls? to HttpClient HttpXsrfInterceptor does not set xsrf token for absolute urls? Sep 13, 2017

@ASEgorov

This comment has been minimized.

ASEgorov commented Oct 3, 2017

Also I found this issue #15052

@ASEgorov

This comment has been minimized.

ASEgorov commented Oct 3, 2017

It seems to me that that it is necessary to improve the XSRF-support to become possible to use URLs starting with http(s)

Why do URLs starting with http(s) require special handling?

There are several possible situations:

  1. The address of the backend does not equals the address from which the application is downloaded (for example, the application is started in the dev environment using ng serve, and the backend is launched on another port/server, etc.)
  2. The application works with one or many servers that provide access to its API and each of them is not connected to each other and accordingly has its own sequence of XSRF-tokens (in this case even the names of the cookies in which the tokens are stored may differ)
  3. The application works with several subdomains of the 3rd or subsequent levels (api.example.com, api2.example.com, etc.), and the sequence of XSRF-tokens is global for all subdomains.

Thus, depending on the scenario, angular can support different strategies, for example:

  1. Each domain sends its own sequence of tokens not allied with each other's domains. In this case, URLs not starting with http(s) are also considered as a separate domain.

or

  1. Subdomains of the 3rd or subsequent levels have a common sequence of tokens.

or

  1. All domains have a common sequence of tokens (is this even possible? And also issue above disables this behavior).

From the user's point of view, it can look something like this:

HttpClientXsrfModule.withConfig({
    commonXsrfsequences: 'NONE' // SUBDOMAINS, ALL etc.
  }),

but in this case it is not possible to configure the cookie name for each domain individually.

It would be possible to give the user the ability to specify his own class that implements the HttpXsrfTokenExtractor interface (maybe this is possible?), But to work properly, it is necessary to delete the checks lcUrl.startsWith ('http: //') || lcUrl.startsWith ('https: //') in

if (req.method === 'GET' || req.method === 'HEAD' || lcUrl.startsWith('http://') ||
lcUrl.startsWith('https://')) {

@alxhub You can explain what the "and absolute URLs require special handling" comment means. Did I understand the problem correctly?

@alxhub

This comment has been minimized.

Contributor

alxhub commented Oct 25, 2017

"and absolute URLs require special handling" means that it's up to the application to handle those URLs - the interceptor cannot know whether a given URL uses the same token, a different token, or no token at all. Providing an interceptor that can handle all of these cases is not in scope for common/http, but would be a great third-party library :)

I think expected behavior should be the token should be added for all requests (except non-mutating ones) irrespective of being absolute urls

This has two problems:

  1. It leaks the security token by default to all origins, including third parties (yikes!)
  2. It breaks CORS by including an extra header that the server will likely not permit.
@haidelber

This comment has been minimized.

haidelber commented Oct 31, 2017

A more sophisticated check for absolute URLs would be nice.. Instead of checking for http:// or https:// it would be amazing to check if the absolute URL matches the URL the application is loaded from. In this case I guess it would be ok to automatically attach a XSRF header. This would resolve this issue for people using absolute URLs that point to the same server.

@jrhite

This comment has been minimized.

jrhite commented Nov 10, 2017

@alxhub

@haidelber's idea seems like the most straightforward and common sense approach (although it's a little bit restrictive).

Another option that I mentioned in my issue (basically a dup of this), is just to allow the the client code to specify the allowable absolute domains by configuring the absolute URLs with .withOptions(). This approach is more flexible.

Breaking such a fundamental use case such as XSRF with CORS will be a deal breaker for a large number of Angular users. I can't use HttpClient because of this.

If @haidelber's suggestion, or my suggestion or not desirable what is the recommended approach for dealing with this?

@haidelber

This comment has been minimized.

haidelber commented Nov 10, 2017

@jrhite until this is fixed you can write your own simple XSRFInterceptor as I did. As long as you are sure to call only one API there are no security implications through this approach.

@Injectable()
export class XsrfInterceptor implements HttpInterceptor {

    constructor(private tokenExtractor: HttpXsrfTokenExtractor) {
    }

    intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
        let requestToForward = req;
        let token = this.tokenExtractor.getToken() as string;
        if (token !== null) {
            requestToForward = req.clone({ setHeaders: { "X-XSRF-TOKEN": token } });
        }
        return next.handle(requestToForward);
    }
}

With CORS you actually have to implement the TokenExtractor yourself as well..

@jrhite

This comment has been minimized.

jrhite commented Nov 11, 2017

@haidelber Thanks for the note, I'll use this for now.

@sajagekar

This comment has been minimized.

sajagekar commented Nov 20, 2017

@haidelber Can you brief about tokenExtractor, I am getting token as null, Also i have tried to fetch token from cookie, code attached. I can see cookie in response of a first request. but while fetching i get document as empty.

 getCookie(name) {
    if (!document.cookie) {
        return null;
    }

    const xsrfCookies = document.cookie.split(';')
        .map(c => c.trim())
        .filter(c => c.startsWith(name + '='));

    if (xsrfCookies.length === 0) {
        return null;
    }

    return decodeURIComponent(xsrfCookies[0].split('=')[1]);
}
@haidelber

This comment has been minimized.

haidelber commented Nov 20, 2017

@sajagekar Why don't you stick to the default extractor and just override the XSRF_COOKIE_NAME to the one your API sets?

@alxhub

This comment has been minimized.

Contributor

alxhub commented Nov 29, 2017

You can write your own interceptor to handle this case.

@alxhub alxhub closed this Nov 29, 2017

@meeroslav

This comment has been minimized.

meeroslav commented Dec 1, 2017

@haidelber, using default extractor does not work in case of CORS. The extractor is trying to read cookie from the document, but this cookie is not set if your app runs on a different domain.

This unfortunately is quite common if you run local client-side and point to deployed backend.

Currently for us, only solution is to rollback to old HttpModule.

@otello1971

This comment has been minimized.

otello1971 commented Jan 17, 2018

@meeroslav ....or you can avoid CORS and absolute urls using a Proxy Configuration:
set proxy.config.json in the root of your project and configure it like:

{
    "/api" : {
        "target" : "http://xx.xxx.xxx.xx", // Your remote address
        "secure" : false,
        "logLevel" : "debug", // Making Debug Logs in console
        "changeOrigin": true
    }
}

see : https://juristr.com/blog/2016/11/configure-proxy-api-angular-cli/

It worked for me! 👍👍👍

@meeroslav

This comment has been minimized.

meeroslav commented Jan 18, 2018

@otello1971, thanks. I am not using cli, but I'll look into the source to see how they handle proxy config.

I still think this should be part of HttpClient, though.

@meeroslav

This comment has been minimized.

meeroslav commented Jan 18, 2018

@otello1971, I just checked CLI and if I'm not wrong this proxy only refers to webpack dev server, which wouldn't help in production if client-side and API are not on the same domain.

@nishanth6

This comment has been minimized.

nishanth6 commented Sep 17, 2018

Does anyone can help me in modifying the below code written

As I'm Unable to generate cookies and add the XSRF Token
So the XSRF Token must be printed in the backend of my PHP script

Here is my below code in an Angular 6

import {HttpClient, HttpClientModule, HttpClientXsrfModule, HttpHeaders, HttpParams} from "@angular/common/http";

constructor(private _http:HttpClient) { }
...
apiURL = "//localhost/simple_api/insert.php";

addUser(info){
    return this._http.post(this.apiURL, info, {
      headers:  { 'Content-Type': 'application/x-www-form-urlencoded' },
      withCredentials: true,
      responseType: "arraybuffer",
    }
    )
    .subscribe(
                data => {
                    console.log("POST Request is successful ", data);
                },
                error => {
                    console.log("Error", error);
                }
            ); 
  }

Since I've changed the
apiURL = "http://localhost/simple_api/insert.php";
TO
apiURL = "//localhost/simple_api/insert.php";

Working Fine. But unable var_dump or print the headers attached in the PHP script.
But I'm getting the form data submitted successfully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment