-
Notifications
You must be signed in to change notification settings - Fork 159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(BA-524): Deprecate X-BackendAI-SSO
header for pipeline service authentication
#3353
Changes from 5 commits
5cab78a
424ec0b
1b9f992
0ef6f44
48748ab
98649d4
d9b5e97
fe42788
a804913
143cc14
83e6e64
91f7220
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Deprecate the JWT-based `X-BackendAI-SSO` header to reduce complexity in authentication process for the pipeline service |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,11 +5,9 @@ | |
import json | ||
import logging | ||
import random | ||
from datetime import datetime, timedelta | ||
from typing import Optional, Tuple, Union, cast | ||
|
||
import aiohttp | ||
import jwt | ||
from aiohttp import web | ||
from Crypto.Cipher import AES | ||
from Crypto.Util.Padding import unpad | ||
|
@@ -164,7 +162,10 @@ async def web_handler(request: web.Request, *, is_anonymous=False) -> web.Stream | |
raise RuntimeError("'pipeline' config must be set to handle pipeline requests.") | ||
endpoint = pipeline_config["endpoint"] | ||
log.info(f"WEB_HANDLER: {request.path} -> {endpoint}/{real_path}") | ||
api_session = await asyncio.shield(get_api_session(request, endpoint)) | ||
if real_path.rstrip("/") == "login": | ||
api_session = await asyncio.shield(get_api_session(request, endpoint)) | ||
else: | ||
api_session = await asyncio.shield(get_anonymous_session(request, endpoint)) | ||
elif is_anonymous: | ||
api_session = await asyncio.shield(get_anonymous_session(request)) | ||
else: | ||
|
@@ -207,23 +208,10 @@ async def web_handler(request: web.Request, *, is_anonymous=False) -> web.Stream | |
for hdr in HTTP_HEADERS_TO_FORWARD: | ||
if request.headers.get(hdr) is not None: | ||
api_rqst.headers[hdr] = request.headers[hdr] | ||
if proxy_path == "pipeline": | ||
session_id = request.headers.get("X-BackendAI-SessionID", "") | ||
if not (sso_token := request.headers.get("X-BackendAI-SSO")): | ||
jwt_secret = config["pipeline"]["jwt"]["secret"] | ||
now = datetime.now().astimezone() | ||
payload = { | ||
# Registered claims | ||
"exp": now + timedelta(seconds=config["session"]["max_age"]), | ||
"iss": "Backend.AI Webserver", | ||
"iat": now, | ||
# Private claims | ||
"aiohttp_session": session_id, | ||
"access_key": api_session.config.access_key, # since 23.03.10 | ||
} | ||
sso_token = jwt.encode(payload, key=jwt_secret, algorithm="HS256") | ||
api_rqst.headers["X-BackendAI-SSO"] = sso_token | ||
api_rqst.headers["X-BackendAI-SessionID"] = session_id | ||
Comment on lines
-210
to
-226
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does Fasttrack handle version compatibility? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is recommended to use the same version as Backend.AI Core. |
||
if proxy_path == "pipeline" and real_path.rstrip("/") == "login": | ||
api_rqst.headers["X-BackendAI-SessionID"] = request.headers.get( | ||
"X-BackendAI-SessionID", "" | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I dont' think rqst is a commonly used abbreviation. Would it be possible to rename it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Renaming the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please create the rename task with priority: low and handle it when you have time. :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The task will be tracked by BA-469 |
||
# Uploading request body happens at the entering of the block, | ||
# and downloading response body happens in the read loop inside. | ||
async with api_rqst.fetch() as up_resp: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there another way to identify it as a login path? Relying on the
real_path
could be fragile.