Skip to content

Commit 269d0f7

Browse files
authored
fix: enhance oidc logs (#3787)
1 parent 7d9163a commit 269d0f7

File tree

7 files changed

+516
-16
lines changed

7 files changed

+516
-16
lines changed
Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
jest.mock('../../../store', () => ({
2+
backend: undefined,
3+
clusterName: undefined,
4+
}));
5+
6+
import {isRedirectToAuth} from '../../../utils/response';
7+
import {handleBaseApiResponseError, recoverXhrResponseFromNetworkError} from '../base';
8+
import * as needResetModule from '../utils/needReset';
9+
10+
interface XhrRequestMockParams {
11+
readyState?: number;
12+
status?: number;
13+
statusText?: string;
14+
responseText?: string;
15+
responseURL?: string;
16+
rawHeaders?: string;
17+
}
18+
19+
interface NetworkErrorMock {
20+
name: string;
21+
message: string;
22+
code: string;
23+
config: {
24+
url: string;
25+
method: string;
26+
};
27+
request?: unknown;
28+
response?: unknown;
29+
status?: number;
30+
}
31+
32+
function createXhrRequestMock({
33+
readyState = 4,
34+
status = 504,
35+
statusText = 'Deadline Exceeded',
36+
responseText = '',
37+
responseURL = 'https://oidc-proxy-preprod.example.net/viewer/json/whoami?database=%2Fdb',
38+
rawHeaders = '',
39+
}: XhrRequestMockParams = {}) {
40+
return {
41+
readyState,
42+
status,
43+
statusText,
44+
responseText,
45+
responseURL,
46+
getAllResponseHeaders: () => rawHeaders,
47+
};
48+
}
49+
50+
function createNetworkError(request?: unknown): NetworkErrorMock {
51+
return {
52+
name: 'AxiosError',
53+
message: 'Network Error',
54+
code: 'ERR_NETWORK',
55+
config: {
56+
url: '/viewer/json/whoami?database=%2Fdb',
57+
method: 'get',
58+
},
59+
request,
60+
};
61+
}
62+
63+
describe('recoverXhrResponseFromNetworkError', () => {
64+
test('restores HTTP response details from recoverable XHR network error', () => {
65+
const error = createNetworkError(
66+
createXhrRequestMock({
67+
rawHeaders: [
68+
'Content-Type: text/plain; charset=utf-8',
69+
'X-Worker-Name: oidc-proxy-1-vm-preprod.example.net',
70+
'X-Trace-Id: trace-id-504',
71+
].join('\r\n'),
72+
}),
73+
);
74+
75+
const response = recoverXhrResponseFromNetworkError(error);
76+
77+
expect(response).toEqual(
78+
expect.objectContaining({
79+
status: 504,
80+
statusText: 'Deadline Exceeded',
81+
data: '',
82+
headers: {
83+
'content-type': 'text/plain; charset=utf-8',
84+
'x-worker-name': 'oidc-proxy-1-vm-preprod.example.net',
85+
'x-trace-id': 'trace-id-504',
86+
},
87+
config: expect.objectContaining({
88+
url: 'https://oidc-proxy-preprod.example.net/viewer/json/whoami?database=%2Fdb',
89+
method: 'get',
90+
}),
91+
code: 'ERR_NETWORK',
92+
message: 'Network Error',
93+
}),
94+
);
95+
expect(error.response).toBe(response);
96+
expect(error.status).toBe(504);
97+
});
98+
99+
test('does not treat array-like errors as recoverable records', () => {
100+
const error = Object.assign([], createNetworkError(createXhrRequestMock()));
101+
102+
const response = recoverXhrResponseFromNetworkError(error);
103+
104+
expect(response).toBeUndefined();
105+
expect(error.response).toBeUndefined();
106+
});
107+
108+
test('restores recovered auth response in shape compatible with redirect-to-auth checks', () => {
109+
const error = createNetworkError(
110+
createXhrRequestMock({
111+
status: 401,
112+
statusText: 'Unauthorized',
113+
responseText: JSON.stringify({authUrl: 'https://auth.example.com/login'}),
114+
rawHeaders: 'Content-Type: application/json',
115+
}),
116+
);
117+
118+
const response = recoverXhrResponseFromNetworkError(error);
119+
120+
expect(response).toEqual(
121+
expect.objectContaining({
122+
status: 401,
123+
data: {authUrl: 'https://auth.example.com/login'},
124+
}),
125+
);
126+
expect(isRedirectToAuth(response)).toBe(true);
127+
});
128+
129+
test.each([
130+
{
131+
title: 'status is zero',
132+
request: createXhrRequestMock({status: 0}),
133+
},
134+
{
135+
title: 'readyState is not done',
136+
request: createXhrRequestMock({readyState: 3}),
137+
},
138+
{
139+
title: 'request does not expose headers reader',
140+
request: {
141+
readyState: 4,
142+
status: 504,
143+
statusText: 'Deadline Exceeded',
144+
},
145+
},
146+
{
147+
title: 'request is missing entirely',
148+
request: undefined,
149+
},
150+
])('does not restore response when $title', ({request}) => {
151+
const error = createNetworkError(request);
152+
153+
const response = recoverXhrResponseFromNetworkError(error);
154+
155+
expect(response).toBeUndefined();
156+
expect(error.response).toBeUndefined();
157+
});
158+
});
159+
160+
describe('handleBaseApiResponseError', () => {
161+
afterEach(() => {
162+
jest.restoreAllMocks();
163+
});
164+
165+
test('preserves NEED_RESET behavior after recovered response JSON parsing', async () => {
166+
const visibilityStateDescriptor = Object.getOwnPropertyDescriptor(
167+
document,
168+
'visibilityState',
169+
);
170+
Object.defineProperty(document, 'visibilityState', {
171+
configurable: true,
172+
value: 'visible',
173+
});
174+
175+
const processNeedResetSpy = jest
176+
.spyOn(needResetModule, 'processNeedReset')
177+
.mockImplementation(jest.fn());
178+
const error = createNetworkError(
179+
createXhrRequestMock({
180+
status: 401,
181+
statusText: 'Unauthorized',
182+
responseText: JSON.stringify({code: 'NEED_RESET'}),
183+
rawHeaders: 'Content-Type: application/json',
184+
}),
185+
);
186+
187+
await expect(handleBaseApiResponseError(error)).rejects.toBe(error);
188+
189+
expect(processNeedResetSpy).toHaveBeenCalledTimes(1);
190+
191+
if (visibilityStateDescriptor) {
192+
Object.defineProperty(document, 'visibilityState', visibilityStateDescriptor);
193+
} else {
194+
Reflect.deleteProperty(document, 'visibilityState');
195+
}
196+
});
197+
});

src/services/api/base.ts

Lines changed: 155 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,160 @@ export interface BaseAPIParams {
2424
useRelativePath: undefined | boolean;
2525
}
2626

27+
interface XhrLikeRequest {
28+
readyState: number;
29+
status: number;
30+
statusText?: string;
31+
responseText?: string;
32+
responseURL?: string;
33+
getAllResponseHeaders: () => string;
34+
}
35+
36+
interface RecoveredNetworkResponse {
37+
status: number;
38+
statusText: string;
39+
data?: unknown;
40+
headers: Record<string, string>;
41+
config: Record<string, unknown>;
42+
request: XhrLikeRequest;
43+
code?: string;
44+
message?: string;
45+
}
46+
47+
function isRecord(value: unknown): value is Record<string, unknown> {
48+
return Boolean(value && typeof value === 'object' && !Array.isArray(value));
49+
}
50+
51+
function isXhrLikeRequest(request: unknown): request is XhrLikeRequest {
52+
return Boolean(
53+
isRecord(request) &&
54+
typeof request.readyState === 'number' &&
55+
typeof request.status === 'number' &&
56+
typeof request.getAllResponseHeaders === 'function',
57+
);
58+
}
59+
60+
function parseXhrResponseHeaders(rawHeaders: string): Record<string, string> {
61+
if (!rawHeaders.trim()) {
62+
return {};
63+
}
64+
65+
const headers: Record<string, string> = {};
66+
67+
for (const line of rawHeaders.split(/\r?\n/)) {
68+
const separatorIndex = line.indexOf(':');
69+
if (separatorIndex <= 0) {
70+
continue;
71+
}
72+
73+
const headerName = line.slice(0, separatorIndex).trim().toLowerCase();
74+
const headerValue = line.slice(separatorIndex + 1).trim();
75+
76+
if (!headerName || !headerValue) {
77+
continue;
78+
}
79+
80+
headers[headerName] = headers[headerName]
81+
? `${headers[headerName]}, ${headerValue}`
82+
: headerValue;
83+
}
84+
85+
return headers;
86+
}
87+
88+
function parseRecoveredResponseData(
89+
responseText: string | undefined,
90+
headers: Record<string, string>,
91+
): unknown {
92+
if (typeof responseText !== 'string') {
93+
return undefined;
94+
}
95+
96+
const trimmedResponse = responseText.trim();
97+
if (!trimmedResponse) {
98+
return responseText;
99+
}
100+
101+
const contentType = headers['content-type']?.toLowerCase();
102+
const shouldParseJson =
103+
Boolean(contentType?.includes('json')) ||
104+
trimmedResponse.startsWith('{') ||
105+
trimmedResponse.startsWith('[');
106+
107+
if (!shouldParseJson) {
108+
return responseText;
109+
}
110+
111+
try {
112+
return JSON.parse(responseText) as unknown;
113+
} catch {
114+
return responseText;
115+
}
116+
}
117+
118+
export function recoverXhrResponseFromNetworkError(
119+
error: unknown,
120+
): RecoveredNetworkResponse | undefined {
121+
if (!isRecord(error) || error.code !== 'ERR_NETWORK' || error.response) {
122+
return undefined;
123+
}
124+
125+
const request = error.request;
126+
if (!isXhrLikeRequest(request) || request.readyState !== 4 || request.status <= 0) {
127+
return undefined;
128+
}
129+
130+
const headers = parseXhrResponseHeaders(request.getAllResponseHeaders());
131+
const config = isRecord(error.config) ? {...error.config} : {};
132+
133+
if (request.responseURL) {
134+
config.url = request.responseURL;
135+
}
136+
137+
const recoveredResponse: RecoveredNetworkResponse = {
138+
status: request.status,
139+
statusText: typeof request.statusText === 'string' ? request.statusText : '',
140+
data: parseRecoveredResponseData(request.responseText, headers),
141+
headers,
142+
config,
143+
request,
144+
};
145+
146+
if (typeof error.code === 'string') {
147+
recoveredResponse.code = error.code;
148+
}
149+
150+
if (typeof error.message === 'string') {
151+
recoveredResponse.message = error.message;
152+
}
153+
154+
Object.assign(error, {
155+
response: recoveredResponse,
156+
status: request.status,
157+
});
158+
159+
return recoveredResponse;
160+
}
161+
162+
export function handleBaseApiResponseError(error: unknown): Promise<never> {
163+
recoverXhrResponseFromNetworkError(error);
164+
165+
const response =
166+
isRecord(error) && 'response' in error && isRecord(error.response)
167+
? error.response
168+
: undefined;
169+
170+
if (isRedirectToAuth(response)) {
171+
window.location.assign(response.data.authUrl);
172+
}
173+
174+
if (isNeedResetResponse(response?.data) && document.visibilityState === 'visible') {
175+
processNeedReset();
176+
}
177+
178+
return Promise.reject(error);
179+
}
180+
27181
export class BaseYdbAPI extends AxiosWrapper {
28182
DEFAULT_RETRIES_COUNT = 0;
29183

@@ -73,22 +227,7 @@ export class BaseYdbAPI extends AxiosWrapper {
73227
});
74228

75229
// Interceptor to process OIDC auth and NEED_RESET
76-
this._axios.interceptors.response.use(null, function (error) {
77-
const response = error.response;
78-
79-
// OIDC proxy returns 401 response with authUrl in it
80-
// authUrl - external auth service link, after successful auth additional cookies will be appended
81-
// that will allow access to clusters where OIDC proxy is a balancer
82-
if (isRedirectToAuth(response)) {
83-
window.location.assign(response.data.authUrl);
84-
}
85-
86-
if (isNeedResetResponse(response?.data) && document.visibilityState === 'visible') {
87-
processNeedReset();
88-
}
89-
90-
return Promise.reject(error);
91-
});
230+
this._axios.interceptors.response.use(null, handleBaseApiResponseError);
92231
}
93232

94233
getPath(path: string) {

0 commit comments

Comments
 (0)