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

[WIP] Validate server responses in authenticators #957

Merged
merged 10 commits into from Jun 15, 2016

Conversation

Projects
None yet
3 participants
@stevenwu
Copy link
Collaborator

stevenwu commented Apr 12, 2016

@marcoow how does this look?

I don't know how to catch the errors from authenticate inside tests, if throwing errors is what we want.

Fix #942

@marcoow

This comment has been minimized.

Copy link
Member

marcoow commented Apr 12, 2016

@stevenwu: thanks, definitely goes in the right direction. The authenticate methods should not throw errors though but reject the returned promise with an error object instead, e.g.

if (isEmpty(response[tokenAttributeName])) {
  run(null, reject, `${tokenAttributeName} is missing in server response`);
}

instead of

if (isEmpty(response[tokenAttributeName])) {
  throw new Error(`${tokenAttributeName} is missing in server response`);
}

That way you can test that easily by handling the rejected promise in the test and assert on the value of the error object, e.g.:

it('fails if no token is present', (done) => {
  authenticator.authenticate('username', 'password').catch((error) => {
    expect(error).to.eql('token is missing!');
  });
});

Finally the validation logic should be extracted into its own method (e.g. _validate or so) that would also be used in the restore methods then as the same validation logic should be applied in both authenticate and restore.

