aws-amplify / amplify-category-api

The AWS Amplify CLI is a toolchain for simplifying serverless web and mobile development. This plugin provides functionality for the API category, allowing for the creation and management of GraphQL and REST based backends for your amplify project.
https://docs.amplify.aws/
Apache License 2.0
87 stars 73 forks source link

RFC - @auth directive improvements #433

Open mikeparisstuff opened 5 years ago

mikeparisstuff commented 5 years ago

RFC - @auth directive improvements

This document will outline designs for a number of new features relating to authentication & authorization within the GraphQL Transform. The goal of these features is too fill holes and introduce new mechanisms that make protecting your valuable information easier.

Proposal 1: Replace 'queries' and 'mutations' arguments with 'operations'

Merged by aws-amplify/amplify-cli#1262

Currently an @auth directive like this:

type Task @model @auth(rules: [{allow: owner}]) {
    id: ID!
    title: String
    owner: String
}

causes these changes to the following resolvers:

  1. Query.getTask - Returns the post only if the logged in user is the post's owner.
  2. Query.listTasks - Filter items such that only owned posts are returned.
  3. Mutation.createTask - If an owner is provided via $ctx.args.input.owner and matches the identity of the logged in user, succeed. If no owner is provided, set logged in user as the owner, else fail.
  4. Mutation.updateTask - Append a conditional expression that will only update the record if the logged in user is its owner.
  5. Mutation.deleteTask - Append a conditional expression that will only delete the record if the logged in user is its owner.

In other words, the @auth directive currently protects the root level query & mutation fields that are generated for an @model type.

Problem: The 'queries' and 'mutations' arguments imply top level protection

GraphQL APIs are a graph and we need to be able to define access rules on any field, not just the top level fields.

Solution

I suggest replacing the queries and mutations arguments on the @auth directive with a single operations argument. This would be the new @auth directive definition.

directive @auth(rules: [AuthRule!]!) on OBJECT
input AuthRule {
    allow: AuthStrategy!
    ownerField: String # defaults to "owner"
    identityField: String # defaults to "cognito:username"
    groupsField: String
    groups: [String]

    # The new argument
    operations: [ModelOperation]

    # Old arguments
    queries: [ModelQuery] @deprecated(reason: "The 'queries' argument will be deprecated in the future. Please replace this argument with the 'operations' argument.")
    mutations: [ModelMutation] @deprecated(reason: "The 'mutations' argument will be deprecated in the future. Please replace this argument with the 'mutations' argument.")
}
enum AuthStrategy { owner groups }

# The new enum
enum ModelOperation { create update delete read }

# The old enums
enum ModelQuery { get list }
enum ModelMutation { create update delete }

This change generalizes the config such that it implies all read operations on that model will be protected. Not just the top level 'get' & 'list' queries. Auth rules that use the 'read' operation will be applied to top level query fields, @connection resolvers, top level fields that query custom indexes, and subscription fields. Auth rules that use the 'create', 'update', and 'delete' operation will be applied to createX, updateX, and deleteX mutations respectively. Those using queries & mutations will have the same behavior and those using operations will get the new behavior. The queries & mutations arguments will eventually be removed in a future major release.

Protect @connections by default

Merged by aws-amplify/amplify-cli#1262

Once the change from queries/mutations -> operations has been implemented, we will want to go back and implement any missing authorization logic in @connection fields by default.

For example, given this schema:

type Post @model @auth(rules: [{allow: owner}], operations: [create, update, delete, read]) {
    id: ID!
    title: String
    owner: String
}
type Blog @model {
    id: ID!
    title: String
    # This connection references type Post which has auth rules and thus should be authorized.
    posts: [Post] @connection
}

The new code would add authorization logic to the Blog.posts resolver such that only owner's of the post would be able to see the posts for a given blog. It is important to note that the new logic will restrict access such that you cannot see records that you are not supposed to see, but it will not change any index structures under the hood. You will be able to use @connection with the new custom index features to optimize the access pattern and then use @auth to protect access within that table or index.

Proposal 2: Implement @auth on @searchable search fields

Github Issues

Problem

Currently Query.searchX resolvers generated by @searchable are not protected by @auth rules.

Solution

The Elasticsearch DSL is very powerful and will allow us inject Elasticsearch query terms and implement authorization checks within Elasticsearch. This work will need to handle static & dynamic ownership and group based authorization rules. Any auth rule that includes the 'read' operation will protect the Query.searchX field.

Proposal 3: Make @auth protect subscription fields

Problem: @auth does not protect subscription fields.

type Post @model @auth(rules: [{allow: owner}]) {
    id: ID!
    title: String
    owner: String
}

Currently subscriptions are not protected automatically.

