Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

SP Guest Account Access locked down by default and deploying user added as admin. #1425

Merged
merged 30 commits into from
Nov 5, 2021

Conversation

nharper285
Copy link
Contributor

@nharper285 nharper285 commented Nov 2, 2021

Summary of the Pull Request

What is this about?
For service principals created by the OneFuzz platform during deployment, we default to setting "User Assignment Required" to false. In the deployment script, this is controlled by the parameter app_role_assignment_required.

Change: set app_role_assignment_required to true by default, and add the user who deploys a given instance of OneFuzz as a pre-authorized user. The owner/deployer could then add additional users, as necessary. We could enable this functionality by including a deployment parameter that takes in an authenticated group, as chosen by the deploying user. Theoretically, this could just use the 'admins' parameter at deployment time to add users.

PR Checklist

  • Applies to work item: User Assignment Required Set By Default #1328
  • CLA signed. If not, go over here and sign the CLI.
  • Tests added/passed
  • Requires documentation to be updated
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Info on Pull Request

What does this include?
deploy.py: app_role_assignment_required set to true and includes new call to add_user()
registration.py: add_user function that takes in principal id(s) and add them to the SP.

Validation Steps Performed

How does someone test & validate?

  • User is added by default to SP and app_role_assignment_required set to true during deployment.
  • Test deploying from service principal
  • [Optional] List of admins specified by set_admins is added.

src/deployment/deploy.py Outdated Show resolved Hide resolved
src/deployment/deploy.py Outdated Show resolved Hide resolved
src/deployment/deploy.py Outdated Show resolved Hide resolved
Co-authored-by: Cheick Keita <kcheick@gmail.com>
src/deployment/deploy.py Outdated Show resolved Hide resolved
@nharper285 nharper285 merged commit de2b7ca into microsoft:main Nov 5, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants