hasura / graphql-engine

Blazing fast, instant realtime GraphQL APIs on your DB with fine grained access control, also trigger webhooks on database events.
https://hasura.io
Apache License 2.0
31.21k stars 2.78k forks source link

Unnecessary headers expected #3659

Closed webdeb closed 4 years ago

webdeb commented 4 years ago

Imagine email-subscriptions permissions for an anonymous-role are set to this:

(simplified)

  insert: {}
  update: { secret: "x-hasura-secret"} (got-from-email)

By requiring x-hasura-secret in update, I get an error, when trying to insert, which should't require the x-hasura-secret header.

"x-hasura-secret" header is expected but not found

Is this a bug, or a feature? Thanks

webdeb commented 4 years ago

The funny thing is, that if I pass "something" the error disappears, but stills requires to put there even an empty string or so.

Btw. I found it very unpleasant to read the code, not because its haskell, which I do not have any experience with, but just because the naming conventions are so cryptic. I feel, that my brain needs a lot of energy to first understand the variables acronyms, before it can eventually dive into the logic

0x777 commented 4 years ago

The update permission's filter condition has to be applied during an insert when on_conflict is used, something along the lines of ON CONFLICT ... SET ... WHEN <update-condition>.

Currently the server blindly tries to check for the existence of session variables in both insert's check expression and update's filter condition during an insert operation. This is a bug, the server should do the validation based on the structure of the query (i.e, update permission's session variables should only be checked for if on_conflict is used). I have a work in progress PR to fix this problem but it depends on https://github.com/hasura/graphql-engine/pull/3598.

Related to #14

aaronhayes commented 4 years ago

Seems like this is the same issue I'm having, so just wanted to record my use-case without creating a new issue...

When using column presets from session variables, each header must be set. Even if the underlying field is nullable.

image

image

{
  "errors": [
    {
      "extensions": {
        "path": "$",
        "code": "not-found"
      },
      "message": "\"x-hasura-organisation-id\" header is expected but not found"
    }
  ]
}
webdeb commented 4 years ago

@0x777 any news on this?

Right now I got an even more strange error, as I tried to use Authorization + x-hasura-custom-header it even did't recognized the header I've send. I guess the authorization overrides all existing headers.

webdeb commented 4 years ago

I was expecting, that hasura will take the default authorization-required headers like user-id and roles from the JWT, but also allow other headers from the request to be additionally provided.

IMHO hasura should take all x-hasura- headers from JWT and assign all other "x-hasura-" headers excluding the ones, which are already passed through the JWT, like its already possible with the x-hasura-role header

tirumaraiselvan commented 4 years ago

Released in v1.3.4-beta.1

IsaacLeeWebDev commented 3 years ago

I guess the authorization overrides all existing headers.

This really helped me realize that I needed to something kinda like this in my AuthN server in order to allow x-hasura-* headers from getting removed before Hasura AuthZ-time:

/** express and TypeScript below */

// collect all headers that start with "x-hasura-" into an object called `customHasuraHeaders`
const x_hasura_ = 'x-hasura-';
const customHasuraHeaders = Object.entries(req.headers).reduce((acc, [key, val]) => {
if (key.substring(0, x_hasura_.length).toLowerCase() === x_hasura_) {
  acc[key] = val;
}
return acc;
}, {} as typeof req.headers);

// spread that object into the response before adding other headers controlled by the AuthN server
return res.status(200).json({
  ...customHasuraHeaders,
  'X-Hasura-Role': 'public',
});