Solution

AppSync subscription queries are authorized at connect time. That means that we need to parameterize the subscription queries in such a way that any relevant authorization logic is included in the subscription query itself. In the case of ownership @auth, this means that the client must pass an owner as a query argument and the subscription resolver should verify that the logged in user and owner are the same.

For example, given this schema:

type Post @model @auth(rules: [{allow: owner}]) {
    id: ID!
    title: String
    owner: String
}

The following subscription fields would be output:

type Subscription {
    onCreatePost(owner: String): Post
    onUpdatePost(owner: String): Post
    onDeletePost(owner: String): Post
}

and when running a subscription query, the client must provide a value for the owner:

subscription OnUpdatePost($owner: String) {
    onUpdatePost(owner: $owner) {
        id
        title
    }
}

The proposed change would create a new subscription resolver for each subscription field generated by the @model. Each subscription resolver would verify the provided owner matches the logged-in identity and would fail the subscription otherwise.

There are a few limitation to this approach:

  1. There is a limit of 5 arguments per subscription field.
    • e.g. a field onCreatePost(owner: String, groups: String, otherOther: String, anotherOwner: String, anotherListOfGroups: String): Post has too many fields and is invalid. To handle this the CLI can emit a warning prompting you to customize your subscription field in the schema itself.
  2. Subscription fields are equality checked against published objects. This means that subscribing to objects with with multi-owner or multi-group auth might behave slightly differently than expected.
    • When you subscribe you will need to pass the full list of owners/groups on the item. Not just the calling identity.

As an example to point (2) above, imagine this auth rule:

type Post @model @auth(rules: [{allow: owner, ownerField: "members"}]) {
    id: ID!
    title: String
    members: [String]
}

Let's say that we want to subscribe to all new posts where I am a member.

subscription {
    onCreatePost(members: ["my-user-id"]) {
        id
        title
        members
    }
}

AppSync messages are published to subscriptions when the result of the mutation, to which the subscription field is subscribed, contains fields that equal the values provided by the subscription arguments. That means that if I were to publish a message via a mutation,

mutation {
    createPost(input: { title: "New Article", members: ["my-user-id", "my-friends-user-id"]}) {
        id
        title
        members
    }
}

the subscription started before would not be triggered because ["my-user-id", "my-friends-user-id"] is not the same as ["my-user-id"]. I bring this up for clarity but I still think this feature is useful. Single owner & group based authorization will behave as expected.

Proposal 4: Field level @auth

Merged by aws-amplify/amplify-cli#1262

Currently an @auth directive like this:

type Task @model @auth(rules: [{allow: owner}], queries: [get, list], mutations: [create, update, delete]) {
    id: ID!
    title: String
    owner: String
}

causes these changes to the following resolvers:

  1. Query.getTask - Returns the post only if the logged in user is the post's owner.
  2. Query.listTasks - Filter items such that only owned posts are returned.
  3. Mutation.createTask - If an owner is provided via $ctx.args.input.owner and matches the identity of the logged in user, succeed. If no owner is provided, set logged in user as the owner, else fail.
  4. Mutation.updateTask - Append a conditional expression that will only update the record if the logged in user is its owner.
  5. Mutation.deleteTask - Append a conditional expression that will only delete the record if the logged in user is its owner.

In other words, the @auth directive currently protects the root level query & mutation fields.

Github Issues

Problem: You cannot protect @connection resolvers

For example, look at this schema.

type Task @model {
    id: ID!
    title: String
    owner: String
    notes: [Task] @connection(name: "TaskNotes")
}
# We are trying to specify that notes should only be visible by the owner but
# we are unintentially opening access via *Task.notes*.
type Notes @model @auth(rules: [{allow: owner}]) {
    id: ID!
    title: String
    task: Task @connection(name: "TaskNotes")
    owner: String
}

Since only top level fields are protected and we do not have an @auth directive on the Task model, we are unintentionally opening access to posts via Task.notes.

Solution

We discussed having @auth rules on OBJECTs automatically protect connection fields in proposal 1, but I also suggest opening the @auth directive such that it can be placed on both FIELD_DEFINITION and OBJECT nodes. This will result in an updated definition for @auth:

directive @auth(rules: [AuthRule!]!) on OBJECT, FIELD_DEFINITION
# ...

You may then use the @auth directive on individual fields in addition to the object type definition. An @auth directive used on an @model OBJECT will augment top level queries & mutations while an @auth directive used on a FIELD_DEFINITION will protect that field's resolver by comparing the identity to the source object designated via $ctx.source.

For example, you might have:

