Skip to content

Commit 7a61930

Browse files
authored
Merge branch 'v4.3.0-dev' into 89_11_565
2 parents f67f02b + 7191c29 commit 7a61930

15 files changed

+349
-2581
lines changed

.github/workflows/codeql-analysis.yml

+1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ name: "CodeQL Semantic Analysis"
1414
on:
1515
push: # all pushes
1616
pull_request: # all PR
17+
types: [review_requested, ready_for_review] # only non-draft PR
1718
schedule:
1819
- cron: '0 2 * * *' # every night at 2am
1920

.github/workflows/tests-release.yml

+151
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
name: Tests for Release
2+
3+
on:
4+
push:
5+
branches:
6+
- release-* # all release-<version> branches
7+
pull_request:
8+
# only non-draft PR and when there are "pushes" to the open PR
9+
types: [review_requested, ready_for_review, synchronize]
10+
branches:
11+
- release-* # all release-<version> branches
12+
13+
14+
jobs:
15+
# STEP 1 - NPM Audit
16+
17+
# Before we even test a thing we want to have a clean audit! Since this is
18+
# sufficient to be done using the lowest node version, we can easily use
19+
# a fixed one:
20+
21+
audit:
22+
name: NPM Audit
23+
runs-on: ubuntu-latest
24+
25+
steps:
26+
- uses: actions/checkout@v2
27+
- uses: actions/setup-node@v2
28+
with:
29+
node-version: '14'
30+
# install to create local package-lock.json but don't cache the files
31+
# also: no audit for dev dependencies
32+
- run: npm i --package-lock-only && npm audit --production
33+
34+
# STEP 2 - basic unit tests
35+
36+
# This is the standard unit tests as we do in the basic tests for every PR
37+
unittest:
38+
name: Basic unit tests
39+
runs-on: ubuntu-latest
40+
needs: [audit]
41+
strategy:
42+
matrix:
43+
node: [14, 16, 18]
44+
steps:
45+
- name: Checkout ${{ matrix.node }}
46+
uses: actions/checkout@v2
47+
48+
- name: Setup node ${{ matrix.node }}
49+
uses: actions/setup-node@v2
50+
with:
51+
node-version: ${{ matrix.node }}
52+
53+
- name: Cache dependencies ${{ matrix.node }}
54+
uses: actions/cache@v1
55+
with:
56+
path: ~/.npm
57+
key: ${{ runner.os }}-node-${{ matrix.node }}-${{ hashFiles('**/package-lock.json') }}
58+
restore-keys: |
59+
${{ runner.os }}-node-${{ matrix.node }}
60+
61+
# for this workflow we also require npm audit to pass
62+
- run: npm i
63+
- run: npm run test:coverage
64+
65+
# with the following action we enforce PRs to have a high coverage
66+
# and ensure, changes are tested well enough so that coverage won't fail
67+
- name: check coverage
68+
uses: VeryGoodOpenSource/very_good_coverage@v1.2.0
69+
with:
70+
path: './coverage/lcov.info'
71+
min_coverage: 95
72+
73+
# STEP 3 - Integration tests
74+
75+
# Since our release may affect several packages that depend on it we need to
76+
# cover the closest ones, like adapters and examples.
77+
78+
integrationtests:
79+
name: Extended integration tests
80+
runs-on: ubuntu-latest
81+
needs: [unittest]
82+
strategy:
83+
matrix:
84+
node: [14, 16, 18] # TODO get running for node 16
85+
steps:
86+
# checkout this repo
87+
- name: Checkout ${{ matrix.node }}
88+
uses: actions/checkout@v2
89+
90+
# checkout express-adapter repo
91+
- name: Checkout express-adapter ${{ matrix.node }}
92+
uses: actions/checkout@v2
93+
with:
94+
repository: node-oauth/express-oauth-server
95+
path: github/testing/express
96+
97+
- name: Setup node ${{ matrix.node }}
98+
uses: actions/setup-node@v2
99+
with:
100+
node-version: ${{ matrix.node }}
101+
102+
- name: Cache dependencies ${{ matrix.node }}
103+
uses: actions/cache@v1
104+
with:
105+
path: ~/.npm
106+
key: ${{ runner.os }}-node-${{ matrix.node }}-node-oauth/express-oauth-server-${{ hashFiles('github/testing/express/**/package-lock.json') }}
107+
restore-keys: |
108+
${{ runner.os }}-node-${{ matrix.node }}-node-oauth/express-oauth-server
109+
110+
# in order to test the adapter we need to use the current checkout
111+
# and install it as local dependency
112+
# we just cloned and install it as local dependency
113+
- run: |
114+
cd github/testing/express
115+
npm i
116+
npm install ../../../
117+
npm run test
118+
119+
# todo repeat with other adapters
120+
121+
publish-npm-dry:
122+
runs-on: ubuntu-latest
123+
needs: [integrationtests]
124+
steps:
125+
- uses: actions/checkout@v2
126+
- uses: actions/setup-node@v2
127+
with:
128+
node-version: 14
129+
registry-url: https://registry.npmjs.org/
130+
- run: npm i
131+
- run: npm publish --dry-run
132+
env:
133+
NODE_AUTH_TOKEN: ${{secrets.npm_token}}
134+
135+
publish-github-dry:
136+
needs: [integrationtests]
137+
runs-on: ubuntu-latest
138+
permissions:
139+
contents: read
140+
packages: write
141+
steps:
142+
- uses: actions/checkout@v2
143+
- uses: actions/setup-node@v2
144+
with:
145+
# we always publish targeting the lowest supported node version
146+
node-version: 14
147+
registry-url: $registry-url(npm)
148+
- run: npm i
149+
- run: npm publish --dry-run
150+
env:
151+
NODE_AUTH_TOKEN: ${{secrets.GITHUB_TOKEN}}

