Skip to content

Commit

Permalink
Merge pull request #2196 from craddm/admin-group-error
Browse files Browse the repository at this point in the history
Raise exception when admin group name is not found
  • Loading branch information
craddm committed Sep 27, 2024
2 parents bfd2088 + 75cd69d commit 59bca6d
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 7 deletions.
12 changes: 8 additions & 4 deletions data_safe_haven/config/shm_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from typing import ClassVar, Self

from data_safe_haven.exceptions import DataSafeHavenMicrosoftGraphError
from data_safe_haven.external import AzureSdk
from data_safe_haven.serialisers import AzureSerialisableModel, ContextBase

Expand All @@ -27,10 +28,13 @@ def from_args(
) -> SHMConfig:
"""Construct an SHMConfig from arguments."""
azure_sdk = AzureSdk(subscription_name=context.subscription_name)
admin_group_id = (
azure_sdk.entra_directory.get_id_from_groupname(context.admin_group_name)
or "admin-group-id-not-found"
)
try:
admin_group_id = azure_sdk.entra_directory.validate_entra_group(
context.admin_group_name
)
except DataSafeHavenMicrosoftGraphError as exc:
msg = f"Admin group '{context.admin_group_name}' not found. Check the group name."
raise DataSafeHavenMicrosoftGraphError(msg) from exc
return SHMConfig.model_construct(
azure=ConfigSectionAzure.model_construct(
location=location,
Expand Down
2 changes: 1 addition & 1 deletion data_safe_haven/external/api/azure_sdk.py
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ def ensure_keyvault(
)
return key_vaults[0]
except AzureError as exc:
msg = f"Failed to create key vault {key_vault_name}."
msg = f"Failed to create key vault {key_vault_name}. Check if a key vault with the same name already exists in a deleted state."
raise DataSafeHavenAzureError(msg) from exc

def ensure_keyvault_key(
Expand Down
17 changes: 15 additions & 2 deletions data_safe_haven/external/api/graph_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ def add_user_to_group(
"""
try:
user_id = self.get_id_from_username(username)
group_id = self.get_id_from_groupname(group_name)
group_id = self.validate_entra_group(group_name)
json_response = self.http_get(
f"{self.base_endpoint}/groups/{group_id}/members",
).json()
Expand Down Expand Up @@ -515,6 +515,19 @@ def get_service_principal_by_name(
except (DataSafeHavenMicrosoftGraphError, StopIteration):
return None

def validate_entra_group(self, group_name: str) -> str:
"""
Ensure that an Entra group exists and return its ID
Raises:
DataSafeHavenMicrosoftGraphError if the group does not exist
"""
if group_id := self.get_id_from_groupname(group_name):
return group_id
else:
msg = f"Group '{group_name}' not found."
raise DataSafeHavenMicrosoftGraphError(msg)

def get_id_from_groupname(self, group_name: str) -> str | None:
try:
return str(
Expand Down Expand Up @@ -1015,7 +1028,7 @@ def remove_user_from_group(
"""
try:
user_id = self.get_id_from_username(username)
group_id = self.get_id_from_groupname(group_name)
group_id = self.validate_entra_group(group_name)
# Check whether user is in group
json_response = self.http_get(
f"{self.base_endpoint}/groups/{group_id}/members",
Expand Down

0 comments on commit 59bca6d

Please sign in to comment.