-
Notifications
You must be signed in to change notification settings - Fork 77
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
Owner @auth authorization limited to fields that are String type #162
Comments
@hisham beside not working could you give more details about this like:
My suspicion is that the "number" type information is lost along the lines somewhere hence the comparison will fail in the resolver. |
Hi @attilah -
You are correct in terms of that this issue is the number type information doesn't seem to be respected in the jwt token and when the jwt token is decoded it becomes a string. I don't know if this a JWT standard thing or cognito limitation. But I do see other JWT token attributes that are numbers (auth_time, exp, iat). Thanks! |
Even if the attribute is returned as number by jwt decoding, the $util.isString code in vtl will return false and so the numbers won't be compared. So there's two issues here - the "number" cognito custom attribute becomes a string at some point, and the owner authorization code in the vtl only does comparisons if the id is of type string. |
Looks like cognito doesn't truly support non-string custom attributes, they are always turned into a string in requests. For example, in my pre token lambda, I check event.request.userAttributes, and my custom attribute there is a string even though it's classified as number type in cognito. I even tried to convert it back to number via claimsToAddOrOverride but that didn't work. Looks like another developer ran into this as well - see aws-amplify/amplify-js#2958 (comment) and https://bobbyhadz.com/blog/aws-cognito-amplify-bad-bugged#default-behavior-2 |
I ended up writing a custom graphql transformer that converts the id from number to string before the auth check then converts it back to number before the dynamodb save. It works but I ran into trouble with the vtls - there's no official supported way to convert string to number. I did $myInt.toString() initially, which works with amplify mock and appsync testing console, but not the "real" appsync backend - it converted the number to a string with scientific notation, so then I used the So I might just succumb and not fight the amplify framework anymore and instead store the id I'm interested in as string... So now there's 3 issues this whole thing uncovered:
Example vtl code my custom transformer generates is below:
and then the following after the auth check:
So @attilah I'm not sure what to do here, whether I should keep my custom transformer workaround until you guys have a better solution or just store everything as string. My concern with my workaround is I lose out on using amplify mock, and that I am converting number to string in a potentially not supported way in AppSync. |
I made the string -> number conversion in both mock and real appsync environments by checking the resulting string from toString and using String.format if it has non-digit characters:
So I have a solution that works now though hacky. Amplify team should decide whether to support Int/Float fields for |
Before opening, please confirm:
How did you install the Amplify CLI?
No response
If applicable, what version of Node.js are you using?
No response
Amplify CLI Version
latest
What operating system are you using?
OSX
Amplify Categories
auth, api
Amplify Commands
codegen
Describe the bug
We have a custom attribute in cognito that is of "number" type.
We'd like to use this attribute as part of owner authorization via the identityClaim attribute.
The field is stored in dynamo as a "Float" type.
In the vtl resolver however, the identityClaim is returned as a string, and so the owner authorization comparison fails (string compared to float) and so owner authorization never works.
I'll send example code / screenshot to amplify-cli@amazon.com
Expected behavior
JWT Token decoding should respect cognito attribute type (something that is a number should be returned as number not string)
Owner authorization should work with both strings and numbers
Reproduction steps
I sent code that reproduces with annotations to amplify-cli@amazon.com
GraphQL schema(s)
Sent to amplify-cli@amazon.com
Log output
Additional information
At minimum this should be documented in amplify cli docs, that owner authorization only works with string fields.
The text was updated successfully, but these errors were encountered: