Skip to content

Commit 6536fe2

Browse files
author
daniel.santos
committed
Merge branch 'development-upstream' into development
# Conflicts: # lib/grant-types/refresh-token-grant-type.js # package-lock.json
2 parents c2e6409 + 039304f commit 6536fe2

24 files changed

+1201
-5929
lines changed

‎.github/workflows/codeql-analysis.yml

Lines changed: 1 addition & 0 deletions
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

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

‎.github/workflows/tests.yml

Lines changed: 16 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,21 @@
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:
4421
node: [12, 14, 16]
@@ -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

Lines changed: 3 additions & 0 deletions
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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
11
test/
2+
package-lock.json
3+
yarn.lock

‎.npmrc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package-lock=false

‎docs/model/overview.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ Model functions used by the authorization code grant:
3737
- :ref:`Model#saveAuthorizationCode`
3838
- :ref:`Model#revokeAuthorizationCode`
3939
- :ref:`Model#validateScope`
40+
- :ref:`Model#validateRedirectUri`
4041

4142
--------
4243

‎docs/model/spec.rst

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -984,3 +984,45 @@ Returns ``true`` if the access token passes, ``false`` otherwise.
984984
let authorizedScopes = token.scope.split(' ');
985985
return requestedScopes.every(s => authorizedScopes.indexOf(s) >= 0);
986986
}
987+
988+
--------
989+
990+
.. _Model#validateRedirectUri:
991+
992+
``validateRedirectUri(redirectUri, client, [callback])``
993+
================================================================
994+
995+
Invoked to check if the provided ``redirectUri`` is valid for a particular ``client``.
996+
997+
This model function is **optional**. If not implemented, the ``redirectUri`` should be included in the provided ``redirectUris`` of the client.
998+
999+
**Invoked during:**
1000+
1001+
- ``authorization_code`` grant
1002+
1003+
**Arguments:**
1004+
1005+
+-----------------+----------+---------------------------------------------------------------------+
1006+
| Name | Type | Description |
1007+
+=================+==========+=====================================================================+
1008+
| redirect_uri | String | The redirect URI to validate. |
1009+
+-----------------+----------+---------------------------------------------------------------------+
1010+
| client | Object | The associated client. |
1011+
+-----------------+----------+---------------------------------------------------------------------+
1012+
1013+
**Return value:**
1014+
1015+
Returns ``true`` if the ``redirectUri`` is valid, ``false`` otherwise.
1016+
1017+
**Remarks:**
1018+
When implementing this method you should take care of possible security risks related to ``redirectUri``.
1019+
.. _rfc6819: https://datatracker.ietf.org/doc/html/rfc6819
1020+
1021+
Section-5.2.3.5 is implemented by default.
1022+
.. _Section-5.2.3.5: https://datatracker.ietf.org/doc/html/rfc6819#section-5.2.3.5
1023+
1024+
::
1025+
1026+
function validateRedirectUri(redirectUri, client) {
1027+
return client.redirectUris.includes(redirectUri);
1028+
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ class RefreshTokenGrantType extends AbstractGrantType {
9494
}
9595

9696
if (token.client.id !== client.id) {
97-
throw new InvalidGrantError('Invalid grant: refresh token is invalid');
97+
throw new InvalidGrantError('Invalid grant: refresh token was issued to another client');
9898
}
9999

100100
if (token.refreshTokenExpiresAt && !(token.refreshTokenExpiresAt instanceof Date)) {
@@ -124,7 +124,7 @@ class RefreshTokenGrantType extends AbstractGrantType {
124124
.call(this.model, token)
125125
.then((status) => {
126126
if (!status) {
127-
throw new InvalidGrantError('Invalid grant: refresh token is invalid');
127+
throw new InvalidGrantError('Invalid grant: refresh token is invalid or could not be revoked');
128128
}
129129

130130
return token;

‎lib/handlers/authenticate-handler.js

Lines changed: 7 additions & 1 deletion
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');

‎lib/handlers/authorize-handler.js

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ AuthorizeHandler.prototype.handle = function(request, response) {
7777
throw new InvalidArgumentError('Invalid argument: `response` must be an instance of Response');
7878
}
7979

80-
if ('false' === request.query.allowed) {
80+
if (request.query.allowed === 'false' || request.body.allowed === 'false') {
8181
return Promise.reject(new AccessDeniedError('Access denied: user denied access to application'));
8282
}
8383

@@ -165,6 +165,7 @@ AuthorizeHandler.prototype.getAuthorizationCodeLifetime = function() {
165165
*/
166166

167167
AuthorizeHandler.prototype.getClient = function(request) {
168+
const self = this;
168169
const clientId = request.body.client_id || request.query.client_id;
169170

170171
if (!clientId) {
@@ -198,10 +199,17 @@ AuthorizeHandler.prototype.getClient = function(request) {
198199
throw new InvalidClientError('Invalid client: missing client `redirectUri`');
199200
}
200201

201-
if (redirectUri && !client.redirectUris.includes(redirectUri)) {
202-
throw new InvalidClientError('Invalid client: `redirect_uri` does not match client value');
202+
if (redirectUri) {
203+
return self.validateRedirectUri(redirectUri, client)
204+
.then(function(valid) {
205+
if (!valid) {
206+
throw new InvalidClientError('Invalid client: `redirect_uri` does not match client value');
207+
}
208+
return client;
209+
});
210+
} else {
211+
return client;
203212
}
204-
return client;
205213
});
206214
};
207215

@@ -295,6 +303,14 @@ AuthorizeHandler.prototype.saveAuthorizationCode = function(authorizationCode, e
295303
return promisify(this.model.saveAuthorizationCode, 3).call(this.model, code, client, user);
296304
};
297305

306+
307+
AuthorizeHandler.prototype.validateRedirectUri = function(redirectUri, client) {
308+
if (this.model.validateRedirectUri) {
309+
return promisify(this.model.validateRedirectUri, 2).call(this.model, redirectUri, client);
310+
}
311+
312+
return Promise.resolve(client.redirectUris.includes(redirectUri));
313+
};
298314
/**
299315
* Get response type.
300316
*/

0 commit comments

Comments
 (0)