Skip to content

Commit ca231d9

Browse files
Jasonxia007Jasonxia007
committed
πŸ› Bug fix: logout problem and failed to get session
πŸ› Bug fix: model healthcheck failed πŸ› Bug fix: sessionListeners unexpectedly request the guest to re-login πŸ› Security bug fix: user can somehow close the re-login modal
1 parent 9c7838f commit ca231d9

15 files changed

+335
-126
lines changed

β€Žbackend/apps/me_model_managment_app.pyβ€Ž

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,22 +33,13 @@ async def get_me_models(
3333
)
3434
except TimeoutException as e:
3535
logging.error(f"Request me model timeout: {str(e)}")
36-
return JSONResponse(status_code=HTTPStatus.REQUEST_TIMEOUT, content={
37-
"message": f"Request me model timeout: {str(e)}",
38-
"data": []
39-
})
36+
raise HTTPException(status_code=HTTPStatus.REQUEST_TIMEOUT, detail="Failed to get ModelEngine model list: timeout")
4037
except NotFoundException as e:
4138
logging.error(f"Request me model not found: {str(e)}")
42-
return JSONResponse(status_code=HTTPStatus.NOT_FOUND, content={
43-
"message": f"Request me model not found: {str(e)}",
44-
"data": []
45-
})
39+
raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="ModelEngine model not found")
4640
except Exception as e:
4741
logging.error(f"Failed to get me model list: {str(e)}")
48-
return JSONResponse(status_code=HTTPStatus.INTERNAL_SERVER_ERROR, content={
49-
"message": f"Failed to get me model list: {str(e)}",
50-
"data": []
51-
})
42+
raise HTTPException(status_code=HTTPStatus.INTERNAL_SERVER_ERROR, detail="Failed to get ModelEngine model list")
5243

5344

5445
@router.get("/healthcheck")

β€Žbackend/apps/user_management_app.pyβ€Ž

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from services.user_management_service import get_authorized_client, validate_token, \
1313
check_auth_service_health, signup_user, signin_user, refresh_user_token, \
1414
get_session_by_authorization
15+
from consts.exceptions import UnauthorizedError
1516
from utils.auth_utils import get_current_user_id
1617

1718

@@ -123,12 +124,16 @@ async def user_refresh_token(request: Request):
123124
async def logout(request: Request):
124125
"""User logout"""
125126
authorization = request.headers.get("Authorization")
126-
if not authorization:
127-
raise HTTPException(status_code=HTTPStatus.UNAUTHORIZED,
128-
detail="User not logged in")
129127
try:
130-
client = get_authorized_client(authorization)
131-
client.auth.sign_out()
128+
# Make logout idempotent: if no token or token expired, still return success
129+
if authorization:
130+
client = get_authorized_client(authorization)
131+
try:
132+
client.auth.sign_out()
133+
except Exception as signout_err:
134+
# Ignore sign out errors to keep logout idempotent
135+
logging.warning(
136+
f"Sign out encountered an error but will be ignored: {str(signout_err)}")
132137
return JSONResponse(status_code=HTTPStatus.OK,
133138
content={"message":"Logout successful"})
134139

