Merge pull request #796 from christianlouis/copilot/fix-dropbox-authentication-toggle
fix: 405 on settings PUT + shared OAuth credentials for Google & Microsoft
This commit is contained in:
@@ -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):
|
||||
"""
|
||||
|
||||
+23
-7
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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": (
|
||||
|
||||
@@ -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",
|
||||
],
|
||||
}
|
||||
)
|
||||
|
||||
@@ -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, {
|
||||
|
||||
+115
-2
@@ -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")
|
||||
|
||||
Reference in New Issue
Block a user