-
Notifications
You must be signed in to change notification settings - Fork 222
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
Create a JSON file to store permission scopes required for ScubaGear #1380
base: main
Are you sure you want to change the base?
Conversation
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.
- Can you move the file from the /Connection folder to a new /Permissions folder instead?
- I noticed you removed the ScubaGearSPPermissions node. In the future ScubaGear will have a utility script that helps users register a service principal to run the tool. Is the thought that our new utility script can just filter the GraphCmdLetPermissions node for entries that are of runtype = application and just use that list to create the needed permissions for the service principal in Entra Id?
- We need to add the following REST APIs as entries to the GraphCmdLetPermissions node along with their associated required permissions. Entra Id now includes direct calls to REST APIs without Cmdlets and therefore we need to ensure those permissions are represented.
- /beta/roleManagement/directory/roleEligibilityScheduleInstances
- /beta/roleManagement/directory/roleAssignmentScheduleInstances
- /beta/identityGovernance/privilegedAccess/group/eligibilityScheduleInstances
- /beta/privilegedAccess/aadGroups/resources
79d40ae
to
1218df3
Compare
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.
Minor recommendation for the format as we've had code trip over string vs list values before when it isn't consistent.
{ | ||
"description": "This section contains the needed Microsoft Graph permissions if aad is selected when running ScubaGear for the ProductName parameter", | ||
"url": "https://github.com/cisagov/ScubaGear/blob/main/docs/prerequisites/interactive.md#application-permissions", | ||
"Product": "aad", |
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 Product
key appears to accept values that are either a single string or a list of strings. Recommend making it require a list that can include a singleton or empty set to ease processing. I note that runType
and cmdlet
can similarly contain a single value, but are always represented as a list, so changing Product to match would provide more internal consistency.
1218df3
to
3dc451f
Compare
🗣 Description
Permissions file containing current SCuBAGear permissions mapping.
💭 Motivation and context
Added permissions file to allow centralized management of all SCuBAGear Microsoft Graph permissions.
🧪 Testing
Testing was conducted across commercial and gcchigh tenants. Created PowerShell code that imported the JSON file and made connections to Microsoft Graph utilizing the permission scopes defined under Permission within the ScubaGearGraphScopes section of the file.
✅ Pre-approval checklist
✅ Pre-merge checklist
PR passed smoke test check.
Feature branch has been rebased against changes from parent branch, as needed
Use
Rebase branch
button below or use this reference to rebase from the command line.Resolved all merge conflicts on branch
Notified merge coordinator that PR is ready for merge via comment mention
✅ Post-merge checklist