@@ -143,17 +148,19 @@ async def get_session(request: Request):
143148
"""Get current user session"""
144149
authorization = request.headers.get("Authorization")
145150
if not authorization:
146-
raise HTTPException(status_code=HTTPStatus.UNAUTHORIZED,
147-
detail="User not logged in")
151+
# Treat as not logged in when missing token
152+
return JSONResponse(status_code=HTTPStatus.OK,
153+
content={"message": "User not logged in",
154+
"data": None})
148155
try:
149156
data = await get_session_by_authorization(authorization)
150157
return JSONResponse(status_code=HTTPStatus.OK,
151158
content={"message": "Session is valid",
152159
"data": data})
153-
except ValueError as e:
154-
logging.error(f"Get user session failed: {str(e)}")
155-
raise HTTPException(status_code=HTTPStatus.UNPROCESSABLE_ENTITY,
156-
detail="Session is invalid")
160+
except UnauthorizedError as e:
161+
logging.error(f"Get user session unauthorized: {str(e)}")
162+
raise HTTPException(status_code=HTTPStatus.UNAUTHORIZED,
163+
detail="User not logged in or session invalid")
157164
except Exception as e:
158165
logging.error(f"error in get user session, {str(e)}")
159166
raise HTTPException(status_code=HTTPStatus.INTERNAL_SERVER_ERROR,
@@ -165,8 +172,10 @@ async def get_user_id(request: Request):
165172
"""Get current user ID, return None if not logged in"""
166173
authorization = request.headers.get("Authorization")
167174
if not authorization:
168-
raise HTTPException(status_code=HTTPStatus.UNAUTHORIZED,
169-
detail="User not logged in")
175+
# Treat as not logged in when missing token, return 200 with null user_id
176+
return JSONResponse(status_code=HTTPStatus.OK,
177+
content={"message": "User not logged in",
178+
"data": {"user_id": None}})
170179
try:
171180
# Use the unified token validation function
172181
is_valid, user = validate_token(authorization)

β€Žbackend/services/model_health_service.pyβ€Ž

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ async def _perform_connectivity_check(
5757
model_type: str,
5858
model_base_url: str,
5959
model_api_key: str,
60-
embedding_dim: int
6160
) -> bool:
6261
"""
6362
Perform specific model connectivity check
@@ -66,7 +65,6 @@ async def _perform_connectivity_check(
6665
model_type: Model type
6766
model_base_url: Model base URL
6867
model_api_key: API key
69-
embedding_dim: Embedding dimension (only for embedding models)
7068
Returns:
7169
bool: Connectivity check result
7270
"""
@@ -82,14 +80,14 @@ async def _perform_connectivity_check(
8280
model_name=model_name,
8381
base_url=model_base_url,
8482
api_key=model_api_key,
85-
embedding_dim=embedding_dim
83+
embedding_dim=0
8684
).dimension_check()) > 0
8785
elif model_type == "multi_embedding":
8886
connectivity = len(await JinaEmbedding(
8987
model_name=model_name,
9088
base_url=model_base_url,
9189
api_key=model_api_key,
92-
embedding_dim=embedding_dim
90+
embedding_dim=0
9391
).dimension_check()) > 0
9492
elif model_type == "llm":
9593
observer = MessageObserver()
@@ -155,7 +153,10 @@ async def check_model_connectivity(display_name: str, tenant_id: str) -> dict:
155153
connect_status = ModelConnectStatusEnum.AVAILABLE.value if connectivity else ModelConnectStatusEnum.UNAVAILABLE.value
156154
update_data = {"connect_status": connect_status}
157155
update_model_record(model["model_id"], update_data)
158-
return {"connectivity": connectivity, "connect_status": connect_status}
156+
return {
157+
"connectivity": connectivity,
158+
"model_name": model_name,
159+
}
159160
except Exception as e:
160161
logger.error(f"Error checking model connectivity: {str(e)}")
161162
if 'model' in locals() and model:
@@ -206,13 +207,11 @@ async def verify_model_config_connectivity(model_config: dict):
206207
model_type = model_config["model_type"]
207208
model_base_url = model_config["base_url"]
208209
model_api_key = model_config["api_key"]
209-
embedding_dim = model_config.get(
210-
"embedding_dim", model_config.get("max_tokens", 1024))
211210

212211
try:
213212
# Use the common connectivity check function
214213
connectivity = await _perform_connectivity_check(
215-
model_name, model_type, model_base_url, model_api_key, embedding_dim
214+
model_name, model_type, model_base_url, model_api_key
216215
)
217216
except ValueError as e:
218217
logger.warning(f"UNCONNECTED: {model_name}; Base URL: {model_base_url}; API Key: {model_api_key}; Error: {str(e)}")

β€Žbackend/services/user_management_service.pyβ€Ž

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
from utils.auth_utils import get_supabase_client, calculate_expires_at, get_jwt_expiry_seconds
1010
from consts.const import INVITE_CODE, SUPABASE_URL, SUPABASE_KEY
11-
from consts.exceptions import NoInviteCodeException, IncorrectInviteCodeException, UserRegistrationException
11+
from consts.exceptions import NoInviteCodeException, IncorrectInviteCodeException, UserRegistrationException, UnauthorizedError
1212

1313
from database.model_management_db import create_model_record
1414
from database.user_tenant_db import insert_user_tenant
@@ -36,11 +36,18 @@ def get_authorized_client(authorization: Optional[str] = Header(None)) -> Client
3636
return client
3737

3838

39-
def get_current_user_from_client(client: Client) -> Optional[Any]:
40-
"""Get current user from client, return user object or None"""
39+
def get_current_user_from_client(client: Client, token: Optional[str] = None) -> Optional[Any]:
40+
"""Get current user from client using provided JWT, return user object or None"""
4141
try:
42-
user_response = client.auth.get_user()
43-
if user_response and user_response.user:
42+
# Prefer explicitly passing the JWT to avoid relying on client-side session state
43+
if token:
44+
jwt_token = token.replace(
45+
"Bearer ", "") if token.startswith("Bearer ") else token
46+
user_response = client.auth.get_user(jwt_token)
47+
else:
48+
user_response = client.auth.get_user()
49+
50+
if user_response and getattr(user_response, "user", None):
4451
return user_response.user
4552
return None
4653
except Exception as e:
@@ -53,7 +60,7 @@ def validate_token(token: str) -> Tuple[bool, Optional[Any]]:
5360
client = get_supabase_client()
5461
set_auth_token_to_client(client, token)
5562
try:
56-
user = get_current_user_from_client(client)
63+
user = get_current_user_from_client(client, token)
5764
if user:
5865
return True, user
5966
return False, None
@@ -286,4 +293,5 @@ async def get_session_by_authorization(authorization):
286293
}
287294
}
288295
else:
289-
raise ValueError("Session is invalid")
296+
# Use domain-specific exception for invalid/expired token
297+
raise UnauthorizedError("Session is invalid or expired")