.github/workflows/tests.yml

+17-45
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,24 @@
1-
name: Test suite
1+
name: Tests
2+
3+
# This workflow runs standard unit tests to ensure basic integrity and avoid
4+
# regressions on pull-requests (and pushes)
25

36
on:
47
push:
58
branches:
6-
- master # allthough master is push protected we still keep it
9+
- master # allthough master is push protected we still keep it
710
- development
8-
pull_request: # runs on all PR
11+
pull_request: # runs on all PR
12+
branches-ignore:
13+
- release-* # on release we run an extended workflow so no need for this
914

1015
jobs:
11-
# ----------------------------------
12-
# uncomment when a linter is added
13-
# ----------------------------------
14-
15-
# lintjs:
16-
# name: Javascript lint
17-
# runs-on: ubuntu-latest
18-
# steps:
19-
# - name: checkout
20-
# uses: actions/checkout@v2
21-
#
22-
# - name: setup node
23-
# uses: actions/setup-node@v1
24-
# with:
25-
# node-version: '12.x'
26-
#
27-
# - name: cache dependencies
28-
# uses: actions/cache@v1
29-
# with:
30-
# path: ~/.npm
31-
# key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }}
32-
# restore-keys: |
33-
# ${{ runner.os }}-node-
34-
# - run: npm ci
35-
# - run: npm run lint
36-
3716
unittest:
3817
name: unit tests
3918
runs-on: ubuntu-latest
40-
# uncomment when a linter is added
41-
# needs: [lintjs]
4219
strategy:
4320
matrix:
44-
node: [12, 14, 16]
21+
node: [14, 16, 18]
4522
steps:
4623
- name: Checkout ${{ matrix.node }}
4724
uses: actions/checkout@v2
@@ -58,18 +35,13 @@ jobs:
5835
key: ${{ runner.os }}-node-${{ matrix.node }}-${{ hashFiles('**/package-lock.json') }}
5936
restore-keys: |
6037
${{ runner.os }}-node-${{ matrix.node }}
61-
- run: npm ci
38+
- run: npm i
6239
- run: npm run test:coverage
6340

64-
# ----------------------------------
65-
# uncomment when a linter is added
66-
# ----------------------------------
67-
68-
# - name: check coverage
69-
# uses: devmasx/coverage-check-action@v1.2.0
70-
# with:
71-
# type: lcov
72-
# result_path: coverage/lcov.info
73-
# min_coverage: 90
74-
# token: ${{github.token}}
75-
41+
# with the following action we enforce PRs to have a high coverage
42+
# and ensure, changes are tested well enough so that coverage won't fail
43+
- name: check coverage
44+
uses: VeryGoodOpenSource/very_good_coverage@v1.2.0
45+
with:
46+
path: './coverage/lcov.info'
47+
min_coverage: 95

.gitignore

+3
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,6 @@ tramp
3939
# coverage
4040
coverage
4141
.nyc_output
42+
43+
package-lock.json
44+
yarn.lock

.npmignore

+2
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
11
test/
2+
package-lock.json
3+
yarn.lock

.npmrc

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package-lock=false

CHANGELOG.md

+34
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,39 @@
11
## Changelog
22

