-
Notifications
You must be signed in to change notification settings - Fork 254
refactor: updated the key logic for storing MRRT token #1031
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
base: master
Are you sure you want to change the base?
Conversation
Auth0/CredentialsManager.swift
Outdated
| private func getAPICredentialsStorageKey(audience: String, scope: String?) -> String { | ||
| // Use audience if scope is null else use a combination of audience and scope | ||
| if let scope = scope { | ||
| return "\(audience)::\(scope.replacingOccurrences(of: " ", with: "::"))" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion
if we have two scopes like this but the order is different ,
"user.read user.write"
"user.write user.read"
This would create duplicate keys for our cache . would recommend to do something like below
let normalisedScopes = scope
.split(separator: " ")
.sorted()
.joined(separator: "::")
sanchitmehta94
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
📋 Changes
This PR updates the current key for storing token while using the MRRT flow. The current implementation maps token to each audience. This change considers scope also into the key logic as same audience can have different scope and can get different tokens issued for each scope. One example is the my-account use case.