-
Notifications
You must be signed in to change notification settings - Fork 79
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
@auth Combining Owner/Groups rules for Multi-Tenant Apps #449
Comments
Probably related to aws-amplify/amplify-cli#305 as well. |
This would also at least partially answer the question I posted over on amplify-js: |
In regards to aws-amplify/amplify-cli#305, that issue has been fixed here aws-amplify/amplify-cli#285. You may see the PR notes to see what has changed as well as an example of the complex auth flows that are possible now. |
In regards to this issue, it seems like the question relates to how to configure advanced auth flows such that rules may be composed together using "and" and "or" in flat and nested configurations. Since this is really a feature request, let's try to define the ideal implementation. To allow for this use case we can extend the directive @auth(rules: [AuthRule!]!) on OBJECT
input AuthRule {
allow: AuthStrategy!
ownerField: String # defaults to "owner"
identityField: String # defaults to "username"
groupsField: String
groups: [String]
queries: [ModelQuery]
mutations: [ModelMutation]
# New Additions
and: [AuthRule]
or: [AuthRule]
}
enum AuthStrategy { owner groups }
enum ModelQuery { get list }
enum ModelMutation { create update delete } This follows the same structure that is used when we generate filter input objects for An example: @auth(rules: [
{ and: [
{ allow: owner } # Call this A
{ or: [
{ allow: groups, groups: ["Admin"] } # Call this B
{ allow: groups, groupsField: "groups"] } # Call this C
]}
]}
]) would result in a final boolean expression:
Conceptually this makes sense and should be possible but I will have to spend more time with the details to figure out if there are any scenarios that will not work. If you have any feedback on this design please share as it would be helpful to have a few alternative to help guide the discussion. Notes:
|
This would definitely solve this specific use case and opens up many more complex auth options. To solve the multi-tenant use case the auth rule becomes:
would result in a final boolean expression:
This would opaquely add/match the TenantId value during any mutations and filter any queries based on the TenantId. The subset of other auth rules would then further control access. Happy to help test out various other combinations to validate this strategy either on the branch your are using for PR aws-amplify/amplify-cli#183 or if you want to handle this separately. |
To clarify, as @lennybr, I think an alternative to a new tenant rule (and a solution for a lot of the complexity I'm trying to work around) could be allowing |
The queries and mutations argument only working on the top-level wouldn’t let us create a multi-tenant setup, since those arguments would be for each group in theory, I would want to do something like:
|
@learningacct interesting idea, might be able to accomplish that by creating a Tenant model and wrapping any other tenant-separated models with a nested resolver that first does a user/tenant lookup and then passes down the Tenant Id to the actual tenant model data resolver. There would likely be performance implications running both resolvers, so not sure about practicality. And linked Cognito Users to Tenants would be much easier after aws-amplify/amplify-cli#56 is solved. A schema could look something like:
and the Data___Tenant resolvers
and the Data resolvers
|
@learningacct As of aws-amplify/amplify-cli#285 you can use the multiple owner auth features to support the For example, type Team @model @auth(rules: [{ allow: owner, ownerField: "members" }]) {
id: ID!
name: String
members: [ID]
} This would only allow team members (as defined by the list of ids in |
Also this discussion might be relevant to this aws-amplify/amplify-cli#318. Here we talk about how you can use the schema to build complex relationships that also effectively act as auth rules for read operations. There are some implications of using this to protect mutations but it may offer some new ideas. |
Ah, great! I did try to look through that PR, but a lot of it went over my head. Thank you! And aws-amplify/amplify-cli#318 looks like it provides some helpful examples too. I'll look that over and do my best to apply it to what I'm doing. :) @mikeparisstuff, am I able to use a variation of aws-amplify/amplify-cli#318 to sort of nest tasks within teams within tenants? |
@mikeparisstuff aws-amplify/amplify-cli#318 seems messy, especially with mutations and filling in owner fields. And while the |
@lennybr You are correct and that is actually the plan as soon as the feature is possible in AppSync. We are going to add new strategies for auth rules and one of them will be to check for membership in a many-to-many connection but this is blocked by a service feature that will be releasing later this year. A similar ask has been made in aws-amplify/amplify-cli#372. |
@mikeparisstuff great news. Any insights into the approach you are going to take for implementation? Will AppSync support middleware we can auth for Auth? Will you use nested resolvers? Or something else? I'm working on my own implementation now (currently using a root nested resolver against a tenantMembership object) but would love to stay in sync and align. Let us know if we can help and where to track progress on the feature. Thanks! |
Pipeline resolvers just got released (https://aws.amazon.com/blogs/mobile/aws-appsync-releases-pipeline-resolvers-aurora-serverless-support-delta-sync/) and I assume this is the feature @mikeparisstuff is referring to. Looks very interesting! :) |
@hisham 👍 Exactly correct. We have a few changes that need to be made first but this is on the roadmap. Unfortunately no set date but I will reference these issues as soon as its in PR. |
Just chiming in to say that our team is also eagerly awaiting this feature. |
@mikeparisstuff pipeline resolvers seem like a great step on this. building an app using amplify right now (which we're finding super promising), and sorely need this feature. Should we bite the bullet and write our own pipeline resolvers, or should we expect to see this via the CLI soon? |
@mikeparisstuff until this issue is solved, what is currently the best approach to a complex multi tenant auth where you need to check again both tenant_id and user's role? A custom resolver using VTL for every table that needs auth? |
@lennybr said:
...and later, @lennybr said:
...and later, @mikeparisstuff said in https://github.com/aws-amplify/amplify-cli/issues/1043:
⚡️🚀⚡️
@lennybr It seems that the solution you projected is now a reality. A solution for exports.handler = (event, context, callback) => {
console.log(
'Received eventObject {} in Invoke Request.',
JSON.stringify(event, 3),
);
event.response = {
"claimsOverrideDetails": {
"claimsToAddOrOverride": {
"tid": event.userPoolId,
},
}
};
// Return to Amazon Cognito
callback(null, event);
}; That would kick-start any multi-tenant Amplify project. Comprehensive docs are here. Is that how you are doing this? |
I got mistaken, @mikeparisstuff meant "Proposals 1 & 4 implemented", so Lambda config itself does not allow setting the pre-token generation lambda trigger (and |
@zepelega @nhruch here is another GitHub issue where I detailed how to break the circular dependency: |
@nhruch looks like Amplify CLI added built-in functionality to break these circular dependencies. |
Any update on this? |
The following is a working Lambda that includes @paulsson 's code with the addition of an API call to graphql. This works by adding
Another hurdle as of CLI v8.5.0 is V2 of graphql. There is a breaking change where the auth rule doesn't like to read from the id's. I've changed my structure to the following and in the front end pass in the same UUID to both
|
AWS team - this feature of supporting multi tenancy is critical for enterprise grade apps. Cant believe this is open from 2018 and no action yet!! @dabit3 @kaustavghosh06 can you please help promote this ? also this is marked P4 ?!!!??? damn this is P1. |
Subscription tiers is another use case for this. Very difficult to model different types of permissions for different users without logical AND. |
Being able to use AND as well as OR to combine different auth rules would make it much easier. We are currently facing the Issue, where we need to have the User (owner) to have access, as well as a group of Admins per Tenant. IF we would be able to use AND we could just have a group "admins" and the tenant_ID in the groupfield. Then you could gran access if ( Owner || ( group: "admins" && groupfield matches)). I feel like this feature should have been added a long time ago. Idk why this has been open since 2018 since it's such a good feature suggestion |
Given the complexity of this we’ve given up trying to manage using directives. In the end custom resolvers with the full power of a coding language and an auth a lambda is the better way to go for our use case when going off the beaten path rather than try to solve with a dsl. Probably not the solution people want to hear but perhaps will convince people to move off it before things go too far down a path. |
Why not at least add an global "option" to switch logic of combining @auth rules from "OR" to "AND" between "ownership" and "group" rules? |
Also interested in into this topic... |
Any news on this? |
Dear AWS Team, this is critical in order to build any scalable SaaS product. We need it urgently, please let us know if that's coming or not anytime soon. |
Any update on this? |
#butTheYoutuberSaidThisProductWasGood
…On Wed, Sep 13, 2023, 5:43 AM Redjon Zaci ***@***.***> wrote:
Honestly, I don't see that here.
[image: image]
<https://user-images.githubusercontent.com/73707194/267608646-fe29309b-4db0-4f4a-8ebe-94f7f11dd179.png>
—
Reply to this email directly, view it on GitHub
<#449 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKJQCJZCBBLSSPDMUEKFGKTX2F53HANCNFSM5WGKWFMQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
@mikeparisstuff @kaustavghosh06 @mdwt Is there anyone even looking at this? Is Amplify still actively supported by Amazon/AWS and if so, is multi-tenant app support a priority in Amplify's roadmap? |
My best guess is that this is not prioritised, hence I avoid using amplify for multi-tenant applications 😩 |
It would be awesome if there were a multi-tenant auth/api setup and schema out-of-the-box, i.e. a template that we could apply to spin up a multi-tenant application without so many gymnastics to make it work. |
@renebrandel Would #430 enable this use case for supporting Combining Owner/Groups rules for Multi-Tenant Apps? |
Now how about we talk about alternative services which might kick AWS out out of bed? Firebase/Firestore doesnt work for me as I'm based in Germany / DSGVO compliance. What other services have you considered as alternatives for these requirements? |
Am I understanding this right that Gen2's introduction of composite identifiers is the answer to solving multi-tenancy? |
One thing to look at in Gen 2 (that also exists to some degree in Gen 1), is the ability to use a Lambda for custom data access. https://docs.amplify.aws/gen2/build-a-backend/data/customize-authz/custom-data-access-patterns/ |
It seems to me it just represents the primary key and sort key.
|
Any updates? |
Is your feature request related to a problem? Please describe.
Ability to support multi-tenancy thru AppSync where individual items are "owned/belong" to a tenant instead of a user and we still have the ability to permission queries and mutations. Generated resolvers today effectively use
isOwner || isInGroup(x for x in cognitoGroups)
logic so multiple @auth rules cannot be combined to create more granular permissions.Describe the solution you'd like
A few ideas:
isOwner && isInGroup(x for x in cognitoGroups)
when we have both rules types declaredisTenant && (isOwner || isInGroup(x for x in cognitoGroups))
Describe alternatives you've considered
Currently using the existing @auth owner strategy with custom ownerField and identityFIeld values, and setting the tid claim on the token with a pre-token generation Lambda function:
When used as the only @auth strategy, it works as intended (e.g. inserting the correct tid value during mutations; filters by tid value during queries, etc.).
But when I combine with @auth static groups strategy for permissions, the authorisation checks use OR logic instead of AND logic. I can't check for instance that a record both belongs to Tenant A (which the user belongs to) and has Permission X.
The text was updated successfully, but these errors were encountered: