Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Different authorization for GET and UPDATE #1652

Closed
2 tasks done
biller-aivy opened this issue Jul 7, 2023 · 14 comments
Closed
2 tasks done

Different authorization for GET and UPDATE #1652

biller-aivy opened this issue Jul 7, 2023 · 14 comments

Comments

@biller-aivy
Copy link

How did you install the Amplify CLI?

npm

If applicable, what version of Node.js are you using?

18.6.0

Amplify CLI Version

12.1.1

What operating system are you using?

macOS

Did you make any manual changes to the cloud resources managed by Amplify? Please describe the changes made.

Describe the bug

When I use:

query MyQuery {
  getCareer(id: "f8489c8f-b45a-4c09-8e33-fe755f6b8346") {
    id
    career_analyse
  }
}

mutation MyMutation {
  updateCareer(input: {id: "f8489c8f-b45a-4c09-8e33-fe755f6b8346", name: "change"}) {
    authorized
    career_analyse
  }
}

With this Schema:

type Career
  @model
  @auth(
    rules: [
      { allow: groups, groups: ["__admin"] }
      { allow: private, provider: iam }
      { allow: private, operations: [read, create] }
      { allow: public, provider: iam, operations: [read] }
      { allow: groups, groupsField: "partner_id" }
      { allow: groups, groupsField: "authorized" }
    ]
  ) {
  id: ID!
  name: String
  career_analyse: AWSJSON
    @auth(
      rules: [
        { allow: groups, groups: ["__admin"] }
        { allow: groups, groupsField: "partner_id" }
        { allow: groups, groupsField: "authorized" }
      ]
    )
  createdAt: String
  updatedAt: String
}

The Response is:

{
  "data": {
    "updateCareer": {
      "id": "f8489c8f-b45a-4c09-8e33-fe755f6b8346",
      "career_analyse": null
    }
  }
}
{
  "data": {
    "getCareer": {
      "id": "f8489c8f-b45a-4c09-8e33-fe755f6b8346",
      "career_analyse": "{\"icc....
    }
  }
}

Expected behavior

The same rules should apply for an update.

Reproduction steps

Try out the Auth Rules

Project Identifier

⠹ Sending zip
DiagnoseReportUploadError
✖ Sending zip

Log output

# Put your logs below this line


Additional information

No response

Before submitting, please confirm:

  • I have done my best to include a minimal, self-contained set of instructions for consistently reproducing the issue.
  • I have removed any sensitive information from my code snippets and submission.
@AnilMaktala
Copy link
Member

Hey @biller-aivy, Thank you for bringing this to our attention. We are currently working on reproducing the issue. Could you kindly inform us about the authorization method you used for the read and update operations?

@biller-aivy
Copy link
Author

It's cognito auth users.

@biller-aivy
Copy link
Author

biller-aivy commented Jul 10, 2023

@AnilMaktala do you need more information? Or the complete Schema? For the career?

type Career
  @model
  @auth(
    rules: [
      { allow: groups, groups: ["__admin"] }
      { allow: private, provider: iam }
      { allow: private, operations: [read, create] }
      { allow: public, provider: iam, operations: [read] }
      { allow: groups, groupsField: "partner_id" }
      { allow: groups, groupsField: "authorized" }
    ]
  ) {
  id: ID!
  name: String
  is_available_in_attract: Boolean
  was_seen_times_in_attract_funnel: Int
  logo: String
  copy_from_career_id: ID
    @auth(
      rules: [
        { allow: groups, groups: ["__admin"] }
        { allow: groups, groupsField: "partner_id" }
        { allow: groups, groupsField: "authorized" }
      ]
    )
  integration_res: AWSJSON #depricated
  photo_url: String
  title: String
  status: CareerStatus
  archived: Boolean
  language: String
  history: [CAREER_HISTORY]
    @auth(
      rules: [
        { allow: groups, groups: ["__admin"] }
        { allow: groups, groupsField: "partner_id" }
        { allow: groups, groupsField: "authorized" }
      ]
    )
  category: String
  comment: [Comment]
    @auth(
      rules: [
        { allow: groups, groups: ["__admin"] }
        { allow: groups, groupsField: "partner_id" }
        { allow: groups, groupsField: "authorized" }
      ]
    )
  use_in: [USE_IN] #depricated
  partner_id: ID!
    @index(
      name: "ByPartnerByDate"
      queryField: "careerByPartnerByDate"
      sortKeyFields: ["createdAt"]
    )
  authorized: [ID]
    @auth(
      rules: [
        { allow: groups, groups: ["__admin"] }
        { allow: groups, groupsField: "partner_id" }
        { allow: groups, groupsField: "authorized" }
      ]
    )
  partner_name: String
  external_link: String
  related_career: String
  riasec_id: ID
  app_settings: AppSettings
  career_cluster_url: String
  forced_dimension: String
  challenge_config_id: ID #ref to challenge config table
  imported_from: String
  career_analyse: AWSJSON
    @auth(
      rules: [
        { allow: groups, groups: ["__admin"] }
        { allow: groups, groupsField: "partner_id" }
        { allow: groups, groupsField: "authorized" }
        # { allow: private }
        # { allow: public, provider: iam }
        # { allow: private, provider: iam }
      ]
    )
  createdAt: String
  updatedAt: String
}

