feat: add OAuth Bearer token support to HttpClient#1168
Conversation
- Allows callers to authenticate with the MusicBrainz API using an OAuth access token, avoiding the need to manage session cookies or digest auth. - Token is passed via config and injected as an Authorization. - Bearer header on every request, only if not already set.
There was a problem hiding this comment.
Pull request overview
This PR adds first-class support for OAuth2 Bearer token authentication by introducing an optional accessToken configuration value and having the HTTP layer automatically inject Authorization: Bearer <token> when configured.
Changes:
- Added
accessToken?: stringtoIMusicBrainzConfigand passed it intoHttpClientinitialization. - Updated
HttpClientto set theAuthorizationheader with a Bearer token when configured (unless an Authorization header is already present). - Documented the new config option and added unit tests asserting header injection behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
lib/musicbrainz-api.ts |
Adds accessToken to config and threads it into HttpClient construction. |
lib/http-client.ts |
Injects Authorization: Bearer ... when accessToken is configured and no Authorization header is present. |
README.md |
Documents accessToken usage in configuration example. |
test/test-musicbrainz-api.ts |
Adds tests for Bearer header injection and absence when not configured. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -252,7 +257,8 @@ export class MusicBrainzApi { | |||
| return new HttpClient({ | |||
| baseUrl: this.config.baseUrl, | |||
| timeout: 500, | |||
| userAgent: `${this.config.appName}/${this.config.appVersion} ( ${this.config.appContactInfo} )` | |||
| userAgent: `${this.config.appName}/${this.config.appVersion} ( ${this.config.appContactInfo} )`, | |||
| accessToken: this.config.accessToken | |||
| }); | |||
There was a problem hiding this comment.
Good catch, will address this.
| @@ -252,7 +257,8 @@ export class MusicBrainzApi { | |||
| return new HttpClient({ | |||
| baseUrl: this.config.baseUrl, | |||
| timeout: 500, | |||
| userAgent: `${this.config.appName}/${this.config.appVersion} ( ${this.config.appContactInfo} )` | |||
| userAgent: `${this.config.appName}/${this.config.appVersion} ( ${this.config.appContactInfo} )`, | |||
| accessToken: this.config.accessToken | |||
| }); | |||
| it('sends configured accessToken as Authorization Bearer header', async () => { | ||
| const accessToken = 'test-access-token'; | ||
| const mbApi = new MusicBrainzApi(await makeSearchApiConfig({ | ||
| accessToken, | ||
| disableRateLimiting: true | ||
| })); | ||
| const fetchStub = sinon.stub(globalThis, 'fetch').resolves(new Response('{}')); | ||
|
|
||
| await mbApi.restGet<IArtist>(`/artist/${mbid.artist.Stromae}`); | ||
|
|
||
| assert.isTrue(fetchStub.calledOnce); | ||
| const requestInit = fetchStub.firstCall.args[1] as RequestInit; | ||
| const headers = requestInit.headers as Headers; | ||
| assert.strictEqual(headers.get('Authorization'), `Bearer ${accessToken}`); | ||
| }); |
|
Dear @elgeeko1, First of all, sorry for not being more involved so far. Thank you for your nice contribution. I think there may be a number of functions that have been ignored for quite some time and could also benefit from the OAuth functionality: musicbrainz-api/test/test-musicbrainz-api.ts Lines 1549 to 1632 in 8d47c86 If you are willing to look into those as well, that would be very welcome. Maybe as a follow up? Thanks again! |
| import { RateLimitThreshold } from 'rate-limit-threshold'; | ||
| import * as mb from './musicbrainz.types.js'; | ||
| import { HttpClient, type MultiQueryFormData } from "./http-client.js"; | ||
| import {HttpClient, IHttpClientOptions, type MultiQueryFormData} from "./http-client.js"; |
| protected getHttpClientOptions(): IHttpClientOptions { | ||
| return { | ||
| baseUrl: this.config.baseUrl, | ||
| timeout: 500, | ||
| userAgent: `${this.config.appName}/${this.config.appVersion} ( ${this.config.appContactInfo} )` | ||
| }); | ||
| userAgent: `${this.config.appName}/${this.config.appVersion} ( ${this.config.appContactInfo} )`, | ||
| accessToken: this.config.accessToken, | ||
| }; | ||
| } |
|
|
||
|
|
| import { RateLimitThreshold } from 'rate-limit-threshold'; | ||
| import * as mb from './musicbrainz.types.js'; | ||
| import { HttpClient, type MultiQueryFormData } from "./http-client.js"; | ||
| import {HttpClient, IHttpClientOptions, type MultiQueryFormData} from "./http-client.js"; |
There was a problem hiding this comment.
Fixed in commit 86c51d5: IHttpClientOptions (and MultiQueryFormData) are now imported with type to match the repo's conventions and avoid any runtime import side effects.
| protected getHttpClientOptions(): IHttpClientOptions { | ||
| return { | ||
| baseUrl: this.config.baseUrl, | ||
| timeout: 500, | ||
| userAgent: `${this.config.appName}/${this.config.appVersion} ( ${this.config.appContactInfo} )` | ||
| }); | ||
| userAgent: `${this.config.appName}/${this.config.appVersion} ( ${this.config.appContactInfo} )`, | ||
| accessToken: this.config.accessToken, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Fixed in commit 86c51d5: the constructor now throws immediately when accessToken is set alongside a non-HTTPS baseUrl. Additionally, post() was updated to fast-fail when neither accessToken nor botAccount credentials are present, and when an accessToken is available it submits XML once with Authorization: Bearer and Content-Type: application/xml, bypassing the Digest challenge loop entirely.
|
|
||
|
|
There was a problem hiding this comment.
Fixed in commit 3e0868c, which adds tests covering: (1) a caller-supplied Authorization header is not overwritten by the configured Bearer token; (2) a whitespace-only Authorization value is treated as absent and replaced with Bearer; (3) constructing MusicBrainzApi with accessToken and a non-HTTPS baseUrl throws; (4) post() sends exactly one request with the Bearer token and Content-Type: application/xml; (5) post() throws when neither accessToken nor bot credentials are configured. A follow-up commit (f649ffc) also adds the equivalent ISRC-submission tests for the Node entrypoint.
|
@Borewit I'm happy to look into auth for additional functions. Will follow-up this week. |
- Use type-only import for IHttpClientOptions and MultiQueryFormData - Throw early when accessToken is configured with a non-HTTPS baseUrl (RFC 6750 §5.3: Bearer tokens require TLS) - post(): require either accessToken or botAccount credentials before serializing XML or hitting the network (fail-fast) - post(): when accessToken is configured, submit XML once with 'Authorization: Bearer <token>' and Content-Type application/xml instead of running the digest-auth challenge loop - Document submit_isrc / submit_barcode OAuth2 scopes inline
- Caller-provided Authorization header is not overwritten by the configured accessToken - Whitespace-only Authorization header is replaced with the configured Bearer token - Constructing MusicBrainzApi with accessToken and a non-HTTPS baseUrl throws - XML web-service post() submits once with the Bearer token and Content-Type application/xml, skipping the Digest challenge - post() throws when neither accessToken nor botAccount credentials are configured
The previously-skipped Node specific API block mixes two unrelated authentication flows:
I just pushed f649ffc which adds a new top-level A follow-up (beyond the scope of this PR) could be to add new OAuth-eligible methods for |
|
I see that CI tests are failing; all tests pass for me locally when setting |
|
Passed local on my end as well. |
|
@Borewit friendly ping that this PR is ready for review; the CI test failure was a one-line fix. |
Add OAuth Bearer token support
Summary
This PR adds support for authenticating API requests using an OAuth Bearer access token, as defined by the MusicBrainz OAuth2 spec and RFC 6750.
Motivation
The MusicBrainz API requires OAuth2 Bearer token authentication for write operations (e.g. submitting ISRCs, barcodes, ratings, collections) and for accessing private user data. Previously, this library had no mechanism to pass a Bearer token, limiting applications that already hold an access token obtained through a standard OAuth2 authorization flow.
Changes
lib/musicbrainz-api.ts: adds optionalaccessTokenfield toIMusicBrainzConfigand threads it intoHttpClientinitialization.lib/http-client.ts: adds optionalaccessTokenfield toIHttpClientOptions; injectsAuthorization: Bearer <token>header on every request when configured, but only if anAuthorizationheader is not already present (allowing callers to override per-request).README.md: documents the newaccessTokenconfig option with an example.test/test-musicbrainz-api.ts: adds two unit tests covering: (1) token is sent as a Bearer header when configured, and (2) noAuthorizationheader is sent when no token is configured.Behavior
accessTokenis optional and fully backward-compatible; existing configurations are unaffected.HttpClientlayer so it applies to all requests uniformly.Authorizationis already set, preserving any future per-request override capability.Testing
Existing tests continue to pass. New tests stub
globalThis.fetchwith sinon to assert header injection behavior without making live network calls.