Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Routine maintenance 2023-07 #111

Merged
merged 14 commits into from
Aug 7, 2023
Merged
4 changes: 4 additions & 0 deletions .husky/pre-commit
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/usr/bin/env sh
. "$(dirname -- "$0")/_/husky.sh"

yarn run docs && git add docs.md
1 change: 1 addition & 0 deletions .nvmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
18
3 changes: 1 addition & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
language: node_js
node_js:
- '8'
dist: focal
cache: yarn
before_script:
- 'yarn lint'
Expand Down
128 changes: 64 additions & 64 deletions docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,28 @@

### Table of Contents

- [api][1]
- [Examples][2]
- [configureApi][3]
- [Parameters][4]
- [Examples][5]
- [configureHttp][6]
- [Parameters][7]
- [Examples][8]
- [composeMiddleware][9]
- [Parameters][10]
- [Examples][11]
- [http][12]
- [Parameters][13]
- [Examples][14]
- [HttpError][15]
- [Parameters][16]
- [Examples][17]
- [isAuthenticated][18]
- [Parameters][19]
- [Examples][20]
- [getAuthenticationContext][21]
- [Examples][22]
* [api][1]
* [Examples][2]
* [configureApi][3]
* [Parameters][4]
* [Examples][5]
* [configureHttp][6]
* [Parameters][7]
* [Examples][8]
* [composeMiddleware][9]
* [Parameters][10]
* [Examples][11]
* [http][12]
* [Parameters][13]
* [Examples][14]
* [HttpError][15]
* [Parameters][16]
* [Examples][17]
* [isAuthenticated][18]
* [Parameters][19]
* [Examples][20]
* [getAuthenticationContext][21]
* [Examples][22]

## api

Expand All @@ -32,12 +32,12 @@ Provides functions to make API requests with specified HTTP methods.

The functions are as follows:

- `get(url, options)` sends a `'GET'` request
- `patch(url, body, options)` sends a `'PATCH'` request
- `post(url, body, options)` sends a `'POST'` request
- `put(url, body, options)` sends a `'PUT'` request
- `destroy(url, body, options)` sends a `'DELETE'` request
- `call(url, method, body, options)` sends a request with specified method
* `get(url, options)` sends a `'GET'` request
* `patch(url, body, options)` sends a `'PATCH'` request
* `post(url, body, options)` sends a `'POST'` request
* `put(url, body, options)` sends a `'PUT'` request
* `destroy(url, body, options)` sends a `'DELETE'` request
* `call(url, method, body, options)` sends a request with specified method

Each function can be passed an `options` object, which will eventually be forwarded
to the [Fetch API][24].
Expand Down Expand Up @@ -66,8 +66,8 @@ Note: This configuration can always be overridden by passing in options manually

### Parameters

- `config` **[Object][26]** An api configuration object
- `baseApi` **[Object][26]?** An existing api instance to extend with the configuration
* `config` **[Object][26]** An api configuration object
* `baseApi` **[Object][26]?** An existing api instance to extend with the configuration

### Examples

Expand All @@ -88,8 +88,8 @@ Note: This configuration can always be overridden by passing in options manually

### Parameters

- `config` **[Object][26]** An http configuration object
- `baseHttp` **[Object][26]?** An existing http instance to extend with the configuration
* `config` **[Object][26]** An http configuration object
* `baseHttp` **[Object][26]?** An existing http instance to extend with the configuration

### Examples

Expand All @@ -108,7 +108,7 @@ This can be used to create more complex `before` hooks for [http][23].

### Parameters

- `middlewares` **...[Function][27]** Functions that receive and return request options
* `middlewares` **...[Function][27]** Functions that receive and return request options

### Examples

Expand Down Expand Up @@ -156,27 +156,27 @@ Any one of these settings can be overriden using the passed-in config object.

In addition to the normal Fetch API settings, the config object may also contain these special settings just for `http`:

