From 0e2817565f4bdaee2ad0f4e1d22d0de330745fa3 Mon Sep 17 00:00:00 2001 From: Marcel van der Veldt Date: Tue, 9 Dec 2025 23:49:12 +0100 Subject: [PATCH] One more simplification of setup/onboard flow --- music_assistant/controllers/config.py | 28 + .../controllers/webserver/README.md | 14 +- .../controllers/webserver/api_docs.py | 3 +- music_assistant/controllers/webserver/auth.py | 70 ++- .../controllers/webserver/controller.py | 145 ++---- .../webserver/helpers/auth_middleware.py | 4 - .../webserver/remote_access/__init__.py | 2 +- .../webserver/remote_access/gateway.py | 4 +- .../controllers/webserver/websocket_client.py | 7 +- music_assistant/helpers/resources/setup.html | 487 +++--------------- tests/test_webserver_auth.py | 4 +- 11 files changed, 207 insertions(+), 561 deletions(-) diff --git a/music_assistant/controllers/config.py b/music_assistant/controllers/config.py index 5fb07266..e0d8ec20 100644 --- a/music_assistant/controllers/config.py +++ b/music_assistant/controllers/config.py @@ -117,6 +117,13 @@ class ConfigController: self._fernet = Fernet(fernet_key) config_entries.ENCRYPT_CALLBACK = self.encrypt_string config_entries.DECRYPT_CALLBACK = self.decrypt_string + if not self.onboard_done: + self.mass.register_api_command( + "config/onboard_complete", + self.set_onboard_complete, + authenticated=True, + alias=True, # hide from public API docs + ) LOGGER.debug("Started.") @property @@ -124,6 +131,25 @@ class ConfigController: """Return True if onboarding is done.""" return bool(self.get(CONF_ONBOARD_DONE, False)) + async def set_onboard_complete(self) -> None: + """ + Mark onboarding as complete. + + This is called by the frontend after the user has completed the onboarding wizard. + Only available when onboarding is not yet complete. + """ + if self.onboard_done: + msg = "Onboarding already completed" + raise InvalidDataError(msg) + + self.set(CONF_ONBOARD_DONE, True) + self.save(immediate=True) + LOGGER.info("Onboarding completed") + + # (re)Announce to Home Assistant if running as addon + if self.mass.running_as_hass_addon: + await self.mass.webserver._announce_to_homeassistant() + async def close(self) -> None: """Handle logic on server stop.""" if not self._timer_handle: @@ -1453,4 +1479,6 @@ class ConfigController: # loading failed, remove config self.remove(conf_key) raise + # mark onboard as complete as soon as the first provider is added + await self.set_onboard_complete() return config diff --git a/music_assistant/controllers/webserver/README.md b/music_assistant/controllers/webserver/README.md index bd3b0d43..4a7e707d 100644 --- a/music_assistant/controllers/webserver/README.md +++ b/music_assistant/controllers/webserver/README.md @@ -137,10 +137,20 @@ Manages individual WebSocket connections: ### First-Time Setup Flow -1. **Initial State**: No users exist, `onboard_done = false` +1. **Initial State**: No users exist 2. **Setup Required**: User is redirected to `/setup` 3. **Admin Creation**: User creates the first admin account with username/password -4. **Onboarding Complete**: `onboard_done` is set to `true` +4. **Setup completes** User gets redirected to the frontend +5. **Onboarding wizard** The frontend shows the onboarding wizard if it detects 'onboard_done' is False +4. **Onboarding Complete**: User completes onboarding and the `onboard_done` flag is set to `true` + +### First-Time Setup Flow when HA Ingress is used + +1. **Initial State**: No users exist +2. **Auto user creation**: User is auto created based on HA user +4. **Setup completes** User gets redirected to the frontend +5. **Onboarding wizard** The frontend shows the onboarding wizard if it detects 'onboard_done' is False +4. **Onboarding Complete**: User completes onboarding and the `onboard_done` flag is set to `true` ### Login Flow (Standard) diff --git a/music_assistant/controllers/webserver/api_docs.py b/music_assistant/controllers/webserver/api_docs.py index 0c3793d7..104cf1b3 100644 --- a/music_assistant/controllers/webserver/api_docs.py +++ b/music_assistant/controllers/webserver/api_docs.py @@ -733,8 +733,7 @@ def generate_openapi_spec( "summary": "Initial server setup", "description": ( "Handle initial setup of the Music Assistant server including creating " - "the first admin user. Only accessible when no users exist " - "(onboard_done=false)." + "the first admin user. Only accessible when no users exist." ), "operationId": "setup", "tags": ["Server"], diff --git a/music_assistant/controllers/webserver/auth.py b/music_assistant/controllers/webserver/auth.py index 51cdc91e..4c2d948e 100644 --- a/music_assistant/controllers/webserver/auth.py +++ b/music_assistant/controllers/webserver/auth.py @@ -25,7 +25,7 @@ from music_assistant_models.errors import ( from music_assistant.constants import ( CONF_AUTH_ALLOW_SELF_REGISTRATION, - CONF_ONBOARD_DONE, + DB_TABLE_PLAYLOG, HOMEASSISTANT_SYSTEM_USER, MASS_LOGGER_NAME, ) @@ -76,6 +76,7 @@ class AuthenticationManager: self.logger = LOGGER # Pending OAuth sessions for remote clients (session_id -> token) self._pending_oauth_sessions: dict[str, str | None] = {} + self._has_users: bool = False async def setup(self) -> None: """Initialize the authentication manager.""" @@ -94,18 +95,7 @@ class AuthenticationManager: # Setup login providers based on config await self._setup_login_providers(allow_self_registration) - # Migration: Reset onboard_done if no users exist - # This handles migration from existing setups (pre schema 28) - # where authentication was still optional - or if the auth db was reset. - # note that we do not do this if running as HA addon, because Ingress - # users are created automatically - if not self.mass.running_as_hass_addon and not await self.has_users(): - self.logger.warning( - "Authentication is mandatory but no users exist. " - "Resetting onboard_done to redirect to setup." - ) - self.mass.config.set(CONF_ONBOARD_DONE, False) - self.mass.config.save(immediate=True) + self._has_users = await self.database.get_count("users") > 0 self.logger.info( "Authentication manager initialized (providers=%d)", len(self.login_providers) @@ -116,6 +106,11 @@ class AuthenticationManager: if self.database: await self.database.close() + @property + def has_users(self) -> bool: + """Check if any users exist in the system.""" + return self._has_users + async def _setup_database(self) -> None: """Set up database schema and handle migrations.""" # Always create tables if they don't exist @@ -343,11 +338,6 @@ class AuthenticationManager: del self.login_providers["homeassistant"] self.logger.info("Home Assistant OAuth provider removed (HA provider not available)") - async def has_users(self) -> bool: - """Check if any users exist in the system.""" - count = await self.database.get_count("users") - return count > 0 - async def authenticate_with_credentials( self, provider_id: str, credentials: dict[str, Any] ) -> AuthResult: @@ -504,7 +494,10 @@ class AuthenticationManager: :param player_filter: Optional list of player IDs user has access to. :param provider_filter: Optional list of provider instance IDs user has access to. """ - username = normalize_username(username) + normalized_username = normalize_username(username) + + # Check if this is the first non-system user + is_first_user = not await self._has_non_system_users() user_id = secrets.token_urlsafe(32) created_at = utc() @@ -517,7 +510,7 @@ class AuthenticationManager: user_data = { "user_id": user_id, - "username": username, + "username": normalized_username, "role": role.value, "enabled": True, "created_at": created_at.isoformat(), @@ -530,9 +523,9 @@ class AuthenticationManager: await self.database.insert("users", user_data) - return User( + user = User( user_id=user_id, - username=username, + username=normalized_username, role=role, enabled=True, created_at=created_at, @@ -543,6 +536,39 @@ class AuthenticationManager: provider_filter=provider_filter, ) + # If this is the first non-system user, migrate playlog entries to them + if is_first_user and normalized_username != HOMEASSISTANT_SYSTEM_USER: + self._has_users = True + await self._migrate_playlog_to_first_user(user_id) + + return user + + async def _has_non_system_users(self) -> bool: + """Check if any non-system users exist.""" + user_rows = await self.database.get_rows("users", limit=10) + return any(row["username"] != HOMEASSISTANT_SYSTEM_USER for row in user_rows) + + async def _migrate_playlog_to_first_user(self, user_id: str) -> None: + """ + Migrate all existing playlog entries to the first user. + + This is called automatically when the first non-system user is created. + All existing playlog entries (which have NULL userid) will be updated + to belong to this first user. + + :param user_id: The user ID of the first user. + """ + try: + # Update all playlog entries with NULL userid to this user + await self.mass.music.database.execute( + f"UPDATE {DB_TABLE_PLAYLOG} SET userid = :userid WHERE userid IS NULL", + {"userid": user_id}, + ) + await self.mass.music.database.commit() + self.logger.info("Migrated existing playlog entries to first user: %s", user_id) + except Exception as err: + self.logger.warning("Failed to migrate playlog entries: %s", err) + async def get_homeassistant_system_user(self) -> User: """ Get or create the Home Assistant system user. diff --git a/music_assistant/controllers/webserver/controller.py b/music_assistant/controllers/webserver/controller.py index 1d831709..f4b491b8 100644 --- a/music_assistant/controllers/webserver/controller.py +++ b/music_assistant/controllers/webserver/controller.py @@ -10,7 +10,6 @@ from __future__ import annotations import asyncio import hashlib import html -import json import os import ssl import tempfile @@ -27,7 +26,7 @@ from aiohttp import ClientTimeout, web from mashumaro.exceptions import MissingField from music_assistant_frontend import where as locate_frontend from music_assistant_models.api import CommandMessage -from music_assistant_models.auth import AuthProviderType, UserRole +from music_assistant_models.auth import UserRole from music_assistant_models.config_entries import ConfigEntry, ConfigValueOption from music_assistant_models.enums import ConfigEntryType @@ -35,8 +34,6 @@ from music_assistant.constants import ( CONF_AUTH_ALLOW_SELF_REGISTRATION, CONF_BIND_IP, CONF_BIND_PORT, - CONF_ONBOARD_DONE, - DB_TABLE_PLAYLOG, RESOURCES_DIR, VERBOSE_LOG_LEVEL, ) @@ -278,7 +275,7 @@ class WebserverController(CoreController): bind_ip = cast("str | None", config.get_value(CONF_BIND_IP)) # print a big fat message in the log where the webserver is running # because this is a common source of issues for people with more complex setups - if not self.mass.config.onboard_done: + if not self.auth.has_users: self.logger.warning( "\n\n################################################################################\n" "### SETUP REQUIRED ###\n" @@ -294,6 +291,7 @@ class WebserverController(CoreController): ) else: self.logger.info( + "\n" "################################################################################\n" "\n" "Webserver available on: %s\n" @@ -466,8 +464,8 @@ class WebserverController(CoreController): async def _handle_jsonrpc_api_command(self, request: web.Request) -> web.Response: """Handle incoming JSON RPC API command.""" - # Block until onboarding is complete - if not self.mass.config.onboard_done: + # Fail early if we don't have any users yet + if not self.auth.has_users: return web.Response(status=503, text="Setup required") if not request.can_read_body: return web.Response(status=400, text="Body required") @@ -601,28 +599,17 @@ class WebserverController(CoreController): return await self._server.serve_static(swagger_html_path, request) async def _handle_index(self, request: web.Request) -> web.StreamResponse: - """Handle request for index page with onboarding check.""" + """Handle request for index page (Vue frontend).""" # If not yet onboarded, redirect to setup - if not self.mass.config.onboard_done or ( - not self.auth.has_users() and not is_request_from_ingress(request) - ): - # Preserve return_url parameter if present (will be passed back after setup) - return_url = request.query.get("return_url") - if return_url: - quoted_return = urllib.parse.quote(return_url, safe="") - setup_url = f"setup?return_url={quoted_return}" - else: - # No return URL - just redirect to setup without the parameter - setup_url = "setup" - return web.Response(status=302, headers={"Location": setup_url}) - + if not self.auth.has_users and not is_request_from_ingress(request): + return web.Response(status=302, headers={"Location": "setup"}) # Serve the Vue frontend index.html return await self._server.serve_static(self._index_path, request) async def _handle_login_page(self, request: web.Request) -> web.Response: """Handle request for login page (external client OAuth callback scenario).""" - # If not yet onboarded, redirect to setup - if not self.mass.config.onboard_done: + if not self.auth.has_users: + # not yet onboarded (no first admin user exists), redirect to setup return_url = request.query.get("return_url", "") device_name = request.query.get("device_name", "") setup_url = ( @@ -631,7 +618,6 @@ class WebserverController(CoreController): else "/setup" ) return web.Response(status=302, headers={"Location": setup_url}) - # Serve login page for external clients login_html_path = str(RESOURCES_DIR.joinpath("login.html")) async with aiofiles.open(login_html_path) as f: @@ -641,7 +627,7 @@ class WebserverController(CoreController): async def _handle_auth_login(self, request: web.Request) -> web.Response: """Handle login request.""" # Block until onboarding is complete - if not self.mass.config.onboard_done: + if not self.auth.has_users: return web.json_response( {"success": False, "error": "Setup required"}, status=403, @@ -946,32 +932,19 @@ class WebserverController(CoreController): else: return_url = "/login" # check if setup is already completed - if self.mass.config.onboard_done: + if self.auth.has_users: # Setup already completed, redirect to login (or provided return_url) return web.Response(status=302, headers={"Location": return_url}) - # Serve setup page + setup_html_path = str(RESOURCES_DIR.joinpath("setup.html")) async with aiofiles.open(setup_html_path) as f: html_content = await f.read() - # Check if this is from Ingress - if so, pre-fill user info - if is_request_from_ingress(request): - ingress_username = request.headers.get("X-Remote-User-Name", "") - ingress_display_name = request.headers.get("X-Remote-User-Display-Name", "") - - # Inject ingress user info into the page (use json.dumps to escape properly) - html_content = html_content.replace( - "const deviceName = urlParams.get('device_name');", - f"const deviceName = urlParams.get('device_name');\n" - f" const ingressUsername = {json.dumps(ingress_username)};\n" - f" const ingressDisplayName = {json.dumps(ingress_display_name)};", - ) - return web.Response(text=html_content, content_type="text/html") async def _handle_setup(self, request: web.Request) -> web.Response: - """Handle first-time setup request to create admin user.""" - if self.mass.config.onboard_done: + """Handle first-time setup request to create admin user (non-ingress only).""" + if self.auth.has_users: return web.json_response( {"success": False, "error": "Setup already completed"}, status=400 ) @@ -982,11 +955,6 @@ class WebserverController(CoreController): body = await request.json() username = body.get("username", "").strip() password = body.get("password", "") - from_ingress = body.get("from_ingress", False) - display_name = body.get("display_name") - - # Check if this is a valid ingress request (from_ingress flag + actual ingress headers) - is_valid_ingress = from_ingress and is_request_from_ingress(request) # Validation if not username or len(username) < 2: @@ -994,51 +962,29 @@ class WebserverController(CoreController): {"success": False, "error": "Username must be at least 2 characters"}, status=400 ) - # Password is only required for non-ingress users - if not is_valid_ingress and (not password or len(password) < 8): + if not password or len(password) < 8: return web.json_response( {"success": False, "error": "Password must be at least 8 characters"}, status=400 ) try: - if is_valid_ingress: - # Ingress setup: Create user with HA provider link only (no password required) - ha_user_id = request.headers.get("X-Remote-User-ID") - if not ha_user_id: - return web.json_response( - {"success": False, "error": "Missing Home Assistant user ID"}, status=400 - ) - - # Create admin user without password - user = await self.auth.create_user( - username=username, - role=UserRole.ADMIN, - display_name=display_name, + builtin_provider = self.auth.login_providers.get("builtin") + if not builtin_provider: + return web.json_response( + {"success": False, "error": "Built-in auth provider not available"}, + status=500, ) - # Link to Home Assistant provider - await self.auth.link_user_to_provider( - user, AuthProviderType.HOME_ASSISTANT, ha_user_id + if not isinstance(builtin_provider, BuiltinLoginProvider): + return web.json_response( + {"success": False, "error": "Built-in provider configuration error"}, + status=500, ) - else: - # Non-ingress setup: Create user with password - builtin_provider = self.auth.login_providers.get("builtin") - if not builtin_provider: - return web.json_response( - {"success": False, "error": "Built-in auth provider not available"}, - status=500, - ) - - if not isinstance(builtin_provider, BuiltinLoginProvider): - return web.json_response( - {"success": False, "error": "Built-in provider configuration error"}, - status=500, - ) - # Create admin user with password - user = await builtin_provider.create_user_with_password( - username, password, role=UserRole.ADMIN, display_name=display_name - ) + # Create admin user with password + user = await builtin_provider.create_user_with_password( + username, password, role=UserRole.ADMIN + ) # Create token for the new admin device_name = body.get( @@ -1046,19 +992,9 @@ class WebserverController(CoreController): ) token = await self.auth.create_token(user, device_name) - # Migrate existing playlog entries to this first user - await self._migrate_playlog_to_first_user(user.user_id) - - # Mark onboarding as complete - self.mass.config.set(CONF_ONBOARD_DONE, True) - self.mass.config.save(immediate=True) - self.logger.info("First admin user created: %s", username) - # Announce to Home Assistant now that onboarding is complete - if self.mass.running_as_hass_addon: - await self._announce_to_homeassistant() - + # Return token - frontend will complete onboarding via config/onboard_complete return web.json_response( { "success": True, @@ -1073,27 +1009,6 @@ class WebserverController(CoreController): {"success": False, "error": f"Setup failed: {e!s}"}, status=500 ) - async def _migrate_playlog_to_first_user(self, user_id: str) -> None: - """ - Migrate all existing playlog entries to the first user. - - This is called during onboarding when the first admin user is created. - All existing playlog entries (which have NULL userid) will be updated - to belong to this first user. - - :param user_id: The user ID of the first admin user. - """ - try: - # Update all playlog entries with NULL userid to this user - await self.mass.music.database.execute( - f"UPDATE {DB_TABLE_PLAYLOG} SET userid = :userid WHERE userid IS NULL", - {"userid": user_id}, - ) - await self.mass.music.database.commit() - self.logger.info("Migrated existing playlog entries to first user: %s", user_id) - except Exception as err: - self.logger.warning("Failed to migrate playlog entries: %s", err) - async def _announce_to_homeassistant(self) -> None: """Announce Music Assistant Ingress server to Home Assistant via Supervisor API.""" supervisor_token = os.environ["SUPERVISOR_TOKEN"] diff --git a/music_assistant/controllers/webserver/helpers/auth_middleware.py b/music_assistant/controllers/webserver/helpers/auth_middleware.py index 35589ba9..c2faff81 100644 --- a/music_assistant/controllers/webserver/helpers/auth_middleware.py +++ b/music_assistant/controllers/webserver/helpers/auth_middleware.py @@ -49,10 +49,6 @@ async def get_authenticated_user(request: web.Request) -> User | None: AuthProviderType.HOME_ASSISTANT, ingress_user_id ) if not user: - # Only auto-create users after onboarding is complete - if not mass.config.onboard_done: - return None - user = await mass.webserver.auth.get_user_by_username(ingress_username) if not user: role = await get_ha_user_role(mass, ingress_user_id) diff --git a/music_assistant/controllers/webserver/remote_access/__init__.py b/music_assistant/controllers/webserver/remote_access/__init__.py index ddc10b7e..3410b2b9 100644 --- a/music_assistant/controllers/webserver/remote_access/__init__.py +++ b/music_assistant/controllers/webserver/remote_access/__init__.py @@ -119,7 +119,7 @@ class RemoteAccessManager: self._using_ha_cloud = bool(ha_cloud_available and ice_servers) mode = "optimized" if self._using_ha_cloud else "basic" - self.logger.info("Starting remote access in %s mode (ID: %s)", mode, self._remote_id) + self.logger.info("Starting remote access in %s mode", mode) self.gateway = WebRTCGateway( http_session=self.mass.http_session, diff --git a/music_assistant/controllers/webserver/remote_access/gateway.py b/music_assistant/controllers/webserver/remote_access/gateway.py index 95108f92..5008188c 100644 --- a/music_assistant/controllers/webserver/remote_access/gateway.py +++ b/music_assistant/controllers/webserver/remote_access/gateway.py @@ -138,7 +138,7 @@ class WebRTCGateway: async def start(self) -> None: """Start the WebRTC Gateway.""" - self.logger.info("Starting WebRTC Gateway with Remote ID: %s", self.remote_id) + self.logger.info("Starting WebRTC Gateway") self.logger.debug("Signaling URL: %s", self.signaling_url) self.logger.debug("Local WS URL: %s", self.local_ws_url) self._running = True @@ -290,7 +290,7 @@ class WebRTCGateway: pass elif msg_type == "registered": self._is_connected = True - self.logger.info("Registered with signaling server as: %s", message.get("remoteId")) + self.logger.info("Registered with signaling server") elif msg_type == "error": error_msg = message.get("error") or message.get("message", "Unknown error") self.logger.error("Signaling server error: %s", error_msg) diff --git a/music_assistant/controllers/webserver/websocket_client.py b/music_assistant/controllers/webserver/websocket_client.py index 72e6ac39..083e8f8f 100644 --- a/music_assistant/controllers/webserver/websocket_client.py +++ b/music_assistant/controllers/webserver/websocket_client.py @@ -93,7 +93,7 @@ class WebsocketClientHandler: await self._send_message(server_info) # Block until onboarding is complete - if not self.mass.config.onboard_done and not self._is_ingress: + if not self.webserver.auth.has_users and not self._is_ingress: await self._send_message(ErrorResultMessage("connection", 503, "Setup required")) await wsock.close() return wsock @@ -368,11 +368,6 @@ class WebsocketClientHandler: ) if not user: - # Only auto-create users after onboarding is complete - if not self.mass.config.onboard_done: - self._logger.warning("Ingress connection attempted before setup") - return - # Check if a user with this username already exists user = await self.webserver.auth.get_user_by_username(ingress_username) diff --git a/music_assistant/helpers/resources/setup.html b/music_assistant/helpers/resources/setup.html index d849dcbd..9c8a3a22 100644 --- a/music_assistant/helpers/resources/setup.html +++ b/music_assistant/helpers/resources/setup.html @@ -3,7 +3,7 @@ - Music Assistant - Setup + Music Assistant - Create Admin Account @@ -293,135 +127,74 @@ Music Assistant

Music Assistant

-

Setup Wizard

+

Create Admin Account

- -
-
-
-
-
- - -
-
-

Welcome to Music Assistant!

-

Let's get you started with your personal music server. This setup wizard will guide you through the initial configuration.

-
- -
-

What you'll set up:

-

Step 1: Create your administrator account

-

Step 2: Complete the setup process

-
- -
- -
+
+

Welcome!

+

Create an administrator account to get started with Music Assistant.

- -
-
-

Create Administrator Account

-

Your admin credentials will be used to access the Music Assistant web interface and mobile apps.

-
- -