potiuk commented on code in PR #52581:
URL: https://github.com/apache/airflow/pull/52581#discussion_r2176638912
##########
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")
except HTTPException:
raise
except InvalidAudienceError:
logger.warning("Invalid audience for the request", exc_info=True)
- abort(403)
+ raise HTTPException(status_code=status.HTTP_403_FORBIDDEN,
detail="Invalid audience")
Review Comment:
Same here and all other cases. It was pretty deliberate to just return 403
here.
We could actually make a comment here to metion that it's deliberate -
otherwise future contributors might try to "fix" it in the same way.
--
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]
