Improve security for SSS webtasks

marktani commented 6 years ago

Issue by schickling Friday Sep 08, 2017 at 09:28 GMT Originally opened as https://github.com/graphcool/prisma/issues/394

Issue by kbrandwijk Sunday Jun 25, 2017 at 21:23 GMT Originally opened as https://github.com/graphcool/api-bugs/issues/149

What is the current behavior? Although it should not be possible for an end user to get hold of the actual URL of a SSS, I would at least expect that it needs some Authorization token to execute. This applies to inline SSS functions.

What is the expected behavior? SSS cannot execute without a proper Authorization header (PAT?).

marktani commented 6 years ago

Comment by schickling Friday Sep 08, 2017 at 09:28 GMT

Comment by sorenbs Monday Jun 26, 2017 at 11:45 GMT

Thank you for raising this issue Kim.

Currently the security of inline functions is based on a secret in the url. We have some planned changes to our infrastructure that will enable us to disable public access to the endpoint of inline functions.

Inline functions are fully managed by Graphcool the developer should not be required to think about how to secure their functions.

If you use something like the Serverless framework to host your functions we encourage you to use headers to secure the endpoint. We will introduce a way to securely manage environment variables on Graphcool, which should make this much easier.

marktani commented 6 years ago

Comment by schickling Friday Sep 08, 2017 at 09:28 GMT

Comment by kbrandwijk Monday Jun 26, 2017 at 11:51 GMT

@sorenbs Thank you for your explanation. It touches on a few good points.

However, you mention that when using 'external' webhooks, you encourage the use of headers. Would that mean manually specifying a Authorization header in the definition of the RP or SSS, or would a Authorization header be passed automatically? In the first case, I have no way to identify my user from an RP or SSS webhook, in the second case, how can I validate that token?

marktani commented 6 years ago

Comment by schickling Friday Sep 08, 2017 at 09:28 GMT

Comment by sorenbs Monday Jun 26, 2017 at 12:23 GMT

To be super clear - a user token DOES NOT grant access to the system API. Please reach out to me or Nilan in slack so we can investigate what is happening for you.

Permanent Access Tokens currently do grant access to the system API. We will be adding more granular control to the PAT concept in the future.

To your previous point. There are two aspects to consider:

Ensuring that only the Graphcool infrastructure can call your functions. For inline functions we handle this concern and for webhooks you can use headers.
Access the execution context for the function in your code to for example perform different actions depending on data on the User node. We will extend the function event to include more context like this.

marktani commented 6 years ago

Comment by schickling Friday Sep 08, 2017 at 09:29 GMT

Comment by kbrandwijk Monday Jun 26, 2017 at 12:31 GMT

@sorenbs I deleted the comment about the user token, I accidentally checked the wrong Authorization header while testing. So I agree that is not the case! I apologize for jumping to that conclusion.

Regarding your points:

To be super clear, for inline functions you currently do not handle that concern. All inline functions for RP and SSS are publicly accessible. Also, they do not run under the context of the authenticated user.
Without Graphcool passing any Authorization headers to my functions, it is currently impossible to check for authorization in a webhook. I can ensure that only Graphcool can call my webhooks however by some arbitrary header.

As you see, both options are seriously flawed.

marktani commented 6 years ago

Comment by schickling Friday Sep 08, 2017 at 09:29 GMT

Comment by sorenbs Monday Jun 26, 2017 at 12:42 GMT

Thanks

Currently the security relies on a secret in the url. In the future we will block public access to all inline functions.
The syntax is not final, but our plan is to augment the function event like this:

currently a function receives:

{
  data: { some: "data" }
}

in the future a function will receive something like this:

{
  data: { some: "data" }
  context: {
    request: RequestContext
    graphcool: GraphcoolContext
    environment: Json
    auth: AuthContext
    sessionCache: Json
  }
}

type RequestContext {
  sourceIp: String
  headers: Json
  httpMethod: HttpMethod
}

type GraphcoolContext {
  projectId: String
  alias: String
}

type AuthContext {
  nodeId: String
  type: AvailableTypes
  token: String
}

I would love your take on 2.

marktani commented 6 years ago

Comment by schickling Friday Sep 08, 2017 at 09:29 GMT

Comment by kbrandwijk Monday Jun 26, 2017 at 13:07 GMT

By the way, although a user cannot access the system API using his own token, it is accessible using any PAT you define on your project. I still believe that shouldn't be the case. Because with a PAT, your now not only giving away full access to the frontend, but also to the backend of your app, and that is not right...

About AuthContext: ~~- Add a checkbox to RP and SSS in the Console 'Run in caller context'~~ - If so, pass the original (user/PAT) Authorization token to the function or webhook, and add a more convenient method to the API to validate a token (other that calling the user query and hoping for the best). ~~- If the box is not checked, pass a one-time PAT-like token (with expiry), and make sure that token can be validated from the webhook against the API.~~ ~~- The AuthContext would (only) be useful if you don't run in user context, to get information about the user the request originated from.~~

Add a dropdown to RP and SSS with 'Pass caller context', 'Pass elevated context', 'Do not pass context'
Use the Authorization Header in the request to provide a token that the webhook can verify (see the Github webhook secret link below)
Use a X-Graphcool-Admin-Token header to pass a PAT, only when 'Run in elevated context' is selected
Use a X-Graphcool-Caller-Token header to pass the token of the caller user, except when 'Do not pass context is selected'
Do not put Authorization data in the body
What are the nodeId and type fields in AuthContext?
Allow the developer to reset the PAT that was being passed to the webhook as elevated context. This is a must, because as soon as you pass it outside the system, you need a way to revoke it!

About GraphcoolContext:

Why would you need the projectId and/or alias? The security context should be linked to the project, is there a use case where you actually need the projectId? Also, the ProjectId is already in the Authorization token...

About RequestContext:

I think headers should be passed as headers, not in the body
The HttpMethod is used to call the webhook in the first place, so that's already accessible without passing it
Would sourceIp be the IP of the client that triggered the RP or SSS webhook? If so, use the `X-Forwarded-For' header, it's the standard way to pass a client IP through any middle layer (for example a proxy).

About environment and SessionCache: What would those be? I guess environment would be variables you want to pass to the webhook? Maybe it's better to allow the developer to specify how to pass the data somehow. Use query parameters, how to format the body data? This would help connecting to external webhooks directly, where you have to implement their API format for calling the webhook and sending the data.

--- Updated with new thoughts about Authorization --- Added requirement to reset PAT

marktani commented 6 years ago

Comment by schickling Friday Sep 08, 2017 at 09:29 GMT

Comment by kbrandwijk Monday Jun 26, 2017 at 13:41 GMT

Addition: found this for how Github deals with authenticating itself against user defined webhooks (using a JWT token based on a predefined secret and a checksum). Sounds great, use this :) https://developer.github.com/webhooks/securing/

Graphcool / graphcool-framework

Improve security for SSS webtasks #157