- `url`: The url for the request. This can also be passed in directly as the first argument (see shorthand example).
- `root`: A path to be appended to the given url (default=`''`).
- `crsf`: The name of the `meta` tag containing the CSRF token (default=`'csrf-token'`). This can be set to `false` to prevent a token from being sent.
- `before`: A function that's called before the request executes. This function is passed the request options and its return value will be added to those options.
It can also return a promise that resolves to a new options object.
- `bearerToken`: A token to use for bearer auth. If provided, `http` will add the header `"Authorization": "Bearer <bearerToken>"` to the request.
- `onSuccess`: A function that will be called if the request succeeds. It will be passed the successful response. If it returns a value, `http` will resolve with this value instead of the response.
- `onFailure`: A function that will be called if the request fails. It will be passed the error that was thrown during the request. If it returns a value, `http` will reject with this value instead of the default error.
- `successDataPath`: A path to response data that the promise will resolve with.
- `failureDataPath`: A path to the errors that will be included in the HttpError object (default=`'errors'`)
- `query`: An object that will be transformed into a query string and appended to the request URL.
- `overrideHeaders`: A boolean flag indicating whether or not default headers should be included in the request (default=`false`).
- `camelizeResponse`: A boolean flag indicating whether or not to camelize the response keys (default=`true`). The helper function that does this is also exported from this library as `camelizeKeys`.
- `decamelizeBody`: A boolean flag indicating whether or not to decamelize the body keys (default=`true`). The helper function that does this is also exported from this library as `decamelizeKeys`.
- `decamelizeQuery`: A boolean flag indicating whether or not to decamelize the query string keys (default=`true`).
- `parseJsonStrictly`: A boolean flag indicating whether or not to return the text of the response body if JSON parsing fails (default=`true`). If set to `true` and invalid JSON is received in the response, then `null` will be returned instead.
- `auth`: An object with the following keys `{ username, password }`. If present, `http` will use [basic auth][28], adding the header `"Authorization": "Basic <authToken>"` to the request, where `<authToken>` is a base64 encoded string of `username:password`.
* `url`: The url for the request. This can also be passed in directly as the first argument (see shorthand example).
* `root`: A path to be appended to the given url (default=`''`).
* `crsf`: The name of the `meta` tag containing the CSRF token (default=`'csrf-token'`). This can be set to `false` to prevent a token from being sent.
* `before`: A function that's called before the request executes. This function is passed the request options and its return value will be added to those options.
It can also return a promise that resolves to a new options object.
* `bearerToken`: A token to use for bearer auth. If provided, `http` will add the header `"Authorization": "Bearer <bearerToken>"` to the request.
* `onSuccess`: A function that will be called if the request succeeds. It will be passed the successful response. If it returns a value, `http` will resolve with this value instead of the response.
* `onFailure`: A function that will be called if the request fails. It will be passed the error that was thrown during the request. If it returns a value, `http` will reject with this value instead of the default error.
* `successDataPath`: A path to response data that the promise will resolve with.
* `failureDataPath`: A path to the errors that will be included in the HttpError object (default=`'errors'`)
* `query`: An object that will be transformed into a query string and appended to the request URL.
* `overrideHeaders`: A boolean flag indicating whether or not default headers should be included in the request (default=`false`).
* `camelizeResponse`: A boolean flag indicating whether or not to camelize the response keys (default=`true`). The helper function that does this is also exported from this library as `camelizeKeys`.
* `decamelizeBody`: A boolean flag indicating whether or not to decamelize the body keys (default=`true`). The helper function that does this is also exported from this library as `decamelizeKeys`.
* `decamelizeQuery`: A boolean flag indicating whether or not to decamelize the query string keys (default=`true`).
* `parseJsonStrictly`: A boolean flag indicating whether or not to return the text of the response body if JSON parsing fails (default=`true`). If set to `true` and invalid JSON is received in the response, then `null` will be returned instead.
* `auth`: An object with the following keys `{ username, password }`. If present, `http` will use [basic auth][28], adding the header `"Authorization": "Basic <authToken>"` to the request, where `<authToken>` is a base64 encoded string of `username:password`.

### Parameters

- `config` **[Object][26]** An object containing config information for the `Fetch` request, as well as the extra keys noted above.
* `config` **[Object][26]** An object containing config information for the `Fetch` request, as well as the extra keys noted above.

### Examples

Expand Down Expand Up @@ -209,17 +209,17 @@ An error class that is thrown by the [http][23] module when a request fails.

In addition to the standard [Error][30] attributes, instances of `HttpError` include the following:

