Skip to content

Commit

Permalink
Resolve comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
Joerger committed Jan 21, 2025
1 parent 37705d7 commit cbc4c2a
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 14 deletions.
26 changes: 16 additions & 10 deletions web/packages/teleport/src/services/api/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import { MfaChallengeResponse } from '../mfa';
import api, {
defaultRequestOptions,
getAuthHeaders,
Expand All @@ -24,9 +25,18 @@ import api, {
} from './api';

describe('api.fetch', () => {
const mockedFetch = jest.spyOn(global, 'fetch').mockResolvedValue({} as any); // we don't care about response
let mockedFetch: jest.SpiedFunction<typeof fetch>;
beforeEach(() => {
mockedFetch = jest
.spyOn(global, 'fetch')
.mockResolvedValue({ json: async () => ({}), ok: true } as Response); // we don't care about response
});

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

const mfaResp = {
const mfaResp: MfaChallengeResponse = {
webauthn_response: {
id: 'some-id',
type: 'some-type',
Expand All @@ -43,18 +53,14 @@ describe('api.fetch', () => {
},
};

const customOpts = {
const customOpts: RequestInit = {
method: 'POST',
// Override the default header from `defaultRequestOptions`.
headers: {
Accept: 'application/json',
},
};

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

test('default (no optional params provided)', async () => {
await api.fetch('/something');
expect(mockedFetch).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -156,7 +162,7 @@ describe('api.fetch', () => {
});
});

// The code below should guard us from changes to api.fetchJson which would cause it to lose type
// The code below should guard us from changes to api.fetchJsonWithMfaAuthnRetry which would cause it to lose type
// information, for example by returning `any`.

const fooService = {
Expand All @@ -171,13 +177,13 @@ const makeFoo = (): { foo: string } => {

// This is a bogus test to satisfy Jest. We don't even need to execute the code that's in the async
// function, we're interested only in the type system checking the code.
test('fetchJson does not return any', () => {
test('fetchJsonWithMfaAuthnRetry does not return any', () => {
const bogusFunction = async () => {
const result = await fooService.doSomething();
// Reading foo is correct. We add a bogus expect to satisfy Jest.
JSON.stringify(result.foo);

// @ts-expect-error If there's no error here, it means that api.fetchJson returns any, which it
// @ts-expect-error If there's no error here, it means that api.fetchJsonWithMfaAuthnRetry returns any, which it
// shouldn't.
JSON.stringify(result.bar);
};
Expand Down
3 changes: 1 addition & 2 deletions web/packages/teleport/src/services/api/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,9 @@ const api = {
}
},

async getAdminActionMfaResponse(allowReuse?: boolean) {
async getAdminActionMfaResponse() {
const challenge = await auth.getMfaChallenge({
scope: MfaChallengeScope.ADMIN_ACTION,
allowReuse,
});

if (!challenge) {
Expand Down
4 changes: 2 additions & 2 deletions web/packages/teleport/src/services/auth/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ const auth = {
async getMfaChallenge(
req: CreateAuthenticateChallengeRequest,
abortSignal?: AbortSignal
): Promise<MfaAuthenticateChallenge> | undefined {
): Promise<MfaAuthenticateChallenge | undefined> {
return api
.post(
cfg.api.mfaAuthnChallengePath,
Expand All @@ -274,7 +274,7 @@ const auth = {
},

// getChallengeResponse gets an MFA challenge response for the provided parameters.
// If challenge is undefined or has no viable challenge options, returns empty.
// If challenge is undefined or has no viable challenge options, returns empty response.
async getMfaChallengeResponse(
challenge: MfaAuthenticateChallenge,
mfaType?: DeviceType,
Expand Down

0 comments on commit cbc4c2a

Please sign in to comment.