3+
## 4.2.0
4+
### Fixed
5+
- fix(core): Bearer regular expression matching in authenticate handler #105
6+
- fix(request): set WWW-Authenticate header for invalid requests #96 oauthjs#646
7+
- fix(handler): deny access when body.allowed is 'false' (#94)
8+
- fix(handlers): skip varcheck for state when allowEmptyState #89 #93
9+
10+
### Added
11+
- supported custom validateRedirectUri
12+
- feature: Supported state in case of denialMerge #99
13+
- Bearer regular expression matching in authenticate handler
14+
- docs: Update extension-grants.rst with example #92
15+
- feature(core): extract is.js into standalone package @node-oauth/formats #55
16+
- feature(authorize): allow custom implementations of validateRedirectUri via model #89 p.4
17+
- support custom validateRedirectUri()
18+
- allow to implement model.validateRedirectUri
19+
- updated AuthorizeHandler
20+
- default conforms with RFC 6819 Section-5.2.3.5
21+
22+
### Tests
23+
- Integration test password grant (#100)
24+
* test example
25+
* created db & model factories
26+
* added refresh_token grant type test
27+
* removed failing test, not implemented feature
28+
* add reference to issue
29+
* client authentication test
30+
* random client credentials in test
31+
* replace math.random by crypto.randomBytes
32+
33+
### CI
34+
- refactor(ci): remove unused ci workflow
35+
- fix(ci): use node-oauth/express-oauth-server for integration test
36+
337
## 4.1.1
438

539
### Added

docs/misc/extension-grants.rst

+45-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,51 @@
22
Extension Grants
33
==================
44

5-
.. todo:: Describe how to implement extension grants.
5+
Create a subclass of ``AbstractGrantType`` and create methods `handle` and `saveToken` along with other required methods according to your needs:
6+
7+
.. code-block:: js
8+
9+
const OAuth2Server = require('oauth2-server');
10+
const AbstractGrantType = OAuth2Server.AbstractGrantType;
11+
const InvalidArgumentError = OAuth2Server.InvalidArgumentError;
12+
const InvalidRequestError = OAuth2Server.InvalidRequestError;
13+
14+
class MyCustomGrantType extends AbstractGrantType {
15+
constructor(opts) {
16+
super(opts);
17+
}
18+
19+
async handle(request, client) {
20+
if (!request) throw new InvalidArgumentError('Missing `request`');
21+
if (!client) throw new InvalidArgumentError('Missing `client`');
22+
23+
let scope = this.getScope(request);
24+
let user = await this.getUserBySomething(request);
25+
26+
return this.saveToken(user, client, scope);
27+
}
28+
29+
async saveToken(user, client, scope) {
30+
this.validateScope(user, client, scope);
31+
32+
let token = {
33+
accessToken: await this.generateAccessToken(client, user, scope),
34+
accessTokenExpiresAt: this.getAccessTokenExpiresAt(),
35+
refreshToken: await this.generateRefreshToken(client, user, scope),
36+
refreshTokenExpiresAt: this.getRefreshTokenExpiresAt(),
37+
scope: scope
38+
};
39+
40+
return this.model.saveToken(token, client, user);
41+
}
42+
43+
async getUserBySomething(request) {
44+
//Get user's data by corresponding data (FB User ID, Google, etc.), etc.
45+
}
46+
}
47+
48+
module.exports = MyCustomGrantType;
649
750
Extension grants are registered through :ref:`OAuth2Server#token() <OAuth2Server#token>` (``options.extendedGrantTypes``).
851

52+
This might require you to approve the new ``grant_type`` for a particular ``client`` if you do checks on valid grant types.

lib/grant-types/refresh-token-grant-type.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ RefreshTokenGrantType.prototype.getRefreshToken = function(request, client) {
101101
}
102102

103103
if (token.client.id !== client.id) {
104-
throw new InvalidGrantError('Invalid grant: refresh token is invalid');
104+
throw new InvalidGrantError('Invalid grant: refresh token was issued to another client');
105105
}
106106

107107
if (token.refreshTokenExpiresAt && !(token.refreshTokenExpiresAt instanceof Date)) {
@@ -130,7 +130,7 @@ RefreshTokenGrantType.prototype.revokeToken = function(token) {
130130
return promisify(this.model.revokeToken, 1).call(this.model, token)
131131
.then(function(status) {
132132
if (!status) {
133-
throw new InvalidGrantError('Invalid grant: refresh token is invalid');
133+
throw new InvalidGrantError('Invalid grant: refresh token is invalid or could not be revoked');
134134
}
135135

136136
return token;

lib/handlers/authenticate-handler.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,12 @@ AuthenticateHandler.prototype.handle = function(request, response) {
9090
// @see https://tools.ietf.org/html/rfc6750#section-3.1
9191
if (e instanceof UnauthorizedRequestError) {
9292
response.set('WWW-Authenticate', 'Bearer realm="Service"');
93+
} else if (e instanceof InvalidRequestError) {
94+
response.set('WWW-Authenticate', 'Bearer realm="Service",error="invalid_request"');
95+
} else if (e instanceof InvalidTokenError) {
96+
response.set('WWW-Authenticate', 'Bearer realm="Service",error="invalid_token"');
97+
} else if (e instanceof InsufficientScopeError) {
98+
response.set('WWW-Authenticate', 'Bearer realm="Service",error="insufficient_scope"');
9399
}
94100

95101
if (!(e instanceof OAuthError)) {
@@ -140,7 +146,7 @@ AuthenticateHandler.prototype.getTokenFromRequest = function(request) {
140146

141147
AuthenticateHandler.prototype.getTokenFromRequestHeader = function(request) {
142148
const token = request.get('Authorization');
143-
const matches = token.match(/Bearer\s(\S+)/);
149+
const matches = token.match(/^Bearer\s(\S+)/);
144150

145151
if (!matches) {
146152
throw new InvalidRequestError('Invalid request: malformed authorization header');

0 commit comments

Comments
 (0)