data-dot-all / dataall

A modern data marketplace that makes collaboration among diverse users (like business, analysts and engineers) easier, increasing efficiency and agility in data projects on AWS.
https://data-dot-all.github.io/dataall/
Apache License 2.0
236 stars 82 forks source link

Validation of Query / Mutation variables in GraphQL APIs #871

Open TejasRGitHub opened 1 year ago

TejasRGitHub commented 1 year ago

Describe the bug

For GraphQL APIs like CreateOrganization , there is not validation before executing the graphQL query.

With a ID token, a simple curl command can be used to create organization with a team which doesn't even exists.

Creating this Github issue to list such APIs where validation is needed before executing APIs

How to Reproduce

  1. Goto Data.all UI
  2. Login into the data.all UI
  3. Get the ID Token from the cookies storage Or Local storage
  4. Use a cURL like this ,

curl 'https://.execute-api.us-east-1.amazonaws.com/prod/graphql/api' \ -H 'authority: .execute-api.us-east-1.amazonaws.com' \ -H 'accept: /' \ -H 'accept-language: en-US,en;q=0.9' \ -H 'access-control-allow-origin: ' \ -H 'accesscontrolallowheaders: ' \ -H 'accesscontrolalloworigin: *' \ -H 'accesskeyid: none' \ -H 'authorization: ID_TOKEN' \ -H 'content-type: application/json' \ -H 'origin: DOMAIN_URL' \ -H 'referer: DOMAIN_URL/' \ -H 'sec-ch-ua: "Google Chrome";v="119", "Chromium";v="119", "Not?A_Brand";v="24"' \ -H 'sec-ch-ua-mobile: ?0' \ -H 'sec-ch-ua-platform: "macOS"' \ -H 'sec-fetch-dest: empty' \ -H 'sec-fetch-mode: cors' \ -H 'sec-fetch-site: cross-site' \ -H 'secretkey: none' \ -H 'user-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/119.0.0.0 Safari/537.36' \ --data-raw '{"operationName":"CreateOrg","variables":{"input":{"label":"Temp-Orgs","description":"","SamlGroupName":"","tags":["Somtage"]}},"query":"mutation CreateOrg($input: NewOrganizationInput) {\n createOrganization(input: $input) {\n organizationUri\n label\n created\n __typename\n }\n}\n"}' \ --compressed



### Expected behavior

GraphQL APIs variables should be validated either in the resolvers or in the API Handler before executing API.

### Your project

_No response_

### Screenshots

_No response_

### OS

Mac

### Python version

3.9

### AWS data.all version

2.0

### Additional context

_No response_
dlpzx commented 1 year ago

Hi @TejasRGitHub I found why you opened this issue. In data.all v2 we established different functional layers in the backend: api, sservices, db... In the modules this separation of concerns is strictly enforced, for the core components however some refactoring tasks are still pending. Coming back to your issue, the permissions checker in apis such as createOrganization. The permissions check should happen in the services of organization, instead it happens in the db layer. It is not that it is not implemented, but that the code is in the wrong placed according to the standards that we set ourselves.

image

Let me know if this is the issue that you were describing, otherwise we will close this issue in favor of #741 that already accounts for the core refactoring

TejasRGitHub commented 1 year ago

Hi @dlpzx , So the issue I am describing is that the query parameters in a graphQL query are not validated. Please correct me if I am wrong, the permissions checker as described by from the screenshot ( @has_tenant_permission ) will only check if the user has permission to create organization but it won't check if the group / team that will be used for that organization is valid or not.