Skip to content
This repository has been archived by the owner on Jan 20, 2024. It is now read-only.

Commit

Permalink
Fixes #98: Do not send empty scopes to an auth server (#154)
Browse files Browse the repository at this point in the history 
If `scopes` is set to `""` or `[]` then we should send an empty string.
If `scopes` is undefined (not set), then we don't send it at all.
  • Loading branch information
@postatum
postatum committed Aug 3, 2020
1 parent 75af020 commit a237644
Show file tree
Hide file tree
Showing 7 changed files with 302 additions and 29 deletions.
58 changes: 38 additions & 20 deletions src/client-oauth2.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,15 +161,19 @@ function createUri (options, tokenType) {
// Check the required parameters are set.
expects(options, 'clientId', 'authorizationUri')

const sep = options.authorizationUri.includes('?') ? '&' : '?'

return options.authorizationUri + sep + Querystring.stringify(Object.assign({
const qs = {
client_id: options.clientId,
redirect_uri: options.redirectUri,
scope: sanitizeScope(options.scopes),
response_type: tokenType,
state: options.state
}, options.query))
}
if (options.scopes !== undefined) {
qs.scope = sanitizeScope(options.scopes)
}

const sep = options.authorizationUri.includes('?') ? '&' : '?'
return options.authorizationUri + sep + Querystring.stringify(
Object.assign(qs, options.query))
}

