ciaranj / connect-auth

Authentication middleware for connect.
MIT License
813 stars 114 forks source link

how to handle authenticated && !req.isAuthenticated()? #120

Open coolaj86 opened 11 years ago

coolaj86 commented 11 years ago

Let's say I try to auth against my provider and instead of "Allow", I click "Deny".

I would expect that authenticated would be false and req.isAuthenticated() would also be false.

However, in my example I get back true and false and I'm not redirected back to the provider's dialog when I make a second attempt. Instead I'm immediately taken to my oauth callback, unable to ever allow.

Am I doing something wrong?

ciaranj commented 11 years ago

i'll take a look, incidentally inside your authentication provider implementation 'this.trace' should be available to you rather than console.log which should provide clearer diagnostics to you.

ciaranj commented 11 years ago

In the version of your code that I'm looking at, verifyAuthSuccess appears to always call 'self.success' is that intended?

coolaj86 commented 11 years ago

Yes and no. As I'm building this out I'm adding more features.

Another bug I'm experiencing is that no matter whether I choose 'allow' or 'deny', I'm always getting a failure (which is ironic because I'm always returning success).

But since I have a failure, I figured why not handle the failure case and then I'll look into why the success case isn't working.

ciaranj commented 11 years ago

And when you choose allow/deny that information is being passed back in the callback to your consumer?

On Thu, Mar 21, 2013 at 4:59 PM, AJ ONeal notifications@github.com wrote:

Yes and no. As I'm building this out I'm adding more features.

Another bug I'm experiencing is that no matter whether I choose 'allow' or 'deny', I'm always getting a failure (which is ironic because I'm always returning success).

But since I have a failure, I figured why not handle the failure case and then I'll look into why the success case isn't working.

— Reply to this email directly or view it on GitHubhttps://github.com/ciaranj/connect-auth/issues/120#issuecomment-15251332 .

coolaj86 commented 11 years ago

I'm not sure. I need to go back to the original example and see if the allow/deny fields are hard-coded or if I messed something up as I was editing it. I don't recall ever changing it.

Would you care to chat over skype sometime today? I'm coolaj86, I've added you.

ciaranj commented 11 years ago

I'm afraid I'm a bit tied up / busy atm. Is the code you've committed up-to-date as I don't seem to have any of the login boxes etc. that your video showed :(

On Thu, Mar 21, 2013 at 5:10 PM, AJ ONeal notifications@github.com wrote:

I'm not sure. I need to go back to the original example and see if the allow/deny fields are hard-coded or if I messed something up as I was editing my example. I don't recall ever changing it.

Would you care to chat over skype sometime today? I'm coolaj86, I've added you.

— Reply to this email directly or view it on GitHubhttps://github.com/ciaranj/connect-auth/issues/120#issuecomment-15252010 .

coolaj86 commented 11 years ago

Yes, I've been testing in Chrome. There may be a bug that affects firefox.

I'll make another video in about an hour and show from git clone to up-and-running what I've done.

coolaj86 commented 11 years ago

Did you rerun grunt build?

ciaranj commented 11 years ago

ok, thank you that would allow me to rather more quickly diagnose where it might be going awry. Fwiw if req.isAuthenticated() is returning true that means that there is a 'user' object currently stored in that session?

On Thu, Mar 21, 2013 at 5:23 PM, AJ ONeal notifications@github.com wrote:

Yes, I've been testing in Chrome. There may be a bug that affects firefox.

I'll make another video in about an hour and show from git clone to up-and-running what I've done.

— Reply to this email directly or view it on GitHubhttps://github.com/ciaranj/connect-auth/issues/120#issuecomment-15252811 .

ciaranj commented 11 years ago

nope, never ran that at all!

On Thu, Mar 21, 2013 at 5:23 PM, AJ ONeal notifications@github.com wrote:

Did you rerun grunt build?

— Reply to this email directly or view it on GitHubhttps://github.com/ciaranj/connect-auth/issues/120#issuecomment-15252838 .

coolaj86 commented 11 years ago

I updated the README with the grunt build steps

ciaranj commented 11 years ago

You may also want to include some of the steps in here: http://stackoverflow.com/questions/15511677/installing-npm-module-results-in-command-not-foundfor those of us not familiar with grunt ;)

On Thu, Mar 21, 2013 at 5:25 PM, AJ ONeal notifications@github.com wrote:

I updated the README with the grunt build steps

— Reply to this email directly or view it on GitHubhttps://github.com/ciaranj/connect-auth/issues/120#issuecomment-15252988 .

coolaj86 commented 11 years ago

if you git pull the README should have that in there

ciaranj commented 11 years ago

I also needed to

cd server/lib

ln -s ../../blogthing-consumer/server.js consumer.js

ln -s ../../bookface-provider/server.js provider.js

before 'node bin/demo' would work for me.

;) (sorry)

On Thu, Mar 21, 2013 at 5:35 PM, AJ ONeal notifications@github.com wrote:

if you git pull the README should have that in there

— Reply to this email directly or view it on GitHubhttps://github.com/ciaranj/connect-auth/issues/120#issuecomment-15253634 .

coolaj86 commented 11 years ago

Try deleting and re-cloning the project. Something is out of sync with what you have and what I have.

ciaranj commented 11 years ago

Cloning has improved things (still got no web forms, but the sym links were not required) ... incidentally some of the problem could be due to:

req.authenticate(['foo'], { scope: ["email", "birthday"] },

logAuthentication);

That will be having weird affects with the currently packaged version of connect-auth. 'scope' in this context means something to connect-auth (something not very well used or implemented I might add :( )

Try req.authenticate(['foo'], logAuthentication); isntead?

On Thu, Mar 21, 2013 at 5:40 PM, AJ ONeal notifications@github.com wrote:

Try deleting and re-cloning the project. Something is out of sync with what you have and what I have.

— Reply to this email directly or view it on GitHubhttps://github.com/ciaranj/connect-auth/issues/120#issuecomment-15253960 .

coolaj86 commented 11 years ago

You were exactly right about that scope option.

Now allow and deny have the proper action.

When I click deny and then go back to click login again (from the consumer) it pops open the dialog, but skips the oauth process. However, clicking on the login (consumer side) a few more times it will eventually get there.

I think it's a cache bug where the browser assumes the previous redirect will be the current redirect. Perhaps there's a way to expire the redirect immediately so that this doesn't happen.

ciaranj commented 11 years ago

Possibly, you should be able to confirm that with wireshark (see if the HTTP traffic is really leaving the browser or not), if that is an issue you could always just append a 'cache breaker' to your url to force the browser to always try.

On Thu, Mar 21, 2013 at 7:48 PM, AJ ONeal notifications@github.com wrote:

You were exactly right about that scope option.

Now allow and deny have the proper action.

When I click deny and then go back to click login again (from the consumer) it pops open the dialog, but skips the oauth process. However, clicking on the login (consumer side) a few more times it will eventually get there.

I think it's a cache bug where the browser assumes the previous redirect will be the current redirect. Perhaps there's a way to expire the redirect immediately so that this doesn't happen.

— Reply to this email directly or view it on GitHubhttps://github.com/ciaranj/connect-auth/issues/120#issuecomment-15261178 .