Pass on_obtaining_tokens via obtain_token_by_refresh_token

[jiasli]

Symptom

While developing a PoC to solve #335, I noticed that skip_account_creation added by #262 never seems to make its way into TokenCache.__add.

skip_account_creation is assigned here and the lambda is passed to obtain_token_by_refresh_token via on_obtaining_tokens:

microsoft-authentication-library-for-python/msal/application.py

Lines 948 to 957 in 0f1ab8d

	response = client.obtain_token_by_refresh_token(
	entry, rt_getter=lambda token_item: token_item["secret"],
	on_removing_rt=lambda rt_item: None, # Disable RT removal,
	# because an invalid_grant could be caused by new MFA policy,
	# the RT could still be useful for other MFA-less scope or tenant
	on_obtaining_tokens=lambda event: self.token_cache.add(dict(
	event,
	environment=authority.instance,
	skip_account_creation=True, # To honor a concurrent remove_account()
	)),

However, on_obtaining_tokens is discarded by obtain_token_by_refresh_token:

microsoft-authentication-library-for-python/msal/oauth2cli/oauth2.py

Lines 769 to 813 in eb0a86f

	def obtain_token_by_refresh_token(self, token_item, scope=None,
	rt_getter=lambda token_item: token_item["refresh_token"],
	on_removing_rt=None,
	on_updating_rt=None,
	on_obtaining_tokens=None,
	**kwargs):
	# type: (Union[str, dict], Union[str, list, set, tuple], Callable) -> dict
	"""This is an overload which will trigger token storage callbacks.
	:param token_item:
	A refresh token (RT) item, in flexible format. It can be a string,
	or a whatever data structure containing RT string and its metadata,
	in such case the `rt_getter` callable must be able to
	extract the RT string out from the token item data structure.
	Either way, this token_item will be passed into other callbacks as-is.
	:param scope: If omitted, is treated as equal to the scope originally
	granted by the resource owner,
	according to https://tools.ietf.org/html/rfc6749#section-6
	:param rt_getter: A callable to translate the token_item to a raw RT string
	:param on_removing_rt: If absent, fall back to the one defined in initialization
	:param on_updating_rt:
	Default to None, it will fall back to the one defined in initialization.
	This is the most common case.
	As a special case, you can pass in a False,
	then this function will NOT trigger on_updating_rt() for RT UPDATE,
	instead it will allow the RT to be added by on_obtaining_tokens().
	This behavior is useful when you are migrating RTs from elsewhere
	into a token storage managed by this library.
	"""
	resp = super(Client, self).obtain_token_by_refresh_token(
	rt_getter(token_item)
	if not isinstance(token_item, string_types) else token_item,
	scope=scope,
	also_save_rt=on_updating_rt is False,
	**kwargs)
	if resp.get('error') == 'invalid_grant':
	(on_removing_rt or self.on_removing_rt)(token_item) # Discard old RT
	RT = "refresh_token"
	if on_updating_rt is not False and RT in resp:
	(on_updating_rt or self.on_updating_rt)(token_item, resp[RT])
	return resp

To Reproduce

1. Add a debug line

   print("__add:", realm, event.get("skip_account_creation"))

   before

   microsoft-authentication-library-for-python/msal/token_cache.py

   Line 178 in 31b24af

   if client_info and not event.get("skip_account_creation"):

2. Do multi-tenant auth with Azure CLI.
3. Verify the output is:

   __add: organizations None
   __add: 72f988bf-86f1-41af-91ab-2d7cd011db47 None
   __add: 246b1785-9030-40d8-a0f0-d94b15dc002c None
   __add: 2b8e6bbc-631a-4bf6-b0c6-d4947b3c79dd None
   __add: ca97aaa0-5a12-4ae3-8929-c8fb57dd93d6 None
   __add: 54826b22-38d6-4fb2-bad9-b7b93a3e9c5a None
   

   None means skip_account_creation is not set.

Change

After the change, the output is

__add: organizations None
__add: 72f988bf-86f1-41af-91ab-2d7cd011db47 True
__add: 246b1785-9030-40d8-a0f0-d94b15dc002c True
__add: 2b8e6bbc-631a-4bf6-b0c6-d4947b3c79dd True
__add: ca97aaa0-5a12-4ae3-8929-c8fb57dd93d6 True
__add: 54826b22-38d6-4fb2-bad9-b7b93a3e9c5a True

[rayluo]

Thanks for the nice catch! I derived a fix a897af4 based on your finding. It is now fixed in the dev branch.