Skip to content

Commit

Permalink
refactor: remove the 'query' option for userinfo, assert access token
Browse files Browse the repository at this point in the history 
BREAKING CHANGE: The 'query' way of passing access token to userinfo
was removed.

BREAKING CHANGE: Access Token is now asserted to be present for the
userinfo call.
  • Loading branch information
@panva
panva committed Oct 27, 2021
1 parent 4e92977 commit eb9d139
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 53 deletions.
21 changes: 8 additions & 13 deletions lib/client.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1116,7 +1116,6 @@ module.exports = (issuer, aadIssValidation = false) =>
headers,
body,
DPoP,

tokenType = DPoP
? 'DPoP'
: accessToken instanceof TokenSet
Expand All @@ -1131,6 +1130,12 @@ module.exports = (issuer, aadIssValidation = false) =>
accessToken = accessToken.access_token;
}

if (!accessToken) {
throw new TypeError('no access token provided');
} else if (typeof accessToken !== 'string') {
throw new TypeError('invalid access token provided');
}

const requestOpts = {
headers: {
Authorization: authorizationHeaderValue(accessToken, tokenType),
Expand Down Expand Up @@ -1169,9 +1174,7 @@ module.exports = (issuer, aadIssValidation = false) =>
throw new TypeError('#userinfo() method can only be POST or a GET');
}

if (via === 'query' && options.method !== 'GET') {
throw new TypeError('userinfo endpoints will only parse query strings for GET requests');
} else if (via === 'body' && options.method !== 'POST') {
if (via === 'body' && options.method !== 'POST') {
throw new TypeError('can only send body on POST');
}

Expand All @@ -1192,15 +1195,7 @@ module.exports = (issuer, aadIssValidation = false) =>

targetUrl = new url.URL(targetUrl || this.issuer.userinfo_endpoint);

// when via is not header we clear the Authorization header and add either
// query string parameters or urlencoded body access_token parameter
if (via === 'query') {
options.headers.Authorization = undefined;
targetUrl.searchParams.append(
'access_token',
accessToken instanceof TokenSet ? accessToken.access_token : accessToken,
);
} else if (via === 'body') {
if (via === 'body') {
options.headers.Authorization = undefined;
options.headers['Content-Type'] = 'application/x-www-form-urlencoded';
options.body = new url.URLSearchParams();
Expand Down
45 changes: 6 additions & 39 deletions test/client/client_instance.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1604,39 +1604,6 @@ describe('Client', () => {
});
});

it('can submit access token in a query when get', function () {
const issuer = new Issuer({ userinfo_endpoint: 'https://op.example.com/me' });
const client = new issuer.Client({
client_id: 'identifier',
token_endpoint_auth_method: 'none',
});

nock('https://op.example.com')
.matchHeader('Authorization', '')
.get('/me?access_token=tokenValue')
.reply(200, {});

return client.userinfo('tokenValue', { via: 'query' }).then(() => {
expect(nock.isDone()).to.be.true;
});
});

it('can only submit access token in a query when get', function () {
const issuer = new Issuer({ userinfo_endpoint: 'https://op.example.com/me' });
const client = new issuer.Client({
client_id: 'identifier',
token_endpoint_auth_method: 'none',
});

return client
.userinfo('tokenValue', { via: 'query', method: 'post' })
.then(fail, ({ message }) => {
expect(message).to.eql(
'userinfo endpoints will only parse query strings for GET requests',
);
});
});

it('is rejected with OPError upon oidc error', function () {
const issuer = new Issuer({ userinfo_endpoint: 'https://op.example.com/me' });
const client = new issuer.Client({
Expand All @@ -1649,7 +1616,7 @@ describe('Client', () => {
error_description: 'bad things are happening',
});

return client.userinfo().then(fail, function (error) {
return client.userinfo('foo').then(fail, function (error) {
expect(error.name).to.equal('OPError');
expect(error).to.have.property('error', 'invalid_token');
expect(error).to.have.property('error_description', 'bad things are happening');
Expand All @@ -1668,7 +1635,7 @@ describe('Client', () => {
'Bearer error="invalid_token", error_description="bad things are happening"',
});

return client.userinfo().then(fail, function (error) {
return client.userinfo('foo').then(fail, function (error) {
expect(error.name).to.equal('OPError');
expect(error).to.have.property('error', 'invalid_token');
expect(error).to.have.property('error_description', 'bad things are happening');
Expand All @@ -1684,7 +1651,7 @@ describe('Client', () => {

nock('https://op.example.com').get('/me').reply(500, 'Internal Server Error');

return client.userinfo().then(fail, function (error) {
return client.userinfo('foo').then(fail, function (error) {
expect(error.name).to.equal('OPError');
expect(error.message).to.eql('expected 200 OK, got: 500 Internal Server Error');
expect(error).to.have.property('response');
Expand All @@ -1700,7 +1667,7 @@ describe('Client', () => {

nock('https://op.example.com').get('/me').reply(200, '{"notavalid"}');

return client.userinfo().then(fail, function (error) {
return client.userinfo('foo').then(fail, function (error) {
expect(error.message).to.eql('Unexpected token } in JSON at position 12');
expect(error).to.have.property('response');
});
Expand Down Expand Up @@ -1757,7 +1724,7 @@ describe('Client', () => {
'content-type': 'application/jwt; charset=utf-8',
});

return client.userinfo().then(fail, (err) => {
return client.userinfo('foo').then(fail, (err) => {
expect(err.message).to.eql('unexpected JWT alg received, expected RS256, got: none');
});
});
Expand All @@ -1774,7 +1741,7 @@ describe('Client', () => {

nock('https://op.example.com').get('/me').reply(200, {});

return client.userinfo().then(fail, (err) => {
return client.userinfo('foo').then(fail, (err) => {
expect(err.message).to.eql(
'expected application/jwt response from the userinfo_endpoint',
);
Expand Down
2 changes: 1 addition & 1 deletion types/index.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ declare class BaseClient {
accessToken: TokenSet | string,
options?: {
method?: 'GET' | 'POST';
via?: 'header' | 'body' | 'query';
via?: 'header' | 'body';
tokenType?: string;
params?: object;
DPoP?: DPoPInput;
Expand Down

0 comments on commit eb9d139

Please sign in to comment.