Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Removed opinionated protocol setting#1563

Closed
troykelly wants to merge 1 commit intofeathersjs:masterfeathersjs/feathers:masterfrom
aperim:masteraperim/feathers:masterCopy head branch name to clipboard
Closed

Removed opinionated protocol setting#1563
troykelly wants to merge 1 commit intofeathersjs:masterfeathersjs/feathers:masterfrom
aperim:masteraperim/feathers:masterCopy head branch name to clipboard

Conversation

@troykelly
Copy link

Problem
OAuth requests do not possess the ability to set the protocol and port from the configuration.
In addition, when in a development environment, the opinionated nature of the protocol setting forced a redirect_url with only a http:// protocol.

@KidkArolis
Copy link
Contributor

KidkArolis commented Sep 17, 2019

This part of the code was just refined 2 weeks ago - #1543 and we were pretty happy with the result.

Could you describe what you're trying to do that's not working as expected (e.g. give example URLs)? You can configure the port/protocol business for each environment in your configuration files if the defaults don't work:

authentication:
  oauth:
    defaults:
      host: custom-host.with-any-port.com:123
      protocol: https

Defaults are designed to fit typical use cases:

  • in dev - http with port
  • in prd - https with no port

@troykelly
Copy link
Author

troykelly commented Sep 17, 2019

I'm trying to not have http:// appended to the URI.
I also need a mechanism to "ignore" the port setting - for example, when running in App Engine, Google Run, or using a service like Ngrok. The local service may be port 3030, but publically it is https://example.com/

The change in #1543 is quite opinionated when it comes to the non-production environment, forcing the protocol to http. It also provides no mechanism for attaching the port number only when needed and does not provide the ability to flag that the port should be ignored in the situation above.

I'm not sure how I could configure https and no port in the config file - it appears to be set here and doesn't reference any config?

@KidkArolis
Copy link
Contributor

OK. I see your issue, but I think this is all configurable. In your config file (I'm using yml, but it's the same in json), there are 2 significant parts of config for your use case:

# this is how the app is running locally
# this is also used to generate defaults for oauth
host: localhost
port: 3030

# but you can override oauth defaults to whatever you need
authentication:
  oauth:
    defaults:
      host: custom-host-with-or-without-port.com
      protocol: custom-protocol-irrespective-of-environment

If you don't put anything in authentication.oauth.defaults - that's when feathers generates that based on "good defaults" heuristic - http with port in dev, https with no port in production.

Since you want to use no port in development, you can just override that directly with:

authentication:
  oauth:
    defaults:
      host: myapp.ngrok.io
      protocol: https

Let me know if that worked or if I'm still missing something.

The relevant part of the code btw is this bit where feathers deeply merges the generated defaults with your own defaults (and the rest of oauth configuration): https://github.com/compwright/feathers/blob/master/packages/authentication-oauth/src/index.ts#L28-L35

@daffl
Copy link
Member

daffl commented Sep 22, 2019

Thank you for the pull request! As per the discussion, I think we should leave it the way it is for now since it is always possible to customize those settings.

@daffl daffl closed this Sep 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments

Close sidebar
Morty Proxy This is a proxified and sanitized view of the page, visit original site.