Skip to content
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

Fixes Handling of ClientID for Password Grant #1679

Merged
merged 12 commits into from
Mar 21, 2019
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,15 @@ export default JSONAPIAdapter.extend(DataAdapterMixin, {
});
```

### Deprecation of Client ID as Header

Sending the Client ID as Base64 Encoded in the Authorization Header was against the spec and caused
incorrect behavior with OAuth2 Servers that had implemented the spec properly.

To change this behavior set `sendClientIdAsQueryParam` to `true`, and the client id will be correctly
sent as a query parameter. Leaving it set to `false` (currently default) will result in a deprecation
notice until the next major version.

## Session Stores

Ember Simple Auth __persists the session state via a session store so it
Expand Down
39 changes: 35 additions & 4 deletions addon/authenticators/oauth2-password-grant.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
merge,
assign as emberAssign
} from '@ember/polyfills';
import { deprecate } from '@ember/application/deprecations';
import Ember from 'ember';
import BaseAuthenticator from './base';
import fetch from 'fetch';
Expand Down Expand Up @@ -53,6 +54,17 @@ export default BaseAuthenticator.extend({
*/
clientId: null,

/**
The OAuth2 standard is to send the client_id as a query parameter. This is a
feature flag that turns on the correct behavior for OAuth2 requests.

@property sendClientIdAsQueryParam
@type Boolean
@default false
@public
*/
sendClientIdAsQueryParam: false,

/**
The endpoint on the server that authentication and token refresh requests
are sent to.
Expand Down Expand Up @@ -117,7 +129,6 @@ export default BaseAuthenticator.extend({

_clientIdHeader: computed('clientId', function() {
const clientId = this.get('clientId');

if (!isEmpty(clientId)) {
const base64ClientId = window.base64.encode(clientId.concat(':'));
return { Authorization: `Basic ${base64ClientId}` };
Expand Down Expand Up @@ -253,6 +264,17 @@ export default BaseAuthenticator.extend({
@public
*/
authenticate(identification, password, scope = [], headers = {}) {
if (!this.get('sendClientIdAsQueryParam')) {
deprecate(`Ember Simple Auth: Client ID as Authorization Header is deprecated in favour of Client ID as Query String Parameter.`,
false,
{
id: 'ember-simple-auth.oauth2-password-grant-authenticator.client-id-as-authorization',
until: '2.0.0',
url: 'https://github.com/simplabs/ember-simple-auth#deprecation-of-client-id-header',
}
);
}

return new RSVP.Promise((resolve, reject) => {
const data = { 'grant_type': 'password', username: identification, password };
const serverTokenEndpoint = this.get('serverTokenEndpoint');
Expand Down Expand Up @@ -336,6 +358,13 @@ export default BaseAuthenticator.extend({
makeRequest(url, data, headers = {}) {
headers['Content-Type'] = 'application/x-www-form-urlencoded';

if (this.get('sendClientIdAsQueryParam')) {
const clientId = this.get('clientId');
if (!isEmpty(clientId)) {
data['client_id'] = this.get('clientId');
}
}

const body = keys(data).map((key) => {
return `${encodeURIComponent(key)}=${encodeURIComponent(data[key])}`;
}).join('&');
Expand All @@ -346,9 +375,11 @@ export default BaseAuthenticator.extend({
method: 'POST'
};

const clientIdHeader = this.get('_clientIdHeader');
if (!isEmpty(clientIdHeader)) {
merge(options.headers, clientIdHeader);
if (!this.get('sendClientIdAsQueryParam')) {
const clientIdHeader = this.get('_clientIdHeader');
if (!isEmpty(clientIdHeader)) {
merge(options.headers, clientIdHeader);
}
}

return new RSVP.Promise((resolve, reject) => {
Expand Down
37 changes: 37 additions & 0 deletions tests/unit/authenticators/oauth2-password-grant-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
import { expect } from 'chai';
import Pretender from 'pretender';
import OAuth2PasswordGrant from 'ember-simple-auth/authenticators/oauth2-password-grant';
import { registerDeprecationHandler } from '@ember/debug';

describe('OAuth2PasswordGrantAuthenticator', () => {
let authenticator;
Expand Down Expand Up @@ -127,6 +128,42 @@ describe('OAuth2PasswordGrantAuthenticator', () => {
authenticator.authenticate('username', 'password');
});

it('shows a deprecation warning when sending the client_id in the Basic Auth header', function(done) {
let warnings;
registerDeprecationHandler((message, options, next) => {
// in case a deprecation is issued before a test is started
if (!warnings) {
warnings = [];
}

warnings.push(message);
next(message, options);
});

server.post('/token', () => done());
authenticator.set('clientId', 'test-client');
authenticator.authenticate('username', 'password');

expect(warnings[0]).to.eq('Ember Simple Auth: Client ID as Authorization Header is deprecated in favour of Client ID as Query String Parameter.');
});

it('sends an AJAX request to the token endpoint with client_id as parameter in the body', function(done) {
server.post('/token', (request) => {
let body = parsePostData(request.requestBody);
expect(body).to.eql({
'client_id': 'test-client',
'grant_type': 'password',
'username': 'username',
'password': 'password'
});
done();
});

authenticator.set('sendClientIdAsQueryParam', true);
authenticator.set('clientId', 'test-client');
authenticator.authenticate('username', 'password');
});

it('sends an AJAX request to the token endpoint with customized headers', function(done) {
server.post('/token', (request) => {
expect(request.requestHeaders['x-custom-context']).to.eql('foobar');
Expand Down