You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix D&D menu items appearing for users without the required scope.
Code Changes
Require DISCOVERY_MONITOR_READ scope to show D&D nav menu items
Update roles in tests
Update detection & discovery cypress test to use the owner role
Steps to Confirm
Login as a internal respondent user
Check no Discovery items in the menu appear
Login as any other type of user
Check Detection & Discovery appears in the menu
Pre-Merge Checklist
Issue requirements met
All CI pipelines succeeded
CHANGELOG.md updated
Add a
db-migrationThis indicates that a change includes a database migration
label to the entry if your change includes a DB migration
Add a
high-riskThis issue suggests changes that have a high-probability of breaking existing code
label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
Updates unreleased work already in Changelog, no new entry necessary
Followup issues:
Followup issues created
No followup issues
Database migrations:
Ensure that your downrev is up to date with the latest revision on main
Ensure that your downgrade() migration is correct and works
If a downgrade migration is not possible for this change, please call this out in the PR description!
If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
The reason will be displayed to describe this comment to others. Learn more.
Good to add a check for this-- can we handle the case where a user has discovery_monitor:read but notdiscovery_monitor:update? This would allow a user with the former but not the latter to both view the pages and make changes, which seems potentially problematic.
Good to add a check for this-- can we handle the case where a user has discovery_monitor:read but notdiscovery_monitor:update? This would allow a user with the former but not the latter to both view the pages and make changes, which seems potentially problematic.
Yeah you're right. I'll create a follow up ticket for that so we don't forget. This is a quick fix I made because I saw the new user (internal respondent) is supposed to be very limited but it was seeing that option in the menu.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Failures
Flaky
Pending
Skipped
Passing
Description Of Changes
Fix D&D menu items appearing for users without the required scope.
Code Changes
Steps to Confirm
Pre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works