potiuk commented on code in PR #52581:
URL: https://github.com/apache/airflow/pull/52581#discussion_r2176635164
##########
airflow-core/src/airflow/utils/serve_logs.py:
##########
@@ -43,74 +44,55 @@
logger = logging.getLogger(__name__)
-def create_app():
- flask_app = Flask(__name__, static_folder=None)
- leeway = conf.getint("webserver", "log_request_clock_grace", fallback=30)
- log_directory = os.path.expanduser(conf.get("logging", "BASE_LOG_FOLDER"))
- log_config_class = conf.get("logging", "logging_config_class")
- if log_config_class:
- logger.info("Detected user-defined logging config. Attempting to load
%s", log_config_class)
- try:
- logging_config = import_string(log_config_class)
- try:
- base_log_folder =
logging_config["handlers"]["task"]["base_log_folder"]
- except KeyError:
- base_log_folder = None
- if base_log_folder is not None:
- log_directory = base_log_folder
- logger.info(
- "Successfully imported user-defined logging config. Flask
App will serve log from %s",
- log_directory,
- )
- else:
- logger.warning(
- "User-defined logging config does not specify
'base_log_folder'. "
- "Flask App will use default log directory %s",
- base_log_folder,
- )
- except Exception as e:
- raise ImportError(f"Unable to load {log_config_class} due to
error: {e}")
- signer = JWTValidator(
- issuer=None,
- secret_key=get_signing_key("api", "secret_key"),
- algorithm="HS512",
- leeway=leeway,
- audience="task-instance-logs",
- )
+class JWTAuthStaticFiles(StaticFiles):
+ """StaticFiles with JWT authentication."""
- # Prevent direct access to the logs port
- @flask_app.before_request
- def validate_pre_signed_url():
+ # reference from
https://github.com/fastapi/fastapi/issues/858#issuecomment-876564020
+
+ def __init__(self, *args, **kwargs) -> None:
+ super().__init__(*args, **kwargs)
+
+ async def __call__(self, scope, receive, send) -> None:
+ request = Request(scope, receive)
+ await self.validate_jwt_token(request)
+ await super().__call__(scope, receive, send)
+
+ async def validate_jwt_token(self, request: Request):
+ # we get the signer from the app state instead of creating a new
instance for each request
+ signer = cast("JWTValidator", request.app.state.signer)
try:
auth = request.headers.get("Authorization")
if auth is None:
logger.warning("The Authorization header is missing: %s.",
request.headers)
- abort(403)
- payload = signer.validated_claims(auth)
+ raise HTTPException(
+ status_code=status.HTTP_403_FORBIDDEN,
detail="Authorization header missing"
+ )
+ payload = await signer.avalidated_claims(auth)
token_filename = payload.get("filename")
- request_filename = request.view_args["filename"]
+ # Extract filename from url path
+ request_filename = request.url.path.lstrip("/log/")
if token_filename is None:
logger.warning("The payload does not contain 'filename' key:
%s.", payload)
- abort(403)
+ raise HTTPException(status_code=status.HTTP_403_FORBIDDEN,
detail="Invalid token payload")
if token_filename != request_filename:
logger.warning(
"The payload log_relative_path key is different than the
one in token:"
"Request path: %s. Token path: %s.",
request_filename,
token_filename,
)
- abort(403)
+ raise HTTPException(status_code=status.HTTP_403_FORBIDDEN,
detail="Token filename mismatch")
Review Comment:
Yeah. This one for example is pretty informative to the attacker - so we
should just return 403 and keep all the details in the server log for
diagnostics.
--
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]
