Pass extra path info and query parameters as 'state' across login #16
Conversation
…cess. By default, a successful login should return the user back to original URL without any loss of fidelity. The only change should be the addition of an access token.
There was a problem hiding this comment.
There are currently two issues that prevent me from taking this great contribution as-is:
- We do not want to change the default behaviour and would prefer to have this opt-in.
- I am concerned about using
statein a way that might expose authenticated webtask users to CSRF or open redirection issues (see inline comment)
Thanks for another fantastic PR.
| return options.loginError({ | ||
| code: 401, | ||
| message: 'Unauthorized.', | ||
| error: 'Missing access token.', | ||
| redirect: routingInfo.baseUrl + '/login' | ||
| redirect: routingInfo.baseUrl + '/login?state=' + encodeURIComponent(JSON.stringify(stateInfo)) |
There was a problem hiding this comment.
In OAuth2, it is my understanding that the state parameter should be (or contain) an unguessable identifier that uniquely identifies the request to the client's (browser) authentication state. This is used to prevent CSRF attacks.
It also seems that because the ultimate redirect is encoded as an unsigned value in the url, a webtask using this authn/authz tooling would be vulnerable to open redirection issues.
One experimental approach I came across to mitigate these issues is described at https://tools.ietf.org/html/draft-bradley-oauth-jwt-encoded-state-00 whereby the state parameter will contain a signed jwt w/ the same information as stateInfo.
What do you think?
There was a problem hiding this comment.
Ahhh, I think you're right.
The security issue is a good point, and using a JWT is a great suggestion. Thanks for the link to the IETF draft.
So, probably a better approach that meets the opt-in desire would be to add a 'state' element to the options object. This would be a function that returns an opaque state object that gets wrapped up into a JWT by the webtask-tools package.
The webtask-tools package would also decode the JWT on the other side, and make the state object available via the loginSuccess() interface upon completion of a login sequence.
The default implementation of options.state could be to simply return a null value, in which case no state information would be carried around, mimicking the current behavior.
Something like that, yes? It seems closer to the design patterns already in place...
There was a problem hiding this comment.
A related IETF draft is described at https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-09. In this draft, all of the query parameters in an OAuth 2.0 authorization request can be encoded in a JWT, not just the state parameter.
There was a problem hiding this comment.
Hi @scott-parsons, I spoke to @jaredhanson about this question. His recommendation was to store the redirect state in some form of client storage (cookie, localstorage) as a mapping of a unique, random thunk to the state.
For example:
{
'adkd9242n22309jalk23': { path: '/...', ... }
}The thunk value is then used as the state query parameter. When the flow redirects to the client upon auth completion, the client (browser) must ensure that it has a record of this thunk and can then take appropriate action. Of course, encoding this state in a signed (and encrypted, ideally) JWT is also possible but ads complexity.
There was a problem hiding this comment.
I like that approach, as the state information only exists between the user agent and the webtask code, and isn't shared with other parties, including the authentication service.
A cookie with a unique name is used to hold the desired post-login state information, while the cookie name is carried through the login process. Thus, state information only exists between the user agent and the webtask, and is not shared with the authentication service.
A cookie with a unique name is used to hold the desired post-login state information, while the cookie name is carried through the login process. Thus, state information only exists between the user agent and the webtask, and is not shared with the authentication service.
|
Commit |
Incorporate 3.2.0 from upstream as the new baseline
On the next revision, can we have webtask-tools remember the original URL request and pass it as state information?
Our use-case is to let a user/app start at a “deep link” like:
https://{container}.us.webtask.io/{jtn}/extra/path/info?param=valuethen go through the authentication process, and end up back at that same link with the same query parameters after a successful login (and the addition of an access token to the request).