-
Notifications
You must be signed in to change notification settings - Fork 254
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
Update ScubaGear authentication documentation for additional GCC High, Defender, and SharePoint details #1557
Conversation
e56fd57
to
2b4ac2f
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.
Some opportunities for grammar and clarification improvements (see below), but nothing critical. Approved.
NOTE: I only reviewed the documentation. I did not attempt to run and test ScubaGear with the permission changes.
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.
I made some tweaks. I wanted to point out that the name of the respective M365 application is missing from the permissions table in the noninteractive.md file, but Microsoft is adding that with their upcoming PR so we don't have to worry about that right now. I put a screenshot of their table with the new API NAME and API APPID columns for reference.
eaf510e
to
0e69730
Compare
706099f
to
32f0a15
Compare
@nanda-katikaneni this is ready to merge |
🗣 Description
Organization.Read.All
as noted from Create a JSON file to store permission scopes required for ScubaGear #1380💭 Motivation and context
Closes #265
Closes #1555
Closes #1556
🧪 Testing
✅ Pre-approval checklist
singlerelated goals - eschew scope creep!✅ 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
N/A
Demonstrate changes to the team for questions and comments.(Note: Only required for issues of size
Medium
or larger)✅ Post-merge checklist