From 39367de732d1169e6f1e08c80b35beecf363723c Mon Sep 17 00:00:00 2001 From: Christopher Allford Date: Tue, 3 Nov 2020 17:02:12 -0800 Subject: [PATCH] Fixed a bug with OAuth signature generation when using query parameters --- .../src/http/axios/__tests__/utils.spec.ts | 15 +++++++--- .../src/http/axios/axios-oauth-interceptor.ts | 8 ++--- .../http/axios/axios-response-interceptor.ts | 3 ++ tests/e2e/api/src/http/axios/utils.ts | 30 +++++++++++-------- 4 files changed, 35 insertions(+), 21 deletions(-) diff --git a/tests/e2e/api/src/http/axios/__tests__/utils.spec.ts b/tests/e2e/api/src/http/axios/__tests__/utils.spec.ts index d7db9199e9a..88affd6d7e3 100644 --- a/tests/e2e/api/src/http/axios/__tests__/utils.spec.ts +++ b/tests/e2e/api/src/http/axios/__tests__/utils.spec.ts @@ -1,4 +1,4 @@ -import { buildURL } from '../utils'; +import { buildURL, buildURLWithParams } from '../utils'; describe( 'buildURL', () => { it( 'should use base when given no url', () => { @@ -15,9 +15,16 @@ describe( 'buildURL', () => { const url = buildURL( { baseURL: 'http://test.test', url: 'yes/test' } ); expect( url ).toBe( 'http://test.test/yes/test' ); } ); +} ); - it( 'should combine base and url with trailing/leading slashes', () => { - const url = buildURL( { baseURL: 'http://test.test/////', url: '////yes/test' } ); - expect( url ).toBe( 'http://test.test/yes/test' ); +describe( 'buildURLWithParams', () => { + it( 'should do nothing without query string', () => { + const url = buildURLWithParams( { baseURL: 'http://test.test' } ); + expect( url ).toBe( 'http://test.test' ); + } ); + + it( 'should append query string', () => { + const url = buildURLWithParams( { baseURL: 'http://test.test', params: { test: 'yes' } } ); + expect( url ).toBe( 'http://test.test?test=yes' ); } ); } ); diff --git a/tests/e2e/api/src/http/axios/axios-oauth-interceptor.ts b/tests/e2e/api/src/http/axios/axios-oauth-interceptor.ts index cd7f6b12fe1..ff565d19170 100644 --- a/tests/e2e/api/src/http/axios/axios-oauth-interceptor.ts +++ b/tests/e2e/api/src/http/axios/axios-oauth-interceptor.ts @@ -2,10 +2,10 @@ import type { AxiosRequestConfig } from 'axios'; import * as createHmac from 'create-hmac'; import * as OAuth from 'oauth-1.0a'; import { AxiosInterceptor } from './axios-interceptor'; -import { buildURL } from './utils'; +import { buildURLWithParams } from './utils'; /** - * A utility class for managing the lifecycle of an authentication interceptor. + * An interceptor for adding OAuth 1.0a signatures to HTTP requests. */ export class AxiosOAuthInterceptor extends AxiosInterceptor { /** @@ -14,7 +14,7 @@ export class AxiosOAuthInterceptor extends AxiosInterceptor { * @type {Object} * @private */ - private oauth: OAuth; + private readonly oauth: OAuth; /** * Creates a new interceptor. @@ -44,7 +44,7 @@ export class AxiosOAuthInterceptor extends AxiosInterceptor { * @return {AxiosRequestConfig} The request with the additional authorization headers. */ protected handleRequest( request: AxiosRequestConfig ): AxiosRequestConfig { - const url = buildURL( request ); + const url = buildURLWithParams( request ); if ( url.startsWith( 'https' ) ) { request.auth = { username: this.oauth.consumer.key, diff --git a/tests/e2e/api/src/http/axios/axios-response-interceptor.ts b/tests/e2e/api/src/http/axios/axios-response-interceptor.ts index b936affe973..00c94459203 100644 --- a/tests/e2e/api/src/http/axios/axios-response-interceptor.ts +++ b/tests/e2e/api/src/http/axios/axios-response-interceptor.ts @@ -2,6 +2,9 @@ import { AxiosResponse } from 'axios'; import { AxiosInterceptor } from './axios-interceptor'; import { HTTPResponse } from '../http-client'; +/** + * An interceptor for transforming the responses from axios into a consistent format for package consumers. + */ export class AxiosResponseInterceptor extends AxiosInterceptor { /** * Transforms the Axios response into our HTTP response. diff --git a/tests/e2e/api/src/http/axios/utils.ts b/tests/e2e/api/src/http/axios/utils.ts index 4f6e5aa6ac8..09248825510 100644 --- a/tests/e2e/api/src/http/axios/utils.ts +++ b/tests/e2e/api/src/http/axios/utils.ts @@ -1,5 +1,10 @@ import { AxiosRequestConfig } from 'axios'; +// @ts-ignore +import buildFullPath = require( 'axios/lib/core/buildFullPath' ); +// @ts-ignore +import appendParams = require( 'axios/lib/helpers/buildURL' ); + /** * Given an Axios request config this function generates the URL that Axios will * use to make the request. @@ -8,17 +13,16 @@ import { AxiosRequestConfig } from 'axios'; * @return {string} The merged URL. */ export function buildURL( request: AxiosRequestConfig ): string { - const base = request.baseURL || ''; - if ( ! request.url ) { - return base; - } - - // Axios ignores the base when the URL is absolute. - const url = request.url; - if ( ! base || url.match( /^([a-z][a-z\d+\-.]*:)?\/\/[^\/]/i ) ) { - return url; - } - - // Remove trailing slashes from the base and leading slashes from the URL so we can combine them consistently. - return base.replace( /\/+$/, '' ) + '/' + url.replace( /^\/+/, '' ); + return buildFullPath( request.baseURL, request.url ); +} + +/** + * Given an Axios request config this function generates the URL that Axios will + * use to make the request with the query parameters included. + * + * @param {AxiosRequestConfig} request The Axios request we're building the URL for. + * @return {string} The merged URL. + */ +export function buildURLWithParams( request: AxiosRequestConfig ): string { + return appendParams( buildURL( request ), request.params, request.paramsSerializer ); }