Parse-SDK-JS icon indicating copy to clipboard operation
Parse-SDK-JS copied to clipboard

Pass Bearer token as query request option

Open stephannielsen opened this issue 2 years ago • 13 comments

New Feature / Enhancement Checklist

Current Limitation

We are using JWT based authentication (https://github.com/parse-community/parse-server/pull/6411) on server side together. Currently, we do not create Parse Sessions anymore as they are not needed. We are using cloud functions, which works fine with the JWT approach but one downside is that it is not easy to run the cloud function in the user context because we have no sessionToken to pass to the SDK. We could use CoreManager to set the Auth token, but it is set globally which we want to avoid in Cloud context where requests are made in multiple user contexts. If we don't reset the code properly, another request might use the token from another user...not to speak of concurrency issues in general with that approach. I proposed request interceptors as a new feature in #1449 which would tackle the same problem but now thought of a different way which would make it especially easier for cloud functions.

Feature / Enhancement Description

It is possible to pass a session token to the Parse.Query call via options:

myQuery.save(null, { sessionToken: 'abc' });

This token is added to the request as a X-Parse-Session-Token header and interpreted on server side. The idea is now to add another option, authorizationHeader to options which is then simply added as Authoriation header to the request. Parse Server would then validate this bearer token like a usual request and use the JWT functionality.

If I am not mistaken, the change would be fairly simple and only touch RestController.js + tests.

Example Use Case

myQuery.save(null, { authorization: 'Bearer abc123' });

This would allow us in Cloud functions to simply pass the auth header of the request along to the new request.

Alternatives / Workarounds

  • Use CoreManager -> Not a good idea
  • Revert to use artificial sessions for users to be able to pass a session token -> Requires a lot of additional code to manage these sessions, protect them correctly etc., as they are a potential security hole if leaked - which works against the purpose of using OAuth2 / JWT approach
  • Not use the JS SDK in cloud functions and call the REST API instead with axios or sth. else -> less comfortable solution and some features have to be implemented manually, e.g. batch requests...

3rd Party References

stephannielsen avatar Aug 15 '22 14:08 stephannielsen

Thanks for opening this issue!

  • 🎉 We are excited about your ideas for improvement!

I am willing to provide a PR for this, if this seems like a good idea to add, @mtrezza ?

stephannielsen avatar Aug 15 '22 14:08 stephannielsen

Another idea would be to add customHeaders as allowed option which can again be a map of header name to header value and the SDK simply adds all custom headers to the request.

myQuery.save(null, { customHeaders: { Authorization: 'Bearer abc123', 'X-Something': 'Whooops' } });

stephannielsen avatar Aug 15 '22 15:08 stephannielsen

How does https://github.com/parse-community/parse-server/pull/7079 play into this? I think any auth-related change should be seen in that context, because that PR will likely get merged soon.

mtrezza avatar Aug 16 '22 09:08 mtrezza

To be honest, I can't put the two into a directly related context. I am not entirely sure what https://github.com/parse-community/parse-server/pull/7079 solves in this regard. Maybe @Moumouls can weigh in on this. But given the latest comments on the PR, it also didn't look like it's going to get merged soon?

The idea described here is about adding the Authorization HTTP header to a Parse Query call. As an auth token, like a JWT Bearer token, is short lived, it changes quite frequently, potentially up to every request. The existing solution to set the Auth info via CoreManager is cumbersome to use as it sets those values globally and you have to reset them after the request to not reuse it accidentally - and potentially leak it / use it in the wrong context (cloud context). The proposed change does not require any change in parse-server - the SDK simply adds another header to this particular request. The SDK does not know why or what it is for, and it fully depends on server side implementation how to handle it. parse-server has - to my knowledge - no default handling for Authorization header so this would have no affect if user sets it but has no corresponding server side implementation.

As proposed in my other comment, I am open for implementing this in a more general fashion as customHeaders option. Then it is not directly connected to auth topics - just a way to pass custom HTTP headers to Parse Query calls.

stephannielsen avatar Aug 16 '22 11:08 stephannielsen

it is not directly connected to auth topics - just a way to pass custom HTTP headers to Parse Query calls.

So it's like https://github.com/parse-community/Parse-SDK-JS/issues/1014 but you want to set custom headers as a request (query) parameter? If so, I think we can leverage existing logic (e.g. allow-listing custom headers server side).

mtrezza avatar Aug 16 '22 11:08 mtrezza

Yes. It is like #1014 but I absolutely want to avoid using CoreManager for this in Cloud functions. The server-side is not really affected by this, accept that the header you sent must be in the list of allowed headers for CORS - but that's always the case for any custom header...

Our users authenticate via JWT token (provided by an Azure service) and Parse.User objects have no related session in our setup - as the JWT token is validated on each request. This works fine to to restrict access to cloud functions. But now, we want to run queries via the JS SDK in the cloud function within the user context, because we also want to use ACLs. Normally, you could pass the user's session token as option, which is then added as a HTTP header by the SDK for the request. But we do not have a session token. It would work fine though if I could simply add the user's JWT token (which is available in the cloud function request) to the HTTP call of the query inside the cloud function - just like the session token.

As the user context is different for each Cloud function run, we don't want to user CoreManager to set the token before running a query - because we would have to reset this every time after the query. If you forget to reset the token, the token of user A might be added to a request of user B - because it is set globally in the JS SDK. It should only be used on that one query request (or if we use batch request like findAll, on all requests involved).

Probably, I could also use the user's JWT token as session token and implement a workaround on server side with a request interceptor/middleware that takes x-parse-session-token value and adds it as authorization header - but that's just a hack to work around Parse missing functionality here.

stephannielsen avatar Aug 16 '22 12:08 stephannielsen

Got it I think it makes sense.

mtrezza avatar Aug 17 '22 14:08 mtrezza

Got it I think it makes sense.

mtrezza avatar Aug 17 '22 14:08 mtrezza

Cool, which proposal do you see more fitting? In #1529 I chose the simple way now but even supporting any custom headers wouldn't be much work. I am just not sure if it's really needed here for now.

stephannielsen avatar Aug 18 '22 09:08 stephannielsen

If a more general custom header option would make a specific custom auth header option unnecessary, we should probably go for the general option:

  • It's more universal and of greater benefit
  • If someone implements the custom header option in the future:
    • We'd have duplicate logic
    • We couldn't easily remove the auth header option because it was a breaking change
    • We'd need to create additional logic and/or it creates ambiguity if both are set

mtrezza avatar Aug 18 '22 10:08 mtrezza

Yep, I see the benefits. I will update the PR accordingly.

stephannielsen avatar Aug 22 '22 13:08 stephannielsen

Amazing, if you need any support, please request!

mtrezza avatar Aug 22 '22 17:08 mtrezza