potiuk commented on code in PR #52581:
URL: https://github.com/apache/airflow/pull/52581#discussion_r2176633136
##########
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")
Review Comment:
We do not actually want to provide more details to client on why we rejected
the request. This is an important security principle - never explain why you
fail to the client side if the reason is authentication problem, just return
"403" without any details - and log details on the server side.
Otherwise it might make easier for potential attacker to see what is wrong
and they can adjust their attack - including leveraging some of the "timing"
attacks for example to see if the tokens are partially matching and things like
that.
The lest we tell the client about reasons, the more secure we are.
--
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]
