Skip to content
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

Merged
merged 12 commits into from
Feb 10, 2025

Conversation

buidav
Copy link
Collaborator

@buidav buidav commented Feb 6, 2025

🗣 Description

  • This PR updates our documentation relating to multiple small but related authentication things.
    • Adds documentation for details necessary for ScubaGear non-interactive auth in GCC High
    • 'Lowers' the necessary interactive auth role from SharePoint admin to Global Reader
    • 'Lowers' the MS Graph permission noted in SharePoint non-interactive auth to Organization.Read.All as noted from Create a JSON file to store permission scopes required for ScubaGear #1380
    • Adds an explanation for why we include permissions with bundled in write privileges to our non-interactive permissions table.
    • Also fixed some extra white space stuff. I saw from a custom extension.

💭 Motivation and context

Closes #265
Closes #1555
Closes #1556

🧪 Testing

  • Screenshots and explanations are located in the various issues being closed by this PR.
  • Testing was done by assigning SPs or creating new user accounts to authenticate into our Tests tenants.
  • Reviewers are not meant to recreate all of the experiments noted in those issues from scratch. Just check that the language added to the documentation is good and accurate.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • PR targets the correct parent branch (e.g., main or release-name) for merge.
  • Changes are limited to single related goals - eschew scope creep!
  • Changes are sized such that they do not touch excessive number of files.
  • All future TODOs are captured in issues, which are referenced in code comments.
  • These code changes follow the ScubaGear content style guide.
  • Related issues these changes resolve are linked preferably via closing keywords.
  • All relevant type-of-change labels added.
  • All relevant project fields are set.
  • All relevant repo and/or project documentation updated to reflect these changes.

✅ 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

  • Feature branch deleted after merge to clean up repository.
  • Verified that all checks pass on parent branch (e.g., main or release-name) after merge.

@buidav buidav self-assigned this Feb 6, 2025
@buidav buidav added the enhancement This issue or pull request will add new or improve existing functionality label Feb 6, 2025
@buidav buidav force-pushed the 265-update-permissions-documentation branch from e56fd57 to 2b4ac2f Compare February 6, 2025 23:01
@buidav buidav added this to the Lionfish milestone Feb 6, 2025
@buidav buidav marked this pull request as ready for review February 6, 2025 23:10
@buidav buidav added the documentation This issue or pull request improves or adds to documentation label Feb 6, 2025
Copy link
Collaborator

@james-garriss james-garriss left a 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.

docs/prerequisites/noninteractive.md Outdated Show resolved Hide resolved
docs/prerequisites/noninteractive.md Outdated Show resolved Hide resolved
docs/prerequisites/noninteractive.md Outdated Show resolved Hide resolved
docs/prerequisites/noninteractive.md Outdated Show resolved Hide resolved
docs/prerequisites/noninteractive.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@tkol2022 tkol2022 left a 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.

image

@buidav buidav force-pushed the 265-update-permissions-documentation branch from eaf510e to 0e69730 Compare February 7, 2025 22:34
@buidav buidav force-pushed the 265-update-permissions-documentation branch from 706099f to 32f0a15 Compare February 10, 2025 17:14
@buidav
Copy link
Collaborator Author

buidav commented Feb 10, 2025

@nanda-katikaneni this is ready to merge

@nanda-katikaneni nanda-katikaneni merged commit ab8fdee into main Feb 10, 2025
20 checks passed
@nanda-katikaneni nanda-katikaneni deleted the 265-update-permissions-documentation branch February 10, 2025 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation This issue or pull request improves or adds to documentation enhancement This issue or pull request will add new or improve existing functionality
Projects
None yet
4 participants