sadpandajoe commented on code in PR #45508: URL: https://github.com/apache/superset/pull/45508#discussion_r4366333675
########## superset/integrations/teams/api.py: ########## @@ -0,0 +1,128 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Independently authenticated, installation-only Teams bot callback.""" + +from __future__ import annotations + +import logging +from typing import TYPE_CHECKING + +from flask import Blueprint, current_app, Flask, request, Response +from sqlalchemy.exc import SQLAlchemyError +from sqlalchemy.orm import Session +from werkzeug.exceptions import RequestEntityTooLarge + +from superset.extensions import csrf, db, feature_flag_manager +from superset.integrations.teams.config import ( + parse_teams_config, + TeamsConfig, + TeamsConfigError, +) +from superset.integrations.teams.store import TeamsInstallationStore + +if TYPE_CHECKING: + from superset.integrations.teams.adapter import TeamsInstallationEvent + +_MAX_EVENT_BYTES = 256 * 1024 + +logger = logging.getLogger(__name__) +blueprint = Blueprint("teams", __name__) + + +def _check_event_body_size() -> None: + """Bound body reading and reject oversized input before JSON parsing.""" + configured_limit = request.max_content_length + limit = ( + min(_MAX_EVENT_BYTES, configured_limit) + if configured_limit is not None + else _MAX_EVENT_BYTES + ) + if request.content_length is not None and request.content_length > limit: + raise RequestEntityTooLarge() + # Werkzeug can truncate lengthless streams at the limit without raising. + # Read one lookahead byte through Flask's bounded stream before parsing JSON. + request.max_content_length = limit + 1 Review Comment: `Request.max_content_length` only became writable in Flask 3.1, but `pyproject.toml` still allows `flask>=2.2.5`. On an older supported Flask this assignment raises `AttributeError`, so every install/remove callback returns 500 before authentication and no installation is ever recorded, which leaves alerts undeliverable. Should the `teams` extra require Flask 3.1+, or should the bounded read avoid assigning to the request? ########## superset/commands/report/update.py: ########## @@ -496,6 +496,8 @@ def validate(self) -> None: # noqa: C901 self._properties.get("report_format", self._model.report_format), exceptions, ) + # Always validate the effective Teams recipients, even for metadata-only PUTs. + self.validate_teams_recipients(exceptions) Review Comment: This now runs for every PUT, including `{"active": false}`. If an operator disables `ALERT_REPORT_TEAMS`, clears the Teams config, or removes a destination key that an existing alert still references, the owner can no longer pause that alert: validation rejects the request and the schedule keeps running its queries and other deliveries. Should deactivation (and edits that leave recipients untouched) skip the Teams deliverability check? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
