Fix authentication-oauth regression using incorrect callback and redirect_uri

[johnheenan]

Fix authentication-oauth regression: authentication callback and redirect_uri were using incorrect protocol and host from configuration

Summary

Please see #2630

Call to defaultsDeep parameters reordered to ensure protocol and host values from both both oauth.defaults and from local variable can be distinguished, with local values as fallback.

Previous version was coding local values instead of oauth.default values. This matters if they are not the same, such as when using reverse proxy server. The local host value is from top host value of configuration file defualt.json.

[johnheenan]

I should let you know that I tried the following with oauth last, instead of second following {} as in the existing solution, but it did not work. The results are the same as the existing solution. I accept the existing solution , with oath second should work.

const grant = defaultsDeep({}, {
    defaults: {
      prefix,
      origin: `${protocol}://${host}`,
      transport: 'session',
      response: ['tokens', 'raw', 'profile']
    }
  }, omit(oauth, ['redirect', 'origins']));

This is the suggested solution that works, but it should have an addition.

const grant = defaultsDeep({
    defaults: {
      prefix,
      origin: `${oauth?.defaults?.protocol ?? protocol}://${oauth?.defaults?.host ?? host}`,
      transport: 'session',
      response: ['tokens', 'raw', 'profile']
    }
  }, omit(oauth, ['redirect', 'origins']));

The following is better to ensure the transport from the configuration is used. I will add another commit to the PR.

  const grant = defaultsDeep({
    defaults: {
      prefix,
      origin: `${oauth?.defaults?.protocol ?? protocol}://${oauth?.defaults?.host ?? host}`,
      transport:  oauth?.defaults?.transport ?? 'session',
      response: ['tokens', 'raw', 'profile']
    }
  }, omit(oauth, ['redirect', 'origins']));

[daffl]

Thank you for the pull request! When comparing it to v4 I'm thinking the order is just wrong which is what you changed but then we can keep the old host settings. I'll quickly update and it will get out with the new release tomorrow and you can let me know if that fixes it.

[johnheenan]

Thanks for the commit. No, changing the order does not fix the problem with my config/default.json. My fix does fix the problem with my config. I thoroughly tested your and my version, to the extent of checking the compiled module js.

We need to step back a bit because we may be at cross purposes given the way I configure feathers for authentication. I need to state how I configure and why I configure this way. These reasons, with a sample configuration, are on #2630, so you and others can comment on this if they wish.

[daffl]

No my bad, you were correct. I thought it would still fill in the correct settings. Will merge your original PR and put it out with the next release shortly.

[cciocov]

@johnheenan @daffl
This merge introduces a bug, in that you are no longer able to override the keys under "defaults". SInce we're using "defaultsDeep" instead of "merge", the order of parameters passed should be inverse (i.e.: higher priority parameters first, which are the ones coming in the "oauth" object).

Opened PR #2659 to fix this.