From 21fb54f70a82dc635ea7b6ddae88cd81aab3d60d Mon Sep 17 00:00:00 2001 From: Pelle Wessman Date: Sat, 26 Aug 2023 00:47:01 +0200 Subject: [PATCH 1/8] Fix `test:coverage` command --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 0f6a964..8f8c071 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,7 @@ "lint": "standard | snazzy", "lint:fix": "standard --fix", "test": "npm run test:unit && npm run test:typescript", - "test:coverage": "npm run unit -- --cov --coverage-report=html", + "test:coverage": "npm run test:unit -- --cov --coverage-report=html", "test:typescript": "tsd", "test:unit": "tap" }, From d54f8777789821d66396ff6eb225f0a8c437603a Mon Sep 17 00:00:00 2001 From: Pelle Wessman Date: Sat, 26 Aug 2023 00:51:44 +0200 Subject: [PATCH 2/8] Add default user-agent + enable custom one Every library appends their user-agent to the one that's been set at a level above them. Hence any provided user-agent comes before the default user agent. To provide context, the version number of `@fastify/oauth2` + the home page of it is included in the User-Agent. That way an API can diagnose possibly faulty consumers and file an issue. If someone wouldn't want to expose this, they can always set a custom User-Agent header directly in the http settings. Especially if #225 gets merged. --- index.js | 21 +++++++- test/index.test.js | 121 ++++++++++++++++++++++++++++++++++++++++----- types/index.d.ts | 1 + 3 files changed, 131 insertions(+), 12 deletions(-) diff --git a/index.js b/index.js index 4732f50..75fae2f 100644 --- a/index.js +++ b/index.js @@ -8,6 +8,10 @@ const kGenerateCallbackUriParams = Symbol.for('fastify-oauth2.generate-callback- const { promisify, callbackify } = require('util') +const { homepage, version } = require('./package.json') + +const USER_AGENT = `fastify-oauth2/${version} (${homepage})` + function defaultGenerateStateFunction () { return randomBytes(16).toString('base64url') } @@ -69,6 +73,9 @@ function fastifyOauth2 (fastify, options, next) { if (options.cookie && typeof options.cookie !== 'object') { return next(new Error('options.cookie should be an object')) } + if (options.userAgent && typeof options.userAgent !== 'string') { + return next(new Error('options.userAgent should be a string')) + } if (!fastify.hasReplyDecorator('cookie')) { fastify.register(require('@fastify/cookie')) @@ -90,6 +97,9 @@ function fastifyOauth2 (fastify, options, next) { const generateCallbackUriParams = (credentials.auth && credentials.auth[kGenerateCallbackUriParams]) || defaultGenerateCallbackUriParams const cookieOpts = Object.assign({ httpOnly: true, sameSite: 'lax' }, options.cookie) + const userAgent = options.userAgent + ? options.userAgent + ' ' + USER_AGENT + : USER_AGENT function generateAuthorizationUri (request, reply) { const state = generateStateFunction(request) @@ -183,7 +193,16 @@ function fastifyOauth2 (fastify, options, next) { revokeAllTokenCallbacked(token, params, callback) } - const oauth2 = new AuthorizationCode(credentials) + const oauth2 = new AuthorizationCode({ + ...credentials, + http: { + ...credentials.http, + headers: { + 'User-Agent': userAgent, + ...credentials.http?.headers + } + } + }) if (startRedirectPath) { fastify.get(startRedirectPath, { schema }, startRedirectHandler) diff --git a/test/index.test.js b/test/index.test.js index f1bbd4a..e2cb29a 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -8,7 +8,7 @@ const fastifyOauth2 = require('..') nock.disableNetConnect() -function makeRequests (t, fastify) { +function makeRequests (t, fastify, userAgentRegexp) { fastify.listen({ port: 0 }, function (err) { t.error(err) @@ -39,17 +39,11 @@ function makeRequests (t, fastify) { } const githubScope = nock('https://github.com') - .post('/login/oauth/access_token', 'grant_type=authorization_code&code=my-code&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2Fcallback', { - reqheaders: { - authorization: 'Basic bXktY2xpZW50LWlkOm15LXNlY3JldA==' - } - }) + .matchHeader('Authorization', 'Basic bXktY2xpZW50LWlkOm15LXNlY3JldA==') + .matchHeader('User-Agent', userAgentRegexp || /^fastify-oauth2\/[0-9.]+ \(http[^)]+\)$/) + .post('/login/oauth/access_token', 'grant_type=authorization_code&code=my-code&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2Fcallback') .reply(200, RESPONSE_BODY) - .post('/login/oauth/access_token', 'grant_type=refresh_token&refresh_token=my-refresh-token', { - reqheaders: { - authorization: 'Basic bXktY2xpZW50LWlkOm15LXNlY3JldA==' - } - }) + .post('/login/oauth/access_token', 'grant_type=refresh_token&refresh_token=my-refresh-token') .reply(200, RESPONSE_BODY_REFRESHED) fastify.inject({ @@ -192,6 +186,89 @@ t.test('fastify-oauth2', t => { }) }) + t.test('custom user-agent', t => { + const fastify = createFastify({ logger: { level: 'silent' } }) + + fastify.register(fastifyOauth2, { + name: 'githubOAuth2', + credentials: { + client: { + id: 'my-client-id', + secret: 'my-secret' + }, + auth: fastifyOauth2.GITHUB_CONFIGURATION + }, + startRedirectPath: '/login/github', + callbackUri: 'http://localhost:3000/callback', + scope: ['notifications'], + userAgent: 'test/1.2.3' + }) + + fastify.get('/', function (request, reply) { + return this.githubOAuth2.getAccessTokenFromAuthorizationCodeFlow(request) + .then(result => { + // attempts to refresh the token + return this.githubOAuth2.getNewAccessTokenUsingRefreshToken(result.token) + }) + .then(token => { + return { + access_token: token.token.access_token, + refresh_token: token.token.refresh_token, + expires_in: token.token.expires_in, + token_type: token.token.token_type + } + }) + }) + + t.teardown(fastify.close.bind(fastify)) + + makeRequests(t, fastify, /^test\/1\.2\.3 fastify-oauth2\/[0-9.]+ \(http[^)]+\)$/) + }) + + t.test('overridden user-agent', t => { + const fastify = createFastify({ logger: { level: 'silent' } }) + + fastify.register(fastifyOauth2, { + name: 'githubOAuth2', + credentials: { + client: { + id: 'my-client-id', + secret: 'my-secret' + }, + auth: fastifyOauth2.GITHUB_CONFIGURATION, + http: { + headers: { + 'User-Agent': 'foo/4.5.6' + } + } + }, + startRedirectPath: '/login/github', + callbackUri: 'http://localhost:3000/callback', + scope: ['notifications'], + userAgent: 'test/1.2.3' + }) + + fastify.get('/', function (request, reply) { + return this.githubOAuth2.getAccessTokenFromAuthorizationCodeFlow(request) + .then(result => { + // attempts to refresh the token + return this.githubOAuth2.getNewAccessTokenUsingRefreshToken(result.token) + }) + .then(token => { + return { + access_token: token.token.access_token, + refresh_token: token.token.refresh_token, + expires_in: token.token.expires_in, + token_type: token.token.token_type + } + }) + }) + + t.teardown(fastify.close.bind(fastify)) + + makeRequests(t, fastify, /^foo\/4\.5\.6$/) + }) + t.end() }) @@ -624,6 +701,28 @@ t.test('options.cookie should be an object', t => { }) }) +t.test('options.userAgent should be a string', t => { + t.plan(1) + + const fastify = createFastify({ logger: { level: 'silent' } }) + + fastify.register(fastifyOauth2, { + name: 'the-name', + credentials: { + client: { + id: 'my-client-id', + secret: 'my-secret' + }, + auth: fastifyOauth2.GITHUB_CONFIGURATION + }, + callbackUri: '/callback', + userAgent: 1 + }) + .ready(err => { + t.strictSame(err.message, 'options.userAgent should be a string') + }) +}) + t.test('options.schema', t => { const fastify = createFastify({ logger: { level: 'silent' }, exposeHeadRoutes: false }) diff --git a/types/index.d.ts b/types/index.d.ts index eef2b11..4bf290f 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -33,6 +33,7 @@ declare namespace fastifyOauth2 { tags?: string[]; schema?: object; cookie?: CookieSerializeOptions; + userAgent?: string; } export type TToken = 'access_token' | 'refresh_token' From cf161ca33b50a42df30d4cd3073f286a4653483b Mon Sep 17 00:00:00 2001 From: Pelle Wessman Date: Sat, 26 Aug 2023 11:57:45 +0200 Subject: [PATCH 3/8] Update index.js Co-authored-by: Manuel Spigolon --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 75fae2f..bcc2555 100644 --- a/index.js +++ b/index.js @@ -98,7 +98,7 @@ function fastifyOauth2 (fastify, options, next) { const generateCallbackUriParams = (credentials.auth && credentials.auth[kGenerateCallbackUriParams]) || defaultGenerateCallbackUriParams const cookieOpts = Object.assign({ httpOnly: true, sameSite: 'lax' }, options.cookie) const userAgent = options.userAgent - ? options.userAgent + ' ' + USER_AGENT + ? `${options.userAgent} ${USER_AGENT}` : USER_AGENT function generateAuthorizationUri (request, reply) { From 9bcfe505f67ea3722841d4318377eff3f0bf8d7f Mon Sep 17 00:00:00 2001 From: Pelle Wessman Date: Sat, 26 Aug 2023 13:47:36 +0200 Subject: [PATCH 4/8] Fix feedback, allow disabling of user-agent --- index.js | 4 ++-- test/index.test.js | 39 +++++++++++++++++++++++++++++++++++++++ types/index.d.ts | 2 +- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index bcc2555..47afd43 100644 --- a/index.js +++ b/index.js @@ -99,7 +99,7 @@ function fastifyOauth2 (fastify, options, next) { const cookieOpts = Object.assign({ httpOnly: true, sameSite: 'lax' }, options.cookie) const userAgent = options.userAgent ? `${options.userAgent} ${USER_AGENT}` - : USER_AGENT + : (options.userAgent === false ? undefined : USER_AGENT) function generateAuthorizationUri (request, reply) { const state = generateStateFunction(request) @@ -198,7 +198,7 @@ function fastifyOauth2 (fastify, options, next) { http: { ...credentials.http, headers: { - 'User-Agent': userAgent, + ...(userAgent ? { 'User-Agent': userAgent } : {}), ...credentials.http?.headers } } diff --git a/test/index.test.js b/test/index.test.js index e2cb29a..fea4d55 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -269,6 +269,45 @@ t.test('fastify-oauth2', t => { makeRequests(t, fastify, /^foo\/4\.5\.6$/) }) + t.test('disabled user-agent', t => { + const fastify = createFastify({ logger: { level: 'silent' } }) + + fastify.register(fastifyOauth2, { + name: 'githubOAuth2', + credentials: { + client: { + id: 'my-client-id', + secret: 'my-secret' + }, + auth: fastifyOauth2.GITHUB_CONFIGURATION + }, + startRedirectPath: '/login/github', + callbackUri: 'http://localhost:3000/callback', + scope: ['notifications'], + userAgent: false + }) + + fastify.get('/', function (request, reply) { + return this.githubOAuth2.getAccessTokenFromAuthorizationCodeFlow(request) + .then(result => { + // attempts to refresh the token + return this.githubOAuth2.getNewAccessTokenUsingRefreshToken(result.token) + }) + .then(token => { + return { + access_token: token.token.access_token, + refresh_token: token.token.refresh_token, + expires_in: token.token.expires_in, + token_type: token.token.token_type + } + }) + }) + + t.teardown(fastify.close.bind(fastify)) + + makeRequests(t, fastify, userAgent => userAgent === undefined) + }) + t.end() }) diff --git a/types/index.d.ts b/types/index.d.ts index 4bf290f..5ed0543 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -33,7 +33,7 @@ declare namespace fastifyOauth2 { tags?: string[]; schema?: object; cookie?: CookieSerializeOptions; - userAgent?: string; + userAgent?: string | false; } export type TToken = 'access_token' | 'refresh_token' From 543a2ddeca161e378e2b49cc9534f128af1d134f Mon Sep 17 00:00:00 2001 From: Pelle Wessman Date: Sat, 26 Aug 2023 14:56:17 +0200 Subject: [PATCH 5/8] Simplify user-agent string --- index.js | 4 +--- test/index.test.js | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index 47afd43..333d16c 100644 --- a/index.js +++ b/index.js @@ -8,9 +8,7 @@ const kGenerateCallbackUriParams = Symbol.for('fastify-oauth2.generate-callback- const { promisify, callbackify } = require('util') -const { homepage, version } = require('./package.json') - -const USER_AGENT = `fastify-oauth2/${version} (${homepage})` +const USER_AGENT = 'fastify-oauth2' function defaultGenerateStateFunction () { return randomBytes(16).toString('base64url') diff --git a/test/index.test.js b/test/index.test.js index fea4d55..757cd3f 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -40,7 +40,7 @@ function makeRequests (t, fastify, userAgentRegexp) { const githubScope = nock('https://github.com') .matchHeader('Authorization', 'Basic bXktY2xpZW50LWlkOm15LXNlY3JldA==') - .matchHeader('User-Agent', userAgentRegexp || /^fastify-oauth2\/[0-9.]+ \(http[^)]+\)$/) + .matchHeader('User-Agent', userAgentRegexp || 'fastify-oauth2') .post('/login/oauth/access_token', 'grant_type=authorization_code&code=my-code&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2Fcallback') .reply(200, RESPONSE_BODY) .post('/login/oauth/access_token', 'grant_type=refresh_token&refresh_token=my-refresh-token') @@ -222,7 +222,7 @@ t.test('fastify-oauth2', t => { t.teardown(fastify.close.bind(fastify)) - makeRequests(t, fastify, /^test\/1\.2\.3 fastify-oauth2\/[0-9.]+ \(http[^)]+\)$/) + makeRequests(t, fastify, 'test/1.2.3 fastify-oauth2') }) t.test('overridden user-agent', t => { From cb1a992ae775f1574c592b9549e1a783c7121640 Mon Sep 17 00:00:00 2001 From: Pelle Wessman Date: Sat, 26 Aug 2023 17:37:58 +0200 Subject: [PATCH 6/8] Move `credentials` initialization to the top --- index.js | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/index.js b/index.js index 333d16c..462d185 100644 --- a/index.js +++ b/index.js @@ -81,7 +81,6 @@ function fastifyOauth2 (fastify, options, next) { const { name, - credentials, callbackUri, callbackUriParams = {}, tokenRequestParams = {}, @@ -93,11 +92,21 @@ function fastifyOauth2 (fastify, options, next) { schema = { tags } } = options - const generateCallbackUriParams = (credentials.auth && credentials.auth[kGenerateCallbackUriParams]) || defaultGenerateCallbackUriParams - const cookieOpts = Object.assign({ httpOnly: true, sameSite: 'lax' }, options.cookie) const userAgent = options.userAgent ? `${options.userAgent} ${USER_AGENT}` : (options.userAgent === false ? undefined : USER_AGENT) + const credentials = { + ...options.credentials, + http: { + ...options.credentials.http, + headers: { + ...(userAgent ? { 'User-Agent': userAgent } : {}), + ...options.credentials.http?.headers + } + } + } + const generateCallbackUriParams = (credentials.auth && credentials.auth[kGenerateCallbackUriParams]) || defaultGenerateCallbackUriParams + const cookieOpts = Object.assign({ httpOnly: true, sameSite: 'lax' }, options.cookie) function generateAuthorizationUri (request, reply) { const state = generateStateFunction(request) @@ -191,16 +200,7 @@ function fastifyOauth2 (fastify, options, next) { revokeAllTokenCallbacked(token, params, callback) } - const oauth2 = new AuthorizationCode({ - ...credentials, - http: { - ...credentials.http, - headers: { - ...(userAgent ? { 'User-Agent': userAgent } : {}), - ...credentials.http?.headers - } - } - }) + const oauth2 = new AuthorizationCode(credentials) if (startRedirectPath) { fastify.get(startRedirectPath, { schema }, startRedirectHandler) From 4b79d590e694ed0660be92dd03968985949e0bec Mon Sep 17 00:00:00 2001 From: Pelle Wessman Date: Sat, 26 Aug 2023 17:42:33 +0200 Subject: [PATCH 7/8] Naming tweak in test --- test/index.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/index.test.js b/test/index.test.js index 757cd3f..081e91b 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -8,7 +8,7 @@ const fastifyOauth2 = require('..') nock.disableNetConnect() -function makeRequests (t, fastify, userAgentRegexp) { +function makeRequests (t, fastify, userAgentHeaderMatcher) { fastify.listen({ port: 0 }, function (err) { t.error(err) @@ -40,7 +40,7 @@ function makeRequests (t, fastify, userAgentRegexp) { const githubScope = nock('https://github.com') .matchHeader('Authorization', 'Basic bXktY2xpZW50LWlkOm15LXNlY3JldA==') - .matchHeader('User-Agent', userAgentRegexp || 'fastify-oauth2') + .matchHeader('User-Agent', userAgentHeaderMatcher || 'fastify-oauth2') .post('/login/oauth/access_token', 'grant_type=authorization_code&code=my-code&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2Fcallback') .reply(200, RESPONSE_BODY) .post('/login/oauth/access_token', 'grant_type=refresh_token&refresh_token=my-refresh-token') From e8e6cd500b5c02081d479c46653e1fd190964c26 Mon Sep 17 00:00:00 2001 From: Pelle Wessman Date: Sun, 27 Aug 2023 18:51:52 +0200 Subject: [PATCH 8/8] Remove multi-`user-agent` use The RFC 9110 states: "The User-Agent field value consists of one or more product identifiers, each followed by zero or more comments (Section 5.6.5), which together identify the user agent software and its significant subproducts. By convention, the product identifiers are listed in decreasing order of their significance for identifying the user agent software." But since many are not aware of it, it may be better to remove it --- index.js | 8 ++++---- test/index.test.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index 462d185..f9ec675 100644 --- a/index.js +++ b/index.js @@ -92,15 +92,15 @@ function fastifyOauth2 (fastify, options, next) { schema = { tags } } = options - const userAgent = options.userAgent - ? `${options.userAgent} ${USER_AGENT}` - : (options.userAgent === false ? undefined : USER_AGENT) + const userAgent = options.userAgent === false + ? undefined + : (options.userAgent || USER_AGENT) const credentials = { ...options.credentials, http: { ...options.credentials.http, headers: { - ...(userAgent ? { 'User-Agent': userAgent } : {}), + 'User-Agent': userAgent, ...options.credentials.http?.headers } } diff --git a/test/index.test.js b/test/index.test.js index 081e91b..635c7f9 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -222,7 +222,7 @@ t.test('fastify-oauth2', t => { t.teardown(fastify.close.bind(fastify)) - makeRequests(t, fastify, 'test/1.2.3 fastify-oauth2') + makeRequests(t, fastify, 'test/1.2.3') }) t.test('overridden user-agent', t => {