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

fix: Workaround csrf redirects #667

Merged
merged 10 commits into from Nov 4, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Expand Up @@ -31,7 +31,7 @@
- [Generator] Fix function imports (OData V2 + V4) and action imports, where the return type is a primitive edm type like `Edm.String`.
- [Generator] Fix path references in generator configuration file to be treated as relative to the configuration file.
- [Core] Fix expiration times in the client credentials cache.

- [Core] Fix csrf request redirects against On-Premise systems

# 1.30.0

Expand Down
46 changes: 46 additions & 0 deletions knowledge-base/how-tos/remote-debugging.md
@@ -0,0 +1,46 @@
# How to remote debug an application on SAP Cloud Platform
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this got in by accident?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes


## TLDR;

Change your app to start to start in debug mode, ssh tunnel your local port 9229 to the remote port of 9229 of your application and start a remote debugging session from your IDE.

## Steps

### Deploy your application with debug mode

* Replace the start script to run in debug mode. E. g. instead of `node start.js` use `node --inspect start.js` for node and instead of `ts-node start.ts` use `node -r ts-node/register --inspect start.ts`.
**Note:** Don't use `--inspect-brk`, this will make the start timeout on SAP Cloud Platform.

Deploy your application as usual, run:
```shell
cf deploy
```

### Open an ssh tunnel to your application
Open an ssh tunnel to your backend application to connect your local debugger with the node inspector running on port 9229. Replace *<your-app-name>* with your application name and run:
```shell
$ cf ssh <your-app-name> -L 9229:127.0.0.1:9229 -T -N
```
### Attach a local debugger
Now you can attach your local debugger. For this you will have to launch a debugger that attaches to the remote session. In VSCode this is a launch configuration you can use, when you replace *<path-to-your-application>* with the relative path to your application directory:
```json
{
"version": "0.2.0",
"configurations": [
{
"type": "node",
"request": "attach",
"name": "Attach to Remote",
"address": "127.0.0.1",
"port": 9229,
"localRoot": "${workspaceFolder}/<path-to-your-application>",
"remoteRoot": "/home/vcap/app",
"skipFiles": [
"<node_internals>/**"
]
}
]
}
```

Start the debugger and happy debugging!
19 changes: 19 additions & 0 deletions packages/core/src/header-builder/csrf-token-header.ts
@@ -1,4 +1,5 @@
import { createLogger, errorWithCause } from '@sap-cloud-sdk/util';
import { AxiosError } from 'axios';
import { HttpRequestConfig, executeHttpRequest } from '../http-client';
import { Destination, DestinationNameAndJwt } from '../scp-cf';
import { filterNullishValues, getHeader, getHeaderValue } from './header-util';
Expand Down Expand Up @@ -49,12 +50,30 @@ function makeCsrfRequest<T extends HttpRequestConfig>(
.then(response => response.headers)
.catch(error => {
if (!error.response) {
// TODO: remove once https://github.com/axios/axios/issues/3369 is fixed
const retry = axiosWorkaround(error, requestConfig, destination);
if (retry) {
return retry;
}
throw errorWithCause('The error response is undefined.', error);
}
return error.response.headers;
});
}

function axiosWorkaround<T extends HttpRequestConfig>(
error: AxiosError,
axiosConfig: Partial<T>,
destination: Destination | DestinationNameAndJwt
) {
if (error.request._isRedirect) {
marikaner marked this conversation as resolved.
Show resolved Hide resolved
return makeCsrfRequest(destination, {
...axiosConfig,
url: error.request._options.path
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a API to get the path? Accessing _options is potentially dangerous because this variable is not part of the public API of the request object.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps a check with good logs statement in case the path is not present.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a log statement. I don't think that there is another api for the request, but for extra safety I added a check that the path exists.

marikaner marked this conversation as resolved.
Show resolved Hide resolved
});
}
}

function validateCsrfTokenResponse(responseHeaders: Record<string, any>) {
if (!responseHeaders['x-csrf-token']) {
logger.warn(
Expand Down
@@ -1,4 +1,5 @@
import { createLogger } from '@sap-cloud-sdk/util';
import nock = require('nock');
import { buildCsrfHeaders } from '../../../src/header-builder';
import {
createCreateRequest,
Expand All @@ -8,9 +9,11 @@ import {
import {
defaultBasicCredentials,
defaultDestination,
defaultHost,
mockHeaderRequest
} from '../../test-util/request-mocker';
import { addCsrfTokenAndCookies } from '../../../src/header-builder/legacy-csrf-token-header';
import { Destination } from '../../../src';

const standardHeaders = {
accept: 'application/json',
Expand Down Expand Up @@ -146,4 +149,36 @@ describe('csrf-token-header', () => {
expect('cookie' in actual).toBeFalsy();
});
});

it('should redirect csrf request when using proxy', async () => {
const destination: Destination = {
...defaultDestination,
proxyType: 'OnPremise'
};
const request = createCreateRequest(destination);
const mockedHeaders = {
'x-csrf-token': 'mocked-x-csrf-token',
'set-cookie': ['mocked-cookie-0;mocked-cookie-1', 'mocked-cookie-2']
};

nock(defaultHost)
.get(request.serviceUrl())
.reply(307, undefined, {
location: `${defaultHost}${request.serviceUrl()}/`
});

nock(defaultHost)
.get(request.serviceUrl() + '/')
.reply(200, undefined, mockedHeaders);

const expected = {
cookie: 'mocked-cookie-0;mocked-cookie-2',
'x-csrf-token': mockedHeaders['x-csrf-token']
};
const headers = await buildCsrfHeaders(request.destination!, {
headers: standardHeaders,
url: request.relativeServiceUrl()
});
expect(headers).toEqual(expected);
});
});