β€Žfrontend/components/auth/loginModal.tsxβ€Ž

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -228,15 +228,17 @@ export function LoginModal() {
228228
</Button>
229229
</Form.Item>
230230

231-
{/* Registration link section */}
232-
<div className="text-center">
233-
<Space>
234-
<Text type="secondary">{t("auth.noAccount")}</Text>
235-
<Button type="link" onClick={handleRegisterClick} className="p-0">
236-
{t("auth.registerNow")}
237-
</Button>
238-
</Space>
239-
</div>
231+
{/* Registration link section (hidden when opened from session expired flow) */}
232+
{!isFromSessionExpired && (
233+
<div className="text-center">
234+
<Space>
235+
<Text type="secondary">{t("auth.noAccount")}</Text>
236+
<Button type="link" onClick={handleRegisterClick} className="p-0">
237+
{t("auth.registerNow")}
238+
</Button>
239+
</Space>
240+
</div>
241+
)}
240242
</Form>
241243
</Modal>
242244
);

β€Žfrontend/components/auth/sessionListeners.tsxβ€Ž

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ export function SessionListeners() {
4141
closable: false,
4242
async onOk() {
4343
try {
44-
await logout(); // Log out first
44+
// Silently logout
45+
await logout({ silent: true });
4546
} finally {
4647
// Mark the source as session expired
4748
setIsFromSessionExpired(true);
@@ -103,13 +104,6 @@ export function SessionListeners() {
103104
useEffect(() => {
104105
// Skip in speed mode
105106
if (isSpeedMode) return;
106-
if (typeof window !== "undefined") {
107-
const localSession = localStorage.getItem("session");
108-
if (!localSession) {
109-
showSessionExpiredModal();
110-
}
111-
}
112-
// Only run once on mount
113107
}, []);
114108

115109
// Session status check
@@ -119,12 +113,18 @@ export function SessionListeners() {
119113
// Check session status on first load
120114
const checkSession = async () => {
121115
try {
116+
// Capture whether there was a local session before validation
117+
const hadLocalSession =
118+
typeof window !== "undefined" && !!localStorage.getItem("session");
119+
122120
// Try to get current session
123121
const session = await authService.getSession();
124-
if (!session) {
122+
123+
// Only show session expired modal if a prior session existed and is now invalid
124+
if (!session && hadLocalSession) {
125125
window.dispatchEvent(
126126
new CustomEvent(EVENTS.SESSION_EXPIRED, {
127-
detail: { message: "η™»ε½•ε·²θΏ‡ζœŸοΌŒθ―·ι‡ζ–°η™»ε½•" },
127+
detail: { message: "Session expired, please sign in again" },
128128
})
129129
);
130130
}

β€Žfrontend/hooks/useAuth.tsβ€Ž

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -302,13 +302,17 @@ export function AuthProvider({ children }: { children: (value: AuthContextType)
302302
}
303303
}
304304

305-
const logout = async () => {
305+
const logout = async (options?: { silent?: boolean }) => {
306306
try {
307307
setIsLoading(true)
308308
await authService.signOut()
309309
setUser(null)
310-
setShouldCheckSession(false) // When logging out, disable session check
311-
message.success(t('auth.logoutSuccess'))
310+
// When logging out, disable session check
311+
setShouldCheckSession(false)
312+
// Only show message when user actively logout
313+
if (!options?.silent) {
314+
message.success(t("auth.logoutSuccess"))
315+
}
312316
// Manually trigger storage event
313317
window.dispatchEvent(new StorageEvent("storage", { key: "session", newValue: null }))
314318
} catch (error: any) {

β€Žfrontend/services/modelService.tsβ€Ž

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,6 @@ export const modelService = {
362362
}
363363
);
364364
const result = await response.json();
365-
await response.json();
366365
if (response.status === 200 && result.data) {
367366
return result.data.connectivity;
368367
}
@@ -392,6 +391,7 @@ export const modelService = {
392391
try {
393392
const response = await fetch(API_ENDPOINTS.model.verifyModelConfig, {
394393
method: "POST",
394+
headers: getAuthHeaders(),
395395
body: JSON.stringify({
396396
model_name: config.modelName,
397397
model_type: config.modelType,
@@ -404,7 +404,6 @@ export const modelService = {
404404
});
405405

406406
const result = await response.json();
407-
await response.json();
408407

409408
if (response.status === 200 && result.data) {
410409
return {

β€Žfrontend/types/auth.tsβ€Ž

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,13 @@ export interface AuthContextType {
4646
closeRegisterModal: () => void;
4747
setIsFromSessionExpired: (value: boolean) => void;
4848
login: (email: string, password: string) => Promise<void>;
49-
register: (email: string, password: string, isAdmin?: boolean, inviteCode?: string) => Promise<void>;
50-
logout: () => Promise<void>;
49+
register: (
50+
email: string,
51+
password: string,
52+
isAdmin?: boolean,
53+
inviteCode?: string
54+
) => Promise<void>;
55+
logout: (options?: { silent?: boolean }) => Promise<void>;
5156
}
5257

5358
// Session response type

0 commit comments

Comments
Β (0)