jason810496 commented on code in PR #52581:
URL: https://github.com/apache/airflow/pull/52581#discussion_r2183170075
##########
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:
> What we can do in this case is to get a random id associated with such
request and report it (and log on the server side) - this makes it easier to
correlate client side requests with errors for legitimate errors.
Cool! Just like the request-id provide by AWS if I be banned by some IAM
rules.
I left it as responding 403 status code as same as previous implementation,
as it _seem_ no user raise the issue of hard to resolving the authz error of
log server.
--
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]
