Skip to content

Conversation

@pmathew92
Copy link
Contributor

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

📋 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.

@pmathew92 pmathew92 requested a review from a team as a code owner December 4, 2025 06:15
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: "::"))"
Copy link

@sanchitmehta94 sanchitmehta94 Dec 4, 2025

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: "::")

NandanPrabhu
NandanPrabhu previously approved these changes Dec 4, 2025
sanchitmehta94
sanchitmehta94 previously approved these changes Dec 5, 2025
Copy link

@sanchitmehta94 sanchitmehta94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants