check input empty strings for metadata api (close #2302) #2300

ajeetdsouza · 2019-05-29T12:53:15Z

Description

The current metadata API allows a user to pass empty strings for things like role names, collection names, query names, etc, which should never be empty. This PR adds checks for these cases - any API calls made with empty strings as values for these types will now fail with a comprehensive error message.

Affected components

Server
Tests

Related Issues

close #2302

Solution and Design

NEText wraps regular text, and uses a smart constructor mkNEText :: Text -> Maybe NEText that fails when provided an empty value. The following type aliases and newtypes used to contain standard Text, and have been changed to contain NEText:

RelName
TQueryName
TriggerName
RoleName
CollectionName
QueryName
RemoteSchemaName

Steps to test and verify

A new PyTest class (TestNEText) has been added to test_v1_queries.py, which test empty text input failure for the following types using the API:

RelName
TriggerName
RoleName
CollectionName
QueryName
RemoteSchemaName

Limitations, known bugs & workarounds

None

To the Reviewer (Server)

No new modules have been added.
A newtype NEText has been created for non-empty Text types. A smart constructor mkNEText :: Text -> Maybe NEText has been created for this.
The following newtypes used to wrap Text, and now wrap NEText: RelName, RoleName, CollectionName, QueryName, RemoteSchemaName
The following types were aliased to Text, and are now newtypes wrapping NEText: TriggerName
The following convenience functions have been added to extract Text from newtypes: relNameToTxt, triggerNameToTxt, roleNameToTxt
The following NEText constants have been created: adminText, rootText
Tests have been added in the TestNEText class

Critical files

src-lib/Hasura/GraphQL/Resolve/Insert.hs
src-lib/Hasura/RQL/Types/Common.hs
src-lib/Hasura/RQL/Types/EventTrigger.hs
src-lib/Hasura/RQL/Types/Permission.hs
src-lib/Hasura/RQL/Types/QueryCollection.hs
src-lib/Hasura/RQL/Types/RemoteSchema.hs
src-lib/Hasura/Server/App.hs
src-lib/Hasura/Server/Auth/JWT.hs
src-lib/Hasura/Server/Init.hs
tests-py/test_v1_queries.py (TestNEText and the corresponding tests)

hasura-bot · 2019-05-29T12:53:18Z

Beep boop! 🤖

Hey @ajeetdsouza, thanks for your PR!

One of my human friends will review this PR and get back to you as soon as possible. 🕐

Stay awesome! 😎

netlify · 2019-05-29T12:54:25Z

Deploy preview for hasura-docs ready!

Built with commit 7816d9d

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

hasura-bot · 2019-05-29T13:01:56Z

Review app for commit 32dc7e2 deployed to Heroku: https://hge-ci-pull-2300.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2300-32dc7e21

hasura-bot · 2019-05-30T06:32:58Z

Review app for commit d25479a deployed to Heroku: https://hge-ci-pull-2300.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2300-d25479a0

rakeshkky · 2019-05-30T08:06:08Z

Identify similar patterns for unNEText and reduce them

server/src-lib/Hasura/RQL/Types.hs

hasura-bot · 2019-06-03T06:22:19Z

Review app for commit 5eb4a54 deployed to Heroku: https://hge-ci-pull-2300.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2300-5eb4a546

server/src-lib/Hasura/RQL/Types/Common.hs

server/src-lib/Hasura/RQL/Types/Permission.hs

…e into newtype-netext

hasura-bot · 2019-06-03T07:03:52Z

Review app for commit 72a578b deployed to Heroku: https://hge-ci-pull-2300.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2300-72a578b5

server/src-lib/Hasura/RQL/DDL/Permission.hs

server/src-lib/Hasura/RQL/Types/EventTrigger.hs

hasura-bot · 2019-06-06T13:53:40Z

Review app for commit 9b239c8 deployed to Heroku: https://hge-ci-pull-2300.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2300-9b239c8f

hasura-bot · 2019-06-19T10:51:51Z

Review app for commit a8a9033 deployed to Heroku: https://hge-ci-pull-2300.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2300-a8a90339

server/src-lib/Hasura/RQL/Types/Permission.hs

hasura-bot · 2019-06-21T08:39:11Z

Review app for commit fa166e8 deployed to Heroku: https://hge-ci-pull-2300.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2300-fa166e8e

hasura-bot · 2019-07-02T12:30:09Z

Review app for commit 875fe13 deployed to Heroku: https://hge-ci-pull-2300.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2300-875fe13d

hasura-bot · 2019-07-08T12:50:18Z

Review app for commit be236b3 deployed to Heroku: https://hge-ci-pull-2300.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2300-be236b37

rakeshkky

server LGTM

hasura-bot · 2019-07-11T08:33:27Z

Review app for commit 7816d9d deployed to Heroku: https://hge-ci-pull-2300.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2300-7816d9d1

shahidhk

LGTM

hasura-bot · 2019-07-11T09:00:48Z

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

hasura-bot · 2019-07-11T09:00:49Z

Beep boop! 🤖

Awesome work @ajeetdsouza! All of us at Hasura ❤️ what you did.

Thanks again 🤗

…asura#2300)

ajeetdsouza added 3 commits May 29, 2019 14:05

add NEText newtype

a86f5f4

newtype TriggerName as NEText

e4a1db0

add tests for NEText

32dc7e2

ajeetdsouza requested a review from rakeshkky May 29, 2019 12:53

ajeetdsouza requested a review from 0x777 as a code owner May 29, 2019 12:53

Fix FromJSONKey instance of NEText

d25479a

rakeshkky changed the title ~~Create newtype for non-empty text~~ check input empty strings for metadata api (close #2302) May 30, 2019

ajeetdsouza and others added 2 commits May 30, 2019 14:56

Reduce code duplication for unboxing NEText

7d8801f

Merge branch 'master' into newtype-netext

5eb4a54

rakeshkky reviewed Jun 3, 2019

View reviewed changes

server/src-lib/Hasura/RQL/Types.hs Outdated Show resolved Hide resolved

rakeshkky reviewed Jun 3, 2019

View reviewed changes

server/src-lib/Hasura/RQL/Types/Common.hs Outdated Show resolved Hide resolved

rakeshkky reviewed Jun 3, 2019

View reviewed changes

server/src-lib/Hasura/RQL/Types/Permission.hs Outdated Show resolved Hide resolved

ajeetdsouza added 2 commits June 3, 2019 12:25

Reduce code duplication for unboxing NEText #2

6582309

Merge branch 'newtype-netext' of github.com:ajeetdsouza/graphql-engin…

72a578b

…e into newtype-netext

ajeetdsouza self-assigned this Jun 3, 2019

rakeshkky reviewed Jun 6, 2019

View reviewed changes

server/src-lib/Hasura/RQL/DDL/Permission.hs Outdated Show resolved Hide resolved

rakeshkky reviewed Jun 6, 2019

View reviewed changes

server/src-lib/Hasura/RQL/Types/EventTrigger.hs Outdated Show resolved Hide resolved

Reduce code duplication for unboxing NEText #3

9b239c8

shahidhk added the c/server Related to server label Jun 10, 2019

Merge branch 'master' into newtype-netext

a8a9033

ajeetdsouza requested a review from rikinsk-zz as a code owner June 19, 2019 10:30

rakeshkky reviewed Jun 21, 2019

View reviewed changes

server/src-lib/Hasura/RQL/Types/Permission.hs Show resolved Hide resolved

Add comments

fa166e8

Merge branch 'master' into newtype-netext

875fe13

dsandip added the k/bug Something isn't working label Jul 2, 2019

rakeshkky previously approved these changes Jul 2, 2019

View reviewed changes

ajeetdsouza added 2 commits July 8, 2019 17:44

resolve merge conflict

d3bfd34

Rename NEText to NonEmptyText

be236b3

ajeetdsouza dismissed rakeshkky’s stale review via be236b3 July 8, 2019 12:42

rakeshkky previously approved these changes Jul 11, 2019

View reviewed changes

resolve merge conflict

7816d9d

ajeetdsouza dismissed rakeshkky’s stale review via 7816d9d July 11, 2019 08:02

shahidhk approved these changes Jul 11, 2019

View reviewed changes

shahidhk merged commit 92c4cff into hasura:master Jul 11, 2019

rakeshkky mentioned this pull request Jul 12, 2019

Failed to build schema-cache because of inconsistent metadata: postgres query error #2521

Closed

polRk pushed a commit to polRk/graphql-engine that referenced this pull request Feb 12, 2020

check input for empty strings for metadata api (close hasura#2302) (h…

e531f26

…asura#2300)

check input empty strings for metadata api (close #2302) #2300

check input empty strings for metadata api (close #2302) #2300

Uh oh!

Conversation

ajeetdsouza commented May 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Affected components

Related Issues

Solution and Design

Steps to test and verify

Limitations, known bugs & workarounds

To the Reviewer (Server)

Uh oh!

hasura-bot commented May 29, 2019

Uh oh!

netlify bot commented May 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hasura-bot commented May 29, 2019

Uh oh!

hasura-bot commented May 30, 2019

Uh oh!

rakeshkky commented May 30, 2019

Uh oh!

Uh oh!

hasura-bot commented Jun 3, 2019

Uh oh!

Uh oh!

Uh oh!

hasura-bot commented Jun 3, 2019

Uh oh!

Uh oh!

Uh oh!

hasura-bot commented Jun 6, 2019

Uh oh!

hasura-bot commented Jun 19, 2019

Uh oh!

Uh oh!

hasura-bot commented Jun 21, 2019

Uh oh!

hasura-bot commented Jul 2, 2019

Uh oh!

hasura-bot commented Jul 8, 2019

Uh oh!

rakeshkky left a comment

Choose a reason for hiding this comment

Uh oh!

hasura-bot commented Jul 11, 2019

Uh oh!

shahidhk left a comment

Choose a reason for hiding this comment

Uh oh!

hasura-bot commented Jul 11, 2019

Uh oh!

hasura-bot commented Jul 11, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ajeetdsouza commented May 29, 2019 •

edited

Loading

netlify bot commented May 29, 2019 •

edited

Loading