load and manage persisted tokens with scope support

core/token-pooling/token-pool.js

@@ -188,6 +188,10 @@ class TokenPool {
     this.priorityQueue = new PriorityQueue(this.constructor.compareTokens)
   }
 
+  count() {
+    return this.tokenIds.size
+  }
+
   /**
    * compareTokens
    *

services/github/auth/acceptor.js

@@ -3,7 +3,7 @@ import request from 'request'
 import { userAgent } from '../../../core/base-service/legacy-request-handler.js'
 import log from '../../../core/server/log.js'
 
-function setRoutes({ server, authHelper, onTokenAccepted }) {
+function setRoutes({ server, authHelper, onTokenAccepted, tokenScopes }) {
   const baseUrl = process.env.GATSBY_BASE_URL || 'https://img.shields.io'
 
   server.route(/^\/github-auth$/, (data, match, end, ask) => {
@@ -15,6 +15,7 @@ function setRoutes({ server, authHelper, onTokenAccepted }) {
       // it's not setting a bad example.
       client_id: authHelper._user,
       redirect_uri: `${baseUrl}/github-auth/done`,
+      scope: tokenScopes,
     })
     ask.res.setHeader(
       'Location',

services/github/auth/acceptor.spec.js

@@ -41,6 +41,7 @@ describe('Github token acceptor', function () {
       server: camp,
       authHelper: oauthHelper,
       onTokenAccepted,
+      tokenScopes: 'read:packages',
     })
   })
 
@@ -52,6 +53,7 @@ describe('Github token acceptor', function () {
     const qs = queryString.stringify({
       client_id: fakeClientId,
       redirect_uri: 'https://img.shields.io/github-auth/done',
+      scope: 'read:packages',
     })
     const expectedLocationHeader = `https://github.com/login/oauth/authorize?${qs}`
     expect(res.headers.location).to.equal(expectedLocationHeader)

services/github/github-api-provider.integration.js

@@ -30,11 +30,10 @@ describe('Github API provider', function () {
     it('should be able to run 10 requests', async function () {
       this.timeout('20s')
       for (let i = 0; i < 10; ++i) {
-        await githubApiProvider.requestAsPromise(
+        await githubApiProvider.requestAsPromise({
           request,
-          '/repos/rust-lang/rust',
+          url: '/repos/rust-lang/rust',
-          {}
+        })
-        )
       }
     })
   })
@@ -52,11 +51,10 @@ describe('Github API provider', function () {
 
     const headers = []
     async function performOneRequest() {
-      const { res } = await githubApiProvider.requestAsPromise(
+      const { res } = await githubApiProvider.requestAsPromise({
         request,
-        '/repos/rust-lang/rust',
+        url: '/repos/rust-lang/rust',
-        {}
+      })
-      )
       expect(res.statusCode).to.equal(200)
       headers.push(res.headers)
     }

services/github/github-api-provider.js

@@ -38,6 +38,7 @@ class GithubApiProvider {
     onTokenInvalidated = tokenString => {},
     globalToken,
     reserveFraction = 0.25,
+    tokenScopeNames = {},
   }) {
     Object.assign(this, {
       baseUrl,
@@ -45,12 +46,14 @@ class GithubApiProvider {
       onTokenInvalidated,
       globalToken,
       reserveFraction,
+      tokenScopeNames,
     })
 
     if (this.withPooling) {
       this.standardTokens = new TokenPool({ batchSize: 25 })
       this.searchTokens = new TokenPool({ batchSize: 5 })
       this.graphqlTokens = new TokenPool({ batchSize: 25 })
+      this.packageScopedTokens = new TokenPool({ batchSize: 25 })
     }
   }
 
@@ -60,17 +63,41 @@ class GithubApiProvider {
         standardTokens: this.standardTokens.serializeDebugInfo({ sanitize }),
         searchTokens: this.searchTokens.serializeDebugInfo({ sanitize }),
         graphqlTokens: this.graphqlTokens.serializeDebugInfo({ sanitize }),
+        packageScopedTokens: this.packageScopedTokens.serializeDebugInfo({
+          sanitize,
+        }),
       }
     } else {
       return {}
     }
   }
 
-  addToken(tokenString) {
+  numReservedScopedTokens() {
+    return this.packageScopedTokens.count()
+  }
+
+  addReservedScopedToken(tokenString, data) {
+    if (!this.withPooling) {
+      throw Error('When not using a token pool, do not provide tokens')
+    }
+
+    const { scopes } = data
+    if (!scopes) {
+      throw new Error('Cannot add unscoped token to reserved token pools')
+    }
+
+    scopes.split('%20').forEach(scope => {
+      if (scope === this.tokenScopeNames.readPackages) {
+        this.packageScopedTokens.add(tokenString, data)
+      }
+    })
+  }
+
+  addToken(tokenString, data) {
     if (this.withPooling) {
-      this.standardTokens.add(tokenString)
+      this.standardTokens.add(tokenString, data)
-      this.searchTokens.add(tokenString)
+      this.searchTokens.add(tokenString, data)
-      this.graphqlTokens.add(tokenString)
+      this.graphqlTokens.add(tokenString, data)
     } else {
       throw Error('When not using a token pool, do not provide tokens')
     }
@@ -141,7 +168,11 @@ class GithubApiProvider {
     this.onTokenInvalidated(token.id)
   }
 
-  tokenForUrl(url) {
+  tokenForUrl(url, { needsPackageScope }) {
+    if (needsPackageScope) {
+      return this.packageScopedTokens.next()
+    }
+
     if (url.startsWith('/search')) {
       return this.searchTokens.next()
     } else if (url.startsWith('/graphql')) {
@@ -154,14 +185,14 @@ class GithubApiProvider {
   // Act like request(), but tweak headers and query to avoid hitting a rate
   // limit. Inject `request` so we can pass in `cachingRequest` from
   // `request-handler.js`.
-  request(request, url, options = {}, callback) {
+  request({ request, url, options = {}, neededScopes = {}, callback }) {
     const { baseUrl } = this
 
     let token
     let tokenString
     if (this.withPooling) {
       try {
-        token = this.tokenForUrl(url)
+        token = this.tokenForUrl(url, neededScopes)
       } catch (e) {
         callback(e)
         return
@@ -198,14 +229,20 @@ class GithubApiProvider {
     })
   }
 
-  requestAsPromise(request, url, options) {
+  requestAsPromise({ request, url, options, neededScopes }) {
     return new Promise((resolve, reject) => {
-      this.request(request, url, options, (err, res, buffer) => {
+      this.request({
-        if (err) {
+        request,
-          reject(err)
+        url,
-        } else {
+        options,
-          resolve({ res, buffer })
+        neededScopes,
-        }
+        callback: (err, res, buffer) => {
+          if (err) {
+            reject(err)
+          } else {
+            resolve({ res, buffer })
+          }
+        },
       })
     })
   }

services/github/github-api-provider.spec.js

@@ -6,7 +6,11 @@ describe('Github API provider', function () {
   const baseUrl = 'https://github-api.example.com'
   const reserveFraction = 0.333
 
-  let mockStandardToken, mockSearchToken, mockGraphqlToken, provider
+  let mockStandardToken,
+    mockSearchToken,
+    mockGraphqlToken,
+    mockPackagesScopedToken,
+    provider
   beforeEach(function () {
     provider = new GithubApiProvider({ baseUrl, reserveFraction })
 
@@ -18,6 +22,11 @@ describe('Github API provider', function () {
 
     mockGraphqlToken = { update: sinon.spy(), invalidate: sinon.spy() }
     sinon.stub(provider.graphqlTokens, 'next').returns(mockGraphqlToken)
+
+    mockPackagesScopedToken = { update: sinon.spy(), invalidate: sinon.spy() }
+    sinon
+      .stub(provider.packageScopedTokens, 'next')
+      .returns(mockPackagesScopedToken)
   })
 
   context('a search API request', function () {
@@ -25,12 +34,16 @@ describe('Github API provider', function () {
       callback()
     }
     it('should obtain an appropriate token', function (done) {
-      provider.request(mockRequest, '/search', {}, (err, res, buffer) => {
+      provider.request({
-        expect(err).to.be.undefined
+        request: mockRequest,
-        expect(provider.searchTokens.next).to.have.been.calledOnce
+        url: '/search',
-        expect(provider.standardTokens.next).not.to.have.been.called
+        callback: (err, res, buffer) => {
-        expect(provider.graphqlTokens.next).not.to.have.been.called
+          expect(err).to.be.undefined
-        done()
+          expect(provider.searchTokens.next).to.have.been.calledOnce
+          expect(provider.standardTokens.next).not.to.have.been.called
+          expect(provider.graphqlTokens.next).not.to.have.been.called
+          done()
+        },
       })
     })
   })
@@ -40,12 +53,37 @@ describe('Github API provider', function () {
       callback()
     }
     it('should obtain an appropriate token', function (done) {
-      provider.request(mockRequest, '/graphql', {}, (err, res, buffer) => {
+      provider.request({
-        expect(err).to.be.undefined
+        request: mockRequest,
-        expect(provider.searchTokens.next).not.to.have.been.called
+        url: '/graphql',
-        expect(provider.standardTokens.next).not.to.have.been.called
+        callback: (err, res, buffer) => {
-        expect(provider.graphqlTokens.next).to.have.been.calledOnce
+          expect(err).to.be.undefined
-        done()
+          expect(provider.searchTokens.next).not.to.have.been.called
+          expect(provider.standardTokens.next).not.to.have.been.called
+          expect(provider.graphqlTokens.next).to.have.been.calledOnce
+          done()
+        },
+      })
+    })
+  })
+
+  context('a request requiring the read:packages scope', function () {
+    const mockRequest = (options, callback) => {
+      callback()
+    }
+    it('should obtain an appropriate token', function (done) {
+      provider.request({
+        request: mockRequest,
+        url: '/graphql',
+        neededScopes: { needsPackageScope: true },
+        callback: (err, res, buffer) => {
+          expect(err).to.be.undefined
+          expect(provider.searchTokens.next).not.to.have.been.called
+          expect(provider.standardTokens.next).not.to.have.been.called
+          expect(provider.graphqlTokens.next).not.to.have.been.called
+          expect(provider.packageScopedTokens.next).to.have.been.calledOnce
+          done()
+        },
       })
     })
   })
@@ -55,12 +93,16 @@ describe('Github API provider', function () {
       callback()
     }
     it('should obtain an appropriate token', function (done) {
-      provider.request(mockRequest, '/repo', {}, (err, res, buffer) => {
+      provider.request({
-        expect(err).to.be.undefined
+        request: mockRequest,
-        expect(provider.searchTokens.next).not.to.have.been.called
+        url: '/repo',
-        expect(provider.standardTokens.next).to.have.been.calledOnce
+        callback: (err, res, buffer) => {
-        expect(provider.graphqlTokens.next).not.to.have.been.called
+          expect(err).to.be.undefined
-        done()
+          expect(provider.searchTokens.next).not.to.have.been.called
+          expect(provider.standardTokens.next).to.have.been.calledOnce
+          expect(provider.graphqlTokens.next).not.to.have.been.called
+          done()
+        },
       })
     })
   })
@@ -84,25 +126,33 @@ describe('Github API provider', function () {
     }
 
     it('should invoke the callback', function (done) {
-      provider.request(mockRequest, '/foo', {}, (err, res, buffer) => {
+      provider.request({
-        expect(err).to.equal(null)
+        request: mockRequest,
-        expect(Object.is(res, mockResponse)).to.be.true
+        url: '/foo',
-        expect(Object.is(buffer, mockBuffer)).to.be.true
+        callback: (err, res, buffer) => {
-        done()
+          expect(err).to.equal(null)
+          expect(Object.is(res, mockResponse)).to.be.true
+          expect(Object.is(buffer, mockBuffer)).to.be.true
+          done()
+        },
       })
     })
 
     it('should update the token with the expected values', function (done) {
-      provider.request(mockRequest, '/foo', {}, (err, res, buffer) => {
+      provider.request({
-        expect(err).to.equal(null)
+        request: mockRequest,
-        const expectedUsesRemaining =
+        url: '/foo',
-          remaining - Math.ceil(reserveFraction * rateLimit)
+        callback: (err, res, buffer) => {
-        expect(mockStandardToken.update).to.have.been.calledWith(
+          expect(err).to.equal(null)
-          expectedUsesRemaining,
+          const expectedUsesRemaining =
-          nextReset
+            remaining - Math.ceil(reserveFraction * rateLimit)
-        )
+          expect(mockStandardToken.update).to.have.been.calledWith(
-        expect(mockStandardToken.invalidate).not.to.have.been.called
+            expectedUsesRemaining,
-        done()
+            nextReset
+          )
+          expect(mockStandardToken.invalidate).not.to.have.been.called
+          done()
+        },
       })
     })
   })
@@ -132,25 +182,33 @@ describe('Github API provider', function () {
     }
 
     it('should invoke the callback', function (done) {
-      provider.request(mockRequest, '/graphql', {}, (err, res, buffer) => {
+      provider.request({
-        expect(err).to.equal(null)
+        request: mockRequest,
-        expect(Object.is(res, mockResponse)).to.be.true
+        url: '/graphql',
-        expect(Object.is(buffer, mockBuffer)).to.be.true
+        callback: (err, res, buffer) => {
-        done()
+          expect(err).to.equal(null)
+          expect(Object.is(res, mockResponse)).to.be.true
+          expect(Object.is(buffer, mockBuffer)).to.be.true
+          done()
+        },
       })
     })
 
     it('should update the token with the expected values', function (done) {
-      provider.request(mockRequest, '/graphql', {}, (err, res, buffer) => {
+      provider.request({
-        expect(err).to.equal(null)
+        request: mockRequest,
-        const expectedUsesRemaining =
+        url: '/graphql',
-          remaining - Math.ceil(reserveFraction * rateLimit)
+        callback: (err, res, buffer) => {
-        expect(mockGraphqlToken.update).to.have.been.calledWith(
+          expect(err).to.equal(null)
-          expectedUsesRemaining,
+          const expectedUsesRemaining =
-          nextReset
+            remaining - Math.ceil(reserveFraction * rateLimit)
-        )
+          expect(mockGraphqlToken.update).to.have.been.calledWith(
-        expect(mockGraphqlToken.invalidate).not.to.have.been.called
+            expectedUsesRemaining,
-        done()
+            nextReset
+          )
+          expect(mockGraphqlToken.invalidate).not.to.have.been.called
+          done()
+        },
       })
     })
   })
@@ -164,11 +222,15 @@ describe('Github API provider', function () {
     }
 
     it('should invoke the callback and update the token with the expected values', function (done) {
-      provider.request(mockRequest, '/foo', {}, (err, res, buffer) => {
+      provider.request({
-        expect(err).to.equal(null)
+        request: mockRequest,
-        expect(mockStandardToken.invalidate).to.have.been.calledOnce
+        url: '/foo',
-        expect(mockStandardToken.update).not.to.have.been.called
+        callback: (err, res, buffer) => {
-        done()
+          expect(err).to.equal(null)
+          expect(mockStandardToken.invalidate).to.have.been.calledOnce
+          expect(mockStandardToken.update).not.to.have.been.called
+          done()
+        },
       })
     })
   })
@@ -180,10 +242,14 @@ describe('Github API provider', function () {
     }
 
     it('should pass the error to the callback', function (done) {
-      provider.request(mockRequest, '/foo', {}, (err, res, buffer) => {
+      provider.request({
-        expect(err).to.be.an.instanceof(Error)
+        request: mockRequest,
-        expect(err.message).to.equal('connection timeout')
+        url: '/foo',
-        done()
+        callback: (err, res, buffer) => {
+          expect(err).to.be.an.instanceof(Error)
+          expect(err.message).to.equal('connection timeout')
+          done()
+        },
       })
     })
   })

services/github/github-auth-service.js

@@ -2,21 +2,22 @@ import gql from 'graphql-tag'
 import { mergeQueries } from '../../core/base-service/graphql.js'
 import { BaseGraphqlService, BaseJsonService } from '../index.js'
 
-function createRequestFetcher(context, config) {
+function createRequestFetcher(context, config, neededScopes) {
   const { sendAndCacheRequestWithCallbacks, githubApiProvider } = context
 
   return async (url, options) =>
-    githubApiProvider.requestAsPromise(
+    githubApiProvider.requestAsPromise({
-      sendAndCacheRequestWithCallbacks,
+      request: sendAndCacheRequestWithCallbacks,
       url,
-      options
+      options,
-    )
+      neededScopes,
+    })
 }
 
 class GithubAuthV3Service extends BaseJsonService {
-  constructor(context, config) {
+  constructor(context, config, neededScopes) {
     super(context, config)
-    this._requestFetcher = createRequestFetcher(context, config)
+    this._requestFetcher = createRequestFetcher(context, config, neededScopes)
     this.staticAuthConfigured = true
   }
 }
@@ -27,10 +28,10 @@ class GithubAuthV3Service extends BaseJsonService {
 // useful when consuming GitHub endpoints which are not rate-limited: it
 // avoids wasting API quota on them in production.
 class ConditionalGithubAuthV3Service extends BaseJsonService {
-  constructor(context, config) {
+  constructor(context, config, neededScopes) {
     super(context, config)
     if (context.githubApiProvider.globalToken) {
-      this._requestFetcher = createRequestFetcher(context, config)
+      this._requestFetcher = createRequestFetcher(context, config, neededScopes)
       this.staticAuthConfigured = true
     } else {
       this.staticAuthConfigured = false
@@ -39,9 +40,9 @@ class ConditionalGithubAuthV3Service extends BaseJsonService {
 }
 
 class GithubAuthV4Service extends BaseGraphqlService {
-  constructor(context, config) {
+  constructor(context, config, neededScopes) {
     super(context, config)
-    this._requestFetcher = createRequestFetcher(context, config)
+    this._requestFetcher = createRequestFetcher(context, config, neededScopes)
     this.staticAuthConfigured = true
   }
 

services/github/github-auth-service.spec.js

@@ -25,17 +25,32 @@ describe('GithubAuthV3Service', function () {
     }
   }
 
-  it('forwards custom Accept header', async function () {
+  class ScopedDummyGithubAuthV3Service extends DummyGithubAuthV3Service {
-    const sendAndCacheRequestWithCallbacks = sinon.stub().returns(
+    constructor(context, config) {
+      super(context, config, { needsPackageScope: true })
+    }
+  }
+
+  let sendAndCacheRequestWithCallbacks, mockToken
+  const githubApiProvider = new GithubApiProvider({
+    baseUrl: 'https://github-api.example.com',
+  })
+
+  beforeEach(function () {
+    sendAndCacheRequestWithCallbacks = sinon.stub().returns(
       Promise.resolve({
         buffer: '{"requiredString": "some-string"}',
         res: { statusCode: 200 },
       })
     )
-    const githubApiProvider = new GithubApiProvider({
+    mockToken = { id: 'abc123', update: sinon.mock(), invalidate: sinon.mock() }
-      baseUrl: 'https://github-api.example.com',
+  })
-    })
+
-    const mockToken = { update: sinon.mock(), invalidate: sinon.mock() }
+  afterEach(function () {
+    sinon.restore()
+  })
+
+  it('forwards custom Accept header', async function () {
     sinon.stub(githubApiProvider.standardTokens, 'next').returns(mockToken)
 
     DummyGithubAuthV3Service.invoke({
@@ -47,7 +62,26 @@ describe('GithubAuthV3Service', function () {
       headers: {
         'User-Agent': 'Shields.io/2003a',
         Accept: 'application/vnd.github.antiope-preview+json',
-        Authorization: 'token undefined',
+        Authorization: 'token abc123',
+      },
+      url: 'https://github-api.example.com/repos/badges/shields/check-runs',
+      baseUrl: 'https://github-api.example.com',
+    })
+  })
+
+  it('uses token with correct read scope', function () {
+    sinon.stub(githubApiProvider.packageScopedTokens, 'next').returns(mockToken)
+
+    ScopedDummyGithubAuthV3Service.invoke({
+      sendAndCacheRequestWithCallbacks,
+      githubApiProvider,
+    })
+
+    expect(sendAndCacheRequestWithCallbacks).to.have.been.calledOnceWith({
+      headers: {
+        'User-Agent': 'Shields.io/2003a',
+        Accept: 'application/vnd.github.antiope-preview+json',
+        Authorization: 'token abc123',
       },
       url: 'https://github-api.example.com/repos/badges/shields/check-runs',
       baseUrl: 'https://github-api.example.com',

services/github/github-constellation.js

@@ -5,6 +5,11 @@ import GithubApiProvider from './github-api-provider.js'
 import { setRoutes as setAdminRoutes } from './auth/admin.js'
 import { setRoutes as setAcceptorRoutes } from './auth/acceptor.js'
 
+const readPackagesScope = 'read:packages'
+// Multiple scopes need to be uri-encoded space delimited
+const tokenScopes = `${readPackagesScope}`
+const persistenceScopeDelimiter = '.scopes.'
+
 // Convenience class with all the stuff related to the Github API and its
 // authorization tokens, to simplify server initialization.
 class GithubConstellation {
@@ -24,6 +29,8 @@ class GithubConstellation {
     this._debugEnabled = config.service.debug.enabled
     this._debugIntervalSeconds = config.service.debug.intervalSeconds
     this.shieldsSecret = config.private.shields_secret
+    this._tokenScopes = {}
+    this._maxNumReservedScopedTokens = 0
 
     const { redis_url: redisUrl, gh_token: globalToken } = config.private
     if (redisUrl) {
@@ -38,6 +45,9 @@ class GithubConstellation {
       baseUrl: process.env.GITHUB_URL || 'https://api.github.com',
       globalToken,
       withPooling: !globalToken,
+      tokenScopeNames: {
+        readPackages: readPackagesScope,
+      },
       onTokenInvalidated: tokenString => this.onTokenInvalidated(tokenString),
     })
 
@@ -70,8 +80,21 @@ class GithubConstellation {
       log.error(e)
     }
 
+    // Reserve a subset of scoped tokens from the total set
+    // to be used for queries which require an explicit scope,
+    // while leaving a sufficient amount of tokens (scoped or unscoped)
+    // for the bulk of our requests which don't care about scopes.
+    this._maxNumReservedScopedTokens = Math.floor(tokens.length * 0.15)
     tokens.forEach(tokenString => {
-      this.apiProvider.addToken(tokenString)
+      const [token, scopes] = tokenString.split(persistenceScopeDelimiter)
+      this._tokenScopes[token] = scopes || null
+      const data = { scopes }
+      const numReserved = this.apiProvider.numReservedScopedTokens()
+      if (scopes && numReserved < this._maxNumReservedScopedTokens) {
+        this.apiProvider.addReservedScopedToken(token, data)
+      } else {
+        this.apiProvider.addToken(token, data)
+      }
     })
 
     const { shieldsSecret, apiProvider } = this
@@ -81,19 +104,53 @@ class GithubConstellation {
       setAcceptorRoutes({
         server,
         authHelper: this.oauthHelper,
-        onTokenAccepted: tokenString => this.onTokenAdded(tokenString),
+        tokenScopes,
+        onTokenAccepted: tokenString =>
+          this.onTokenAdded(tokenString, tokenScopes),
       })
     }
   }
 
-  onTokenAdded(tokenString) {
+  onTokenAdded(tokenString, tokenScopes) {
     if (!this.persistence) {
       throw Error('Token persistence is not configured')
     }
-    this.apiProvider.addToken(tokenString)
+    const data = { scopes: tokenScopes }
+    const numReserved = this.apiProvider.numReservedScopedTokens()
+    if (numReserved < this._maxNumReservedScopedTokens) {
+      this.apiProvider.addReservedScopedToken(tokenString, data)
+    } else {
+      this.apiProvider.addToken(tokenString, data)
+    }
+
     process.nextTick(async () => {
       try {
-        await this.persistence.noteTokenAdded(tokenString)
+        // To avoid having multiple set entries for re-authorized/re-scoped
+        // tokens we need to first remove the previous entry that had different scopes
+        if (
+          Object.prototype.hasOwnProperty.call(this._tokenScopes, tokenString)
+        ) {
+          const currentScopes = this._tokenScopes[tokenString]
+          // These scopes shouldn't match in practice, as that would
+          // indicate the function has somehow been invoked with an existing
+          // token but without any scope changes. Nevertheless, the conditional
+          // guard is here in case there are circumstances that assumption fails
+          // to be upheld.
+          if (currentScopes !== tokenScopes) {
+            const token = currentScopes
+              ? `${tokenString}${persistenceScopeDelimiter}${currentScopes}`
+              : tokenString
+            await this.persistence.noteTokenRemoved(token)
+          }
+        }
+        // It's unlikely that we'd evert revert back to no longer requesting any scopes
+        // but handling that scenario regardless so we don't end up
+        // with junk like `abc123.scopes.undefined` in redis
+        const token = tokenScopes
+          ? `${tokenString}${persistenceScopeDelimiter}${tokenScopes}`
+          : tokenString
+        await this.persistence.noteTokenAdded(token)
+        this._tokenScopes[tokenString] = tokenScopes || null
       } catch (e) {
         log.error(e)
       }
@@ -104,7 +161,12 @@ class GithubConstellation {
     if (this.persistence) {
       process.nextTick(async () => {
         try {
-          await this.persistence.noteTokenRemoved(tokenString)
+          const scopes = this._tokenScopes[tokenString]
+          const token = scopes
+            ? `${tokenString}${persistenceScopeDelimiter}${scopes}`
+            : tokenString
+          await this.persistence.noteTokenRemoved(token)
+          delete this._tokenScopes[tokenString]
         } catch (e) {
           log.error(e)
         }

services/github/github-constellation.spec.js

@@ -0,0 +1,208 @@
+import { expect } from 'chai'
+import sinon from 'sinon'
+import log from '../../core/server/log.js'
+import RedisTokenPersistence from '../../core/token-pooling/redis-token-persistence.js'
+import GithubConstellation from './github-constellation.js'
+import GithubApiProvider from './github-api-provider.js'
+
+describe('GithubConstellation', function () {
+  const tokens = [
+    'abc123',
+    'def4567.scopes.read:packages%20read:user',
+    'def789.scopes.read:packages',
+    'ghi012',
+    'fff444.scopes.read:packages',
+    '555eee.scopes.read:packages',
+    'ddd666',
+    '777ccc',
+    'bbb888',
+    '999aaa',
+    '000111.scopes.read:packages',
+    '222333.scopes.read:packages',
+    '111111',
+    '888888',
+  ]
+  const config = {
+    private: {
+      redis_url: 'localhost',
+    },
+    service: {
+      debug: {
+        enabled: false,
+      },
+    },
+  }
+  const server = { ajax: { on: sinon.stub() } }
+
+  beforeEach(function () {
+    sinon.stub(log, 'log')
+    sinon
+      .stub(GithubConstellation, '_createOauthHelper')
+      .returns({ isConfigured: false })
+    sinon.stub(GithubConstellation.prototype, 'scheduleDebugLogging')
+    sinon.stub(RedisTokenPersistence.prototype, 'initialize').returns(tokens)
+    sinon.stub(RedisTokenPersistence.prototype, 'noteTokenAdded')
+    sinon.stub(RedisTokenPersistence.prototype, 'noteTokenRemoved')
+    sinon.spy(GithubApiProvider.prototype, 'addToken')
+    sinon.spy(GithubApiProvider.prototype, 'addReservedScopedToken')
+  })
+
+  afterEach(function () {
+    sinon.restore()
+  })
+
+  context('initialize', function () {
+    it('does not fetch tokens when pooling disabled', async function () {
+      const constellation = new GithubConstellation({
+        ...config,
+        ...{ private: { gh_token: 'secret' } },
+      })
+      await constellation.initialize(server)
+      expect(RedisTokenPersistence.prototype.initialize).not.to.have.been.called
+    })
+
+    it('loads both scoped and unscoped tokens', async function () {
+      const constellation = new GithubConstellation(config)
+      await constellation.initialize(server)
+      expect(constellation.apiProvider.graphqlTokens.count()).to.equal(12)
+      expect(constellation.apiProvider.searchTokens.count()).to.equal(12)
+      expect(constellation.apiProvider.standardTokens.count()).to.equal(12)
+      expect(constellation.apiProvider.packageScopedTokens.count()).to.equal(2)
+      expect(
+        GithubApiProvider.prototype.addReservedScopedToken
+      ).to.be.calledWithExactly('def4567', {
+        scopes: 'read:packages%20read:user',
+      })
+      expect(
+        GithubApiProvider.prototype.addReservedScopedToken
+      ).to.be.calledWithExactly('def789', {
+        scopes: 'read:packages',
+      })
+    })
+  })
+
+  context('onTokenAdded', function () {
+    it('adds new scoped token with met reserves', async function () {
+      const token = 'shh_secret'
+      sinon
+        .stub(GithubApiProvider.prototype, 'numReservedScopedTokens')
+        .returns(2)
+      const clock = sinon.useFakeTimers()
+      const constellation = new GithubConstellation(config)
+      await constellation.initialize(server)
+      constellation._maxNumReservedScopedTokens = 2
+      constellation.onTokenAdded(token, 'read:packages')
+      await clock.tickAsync()
+      expect(GithubApiProvider.prototype.addToken).to.be.calledWithExactly(
+        token,
+        { scopes: 'read:packages' }
+      )
+      expect(
+        GithubApiProvider.prototype.addReservedScopedToken
+      ).to.not.be.calledWith(token)
+      expect(RedisTokenPersistence.prototype.noteTokenAdded).to.be.calledWith(
+        `${token}.scopes.read:packages`
+      )
+      expect(RedisTokenPersistence.prototype.noteTokenRemoved).to.not.be.called
+      expect(Object.keys(constellation._tokenScopes).length).to.equal(15)
+      expect(constellation._tokenScopes[token]).to.equal('read:packages')
+    })
+
+    it('adds new scoped token with unmet reserves', async function () {
+      const token = 'shh_secret'
+      sinon
+        .stub(GithubApiProvider.prototype, 'numReservedScopedTokens')
+        .returns(2)
+      const clock = sinon.useFakeTimers()
+      const constellation = new GithubConstellation(config)
+      await constellation.initialize(server)
+      constellation._maxNumReservedScopedTokens = 3
+      constellation.onTokenAdded(token, 'read:packages')
+      await clock.tickAsync()
+      expect(
+        GithubApiProvider.prototype.addReservedScopedToken
+      ).to.be.calledWithExactly(token, { scopes: 'read:packages' })
+      expect(GithubApiProvider.prototype.addToken).to.not.be.calledWith(token)
+      expect(RedisTokenPersistence.prototype.noteTokenAdded).to.be.calledWith(
+        `${token}.scopes.read:packages`
+      )
+      expect(RedisTokenPersistence.prototype.noteTokenRemoved).to.not.be.called
+      expect(Object.keys(constellation._tokenScopes).length).to.equal(15)
+      expect(constellation._tokenScopes[token]).to.equal('read:packages')
+    })
+
+    it('adds new unscoped token', async function () {
+      const token = '1234567890987654321'
+      const clock = sinon.useFakeTimers()
+      const constellation = new GithubConstellation(config)
+      await constellation.initialize(server)
+      constellation.onTokenAdded(token)
+      await clock.tickAsync()
+      expect(GithubApiProvider.prototype.addToken).to.be.calledWithExactly(
+        token,
+        { scopes: undefined }
+      )
+      expect(
+        GithubApiProvider.prototype.addReservedScopedToken
+      ).to.not.be.calledWith(token)
+      expect(RedisTokenPersistence.prototype.noteTokenAdded).to.be.calledWith(
+        token
+      )
+      expect(RedisTokenPersistence.prototype.noteTokenRemoved).to.not.be.called
+      expect(Object.keys(constellation._tokenScopes).length).to.equal(15)
+      expect(constellation._tokenScopes[token]).to.equal(null)
+    })
+
+    it('updates scopes on existing token', async function () {
+      const existingToken = 'abc123'
+      const clock = sinon.useFakeTimers()
+      const constellation = new GithubConstellation(config)
+      await constellation.initialize(server)
+      sinon
+        .stub(GithubApiProvider.prototype, 'numReservedScopedTokens')
+        .returns(1)
+      constellation.onTokenAdded(existingToken, 'read:packages')
+      await clock.tickAsync()
+      expect(
+        GithubApiProvider.prototype.addReservedScopedToken
+      ).to.be.calledWithExactly(existingToken, { scopes: 'read:packages' })
+      expect(GithubApiProvider.prototype.addToken.callCount).to.equal(12)
+      expect(RedisTokenPersistence.prototype.noteTokenAdded).to.be.calledWith(
+        `${existingToken}.scopes.read:packages`
+      )
+      expect(RedisTokenPersistence.prototype.noteTokenRemoved).to.be.calledWith(
+        existingToken
+      )
+      expect(Object.keys(constellation._tokenScopes).length).to.equal(14)
+      expect(constellation._tokenScopes[existingToken]).to.equal(
+        'read:packages'
+      )
+    })
+  })
+
+  context('onTokenInvalidated', function () {
+    it('removes scoped token', async function () {
+      const clock = sinon.useFakeTimers()
+      const constellation = new GithubConstellation(config)
+      await constellation.initialize(server)
+      constellation.onTokenInvalidated('def789')
+      await clock.tickAsync()
+      expect(RedisTokenPersistence.prototype.noteTokenRemoved).to.be.calledWith(
+        'def789.scopes.read:packages'
+      )
+      expect(Object.keys(constellation._tokenScopes).length).to.equal(13)
+    })
+
+    it('removes unscoped token', async function () {
+      const clock = sinon.useFakeTimers()
+      const constellation = new GithubConstellation(config)
+      await constellation.initialize(server)
+      constellation.onTokenInvalidated('888888')
+      await clock.tickAsync()
+      expect(
+        RedisTokenPersistence.prototype.noteTokenRemoved
+      ).to.be.calledWithExactly('888888')
+      expect(Object.keys(constellation._tokenScopes).length).to.equal(13)
+    })
+  })
+})

services/suggest.js

@@ -75,10 +75,10 @@ async function githubLicense(githubApiProvider, user, repo) {
 
   let link = `https://github.com/${repoSlug}`
 
-  const { buffer } = await githubApiProvider.requestAsPromise(
+  const { buffer } = await githubApiProvider.requestAsPromise({
     request,
-    `/repos/${repoSlug}/license`
+    url: `/repos/${repoSlug}/license`,
-  )
+  })
   try {
     const data = JSON.parse(buffer)
     if ('html_url' in data) {