Skip to content

Commit

Permalink
🐛 Ensure that GraphAPI token has correct permissions for granting app…
Browse files Browse the repository at this point in the history
…lication permissions
  • Loading branch information
jemrobinson committed Apr 10, 2024
1 parent 73d7dbf commit a091222
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 16 deletions.
7 changes: 6 additions & 1 deletion data_safe_haven/commands/deploy.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,12 @@ def sre(
# part of a Pulumi declarative command
graph_api = GraphApi(
tenant_id=config.shm.aad_tenant_id,
default_scopes=["Application.ReadWrite.All", "Group.ReadWrite.All"],
default_scopes=[
"Application.ReadWrite.All",
"AppRoleAssignment.ReadWrite.All",
"Directory.ReadWrite.All",
"Group.ReadWrite.All",
],
)

# Initialise Pulumi stack
Expand Down
43 changes: 39 additions & 4 deletions data_safe_haven/external/api/graph_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ class GraphApi:
"Global Administrator": "62e90394-69f5-4237-9190-012177145e10"
}
uuid_application: ClassVar[dict[str, str]] = {
"Application.ReadWrite.All": "1bfefb4e-e0b5-418b-a88f-73c46d2cc8e9",
"AppRoleAssignment.ReadWrite.All": "06b708a9-e830-4db3-a914-8e69da51d44f",
"Directory.Read.All": "7ab1d382-f21e-4acd-a863-ba3e13f7da61",
"Domain.Read.All": "dbb9058a-0e50-45d7-ae91-66909b5d4664",
"Group.Read.All": "5b567255-7703-4780-807c-7be8301ae99b",
Expand Down Expand Up @@ -597,6 +599,25 @@ def get_id_from_username(self, username: str) -> str | None:
except (DataSafeHavenMicrosoftGraphError, StopIteration):
return None

def grant_role_permissions(
self,
application_name: str,
*,
application_role_assignments: Sequence[str],
delegated_role_assignments: Sequence[str],
) -> None:
"""Grant admin permission for requested application roles"""
# Ensure that the application has a service principal
self.ensure_application_service_principal(application_name)

# Grant any requested application role permissions
for role_name in application_role_assignments:
self.grant_application_role_permissions(application_name, role_name)

# Grant any requested delegated role permissions
for role_name in delegated_role_assignments:
self.grant_delegated_role_permissions(application_name, role_name)

def grant_application_role_permissions(
self, application_name: str, application_role_name: str
) -> None:
Expand Down Expand Up @@ -1000,10 +1021,24 @@ def remove_user_from_group(
try:
user_id = self.get_id_from_username(username)
group_id = self.get_id_from_groupname(group_name)
# Attempt to remove user from group
self.http_delete(
f"{self.base_endpoint}/groups/{group_id}/members/{user_id}/$ref",
)
# Check whether user is in group
json_response = self.http_get(
f"{self.base_endpoint}/groups/{group_id}/members",
).json()
# Remove user from group if it is a member
if user_id in [
group_member["id"] for group_member in json_response["value"]
]:
self.http_delete(
f"{self.base_endpoint}/groups/{group_id}/members/{user_id}/$ref",
)
self.logger.info(
f"Removed [green]'{username}'[/] from group [green]'{group_name}'[/]."
)
else:
self.logger.info(
f"User [green]'{username}'[/] does not belong to group [green]'{group_name}'[/]."
)
except Exception as exc:
msg = (
f"Could not remove user '{username}' from group '{group_name}'.\n{exc}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,17 @@ def refresh(props: dict[str, Any]) -> dict[str, Any]:
):
outs["object_id"] = json_response["id"]
outs["application_id"] = json_response["appId"]

# Ensure that requested role permissions have been granted
graph_api.grant_role_permissions(
outs["application_name"],
application_role_assignments=props.get(
"application_role_assignments", []
),
delegated_role_assignments=props.get(
"delegated_role_assignments", []
),
)
return outs
except Exception as exc:
msg = f"Failed to refresh application [green]{props['application_name']}[/] in AzureAD.\n{exc}"
Expand Down Expand Up @@ -82,17 +93,14 @@ def create(self, props: dict[str, Any]) -> CreateResult:
outs["object_id"] = json_response["id"]
outs["application_id"] = json_response["appId"]

# Grant any requested application role permissions
for role_name in props.get("application_role_assignments", []):
graph_api.grant_application_role_permissions(
outs["application_name"], role_name
)

# Grant any requested delegated role permissions
for role_name in props.get("delegated_role_assignments", []):
graph_api.grant_delegated_role_permissions(
outs["application_name"], role_name
)
# Grant requested role permissions
graph_api.grant_role_permissions(
outs["application_name"],
application_role_assignments=props.get(
"application_role_assignments", []
),
delegated_role_assignments=props.get("delegated_role_assignments", []),
)

# Attach an application secret if requested
outs["application_secret"] = (
Expand Down

0 comments on commit a091222

Please sign in to comment.