mirror of
https://github.com/langgenius/dify.git
synced 2026-06-07 16:32:01 +08:00
fix(security): enforce tenant scoping on app trace-config endpoints (GHSA-48xc-wmw8-3jr3) (#35793)
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Ido Shani <ido@zafran.io> Co-authored-by: -LAN- <laipz8200@outlook.com>
This commit is contained in:
parent
0d500e6965
commit
55d05fe52d
@ -3,7 +3,6 @@ import re
|
||||
import uuid
|
||||
from datetime import datetime
|
||||
from typing import Any, Literal
|
||||
from uuid import UUID
|
||||
|
||||
from flask import request
|
||||
from flask_restx import Resource
|
||||
@ -850,10 +849,11 @@ class AppTraceApi(Resource):
|
||||
@setup_required
|
||||
@login_required
|
||||
@account_initialization_required
|
||||
def get(self, app_id: UUID):
|
||||
@get_app_model
|
||||
def get(self, app_model):
|
||||
"""Get app trace"""
|
||||
with session_factory.create_session() as session:
|
||||
app_trace_config = OpsTraceManager.get_app_tracing_config(str(app_id), session)
|
||||
app_trace_config = OpsTraceManager.get_app_tracing_config(app_model.id, session)
|
||||
|
||||
return app_trace_config
|
||||
|
||||
@ -867,12 +867,13 @@ class AppTraceApi(Resource):
|
||||
@login_required
|
||||
@account_initialization_required
|
||||
@edit_permission_required
|
||||
def post(self, app_id: UUID):
|
||||
@get_app_model
|
||||
def post(self, app_model):
|
||||
# add app trace
|
||||
args = AppTracePayload.model_validate(console_ns.payload)
|
||||
|
||||
OpsTraceManager.update_app_tracing_config(
|
||||
app_id=str(app_id),
|
||||
app_id=app_model.id,
|
||||
enabled=args.enabled,
|
||||
tracing_provider=args.tracing_provider,
|
||||
)
|
||||
|
||||
@ -1,5 +1,4 @@
|
||||
from typing import Any
|
||||
from uuid import UUID
|
||||
|
||||
from flask import request
|
||||
from flask_restx import Resource, fields
|
||||
@ -9,8 +8,10 @@ from werkzeug.exceptions import BadRequest
|
||||
from controllers.common.schema import register_schema_models
|
||||
from controllers.console import console_ns
|
||||
from controllers.console.app.error import TracingConfigCheckError, TracingConfigIsExist, TracingConfigNotExist
|
||||
from controllers.console.app.wraps import get_app_model
|
||||
from controllers.console.wraps import account_initialization_required, setup_required
|
||||
from libs.login import login_required
|
||||
from models import App
|
||||
from services.ops_service import OpsService
|
||||
|
||||
|
||||
@ -43,11 +44,14 @@ class TraceAppConfigApi(Resource):
|
||||
@setup_required
|
||||
@login_required
|
||||
@account_initialization_required
|
||||
def get(self, app_id: UUID):
|
||||
args = TraceProviderQuery.model_validate(request.args.to_dict(flat=True))
|
||||
@get_app_model
|
||||
def get(self, app_model: App):
|
||||
args = TraceProviderQuery.model_validate(request.args.to_dict(flat=True)) # type: ignore
|
||||
|
||||
try:
|
||||
trace_config = OpsService.get_tracing_app_config(app_id=str(app_id), tracing_provider=args.tracing_provider)
|
||||
trace_config = OpsService.get_tracing_app_config(
|
||||
app_id=app_model.id, tracing_provider=args.tracing_provider
|
||||
)
|
||||
if not trace_config:
|
||||
return {"has_not_configured": True}
|
||||
return trace_config
|
||||
@ -65,13 +69,14 @@ class TraceAppConfigApi(Resource):
|
||||
@setup_required
|
||||
@login_required
|
||||
@account_initialization_required
|
||||
def post(self, app_id: UUID):
|
||||
@get_app_model
|
||||
def post(self, app_model: App):
|
||||
"""Create a new trace app configuration"""
|
||||
args = TraceConfigPayload.model_validate(console_ns.payload)
|
||||
|
||||
try:
|
||||
result = OpsService.create_tracing_app_config(
|
||||
app_id=str(app_id), tracing_provider=args.tracing_provider, tracing_config=args.tracing_config
|
||||
app_id=app_model.id, tracing_provider=args.tracing_provider, tracing_config=args.tracing_config
|
||||
)
|
||||
if not result:
|
||||
raise TracingConfigIsExist()
|
||||
@ -90,13 +95,14 @@ class TraceAppConfigApi(Resource):
|
||||
@setup_required
|
||||
@login_required
|
||||
@account_initialization_required
|
||||
def patch(self, app_id: UUID):
|
||||
@get_app_model
|
||||
def patch(self, app_model: App):
|
||||
"""Update an existing trace app configuration"""
|
||||
args = TraceConfigPayload.model_validate(console_ns.payload)
|
||||
|
||||
try:
|
||||
result = OpsService.update_tracing_app_config(
|
||||
app_id=str(app_id), tracing_provider=args.tracing_provider, tracing_config=args.tracing_config
|
||||
app_id=app_model.id, tracing_provider=args.tracing_provider, tracing_config=args.tracing_config
|
||||
)
|
||||
if not result:
|
||||
raise TracingConfigNotExist()
|
||||
@ -113,12 +119,13 @@ class TraceAppConfigApi(Resource):
|
||||
@setup_required
|
||||
@login_required
|
||||
@account_initialization_required
|
||||
def delete(self, app_id: UUID):
|
||||
@get_app_model
|
||||
def delete(self, app_model: App):
|
||||
"""Delete an existing trace app configuration"""
|
||||
args = TraceProviderQuery.model_validate(request.args.to_dict(flat=True))
|
||||
|
||||
try:
|
||||
result = OpsService.delete_tracing_app_config(app_id=str(app_id), tracing_provider=args.tracing_provider)
|
||||
result = OpsService.delete_tracing_app_config(app_id=app_model.id, tracing_provider=args.tracing_provider)
|
||||
if not result:
|
||||
raise TracingConfigNotExist()
|
||||
return {"result": "success"}, 204
|
||||
|
||||
@ -8,7 +8,9 @@ from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from flask import Flask
|
||||
from flask.testing import FlaskClient
|
||||
from pydantic import ValidationError
|
||||
from sqlalchemy.orm import Session
|
||||
from werkzeug.exceptions import BadRequest, NotFound
|
||||
|
||||
from controllers.console import console_ns
|
||||
@ -57,6 +59,12 @@ from controllers.console.app.workflow_app_log import WorkflowAppLogQuery
|
||||
from controllers.console.app.workflow_draft_variable import WorkflowDraftVariableUpdatePayload
|
||||
from controllers.console.app.workflow_statistic import WorkflowStatisticQuery
|
||||
from controllers.console.app.workflow_trigger import Parser, ParserEnable
|
||||
from models.model import AppMode
|
||||
from tests.test_containers_integration_tests.controllers.console.helpers import (
|
||||
authenticate_console_client,
|
||||
create_console_account_and_tenant,
|
||||
create_console_app,
|
||||
)
|
||||
|
||||
|
||||
def _unwrap(func):
|
||||
@ -270,6 +278,35 @@ class TestOpsTraceEndpoints:
|
||||
def app(self, flask_app_with_containers: Flask):
|
||||
return flask_app_with_containers
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"path_template",
|
||||
[
|
||||
"/console/api/apps/{app_id}/trace-config?tracing_provider=langfuse",
|
||||
"/console/api/apps/{app_id}/trace",
|
||||
],
|
||||
)
|
||||
def test_trace_endpoints_hide_apps_from_other_tenants(
|
||||
self,
|
||||
db_session_with_containers: Session,
|
||||
test_client_with_containers: FlaskClient,
|
||||
path_template: str,
|
||||
):
|
||||
account, _tenant = create_console_account_and_tenant(db_session_with_containers)
|
||||
foreign_account, foreign_tenant = create_console_account_and_tenant(db_session_with_containers)
|
||||
foreign_app = create_console_app(
|
||||
db_session_with_containers,
|
||||
tenant_id=foreign_tenant.id,
|
||||
account_id=foreign_account.id,
|
||||
mode=AppMode.CHAT,
|
||||
)
|
||||
|
||||
response = test_client_with_containers.get(
|
||||
path_template.format(app_id=foreign_app.id),
|
||||
headers=authenticate_console_client(test_client_with_containers, account),
|
||||
)
|
||||
|
||||
assert response.status_code == 404
|
||||
|
||||
def test_ops_trace_query_basic(self):
|
||||
query = TraceProviderQuery(tracing_provider="langfuse")
|
||||
assert query.tracing_provider == "langfuse"
|
||||
@ -289,7 +326,7 @@ class TestOpsTraceEndpoints:
|
||||
)
|
||||
|
||||
with app.test_request_context("/?tracing_provider=langfuse"):
|
||||
result = method(app_id="app-1")
|
||||
result = method(app_model=MagicMock(id="app-1"))
|
||||
|
||||
assert result == {"has_not_configured": True}
|
||||
|
||||
@ -308,7 +345,7 @@ class TestOpsTraceEndpoints:
|
||||
json={"tracing_provider": "langfuse", "tracing_config": {"api_key": "k"}},
|
||||
):
|
||||
with pytest.raises(BadRequest):
|
||||
method(app_id="app-1")
|
||||
method(app_model=MagicMock(id="app-1"))
|
||||
|
||||
def test_trace_app_config_delete_not_found(self, app: Flask, monkeypatch: pytest.MonkeyPatch):
|
||||
api = ops_trace_module.TraceAppConfigApi()
|
||||
@ -322,7 +359,7 @@ class TestOpsTraceEndpoints:
|
||||
|
||||
with app.test_request_context("/?tracing_provider=langfuse"):
|
||||
with pytest.raises(BadRequest):
|
||||
method(app_id="app-1")
|
||||
method(app_model=MagicMock(id="app-1"))
|
||||
|
||||
|
||||
class TestSiteEndpoints:
|
||||
|
||||
@ -6,17 +6,17 @@ import uuid
|
||||
from flask.testing import FlaskClient
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from configs import dify_config
|
||||
from constants import HEADER_NAME_CSRF_TOKEN
|
||||
from graphon.enums import WorkflowExecutionStatus
|
||||
from libs.datetime_utils import naive_utc_now
|
||||
from libs.token import _real_cookie_name, generate_csrf_token
|
||||
from models import Account, DifySetup, Tenant, TenantAccountJoin
|
||||
from models import Account, Tenant, TenantAccountJoin
|
||||
from models.account import AccountStatus, TenantAccountRole, TenantStatus
|
||||
from models.enums import ConversationFromSource, CreatorUserRole
|
||||
from models.model import App, AppMode, Conversation, Message
|
||||
from models.workflow import WorkflowRun
|
||||
from services.account_service import AccountService
|
||||
from tests.test_containers_integration_tests.controllers.console.helpers import ensure_dify_setup
|
||||
|
||||
|
||||
def _create_account_and_tenant(db_session: Session) -> tuple[Account, Tenant]:
|
||||
@ -47,9 +47,7 @@ def _create_account_and_tenant(db_session: Session) -> tuple[Account, Tenant]:
|
||||
account.timezone = "UTC"
|
||||
db_session.commit()
|
||||
|
||||
dify_setup = DifySetup(version=dify_config.project.version)
|
||||
db_session.add(dify_setup)
|
||||
db_session.commit()
|
||||
ensure_dify_setup(db_session)
|
||||
|
||||
return account, tenant
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user