Skip to content

Commit

Permalink
fix: remove headers with undefined value (#468)
Browse files Browse the repository at this point in the history
* fix: remove headers with undefined value
  • Loading branch information
aarongranick-okta committed Oct 29, 2020
1 parent e49f969 commit cd2c14c
Show file tree
Hide file tree
Showing 7 changed files with 295 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

- [#489](https://github.com/okta/okta-auth-js/pull/489) Fixes sameSite cookie setting when running on HTTP connection

- [#468](https://github.com/okta/okta-auth-js/pull/468) Fixes issue where HTTP headers with an undefined value were being sent with the value "undefined". These headers are now removed before the request is sent.

## 4.0.1

### Bug Fixes
Expand Down
4 changes: 3 additions & 1 deletion jest.browser.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
var OktaAuth = '<rootDir>/lib/browser';
var SDK_VERSION = require('./package.json').version;
var USER_AGENT = 'okta-auth-js/' + SDK_VERSION;

module.exports = {
'coverageDirectory': '<rootDir>/build2/reports/coverage-browser',
'collectCoverage': true,
'collectCoverageFrom': ['./lib/**','!./test/**'],
'globals': {
SDK_VERSION: SDK_VERSION
SDK_VERSION,
USER_AGENT
},
'restoreMocks': true,
'moduleNameMapper': {
Expand Down
6 changes: 6 additions & 0 deletions jest.server.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
var SDK_VERSION = require('./package.json').version;
var USER_AGENT = 'okta-auth-js-server/' + SDK_VERSION;
var OktaAuth = '<rootDir>/lib/server';

module.exports = {
'coverageDirectory': '<rootDir>/build2/reports/coverage-server',
'collectCoverage': true,
'collectCoverageFrom': ['./lib/**','!./test/**'],
'globals': {
SDK_VERSION,
USER_AGENT
},
'restoreMocks': true,
'moduleNameMapper': {
'^@okta/okta-auth-js$': OktaAuth
Expand Down
1 change: 1 addition & 0 deletions lib/OktaAuthBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export default class OktaAuthBase implements OktaAuth, SigninAPI {
this.options = {
issuer: removeTrailingSlash(args.issuer),
httpRequestClient: args.httpRequestClient,
transformErrorXHR: args.transformErrorXHR,
storageUtil: args.storageUtil,
headers: args.headers
};
Expand Down
1 change: 0 additions & 1 deletion lib/browser/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ class OktaAuthBrowser extends OktaAuthBase implements OktaAuth, SignoutAPI {
redirectUri: args.redirectUri,
postLogoutRedirectUri: args.postLogoutRedirectUri,
responseMode: args.responseMode,
transformErrorXHR: args.transformErrorXHR,
cookies: getCookieSettings(this, args)
});

Expand Down
5 changes: 3 additions & 2 deletions lib/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
*/

/* eslint-disable complexity */
import { isString, clone, isAbsoluteUrl } from './util';
import { isString, clone, isAbsoluteUrl, removeNils } from './util';
import AuthApiError from './errors/AuthApiError';
import { STATE_TOKEN_KEY_NAME, DEFAULT_CACHE_DURATION } from './constants';
import { OktaAuth, RequestOptions, FetchOptions, RequestData } from './types';
Expand All @@ -37,12 +37,13 @@ function httpRequest(sdk: OktaAuth, options: RequestOptions): Promise<any> {
}
}

var headers = {
var headers: HeadersInit = {
'Accept': 'application/json',
'Content-Type': 'application/json',
'X-Okta-User-Agent-Extended': sdk.userAgent
};
Object.assign(headers, sdk.options.headers, options.headers);
headers = removeNils(headers) as HeadersInit;

if (accessToken && isString(accessToken)) {
headers['Authorization'] = 'Bearer ' + accessToken;
Expand Down
280 changes: 280 additions & 0 deletions test/spec/http.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,280 @@
/* global USER_AGENT */
jest.mock('cross-fetch');

import http from '../../lib/http';
import { OktaAuth, DEFAULT_CACHE_DURATION, AuthApiError, STATE_TOKEN_KEY_NAME } from '@okta/okta-auth-js';

describe('HTTP Requestor', () => {
let sdk;
let httpRequestClient;
let url;
let response1;
let response2;

beforeEach(() => {
url = 'http://my-fake-url';
response1 = 'my fake response 1';
response2 = 'my fake response 2';
});
afterEach(() => {
sdk = null;
httpRequestClient = null;
});
function createAuthClient(options) {
httpRequestClient = httpRequestClient || jest.fn().mockReturnValue(Promise.resolve({
responseText: JSON.stringify(response1)
}));
sdk = new OktaAuth(Object.assign({
issuer: 'http://my-okta-domain',
pkce: false,
httpRequestClient,
tokenManager: { autoRenew: false }
}, options));
}
describe('withCredentials', () => {
it('can be disabled', () => {
createAuthClient();
return http.httpRequest(sdk, { url, withCredentials: false })
.then(res => {
expect(res).toBe(response1);
expect(httpRequestClient).toHaveBeenCalledWith(undefined, url, {
data: undefined,
headers: {
'Accept': 'application/json',
'Content-Type': 'application/json',
'X-Okta-User-Agent-Extended': USER_AGENT
},
withCredentials: false
});
});
});
});
describe('headers', () => {
it('sets defaults', () => {
createAuthClient();
return http.httpRequest(sdk, { url })
.then(res => {
expect(res).toBe(response1);
expect(httpRequestClient).toHaveBeenCalledWith(undefined, url, {
data: undefined,
headers: {
'Accept': 'application/json',
'Content-Type': 'application/json',
'X-Okta-User-Agent-Extended': USER_AGENT
},
withCredentials: true
});
});
});
it('accepts headers on sdk instance', () => {
createAuthClient({
headers: {
'fake': 'value'
}
});
return http.httpRequest(sdk, { url })
.then(res => {
expect(res).toBe(response1);
expect(httpRequestClient).toHaveBeenCalledWith(undefined, url, {
data: undefined,
headers: {
'Accept': 'application/json',
'Content-Type': 'application/json',
'X-Okta-User-Agent-Extended': USER_AGENT,
'fake': 'value'
},
withCredentials: true
});
});
});
it('accepts headers on httpRequest', () => {
createAuthClient();
return http.httpRequest(sdk, {
url,
headers: {
'fake': 'value'
}
})
.then(res => {
expect(res).toBe(response1);
expect(httpRequestClient).toHaveBeenCalledWith(undefined, url, {
data: undefined,
headers: {
'Accept': 'application/json',
'Content-Type': 'application/json',
'X-Okta-User-Agent-Extended': USER_AGENT,
'fake': 'value'
},
withCredentials: true
});
});
});
it('removes headers with undefined value', () => {
createAuthClient();
return http.httpRequest(sdk, {
url,
headers: {
'fake': undefined
}
})
.then(res => {
expect(res).toBe(response1);
expect(httpRequestClient).toHaveBeenCalledWith(undefined, url, {
data: undefined,
headers: {
'Accept': 'application/json',
'Content-Type': 'application/json',
'X-Okta-User-Agent-Extended': USER_AGENT
},
withCredentials: true
});
});
});
it('can set an Authorization header using accessToken', () => {
createAuthClient();
return http.httpRequest(sdk, {
url,
accessToken: 'fake'
})
.then(res => {
expect(res).toBe(response1);
expect(httpRequestClient).toHaveBeenCalledWith(undefined, url, {
data: undefined,
headers: {
'Accept': 'application/json',
'Content-Type': 'application/json',
'X-Okta-User-Agent-Extended': USER_AGENT,
'Authorization': 'Bearer fake'
},
withCredentials: true
});
});
});
});
describe('cacheResponse', () => {
let storageUtil;
let httpCache;
beforeEach(() => {
createAuthClient();
storageUtil = sdk.options.storageUtil;
httpCache = storageUtil.getHttpCache(sdk.options.cookies);
jest.spyOn(storageUtil, 'getHttpCache').mockReturnValue(httpCache); // force same object on each call
});
afterEach(() => {
httpCache.clearStorage();
});
it('can return a cached response', () => {
httpCache.updateStorage(url, {
expiresAt: Math.floor(Date.now()/1000) + DEFAULT_CACHE_DURATION,
response: response2
});
return http.httpRequest(sdk, { url, cacheResponse: true })
.then(res => {
expect(res).toBe(response2);
expect(httpRequestClient).not.toHaveBeenCalled();
});
});
it('will update cache', () => {
jest.spyOn(httpCache, 'updateStorage');
jest.spyOn(Date, 'now').mockReturnValue(1000);
return http.httpRequest(sdk, { url, cacheResponse: true, method: 'GET' })
.then(res => {
expect(res).toBe(response1);
expect(httpRequestClient).toHaveBeenCalledWith('GET', url, expect.any(Object));
expect(httpCache.updateStorage).toHaveBeenCalledWith(url, {
expiresAt: 1 + DEFAULT_CACHE_DURATION,
response: response1
});
});
});
it('cache can be disabled', () => {
jest.spyOn(httpCache, 'updateStorage');
httpCache.updateStorage(url, {
expiresAt: Math.floor(Date.now()/1000) + DEFAULT_CACHE_DURATION,
response: response2
});
httpCache.updateStorage.mockClear();
return http.httpRequest(sdk, { url, cacheResponse: false, method: 'GET' })
.then(res => {
expect(res).toBe(response1);
expect(httpRequestClient).toHaveBeenCalledWith('GET', url, expect.any(Object));
expect(httpCache.updateStorage).not.toHaveBeenCalled();
});
});
});
describe('error handling', () => {
function initWithErrorResponse(response) {
httpRequestClient = jest.fn().mockReturnValue(Promise.reject(response));
}
it('handles string errors', () => {
const response = { responseText: 'fake error', status: 404 };
initWithErrorResponse(response);
createAuthClient();
return http.httpRequest(sdk, { url })
.catch(err => {
expect(err).toBeInstanceOf(AuthApiError);
expect(err.errorSummary).toBe('Unknown error');
expect(err.xhr).toEqual(response);
});
});
it('handles JSON errors', () => {
const json = { errorSummary: 'fake error' };
const response = { responseText: JSON.stringify(json), status: 404 };
initWithErrorResponse(response);
createAuthClient();
return http.httpRequest(sdk, { url })
.catch(err => {
expect(err).toBeInstanceOf(AuthApiError);
expect(err.xhr).toEqual(response);
expect(err.errorSummary).toBe('fake error');
});
});
it('sets errorSummary to "Unknown error" on 50x errors', () => {
const json = { errorSummary: '501 error' };
const response = { responseText: JSON.stringify(json), status: 501 };
initWithErrorResponse(response);
createAuthClient();
return http.httpRequest(sdk, { url })
.catch(err => {
expect(err).toBeInstanceOf(AuthApiError);
expect(err.xhr).toEqual(response);
expect(err.errorSummary).toBe('Unknown error');
});
});
it('can transform the error XHR with "transformErrorXHR" option', () => {
const json = { errorCode: 'original error' };
const response = { responseText: JSON.stringify(json), status: 501 };
initWithErrorResponse(response);
const transformErrorXHR = jest.fn().mockImplementation(xhr => {
xhr.status = 404;
return xhr;
});
createAuthClient({
transformErrorXHR
});
return http.httpRequest(sdk, { url })
.catch(err => {
expect(err).toBeInstanceOf(AuthApiError);
expect(err.xhr).toEqual({
responseText: '{"errorCode":"original error"}',
status: 404
});
expect(err.errorCode).toBe('original error');
});
});
it('will delete state token if receives error "E0000011"', () => {
const json = { errorCode: 'E0000011' };
const response = { responseText: JSON.stringify(json), status: 403 };
initWithErrorResponse(response);
createAuthClient();
const storage = sdk.options.storageUtil.storage;
jest.spyOn(storage, 'delete');
return http.httpRequest(sdk, { url })
.catch(err => {
expect(err).toBeInstanceOf(AuthApiError);
expect(err.xhr).toEqual(response);
expect(storage.delete).toHaveBeenCalledWith(STATE_TOKEN_KEY_NAME);
});
});
});
});

0 comments on commit cd2c14c

Please sign in to comment.