@AnilMaktala
Copy link
Member

Hey @biller-aivy, We appreciate the additional information you have provided. We will make an effort to replicate the issue and will reach out to you if further details are required.

@alharris-at alharris-at added the p2 label Jul 11, 2023
@AnilMaktala
Copy link
Member

Hey @biller-aivy, By following the steps outlined in the description, we have successfully replicated the issue. Therefore, we are categorizing it as a bug for the team to conduct further evaluation.

image

@AnilMaktala AnilMaktala added bug Something isn't working and removed pending-triage labels Jul 11, 2023
@biller-aivy
Copy link
Author

@AnilMaktala Do you know in which files the bug could be? Then I could try to solve it.

@biller-aivy
Copy link
Author

any news here? its important because we can't open our system for all users to get the data.

@biller-aivy
Copy link
Author

Any news here @sundersc @AnilMaktala

I could help here if needed.

@sundersc
Copy link
Contributor

sundersc commented Jun 3, 2024

Hey @biller-aivy, The field career_analyse is intentionally set to null to protect the data from subscribed clients. The way subscriptions work is they listen to mutations and directly forward the results of a mutation to the subscribed clients using different authorization modes. The individual messages are not validated against the auth rules.

The auth rules on the field career_analyse is restrictive than what is defined in the model-level. With the provided schema, if a client is connecting with iam::public rule, we shouldn't allow this user to view the field career_analyse. Currently, it is not possible to redact a field in AppSync for subscriptions based on authorization mode. That's why we over-redact the restricted fields.

If you are not using subscriptions, you turn off the subscriptions in the @model(subscriptions: null) directive. Then the mutation response will display the restrictive field.

type Career
  @model
  @auth(
    rules: [
      { allow: groups, groups: ["__admin"] }
      { allow: private, provider: iam }
      { allow: private, operations: [read, create] }
      { allow: public, provider: iam, operations: [read] }
      { allow: groups, groupsField: "partner_id" }
      { allow: groups, groupsField: "authorized" }
    ]
  ) {
  id: ID!
  name: String
  career_analyse: AWSJSON
    @auth(
      rules: [
        { allow: groups, groups: ["__admin"] }
        { allow: groups, groupsField: "partner_id" }
        { allow: groups, groupsField: "authorized" }
      ]
    )
  createdAt: String
  updatedAt: String
}

The behavior is called out in the docs.
https://docs.amplify.aws/gen1/react/build-a-backend/graphqlapi/customize-authorization-rules/#field-level-authorization-rules

@sundersc sundersc added pending-response and removed bug Something isn't working p2 labels Jun 3, 2024
@biller-aivy
Copy link
Author

Hey @biller-aivy, The field career_analyse is intentionally set to null to protect the data from subscribed clients. The way subscriptions work is they listen to mutations and directly forward the results of a mutation to the subscribed clients using different authorization modes. The individual messages are not validated against the auth rules.

The auth rules on the field career_analyse is restrictive than what is defined in the model-level. With the provided schema, if a client is connecting with iam::public rule, we shouldn't allow this user to view the field career_analyse. Currently, it is not possible to redact a field in AppSync for subscriptions based on authorization mode. That's why we over-redact the restricted fields.

If you are not using subscriptions, you turn off the subscriptions in the @model(subscriptions: null) directive. Then the mutation response will display the restrictive field.

type Career
  @model
  @auth(
    rules: [
      { allow: groups, groups: ["__admin"] }
      { allow: private, provider: iam }
      { allow: private, operations: [read, create] }
      { allow: public, provider: iam, operations: [read] }
      { allow: groups, groupsField: "partner_id" }
      { allow: groups, groupsField: "authorized" }
    ]
  ) {
  id: ID!
  name: String
  career_analyse: AWSJSON
    @auth(
      rules: [
        { allow: groups, groups: ["__admin"] }
        { allow: groups, groupsField: "partner_id" }
        { allow: groups, groupsField: "authorized" }
      ]
    )
  createdAt: String
  updatedAt: String
}

The behavior is called out in the docs. https://docs.amplify.aws/gen1/react/build-a-backend/graphqlapi/customize-authorization-rules/#field-level-authorization-rules

But I don't use subscription. The diff comes from a simple update call?!

@sundersc
Copy link
Contributor

sundersc commented Jun 4, 2024

Though you are not using subscriptions, it is enabled on the model. You can disable subscriptions using one of the below options.

type Career
  @model(subscriptions: null) ## or subscriptions: { level: off }
  @auth(
    rules: [
       ...
       ...
    ]
  ) {
       ...
       ...
}

@biller-aivy
Copy link
Author

Though you are not using subscriptions, it is enabled on the model. You can disable subscriptions using one of the below options.

type Career

  @model(subscriptions: null) ## or subscriptions: { level: off }

  @auth(

    rules: [

       ...

       ...

    ]

  ) {

       ...

       ...

}

Mhhh, and than the res of the update mutation is the same like the get?

@dpilch
Copy link
Member

dpilch commented Jun 6, 2024

Correct, if you disable subscriptions for this model the career_analyse will no longer be redacted in the update mutation result.

Copy link

github-actions bot commented Jul 1, 2024

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

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

No branches or pull requests

6 participants