/**
Expand Down Expand Up @@ -417,18 +421,22 @@ OwnerFlow.prototype.getToken = function (username, password, opts) {
var self = this
var options = Object.assign({}, this.client.options, opts)

const body = {
username: username,
password: password,
grant_type: 'password'
}
if (options.scopes !== undefined) {
body.scope = sanitizeScope(options.scopes)
}

return this.client._request(requestOptions({
url: options.accessTokenUri,
method: 'POST',
headers: Object.assign({}, DEFAULT_HEADERS, {
Authorization: auth(options.clientId, options.clientSecret)
}),
body: {
scope: sanitizeScope(options.scopes),
username: username,
password: password,
grant_type: 'password'
}
body: body
}, options))
.then(function (data) {
return self.client.createToken(data)
Expand Down Expand Up @@ -530,16 +538,21 @@ CredentialsFlow.prototype.getToken = function (opts) {

expects(options, 'clientId', 'clientSecret', 'accessTokenUri')

const body = {
grant_type: 'client_credentials'
}

if (options.scopes !== undefined) {
body.scope = sanitizeScope(options.scopes)
}

return this.client._request(requestOptions({
url: options.accessTokenUri,
method: 'POST',
headers: Object.assign({}, DEFAULT_HEADERS, {
Authorization: auth(options.clientId, options.clientSecret)
}),
body: {
scope: sanitizeScope(options.scopes),
grant_type: 'client_credentials'
}
body: body
}, options))
.then(function (data) {
return self.client.createToken(data)
Expand Down Expand Up @@ -671,15 +684,20 @@ JwtBearerFlow.prototype.getToken = function (token, opts) {
headers.Authorization = auth(options.clientId, options.clientSecret)
}

const body = {
grant_type: 'urn:ietf:params:oauth:grant-type:jwt-bearer',
assertion: token
}

if (options.scopes !== undefined) {
body.scope = sanitizeScope(options.scopes)
}

return this.client._request(requestOptions({
url: options.accessTokenUri,
method: 'POST',
headers: headers,
body: {
scope: sanitizeScope(options.scopes),
grant_type: 'urn:ietf:params:oauth:grant-type:jwt-bearer',
assertion: token
}
body: body
}, options))
.then(function (data) {
return self.client.createToken(data)
Expand Down
53 changes: 51 additions & 2 deletions test/code.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,56 @@ describe('code', function () {
expect(githubAuth.code.getUri()).to.equal(
config.authorizationUri + '?client_id=abc&' +
'redirect_uri=http%3A%2F%2Fexample.com%2Fauth%2Fcallback&' +
'scope=notifications&response_type=code&state='
'response_type=code&state=&scope=notifications'
)
})
context('when scopes are undefined', function () {
it('should not include scope in the uri', function () {
var authWithoutScopes = new ClientOAuth2({
clientId: config.clientId,
clientSecret: config.clientSecret,
accessTokenUri: config.accessTokenUri,
authorizationUri: config.authorizationUri,
authorizationGrants: ['code'],
redirectUri: config.redirectUri
})
expect(authWithoutScopes.code.getUri()).to.equal(
config.authorizationUri + '?client_id=abc&' +
'redirect_uri=http%3A%2F%2Fexample.com%2Fauth%2Fcallback&' +
'response_type=code&state='
)
})
})
it('should include empty scopes array as an empty string', function () {
var authWithEmptyScopes = new ClientOAuth2({
clientId: config.clientId,
clientSecret: config.clientSecret,
accessTokenUri: config.accessTokenUri,
authorizationUri: config.authorizationUri,
authorizationGrants: ['code'],
redirectUri: config.redirectUri,
scopes: []
})
expect(authWithEmptyScopes.code.getUri()).to.equal(
config.authorizationUri + '?client_id=abc&' +
'redirect_uri=http%3A%2F%2Fexample.com%2Fauth%2Fcallback&' +
'response_type=code&state=&scope='
)
})
it('should include empty scopes string as an empty string', function () {
var authWithEmptyScopes = new ClientOAuth2({
clientId: config.clientId,
clientSecret: config.clientSecret,
accessTokenUri: config.accessTokenUri,
authorizationUri: config.authorizationUri,
authorizationGrants: ['code'],
redirectUri: config.redirectUri,
scopes: ''
})
expect(authWithEmptyScopes.code.getUri()).to.equal(
config.authorizationUri + '?client_id=abc&' +
'redirect_uri=http%3A%2F%2Fexample.com%2Fauth%2Fcallback&' +
'response_type=code&state=&scope='
)
})
context('when authorizationUri contains query parameters', function () {
Expand All @@ -38,7 +87,7 @@ describe('code', function () {
expect(authWithParams.code.getUri()).to.equal(
config.authorizationUri + '?bar=qux&client_id=abc&' +
'redirect_uri=http%3A%2F%2Fexample.com%2Fauth%2Fcallback&' +
'scope=notifications&response_type=code&state='
'response_type=code&state=&scope=notifications'
)
})
})
Expand Down
56 changes: 55 additions & 1 deletion test/credentials.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* global describe, it */
/* global describe, it, context */
var expect = require('chai').expect
var config = require('./support/config')
var ClientOAuth2 = require('../')
Expand All @@ -19,8 +19,62 @@ describe('credentials', function () {
expect(user).to.an.instanceOf(ClientOAuth2.Token)
expect(user.accessToken).to.equal(config.accessToken)
expect(user.tokenType).to.equal('bearer')
expect(user.data.scope).to.equal('notifications')
})
})
context('when scopes are undefined', function () {
it('should not send scopes to an auth server', function () {
var authWithoutScopes = new ClientOAuth2({
clientId: config.clientId,
clientSecret: config.clientSecret,
accessTokenUri: config.accessTokenUri,
authorizationGrants: ['credentials']
})
return authWithoutScopes.credentials.getToken()
.then(function (user) {
expect(user).to.an.instanceOf(ClientOAuth2.Token)
expect(user.accessToken).to.equal(config.accessToken)
expect(user.tokenType).to.equal('bearer')
expect(user.data.scope).to.equal(undefined)
})
})
})
context('when scopes is an empty array', function () {
it('should send empty scope string to an auth server', function () {
var authWithoutScopes = new ClientOAuth2({
clientId: config.clientId,
clientSecret: config.clientSecret,
accessTokenUri: config.accessTokenUri,
authorizationGrants: ['credentials'],
scopes: []
})
return authWithoutScopes.credentials.getToken()
.then(function (user) {
expect(user).to.an.instanceOf(ClientOAuth2.Token)
expect(user.accessToken).to.equal(config.accessToken)
expect(user.tokenType).to.equal('bearer')
expect(user.data.scope).to.equal('')
})
})
})
context('when scopes is an empty string', function () {
it('should send empty scope string to an auth server', function () {
var authWithoutScopes = new ClientOAuth2({
clientId: config.clientId,
clientSecret: config.clientSecret,
accessTokenUri: config.accessTokenUri,
authorizationGrants: ['credentials'],
scopes: ''
})
return authWithoutScopes.credentials.getToken()
.then(function (user) {
expect(user).to.an.instanceOf(ClientOAuth2.Token)
expect(user.accessToken).to.equal(config.accessToken)
expect(user.tokenType).to.equal('bearer')
expect(user.data.scope).to.equal('')
})
})
})

