Open dnys1 opened 3 years ago
From our discussion, one way to fix this is to check the provider from the authrule, if provider is oidc, do not take off the "cognito:" portion of the identity claim key. With this, we can update our integration test for OIDC: update the README.md to provision OIDC with Cognito, enable/refactor the disabled OIDC integ tests
@lawmicha Not sure I understand this issue on a couple fronts. First, why are we using Cognito with OIDC settings? The standard Cognito User Pools auth does this behind the scenes anyway. Secondly, what is the purpose of the code which @dnys1 is referencing? We should be sending JWT tokens through to AppSync in the Authorization header unmodified so that AuthZ at the API level can take place followed by AuthZ at the Resolver level by looking at scopes can take place in the GraphQL request. Let's chat internally on this so we can come to a consistency resolution around implementation.
Hi @undefobj, I recall this was testing different use cases that developers could be using, and one of them being using Cognito as the OIDC provider, i'll have to defer to @dnys1 to follow up on this
Hi guys - yeah, sorry for the lack of context to this ticket. I had to rack my brain to remember why this was needed, but it came up again today with another user who wanted to use a custom Cognito claim for the identity claim. User Pool authorization will send the Cognito access token, which does not include these claims. By switching to OIDC, you can send the ID token, but because of the logic noted above, the Cognito ID token cannot be used even though it fits all the other criteria of an OIDC provider.
@undefobj the reason why the library has this code is for DataStore when establishing subscriptions, it will do two things- add the token to the request header and extract the identityClaim value from the token and add it as the owner input to the subscription operation.
@dnys1 I think you might have mentioned it back then when we originally discussed this, i thinnk the fix should be something like this:
// Current code - assumes when identityClaim from codegen is "cognito:username", then retrieves AccessToken.username
public func identityClaimOrDefault() -> String {
guard let identityClaim = self.identityClaim else {
return "username"
}
if identityClaim == "cognito:username" {
return "username"
}
return identityClaim
}
// Proposed code for the fix to work with OIDC providers
public func identityClaimOrDefault() -> String {
guard let identityClaim = self.identityClaim else {
return "username"
}
if identityClaim == "cognito:username" && provider == .userPools {
return "username"
}
return identityClaim
}
This way, when developer is using Cognito as the provider type, it will continue to return "username" to retrieve the value from the access token. When the provider is .odic then it will take the identityClaim as is which can be "cognito:username" or a custom field defined by the developer, from the ID token.
Any news on this one? :)
@diogoepsy Thank you - our team will investigate and post updates here. Could you share more details on the OIDC setup you're using. Also, if your schema and reproduction steps are different from the one mentioned above, it'd helpful to share it too.
Describe the bug
Cognito Identity Tokens cannot be used with owner auth due to logic in AWSPluginsCore switching the "cognito:username" identity claim for "username".
Cognito ID tokens and Access Tokens have different structures. This logic in core seems to be accommodating changes to the latter; however, as a result, it seems to have also broken the former.
ID Token Payload
Access Token Payload
Schema
Steps To Reproduce
Expected behavior
API plugin is able to pull "cognito:username" field from ID token when OIDC is being used as the API auth provider.
Amplify Framework Version
1.15.0 (likely applies to 1.13.0+, though)
Amplify Categories
API
Dependency manager
Cocoapods
Swift version
5.5
CLI version
6.3.0
Xcode version
13.0
Relevant log output
Is this a regression? (i.e. was this working before a version upgrade)
No response
Device
iPhone 13 Pro Max - Simulator
iOS Version
iOS 15
Specific to simulators
No response
Additional context
No response