- `status`: the status code of the response
- `statusText`: the status text of the response
- `response`: the full response object
- `message`: A readable error message with format `<status> - <statusText>`
* `status`: the status code of the response
* `statusText`: the status text of the response
* `response`: the full response object
* `message`: A readable error message with format `<status> - <statusText>`

### Parameters

- `status` **[Number][31]** the status code of the response
- `statusText` **[String][32]** the status text of the response
- `response` **[Object][26]** the full response object
- `errors` **[Object][26]** an object containing error messages associated with the response (optional, default `{}`)
* `status` **[Number][31]** the status code of the response
* `statusText` **[String][32]** the status text of the response
* `response` **[Object][26]** the full response object
* `errors` **[Object][26]** an object containing error messages associated with the response (optional, default `{}`)

### Examples

Expand Down Expand Up @@ -247,7 +247,7 @@ presence, validation must be done on the server.

### Parameters

- `options` **[Object][26]** config object containing the context (optional, default `{}`)
* `options` **[Object][26]** config object containing the context (optional, default `{}`)

### Examples

Expand All @@ -273,14 +273,14 @@ isAuthenticated({ context: 'admin' }) // false
isAuthenticated({ context: 'non-admin' }) // false
```

Returns **[Boolean][33]**
Returns **[Boolean][33]**&#x20;

## getAuthenticationContext

A helper function to retrieve the authentication context for the
A helper function to retrieve the authentication context for the
authenticated user.

This function returns the context string when the LP Auth Api cookie exists,
This function returns the context string when the LP Auth Api cookie exists,
contains a valid token, and contains a context.

This function returns `undefined` when there is no context present,
Expand All @@ -301,7 +301,7 @@ getAuthenticationContext() // undefined
*
```

Returns **[String][32]**
Returns **[String][32]**&#x20;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is weird, I need to look into it

Copy link
Contributor Author

@chawes13 chawes13 Aug 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later version(s?) of documentation expect an explanation for the return value. We were defining that in the description of both of these, instead of here.


[1]: #api

Expand Down
1 change: 1 addition & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
module.exports = {
testEnvironment: 'jsdom',
chawes13 marked this conversation as resolved.
Show resolved Hide resolved
'setupFiles': [
'./test/setup.js',
],
Expand Down
26 changes: 13 additions & 13 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@launchpadlab/lp-requests",
"version": "4.2.2",
"version": "4.3.0",
"description": "Request helpers",
"main": "lib/index.js",
"scripts": {
Expand All @@ -9,12 +9,11 @@
"build:development": "mkdir -p lib && babel src --watch --out-dir lib",
"clean": "rm -rf lib",
"docs": "documentation build src/index.js -f md -o docs.md",
"prepare": "husky install && yarn run lint && yarn run clean && yarn run build",
"lint": "eslint src",
"precommit": "yarn run docs && git add docs.md",
"prepublish": "yarn run lint && yarn run clean && yarn run build",
"report-coverage": "codeclimate-test-reporter < coverage/lcov.info",
"test": "jest --coverage",
"test:coverage": "jest --coverage",
"report-coverage": "codeclimate-test-reporter < coverage/lcov.info"
"test:coverage": "jest --coverage"
},
"repository": "launchpadlab/lp-requests",
"homepage": "https://github.com/launchpadlab/lp-requests",
Expand All @@ -33,21 +32,22 @@
"@babel/core": "^7.5.5",
"@babel/polyfill": "^7.4.4",
"@launchpadlab/babel-preset": "^2.1.0",
"@launchpadlab/eslint-config": "^2.2.3",
"@launchpadlab/eslint-config": "^3.0.1",
"codeclimate-test-reporter": "^0.5.0",
"documentation": "^12.1.1",
"eslint": "^6.1.0",
"husky": "^3.0.3",
"jest": "^24.8.0"
"documentation": "^14.0.2",
"eslint": "^8.46.0",
"husky": "^8.0.0",
"jest": "^29.6.2",
"jest-environment-jsdom": "^29.6.2"
chawes13 marked this conversation as resolved.
Show resolved Hide resolved
},
"dependencies": {
"Base64": "^1.0.1",
"humps": "^2.0.0",
"is-promise": "^2.1.0",
"is-promise": "^4.0.0",
"isomorphic-fetch": "^3.0.0",
"js-cookie": "^2.1.4",
"js-cookie": "^3.0.5",
"lodash": "^4.17.4",
"query-string": "^5.0.0",
"query-string": "^7.1.3",
"url-join": "^4.0.0"
}
}
5 changes: 5 additions & 0 deletions src/http/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ async function http (...args) {
...options
} = parseArguments(...args)

if (!options.url) {
throw new Error('`url` option cannot be empty')
}

const parsedOptions = await composeMiddleware(
before,
setDefaults,
Expand All @@ -132,6 +136,7 @@ async function http (...args) {
fetchOptions,
parseJsonStrictly,
} = parsedOptions

// responseData is either the body of the response, or an error object.
const responseData = await attemptAsync(async () => {
// Make request
Expand Down
6 changes: 3 additions & 3 deletions test/get-authentication-context.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@ test('getAuthenticationContext returns undefined when lp_auth cookie is set to a
})

test('getAuthenticationContext returns undefined when lp_auth cookie does not contain a token', () => {
Cookies.set('lp_auth', {})
Cookies.set('lp_auth', JSON.stringify({}))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In v3, Cookies.set no longer implicitly calls stringify. We aren't using this method in the code and we weren't using toJson() either, so it doesn't affect any source code.

https://github.com/js-cookie/js-cookie/releases/tag/v3.0.0

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. Any value in creating a testing util function/interface to Cookies that handles the JSON.stringify. There are 3 examples here 😏 (I'm good on holding off too but figured I would at least mention it)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could be convinced if the source code was using it, but I'm pretty ok with being repetitive in tests!

expect(getAuthenticationContext()).toEqual(undefined)
})

test('getAuthenticationContext returns undefined when lp_auth cookie contains an invalid token', () => {
Cookies.set('lp_auth', { token: false, context: CONTEXT })
Cookies.set('lp_auth', JSON.stringify({ token: false, context: CONTEXT }))
expect(getAuthenticationContext()).toEqual(undefined)
})

test('getAuthenticationContext returns the context string when lp_auth cookie contains a valid context', () => {
Cookies.set('lp_auth', { token: true, context: CONTEXT })
Cookies.set('lp_auth', JSON.stringify({ token: true, context: CONTEXT }))
expect(getAuthenticationContext()).toEqual(CONTEXT)
})
29 changes: 29 additions & 0 deletions test/http/http.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,19 @@ test('http adds custom CSRF token to request', () => {
})
})

test('http does not add attempt to CSRF token on the server', () => {
const spyDocument = jest.spyOn(global, 'document', 'get')
spyDocument.mockImplementation(() => undefined)
Comment on lines +54 to +55
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This gets us test coverage for line 11 of getTokenFromDocument


return http(successUrl, {
method: 'POST',
csrf: CUSTOM_TAG_NAME
}).then((res) => {
expect(res.headers.xCSRFToken).toEqual(undefined)
spyDocument.mockRestore()
})
})

test('http passes url to request', () => {
return http(successUrl).then((res) => {
expect(res.url).toEqual(successUrl)
Expand All @@ -62,6 +75,12 @@ test('http can accept a single object argument', () => {
})
})

test('http throws an error if `url` is not provided', async () => {
await expect(http()).rejects.toThrow()
await expect(http("", {})).rejects.toThrow()
await expect(http({ url : null })).rejects.toThrow()
chawes13 marked this conversation as resolved.
Show resolved Hide resolved
})

test('http prepends custom root to request', () => {
const root = 'http://root/api/v1'
return http(successUrl, {
Expand Down Expand Up @@ -286,6 +305,16 @@ test('http sets basic auth header if `auth` is present', () => {
})
})

// Note: this isn't recommended and is super insecure, but technically the HTTP spec does allow it
test('http sets basic auth, even if username and password are not provided', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a test for behavior that already exists. It gives us test coverage for the right-hand expression of auth.username || "" and auth.password || ""

return http(successUrl, {
method: 'POST',
auth: {},
}).then(res => {
expect(res.headers.authorization).toEqual(`Basic ${Base64.btoa(':')}`)
})
})

test('http returns null when content-length is zero', () => {
return http(successUrl, { headers: { 'Content-Length': '0' }})
.then((res) => {
Expand Down
Loading