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

Connect Router Deprecation and Support for Express Mounting #83

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

DamonOehlman
Copy link

Brian,

First of all thanks for making this library - it's awesome and takes care of so much heavy lifting with regards to working with OAuth providers. Nice job.

This pull request covers two changes that I'd love to see integrated at some stage:

1. Tweaks to deal with Connect Router being deprecated

I'm not sure if you've seen the discussion, but the connect router has been deprecated and now only exists in Express:

senchalabs/connect#262

While I found this surprising at first, I've moved over to using express rather than just connect which has been an extremely pleasant experience. I don't think I implemented the changes in the best way, but the changes do demonstrate what is required to interact with the express routing rather than connect (i.e. not much).

2. OAuth changes to support Express Application Mounting

A feature that I have found extremely useful in Express, is the ability to define a number of express applications independently and then mount them into a "master" application definition. It's a very effective way of modularizing and composing your applications.

The one trick is that the url of an application may change based on mounting it. For example, the example in the express repo shows mounting an example blog application into a blog subdirectory. The problem is that the OAuth callback urls are based only on the path set in the configuration and don't take into account the app.route value that is passed around when an application is mounted (and not). I've hacked the code to make it work as a proof of concept, and it definitely does the job, but you might know a more elegant way to implement this.

Anyway, thanks again, and I'd love to see these changes make it in to the repo at some stage so I can blow away my fork :)

Cheers,
Damon.

@timshadel
Copy link
Contributor

This definitely fixes the problem I've had using everyauth with zappa.

I think it is a bit odd to mix together the use middleware and the dynamic helper injection... Any way to separate the two, or can you give a usage scenario that clears it up? I basically skipped the use section of my app altogether and just did the helper:

configure ->
  set views: "#{__dirname}/views", 'view engine': 'eco'
  app.register '.eco', strong.maker( zappa.adapter 'eco' )
  use 'bodyParser', 'cookieParser', {session:{secret: 'bigsecret'}}, 'static'

everyauth.helpExpress app

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.

2 participants