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

OpenID returnURL fix (Google Hybrid fix) #341

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

Conversation

parisholley
Copy link

The current openId module was creating a RelyingParty with a return URL before a hostname could properly be detected. I delayed the instance created until a request is available, otherwise services like google hybrid does not work.

bigokro and others added 30 commits April 21, 2012 00:14
…defined user when there are multiple tumblelogs associated to the account.
password was not being deleted.
Changed github apiHost url and fetchOAuthUser to work with github api v3
hstove and others added 27 commits July 13, 2012 17:03
added provider for dailycred.com
Fixed bug in the example code; saving a vimeo user
…akinsella-master

Conflicts:
	README.md
	example/conf.js
	example/server.js
	example/views/home.jade
Hi! I fixed some calls to "sys" for you!
corrected a typo in readme code sample
…into jonathana-meetup_oauth2

Conflicts:
	example/conf.js
	example/server.js
	example/views/home.jade
fix for issue bnoguchi#244 where the tumblr module is returning an undefined user when there are multiple tumblelogs associated to the account.
Rudimentary support for the Facebook Canvas
@kyriesent
Copy link
Contributor

I had trouble getting this patch to work; verifyAssertion in verifyAttributes (line 37) is failing with "cannot call method verifyAssertion of undefined" I think the relyingParty is not transferring over from sendToAuthenticationURI. Perhaps it's just my implementation, but I think it would be easier just to update the documentation with this one line:

.myHostname('localhost:3000')

That fixed the issue immediately for me.

@kyriesent
Copy link
Contributor

Although now that I think of it, if it could detect hostname automatically that would probably be ideal.

@parisholley
Copy link
Author

I had fixed this in express 3 so it may not be compatible. @kyriesent The problem I ran into, was the hostname detection requires a request to be available, but at the time myHostname is set, that request isn't available. I have a fix working in the expres3 branch pull request but it may not in this, may need to revisit.

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.