@azure/arm-appservice - webApps.getFunctionsAdminToken fails with error about Cookie

[johlrich]

• Package Name: @azure/arm-appservice
• Package Version: 0.1.0
• Operating system: Windows
• nodejs
  • version: v10.15.0
• browser
  • name/version:
• typescript
  • version: 3.2.4
• Is the bug related to documentation in
  • README.md
  • source code documentation
  • SDK API docs on https://docs.microsoft.com

Describe the bug
client.webApps.getFunctionsAdminToken returns an exception about Cookie not in expected domain
UnhandledPromiseRejectionWarning: Error: Cookie not in this host's domain. Cookie:my-azfn.scm.azurewebsites.net Request:management.azure.com

Setting Cookie header myself did nothing and I couldn't find any other options to overwrite this behavior

To Reproduce
Steps to reproduce the behavior:

1. call client.webApps.getFunctionsAdminToken where client is a WebSiteManagementClient

Expected behavior
Expect the call to complete

Screenshots
N/A

Additional context
I am using service principal authentication.
This works with the "azure-arm-website" package.

[wbreza]

@amarzavery - After looking into this further the Rest SDK should either not write the response cookies or at least provider an option to ignore these errors.

Suspect code is @ https://github.com/Azure/ms-rest-js/blob/ea7ceb86f1e6e6f7879e7e7ddfe791113762b65d/lib/axiosHttpClient.ts#L197

[amarzavery]

API that is causing issues:

GET https://management.azure.com/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Web/sites/{name}/functions/{functionName}?api-version=2018-02-01

The client does receive the 200 response. However, it fails on reading the cookie value present in the "Set-Cookie" response header.
Set-Cookie: 'ARRAffinity=20a1717378e36ca6eef5d7a9ef01b04cdb43003687b1d2f915704ceec819965a;Path=/;HttpOnly;Domain=eventgridtesting.scm.azurewebsites.net'.

tough-cookie fails on validation because the domain in the cookie is different from the host/domain of request url.

Error: Cookie not in this host's domain. Cookie:eventgridtesting.scm.azurewebsites.net Request:management.azure.com

I feel this is a server side issue. The server shouldn't be doing that. This seems to be a security flaw with the server.

Why it does not work in the new JS sdk ?

In the new sdk @azure/arm-appservice, we have enabled cookies by default in the new runtime @azure/ms-rest-js. We are also using a different http request library named axios for node.js which does not support cookies out of the box. Hence we had to add support for cookies in the runtime.

Why it works in the old node sdk?

In the old node sdk azure-arm-website, we were using the request library in our runtime. request has in built support for cookies. It uses the same library tough-cookie that we use in the new runtime. However, cookies are disabled by default. I tried a sample with "jar": true in the requestOptions. It did enable cookie support. However, request does that with ignoreErrors: true. Thus errors from tough-cookie do not bubble up to the request library.

This makes me wonder, what is the purpose of using the cookie library then.

Cookies are important for browser scenario. However for node.js, it seems that the default approach that we would want is: if it works then that's great. However do not block on failures.

The tough cookie library validation for domain matching fails, which fails the request.

@daschult @bterlson @kpajdzik - What are your thoughts on this?

@naveedaz - It would be nice if the AppService team can fix this bug on the server side.

[mathewc]

The ARM API you're using to get the function is returning that SCM domain header because many of the Functions ARM APIs delegate internally to the Kudu/SCM site which adds that header. We return that response from the ARM API. I wouldn't characterize this as an AppService bug. It sounds like you're just using an overly sensitive cookie library. Can't you just stop using that?

[bterlson]

I am ok with not throwing on cookie validation failures in Node assuming people more familiar with the security model are also OK with that. But wouldn't the cookie just fail to set in the browser, so this should be fixed regardless?

[amarzavery]

@bterlson - That is true. This will fail in a browser.

@mathewc - The cookie library that we are using is fairly popular in the JS world. If the strict validation would have been a problem then we would see some issues related to that on their github repo.

It would be nice if the AppService can scrub those headers before returning that response to the customer.

[jeremymeng]

The ARM API you're using to get the function is returning that SCM domain header because many of the Functions ARM APIs delegate internally to the Kudu/SCM site which adds that header. We return that response from the ARM API.

@mathewc would app service API still work fine if our SDK ignores the Set-Cookie header and don't do anything? We are considering removing cookie support from SDK core libraries, and we don't feel that there's an actual need to use cookies in REST APIs.