search

feathersjs-ecosystem / authentication-oauth2

[MOVED] OAuth 2 plugin for feathers-authentication
https://github.com/feathersjs/feathers
MIT License
26 stars 15 forks source link

0.2.5 introduced breaking changes #36

Closed imaustink closed 7 years ago

imaustink commented 7 years ago

The following is a list of breaking changes. I believe these should have been in a major release.

These changes break feathers-authentication on versions below 1.2.7. This is the commit where auth settings were moved from auth to authentication in feathers-authentication: https://github.com/feathersjs/feathers-authentication/commit/79c5be10c6741065e381fa0a61f1b452f3f7576f

cc @marshallswain

marshallswain commented 7 years ago

Hmm. I think Feathers-authentication was supposed to be setup to normalize it into the right place. I'll check this out tomorrow.

daffl commented 7 years ago

It is indeed, I wonder how that slipped by (and didn't come up yet), all the other ones do have the fallback so I added it in a PR in #37.

daffl commented 7 years ago

@marshallswain Feel free to merge and release if you think this fixes the issue.

imaustink commented 7 years ago

Sorry, my lack of knowledge in feathers led me to believe these were breaking changes. After digging around further, I think I was wrong.

It seems the location of auth settings are not documented within feathers-authentication-oauth2 at all, and since the old convention is still supported within feathers-authentication, these aren't technically breaking.

Although, and correct me if I am wrong, I beleive minors are suppose to be backwards-compatible. This is not compatible with versions of feathers-authentication below 1.2.7.

imaustink commented 7 years ago

Oh wow, I should have refreshed before replying. Thank you @daffl!