Skip to content

Commit 06f2abb

Browse files
committed
Remove hardcoded profile scope
1 parent ba5e90b commit 06f2abb

File tree

3 files changed

+28
-10
lines changed

3 files changed

+28
-10
lines changed

msal/application.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -757,6 +757,13 @@ def get_accounts(self, username=None):
757757
lowercase_username = username.lower()
758758
accounts = [a for a in accounts
759759
if a["username"].lower() == lowercase_username]
760+
if not accounts:
761+
logger.warning((
762+
"get_accounts(username='{}') finds no account. "
763+
"If tokens were acquired without 'profile' scope, "
764+
"they would contain no username for filtering. "
765+
"Consider calling get_accounts(username=None) instead."
766+
).format(username))
760767
# Does not further filter by existing RTs here. It probably won't matter.
761768
# Because in most cases Accounts and RTs co-exist.
762769
# Even in the rare case when an RT is revoked and then removed,
@@ -1251,7 +1258,13 @@ def _acquire_token_by_username_password_federated(
12511258
self.client.grant_assertion_encoders.setdefault( # Register a non-standard type
12521259
grant_type, self.client.encode_saml_assertion)
12531260
return self.client.obtain_token_by_assertion(
1254-
wstrust_result["token"], grant_type, scope=scopes, **kwargs)
1261+
wstrust_result["token"], grant_type, scope=scopes,
1262+
on_obtaining_tokens=lambda event: self.token_cache.add(dict(
1263+
event,
1264+
environment=self.authority.instance,
1265+
username=username, # Useful in case IDT contains no such info
1266+
)),
1267+
**kwargs)
12551268

12561269

12571270
class PublicClientApplication(ClientApplication): # browser app or mobile app

msal/token_cache.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,11 +108,13 @@ def wipe(dictionary, sensitive_fields): # Masks sensitive info
108108
if sensitive in dictionary:
109109
dictionary[sensitive] = "********"
110110
wipe(event.get("data", {}),
111-
("password", "client_secret", "refresh_token", "assertion", "username"))
111+
("password", "client_secret", "refresh_token", "assertion"))
112112
try:
113113
return self.__add(event, now=now)
114114
finally:
115-
wipe(event.get("response", {}), ("access_token", "refresh_token"))
115+
wipe(event.get("response", {}), ( # These claims were useful during __add()
116+
"access_token", "refresh_token", "username"))
117+
wipe(event, ["username"]) # Needed for federated ROPC
116118
logger.debug("event=%s", json.dumps(
117119
# We examined and concluded that this log won't have Log Injection risk,
118120
# because the event payload is already in JSON so CR/LF will be escaped.
@@ -184,6 +186,8 @@ def __add(self, event, now=None):
184186
"oid", id_token_claims.get("sub")),
185187
"username": id_token_claims.get("preferred_username") # AAD
186188
or id_token_claims.get("upn") # ADFS 2019
189+
or data.get("username") # Falls back to ROPC username
190+
or event.get("username") # Falls back to Federated ROPC username
187191
or "", # The schema does not like null
188192
"authority_type":
189193
self.AuthorityType.ADFS if realm == "adfs"

tests/test_e2e.py

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,9 @@ def _test_username_password(self,
132132
result = self.app.acquire_token_by_username_password(
133133
username, password, scopes=scope)
134134
self.assertLoosely(result)
135-
# self.assertEqual(None, result.get("error"), str(result))
136135
self.assertCacheWorksForUser(
137136
result, scope,
138-
username=username if ".b2clogin.com" not in authority else None,
137+
username=username, # Our implementation works even when "profile" scope was not requested, or when profile claims is unavailable in B2C
139138
)
140139

141140
def _test_device_flow(
@@ -554,11 +553,13 @@ def _test_acquire_token_obo(self, config_pca, config_cca):
554553
# Assuming you already did that (which is not shown in this test case),
555554
# the following part shows one of the ways to obtain an AT from cache.
556555
username = cca_result.get("id_token_claims", {}).get("preferred_username")
557-
self.assertEqual(config_cca["username"], username)
558-
if username: # A precaution so that we won't use other user's token
559-
account = cca.get_accounts(username=username)[0]
560-
result = cca.acquire_token_silent(config_cca["scope"], account)
561-
self.assertEqual(cca_result["access_token"], result["access_token"])
556+
if username: # It means CCA have requested an IDT w/ "profile" scope
557+
self.assertEqual(config_cca["username"], username)
558+
accounts = cca.get_accounts(username=username)
559+
assert len(accounts) == 1, "App is expected to partition token cache per user"
560+
account = accounts[0]
561+
result = cca.acquire_token_silent(config_cca["scope"], account)
562+
self.assertEqual(cca_result["access_token"], result["access_token"])
562563

563564
def _test_acquire_token_by_client_secret(
564565
self, client_id=None, client_secret=None, authority=None, scope=None,

0 commit comments

Comments
 (0)