type User @model {
    id: ID!
    username: String

    # Can be used to protect @connection fields.
    # This resolver will compare the $ctx.identity to the "username" attribute on the User object (via $ctx.source in the User.posts resolver).
    # In other words, we are authorizing access to posts based on information in the user object.
    posts: [Post] @connection(name: "UserPosts") @auth(rules: [{ allow: owner, ownerField: "username" }])

    # Can also be used to protect other fields
    ssn: String @auth(rules: [{ allow: owner, ownerField: "username" }])
}
# Users may create, update, delete, get, & list at the top level if they are the
# owner of this post itself.
type Post @model @auth(rules: [{ allow: owner }]) {
    id: ID!
    title: String
    author: User @connection(name: "UserPosts")
    owner: String
}

An important thing to notice is that the @auth directives compares the logged-in identity to the object exposed by $ctx.source in the resolver of that field. A side effect of (1) is that an @auth directive on a field in the top level query type doesn't have much meaning since $ctx.source will be an empty object. This is ok since @auth rules on OBJECT types handle protecting top level query/mutation fields.

Also note that the queries and mutations arguments on the @auth directive are invalid but the operations argument is allowed. The transform will validate this and fail at compile time with an error message pointing you to the mistake. E.G. this is invalid:

type User @model {
    # @auth on FIELD_DEFINITION is always protecting a field that reads data.
    # Fails with error "@auth directive used on field User.ssn cannot specify arguments 'mutations' and 'queries'"
    ssn: String @auth(rules: [{allow: owner}, mutations: [create], queries: [get]]) 

    # No one but the owner may update/delete/read their own email.
    email: String @auth(rules: [{allow: owner}, operations: [update, delete, read]) 
}

The implementation for allowing operations in field level @auth directives is a little different.

  1. create - When placed on a @model type, will verify that only the owner may pass the field in the input arg. When used on a non @model type, this does nothing.
  2. update - When placed on a @model type, will verify that only the owner may pass the field in the input arg. When used on a non @model type, this does nothing.
  3. delete - When placed on a @model type, will verify that only the owner may set the value to null. Only object level @auth directives impact delete operations so this will actually augment the update mutation and prevent passing null if you are not the owner.
  4. read - Places a resolver on the field (or updates an existing resolver in the case of @connection) that restricts access to the field. When used on a non-model type, this still protects access to the resolver.

Proposal 5: And/Or in @auth rules

Github Issues

Problem

Currently all @auth rules are joined via a top level OR operation. For example, the schema below results in rules where you can access Post objects if you are the owner OR if you are member of the "Admin" group.

type Post @model @auth(rules: [{ allow: owner }, { allow: groups, groups: ["Admin"] }]) {
    id: ID!
    title: String
    author: User @connection(name: "UserPosts")
    owner: String
}

It would be useful if you could organize these auth rules using more complex rules combined with AND and OR.

Solution

We can accomplish this by adding to the the @auth definition.

directive @auth(rules: [TopLevelAuthRule!]!) on OBJECT, FIELD_DEFINITION
input TopLevelAuthRule {
    # For backwards compat, any rule specified at the same level as an "and"/"or" will be joined via an OR.
    allow: AuthStrategy!
    ownerField: String # defaults to "owner"
    identityField: String # defaults to "cognito:username" for UserPools, "username" for IAM, "sub" for OIDC
    groupsField: String
    groups: [String]

    # This only exists in top level rules and specifies operations for all the rules even when combined with and/or.
    # Neseted "operations" tags are not allowed because it would confuse evaluation logic.
    operations: [ModelOperation]

    # New recursive fields on AuthRule
    and: [AuthRule]
    or: [AuthRule]   
}
input AuthRule {
    allow: AuthStrategy!
    ownerField: String # defaults to "owner"
    identityField: String # defaults to "cognito:username" for UserPools, "username" for IAM, "sub" for OIDC
    groupsField: String
    groups: [String]

    # New recursive fields on AuthRule
    and: [AuthRule]
    or: [AuthRule]
}
enum AuthStrategy { owner groups }
# Reduces get/list to read. See explanation below.
enum ModelOperation { create update delete read }

This would allow users to define advanced auth configurations like:

type User
  @model 
  @auth(rules: [{
    and: [
      { allow: owner },
      { or: [
        { allow: groups, groups: ["Admin"] },
        { allow: owner, ownerField: ["admins"] },
      }
    ],
    operations: [read]
  }]) {
  id: ID!
  admins: [String]
  owner: String
}
# Logically: ( isOwner && ( isInAdminGroup || isMemberOfAdminsField ) )

The generated resolver logic will need to be updated to evaluate the expression tree.

Proposal 6: Deny by default mode.

Github Issues

Problem: There is currently no way to specify deny access by default for Amplify APIs.

If you create an API using a schema:

type Post @model {
    id: ID!
    title: String!
}

then the generated create, update, delete, get, and list resolvers allow access to any request that includes a valid user pool token (for USER_POOL auth). This proposal will introduce a flag that specifies that all operations should be denied by default and thus all fields that do not contain an explicit auth rule will be denied. This will also change the behavior of create mutations such that the logged in user identity is never added automatically when creating objects with ownership auth.

Solution: Provide a flag that enables deny by default

By adding a DenyByDefault flag to parameters.json or transform.conf.json will allow users to specify whether fields without an @auth directive will allow access or not. When deny by default is enabled the following changes will be made.

  1. Mutation.createX resolvers will no longer auto-inject the ownership credential when the ownership credential is not provided when creating objects. Users will have to supply the ownership credential from the client and it will be validated in the mutation resolver (this happens already when you provide the ownership credential in the input).
  2. All resolvers created by a @model without an @auth directive will be denied by default.
  3. All resolvers created by @searchable on a @model without an @auth will be denied by default.
  4. All resolvers created by @connection that return a @model with an @auth AND that do not have their own field level @auth will be denied by default.

For example, with deny by default enabled

type Post @model {
    id: ID!
    title: String
    author: User @connection(name: "UserPosts")
    comments: [Comment] @connection(name: "PostComments")
}
type User @model @auth(rules: [{allow: owner}]) {
    id: ID!
    username: String!
    posts: [Post] @connection(name: "UserPosts")
}
type Comment @model {
    id: ID!
    content: String
    post: [Comment] @connection(name: "PostComments")
}

This mutation would fail:

mutation CreatePost {
    createPost(...) {
        id
        title
    }
}

This mutation would succeed:

mutation CreateUser {
    createUser(input: { username: "me@email.co" }) { # Assuming the logged in user identity is the same
        id
        title
    }
}

This top level query would succeed but Post.comments would fail.

query GetUserAndPostsAndComments {
    getUser(id: 1) { # succeeds assuming this is my user.
        posts { # succeeds because the @auth on User authorizes the child fields
            items {
                title
                comments { # fails because there is no auth rule on Post, Comment, or the Post.comments field.
                    items {
                        content
                    }
                }
            }
        }
    }
}

More details coming soon

  1. Write custom auth logic w/ pipeline functions
  2. Enable IAM auth within the API category

Request for comments

This document details a road map for authorization improvements in the Amplify CLI's API category. If there are use cases that are not covered or you have a suggestion for one of the proposals above please comment below.

rawadrifai commented 4 years ago

EDITED @undefobj I understand.

Could be an ignorant question since I don't know the internals of Amplify well.. but wouldn't it be possible to extract the user group from a connected subscription? If so, we can extract all the users subscribed with their auth claims/groups. When a new message comes in we can check the group it belongs to against every user subscribed.. if there's a group match, you send an event?

A radical solution for this in my opinion would be to allow maintaining claims in DynamoDB alongside Cognito. So we would add a parameter to the @auth called source which can be Cognito or Dynamo table with a certain structure that matches Cognito. I think this is a common use case when it comes to subscriptions. Groups and memberships should be as dynamic as possible, and virtually unlimited.

rawadrifai commented 4 years ago

@undefobj addition: can dynamo streams be used after the item has been created to support dynamic groups auth for subscriptions?

undefobj commented 4 years ago

All - @auth protection on subscriptions have now been released with a major version bump of the CLI to 2.0. The new behavior along with additions of custom claims can be found here: https://aws-amplify.github.io/docs/cli-toolchain/graphql#authorizing-subscriptions

undefobj commented 4 years ago

EDITED @undefobj I understand.

but wouldn't it be possible to extract the user group from a connected subscription? If so, we can extract all the users subscribed with their auth claims/groups. When a new message comes in we can check the group it belongs to against every user subscribed.. if there's a group match, you send an event?

No AppSync does not support runtime execution of resolvers, it only happens at connect time.

A radical solution for this in my opinion would be to allow maintaining claims in DynamoDB alongside Cognito. So we would add a parameter to the @auth called source which can be Cognito or Dynamo table with a certain structure that matches Cognito. I think this is a common use case when it comes to subscriptions. Groups and memberships should be as dynamic as possible, and virtually unlimited.

This is what the groupClaim allows you to do: https://aws-amplify.github.io/docs/cli-toolchain/graphql#custom-claims Your pre-token trigger can read from your DynamoDB table to populate the claim and then when the subscription connects you're performing authorization against it.

undefobj commented 4 years ago

@undefobj addition: can dynamo streams be used after the item has been created to support dynamic groups auth for subscriptions?

Not sure what that would do or why it's needed. Again, you can't check something that doesn't yet exist. Dynamic Group functionality was created to solve one problem which was marking an item on a query with authorization metadata to allow it to be read. This is different than a push mechanism.

Per my earlier comments subscription authorization happens at connect time, not at runtime. Therefore even if you changed the subscription resolver to query a database it's effectively doing the same thing as the state which has been hydrated in your token from the pre-token Lambda trigger so you don't get any change in behavior. This is why we added the claim information based on your earlier feedback and use case.

My suggestion is to see what use case you are not able to achieve with the current system and we can look at alternatives in the future, based on the feedback and use case you gave earlier this seems to get the job done.

rawadrifai commented 4 years ago

@undefobj thank you for taking the time. There's clear miscommunication, we're going in circles.. My use case is a Slack-like app. Here's where I state that: https://github.com/aws-amplify/amplify-cli/issues/1766#issuecomment-510699355

You can have multiple "spaces", multiple users, channels, conversations and Messages in each. All in one DynamoDB, and virtually no limits on anything except spaces.

The Messages type contains every message. It is protected with dynamic groups auth and points to a group field. Every message should come with permissions. If it's a channel message, the "group" value is <space-name>@<group-name>. This way only the users that belong in that channel can access this message. I keep a mapping of which users belong to which groups in a different dynamo table.

When a user logs in, I query the mapping in dynamo, I inject the groupClaims in the token (in the pre token generation trigger). So now, I expect a user to receive subscriptions for messages he or she is allowed to read. This works GREAT for queries and mutations, but not for subscriptions since I'm relying on dynamic grouping.

Is this doable with static grouping? Am I missing something obvious here?

undefobj commented 4 years ago

@undefobj thank you for taking the time. There's clear miscommunication. My use case is a Slack-like app. Here's where I state that: #1766 (comment)

You can have multiple "spaces", multiple users, channels, conversations and Messages in each. All in one DynamoDB, and virtually no limits on anything except spaces.

The Messages type contains every message. It is protected with dynamic groups auth and points to a group field. Every message should come with permissions. If it's a channel message, the "group" value is <space-name>@<group-name>. This way only the users that belong in that channel can access this message. I keep a mapping of which users belong to which groups in a different dynamo table.

When a user logs in, I query the mapping in dynamo, I inject the groupClaims in the token (in the pre token generation trigger). So now, I expect a user to receive subscriptions for messages he or she is allowed to read. This works GREAT for queries and mutations, but not for subscriptions since I'm relying on dynamic grouping.

Is this doable with static grouping? Am I missing something obvious here?

Yes either use a Cognito Group for your channel or inject them into the claim by reading a separate DynamoDB table as you mention above. This was based on your reply here: https://github.com/aws-amplify/amplify-cli/issues/1766#issuecomment-522839444

rawadrifai commented 4 years ago

@undefobj

use a Cognito Group

I can't, there's a cap on the number of groups.

inject them into the claim

I'm doing that already.

Yes either ..

How would you re-write this with static groups so that 2.0 subs work?

type Message @auth(rules: [
  { allow: owner },
  { allow: groups, groupsField: "group", operations: [read] }
]) {
  id: ID!
  owner: String
  group: String
}

Example messages

id    |   owner |  group
---------------------
id-1 | user-1 | channel-1
id-2 | user-2 | channel-2
id-3 | user-1 | channel-2

Membership mapping

user | group
--------------
user-1 | channel-1
user-1 | channel-2
user-2 | channel-2

Let me know if you prefer to take this offline.

undefobj commented 4 years ago

@rawadrifai I think at this point you're looking for design assistance on your data model. It doesn't need to be offline, but I'd suggest opening up a separate issue that just covers that topic rather than go through it in an RFC.

rawadrifai commented 4 years ago

@undefobj awfully ungrateful and unprofessional. I took many hours of my time to assist you with your requirements. You asked for my use case, I gave it to you. If you insist the solution has been provided, so be it. I'm done wasting time on this.

undefobj commented 4 years ago

@rawadrifai I think theres a bit of a disconnect here, I actually was offering that we don't conflate the threads here as this is a massive RFC with multiple concerns and it would be clearer to assist you in a single issue. By no means was the intent to dismiss and I apologize if you took it that way.

rawadrifai commented 4 years ago

@undefobj no worries. We’re still building with amplify at this point too deep, hoping there would be support for dynamic groups. It’s impossible to design something like slack with static groups only. Let us know if anything changes.

aej11a commented 4 years ago

Hey @undefobj , thanks for all your work on this.

There haven't been any updates or comments on Proposal 4 in a long time, is that still in the roadmap? any progress on it yet?

thanks again

amirmishani commented 4 years ago

@aej11a field level authorization has been implemented for some time now. See docs here.

aej11a commented 4 years ago

@amirmishani Thank you! Missed that somehow

rawadrifai commented 4 years ago

@undefobj I opened this (sorry for posting on the RFC) aws-amplify/amplify-category-api#389. If there is no intention to support dynamic groups with subscriptions in the near future, please let us know. If you have an alternative solution staying within aws, also pls comment on the open request.

undefobj commented 4 years ago

Hello everyone, an update on auth based off of feedback from the RFC: today we released multi-auth functionality which includes explicit controls and overrides for public & private access, provider overrides, and mixing/matching authorization controls. You can read more here:

https://aws-amplify.github.io/docs/cli-toolchain/graphql#public-authorization https://aws-amplify.github.io/docs/cli-toolchain/graphql#private-authorization https://aws-amplify.github.io/docs/cli-toolchain/graphql#combining-authorization-rules https://aws-amplify.github.io/docs/cli-toolchain/graphql#allowed-authorization-mode-vs-provider-combinations

rawadrifai commented 4 years ago

@undefobj Good updates. Any news on subscriptions and dynamic group auth? If it's not looking likely pls let us know.

amirmishani commented 4 years ago

@undefobj In the docs you mention:

Note identityField is being deprecated for identityClaim.

Not sure I understand if there's a difference between identityField and identityClaim. Can I just swap identityField with identityClaim in the example below?

type User
  @model
  @key(fields: ["tenantId", "userId"])
  @auth(
    rules: [
      {
        allow: owner
        ownerField: "userId"
        identityField: "sub"
      }
      {
        allow: owner
        ownerField: "tenantId"
        identityField: "custom:tenant_id"
      }
    ]
  ) { ... }

Side note: Unfortunately until we get the AND rule mentioned in proposal 5, I have to manually connect the two rules and edit all my resolvers, but still better than creating them from scratch!

undefobj commented 4 years ago

@amirmishani no difference in functionality just an alias for clarity

undefobj commented 4 years ago

@rawadrifai probably not because it doesn't make sense for Subscriptions. We spent several hours whiteboarding this and it's not the same use case as a request/response operation. You can dynamically grant access at connection time in your token as previously mentioned.

mlecoq commented 4 years ago

Hi,

I have an application where users create reports that can be private or shared with on or several users.

I first thought I could use an editors field in reports to manage the set of allowed users on a specific report.

I encountered the following limitation on subscription (detailed in proposal 3) :

Subscription fields are equality checked against published objects. This means that subscribing to objects with with multi-owner or multi-group auth might behave slightly differently than expected. When you subscribe you will need to pass the full list of owners/groups on the item. Not just the calling identity.

I cannot use groups since each reports can be shared to one or several users independently.

Is there anything planned to manage this kind of use case ?

Thanks

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

amirmishani commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

I'm still waiting to see if we can get Proposal 5 and have "And" in @auth rules. This is very important for multi-tenancy where you always need to check for tenant id AND other auth rules.

mvogttech commented 4 years ago

@amirmishani Same here. This is the last thing I am waiting for to release my app.

edlefebvre commented 4 years ago

Hi there, any update on Proposal 5 "And" in @auth rules? @mikeparisstuff ?

tomyitav commented 3 years ago

Hi, is there anything new about granting read access for non-authenticated users?

mir1198yusuf commented 3 years ago

In proposal 3 - add a new sub-proposal or something -

the subscription started before would not be triggered because ["my-user-id", "my-friends-user-id"] is not the same as ["my-user-id"]. I bring this up for clarity but I still think this feature is useful. Single owner & group based authorization will behave as expected.

Single and group owner should behave as expected but sometimes we need a if-found condition. Add a new if-in-group owner. Allow subscriptions if a user is present in a member array is very frequently needed use case especially when the members list could be dynamic (users can be added or removed )

Example whatsapp application https://github.com/aws-amplify/amplify-js/issues/7534#issuecomment-760412322

kylekirkby commented 3 years ago

I'm trying to create a YouTube-like resources hub where some sources are public and others are private.

If I use the current public @auth directive, all resources are then accesible to the public but only a subset of them should be.

For example:

type Resource
    @model
    @auth(
        rules: [
            { allow: groups, groupField: "visibleTo", operations: [read] }
            { allow: groups, groupField: "editors", operations: [update, read] }
            { allow: groups, groups: ["admin"], operations: [create, update, read, delete] }
        ]
    ) {
    id: ID!
    title: String!
    description: String!
    videoUrl: String
    presentationUrl: String
    editors: [String]!
    visibleTo: [String]!
}

The above would look at the visibleTo array for the names of groups that have access to read the resources. The editors array would contain the names of Cognito groups that have access to edit resources. Admins can perform CRUD operations.

With the above example, how does one enable public access without writing countless custom resolvers/lambda functions?

There is no public group assigned to unauthenticated users that receive their credentials (without signing in to the app) via Auth.getCredentials().

Any suggestions on how we can enable public/private access that can change? It would be great if there was a way add a public group to the editors/viewers fields that resolves to public access.

Maybe the @auth directive can be updated so that:

@auth(rules: [{ allow: groups, groupField: "visibleTo", operations: [read], includePublicGroup: "true"}])

If includePublicGroup is set to "true" then the presence of a special group name applies public access rules. I.e ["__PUBLIC_GROUP__", "admins", "editors", "basic"].

ryanhollander commented 3 years ago

@kylekirkby I'm sorry I don't have time for more thorough research or a better answer. You should look at both the "custom claims" in the amplify cli graphql transformer documentation, and pre-token trigger in cognito. I used the latter to inject group claims for federated users that are linked to cognito identity pool users. I haven't tried this with public users, I suspect one of these features might be your best path.

kylekirkby commented 3 years ago

@ryanhollander - Thanks for your reply! I think the pre token trigger method would work as long as unauthenticated requests sent to app sync respect the custom identity claims. The pre token trigger function would have to inject a cognito:groups value of public for unauth users. I’ll have a play with it and see if I can get something working.

ryanhollander commented 3 years ago

@kylekirkby Good luck. FYI, You can use the Amplify CLI to create the trigger w/Lambda through the Auth Flow.

kylekirkby commented 3 years ago

@ryanhollander cheers! I used the cli to add a pre sign up trigger hoping to be able to link External Provider accounts with Cognito Users only to find that the AdminLinkUsers api function is broken :/

https://blog.ilearnaws.com/2020/08/06/error-cognito-auth-flow-fails-with-already-found-an-entry-for-username/

ryanhollander commented 3 years ago

@kylekirkby briefly, I just implemented this using the pre-sign-up trigger and adminLinkProviderForUser. I did this using the Auth Flow and a few lambdas. There were a lot of gotcha's! What I thought might take a morning took me almost 3 days. I did not experience this particular issue, though I read about it alot, it's possible the amplify js eats the issue or cognito was fixed, not sure. FWIW: I'm using Amplify's FederatedLogin.

What I ended up doing was a tad convoluted. The issue is that you can't give the trigger lambda the permissions for the user pool. This is because it would create a circular dependency in the build. The workaround is to create a second lambda, give it the auth permissions via the cli, and manually add the execute permissions to the second lambda for the trigger via the cloudformation json (because there is no dependency for this as IAM doesn't care if your resource exists). Let the trigger call the second lambda for any cognito functions. What I found was I needed two of these, a pre-sign-up to link the federated user to the pool user, and a pre-token to inject the group assignments from the linked cognito user into the token of the federated user. For this project I am using the user pool as the primary user store but users can only login with their domain email through a federated login (for now).

Anyway, I might get around to posting more detail and code on this at some point...

kylekirkby commented 3 years ago

@ryanhollander nice one for the write up! Sounds familiar! I’ve done the whole create a secondary lambda to be called from a post confirmation trigger in another project. I managed to get the pre signup trigger to work with wildcard permissions without needing to add to another lambda function. I know the full cognito pool arn should be used but that was a quick work around and I assumed that since the lambda is triggered directly from Cognito itself, it should be safe some what to assume the correct pull is used.

As for the @auth situation. I’m thinking it might be easier to separate out the PrivateResources into its own type. And give full public read access to a Resource type (public).

As for the storage of the videos in S3 I think I’m going to need to write a custom query to fetch a pre-signed url from s3 if the user belongs to a valid group for that resource.

ryanhollander commented 3 years ago

@kylekirkby with the amplify cli you can give the lambda the exact pool arn for each env with 0 overhead after you do the 1 custom config. Another type/model is always an option, both paths have advantages. I've done that signed URL to S3 scheme too. It's not hard to implement and works great. I think I pulled most of the code from a blog post somewhere.

kylekirkby commented 3 years ago

@ryanhollander how did you arrange the bucket for user uploads? Did you use the Amplify Storage feature and then set uploads to private?

ryanhollander commented 3 years ago

@kylekirkby I used the Amplify Storage and amplify.js on the front end, I store the reference in DynamoDB. I then use the aws sdk in a lambda to generate a signed URL with a 24 hour expiration and a custom URL shortener (lambda/s3) to generate a short, branded link pointing to the signed url. This is what we send to the user when they request a document. The user is interacting with an Alexa skill in this case, and the skill sends the link via SMS or email immediately upon request.

kylekirkby commented 3 years ago

@ryanhollander Thanks for the insight!

octaneC8H18 commented 3 years ago

Proposals 1 & 5 implemented in aws-amplify/amplify-cli#1262

But Proposal 5 is still not working. I still can not implement multi tenancy with user roles as proposed in aws-amplify/amplify-category-api#449.

I want only the admin to have access on the type and only within the tenant.

My query is

  @auth(
    rules: [
      { allow: groups, groups: ["admin"] }
      { allow: groups, groupsField: "tenantId", groupClaim: "tenantId" }
    ]
  )

It still generates

#if( !($isStaticGroupAuthorized == true || $isDynamicGroupAuthorized == true || $isOwnerAuthorized == true) )
    $util.unauthorized()
  #end

Or am I missing something. What have I done wrong? could someone please help me? :)

@mikeparisstuff @undefobj @edwardfoyle

mir1198yusuf commented 3 years ago

Can anyone help me in this ? It is a pretty common use case, I am sure someone might have found solution for group-owner if-found match instead of whole array comparison criteria ..

aws-amplify/amplify-category-api#433

loganpowell commented 2 years ago

I believe something like this might help address some of the concerns brought up in other issues. Some users - including myself - are looking at graphql interfaces as a channel for discussing granting permissions to various table items based on those implementations' @auth rules. However, if we could have auth set by an enum, we could have control over the publicity of an item from within that item instead of by the table.


enum PermissionType  {
  PUBLIC @auth( rules: [
      { allow: public, operations: [ read ] },
      { allow: groups, groups: ["Admins", "Editors"] }
    ])
  PRIVATE @auth( rules: [
      { allow: groups, groups: ["Admins", "Editors"] }
   ])
}

type Resource @model {
  id: ID! @primaryKey
  status: PermissionType! @index(name: "Resources_by_Status", queryField: "resourcesByStatus")
  name: String!
}

type Node @model {
  id: ID! @primaryKey
  status: PermissionType! @index(name: "Nodes_by_Status", queryField: "nodesByStatus")
  resources: [Resource] @hasMany(indexName: "Resources_by_Node", fields: ["id"])
  edges: [Edge] @manyToMany(relationName: "EdgeNode")
}

type Edge @model {
  id: ID! @primaryKey
  nodes: [Node] @manyToMany(relationName: "EdgeNode")
}

This would allow us to toggle permissions on an item by simply updating a single item's field, rather than having to delete a private item, create a public one and reconnect them to their various connections... Would love to hear your thoughts on an approach such as this

kylekirkby commented 2 years ago

@loganpowell - this looks ideal. We've currently had to write custom resolvers to check if the user is using the unauth role, which, if they are, we don't return private resources. If the user is logged in then the permissions for resources are entirely based on their group membership.

loganpowell commented 2 years ago

@kylekirkby do you have a GSI that has the resources indexed by auth role? Might you share your custom resolver?

kylekirkby commented 2 years ago

@kylekirkby do you have a GSI that has the resources indexed by auth role? Might you share your custom resolver?

Hi @loganpowell ,

So currently our Resource type has a private boolean field. This is then used to prevent users from seeing private resources in queries via custom VTL code. Currently, I've only managed to get this working in the response resolver, which means when you expect 12 resources in a query you get 12 - any that were private.

What I'm looking at now is updating the client-side queries to filter based on the private value so the correct number of resources are returned to unauth users. It would be much cleaner to get the req resolver filtering these based on the use of the unauth role but, as you probably know, documentation and guides are severely lacking for VTL + Amplify.

So in

amplify/backend/api/yourAPIName/resolvers/Query.resourcesByCreatedDate.res.vtl

I've got this:

#if( $ctx.identity.userArn == $ctx.stash.unauthRole )
  #set( $items = [] )
  #foreach( $item in $ctx.result.items )
    #set( $isPrivate = $util.defaultIfNull($item.private, false) )
    #if( $isPrivate == false )
      $util.qr($items.add($item))
    #end
  #end
  #set( $ctx.result.items = $items )
#end

#if( $ctx.error )
  $util.error($ctx.error.message, $ctx.error.type)
#end
$util.toJson($ctx.result)

If the user is using the unauth role, then private resources are not returned. You'll have to add this logic to all of your resource queries by overriding the amplify generated resolvers.

There may be bits I'm missing but I hope this helps! This has been one hell of an issue to solve for me!

Edit: this will only work on the v2 GraphQL transformer.

redjonzaci commented 11 months ago

What's the progress in here? I also get the following warning: This issue is being transferred. Timeline may not be complete until it finishes.