Skip to content

Commit

Permalink
Web: parse the proxy version from error if any
Browse files Browse the repository at this point in the history
  • Loading branch information
kimlisa committed Jan 2, 2025
1 parent a64780b commit e7fc407
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ test('generate command', async () => {

// Test create is still called with 404 ping error.
jest.clearAllMocks();
let error = new ApiError('', { status: 404 } as Response);
let error = new ApiError('', { status: 404 } as Response, null);
spyPing = jest
.spyOn(integrationService, 'pingAwsOidcIntegration')
.mockRejectedValue(error);
Expand All @@ -136,7 +136,7 @@ test('generate command', async () => {

// Test create isn't called with non 404 error
jest.clearAllMocks();
error = new ApiError('', { status: 400 } as Response);
error = new ApiError('', { status: 400 } as Response, null);
spyPing = jest
.spyOn(integrationService, 'pingAwsOidcIntegration')
.mockRejectedValue(error);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,13 @@ describe('session', () => {
});

test('valid session and invalid cookie', async () => {
const mockForbiddenError = new ApiError('some error', {
status: 403,
} as Response);
const mockForbiddenError = new ApiError(
'some error',
{
status: 403,
} as Response,
null
);

jest
.spyOn(session, 'validateCookieAndSession')
Expand Down
14 changes: 11 additions & 3 deletions web/packages/teleport/src/services/api/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ import websession from 'teleport/services/websession';

import { MfaChallengeResponse } from '../mfa';
import { storageService } from '../storageService';
import parseError, { ApiError } from './parseError';

import parseError, { ApiError, parseProxyVersion } from './parseError';

export const MFA_HEADER = 'Teleport-Mfa-Response';

Expand Down Expand Up @@ -148,10 +149,11 @@ const api = {
try {
json = await response.json();
} catch (err) {
// error reading JSON
const message = response.ok
? err.message
: `${response.status} - ${response.url}`;
throw new ApiError(message, response, { cause: err });
throw new ApiError(message, response, null, { cause: err });
}

if (response.ok) {
Expand All @@ -176,7 +178,13 @@ const api = {
);
const shouldRetry = isAdminActionMfaError && !mfaResponse;
if (!shouldRetry) {
throw new ApiError(parseError(json), response, undefined, json.messages);
throw new ApiError(
parseError(json),
response,
parseProxyVersion(json),
undefined,
json.messages
);
}

let mfaResponseForRetry;
Expand Down
36 changes: 36 additions & 0 deletions web/packages/teleport/src/services/api/parseError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,29 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

/**
* The version of the proxy where the error occurred.
*
* Currently, the proxy version field is only returned
* with api routes "not found" error.
*
* Used to determine out dated proxies.
*/
interface ProxyVersion {
major: number;
minor: number;
patch: number;
/**
* defined if version is not for production eg:
* the prerelease value for version 17.0.0-dev, is "dev"
*/
preRelease: string;
/**
* full version in string eg: "17.0.0-dev"
*/
string: string;
}

export default function parseError(json) {
let msg = '';

Expand All @@ -29,6 +52,12 @@ export default function parseError(json) {
return msg;
}

export function parseProxyVersion(json): ProxyVersion | null {
if (json && json.fields && json.fields.proxyVersion) {
return json.fields.proxyVersion;
}
}

export class ApiError extends Error {
response: Response;
/**
Expand All @@ -41,9 +70,15 @@ export class ApiError extends Error {
*/
messages: string[];

/**
* Only defined with api routes "not found" error.
*/
proxyVersion?: ProxyVersion;

constructor(
message: string,
response: Response,
proxyVersion: ProxyVersion,
opts?: ErrorOptions,
messages?: string[]
) {
Expand All @@ -53,5 +88,6 @@ export class ApiError extends Error {
this.response = response;
this.name = 'ApiError';
this.messages = messages || [];
this.proxyVersion = proxyVersion;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ import api from 'teleport/services/api';
import { integrationService } from './integrations';
import { IntegrationAudience, IntegrationStatusCode } from './types';

beforeEach(() => {
jest.resetAllMocks();
});

test('fetch a single integration: fetchIntegration()', async () => {
// test a valid response
jest.spyOn(api, 'get').mockResolvedValue(awsOidcIntegration);
Expand Down Expand Up @@ -196,6 +200,50 @@ test('fetchAwsDatabases response', async () => {
});
});

test('enrollEksClusters without labels calls v1', async () => {
jest.spyOn(api, 'post').mockResolvedValue({});

await integrationService.enrollEksClusters('integration', {
region: 'us-east-1',
enableAppDiscovery: false,
clusterNames: ['cluster'],
});

expect(api.post).toHaveBeenCalledWith(
cfg.getEnrollEksClusterUrl('integration'),
{
clusterNames: ['cluster'],
enableAppDiscovery: false,
region: 'us-east-1',
},
null,
undefined
);
});

test('enrollEksClusters with labbels calls v2', async () => {
jest.spyOn(api, 'post').mockResolvedValue({});

await integrationService.enrollEksClusters('integration', {
region: 'us-east-1',
enableAppDiscovery: false,
clusterNames: ['cluster'],
extraLabels: [{ name: 'env', value: 'staging' }],
});

expect(api.post).toHaveBeenCalledWith(
cfg.getEnrollEksClusterUrlV2('integration'),
{
clusterNames: ['cluster'],
enableAppDiscovery: false,
region: 'us-east-1',
extraLabels: [{ name: 'env', value: 'staging' }],
},
null,
undefined
);
});

describe('fetchAwsDatabases() request body formatting', () => {
test.each`
protocol | expectedEngines | expectedRdsType
Expand Down
24 changes: 24 additions & 0 deletions web/packages/teleport/src/services/joinToken/joinToken.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ import api from 'teleport/services/api';
import JoinTokenService from './joinToken';
import type { JoinTokenRequest } from './types';

beforeEach(() => {
jest.resetAllMocks();
});

test('fetchJoinToken with an empty request properly sets defaults', () => {
const svc = new JoinTokenService();
jest.spyOn(api, 'post').mockResolvedValue(null);
Expand Down Expand Up @@ -62,3 +66,23 @@ test('fetchJoinToken request fields are set as requested', () => {
null
);
});

test('fetchJoinToken with labels calls v2 endpoint', () => {
const svc = new JoinTokenService();
jest.spyOn(api, 'post').mockResolvedValue(null);

const mock: JoinTokenRequest = {
suggestedLabels: [{ name: 'env', value: 'testing' }],
};
svc.fetchJoinToken(mock);
expect(api.post).toHaveBeenCalledWith(
cfg.getJoinTokenUrlV2(),
{
suggested_labels: { env: ['testing'] },
suggested_agent_matcher_labels: {},
join_method: 'token',
allow: [],
},
null
);
});
2 changes: 1 addition & 1 deletion web/packages/teleport/src/services/joinToken/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export type CreateJoinTokenRequest = {
export type JoinTokenRequest = {
// roles is a list of join roles, since there can be more than
// one role associated with a token.
roles: JoinRole[];
roles?: JoinRole[];
// rules is a list of allow rules associated with the join token
// and the node using this token must match one of the rules.
rules?: JoinRule[];
Expand Down
21 changes: 14 additions & 7 deletions web/packages/teleport/src/services/webUiVersion/webUiVersion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,24 @@ export function getWebUiVersion() {
}

export function withUnsupportedLabelFeatureErrorConversion(
err: any
): Promise<any> {
if (err instanceof ApiError) {
if (err.response.status === 404) {
err: unknown
): never {
if (err instanceof ApiError && err.response.status === 404) {
if (err.proxyVersion && err.proxyVersion.string) {
throw new Error(
'We could not complete your request. ' +
'Your proxy may be behind the minimum required version ' +
`(${getWebUiVersion()}) to support adding resource labels. ` +
'Upgrade your proxy version or remove labels and try again.'
`Your proxy (v${err.proxyVersion.string}) may be behind the minimum required version ` +
`(v17.2.0) to support adding resource labels. ` +
'Ensure all proxies are upgraded or remove labels and try again.'
);
}

throw new Error(
'We could not complete your request. ' +
'Your proxy may be behind the minimum required version ' +
`(v17.2.0) to support adding resource labels. ` +
'Ensure all proxies are upgraded or remove labels and try again.'
);
}
throw err;
}

0 comments on commit e7fc407

Please sign in to comment.