describe('#sign', function () {
it('should be able to sign a standard request object', function () {
Expand Down
56 changes: 55 additions & 1 deletion test/jwtbearer.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* global describe, it */
/* global describe, it, context */
var expect = require('chai').expect
var config = require('./support/config')
var ClientOAuth2 = require('../')
Expand All @@ -19,8 +19,62 @@ describe('jwt', function () {
expect(user).to.an.instanceOf(ClientOAuth2.Token)
expect(user.accessToken).to.equal(config.accessToken)
expect(user.tokenType).to.equal('bearer')
expect(user.data.scope).to.equal('notifications')
})
})
context('when scopes are undefined', function () {
it('should not send scopes to an auth server', function () {
var scopelessAuth = new ClientOAuth2({
clientId: config.clientId,
clientSecret: config.clientSecret,
accessTokenUri: config.accessTokenUri,
authorizationGrants: ['jwt']
})
return scopelessAuth.jwt.getToken(config.jwt)
.then(function (user) {
expect(user).to.an.instanceOf(ClientOAuth2.Token)
expect(user.accessToken).to.equal(config.accessToken)
expect(user.tokenType).to.equal('bearer')
expect(user.data.scope).to.equal(undefined)
})
})
})
context('when scopes are an empty array', function () {
it('should send empty scope string to an auth server', function () {
var scopelessAuth = new ClientOAuth2({
clientId: config.clientId,
clientSecret: config.clientSecret,
accessTokenUri: config.accessTokenUri,
authorizationGrants: ['jwt'],
scopes: []
})
return scopelessAuth.jwt.getToken(config.jwt)
.then(function (user) {
expect(user).to.an.instanceOf(ClientOAuth2.Token)
expect(user.accessToken).to.equal(config.accessToken)
expect(user.tokenType).to.equal('bearer')
expect(user.data.scope).to.equal('')
})
})
})
context('when scopes are an empty array', function () {
it('should send empty scope string to an auth server', function () {
var scopelessAuth = new ClientOAuth2({
clientId: config.clientId,
clientSecret: config.clientSecret,
accessTokenUri: config.accessTokenUri,
authorizationGrants: ['jwt'],
scopes: ''
})
return scopelessAuth.jwt.getToken(config.jwt)
.then(function (user) {
expect(user).to.an.instanceOf(ClientOAuth2.Token)
expect(user.accessToken).to.equal(config.accessToken)
expect(user.tokenType).to.equal('bearer')
expect(user.data.scope).to.equal('')
})
})
})

describe('#sign', function () {
it('should be able to sign a standard request object', function () {
Expand Down
58 changes: 56 additions & 2 deletions test/owner.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* global describe, it */
/* global describe, it, context */
var expect = require('chai').expect
var config = require('./support/config')
var ClientOAuth2 = require('../')
Expand All @@ -9,7 +9,7 @@ describe('owner', function () {
clientSecret: config.clientSecret,
accessTokenUri: config.accessTokenUri,
authorizationGrants: ['owner'],
scope: 'notifications'
scopes: 'notifications'
})

describe('#getToken', function () {
Expand All @@ -19,8 +19,62 @@ describe('owner', function () {
expect(user).to.an.instanceOf(ClientOAuth2.Token)
expect(user.accessToken).to.equal(config.accessToken)
expect(user.tokenType).to.equal('bearer')
expect(user.data.scope).to.equal('notifications')
})
})
context('when scopes are undefined', function () {
it('should not send scope to an auth server', function () {
var scopelessAuth = new ClientOAuth2({
clientId: config.clientId,
clientSecret: config.clientSecret,
accessTokenUri: config.accessTokenUri,
authorizationGrants: ['owner']
})
return scopelessAuth.owner.getToken(config.username, config.password)
.then(function (user) {
expect(user).to.an.instanceOf(ClientOAuth2.Token)
expect(user.accessToken).to.equal(config.accessToken)
expect(user.tokenType).to.equal('bearer')
expect(user.data.scope).to.equal(undefined)
})
})
})
context('when scopes are an empty array', function () {
it('should send empty scope string to an auth server', function () {
var scopelessAuth = new ClientOAuth2({
clientId: config.clientId,
clientSecret: config.clientSecret,
accessTokenUri: config.accessTokenUri,
authorizationGrants: ['owner'],
scopes: []
})
return scopelessAuth.owner.getToken(config.username, config.password)
.then(function (user) {
expect(user).to.an.instanceOf(ClientOAuth2.Token)
expect(user.accessToken).to.equal(config.accessToken)
expect(user.tokenType).to.equal('bearer')
expect(user.data.scope).to.equal('')
})
})
})
context('when scopes are an empty string', function () {
it('should send empty scope string to an auth server', function () {
var scopelessAuth = new ClientOAuth2({
clientId: config.clientId,
clientSecret: config.clientSecret,
accessTokenUri: config.accessTokenUri,
authorizationGrants: ['owner'],
scopes: ''
})
return scopelessAuth.owner.getToken(config.username, config.password)
.then(function (user) {
expect(user).to.an.instanceOf(ClientOAuth2.Token)
expect(user.accessToken).to.equal(config.accessToken)
expect(user.tokenType).to.equal('bearer')
expect(user.data.scope).to.equal('')
})
})
})

describe('#sign', function () {
it('should be able to sign a standard request object', function () {
Expand Down
2 changes: 1 addition & 1 deletion test/support/server.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ app.post(
access_token: config.accessToken,
refresh_token: config.refreshToken,
token_type: 'bearer',
scope: 'notifications'
scope: req.body.scope
})
}
)
Expand Down
Loading

0 comments on commit a237644

Please sign in to comment.