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
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
4f2287b
Service Principal locked down by default and deploying user added.
nharper32 Nov 2, 2021
bebc2f5
Fixing call to client.
nharper32 Nov 2, 2021
f6eb239
Adding multiple users.
nharper32 Nov 2, 2021
aa710cf
Retrieving SP id.
nharper32 Nov 2, 2021
31dcd1d
Refactorin gcall.
nharper32 Nov 2, 2021
259ddca
Getting closer.
nharper32 Nov 2, 2021
eb84d0b
Can retrieve object_id from create
nharper32 Nov 3, 2021
dd727f6
New retrieve sp functionality.
nharper285 Nov 3, 2021
59ece5d
mypy fixes.
nharper285 Nov 3, 2021
aa93fc9
Separating functionality into new function.
nharper285 Nov 3, 2021
f483460
mypy errors.
nharper32 Nov 3, 2021
d0fbae8
Merge branch 'main' into user/noharper/guest-access-sp
nharper285 Nov 3, 2021
f92f642
Logic for updating appRoleAssignemntRequired param.
nharper32 Nov 4, 2021
b735365
Changing to patch.
nharper32 Nov 4, 2021
10beca1
Updating to Patch.
nharper32 Nov 4, 2021
d3c9906
Fixing bug.
nharper32 Nov 4, 2021
3edb282
Fixing bad assignment.
nharper32 Nov 4, 2021
9cd5c22
Responding to comments.
nharper32 Nov 4, 2021
f1e87f8
Merge branch 'main' into user/noharper/guest-access-sp
nharper285 Nov 4, 2021
a7d9647
Removing functionality for updating setting.
nharper32 Nov 4, 2021
e589355
Merge branch 'user/noharper/guest-access-sp' of https://github.com/nh…
nharper32 Nov 4, 2021
6257a32
Update src/deployment/deploy.py
nharper285 Nov 4, 2021
f1a85c1
UPdating error message.
nharper32 Nov 4, 2021
257423e
Merge conflict.
nharper32 Nov 4, 2021
420973c
Retriggering?
nharper285 Nov 5, 2021
f4deda1
Trying to emulate new file structure.
nharper285 Nov 5, 2021
94a125d
Merge branch 'main' into user/noharper/guest-access-sp
nharper285 Nov 5, 2021
1a08f5a
Fixing lint issues.
nharper32 Nov 5, 2021
69f1e00
Fixing create sp.
nharper32 Nov 5, 2021
a7ed278
Merge branch 'main' into user/noharper/guest-access-sp
nharper285 Nov 5, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 46 additions & 5 deletions src/deployment/deploy.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,12 @@
GraphQueryError,
OnefuzzAppRole,
add_application_password,
add_user,
assign_instance_app_role,
authorize_application,
get_application,
get_service_principal,
get_signed_in_user,
get_tenant_id,
query_microsoft_graph,
register_application,
Expand Down Expand Up @@ -291,7 +294,6 @@ def setup_rbac(self) -> None:
display_name=self.application_name,
subscription_id=self.get_subscription_id(),
)

app_roles = [
{
"allowedMemberTypes": ["Application"],
Expand All @@ -309,6 +311,14 @@ def setup_rbac(self) -> None:
"isEnabled": True,
"value": OnefuzzAppRole.ManagedNode.value,
},
{
"allowedMemberTypes": ["User"],
"description": "Allows user access from the CLI.",
"displayName": OnefuzzAppRole.UserAssignment.value,
"id": str(uuid.uuid4()),
"isEnabled": True,
"value": OnefuzzAppRole.UserAssignment.value,
},
]

if not app:
Expand Down Expand Up @@ -363,7 +373,7 @@ def setup_rbac(self) -> None:

service_principal_params = {
"accountEnabled": True,
"appRoleAssignmentRequired": False,
"appRoleAssignmentRequired": True,
"servicePrincipalType": "Application",
"appId": app["appId"],
}
Expand All @@ -378,7 +388,6 @@ def try_sp_create() -> None:
body=service_principal_params,
subscription=self.get_subscription_id(),
)
return
except GraphQueryError as err:
# work around timing issue when creating service principal
# https://github.com/Azure/azure-cli/issues/14767
Expand Down Expand Up @@ -422,11 +431,10 @@ def try_sp_create() -> None:
# this is a requirement to update the application roles
for role in app["appRoles"]:
role["isEnabled"] = False

query_microsoft_graph(
method="PATCH",
resource=f"applications/{app['id']}",
body={"appRoles": app["AppRoles"]},
body={"appRoles": app["appRoles"]},
subscription=self.get_subscription_id(),
)

Expand Down Expand Up @@ -594,6 +602,38 @@ def assign_scaleset_identity_role(self) -> None:
OnefuzzAppRole.ManagedNode,
)

def assign_user_access(self) -> None:
logger.info("assinging user access to service principal")
app = get_application(
display_name=self.application_name,
subscription_id=self.get_subscription_id(),
)
user = get_signed_in_user(self.subscription_id)

if app:
sp = get_service_principal(app["appId"], self.subscription_id)
# Update appRoleAssignmentRequired if necessary
if not sp["appRoleAssignmentRequired"]:
chkeita marked this conversation as resolved.
Show resolved Hide resolved
logger.warning(
"The Service Principal does not have 'appRoleAssignmentRequired' set to True."
nharper285 marked this conversation as resolved.
Show resolved Hide resolved
+ " This means that any authenticated user can access the principal."
nharper285 marked this conversation as resolved.
Show resolved Hide resolved
+ " If you are manually upgrading an instance, and want this value set to 'True',"
+ " you must manually update the setting."
)

# Assign Roles and Add Users
roles = [
x["id"]
for x in app["appRoles"]
if x["displayName"] == OnefuzzAppRole.UserAssignment.value
]
users = [user["id"]]
if self.admins:
admins_str = [str(x) for x in self.admins]
users += admins_str
for user_id in users:
add_user(sp["id"], user_id, roles[0])

def apply_migrations(self) -> None:
logger.info("applying database migrations")
name = self.results["deploy"]["func-name"]["value"]
Expand Down Expand Up @@ -951,6 +991,7 @@ def main() -> None:
("rbac", Client.setup_rbac),
("arm", Client.deploy_template),
("assign_scaleset_identity_role", Client.assign_scaleset_identity_role),
("assign_user_access", Client.assign_user_access),
]

full_deployment_states = rbac_only_states + [
Expand Down
79 changes: 79 additions & 0 deletions src/deployment/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ class ApplicationInfo(NamedTuple):
class OnefuzzAppRole(Enum):
ManagedNode = "ManagedNode"
CliClient = "CliClient"
UserAssignment = "UserAssignment"


def register_application(
Expand Down Expand Up @@ -646,6 +647,84 @@ def set_app_audience(
raise Exception(err_str)


def get_signed_in_user(subscription_id: Optional[str]) -> Any:
# Get principalId by retrieving owner for SP
try:
app = query_microsoft_graph(
method="GET",
resource="me/",
subscription=subscription_id,
)
return app

except GraphQueryError:
query = (
"az rest --method post --url "
"https://graph.microsoft.com/v1.0/me "
'--headers "Content-Type"=application/json'
)
logger.warning(
"execute the following query in the azure portal bash shell and "
"run deploy.py again : \n%s",
query,
)
err_str = "Unable to retrieve signed-in user via Microsoft Graph Query API. \n"
raise Exception(err_str)


def get_service_principal(app_id: str, subscription_id: Optional[str]) -> Any:
try:
service_principals = query_microsoft_graph_list(
method="GET",
resource="servicePrincipals",
params={"$filter": f"appId eq '{app_id}'"},
subscription=subscription_id,
)
if len(service_principals) != 0:
return service_principals[0]
else:
raise GraphQueryError(
f"Could not retrieve any service principals for App Id: {app_id}", 400
)

except GraphQueryError:
err_str = "Unable to add retrieve SP using Microsoft Graph Query API. \n"
raise Exception(err_str)


def add_user(object_id: str, principal_id: str, role_id: str) -> None:
# Get principalId by retrieving owner for SP
# need to add users with proper role assignment
http_body = {
"principalId": principal_id,
"resourceId": object_id,
"appRoleId": role_id,
}
try:
query_microsoft_graph(
method="POST",
resource="users/%s/appRoleAssignments" % principal_id,
body=http_body,
)
except GraphQueryError as ex:
if "Permission being assigned already exists" not in ex.message:
query = (
"az rest --method post --url "
"https://graph.microsoft.com/v1.0/users/%s/appRoleAssignments "
"--body '%s' --headers \"Content-Type\"=application/json"
% (principal_id, http_body)
)
logger.warning(
"execute the following query in the azure portal bash shell and "
"run deploy.py again : \n%s",
query,
)
err_str = "Unable to add user to SP using Microsoft Graph Query API. \n"
raise Exception(err_str)
else:
logger.info("User already assigned to application.")


def main() -> None:
formatter = argparse.ArgumentDefaultsHelpFormatter

Expand Down