这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@0x777
Copy link
Member

@0x777 0x777 commented Jul 5, 2019

Session variables can now be used with operators like _in, _nin, _has_keys_any and _has_keys_all. For example you now now define such a permission:

{"org_id": {"_in": "x-hasura-allowed-org-ids"}}

and then provide x-hasura-allowed-org-ids from your authn layer in this format. This helps in cases where some of the information used in your authn layer is not part of the database but is derived from an external source.

Affected components

  • Server
  • Console
  • Docs

Related Issues

#1333

Solution and Design

What we have in #1799 will not work with multiplexed subscriptions. So I had to change the implementation. The SQL generation changes from col IN (v1, v2 ..) to col = ANY (ARRAY[v1,v2 ..]::[v]).

Breaking changes

  1. The type of input values for _in and _nin operators is previously [t] (t being the type of the column). It is now changed to [t!]. Note that even previously when you have a query with say {owner_id: {_in: [1, null]}}, you would only get rows where owner_id is 1 and not rows with owner_id set as NULL. This change makes the previous behaviour explicit.
  2. Previously you can define a permission like {"org_id": {"_in": ["x-hasura-org-id1", "x-hasura-org-id2", ..]}. Such rules don't work as expected any more. The server only expects the session variable to be present only at the top level, for the above mentioned array operators.

Steps to test and verify

I'll update this once the console changes are in.

Limitations, known bugs & workarounds

The biggest limitation is that session variables are still a map of strings to strings. i.e, even arrays have to be expressed as strings as per Postgres's literal syntax for arrays ({1,2,3..}). This is purely an implementation limitation and requires a fair amount of work but will be lifted as soon as possible.

Notes to the reviewer

Previously we assumed that the type of a session variable is the type of the column. So, if you have a clause like {"user_id":{"_eq": "x-hasura-user-id"}, it implies that x-hasura-user-id should be of type, type(user_id). This changes. The type of a session variable now need not be that of column, for example, {"user_id":{"_in": "x-hasura-user-ids"}, the type of x-hasura-user-ids is type(user_id)[].

However our modelling of postgres types (PGColType) is very rudimentary, while it captures all possible types, we can't really construct a type say int[]. This has to be fixed and will be part of the PR which adds support for all possible types. However till then I introduced a new type called PgType which is either a simple type or an array type, a gross oversimplification of postgres types but will help us with this feature.

@0x777 0x777 requested a review from rikinsk-zz as a code owner July 5, 2019 07:03
@netlify
Copy link

netlify bot commented Jul 5, 2019

Deploy preview for hasura-docs ready!

Built with commit 8cca218

https://deploy-preview-2475--hasura-docs.netlify.com

@hasura-bot
Copy link
Contributor

Review app for commit d632830 deployed to Heroku: https://hge-ci-pull-2475.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2475-d6328300

@rikinsk-zz rikinsk-zz self-assigned this Jul 5, 2019
@0x777 0x777 requested a review from rakeshkky July 5, 2019 09:34
@hasura-bot
Copy link
Contributor

Review app for commit 1975293 deployed to Heroku: https://hge-ci-pull-2475.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2475-1975293f

rikinsk-zz
rikinsk-zz previously approved these changes Jul 5, 2019
@rikinsk-zz rikinsk-zz added c/console Related to console c/docs Related to docs c/server Related to server labels Jul 5, 2019
@rikinsk-zz rikinsk-zz assigned rakeshkky and unassigned rikinsk-zz Jul 5, 2019
@rikinsk-zz rikinsk-zz self-requested a review July 5, 2019 13:25
rakeshkky
rakeshkky previously approved these changes Jul 8, 2019
Copy link
Member

@rakeshkky rakeshkky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

server LGTM

@0x777 0x777 requested a review from marionschleifer as a code owner July 9, 2019 04:58
@hasura-bot
Copy link
Contributor

Review app for commit 66789d3 deployed to Heroku: https://hge-ci-pull-2475.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2475-66789d33

@rikinsk-zz rikinsk-zz dismissed stale reviews from rakeshkky and themself via 6c6ad53 July 9, 2019 10:02
Copy link
Member

@rakeshkky rakeshkky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Server LGTM

@hasura-bot
Copy link
Contributor

Review app for commit 6c6ad53 deployed to Heroku: https://hge-ci-pull-2475.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2475-6c6ad53d

@hasura-bot
Copy link
Contributor

Review app for commit 8cca218 deployed to Heroku: https://hge-ci-pull-2475.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2475-8cca2185

@0x777 0x777 merged commit f1cf6d0 into hasura:master Jul 10, 2019
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-2475.herokuapp.com is deleted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c/console Related to console c/docs Related to docs c/server Related to server s/has-breaking-changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants