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

Pass Bearer token as query request option #1528

Open
3 tasks done
stephannielsen opened this issue Aug 15, 2022 · 13 comments
Open
3 tasks done

Pass Bearer token as query request option #1528

stephannielsen opened this issue Aug 15, 2022 · 13 comments
Labels
type:feature New feature or improvement of existing feature

Comments

@stephannielsen
Copy link

stephannielsen commented Aug 15, 2022

New Feature / Enhancement Checklist

Current Limitation

We are using JWT based authentication (parse-community/parse-server#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

@parse-github-assistant
Copy link

parse-github-assistant bot commented Aug 15, 2022

Thanks for opening this issue!

  • 🎉 We are excited about your ideas for improvement!

@stephannielsen
Copy link
Author

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

@stephannielsen
Copy link
Author

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' } });

@mtrezza mtrezza added the type:feature New feature or improvement of existing feature label Aug 16, 2022
@mtrezza
Copy link
Member

mtrezza commented Aug 16, 2022

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

@stephannielsen
Copy link
Author

stephannielsen commented Aug 16, 2022

To be honest, I can't put the two into a directly related context. I am not entirely sure what parse-community/parse-server#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.

@mtrezza
Copy link
Member

mtrezza commented Aug 16, 2022

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

So it's like #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).

@stephannielsen
Copy link
Author

stephannielsen commented Aug 16, 2022

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.

@mtrezza
Copy link
Member

mtrezza commented Aug 17, 2022

Got it I think it makes sense.

1 similar comment
@mtrezza
Copy link
Member

mtrezza commented Aug 17, 2022

Got it I think it makes sense.

@stephannielsen
Copy link
Author

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.

@mtrezza
Copy link
Member

mtrezza commented Aug 18, 2022

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

@stephannielsen
Copy link
Author

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

@mtrezza
Copy link
Member

mtrezza commented Aug 22, 2022

Amazing, if you need any support, please request!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or improvement of existing feature
Projects
None yet
Development

No branches or pull requests

2 participants