Fixes #98: Do not send empty scopes to an auth server

src/client-oauth2.js

@@ -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 && options.scopes.length > 0) {
+    qs.scope = sanitizeScope(options.scopes)
+  }
+
+  const sep = options.authorizationUri.includes('?') ? '&' : '?'
+  return options.authorizationUri + sep + Querystring.stringify(
+    Object.assign(qs, options.query))
 }
 
 /**
@@ -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 && options.scopes.length > 0) {
+    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)
@@ -530,16 +538,21 @@ CredentialsFlow.prototype.getToken = function (opts) {
 
   expects(options, 'clientId', 'clientSecret', 'accessTokenUri')
 
+  const body = {
+    grant_type: 'client_credentials'
+  }
+
+  if (options.scopes && options.scopes.length > 0) {
+    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)
@@ -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 && options.scopes.length > 0) {
+    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)

test/code.js

@@ -21,7 +21,23 @@ 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'
+      )
+    })
+    it('should omit empty scopes', function () {
+      var authWithoutScopes = new ClientOAuth2({
+        clientId: config.clientId,
+        clientSecret: config.clientSecret,
+        accessTokenUri: config.accessTokenUri,
+        authorizationUri: config.authorizationUri,
+        authorizationGrants: ['code'],
+        redirectUri: config.redirectUri,
+        scopes: ''
+      })
+      expect(authWithoutScopes.code.getUri()).to.equal(
+        config.authorizationUri + '?client_id=abc&' +
+        'redirect_uri=http%3A%2F%2Fexample.com%2Fauth%2Fcallback&' +
+        'response_type=code&state='
       )
     })
     context('when authorizationUri contains query parameters', function () {
@@ -38,7 +54,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'
         )
       })
     })

test/credentials.js

@@ -19,6 +19,24 @@ 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')
+        })
+    })
+
+    it('should not include empty scopes in auth server request', function () {
+      var scopelessAuth = new ClientOAuth2({
+        clientId: config.clientId,
+        clientSecret: config.clientSecret,
+        accessTokenUri: config.accessTokenUri,
+        authorizationGrants: ['credentials'],
+        scopes: []
+      })
+      return scopelessAuth.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)
         })
     })
 

test/jwtbearer.js

@@ -19,6 +19,24 @@ 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')
+        })
+    })
+
+    it('should not include empty scopes in auth server request', 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(undefined)
         })
     })
 

test/owner.js

@@ -9,7 +9,7 @@ describe('owner', function () {
     clientSecret: config.clientSecret,
     accessTokenUri: config.accessTokenUri,
     authorizationGrants: ['owner'],
-    scope: 'notifications'
+    scopes: 'notifications'
   })
 
   describe('#getToken', function () {
@@ -19,6 +19,24 @@ 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')
+        })
+    })
+
+    it('should not include empty scopes in auth erver request', function () {
+      var scopelessAuth = new ClientOAuth2({
+        clientId: config.clientId,
+        clientSecret: config.clientSecret,
+        accessTokenUri: config.accessTokenUri,
+        authorizationGrants: ['owner'],
+        scope: ''
+      })
+      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)
         })
     })
 

test/support/server.js

@@ -51,7 +51,7 @@ app.post(
       access_token: config.accessToken,
       refresh_token: config.refreshToken,
       token_type: 'bearer',
-      scope: 'notifications'
+      scope: req.body.scope
     })
   }
 )

test/token.js

@@ -19,7 +19,21 @@ describe('token', function () {
       expect(githubAuth.token.getUri()).to.equal(
         config.authorizationUri + '?client_id=abc&' +
         'redirect_uri=http%3A%2F%2Fexample.com%2Fauth%2Fcallback&' +
-        'scope=notifications&response_type=token&state='
+        'response_type=token&state=&scope=notifications'
+      )
+    })
+    it('should omit empty scopes', function () {
+      var authWithoutScopes = new ClientOAuth2({
+        clientId: config.clientId,
+        authorizationUri: config.authorizationUri,
+        authorizationGrants: ['token'],
+        redirectUri: config.redirectUri,
+        scopes: []
+      })
+      expect(authWithoutScopes.token.getUri()).to.equal(
+        config.authorizationUri + '?client_id=abc&' +
+        'redirect_uri=http%3A%2F%2Fexample.com%2Fauth%2Fcallback&' +
+        'response_type=token&state='
       )
     })
     context('when authorizationUri contains query parameters', function () {
@@ -34,7 +48,7 @@ describe('token', function () {
         expect(authWithParams.token.getUri()).to.equal(
           config.authorizationUri + '?bar=qux&client_id=abc&' +
           'redirect_uri=http%3A%2F%2Fexample.com%2Fauth%2Fcallback&' +
-          'scope=notifications&response_type=token&state='
+          'response_type=token&state=&scope=notifications'
         )
       })
     })