-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Support CAE in azure-identity #16323
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
# ------------------------------------ | ||
# Copyright (c) Microsoft Corporation. | ||
# Licensed under the MIT License. | ||
# ------------------------------------ | ||
import base64 | ||
import binascii | ||
import hashlib | ||
import json | ||
import re | ||
import time | ||
|
||
from azure_devtools.scenario_tests import RecordingProcessor | ||
import six | ||
|
||
|
||
SECRETS = frozenset({ | ||
"access_token", | ||
"client_secret", | ||
"code", | ||
"device_code", | ||
"message", | ||
"password", | ||
"refresh_token", | ||
"user_code", | ||
}) | ||
|
||
|
||
class RecordingRedactor(RecordingProcessor): | ||
"""Removes authentication secrets from recordings""" | ||
|
||
def process_request(self, request): | ||
# don't record the body because it probably contains secrets and is formed by msal anyway, | ||
# i.e. it isn't this library's responsibility | ||
request.body = None | ||
return request | ||
|
||
def process_response(self, response): | ||
try: | ||
body = json.loads(response["body"]["string"]) | ||
except (KeyError, ValueError): | ||
return response | ||
|
||
for field in body: | ||
if field in SECRETS: | ||
# record a hash of the secret instead of a simple replacement like "redacted" | ||
# because some tests (e.g. for CAE) require unique, consistent values | ||
digest = hashlib.sha256(six.ensure_binary(body[field])).digest() | ||
body[field] = six.ensure_str(binascii.hexlify(digest)) | ||
|
||
response["body"]["string"] = json.dumps(body) | ||
return response | ||
|
||
|
||
class IdTokenProcessor(RecordingProcessor): | ||
def process_response(self, response): | ||
"""Changes the "exp" claim of recorded id tokens to be in the future during playback | ||
|
||
This is necessary because msal always validates id tokens, raising an exception when they've expired. | ||
""" | ||
try: | ||
# decode the recorded token | ||
body = json.loads(six.ensure_str(response["body"]["string"])) | ||
header, encoded_payload, signed = body["id_token"].split(".") | ||
decoded_payload = base64.b64decode(encoded_payload + "=" * (4 - len(encoded_payload) % 4)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of curiosity, what are the "=" added onto the payload for? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A base64 encoded string should be padded with "=" to make its length divisible by 4. CPython's |
||
|
||
# set the token's expiry time to one hour from now | ||
payload = json.loads(six.ensure_str(decoded_payload)) | ||
payload["exp"] = int(time.time()) + 3600 | ||
|
||
# write the modified token to the response body | ||
new_payload = six.ensure_binary(json.dumps(payload)) | ||
body["id_token"] = ".".join((header, base64.b64encode(new_payload).decode("utf-8"), signed)) | ||
response["body"]["string"] = six.ensure_binary(json.dumps(body)) | ||
except KeyError: | ||
pass | ||
|
||
return response |
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.
The removal of
**kwargs
will make acquiring SSH certificate stop working asdata
is passed inkwargs
.Azure CLI is aiming to support acquiring SSH certificate for Service Principal as well (#16397).
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.
Summarizing offline discussion here, there are two orthogonal concerns: acquiring SSH certificates, and allowing applications to pass keyword arguments through get_token to MSAL. Neither is supported today. Acquiring SSH certificates may be supported in a future version. Passing MSAL arguments through get_token is possible in some cases but really isn't supportable because routine maintenance requires internal changes (e.g. #16449) that can break applications relying on this behavior. My goal with this change is to prevent applications from taking a dependency on such implementation details.