Repository navigation
Encrypting the Managed CleanRoom token cache file - #10303
Conversation
|
Hi DevBaburaj, |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
|
Managed CleanRoom |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟡 Changes recommended
The new encrypted cache handling has likely runtime/upgrade-breakage issues (missing/failed msal_extensions path and plaintext-cache migration) that should be addressed before release.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the managedcleanroom Azure CLI extension to store its MSAL token cache using encrypted persistence (via msal-extensions), and bumps the extension version/history accordingly.
Changes:
- Encrypt MSAL token cache load/save using
msal_extensionspersistence. - Add a
build_persistence()helper to select encrypted persistence (with optional plaintext fallback). - Bump extension version to
1.0.0b8and document the change inHISTORY.rst.
File summaries
| File | Description |
|---|---|
src/managedcleanroom/azext_managedcleanroom/_msal_auth.py |
Switches token cache persistence to encrypted storage and introduces a persistence factory helper. |
src/managedcleanroom/setup.py |
Version bump to 1.0.0b8. |
src/managedcleanroom/HISTORY.rst |
Adds release notes for 1.0.0b8 describing token cache encryption. |
Review details
Suppressed comments (1)
src/managedcleanroom/azext_managedcleanroom/_msal_auth.py:158
build_persistencecurrently importsmsal_extensionsoutside thetry, sofallback_to_plaintextnever takes effect for the most likely failure mode (ImportError whenmsal-extensionsisn't installed). In addition,except:is too broad and the warning doesn't capture the underlying exception, making failures hard to diagnose. Consider handlingImportErrorexplicitly, catchingExceptioninstead of bare-except, and loggingexc_infofor troubleshooting.
from msal_extensions import build_encrypted_persistence, FilePersistence
try:
return build_encrypted_persistence(msal_token_cache_file)
except: # pylint: disable=bare-except
if not fallback_to_plaintext:
raise
logger.warning("Encryption unavailable. Opting in to plain text.")
return FilePersistence(msal_token_cache_file)
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
|
[Release] Update index.json for extension [ managedcleanroom-1.0.0b8 ] : https://dev.azure.com/msazure/One/_build/results?buildId=180459932&view=results |
🤖 PR Validation — ️✔️ All clear
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally? (pip install azdevrequired)For new extensions:
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update
src/index.jsonautomatically.You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify
src/index.json.