hterik commented on code in PR #35085:
URL: https://github.com/apache/airflow/pull/35085#discussion_r1410850196
##########
airflow/auth/managers/fab/security_manager/override.py:
##########
@@ -1432,23 +1430,14 @@ def get_oauth_user_info(self, provider, resp):
"last_name": data.get("family_name", ""),
"email": data.get("email", ""),
}
- # for Azure AD Tenant. Azure OAuth response contains
- # JWT token which has user info.
- # JWT token needs to be base64 decoded.
- # https://docs.microsoft.com/en-us/azure/active-directory/develop/
- # active-directory-protocols-oauth-code
if provider == "azure":
- log.debug("Azure response received : %s", resp)
- id_token = resp["id_token"]
- log.debug(str(id_token))
- me = self._azure_jwt_token_parse(id_token)
- log.debug("Parse JWT token : %s", me)
+ me = self._decode_and_validate_azure_jwt(resp["id_token"])
+ log.debug("User info from Azure: %s", me)
+ #
https://learn.microsoft.com/en-us/azure/active-directory/develop/id-token-claims-reference#payload-claims
return {
- "name": me.get("name", ""),
- "email": me["upn"],
+ "email": me["email"],
Review Comment:
Is this correct? After pulling latest main, this fails. Even if `email` is
included in the scope claims it's not returned in the token, `upn` is included
as before. (This could of course be an integration issue with the IDP on our
end)
Either way, remember to include this in the release notes, because it may
require updated scope in the webserver_config by the administrator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
