Skip to content

Commit 55e236c

Browse files
chrisfrankochrisfranko
authored
Merge branch 'development' into patch-1
2 parents 052b2bf + ac68291 commit 55e236c

25 files changed

+1310
-238
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 ci
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 ci
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 ci
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 ci
147+
- run: npm publish --dry-run
148+
env:
149+
NODE_AUTH_TOKEN: ${{secrets.GITHUB_TOKEN}}

‎.github/workflows/tests.yml

Lines changed: 15 additions & 43 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]
@@ -61,15 +38,10 @@ jobs:
6138
- run: npm ci
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

‎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: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ An ``Object`` representing the access token and associated data.
214214

215215
function getAccessToken(accessToken) {
216216
// imaginary DB queries
217-
db.queryAccessToken({access_token: accessToken})
217+
return db.queryAccessToken({access_token: accessToken})
218218
.then(function(token) {
219219
return Promise.all([
220220
token,
@@ -288,7 +288,7 @@ An ``Object`` representing the refresh token and associated data.
288288

289289
function getRefreshToken(refreshToken) {
290290
// imaginary DB queries
291-
db.queryRefreshToken({refresh_token: refreshToken})
291+
return db.queryRefreshToken({refresh_token: refreshToken})
292292
.then(function(token) {
293293
return Promise.all([
294294
token,
@@ -364,7 +364,7 @@ An ``Object`` representing the authorization code and associated data.
364364

365365
function getAuthorizationCode(authorizationCode) {
366366
// imaginary DB queries
367-
db.queryAuthorizationCode({authorization_code: authorizationCode})
367+
return db.queryAuthorizationCode({authorization_code: authorizationCode})
368368
.then(function(code) {
369369
return Promise.all([
370370
code,
@@ -446,7 +446,7 @@ The return value (``client``) can carry additional properties that will be ignor
446446
if (clientSecret) {
447447
params.client_secret = clientSecret;
448448
}
449-
db.queryClient(params)
449+
return db.queryClient(params)
450450
.then(function(client) {
451451
return {
452452
id: client.id,
@@ -985,3 +985,44 @@ Returns ``true`` if the access token passes, ``false`` otherwise.
985985
return requestedScopes.every(s => authorizedScopes.indexOf(s) >= 0);
986986
}
987987

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/abstract-grant-type.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const InvalidArgumentError = require('../errors/invalid-argument-error');
88
const InvalidScopeError = require('../errors/invalid-scope-error');
99
const Promise = require('bluebird');
1010
const promisify = require('promisify-any').use(Promise);
11-
const is = require('../validator/is');
11+
const isFormat = require('@node-oauth/formats');
1212
const tokenUtil = require('../utils/token-util');
1313

1414
/**
@@ -83,7 +83,7 @@ AbstractGrantType.prototype.getRefreshTokenExpiresAt = function() {
8383
*/
8484

8585
AbstractGrantType.prototype.getScope = function(request) {
86-
if (!is.nqschar(request.body.scope)) {
86+
if (!isFormat.nqschar(request.body.scope)) {
8787
throw new InvalidArgumentError('Invalid parameter: `scope`');
8888
}
8989

‎lib/grant-types/authorization-code-grant-type.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const InvalidRequestError = require('../errors/invalid-request-error');
1111
const Promise = require('bluebird');
1212
const promisify = require('promisify-any').use(Promise);
1313
const ServerError = require('../errors/server-error');
14-
const is = require('../validator/is');
14+
const isFormat = require('@node-oauth/formats');
1515
const util = require('util');
1616

1717
/**
@@ -85,7 +85,7 @@ AuthorizationCodeGrantType.prototype.getAuthorizationCode = function(request, cl
8585
throw new InvalidRequestError('Missing parameter: `code`');
8686
}
8787

88-
if (!is.vschar(request.body.code)) {
88+
if (!isFormat.vschar(request.body.code)) {
8989
throw new InvalidRequestError('Invalid parameter: `code`');
9090
}
9191
return promisify(this.model.getAuthorizationCode, 1).call(this.model, request.body.code)
@@ -114,7 +114,7 @@ AuthorizationCodeGrantType.prototype.getAuthorizationCode = function(request, cl
114114
throw new InvalidGrantError('Invalid grant: authorization code has expired');
115115
}
116116

117-
if (code.redirectUri && !is.uri(code.redirectUri)) {
117+
if (code.redirectUri && !isFormat.uri(code.redirectUri)) {
118118
throw new InvalidGrantError('Invalid grant: `redirect_uri` is not a valid URI');
119119
}
120120

@@ -140,7 +140,7 @@ AuthorizationCodeGrantType.prototype.validateRedirectUri = function(request, cod
140140

141141
const redirectUri = request.body.redirect_uri || request.query.redirect_uri;
142142

143-
if (!is.uri(redirectUri)) {
143+
if (!isFormat.uri(redirectUri)) {
144144
throw new InvalidRequestError('Invalid request: `redirect_uri` is not a valid URI');
145145
}
146146

0 commit comments

Comments
 (0)