search

joscha / play-authenticate

An authentication plugin for Play Framework 2.x (Java)
http://joscha.github.com/play-authenticate/
Other
807 stars 367 forks source link

Play 2.4 upgrade #259

Closed joscha closed 9 years ago

mkurz commented 9 years ago

The tests fail. Just FYI, I was following a bug for selenium regarding Firefox, maybe it is related: https://code.google.com/p/selenium/issues/detail?id=7810 (Probably it's something different anyway, but just to consider that maybe selenium could be broken)

joscha commented 9 years ago

can't seem to get it green :(

mkurz commented 9 years ago

@joscha It looks like FB changed their id's. Try to change this line

.find("#u_0_1").click();

to

.find("#u_0_2").click();
mkurz commented 9 years ago

#u_0_1 is some hidden field now, probably for CSRF protection. #u_0_2 is the login button we want now.

mkurz commented 9 years ago

@joscha That helped, we are now getting further. It now fails in the checkLoginLayout method at this line, it is expecting "page" but is getting null... I don't know why it fails - if I run the test manually by hand, I also get to this screen, and for me there is a element with name display which also contains the value page when running

document.querySelector("[name='display']")
document.querySelector("[name='display']").value

in my browser console. So for me that's fine. I even downloaded Firefox 31 to have the same conditions, also even running it on Linux.

Can you add a commit to this PR which logs the content of browser.find(selector) in some readable format before the line I referenced above, so we know if it evens finds the element? Maybe there is a bug in .getValue() now? Maybe we could try something like .getAttribute("value") instead? Just suggestions where to start...

joscha commented 9 years ago

Sorry, no computer access currently. As a collaborator you should be able to do anything to that PR though - as soon as it is green it can be merged in stable and a snapshot will automatically be published to sonatype. Am 12.06.2015 00:04 schrieb "Matthias Kurz" notifications@github.com:

@joscha https://github.com/joscha That helped, we are now getting further. It now fails in the checkLoginLayout method at this line https://github.com/joscha/play-authenticate/blob/master/samples/java/play-authenticate-usage/test/test/FacebookOAuth2Test.java#L100, it is expecting "page" but is getting null... I don't know why it fails - if I run the test manually by hand, I also get to this screen, and for me there is a element with name display which also contains the value page when running

document.querySelector("[name='display']") document.querySelector("[name='display']").value

in my browser console. So for me that's fine. I even downloaded Firefox 31 to have the same conditions, also even running it on Linux.

Can you add a commit to this PR which logs the content of browser.find(selector) in some readable format before the line I referenced above, so we know if it evens finds the element? Maybe there is a bug in .getValue() now? Maybe we could try something like .getAttribute("value") instead? Just suggestions where to start...

— Reply to this email directly or view it on GitHub https://github.com/joscha/play-authenticate/pull/259#issuecomment-111386848 .

mkurz commented 9 years ago

I logged the html of the root <html> tag, all we get is:

<head></head><body></body>

so basically there is an empty page, strange.

mkurz commented 9 years ago

Are there any security settings applied for this Facebook account so it is not allowed to login from different machines anymore? Or something similar?

joscha commented 9 years ago

No, it's a test user - should work from anywhere. Am 12.06.2015 01:17 schrieb "Matthias Kurz" notifications@github.com:

Are there any security settings applied for this Facebook account so it is not allowed to login from different machines anymore? Or something similar?

— Reply to this email directly or view it on GitHub https://github.com/joscha/play-authenticate/pull/259#issuecomment-111408200 .

mkurz commented 9 years ago

@joscha It looks like the FB username and/or password is wrong, could this be? Or the FB account has expired. I now log the URL directly after clicking the login button and it gets me to following url:

[debug] application - after login url: https://www.facebook.com/login.php?login_attempt=1

It looks like this is the error page when you e.g. enter a wrong password. I also get the login_attempt=1 param when I try to login to my facebook account with a wrong password. Could you please check the credentials of this FB test user and try to login in manually in your browser so we can be sure login works?

mkurz commented 9 years ago

I think I found something more. Checkout following line in the log:

generated redirect URL for dialog: https://www.facebook.com/v2.1/dialog/oauth?client_id=471747832966198&redirect_uri=http%3A%2F%2Flocalhost%3A9000%2Fauthenticate%2Ffacebook&scope=email&response_type=code&state=5192ea48-0f67-483d-bfa3-aa9862ecc9bc

When I navigate to this url I get following FB error: fb_error Could you check and tweak the FB settings of the app so that this url (http://localhost:9000/authenticate/facebook&scope=email&response_type) is allowed to access it?

joscha commented 9 years ago

It is: play_authenticate_ci Something else must be wrong...

mkurz commented 9 years ago

Did you also check under Settings -> Basic Tab -> Website -> Site URL I think there should be http://localhost:9000/ as well.

joscha commented 9 years ago

I added an additional web page URL and removed the debug commits.

mkurz commented 9 years ago

Woohoo, FB seems to work finally, but now Google seems to fail :cry:

mkurz commented 9 years ago

Got it, will add a commit.

mkurz commented 9 years ago

Green! Ready to merge :smile:

joscha commented 9 years ago

Yeah!

joscha commented 9 years ago

snapshot is on sonatype

mkurz commented 9 years ago

@joscha Nice work, thanks!

joscha commented 9 years ago

Thank YOU! Let me know how it goes with the snapshot, then I can do a timely release.

mkurz commented 9 years ago

Sure, I will :grinning: