From e00c47a56f996c718ca35292be3e21935dfb3536 Mon Sep 17 00:00:00 2001 From: inorishio Date: Mon, 22 Dec 2025 09:57:49 +0100 Subject: [PATCH] What? --- auth/azure_authenticator.py | 120 +++++++++++++++++++++------- auth/graph_authenticator.py | 46 +++++++---- main.py | 23 ++++-- ui/components/unified_dropdown.py | 125 ++++++++++++++++++++++++------ 4 files changed, 236 insertions(+), 78 deletions(-) diff --git a/auth/azure_authenticator.py b/auth/azure_authenticator.py index 038857c..a9c2c0f 100644 --- a/auth/azure_authenticator.py +++ b/auth/azure_authenticator.py @@ -21,12 +21,57 @@ class AzureAuthenticator: self.credential: Optional[InteractiveBrowserCredential] = None self.subscriptions: List[Dict[str, str]] = [] - def authenticate(self, credential: InteractiveBrowserCredential = None) -> bool: + def discover_tenant_id(self) -> tuple[str, InteractiveBrowserCredential, List[Dict[str, str]]]: """ - Authenticate to Azure. Can reuse credential from Graph authenticator. + Discover user's tenant ID by authenticating with organizations endpoint. + + Returns: + tuple: (tenant_id, credential, subscriptions) - The discovered tenant ID, credential to reuse, and subscription list + + Raises: + Exception: If no subscriptions found or authentication fails + """ + try: + # Create temporary credential with "organizations" for discovery + temp_credential = InteractiveBrowserCredential( + tenant_id="organizations", + additionally_allowed_tenants=["*"] + ) + + # List subscriptions to extract tenant + sub_client = SubscriptionClient(temp_credential) + subscriptions_list = list(sub_client.subscriptions.list()) + + if not subscriptions_list: + raise Exception("No Azure subscriptions found. Please ensure you have access to at least one subscription.") + + # Extract tenant ID from first subscription + tenant_id = subscriptions_list[0].tenant_id + print(f"Discovered Tenant ID: {tenant_id}") + + # Convert subscriptions to dict format + subscriptions = [ + { + 'id': sub.subscription_id, + 'name': sub.display_name + } + for sub in subscriptions_list + ] + + # Return tenant_id, credential, and subscriptions for reuse + return tenant_id, temp_credential, subscriptions + + except Exception as e: + raise Exception(f"Failed to discover tenant: {str(e)}") + + def authenticate(self, credential: InteractiveBrowserCredential = None, tenant_id: str = None, subscriptions: List[Dict[str, str]] = None) -> bool: + """ + Authenticate to Azure. Can reuse credential, use specific tenant, or discover tenant. Args: credential: Optional credential to reuse (from GraphAuthenticator) + tenant_id: Optional specific tenant ID to use + subscriptions: Optional pre-fetched subscriptions list (avoids re-listing) Returns: bool: True if authentication succeeded, False otherwise @@ -36,45 +81,60 @@ class AzureAuthenticator: """ try: if credential: - # Reuse credential from Graph authentication + # Reuse credential (recommended path to avoid multiple auth prompts) self.credential = credential else: - # Create new interactive browser credential with "organizations" tenant - # This allows the user to login with any organizational account - # additionally_allowed_tenants="*" allows acquiring tokens for any tenant + # Create credential with specific tenant ID + if not tenant_id: + raise Exception("Either credential or tenant_id must be provided") + + self.tenant_id = tenant_id + + # Create NEW credential with specific tenant_id (avoids redirect) self.credential = InteractiveBrowserCredential( - tenant_id="organizations", + tenant_id=tenant_id, additionally_allowed_tenants=["*"] ) - # List all subscriptions (this will use the cached token from Graph auth) - sub_client = SubscriptionClient(self.credential) - subscriptions_list = list(sub_client.subscriptions.list()) + # Use provided subscriptions or list them if not provided + if subscriptions: + # Reuse pre-fetched subscriptions (avoids duplicate API call) + self.subscriptions = subscriptions - # Extract tenant ID from the first subscription - if subscriptions_list: - # Get tenant ID from subscription (format: /subscriptions/{sub-id}) - # The tenant info is in the subscription object - first_sub = subscriptions_list[0] - self.tenant_id = first_sub.tenant_id if hasattr(first_sub, 'tenant_id') else None - if self.tenant_id: - print(f"Detected Tenant ID: {self.tenant_id}") + # Set tenant ID if provided + if tenant_id: + self.tenant_id = tenant_id - if subscriptions_list: - print(f"Successfully authenticated to Azure. Found {len(subscriptions_list)} subscription(s).") - - # Store subscriptions for later selection - self.subscriptions = [ - { - 'id': sub.subscription_id, - 'name': sub.display_name - } - for sub in subscriptions_list - ] + print(f"Using cached authentication. Found {len(subscriptions)} subscription(s).") return True + else: + # List all subscriptions (fallback for legacy code path) + sub_client = SubscriptionClient(self.credential) + subscriptions_list = list(sub_client.subscriptions.list()) - return False + # Extract tenant ID if not already set + if subscriptions_list and not self.tenant_id: + first_sub = subscriptions_list[0] + self.tenant_id = first_sub.tenant_id if hasattr(first_sub, 'tenant_id') else None + if self.tenant_id: + print(f"Detected Tenant ID: {self.tenant_id}") + + if subscriptions_list: + print(f"Successfully authenticated to Azure. Found {len(subscriptions_list)} subscription(s).") + + # Store subscriptions for later selection + self.subscriptions = [ + { + 'id': sub.subscription_id, + 'name': sub.display_name + } + for sub in subscriptions_list + ] + + return True + + return False except Exception as e: raise Exception(f"Azure authentication failed: {str(e)}") diff --git a/auth/graph_authenticator.py b/auth/graph_authenticator.py index b12b103..9484264 100644 --- a/auth/graph_authenticator.py +++ b/auth/graph_authenticator.py @@ -24,10 +24,15 @@ class GraphAuthenticator: self.credential: Optional[InteractiveBrowserCredential] = None self.client: Optional[GraphServiceClient] = None - async def authenticate(self) -> bool: + async def authenticate(self, tenant_id: str = None, credential: InteractiveBrowserCredential = None, skip_validation: bool = False) -> bool: """ Authenticate to Microsoft Graph using interactive browser login. + Args: + tenant_id: Optional specific tenant ID (recommended to avoid double auth) + credential: Optional credential to reuse (avoids creating new credential) + skip_validation: Skip the me.get() validation call (use when reusing credential to avoid extra auth) + Returns: bool: True if authentication succeeded, False otherwise @@ -35,14 +40,19 @@ class GraphAuthenticator: Exception: If authentication fails """ try: - # Create interactive browser credential - # Using "organizations" allows login with any organizational account - # additionally_allowed_tenants="*" allows acquiring tokens for any tenant (needed for Key Vault access) - self.credential = InteractiveBrowserCredential( - tenant_id="organizations", - client_id=self.client_id, - additionally_allowed_tenants=["*"] - ) + if credential: + # Reuse provided credential (recommended to avoid multiple auth prompts) + self.credential = credential + else: + # Use specific tenant if provided, otherwise fall back to "organizations" + auth_tenant = tenant_id if tenant_id else "organizations" + + # Create interactive browser credential + self.credential = InteractiveBrowserCredential( + tenant_id=auth_tenant, + client_id=self.client_id, + additionally_allowed_tenants=["*"] + ) # Define scopes for Microsoft Graph scopes = ['https://graph.microsoft.com/.default'] @@ -53,17 +63,19 @@ class GraphAuthenticator: scopes=scopes ) - # Authenticate to Graph API FIRST - # This triggers the initial browser auth for Graph scope - # Then Management API will use SSO (single sign-on) from this auth - me = await self.client.me.get() + # Validate authentication (unless skip_validation is True) + if not skip_validation: + me = await self.client.me.get() - if me: - print(f"Successfully authenticated as: {me.display_name} ({me.user_principal_name})") + if me: + print(f"Successfully authenticated as: {me.display_name} ({me.user_principal_name})") + return True + + return False + else: + print("Skipping Graph validation (using cached authentication)") return True - return False - except Exception as e: raise Exception(f"Graph authentication failed: {str(e)}") diff --git a/main.py b/main.py index 7106c4b..63e54b0 100644 --- a/main.py +++ b/main.py @@ -82,16 +82,23 @@ class Application: future.add_done_callback(on_complete) async def _authenticate(self): - """Perform authentication.""" + """Perform authentication with tenant discovery.""" try: - self.logger.info("Starting authentication...") + self.logger.info("Starting authentication with tenant discovery...") - # Authenticate to Microsoft Graph - await self.graph_auth.authenticate() + # PHASE 1: Discover tenant ID using "organizations" endpoint (single auth prompt) + self.logger.info("Discovering tenant ID...") + discovered_tenant_id, orgs_credential, subscriptions = self.azure_auth.discover_tenant_id() - # Authenticate to Azure (reuse credential) - credential = self.graph_auth.get_credential() - self.azure_auth.authenticate(credential) + self.logger.info(f"Discovered tenant: {discovered_tenant_id}") + + # PHASE 2: Reuse the "organizations" credential for Graph + # Skip validation to avoid triggering Graph API auth here - it will auth when first used + self.logger.info("Initializing Microsoft Graph with shared credential...") + await self.graph_auth.authenticate(credential=orgs_credential, skip_validation=True) + + self.logger.info("Initializing Azure with shared credential...") + self.azure_auth.authenticate(credential=orgs_credential, tenant_id=discovered_tenant_id, subscriptions=subscriptions) # Initialize Graph services graph_client = self.graph_auth.get_client() @@ -103,7 +110,7 @@ class Application: self.logger.info("Authentication successful") - # Load subscriptions (should use SSO from Graph auth above) + # Load subscriptions (already loaded during discover_tenant_id) await self._load_subscriptions() except Exception as e: diff --git a/ui/components/unified_dropdown.py b/ui/components/unified_dropdown.py index 34250bb..50ac583 100644 --- a/ui/components/unified_dropdown.py +++ b/ui/components/unified_dropdown.py @@ -69,6 +69,8 @@ class UnifiedDropdown(ctk.CTkFrame): # Popup window self.popup_window: Optional[tk.Toplevel] = None self.popup_frame: Optional[ctk.CTkScrollableFrame] = None + self._closing = False # Flag to prevent race conditions + self._popup_height = 0 # Store calculated popup height # Configure frame self.configure(corner_radius=10, border_width=2) @@ -118,6 +120,9 @@ class UnifiedDropdown(ctk.CTkFrame): self.dropdown_button.bind("", lambda e: self._open_dropdown()) self.dropdown_button.bind("", lambda e: self._toggle_dropdown()) + # Bind button click to ensure toggle works even when popup has focus + self.dropdown_button.bind("", self._on_button_click, add="+") + # Configure grid self.grid_columnconfigure(0, weight=1) @@ -170,8 +175,19 @@ class UnifiedDropdown(ctk.CTkFrame): else: return str(item) + def _on_button_click(self, event): + """Handle explicit button click to ensure toggle works.""" + # This is called on Button-1 event, which happens before the command callback + # We'll let the command callback (_toggle_dropdown) handle the actual toggle + # But we need to make sure the event propagates correctly + pass + def _toggle_dropdown(self): """Toggle dropdown popup visibility.""" + # Check if we're in the middle of closing (to prevent race conditions) + if self._closing: + return + if self.popup_window: self._close_dropdown() else: @@ -187,21 +203,42 @@ class UnifiedDropdown(ctk.CTkFrame): self.popup_window.wm_overrideredirect(True) # Remove window decorations self.popup_window.wm_attributes("-topmost", True) # Always on top + # Set background color to prevent white flash + appearance_mode = ctk.get_appearance_mode() + if appearance_mode == "Dark": + bg_color = "#2b2b2b" + else: + bg_color = "#dbdbdb" + self.popup_window.configure(bg=bg_color) + + self.popup_window.withdraw() # Hide initially to prevent flash in top-left corner + # Calculate dynamic height based on number of items - # Each item: 40px button + 4px padding = 44px per item - item_height = 44 - calculated_height = len(self.items) * item_height + 10 # +10 for padding + # Show first 10 items (or all if less than 10) + # Each item: 40px button + 2px borders (1px top + 1px bottom) + 4px padding (2px top + 2px bottom) = 46px per item + item_height = 46 + max_visible_items = 10 + items_to_show = min(len(self.items), max_visible_items) - # Use calculated height, but cap at max_dropdown_height - popup_height = min(calculated_height, self.max_dropdown_height) + # Height calculation: (items × 46px) + extra padding for frame borders + # Add 20px padding: 10px top + 10px bottom (5px pack padding + 5px extra for scrollbar/borders) + calculated_height = (items_to_show * item_height) + 20 + + self._popup_height = calculated_height + + # Set window size explicitly to ensure proper height + self.popup_window.wm_geometry(f"{self.button_width}x{self._popup_height}") + + # Create scrollable frame for items with fixed width + # Frame height = window height - padding (10px pack padding total) + frame_height = self._popup_height - 10 - # Create scrollable frame for items self.popup_frame = ctk.CTkScrollableFrame( self.popup_window, width=self.button_width - 20, - height=popup_height + height=frame_height ) - self.popup_frame.pack(fill="both", expand=True) + self.popup_frame.pack(fill="both", expand=False, padx=5, pady=5) # Configure scroll speed to match main window (40px per scroll unit = 2x faster) self.popup_frame._parent_canvas.configure(yscrollincrement=40) @@ -220,6 +257,14 @@ class UnifiedDropdown(ctk.CTkFrame): max_chars = 60 truncated = display_text if len(display_text) <= max_chars else display_text[:max_chars] + "..." + # Add extra padding for first and last items to prevent border cutoff + if idx == 0: + pady_val = (3, 2) # Extra padding at top + elif idx == len(self.items) - 1: + pady_val = (2, 3) # Extra padding at bottom + else: + pady_val = 2 + btn = ctk.CTkButton( self.popup_frame, text=truncated, @@ -232,7 +277,7 @@ class UnifiedDropdown(ctk.CTkFrame): hover_color=("gray70", "gray30"), anchor="w" ) - btn.grid(row=idx, column=0, padx=5, pady=2, sticky="ew") + btn.grid(row=idx, column=0, padx=5, pady=pady_val, sticky="ew") self.popup_frame.grid_columnconfigure(0, weight=1) # Add tooltip if text was truncated @@ -276,7 +321,7 @@ class UnifiedDropdown(ctk.CTkFrame): self.popup_window.bind("", self._navigate_home) self.popup_window.bind("", self._navigate_end) self.popup_window.bind("", self._confirm_selection) - self.popup_window.bind("", lambda e: self._close_dropdown()) + # Removed FocusOut binding - it causes race conditions with button clicks # Set focus to popup self.popup_window.focus_set() @@ -286,7 +331,10 @@ class UnifiedDropdown(ctk.CTkFrame): def _close_dropdown(self): """Close the dropdown popup.""" - if self.popup_window: + if self.popup_window and not self._closing: + # Set flag to prevent race conditions + self._closing = True + # Clean up tooltips for tooltip in self.item_tooltips: tooltip.destroy() @@ -310,22 +358,37 @@ class UnifiedDropdown(ctk.CTkFrame): self.popup_frame = None self.item_buttons.clear() + # Reset flag after a short delay to allow event handling to complete + self.after(100, lambda: setattr(self, '_closing', False)) + def _position_popup(self): """Position the popup window below the button.""" - # Update to get accurate dimensions + # Force complete update to ensure all widgets are rendered self.popup_window.update_idletasks() + self.dropdown_button.update_idletasks() - # Get button position - x = self.dropdown_button.winfo_rootx() - y = self.dropdown_button.winfo_rooty() + self.dropdown_button.winfo_height() + # Get button position (ensure widgets are fully laid out) + btn_x = self.dropdown_button.winfo_rootx() + btn_y = self.dropdown_button.winfo_rooty() + btn_height = self.dropdown_button.winfo_height() + + # Position BELOW the button (not in the middle) + x = btn_x + y = btn_y + btn_height + + # Fallback if coordinates are invalid (0,0 means not rendered yet) + if btn_x == 0 and btn_y == 0: + # Wait a bit and try again + self.popup_window.after(10, self._position_popup) + return # Get screen dimensions screen_width = self.winfo_screenwidth() screen_height = self.winfo_screenheight() - # Get popup dimensions - popup_width = self.popup_window.winfo_width() - popup_height = self.popup_window.winfo_height() + # Use stored dimensions (already calculated in _open_dropdown) + popup_width = self.button_width + popup_height = self._popup_height # Adjust if near screen edges if x + popup_width > screen_width: @@ -333,10 +396,14 @@ class UnifiedDropdown(ctk.CTkFrame): # Position above if not enough space below if y + popup_height > screen_height: - y = self.dropdown_button.winfo_rooty() - popup_height + y = btn_y - popup_height + # Set position explicitly (size already set in _open_dropdown) self.popup_window.wm_geometry(f"+{x}+{y}") + # Now show the popup (after positioning to prevent flash in top-left) + self.popup_window.deiconify() + def _check_click_outside(self, event): """Check if click was outside popup and close if so.""" if self.popup_window: @@ -346,8 +413,20 @@ class UnifiedDropdown(ctk.CTkFrame): popup_width = self.popup_window.winfo_width() popup_height = self.popup_window.winfo_height() - if not (popup_x <= x <= popup_x + popup_width and - popup_y <= y <= popup_y + popup_height): + # Check if click is inside popup + inside_popup = (popup_x <= x <= popup_x + popup_width and + popup_y <= y <= popup_y + popup_height) + + # Check if click is on the dropdown button (to allow toggling) + btn_x = self.dropdown_button.winfo_rootx() + btn_y = self.dropdown_button.winfo_rooty() + btn_width = self.dropdown_button.winfo_width() + btn_height = self.dropdown_button.winfo_height() + inside_button = (btn_x <= x <= btn_x + btn_width and + btn_y <= y <= btn_y + btn_height) + + # Close if click is outside both popup and button + if not inside_popup and not inside_button: self._close_dropdown() def _select_item(self, index: int, trigger_callback: bool = True, close_popup: bool = False): @@ -390,8 +469,8 @@ class UnifiedDropdown(ctk.CTkFrame): for i, btn in enumerate(self.item_buttons): if i == index: btn.configure( - fg_color=("gray75", "gray25"), - border_color="blue" + fg_color=("#3b8ed0", "#1f538d"), + border_color=("#3b8ed0", "#1f538d") ) else: btn.configure(