feat: add MDM authentication notification when navigation is blocked #7291
Conversation
- Show VSCode warning when users under MDM policy try to leave AccountView without auth - Add showMdmAuthRequiredNotification message type to WebviewMessage interface - Implement handler in webviewMessageHandler to display localized warning - Add 'Your organization requires authentication' translation in all 17 languages - Fix translation key path to use common:mdm.info.organization_requires_auth
dosubot
bot
added
size:S
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thank you for your contribution! I've reviewed the changes and the implementation looks solid. The MDM authentication notification feature correctly addresses the issue of providing user feedback when navigation is blocked. The localization is comprehensive across all supported languages, and the logic properly distinguishes between having no MDM policy vs. being non-compliant. I have some suggestions for improvement below.
| // If mdmCompliant is undefined or true, allow tab switching | ||
| if (mdmCompliant === false && newTab !== "account") { | ||
| // Notify the user that authentication is required by their organization | ||
| vscode.postMessage({ type: "showMdmAuthRequiredNotification" }) |
There was a problem hiding this comment.
Consider adding test coverage for this new notification feature. It would be valuable to verify:
- The notification is triggered when
mdmCompliant === falseand user tries to switch tabs - The correct message is displayed to the user
- The notification handler works correctly
Also, have you considered the UX for repeated attempts? Currently, the notification will show every time the user clicks a different tab. Would it make sense to throttle this or show it only once per session with a more persistent indicator?
| // If mdmCompliant is undefined or true, allow tab switching | ||
| if (mdmCompliant === false && newTab !== "account") { | ||
| // Notify the user that authentication is required by their organization | ||
| vscode.postMessage({ type: "showMdmAuthRequiredNotification" }) |
There was a problem hiding this comment.
Consider adding a telemetry event here to track when users encounter MDM blocks. This could help understand usage patterns and potential friction points:
|
|
||
| // Check MDM compliance and send user to account tab if not compliant | ||
| if (!this.checkMdmCompliance()) { | ||
| // Only redirect if there's an actual MDM policy requiring authentication |
There was a problem hiding this comment.
Great improvement to only check MDM compliance when there's actually an MDM policy! Consider adding JSDoc comments to document the three states of mdmCompliant:
undefined: No MDM policy existstrue: MDM policy exists and user is compliantfalse: MDM policy exists and user is non-compliant
This would help future developers understand the distinction.
| codebaseIndexSearchMinScore: codebaseIndexConfig?.codebaseIndexSearchMinScore, | ||
| }, | ||
| mdmCompliant: this.checkMdmCompliance(), | ||
| // Only set mdmCompliant if there's an actual MDM policy |
There was a problem hiding this comment.
Good consistency with the MDM compliance check. The comment clearly explains the three states, which is helpful.
| // Check MDM compliance before allowing tab switching | ||
| // Only check MDM compliance if mdmCompliant is explicitly false (meaning there's an MDM policy and user is non-compliant) | ||
| // If mdmCompliant is undefined or true, allow tab switching | ||
| if (mdmCompliant === false && newTab !== "account") { |
There was a problem hiding this comment.
Consider extracting this MDM check logic into a custom hook or utility function for better reusability and testability. For example:
This would make the logic easier to test and reuse if needed elsewhere.
github-project-automation
bot
moved this from PR [Needs Review]
to Done
in Roo Code Roadmap
Summary
This PR adds a VSCode notification to inform users when their organization's MDM policy requires authentication and they attempt to navigate away from the AccountView without being authenticated.
Changes
Core Functionality
App.tsxto send a notification message when MDM blocks navigation (whenmdmCompliant === falseand user tries to switch tabs)showMdmAuthRequiredNotificationmessage type to theWebviewMessageinterfacewebviewMessageHandler.tsto display a VSCode warning notificationLocalization
mdm.info.organization_requires_authwith value "Your organization requires authentication" to all 17 supported language filescommon:mdm.info.organization_requires_authfor proper message displayTesting
mdmCompliant === false)Related Issues
Fixes the issue where users under MDM policy had no feedback when navigation was blocked due to authentication requirements.
Important
Adds MDM authentication notification in
App.tsxto inform users when navigation is blocked due to policy requirements.App.tsx, added logic to block tab switching and notify users whenmdmCompliant === falseand they try to switch away fromaccounttab.showMdmAuthRequiredNotificationmessage type toWebviewMessageinterface.mdm.info.organization_requires_authto all supported language files for the message "Your organization requires authentication".mdmCompliant === false), attempts to navigate away fromAccountView, and is not authenticated.This description was created by
for 277e7bc. You can customize this summary. It will automatically update as commits are pushed.