-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add authentication for Libraries.io-based badges, run [Libraries Bower] #7080
Changes from 4 commits
baa4cce
288c70a
a9d3e06
4790cef
e331c2e
8c912ba
e42648f
74b40ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ import Camp from '@shields_io/camp' | |
import originalJoi from 'joi' | ||
import makeBadge from '../../badge-maker/lib/make-badge.js' | ||
import GithubConstellation from '../../services/github/github-constellation.js' | ||
import LibrariesIoConstellation from '../../services/librariesio/librariesio-constellation.js' | ||
import { setRoutes } from '../../services/suggest.js' | ||
import { loadServiceClasses } from '../base-service/loader.js' | ||
import { makeSend } from '../base-service/legacy-result-sender.js' | ||
|
@@ -170,6 +171,7 @@ const privateConfigSchema = Joi.object({ | |
jira_pass: Joi.string(), | ||
bitbucket_server_username: Joi.string(), | ||
bitbucket_server_password: Joi.string(), | ||
librariesio_tokens: Joi.arrayFromString().items(Joi.string()), | ||
nexus_user: Joi.string(), | ||
nexus_pass: Joi.string(), | ||
npm_token: Joi.string(), | ||
|
@@ -242,6 +244,11 @@ class Server { | |
private: privateConfig, | ||
}) | ||
|
||
this.librariesioConstellation = new LibrariesIoConstellation({ | ||
service: publicConfig.services.librariesio, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I can see, there is no There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch. this is a leftover from the original route i'd taken of using a backing persistence for the tokens, with some of the associated (non-secret) config items going under this key There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in e42648f |
||
private: privateConfig, | ||
}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This constellation class is a borderline no-op. It originally looked a bit more like the corresponding GitHub class, but after the decision earlier today to skip persistence it was dramatically simplified. I've opted to keep the class as somewhat of an abstraction/keeping the libraries-specific instantiations inside there, but let me know if folks think it'd be better to just deal with the api provider instantiation inline here in the server. |
||
|
||
if (publicConfig.metrics.prometheus.enabled) { | ||
this.metricInstance = new PrometheusMetrics() | ||
if (publicConfig.metrics.influx.enabled) { | ||
|
@@ -414,10 +421,17 @@ class Server { | |
async registerServices() { | ||
const { config, camp, metricInstance } = this | ||
const { apiProvider: githubApiProvider } = this.githubConstellation | ||
|
||
const { apiProvider: librariesIoApiProvider } = | ||
this.librariesioConstellation | ||
;(await loadServiceClasses()).forEach(serviceClass => | ||
serviceClass.register( | ||
{ camp, handleRequest, githubApiProvider, metricInstance }, | ||
{ | ||
camp, | ||
handleRequest, | ||
githubApiProvider, | ||
librariesIoApiProvider, | ||
metricInstance, | ||
}, | ||
{ | ||
handleInternalErrors: config.public.handleInternalErrors, | ||
cacheHeaders: config.public.cacheHeaders, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,6 +80,10 @@ class Token { | |
return this.usesRemaining <= 0 && !this.hasReset | ||
} | ||
|
||
get decrementedUsesRemaining() { | ||
return this._usesRemaining - 1 | ||
} | ||
|
||
Comment on lines
+83
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The rationale/necessity for this is explained in the inline comments in the |
||
/** | ||
* Update the uses remaining and next reset time for a token. | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,15 +6,6 @@ t.create('licence') | |
.get('/bootstrap.json') | ||
.expectBadge({ label: 'license', message: 'MIT' }) | ||
|
||
t.create('license not declared') | ||
.get('/bootstrap.json') | ||
.intercept(nock => | ||
nock('https://libraries.io') | ||
.get('/api/bower/bootstrap') | ||
.reply(200, { normalized_licenses: [] }) | ||
) | ||
.expectBadge({ label: 'license', message: 'missing' }) | ||
|
||
Comment on lines
-9
to
-17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Our Bower badges had a few such mocked requests. With the hard requirement that we have the API Keys enabled in CI (we always exceed the rate limit with our ~20+ tests without auth), trying to mock these becomes increasingly difficult. We've used some nock tricks elsewhere (e.g. |
||
t.create('licence for Invalid Package') | ||
.timeout(10000) | ||
.get('/it-is-a-invalid-package-should-error.json') | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,9 +27,7 @@ class BowerVersion extends BaseBowerService { | |
|
||
static defaultBadgeData = { label: 'bower' } | ||
|
||
async handle({ packageName }, queryParams) { | ||
const data = await this.fetch({ packageName }) | ||
const includePrereleases = queryParams.include_prereleases !== undefined | ||
static transform(data, includePrereleases) { | ||
const version = includePrereleases | ||
? data.latest_release_number | ||
: data.latest_stable_release_number | ||
|
@@ -38,6 +36,14 @@ class BowerVersion extends BaseBowerService { | |
throw new InvalidResponse({ prettyMessage: 'no releases' }) | ||
} | ||
|
||
return version | ||
} | ||
|
||
async handle({ packageName }, queryParams) { | ||
const data = await this.fetch({ packageName }) | ||
const includePrereleases = queryParams.include_prereleases !== undefined | ||
const version = this.constructor.transform(data, includePrereleases) | ||
|
||
return renderVersionBadge({ version }) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. transformation logic separated out here to make it easier to use standard unit tests, because we were using mocked service tests to validate this transformation logic before. |
||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
import { expect } from 'chai' | ||
import { test, given } from 'sazerac' | ||
import nock from 'nock' | ||
import { cleanUpNockAfterEach, defaultContext } from '../test-helpers.js' | ||
import { InvalidResponse } from '../index.js' | ||
import LibrariesIoApiProvider from '../librariesio/librariesio-api-provider.js' | ||
import { BowerVersion } from './bower-version.service.js' | ||
|
||
describe('BowerVersion', function () { | ||
test(BowerVersion.transform, () => { | ||
given( | ||
{ | ||
latest_release_number: '2.0.0-beta', | ||
latest_stable_release_number: '1.8.3', | ||
}, | ||
false | ||
).expect('1.8.3') | ||
given( | ||
{ | ||
latest_release_number: '2.0.0-beta', | ||
latest_stable_release_number: '1.8.3', | ||
}, | ||
true | ||
).expect('2.0.0-beta') | ||
}) | ||
|
||
it('throws `no releases` InvalidResponse if no stable version', function () { | ||
expect(() => | ||
BowerVersion.transform({ latest_release_number: 'panda' }, false) | ||
) | ||
.to.throw(InvalidResponse) | ||
.with.property('prettyMessage', 'no releases') | ||
}) | ||
|
||
it('throws `no releases` InvalidResponse if no prereleases', function () { | ||
expect(() => | ||
BowerVersion.transform({ latest_stable_release_number: 'penguin' }, true) | ||
) | ||
.to.throw(InvalidResponse) | ||
.with.property('prettyMessage', 'no releases') | ||
}) | ||
|
||
context('auth', function () { | ||
cleanUpNockAfterEach() | ||
const fakeApiKey = 'fakeness' | ||
const response = { | ||
normalized_licenses: [], | ||
latest_release_number: '2.0.0-beta', | ||
latest_stable_release_number: '1.8.3', | ||
} | ||
const config = { | ||
private: { | ||
librariesio_tokens: fakeApiKey, | ||
}, | ||
} | ||
const librariesIoApiProvider = new LibrariesIoApiProvider({ | ||
baseUrl: 'https://libraries.io/api', | ||
tokens: [fakeApiKey], | ||
}) | ||
|
||
it('sends the auth information as configured', async function () { | ||
const scope = nock('https://libraries.io/api') | ||
// This ensures that the expected credentials are actually being sent with the HTTP request. | ||
// Without this the request wouldn't match and the test would fail. | ||
.get(`/bower/bootstrap?api_key=${fakeApiKey}`) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For whatever reason, I was unable to get nock to intercept this request when I using the options arg (e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It works in mysterious ways 🧙 |
||
.reply(200, response) | ||
|
||
expect( | ||
await BowerVersion.invoke( | ||
{ | ||
...defaultContext, | ||
librariesIoApiProvider, | ||
}, | ||
config, | ||
{ | ||
platform: 'bower', | ||
packageName: 'bootstrap', | ||
}, | ||
{ | ||
include_prereleases: '', | ||
} | ||
) | ||
).to.deep.equal({ | ||
message: 'v2.0.0-beta', | ||
color: 'orange', | ||
label: undefined, | ||
}) | ||
|
||
scope.done() | ||
}) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,24 +34,6 @@ t.create('Pre Version for Invalid Package') | |
.get('/v/it-is-a-invalid-package-should-error.json?include_prereleases') | ||
.expectBadge({ label: 'bower', message: 'package not found' }) | ||
|
||
t.create('Version label should be `no releases` if no stable version') | ||
.get('/v/bootstrap.json') | ||
.intercept(nock => | ||
nock('https://libraries.io') | ||
.get('/api/bower/bootstrap') | ||
.reply(200, { normalized_licenses: [], latest_stable_release: null }) | ||
) | ||
.expectBadge({ label: 'bower', message: 'no releases' }) | ||
|
||
t.create('Version label should be `no releases` if no pre-release') | ||
.get('/v/bootstrap.json?include_prereleases') | ||
.intercept(nock => | ||
nock('https://libraries.io') | ||
.get('/api/bower/bootstrap') | ||
.reply(200, { normalized_licenses: [], latest_release_number: null }) | ||
) | ||
.expectBadge({ label: 'bower', message: 'no releases' }) | ||
|
||
Comment on lines
-37
to
-54
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Covered with new unit tests, mocked tests removed for the aforementioned reasons |
||
t.create('Version (legacy redirect: vpre)') | ||
.get('/vpre/bootstrap.svg') | ||
.expectRedirect('/bower/v/bootstrap.svg?include_prereleases') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opted to just stick an
s
on the end to, along with the docs, convey this supports accepting multiple tokens. I'm not sure we have any prior art with this type of value beyond the various*origins
vars though, so let me know if folks have any preferred alternatives