Open bradyrichmond opened 3 months ago
Hey,👋 thanks for raising this! I'm going to transfer this over to our API repository for better assistance 🙂
Hi @bradyrichmond 👋 thanks for raising this issue. If I understand correctly, you want to be able to use an operation name to allow/deny users to perform an operation in a custom authorizer lambda.
I imagine you're referring to using the deniedFields like so:
deniedFields: [
`arn:aws:appsync:${process.env.AWS_REGION}:${accountId}:apis/${apiId}/types/Event/fields/comments`,
`Mutation.createEvent`
],
However, the operation name is set in client clode and can include any qraphql queries within it. If you intend to block a user from accessing a query by operation name, wouldn't the user be able to access the same query under a different operation name? The custom authorizer example we have uses the name of the query/mutation to determine access instead. Would that fulfill your use case?
If I'm misunderstanding, please provide a more complete example to explain your use case.
Thanks for taking a look at this, @chrisbonifacio!
I realize that Amplify doesn’t explicitly support a Multi Tenant SaaS configuration, but that’s what I’m trying to set up. Since the authorization function on the schema uses or for the allow array, I need to use a custom authorizer to make sure that users only perform operations for their organization and that they have been provided permission to do an action within their organization.
Right now, I am just using Roles for this, but have a path forward on more granular permissions control.
So what I want my flow to be, just focusing on the organization + role authentication this:
User wants to create a new user for their organization.
They fill out the new user form, with first name, last name, and email address, and submit the form to my mutation createUser
.
Before the mutation is executed, the schema checks my customAuthorizer
function.
In the customAuthorizer
function, I verify the token, and get the custom user attribute custom:orgId
. I use this orgId to make sure that the item, UserProfile
in this case, will be created for the correct organization, and not allow any user to create users in other orgs.
The second half of this is the user roles. I want to only allow certain roles to do certain actions, or retrieve certain data. So, in this case, I only want ADMINS and OWNER roles to be able to create users, and to make sure that ADMINS cannot add users as OWNER role.
so, in order to handle these individual actions based on the action, I need a reliable way to detect the action that they are trying to perform. Then eventually build out the deny fields.
The docs reference operationName
in the requestContext
, but the gql queries generated by generateClient
, when executed, do not include an operationName
in the requestContext
.
Am I missing something for how to authorize individual users for individual actions?
Just wanted to bump this. Let me know if you need something else from me. :)
Hi @bradyrichmond I've marked this as a feature request for the team to consider. I'm still not convinced that this is the appropriate way to determine authorization in a multi-tenant context because operation names are controlled by the client but I'll discuss with the team and provide feedback.
As an example of what I mean:
# Intended query with restricted access
query GetSensitiveUserData {
getSensitiveUserInfo(userId: "123") {
id
socialSecurityNumber
bankAccountDetails
}
}
Your custom auth logic would need to prevent queries with arbitrary operation names from clients:
# Malicious query with a misleading operation name
query GetPublicUserProfile {
getSensitiveUserInfo(userId: "123") {
id
socialSecurityNumber
bankAccountDetails
}
}
# Another malicious query without an operation name
{
getSensitiveUserInfo(userId: "123") {
id
socialSecurityNumber
bankAccountDetails
}
}
Okay. That does make sense. Query string could be altered on the client side, making it pass my auth checks.
Can deniedFields
be used securely for this kind of authorization? There's the reference to Mutation.createEvent
in the docs. Would setting something like that prevent access to the createEvent
mutation entirely?
So, user makes a request to use createEvent
. They don't actually have permission to use this, but the request is coming through anyway.
Schema is set to custom authorization, so it goes to my custom authorizer lambda.
Token is verified, so isAuthorized: true
.
Lambda function verifies the token, checks the user group/permissions and builds the deniedFields
array based on that.
Lambda function returns the object
{
isAuthorized: true,
deniedFields: ['Mutation.createEvent']
}
The api gets this auth object and says "This user is authorized, but not to use createEvent
" and throws an UnauthorizedException
?
You can use deniedFields to nullify fields in the response that the user shouldn't have access to.
For instance, maybe a user has access to Query.listUserProfiles
but not UserProfile.ssn
. You would add UserProfile.ssn
to the deniedFields
list. It should return null
in the response even if a value is returned from the resolver.
I would recommend initializing isAuthorized
as false
to deny requests by default unless all conditions are met. For multi-tenancy, you can store orgId
as a custom attribute on a Cognito user or as a claim on the access token. You can then use it to determine whether or not they can create a record that "belongs" to a specific tenant/org.
example:
exports.handler = async (event) => {
const { authorizationToken, requestContext } = event;
try {
// Verify and decode the JWT (implementation details omitted for brevity)
const decodedToken = verifyAndDecodeJWT(authorizationToken);
const { sub: userId, 'custom:orgId': orgId, 'custom:role': role } = decodedToken;
// Initialize deniedFields array
let deniedFields = [];
// Example: Restrict access to sensitive fields for non-admin users
if (role !== 'ADMIN') {
deniedFields.push('UserProfile.ssn');
deniedFields.push('UserProfile.bankAccountDetails');
}
// Example: Restrict access to other organizations' data
if (requestContext?.arguments?.orgId !== orgId) {
deniedFields.push('Organization');
}
return {
isAuthorized: true,
resolverContext: {
userId,
orgId,
role,
},
deniedFields: deniedFields,
};
} catch (error) {
console.error('Authorization failed:', error);
return { isAuthorized: false };
}
};
This is just an example but you can send also extra context to the resolver through the resolverContext
to further narrow the data as needed in the resolver.
In the resolver you can perform checks against a user's identity (or "tenant") like so:
import { util, Context } from '@aws-appsync/utils';
import { get } from '@aws-appsync/utils/dynamodb';
export function request(ctx) {
return get({ key: { ctx.args.id } });
}
export function response(ctx) {
if (ctx.result.orgId === ctx.orgId) {
return ctx.result;
}
return null;
}
Okay. Gave that a try. I am able to deny access to Queries and Mutations, but not to individual fields. For example, I have this InventoryItem
in my schema:
InventoryItem: a
.model({
costPerUnit: a.float().required(),
leadTime: a.integer().required(),
markup: a.float().required(),
partName: a.string().required(),
organization: a.belongsTo('Organization', 'orgId'),
orgId: a.id().required(),
quantity: a.integer(),
sources: a.hasMany('InventoryItemSource', 'inventoryItemId'),
unitType: a.string().required(),
})
.secondaryIndexes((index) => [index('orgId')]),
I tested denying access to partName
by creating a deniedFields
array like this:
const response = {
isAuthorized: true,
resolverContext: {
orgId,
},
ttlOverride: 10,
deniedFields: ['InventoryItem.partName'],
}
I get this message back when I attempt to run listInventoryItemByOrgId
:
Not Authorized to access partName on type InventoryItem
and items
is null.
Hi @bradyrichmond 👋 can you try making a request with a modified selection set so that it's omitting the partName
field and see if that request gets authorized? I think the issue might be that the field can't be requested.
That does get it authorized, but seems to conflict with the documentation IMO. My thought was that it could take the exact same query from different users, and it would still be authorized, but the value would be wiped out from the response. Is this a bug? Can you verify the expected behavior?
Environment information
Description
Right now query strings are generated without an operation name. e.g
"query ($id: ID!) {\n getUserProfile(id: $id) {\n id\n email\n firstName\n lastName\n createdAt\n orgId\n updatedAt\n }\n}\n"
. When using custom resolvers, the requestContext is supposed to include anoperationName
property. Right now it ends up undefined. I would like to be able to use the operationName property in my custom authorizer.I dug around for a while and found code in @aws-amplify/data-schema that I believe is responsible for generating the queryStrings that get used my V6Client.
The operation name already exists in the function, although in a slightly different case. I believe that the only change that would need to happen is to create a pascal case of
graphQLFieldName
and update the graphQLDocument string literal like this:Right now what I'm doing is just searching the
queryString
fromrequestContext
for operation names I'm expecting to handle. However, this runs into collisions quite easily. For example, something as simple as aUserProfile
with anorgId
, and a secondary index oforgId
. This will generatelistUserProfile
, andlistUserProfileByOrgId
. If I search the queryString forlistUserProfile
, it would be true for thelistUserProfileByOrgId
, as well aslistUserProfile
. I may want to handle those two things differently.Is there a technical reason that I don't know about that makes this a bigger issue than I think it is?