From 28cd5e565be9ca45bbbee8ad4b00963bd10ed699 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Mar 2026 14:54:17 +0000 Subject: [PATCH 1/2] Initial plan From 7d6128d78f7782d4a687b51220747714ab59df6d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Mar 2026 15:13:16 +0000 Subject: [PATCH 2/2] fix(api): add PUT /api/settings/{key} endpoint and shared credentials for Google/Microsoft social login MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add PUT /{key} endpoint to settings API with SettingValueUpdate body model (only requires value, key comes from URL path) — fixes 405 Method Not Allowed errors from the admin Connections wizard which used PUT to save settings - Fix grey toggles on /admin/connections: they appeared grey because all saves were silently failing with 405; now saves succeed and toggles reflect actual state - Add social_auth_google_use_global_credentials config field and auth.py logic to reuse google_drive_client_id/google_drive_client_secret for Google Sign-In - Add social_auth_microsoft_use_global_credentials config field and auth.py logic to reuse onedrive_client_id/onedrive_client_secret for Microsoft Sign-In - Also apply consistent both-field check for Dropbox global credentials fallback - Add settings metadata entries for the two new boolean settings - Add Google and Microsoft settings_keys to admin_connections service definitions - Add JS visibility toggle logic for Google/Microsoft credential fields in admin UI - Add 6 new unit/integration tests for PUT endpoint and SettingValueUpdate model Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> Agent-Logs-Url: https://github.com/christianlouis/DocuElevate/sessions/ac66041a-2cbd-4d90-8f8e-3588c629d4d8 --- app/api/settings.py | 62 ++++++++++++ app/auth.py | 30 ++++-- app/config.py | 22 ++++ app/utils/settings_service.py | 28 ++++++ app/views/settings.py | 2 + frontend/templates/admin_connections.html | 52 ++++++++++ tests/test_api_settings.py | 117 +++++++++++++++++++++- 7 files changed, 304 insertions(+), 9 deletions(-) diff --git a/app/api/settings.py b/app/api/settings.py index ffe1dfcf..dc560b8f 100644 --- a/app/api/settings.py +++ b/app/api/settings.py @@ -55,6 +55,12 @@ class SettingUpdate(BaseModel): value: Optional[str] = Field(None, description="Setting value (None to delete)") +class SettingValueUpdate(BaseModel): + """Model for updating a setting value by key (key is provided in the URL path).""" + + value: Optional[str] = Field(None, description="Setting value (None to delete)") + + class SettingResponse(BaseModel): """Model for setting response""" @@ -323,6 +329,62 @@ async def update_setting( ) +@router.put("/{key}") +async def put_setting( + key: str, + body: SettingValueUpdate, + request: Request, + db: DbSession, + admin: AdminUser, +): + """ + Update a specific setting by key (RESTful PUT). + + Accepts a body with only ``value``; the key is taken from the URL path. + This is the endpoint used by the admin Connections wizard. + Admin only. + """ + validate_setting_key(key) + try: + if body.value is not None: + is_valid, error_message = validate_setting_value(key, body.value) + if not is_valid: + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=error_message) + + user = request.session.get("user", {}) if hasattr(request, "session") else {} + changed_by = ( + user.get("preferred_username") or user.get("username") or user.get("email") or user.get("id") or "admin" + ) + + success = save_setting_to_db(db, key, body.value, changed_by=changed_by) + if not success: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Failed to save setting to database", + ) + + notify_settings_updated() + + metadata = get_setting_metadata(key) + restart_required = metadata.get("restart_required", False) + + return { + "success": True, + "message": f"Setting '{key}' updated successfully", + "restart_required": restart_required, + "key": key, + "value": body.value, + } + except HTTPException: + raise + except Exception as e: + logger.error(f"Error updating setting {key}: {e}") + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Failed to update setting: {key}", + ) + + @router.delete("/{key}") async def delete_setting(key: str, request: Request, db: DbSession, admin: AdminUser): """ diff --git a/app/auth.py b/app/auth.py index 112ad22c..46fd8234 100644 --- a/app/auth.py +++ b/app/auth.py @@ -58,11 +58,18 @@ if AUTH_ENABLED and settings.authentik_client_id and settings.authentik_client_s # --- Social Login Providers --------------------------------------------------- if AUTH_ENABLED and settings.social_auth_google_enabled: - if settings.social_auth_google_client_id and settings.social_auth_google_client_secret: + # Determine which credentials to use for Google social login + _google_client_id = settings.social_auth_google_client_id + _google_client_secret = settings.social_auth_google_client_secret + if settings.social_auth_google_use_global_credentials and not (_google_client_id and _google_client_secret): + _google_client_id = settings.google_drive_client_id + _google_client_secret = settings.google_drive_client_secret + + if _google_client_id and _google_client_secret: oauth.register( name="google", - client_id=settings.social_auth_google_client_id, - client_secret=settings.social_auth_google_client_secret, + client_id=_google_client_id, + client_secret=_google_client_secret, server_metadata_url="https://accounts.google.com/.well-known/openid-configuration", client_kwargs={"scope": "openid profile email"}, ) @@ -72,12 +79,21 @@ if AUTH_ENABLED and settings.social_auth_google_enabled: logger.warning("SOCIAL_AUTH_GOOGLE_ENABLED=true but client ID/secret not configured") if AUTH_ENABLED and settings.social_auth_microsoft_enabled: - if settings.social_auth_microsoft_client_id and settings.social_auth_microsoft_client_secret: + # Determine which credentials to use for Microsoft social login + _microsoft_client_id = settings.social_auth_microsoft_client_id + _microsoft_client_secret = settings.social_auth_microsoft_client_secret + if settings.social_auth_microsoft_use_global_credentials and not ( + _microsoft_client_id and _microsoft_client_secret + ): + _microsoft_client_id = settings.onedrive_client_id + _microsoft_client_secret = settings.onedrive_client_secret + + if _microsoft_client_id and _microsoft_client_secret: tenant = settings.social_auth_microsoft_tenant or "common" oauth.register( name="microsoft", - client_id=settings.social_auth_microsoft_client_id, - client_secret=settings.social_auth_microsoft_client_secret, + client_id=_microsoft_client_id, + client_secret=_microsoft_client_secret, server_metadata_url=f"https://login.microsoftonline.com/{tenant}/v2.0/.well-known/openid-configuration", client_kwargs={"scope": "openid profile email"}, ) @@ -133,7 +149,7 @@ if AUTH_ENABLED and settings.social_auth_dropbox_enabled: # Determine which credentials to use for Dropbox social login _dropbox_client_id = settings.social_auth_dropbox_client_id _dropbox_client_secret = settings.social_auth_dropbox_client_secret - if settings.social_auth_dropbox_use_global_credentials and not _dropbox_client_id: + if settings.social_auth_dropbox_use_global_credentials and not (_dropbox_client_id and _dropbox_client_secret): _dropbox_client_id = settings.dropbox_app_key _dropbox_client_secret = settings.dropbox_app_secret diff --git a/app/config.py b/app/config.py index cb5c207a..aabfdff0 100644 --- a/app/config.py +++ b/app/config.py @@ -343,6 +343,17 @@ class Settings(BaseSettings): social_auth_google_enabled: bool = False social_auth_google_client_id: Optional[str] = None social_auth_google_client_secret: Optional[str] = None + social_auth_google_use_global_credentials: bool = Field( + default=False, + description=( + "When True, Google social login uses the global GOOGLE_DRIVE_CLIENT_ID / " + "GOOGLE_DRIVE_CLIENT_SECRET credentials (the Google Drive OAuth integration credentials) " + "instead of requiring separate SOCIAL_AUTH_GOOGLE_CLIENT_ID / " + "SOCIAL_AUTH_GOOGLE_CLIENT_SECRET values. " + "Requires SOCIAL_AUTH_GOOGLE_ENABLED=True and the global Google Drive OAuth credentials to be set. " + "Default: False." + ), + ) # Microsoft OAuth2 (Azure AD / Microsoft Entra ID) social_auth_microsoft_enabled: bool = False @@ -357,6 +368,17 @@ class Settings(BaseSettings): "Default: common." ), ) + social_auth_microsoft_use_global_credentials: bool = Field( + default=False, + description=( + "When True, Microsoft social login uses the global ONEDRIVE_CLIENT_ID / " + "ONEDRIVE_CLIENT_SECRET credentials (the OneDrive integration credentials) " + "instead of requiring separate SOCIAL_AUTH_MICROSOFT_CLIENT_ID / " + "SOCIAL_AUTH_MICROSOFT_CLIENT_SECRET values. " + "Requires SOCIAL_AUTH_MICROSOFT_ENABLED=True and the global OneDrive credentials to be set. " + "Default: False." + ), + ) # Apple Sign-In social_auth_apple_enabled: bool = False diff --git a/app/utils/settings_service.py b/app/utils/settings_service.py index 600597db..e4961dab 100644 --- a/app/utils/settings_service.py +++ b/app/utils/settings_service.py @@ -303,6 +303,20 @@ SETTING_METADATA = { "required": False, "restart_required": True, }, + "social_auth_google_use_global_credentials": { + "category": "Social Login", + "description": ( + "When True, Google social login uses the global GOOGLE_DRIVE_CLIENT_ID / " + "GOOGLE_DRIVE_CLIENT_SECRET credentials (the Google Drive OAuth integration) " + "instead of requiring separate SOCIAL_AUTH_GOOGLE_CLIENT_ID / " + "SOCIAL_AUTH_GOOGLE_CLIENT_SECRET values. " + "Requires SOCIAL_AUTH_GOOGLE_ENABLED=True and global Google Drive OAuth credentials to be set." + ), + "type": "boolean", + "sensitive": False, + "required": False, + "restart_required": True, + }, "social_auth_microsoft_enabled": { "category": "Social Login", "description": ( @@ -345,6 +359,20 @@ SETTING_METADATA = { "required": False, "restart_required": True, }, + "social_auth_microsoft_use_global_credentials": { + "category": "Social Login", + "description": ( + "When True, Microsoft social login uses the global ONEDRIVE_CLIENT_ID / " + "ONEDRIVE_CLIENT_SECRET credentials (the OneDrive integration credentials) " + "instead of requiring separate SOCIAL_AUTH_MICROSOFT_CLIENT_ID / " + "SOCIAL_AUTH_MICROSOFT_CLIENT_SECRET values. " + "Requires SOCIAL_AUTH_MICROSOFT_ENABLED=True and global OneDrive credentials to be set." + ), + "type": "boolean", + "sensitive": False, + "required": False, + "restart_required": True, + }, "social_auth_apple_enabled": { "category": "Social Login", "description": ( diff --git a/app/views/settings.py b/app/views/settings.py index 25ccccea..a026a124 100644 --- a/app/views/settings.py +++ b/app/views/settings.py @@ -257,6 +257,7 @@ async def connections_page(request: Request, db: Session = Depends(get_db)): "social_auth_google_enabled", "social_auth_google_client_id", "social_auth_google_client_secret", + "social_auth_google_use_global_credentials", ], } ) @@ -292,6 +293,7 @@ async def connections_page(request: Request, db: Session = Depends(get_db)): "social_auth_microsoft_client_id", "social_auth_microsoft_client_secret", "social_auth_microsoft_tenant", + "social_auth_microsoft_use_global_credentials", ], } ) diff --git a/frontend/templates/admin_connections.html b/frontend/templates/admin_connections.html index 9472f5c6..3e9db7c2 100644 --- a/frontend/templates/admin_connections.html +++ b/frontend/templates/admin_connections.html @@ -229,6 +229,18 @@ function openServiceModal(serviceKey) { updateDropboxCredentialFieldsVisibility(this.checked); }); } + // When the Google "use global credentials" toggle changes, update visibility. + if (field.key === 'social_auth_google_use_global_credentials') { + checkbox.addEventListener('change', function() { + updateGoogleCredentialFieldsVisibility(this.checked); + }); + } + // When the Microsoft "use global credentials" toggle changes, update visibility. + if (field.key === 'social_auth_microsoft_use_global_credentials') { + checkbox.addEventListener('change', function() { + updateMicrosoftCredentialFieldsVisibility(this.checked); + }); + } } else { const input = document.createElement('input'); input.type = meta.sensitive ? 'password' : 'text'; @@ -266,6 +278,24 @@ function openServiceModal(serviceKey) { updateDropboxCredentialFieldsVisibility(isGlobal); } } + // Apply initial visibility for Google credential fields. + if (currentServiceKey === 'google') { + const useGlobalField = fields.find(function(f) { return f.key === 'social_auth_google_use_global_credentials'; }); + if (useGlobalField) { + const val = useGlobalField.value; + const isGlobal = val === true || val === 'true' || val === '1' || val === 'True'; + updateGoogleCredentialFieldsVisibility(isGlobal); + } + } + // Apply initial visibility for Microsoft credential fields. + if (currentServiceKey === 'microsoft') { + const useGlobalField = fields.find(function(f) { return f.key === 'social_auth_microsoft_use_global_credentials'; }); + if (useGlobalField) { + const val = useGlobalField.value; + const isGlobal = val === true || val === 'true' || val === '1' || val === 'True'; + updateMicrosoftCredentialFieldsVisibility(isGlobal); + } + } modal.classList.remove('hidden'); // Focus first input @@ -341,6 +371,28 @@ function updateDropboxCredentialFieldsVisibility(useGlobal) { }); } +// Show or hide the Google client-id / client-secret fields depending on +// whether the "use global credentials" toggle is enabled. +function updateGoogleCredentialFieldsVisibility(useGlobal) { + ['social_auth_google_client_id', 'social_auth_google_client_secret'].forEach(function(key) { + const el = document.querySelector('[data-field-key="' + key + '"]'); + if (el) { + el.style.display = useGlobal ? 'none' : ''; + } + }); +} + +// Show or hide the Microsoft client-id / client-secret fields depending on +// whether the "use global credentials" toggle is enabled. +function updateMicrosoftCredentialFieldsVisibility(useGlobal) { + ['social_auth_microsoft_client_id', 'social_auth_microsoft_client_secret'].forEach(function(key) { + const el = document.querySelector('[data-field-key="' + key + '"]'); + if (el) { + el.style.display = useGlobal ? 'none' : ''; + } + }); +} + function toggleSetting(key, value) { const csrfToken = document.querySelector('meta[name="csrf-token"]')?.content || ''; fetch('/api/settings/' + key, { diff --git a/tests/test_api_settings.py b/tests/test_api_settings.py index 8512387d..154f9bf7 100644 --- a/tests/test_api_settings.py +++ b/tests/test_api_settings.py @@ -180,6 +180,27 @@ class TestSettingModels: assert update.key == "test_key" assert update.value is None + def test_setting_value_update_model(self): + """Test SettingValueUpdate model (PUT body — no key required).""" + from app.api.settings import SettingValueUpdate + + body = SettingValueUpdate(value="test_value") + assert body.value == "test_value" + + def test_setting_value_update_model_with_none_value(self): + """Test SettingValueUpdate model accepts None value.""" + from app.api.settings import SettingValueUpdate + + body = SettingValueUpdate(value=None) + assert body.value is None + + def test_setting_value_update_model_defaults_to_none(self): + """Test SettingValueUpdate model value defaults to None when omitted.""" + from app.api.settings import SettingValueUpdate + + body = SettingValueUpdate() + assert body.value is None + def test_setting_response_model(self): """Test SettingResponse model.""" from app.api.settings import SettingResponse @@ -205,8 +226,100 @@ class TestSettingModels: assert "test_key" in response.db_settings -@pytest.mark.unit -class TestListCredentials: +@pytest.mark.integration +class TestPutSettingEndpoint: + """Tests for PUT /api/settings/{key} endpoint.""" + + def test_put_setting_requires_admin(self, client): + """Test PUT /settings/{key} requires admin access.""" + response = client.put("/api/settings/social_auth_dropbox_enabled", json={"value": "true"}) + assert response.status_code in [302, 401, 403] + + @patch("app.api.settings.notify_settings_updated") + @patch("app.api.settings.get_setting_metadata") + @patch("app.api.settings.validate_setting_value") + @patch("app.api.settings.save_setting_to_db") + def test_put_setting_saves_value(self, mock_save, mock_validate, mock_metadata, mock_notify, client): + """Test PUT /settings/{key} saves the value when authenticated as admin.""" + from app.api.settings import require_admin + from app.main import app as fastapi_app + + mock_validate.return_value = (True, None) + mock_save.return_value = True + mock_metadata.return_value = {"restart_required": True} + + def override_require_admin(): + return {"id": "admin", "is_admin": True, "preferred_username": "admin"} + + fastapi_app.dependency_overrides[require_admin] = override_require_admin + try: + response = client.put( + "/api/settings/social_auth_dropbox_enabled", + json={"value": "true"}, + ) + assert response.status_code == 200 + data = response.json() + assert data["success"] is True + assert data["key"] == "social_auth_dropbox_enabled" + assert data["value"] == "true" + assert data["restart_required"] is True + finally: + fastapi_app.dependency_overrides.pop(require_admin, None) + + @patch("app.api.settings.notify_settings_updated") + @patch("app.api.settings.get_setting_metadata") + @patch("app.api.settings.validate_setting_value") + @patch("app.api.settings.save_setting_to_db") + def test_put_setting_body_without_key_field_is_accepted( + self, mock_save, mock_validate, mock_metadata, mock_notify, client + ): + """Test PUT /settings/{key} body need not contain a key field.""" + from app.api.settings import require_admin + from app.main import app as fastapi_app + + mock_validate.return_value = (True, None) + mock_save.return_value = True + mock_metadata.return_value = {"restart_required": False} + + def override_require_admin(): + return {"id": "admin", "is_admin": True, "preferred_username": "admin"} + + fastapi_app.dependency_overrides[require_admin] = override_require_admin + try: + # Body only contains "value" — no "key" field (mirrors admin_connections.html behaviour) + response = client.put( + "/api/settings/social_auth_dropbox_use_global_credentials", + json={"value": "false"}, + ) + assert response.status_code == 200 + data = response.json() + assert data["success"] is True + finally: + fastapi_app.dependency_overrides.pop(require_admin, None) + + @patch("app.api.settings.validate_setting_value") + @patch("app.api.settings.get_setting_metadata") + def test_put_setting_returns_400_on_invalid_value(self, mock_metadata, mock_validate, client): + """Test PUT /settings/{key} returns 400 for invalid values.""" + from app.api.settings import require_admin + from app.main import app as fastapi_app + + mock_validate.return_value = (False, "Invalid boolean value") + mock_metadata.return_value = {"restart_required": False} + + def override_require_admin(): + return {"id": "admin", "is_admin": True} + + fastapi_app.dependency_overrides[require_admin] = override_require_admin + try: + response = client.put( + "/api/settings/social_auth_dropbox_enabled", + json={"value": "not_a_bool"}, + ) + assert response.status_code == 400 + finally: + fastapi_app.dependency_overrides.pop(require_admin, None) + """Tests for the list_credentials function (GET /api/settings/credentials).""" @patch("app.api.settings.get_all_settings_from_db")