const refreshAccessTokens = this.get('refreshAccessTokens');
if (refreshAccessTokens) {
this._validate(reject, response, 'expires_in');
this._validate(reject, response, 'refresh_token');

This comment has been minimized.

@stevenwu

stevenwu Apr 19, 2016

Collaborator
{ error: 'invalid_grant' }

is returned elsewhere if 'expires_in' and 'access_token' are already in the response. Maybe we can remove this?

This comment has been minimized.

@marcoow

marcoow Apr 19, 2016

Member

If { error: 'invalid_grant' } nothing else should be in the response actually. But in this case the response code would also be 422 or so so that the request's success handler should never be called anyway. I think we only need to validate successful server responses as erroneous responses will already reject the returned promise anyway.

@@ -81,6 +81,8 @@ export default BaseAuthenticator.extend({
if (!isEmpty(tokenAttribute) && !isEmpty(identificationAttribute)) {
return Promise.resolve(data);
} else {
this._validate(Promise.reject, data, tokenAttributeName);
this._validate(Promise.reject, data, identificationAttributeName);
return Promise.reject();

This comment has been minimized.

@marcoow

marcoow Apr 19, 2016

Member

Might be easier to return a boolean from _validate and get tokenAttributeName and identificationAttributeName internally in the method so that you could just do

if (!this._validate(data)) {
  return Promise.reject();
}
@stevenwu

This comment has been minimized.

Copy link
Collaborator

stevenwu commented May 4, 2016

I forgot to mention this in the call. Occasionally, I'll see

Error: timeout of 2000ms exceeded. Ensure the done() callback is being called in this test.

even though done() is called in the tests.

This error showed up after I made the latest refactor.

@marcoow

This comment has been minimized.

Copy link
Member

marcoow commented May 4, 2016

Not sure what might be causing the timeout actually. Instead of using done you can return the promise though - it's a bit more concise anyway, e.g.:

describe('when the server returns incomplete data', () => {
  it('fails when no access_token is present', () => {
    server.respondWith('POST', '/token', [
        200,
        { 'Content-Type': 'application/json' },
        '{}'
    ]);

    return authenticator.authenticate('username', 'password').catch((error) => {
      expect(error).to.eql('access_token is missing in server response');
    });
  });
});
run(null, reject, `Check that server response includes ${tokenAttributeName} and ${identificationAttributeName}`);
} else {
run(null, resolve, response);
}

This comment has been minimized.

@marcoow

marcoow May 15, 2016

Member

I'd turn this around for better readability:

if (this._validate(response)) {
  run(null, resolve, response);
} else {
 run(null, reject, `Check that server response includes ${tokenAttributeName} and ${identificationAttributeName}`);
}
@@ -164,11 +164,16 @@ export default BaseAuthenticator.extend({
}
this.makeRequest(serverTokenEndpoint, data).then((response) => {
run(() => {
if (!this._validate(response)) {
run(null, reject, 'access_token is missing in server response');
}

This comment has been minimized.

@marcoow

marcoow May 15, 2016

Member

This code is already wrapped in run, see line 166.

@marcoow

This comment has been minimized.

Copy link
Member

marcoow commented May 25, 2016

@stevenwu: this should be ready to merge after rebasing on master

@marcoow marcoow added the enhancement label May 25, 2016

@stevenwu stevenwu force-pushed the stevenwu:fix-authenticate-validation branch from 59063ac to c6679d0 May 25, 2016

@stevenwu

This comment has been minimized.

Copy link
Collaborator

stevenwu commented May 25, 2016

Should we always expect the response to contain a resource name? Is it ever
{ "token": "secret token!", "email": "email@address.com" }

When we validate the data and return it, should it be returned unchanged? I noticed

expect(data).to.eql({ 'access_token': 'secret token!' });
does not contain the resource name.

@marcoow

This comment has been minimized.

Copy link
Member

marcoow commented May 27, 2016

Should we always expect the response to contain a resource name? Is it ever
{ "token": "secret token!", "email": "email@address.com" }

The resource name is optional. Both { "token": "secret token!", "email": "email@address.com" } and { user: { "token": "secret token!", "email": "email@address.com" } } might be valid depending on whether a resource name was configured.

When we validate the data and return it, should it be returned unchanged? I noticed

expect(data).to.eql({ 'access_token': 'secret token!' });
does not contain the resource name.

Validation should not modify the data but the authenticate method will only resolve with the unscoped data (e.g. with { "token": "secret token!", "email": "email@address.com" } both for response { "token": "secret token!", "email": "email@address.com" } as well as for { user: { "token": "secret token!", "email": "email@address.com" } }).

@stepantubanov

This comment has been minimized.

Copy link

stepantubanov commented Jun 3, 2016

Thanks so much for this, I've spent an hour trying to figure out why restoring the session was not working and it turned out that the problem was earlier (when logging in). Failing fast can definitely save a lot of debugging time.

What do you think about adding "_validate" call in the OAuth2 authorizer as well? And what else is left to finish it up? I'd be happy to help.

return !isEmpty(data[resourceName][tokenAttributeName]) && !isEmpty(data[resourceName][identificationAttributeName]);
} else {
return !isEmpty(data[tokenAttributeName]) && !isEmpty(data[identificationAttributeName]);
}

This comment has been minimized.

@marcoow

marcoow Jun 13, 2016

Member

I think this would be simpler as

const resourceName = this.get('resourceName');
const _data = data[resourceName] ? data[resourceName] : data;

return !isEmpty(_data[tokenAttributeName]) && !isEmpty(_data[identificationAttributeName]);
@marcoow

This comment has been minimized.

Copy link
Member

marcoow commented Jun 13, 2016

@stevenwu: just one small comment - I swear that will be the last ;)

@marcoow marcoow merged commit 61f0971 into simplabs:master Jun 15, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@marcoow

This comment has been minimized.

Copy link
Member

marcoow commented Jun 15, 2016

awesome 🎉

josemarluedke added a commit to josemarluedke/ember-simple-auth that referenced this pull request Jul 21, 2016

[WIP] Validate server responses in authenticators (simplabs#957)
* Validate response data in devise authenticator

* Validate response data in OAuth2 authenticator

* Add tests for oauth2 data validations

* Add tests for devise data validations

* Remove unncessary validations

* Refactor 'restore' in devise authenticator

* Fix test timeout errors

* Minor cleanup

* Consider resource name when validating response

* Refactor devise authenticator _validate method

arjansingh added a commit to arjansingh/ember-simple-auth that referenced this pull request Jul 27, 2016

[WIP] Validate server responses in authenticators (simplabs#957)
* Validate response data in devise authenticator

* Validate response data in OAuth2 authenticator

* Add tests for oauth2 data validations

* Add tests for devise data validations

* Remove unncessary validations

* Refactor 'restore' in devise authenticator

* Fix test timeout errors

* Minor cleanup

* Consider resource name when validating response

* Refactor devise authenticator _validate method

marcoow added a commit that referenced this pull request Jul 28, 2016

Fastboot Feature Branch (#1032)
* ignore store events while the session is busy (#965)

* 1.1.0-beta.5

* 1.1.0

* Check for resourceName in response in Devise Authenticator

* change cookie default key names to be rfc2616 compliant (#978)

fixes #977

* Test for session service data being set with Ember.set (#972)

* code/docs cleanup

* Add tokenRefreshOffset property to OAuth2PasswordGrantAuthenticator (#840)

tokenRefreshOffset determines the offset seconds before the token
expiration to refresh the token. This is randomized so as to reduce
race conditions between multiple tabs from refreshing at the same time.

This is configurable because in some cases, the offset randomization
needs to be increased to decrease the probability of the above mentioned
race conditions.

Once more case would be in slow internet connections, you make a call to
refresh the token but the server doesn't process it in time (or receive
it in time), the server will check and the token that you sent up is now
expired so the refresh will fail.

* cleanup transition usage in authenticated and unauthenticated route mixins (#992)

no issue
- fixes potential test timing issue
- removes unecessary abort call

* [BUGFIX] Remove Ember.Logger (#993)

Ember.Logger is not substituted by noops in production.

More info in emberjs/guides#1467

* [WIP] Validate server responses in authenticators (#957)

* Validate response data in devise authenticator

* Validate response data in OAuth2 authenticator

* Add tests for oauth2 data validations

* Add tests for devise data validations

* Remove unncessary validations

* Refactor 'restore' in devise authenticator

* Fix test timeout errors

* Minor cleanup

* Consider resource name when validating response

* Refactor devise authenticator _validate method

* update dependencies (#1004)

This updates Ember, Ember Data, Ember CLI etc. to the latest
versions. This also fixes a lot of JSCS warnings that were introduced
by the latest version of ember-suave.

* Use the term "squash" when referring to collapsing commits into one (#1011)

That's consistent with the term used in git-rebase and with the general public.

* Add rejectWithXhr to optionally reject with XHR vs response body (#1012)

Allows ember apps using ember-simple-auth to receive the whole XHR object if the backend fails, instead of the response body, if they so choose.

In the case of OAuth 2.0 backends, it's been a pattern in the wild to use X- headers to send context as to why a grant has failed. Examples include API throttling, brute force lockouts, and OTP/two-factor authentication information.

Selfishly, I require this change so my application can be notified when the API has locked out an account due to suspicious activity via an X- header.

The decision to expose it as an option was chosen so backwards compatibility is maintained and keeps the addon simple for those who need not be concerned with complex backends.

* Add fastboot-dist to npmignore (#1015)

* Optionally send custom headers in authentication call (#1018)

Complex systems that offer Two Factor Authentication with their OAuth 2.0 implementation need to send additional context via the HTTP headers. This pattern has been observed in the wild by such systems such as GitHub. Because of the restrictions of OAuth 2.0 RFC, only headers can be used for additional context, not request/response bodies.

This could be seen as a counterpart to #1012, where using both features allow bi-directional context enabling 2FA, brute force lockouts, etc.

* [fastboot-compatibility] initial work

* [fastboot-compatbility] improve support

* [fastboot-compatibility] Use ember-cookies@0.0.7

ember-cookies 0.0.6

ember-cookies@0.0.7

* [fastboot-compatbility] fix ember-build-cli.js

* [fastboot-compatibility] fix route mixin transitions

* [fastboot-compatibility] Update `session-stores/cookie` with `typeof` guard (#1)

* [fastboot-compatiblity] fix tests

* Use apiHost config for dummy app.

better name for api host conf setting

fix dummy app API endpoints

* Helpful instructions for `npm run fastboot`

* Restore cookie session renewal

* Fix various rebase issues

* ember-cli-fasboot@1.0.0-beta.7

marcoow added a commit that referenced this pull request Jul 28, 2016

Fastboot Feature Branch (#1032)
* ignore store events while the session is busy (#965)

* 1.1.0-beta.5

* 1.1.0

* Check for resourceName in response in Devise Authenticator

* change cookie default key names to be rfc2616 compliant (#978)

fixes #977

* Test for session service data being set with Ember.set (#972)

* code/docs cleanup

* Add tokenRefreshOffset property to OAuth2PasswordGrantAuthenticator (#840)

tokenRefreshOffset determines the offset seconds before the token
expiration to refresh the token. This is randomized so as to reduce
race conditions between multiple tabs from refreshing at the same time.

This is configurable because in some cases, the offset randomization
needs to be increased to decrease the probability of the above mentioned
race conditions.

Once more case would be in slow internet connections, you make a call to
refresh the token but the server doesn't process it in time (or receive
it in time), the server will check and the token that you sent up is now
expired so the refresh will fail.

* cleanup transition usage in authenticated and unauthenticated route mixins (#992)

no issue
- fixes potential test timing issue
- removes unecessary abort call

* [BUGFIX] Remove Ember.Logger (#993)

Ember.Logger is not substituted by noops in production.

More info in emberjs/guides#1467

* [WIP] Validate server responses in authenticators (#957)

* Validate response data in devise authenticator

* Validate response data in OAuth2 authenticator

* Add tests for oauth2 data validations

* Add tests for devise data validations

* Remove unncessary validations

* Refactor 'restore' in devise authenticator

* Fix test timeout errors

* Minor cleanup

* Consider resource name when validating response

* Refactor devise authenticator _validate method

* update dependencies (#1004)

This updates Ember, Ember Data, Ember CLI etc. to the latest
versions. This also fixes a lot of JSCS warnings that were introduced
by the latest version of ember-suave.

* Use the term "squash" when referring to collapsing commits into one (#1011)

That's consistent with the term used in git-rebase and with the general public.

* Add rejectWithXhr to optionally reject with XHR vs response body (#1012)

Allows ember apps using ember-simple-auth to receive the whole XHR object if the backend fails, instead of the response body, if they so choose.

In the case of OAuth 2.0 backends, it's been a pattern in the wild to use X- headers to send context as to why a grant has failed. Examples include API throttling, brute force lockouts, and OTP/two-factor authentication information.

Selfishly, I require this change so my application can be notified when the API has locked out an account due to suspicious activity via an X- header.

The decision to expose it as an option was chosen so backwards compatibility is maintained and keeps the addon simple for those who need not be concerned with complex backends.

* Add fastboot-dist to npmignore (#1015)

* Optionally send custom headers in authentication call (#1018)

Complex systems that offer Two Factor Authentication with their OAuth 2.0 implementation need to send additional context via the HTTP headers. This pattern has been observed in the wild by such systems such as GitHub. Because of the restrictions of OAuth 2.0 RFC, only headers can be used for additional context, not request/response bodies.

This could be seen as a counterpart to #1012, where using both features allow bi-directional context enabling 2FA, brute force lockouts, etc.

* [fastboot-compatibility] initial work

* [fastboot-compatbility] improve support

* [fastboot-compatibility] Use ember-cookies@0.0.7

ember-cookies 0.0.6

ember-cookies@0.0.7

* [fastboot-compatbility] fix ember-build-cli.js

* [fastboot-compatibility] fix route mixin transitions

* [fastboot-compatibility] Update `session-stores/cookie` with `typeof` guard (#1)

* [fastboot-compatiblity] fix tests

* Use apiHost config for dummy app.

better name for api host conf setting

fix dummy app API endpoints

* Helpful instructions for `npm run fastboot`

* Restore cookie session renewal

* Fix various rebase issues

* ember-cli-fasboot@1.0.0-beta.7

marcoow added a commit that referenced this pull request Sep 7, 2016

Fastboot Feature Branch (#1032)
* ignore store events while the session is busy (#965)

* 1.1.0-beta.5

* 1.1.0

* Check for resourceName in response in Devise Authenticator

* change cookie default key names to be rfc2616 compliant (#978)

fixes #977

* Test for session service data being set with Ember.set (#972)

* code/docs cleanup

* Add tokenRefreshOffset property to OAuth2PasswordGrantAuthenticator (#840)

tokenRefreshOffset determines the offset seconds before the token
expiration to refresh the token. This is randomized so as to reduce
race conditions between multiple tabs from refreshing at the same time.

This is configurable because in some cases, the offset randomization
needs to be increased to decrease the probability of the above mentioned
race conditions.

Once more case would be in slow internet connections, you make a call to
refresh the token but the server doesn't process it in time (or receive
it in time), the server will check and the token that you sent up is now
expired so the refresh will fail.

* cleanup transition usage in authenticated and unauthenticated route mixins (#992)

no issue
- fixes potential test timing issue
- removes unecessary abort call

* [BUGFIX] Remove Ember.Logger (#993)

Ember.Logger is not substituted by noops in production.

More info in emberjs/guides#1467

* [WIP] Validate server responses in authenticators (#957)

* Validate response data in devise authenticator

* Validate response data in OAuth2 authenticator

* Add tests for oauth2 data validations

* Add tests for devise data validations

* Remove unncessary validations

* Refactor 'restore' in devise authenticator

* Fix test timeout errors

* Minor cleanup

* Consider resource name when validating response

* Refactor devise authenticator _validate method

* update dependencies (#1004)

This updates Ember, Ember Data, Ember CLI etc. to the latest
versions. This also fixes a lot of JSCS warnings that were introduced
by the latest version of ember-suave.

* Use the term "squash" when referring to collapsing commits into one (#1011)

That's consistent with the term used in git-rebase and with the general public.

* Add rejectWithXhr to optionally reject with XHR vs response body (#1012)

Allows ember apps using ember-simple-auth to receive the whole XHR object if the backend fails, instead of the response body, if they so choose.

In the case of OAuth 2.0 backends, it's been a pattern in the wild to use X- headers to send context as to why a grant has failed. Examples include API throttling, brute force lockouts, and OTP/two-factor authentication information.

Selfishly, I require this change so my application can be notified when the API has locked out an account due to suspicious activity via an X- header.

The decision to expose it as an option was chosen so backwards compatibility is maintained and keeps the addon simple for those who need not be concerned with complex backends.

* Add fastboot-dist to npmignore (#1015)

* Optionally send custom headers in authentication call (#1018)

Complex systems that offer Two Factor Authentication with their OAuth 2.0 implementation need to send additional context via the HTTP headers. This pattern has been observed in the wild by such systems such as GitHub. Because of the restrictions of OAuth 2.0 RFC, only headers can be used for additional context, not request/response bodies.

This could be seen as a counterpart to #1012, where using both features allow bi-directional context enabling 2FA, brute force lockouts, etc.

* [fastboot-compatibility] initial work

* [fastboot-compatbility] improve support

* [fastboot-compatibility] Use ember-cookies@0.0.7

ember-cookies 0.0.6

ember-cookies@0.0.7

* [fastboot-compatbility] fix ember-build-cli.js

* [fastboot-compatibility] fix route mixin transitions

* [fastboot-compatibility] Update `session-stores/cookie` with `typeof` guard (#1)

* [fastboot-compatiblity] fix tests

* Use apiHost config for dummy app.

better name for api host conf setting

fix dummy app API endpoints

* Helpful instructions for `npm run fastboot`

* Restore cookie session renewal

* Fix various rebase issues

* ember-cli-fasboot@1.0.0-beta.7

bakerac4 added a commit to bakerac4/ember-simple-auth that referenced this pull request Sep 8, 2016

Fastboot Feature Branch (simplabs#1032)
* ignore store events while the session is busy (simplabs#965)

* 1.1.0-beta.5

* 1.1.0

* Check for resourceName in response in Devise Authenticator

* change cookie default key names to be rfc2616 compliant (simplabs#978)

fixes simplabs#977

* Test for session service data being set with Ember.set (simplabs#972)

* code/docs cleanup

* Add tokenRefreshOffset property to OAuth2PasswordGrantAuthenticator (simplabs#840)

tokenRefreshOffset determines the offset seconds before the token
expiration to refresh the token. This is randomized so as to reduce
race conditions between multiple tabs from refreshing at the same time.

This is configurable because in some cases, the offset randomization
needs to be increased to decrease the probability of the above mentioned
race conditions.

Once more case would be in slow internet connections, you make a call to
refresh the token but the server doesn't process it in time (or receive
it in time), the server will check and the token that you sent up is now
expired so the refresh will fail.

* cleanup transition usage in authenticated and unauthenticated route mixins (simplabs#992)

no issue
- fixes potential test timing issue
- removes unecessary abort call

* [BUGFIX] Remove Ember.Logger (simplabs#993)

Ember.Logger is not substituted by noops in production.

More info in emberjs/guides#1467

* [WIP] Validate server responses in authenticators (simplabs#957)

* Validate response data in devise authenticator

* Validate response data in OAuth2 authenticator

* Add tests for oauth2 data validations

* Add tests for devise data validations

* Remove unncessary validations

* Refactor 'restore' in devise authenticator

* Fix test timeout errors

* Minor cleanup

* Consider resource name when validating response

* Refactor devise authenticator _validate method

* update dependencies (simplabs#1004)

This updates Ember, Ember Data, Ember CLI etc. to the latest
versions. This also fixes a lot of JSCS warnings that were introduced
by the latest version of ember-suave.

* Use the term "squash" when referring to collapsing commits into one (simplabs#1011)

That's consistent with the term used in git-rebase and with the general public.

* Add rejectWithXhr to optionally reject with XHR vs response body (simplabs#1012)

Allows ember apps using ember-simple-auth to receive the whole XHR object if the backend fails, instead of the response body, if they so choose.

In the case of OAuth 2.0 backends, it's been a pattern in the wild to use X- headers to send context as to why a grant has failed. Examples include API throttling, brute force lockouts, and OTP/two-factor authentication information.

Selfishly, I require this change so my application can be notified when the API has locked out an account due to suspicious activity via an X- header.

The decision to expose it as an option was chosen so backwards compatibility is maintained and keeps the addon simple for those who need not be concerned with complex backends.

* Add fastboot-dist to npmignore (simplabs#1015)

* Optionally send custom headers in authentication call (simplabs#1018)

Complex systems that offer Two Factor Authentication with their OAuth 2.0 implementation need to send additional context via the HTTP headers. This pattern has been observed in the wild by such systems such as GitHub. Because of the restrictions of OAuth 2.0 RFC, only headers can be used for additional context, not request/response bodies.

This could be seen as a counterpart to simplabs#1012, where using both features allow bi-directional context enabling 2FA, brute force lockouts, etc.

* [fastboot-compatibility] initial work

* [fastboot-compatbility] improve support

* [fastboot-compatibility] Use ember-cookies@0.0.7

ember-cookies 0.0.6

ember-cookies@0.0.7

* [fastboot-compatbility] fix ember-build-cli.js

* [fastboot-compatibility] fix route mixin transitions

* [fastboot-compatibility] Update `session-stores/cookie` with `typeof` guard (simplabs#1)

* [fastboot-compatiblity] fix tests

* Use apiHost config for dummy app.

better name for api host conf setting

fix dummy app API endpoints

* Helpful instructions for `npm run fastboot`

* Restore cookie session renewal

* Fix various rebase issues

* ember-cli-fasboot@1.0.0-beta.7

bakerac4 added a commit to bakerac4/ember-simple-auth that referenced this pull request Sep 8, 2016

Fastboot Feature Branch (simplabs#1032)
* ignore store events while the session is busy (simplabs#965)

* 1.1.0-beta.5

* 1.1.0

* Check for resourceName in response in Devise Authenticator

* change cookie default key names to be rfc2616 compliant (simplabs#978)

fixes simplabs#977

* Test for session service data being set with Ember.set (simplabs#972)

* code/docs cleanup

* Add tokenRefreshOffset property to OAuth2PasswordGrantAuthenticator (simplabs#840)

tokenRefreshOffset determines the offset seconds before the token
expiration to refresh the token. This is randomized so as to reduce
race conditions between multiple tabs from refreshing at the same time.

This is configurable because in some cases, the offset randomization
needs to be increased to decrease the probability of the above mentioned
race conditions.

Once more case would be in slow internet connections, you make a call to
refresh the token but the server doesn't process it in time (or receive
it in time), the server will check and the token that you sent up is now
expired so the refresh will fail.

* cleanup transition usage in authenticated and unauthenticated route mixins (simplabs#992)

no issue
- fixes potential test timing issue
- removes unecessary abort call

* [BUGFIX] Remove Ember.Logger (simplabs#993)

Ember.Logger is not substituted by noops in production.

More info in emberjs/guides#1467

* [WIP] Validate server responses in authenticators (simplabs#957)

* Validate response data in devise authenticator

* Validate response data in OAuth2 authenticator

* Add tests for oauth2 data validations

* Add tests for devise data validations

* Remove unncessary validations

* Refactor 'restore' in devise authenticator

* Fix test timeout errors

* Minor cleanup

* Consider resource name when validating response

* Refactor devise authenticator _validate method

* update dependencies (simplabs#1004)

This updates Ember, Ember Data, Ember CLI etc. to the latest
versions. This also fixes a lot of JSCS warnings that were introduced
by the latest version of ember-suave.

* Use the term "squash" when referring to collapsing commits into one (simplabs#1011)

That's consistent with the term used in git-rebase and with the general public.

* Add rejectWithXhr to optionally reject with XHR vs response body (simplabs#1012)

Allows ember apps using ember-simple-auth to receive the whole XHR object if the backend fails, instead of the response body, if they so choose.

In the case of OAuth 2.0 backends, it's been a pattern in the wild to use X- headers to send context as to why a grant has failed. Examples include API throttling, brute force lockouts, and OTP/two-factor authentication information.

Selfishly, I require this change so my application can be notified when the API has locked out an account due to suspicious activity via an X- header.

The decision to expose it as an option was chosen so backwards compatibility is maintained and keeps the addon simple for those who need not be concerned with complex backends.

* Add fastboot-dist to npmignore (simplabs#1015)

* Optionally send custom headers in authentication call (simplabs#1018)

Complex systems that offer Two Factor Authentication with their OAuth 2.0 implementation need to send additional context via the HTTP headers. This pattern has been observed in the wild by such systems such as GitHub. Because of the restrictions of OAuth 2.0 RFC, only headers can be used for additional context, not request/response bodies.

This could be seen as a counterpart to simplabs#1012, where using both features allow bi-directional context enabling 2FA, brute force lockouts, etc.

* [fastboot-compatibility] initial work

* [fastboot-compatbility] improve support

* [fastboot-compatibility] Use ember-cookies@0.0.7

ember-cookies 0.0.6

ember-cookies@0.0.7

* [fastboot-compatbility] fix ember-build-cli.js

* [fastboot-compatibility] fix route mixin transitions

* [fastboot-compatibility] Update `session-stores/cookie` with `typeof` guard (simplabs#1)

* [fastboot-compatiblity] fix tests

* Use apiHost config for dummy app.

better name for api host conf setting

fix dummy app API endpoints

* Helpful instructions for `npm run fastboot`

* Restore cookie session renewal

* Fix various rebase issues

* ember-cli-fasboot@1.0.0-beta.7

marcoow added a commit that referenced this pull request Dec 6, 2016

FastBoot Compatibility (#1035)
* DRAFT - Fastboot compatibility

* improve fastboot support

* add FastBoot info to README

* fix tests

* better name for api host conf setting

* WIP: fixing tests

* fix tests

* Use version of ember-cookies with fix for fasboot

* ember-cookies 0.0.6

* fix config for fastboot

* never abort current transition in FastBoot

* don't abort transitions in FastBoot

* Fastboot Feature Branch (#1032)

* ignore store events while the session is busy (#965)

* 1.1.0-beta.5

* 1.1.0

* Check for resourceName in response in Devise Authenticator

* change cookie default key names to be rfc2616 compliant (#978)

fixes #977

* Test for session service data being set with Ember.set (#972)

* code/docs cleanup

* Add tokenRefreshOffset property to OAuth2PasswordGrantAuthenticator (#840)

tokenRefreshOffset determines the offset seconds before the token
expiration to refresh the token. This is randomized so as to reduce
race conditions between multiple tabs from refreshing at the same time.

This is configurable because in some cases, the offset randomization
needs to be increased to decrease the probability of the above mentioned
race conditions.

Once more case would be in slow internet connections, you make a call to
refresh the token but the server doesn't process it in time (or receive
it in time), the server will check and the token that you sent up is now
expired so the refresh will fail.

* cleanup transition usage in authenticated and unauthenticated route mixins (#992)

no issue
- fixes potential test timing issue
- removes unecessary abort call

* [BUGFIX] Remove Ember.Logger (#993)

Ember.Logger is not substituted by noops in production.

More info in emberjs/guides#1467

* [WIP] Validate server responses in authenticators (#957)

* Validate response data in devise authenticator

* Validate response data in OAuth2 authenticator

* Add tests for oauth2 data validations

* Add tests for devise data validations

* Remove unncessary validations

* Refactor 'restore' in devise authenticator

* Fix test timeout errors

* Minor cleanup

* Consider resource name when validating response

* Refactor devise authenticator _validate method

* update dependencies (#1004)

This updates Ember, Ember Data, Ember CLI etc. to the latest
versions. This also fixes a lot of JSCS warnings that were introduced
by the latest version of ember-suave.

* Use the term "squash" when referring to collapsing commits into one (#1011)

That's consistent with the term used in git-rebase and with the general public.

* Add rejectWithXhr to optionally reject with XHR vs response body (#1012)

Allows ember apps using ember-simple-auth to receive the whole XHR object if the backend fails, instead of the response body, if they so choose.

In the case of OAuth 2.0 backends, it's been a pattern in the wild to use X- headers to send context as to why a grant has failed. Examples include API throttling, brute force lockouts, and OTP/two-factor authentication information.

Selfishly, I require this change so my application can be notified when the API has locked out an account due to suspicious activity via an X- header.

The decision to expose it as an option was chosen so backwards compatibility is maintained and keeps the addon simple for those who need not be concerned with complex backends.

* Add fastboot-dist to npmignore (#1015)

* Optionally send custom headers in authentication call (#1018)

Complex systems that offer Two Factor Authentication with their OAuth 2.0 implementation need to send additional context via the HTTP headers. This pattern has been observed in the wild by such systems such as GitHub. Because of the restrictions of OAuth 2.0 RFC, only headers can be used for additional context, not request/response bodies.

This could be seen as a counterpart to #1012, where using both features allow bi-directional context enabling 2FA, brute force lockouts, etc.

* [fastboot-compatibility] initial work

* [fastboot-compatbility] improve support

* [fastboot-compatibility] Use ember-cookies@0.0.7

ember-cookies 0.0.6

ember-cookies@0.0.7

* [fastboot-compatbility] fix ember-build-cli.js

* [fastboot-compatibility] fix route mixin transitions

* [fastboot-compatibility] Update `session-stores/cookie` with `typeof` guard (#1)

* [fastboot-compatiblity] fix tests

* Use apiHost config for dummy app.

better name for api host conf setting

fix dummy app API endpoints

* Helpful instructions for `npm run fastboot`

* Restore cookie session renewal

* Fix various rebase issues

* ember-cli-fasboot@1.0.0-beta.7

* Fix tests for fastboot feature branch (#1034)

* fix my messed up rebase 😳

* fix cookie store

* [Fastboot] Remove inject of cookies in initializer; Bump ember-cookies; Add fastboot host whitelist (#1039)

* Add fastboot host whitelist

* Bump ember-cookies to make dummy app work

* Remove inject of cookies in initializers

* [Fastboot] Integration testing for fastboot (#1045)

* Add session register back

* Create a fastboot test app and create basic tests against fastboot.

* fix bad merge 😞

* I suck at merging 😱

* we might need to cleanup for the node tests…

* run both node test and FastBoot node test

* fix node tests

* increase timeout for fastboot node tests

* Update OAuth2PasswordGrant Authenticator to use Fetch (#1066)

* Using ember-network fetch for the oauth2 password grand authenticator

* Fixing unit tests for adding fetch to ember simple auth

* Changing naming of rejectWithRequest to rejectWithResponse

* Making rejectWithXhr a deprecatingAlias and adding import-polyfill to the node-tests app

* docs fixes

* don't bind to localStorage events in FastBoot

* fix test

* don't use window.location in fastboot

* update docs for fastboot

* use ember-network in devise authenticator

* fix tests

* rename fastboot app fixture

* rename fastboot test

* WIP: FastBoot regression test

* WIP: FastBoot regression test

* fix adaptive store for fastboot

* Merge branch 'master' into fastboot (#1098)

* Merge branch 'master' into fastboot

* Remove unneeded test

* Refactor to use ember-cookies clear()

* Try fix for 1.12

* Cleanup

* Cookie store `_secureCookies`: use correct protocol property in FastBoot mode (#1104)

* Merge latest master into fastboot (#1110)

* Update managing-current-user.md (#1068)

Avoid making a current user request if not authenticated

* Add explanatory comment to application route (#1069)

The setup-session-restoration initializer needs the application route to be explicitly defined in the consuming app, so simple auth must ship with this file in case the consuming app hasn't defined their own. However, if the consuming app has another addon that also provides an application route via it's app folder, simple auth may override that addon's file with this one, depending on the load order.

This is easily fixed by specifying an explicit load order (i.e. `after: 'ember-simple-auth'`). However, because the contents of this file are completely generic, and Ember's build process merges this file into the app folder without retaining source information, it's difficult to know from the consuming app's perspective that simple-auth is the addon providing this generic file (and therefore the `after` option needs to specify "ember-simple-auth").

This adds an explanatory comment, hopefully making it a bit clearer in case anyone is trying to figure out where this empty application route file is coming from.

* Cookie store rewrite (#1056)

* Persist to cookie when relevant attributes change

* Refactor cookie properties on adaptive store

* Fix cookie store getters and setters

* Fix adaptive store test

* Remove unused code

* Fix cookie properties on adaptive store

* Change cookie properties to adaptive store

* Fix cookie setters

* Fix jshint errors

* Remove unused code

* Wrap `rewriteCookie` in Ember.run.scheduleOnce

* Remove extra run.scheduleOnce

* Remove unnecessary jshint ignores

* Minor cleanup

* Fix cookie domain test

* Add tests for rewrite behavior

* Save WIP on shared cookie behavior

* fix cookie store rewrite behavior tests

* Small fixes (#1047)

* Fix jshint error

* Allow cookie to write blank values on clear

* Restore previous cookieDomain test

* Persist value in test so cookie will write

* Fix cookie domain test

* simplify AdaptiveStore code

* simplify cooke store code

* use CPs consistently internally

* code cleanup

* handle changes of cookie name correctly

* no need to clear before rewrite

* clear old expiration cookie as well

* Include path when deleting cookie (#1067)

* Refactor cookie string (#1074)

* Include path when deleting cookie

* Add a cookie string method

* Should `resolve` after fetching user. (#1077)

* Should `resolve` after fetching user.

Else the promise remains unresolved.

* Remove return as suggested in the review

* Remove yet another return as suggested

* Update "ember-cli-mocha" to v0.12.0 (#1105)

* Update "ember-cli-mocha" to v0.12.0

* tests: Import it() from "mocha"

* tests: Use new testing API in ember-mocha

* Fix OAuth2 authenticator tokenRefreshOffset (#1106)

* Fix OAuth2 authenticator tokenRefreshOffset

* Fix lint mistake

* Ensure correct root url handling (#1070)

* set rootURL on router correctly

* document readonly correctly

* use rootURL if present

* Fix typo (#1109)

* cleanup

* remove unnecessary injection

* remove unnecessary property

* fix dependency versions

* use cookie session store in dummy app

olleolleolle added a commit to olleolleolle/ember-simple-auth that referenced this pull request Dec 7, 2016

FastBoot Compatibility (simplabs#1035)
* DRAFT - Fastboot compatibility

* improve fastboot support

* add FastBoot info to README

* fix tests

* better name for api host conf setting

* WIP: fixing tests

* fix tests

* Use version of ember-cookies with fix for fasboot

* ember-cookies 0.0.6

* fix config for fastboot

* never abort current transition in FastBoot

* don't abort transitions in FastBoot

* Fastboot Feature Branch (simplabs#1032)

* ignore store events while the session is busy (simplabs#965)

* 1.1.0-beta.5

* 1.1.0

* Check for resourceName in response in Devise Authenticator

* change cookie default key names to be rfc2616 compliant (simplabs#978)

fixes simplabs#977

* Test for session service data being set with Ember.set (simplabs#972)

* code/docs cleanup

* Add tokenRefreshOffset property to OAuth2PasswordGrantAuthenticator (simplabs#840)

tokenRefreshOffset determines the offset seconds before the token
expiration to refresh the token. This is randomized so as to reduce
race conditions between multiple tabs from refreshing at the same time.

This is configurable because in some cases, the offset randomization
needs to be increased to decrease the probability of the above mentioned
race conditions.

Once more case would be in slow internet connections, you make a call to
refresh the token but the server doesn't process it in time (or receive
it in time), the server will check and the token that you sent up is now
expired so the refresh will fail.

* cleanup transition usage in authenticated and unauthenticated route mixins (simplabs#992)

no issue
- fixes potential test timing issue
- removes unecessary abort call

* [BUGFIX] Remove Ember.Logger (simplabs#993)

Ember.Logger is not substituted by noops in production.

More info in emberjs/guides#1467

* [WIP] Validate server responses in authenticators (simplabs#957)

* Validate response data in devise authenticator

* Validate response data in OAuth2 authenticator

* Add tests for oauth2 data validations

* Add tests for devise data validations

* Remove unncessary validations

* Refactor 'restore' in devise authenticator

* Fix test timeout errors

* Minor cleanup

* Consider resource name when validating response

* Refactor devise authenticator _validate method

* update dependencies (simplabs#1004)

This updates Ember, Ember Data, Ember CLI etc. to the latest
versions. This also fixes a lot of JSCS warnings that were introduced
by the latest version of ember-suave.

* Use the term "squash" when referring to collapsing commits into one (simplabs#1011)

That's consistent with the term used in git-rebase and with the general public.

* Add rejectWithXhr to optionally reject with XHR vs response body (simplabs#1012)

Allows ember apps using ember-simple-auth to receive the whole XHR object if the backend fails, instead of the response body, if they so choose.

In the case of OAuth 2.0 backends, it's been a pattern in the wild to use X- headers to send context as to why a grant has failed. Examples include API throttling, brute force lockouts, and OTP/two-factor authentication information.

Selfishly, I require this change so my application can be notified when the API has locked out an account due to suspicious activity via an X- header.

The decision to expose it as an option was chosen so backwards compatibility is maintained and keeps the addon simple for those who need not be concerned with complex backends.

* Add fastboot-dist to npmignore (simplabs#1015)

* Optionally send custom headers in authentication call (simplabs#1018)

Complex systems that offer Two Factor Authentication with their OAuth 2.0 implementation need to send additional context via the HTTP headers. This pattern has been observed in the wild by such systems such as GitHub. Because of the restrictions of OAuth 2.0 RFC, only headers can be used for additional context, not request/response bodies.

This could be seen as a counterpart to simplabs#1012, where using both features allow bi-directional context enabling 2FA, brute force lockouts, etc.

* [fastboot-compatibility] initial work

* [fastboot-compatbility] improve support

* [fastboot-compatibility] Use ember-cookies@0.0.7

ember-cookies 0.0.6

ember-cookies@0.0.7

* [fastboot-compatbility] fix ember-build-cli.js

* [fastboot-compatibility] fix route mixin transitions

* [fastboot-compatibility] Update `session-stores/cookie` with `typeof` guard (#1)

* [fastboot-compatiblity] fix tests

* Use apiHost config for dummy app.

better name for api host conf setting

fix dummy app API endpoints

* Helpful instructions for `npm run fastboot`

* Restore cookie session renewal

* Fix various rebase issues

* ember-cli-fasboot@1.0.0-beta.7

* Fix tests for fastboot feature branch (simplabs#1034)

* fix my messed up rebase 😳

* fix cookie store

* [Fastboot] Remove inject of cookies in initializer; Bump ember-cookies; Add fastboot host whitelist (simplabs#1039)

* Add fastboot host whitelist

* Bump ember-cookies to make dummy app work

* Remove inject of cookies in initializers

* [Fastboot] Integration testing for fastboot (simplabs#1045)

* Add session register back

* Create a fastboot test app and create basic tests against fastboot.

* fix bad merge 😞

* I suck at merging 😱

* we might need to cleanup for the node tests…

* run both node test and FastBoot node test

* fix node tests

* increase timeout for fastboot node tests

* Update OAuth2PasswordGrant Authenticator to use Fetch (simplabs#1066)

* Using ember-network fetch for the oauth2 password grand authenticator

* Fixing unit tests for adding fetch to ember simple auth

* Changing naming of rejectWithRequest to rejectWithResponse

* Making rejectWithXhr a deprecatingAlias and adding import-polyfill to the node-tests app

* docs fixes

* don't bind to localStorage events in FastBoot

* fix test

* don't use window.location in fastboot

* update docs for fastboot

* use ember-network in devise authenticator

* fix tests

* rename fastboot app fixture

* rename fastboot test

* WIP: FastBoot regression test

* WIP: FastBoot regression test

* fix adaptive store for fastboot

* Merge branch 'master' into fastboot (simplabs#1098)

* Merge branch 'master' into fastboot

* Remove unneeded test

* Refactor to use ember-cookies clear()

* Try fix for 1.12

* Cleanup

* Cookie store `_secureCookies`: use correct protocol property in FastBoot mode (simplabs#1104)

* Merge latest master into fastboot (simplabs#1110)

* Update managing-current-user.md (simplabs#1068)

Avoid making a current user request if not authenticated

* Add explanatory comment to application route (simplabs#1069)

The setup-session-restoration initializer needs the application route to be explicitly defined in the consuming app, so simple auth must ship with this file in case the consuming app hasn't defined their own. However, if the consuming app has another addon that also provides an application route via it's app folder, simple auth may override that addon's file with this one, depending on the load order.

This is easily fixed by specifying an explicit load order (i.e. `after: 'ember-simple-auth'`). However, because the contents of this file are completely generic, and Ember's build process merges this file into the app folder without retaining source information, it's difficult to know from the consuming app's perspective that simple-auth is the addon providing this generic file (and therefore the `after` option needs to specify "ember-simple-auth").

This adds an explanatory comment, hopefully making it a bit clearer in case anyone is trying to figure out where this empty application route file is coming from.

* Cookie store rewrite (simplabs#1056)

* Persist to cookie when relevant attributes change

* Refactor cookie properties on adaptive store

* Fix cookie store getters and setters

* Fix adaptive store test

* Remove unused code

* Fix cookie properties on adaptive store

* Change cookie properties to adaptive store

* Fix cookie setters

* Fix jshint errors

* Remove unused code

* Wrap `rewriteCookie` in Ember.run.scheduleOnce

* Remove extra run.scheduleOnce

* Remove unnecessary jshint ignores

* Minor cleanup

* Fix cookie domain test

* Add tests for rewrite behavior

* Save WIP on shared cookie behavior

* fix cookie store rewrite behavior tests

* Small fixes (simplabs#1047)

* Fix jshint error

* Allow cookie to write blank values on clear

* Restore previous cookieDomain test

* Persist value in test so cookie will write

* Fix cookie domain test

* simplify AdaptiveStore code

* simplify cooke store code

* use CPs consistently internally

* code cleanup

* handle changes of cookie name correctly

* no need to clear before rewrite

* clear old expiration cookie as well

* Include path when deleting cookie (simplabs#1067)

* Refactor cookie string (simplabs#1074)

* Include path when deleting cookie

* Add a cookie string method

* Should `resolve` after fetching user. (simplabs#1077)

* Should `resolve` after fetching user.

Else the promise remains unresolved.

* Remove return as suggested in the review

* Remove yet another return as suggested

* Update "ember-cli-mocha" to v0.12.0 (simplabs#1105)

* Update "ember-cli-mocha" to v0.12.0

* tests: Import it() from "mocha"

* tests: Use new testing API in ember-mocha

* Fix OAuth2 authenticator tokenRefreshOffset (simplabs#1106)

* Fix OAuth2 authenticator tokenRefreshOffset

* Fix lint mistake

* Ensure correct root url handling (simplabs#1070)

* set rootURL on router correctly

* document readonly correctly

* use rootURL if present

* Fix typo (simplabs#1109)

* cleanup

* remove unnecessary injection

* remove unnecessary property

* fix dependency versions

* use cookie session store in dummy app
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment