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

Migration to Play 2.5 #298

Closed pdolega closed 8 years ago

pdolega commented 8 years ago

Upgrading project to Play 2.5 version.

Basically the the main theme is to switch to DI completely and get rid of Plugin.

Things to do:

oexza commented 8 years ago

other than the little things i pointed out, good work!!

On 3/21/16, pdolega notifications@github.com wrote:

-- This is work in progress --

Upgrading project to Play 2.5 version. You can view, comment on, or merge this pull request online at:

https://github.com/joscha/play-authenticate/pull/298

-- Commit Summary --

  • Core part migrated to Play 2.5

-- File Changes --

M README.md (1)
M code/app/com/feth/play/module/pa/PlayAuthenticate.java (2)
M code/app/com/feth/play/module/pa/exceptions/AccessTokenException.java

(4) M code/app/com/feth/play/module/pa/exceptions/AuthException.java (4) M code/app/com/feth/play/module/pa/providers/AuthProvider.java (39) M code/app/com/feth/play/module/pa/providers/ext/ExternalAuthProvider.java (7) M code/app/com/feth/play/module/pa/providers/oauth1/OAuth1AuthProvider.java (53) M code/app/com/feth/play/module/pa/providers/oauth1/linkedin/LinkedinAuthProvider.java (6) M code/app/com/feth/play/module/pa/providers/oauth1/twitter/TwitterAuthProvider.java (6) M code/app/com/feth/play/module/pa/providers/oauth1/xing/XingAuthProvider.java (19) M code/app/com/feth/play/module/pa/providers/oauth2/OAuth2AuthProvider.java (70) M code/app/com/feth/play/module/pa/providers/oauth2/eventbrite/EventBriteAuthProvider.java (17) M code/app/com/feth/play/module/pa/providers/oauth2/facebook/FacebookAuthProvider.java (38) M code/app/com/feth/play/module/pa/providers/oauth2/foursquare/FoursquareAuthProvider.java (29) M code/app/com/feth/play/module/pa/providers/oauth2/github/GithubAuthProvider.java (29) M code/app/com/feth/play/module/pa/providers/oauth2/google/GoogleAuthProvider.java (26) M code/app/com/feth/play/module/pa/providers/oauth2/pocket/PocketAuthProvider.java (33) M code/app/com/feth/play/module/pa/providers/oauth2/untappd/UntappdAuthProvider.java (63) M code/app/com/feth/play/module/pa/providers/oauth2/vk/VkAuthProvider.java (25) M code/app/com/feth/play/module/pa/providers/openid/OpenIdAuthProvider.java (42) M code/app/com/feth/play/module/pa/providers/password/UsernamePasswordAuthProvider.java (11) M code/app/com/feth/play/module/pa/providers/wwwauth/WWWAuthenticateProvider.java (7) M code/app/com/feth/play/module/pa/providers/wwwauth/basic/BasicAuthProvider.java (28) M code/app/com/feth/play/module/pa/providers/wwwauth/negotiate/SpnegoAuthProvider.java (24) M code/app/com/feth/play/module/pa/service/UserServicePlugin.java (12) M code/project/build.properties (2) M code/project/plugins.sbt (2) M code/version.sbt (2)

-- Patch Links --

https://github.com/joscha/play-authenticate/pull/298.patch https://github.com/joscha/play-authenticate/pull/298.diff

You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/joscha/play-authenticate/pull/298

joscha commented 8 years ago

nice work @pdolega

oexza commented 8 years ago

ping! @pdolega :)

pdolega commented 8 years ago

Status is:

pdolega commented 8 years ago

So status is - code compile, test app working, unit tests working.

Gotta see not samples and make them work and then make code a little bit nicer (probably some namings could be improved).

oexza commented 8 years ago

LGTM. @joscha

pdolega commented 8 years ago

Guys, let me look first at samples (before merging) - they stopped working definitely. I should be able to do this in two days max.

pdolega commented 8 years ago

Update: one of the samples upgraded, readme files updated.

Everything seems to work.

Last thing that seems to be missing is play-authenticate-usage sample (which I am right now half-way through)

oexza commented 8 years ago

that one will be the most challenging as there's a lot to migrate in it. good work

pdolega commented 8 years ago

I am on it - trying to squeeze it somehow between may other daily responsibilities. I am nearly there - needed to migrate Deadbolt and eBean too unfortunately (both were not working with 2.5)

pdolega commented 8 years ago

OK - so basically everything is now upgraded. play-authenticate-usage also seems to work. The only thing that is left is running unit tests in this particular sample project.

I think it should work (classes are loaded correctly etc) - the problem I have is probably more correlated with selenium test. Here is the exception I am getting (below).

I think it may be worth to look at this issue on separate PR as this one is already kinda huge. If so, let me know what comments you guys @oexza / @joscha may have so I could fix them.

Also let me know if you want to have commits squashed.

Exception I am getting from selenium:

1459435423391   addons.xpi      DEBUG   Updating XPIState for {"id":"{972ce4c6-7e08-4474-a285-3208198ce6fd}","syncGUID":"CutQTTaH2OVQ","location":"app-global","version":"45.0.1","type":"theme","internalName":"classic/1.0","updateURL":null,"updateKey":null,"optionsURL":null,"optionsType":null,"aboutURL":null,"icons":{"32":"icon.png","48":"icon.png"},"iconURL":null,"icon64URL":null,"defaultLocale":{"name":"Default","description":"The default theme.","creator":"Mozilla","homepageURL":null,"contributors":["Mozilla Contributors"]},"visible":true,"active":true,"userDisabled":false,"appDisabled":false,"descriptor":"/usr/lib64/firefox/browser/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}.xpi","installDate":1458554148000,"updateDate":1458554148000,"applyBackgroundUpdates":1,"skinnable":true,"size":5028,"sourceURI":null,"releaseNotesURI":null,"softDisabled":false,"foreignInstall":false,"hasBinaryComponents":false,"strictCompatibility":true,"locales":[],"targetApplications":[{"id":"{ec8030f7-c20a-464f-9b0e-13a3a9e97384}","minVersion":"45.0.1","maxVersion":"45.0.1"}],"targetPlatforms":[],"seen":true}
1459435423391   addons.xpi      DEBUG   getModTime: Recursive scan of {972ce4c6-7e08-4474-a285-3208198ce6fd}
1459435423392   DeferredSave.extensions.json    DEBUG   Save changes
1459435423392   addons.xpi      DEBUG   Updating database with changes to installed add-ons
1459435423392   addons.xpi-utils        DEBUG   Updating add-on states
1459435423392   addons.xpi-utils        DEBUG   Writing add-ons list
1459435423393   addons.xpi      DEBUG   Registering manifest for /usr/lib64/firefox/browser/features/loop@mozilla.org.xpi
1459435423393   addons.xpi      DEBUG   Calling bootstrap method startup on loop@mozilla.org version 0.1
1459435423403   addons.manager  DEBUG   Starting provider: <unnamed-provider>
1459435423404   addons.manager  DEBUG   Registering shutdown blocker for <unnamed-provider>
1459435423404   addons.manager  DEBUG   Provider finished startup: <unnamed-provider>
1459435423413   addons.manager  DEBUG   Registering shutdown blocker for XPIProvider
1459435423413   addons.manager  DEBUG   Provider finished startup: XPIProvider
1459435423413   addons.manager  DEBUG   Starting provider: LightweightThemeManager
1459435423413   addons.manager  DEBUG   Registering shutdown blocker for LightweightThemeManager
1459435423413   addons.manager  DEBUG   Provider finished startup: LightweightThemeManager
1459435423413   addons.manager  DEBUG   Starting provider: GMPProvider
1459435423417   addons.manager  DEBUG   Registering shutdown blocker for GMPProvider
1459435423417   addons.manager  DEBUG   Provider finished startup: GMPProvider
1459435423417   addons.manager  DEBUG   Starting provider: PluginProvider
1459435423417   addons.manager  DEBUG   Registering shutdown blocker for PluginProvider
1459435423417   addons.manager  DEBUG   Provider finished startup: PluginProvider
1459435423418   addons.manager  DEBUG   Completed startup sequence
1459435423578   DeferredSave.extensions.json    DEBUG   Starting write
1459435423673   addons.repository       DEBUG   No addons.json found.
1459435423673   DeferredSave.addons.json        DEBUG   Save changes
1459435423675   DeferredSave.addons.json        DEBUG   Starting timer
1459435423686   addons.manager  DEBUG   Starting provider: PreviousExperimentProvider
1459435423686   addons.manager  DEBUG   Registering shutdown blocker for PreviousExperimentProvider
1459435423686   addons.manager  DEBUG   Provider finished startup: PreviousExperimentProvider
1459435423688   DeferredSave.extensions.json    DEBUG   Write succeeded
1459435423689   addons.xpi-utils        DEBUG   XPI Database saved, setting schema version preference to 17
1459435423752   DeferredSave.addons.json        DEBUG   Starting write
1459435423764   DeferredSave.addons.json        DEBUG   Write succeeded

        at org.openqa.selenium.firefox.internal.NewProfileExtensionConnection.start(NewProfileExtensionConnection.java:123)
        at org.openqa.selenium.firefox.FirefoxDriver.startClient(FirefoxDriver.java:271)
        at org.openqa.selenium.remote.RemoteWebDriver.<init>(RemoteWebDriver.java:117)
        at org.openqa.selenium.firefox.FirefoxDriver.<init>(FirefoxDriver.java:216)
        at org.openqa.selenium.firefox.FirefoxDriver.<init>(FirefoxDriver.java:211)
        at org.openqa.selenium.firefox.FirefoxDriver.<init>(FirefoxDriver.java:207)
        at org.openqa.selenium.firefox.FirefoxDriver.<init>(FirefoxDriver.java:120)
        at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
        at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
        at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
        at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
        at java.lang.Class.newInstance(Class.java:442)
        at play.api.test.WebDriverFactory$.apply(Selenium.scala:116)
        at play.api.test.WebDriverFactory.apply(Selenium.scala)
        at play.test.TestBrowser.<init>(TestBrowser.java:27)
        at play.test.Helpers.testBrowser(Helpers.java:507)
        at test.OAuth2Test.provideBrowser(OAuth2Test.java:28)
        at play.test.WithBrowser.createBrowser(WithBrowser.java:28)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
        at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
        at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
        at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:24)
        at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
        at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
        at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
        at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
        at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
        at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
        at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
        at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
        at org.junit.runners.Suite.runChild(Suite.java:128)
        at org.junit.runners.Suite.runChild(Suite.java:27)
        at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
        at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
        at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
        at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
        at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
        at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
        at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
        at org.junit.runner.JUnitCore.run(JUnitCore.java:115)
        at com.novocode.junit.JUnitRunner$1.execute(JUnitRunner.java:132)
        at sbt.ForkMain$Run$2.call(ForkMain.java:296)
        at sbt.ForkMain$Run$2.call(ForkMain.java:286)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
        at java.lang.Thread.run(Thread.java:745)
[info] application - Shutting down connection pool.
oexza commented 8 years ago

Awesome! there's no need for a new issue as the problem is a part of this PR, squashing the commits would be good since this PR solves ONE problem "migrating to play 2.5" but then i doubt if its neccessary i'd leave that to @joscha.

what version of selenium is in the class path? the problem may be related to this issue https://github.com/seleniumhq/selenium/issues/1431 upgrading to the latest selenium driver may solve it.

pdolega commented 8 years ago

@oexza you were right, needed to update selenium lib to newer version. Works now :+1:

@joscha / @oexza let me know guys what you want to have here before we could merge this in.

Also I squashed all the commits already

oexza commented 8 years ago

Good Good Good work! LGTM @joscha

On 4/2/16, pdolega notifications@github.com wrote:

@oexza you were right, needed to update selenium lib to newer version. Works now :+1: https://github.com/joscha/play-authenticate/pull/298/commits/9a58aad1371aa7e6c5b2959816ff5fb6839795b0

@joscha / @oexza let me know guys what you want to have here before we could merge this in.

You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/joscha/play-authenticate/pull/298#issuecomment-204660901

joscha commented 8 years ago

Really good! I will create a new branch and a Snapshot Release later today.

oexza commented 8 years ago

ping! @joscha :smile: i'm looking to submit a PR this week that will spring board from this pr

joscha commented 8 years ago

0.8.0-SNAPSHOT was not published due to problems in the tests: https://travis-ci.org/joscha/play-authenticate/builds/120603837

pdolega commented 8 years ago

@joscha I'll take a look. Tests were passing on this PR here: https://travis-ci.org/joscha/play-authenticate/builds/120244214.

I'll take a look within 1 hour

joscha commented 8 years ago

the tests were not run there, as they are only run when the secret vars are available.

It's really strange, when I run them locally:

➜  play-authenticate-usage git:(master) ✗ sbt --info ++2.11.7 test
[info] Loading global plugins from /Users/joscha/.sbt/0.13/plugins
[info] Loading project definition from /Users/joscha/Development/play-authenticate/samples/java/play-authenticate-usage/project
[info] Set current project to play-authenticate-usage (in build file:/Users/joscha/Development/play-authenticate/samples/java/play-authenticate-usage/)
[info] Setting version to 2.11.7
[info] Reapplying settings...
[info] Set current project to play-authenticate-usage (in build file:/Users/joscha/Development/play-authenticate/samples/java/play-authenticate-usage/)
[success] Total time: 1 s, completed 04.04.2016 23:01:20

they are not even found.

joscha commented 8 years ago

You can run them with something like this:

#!/bin/sh
export GOOGLE_USER_PASSWORD=XXX \
       GOOGLE_CLIENT_ID=XXX \
       GOOGLE_CLIENT_SECRET=XXX \
       EVENTBRITE_CLIENT_ID=XXX \
       EVENTBRITE_CLIENT_SECRET=XXX \
       EVENTBRITE_USER_PASSWORD=XXX \
       FACEBOOK_USER_PASSWORD=XXX \
       FACEBOOK_CLIENT_ID=XXX \
       FACEBOOK_CLIENT_SECRET=XXX
#activator -jvm-debug 5005
sbt --info test

the error is actually that the config can not be found:

Error injecting constructor, java.lang.RuntimeException: Provider 'eventbrite' missing needed setting 'clientId'

so proper credentials are most likely not needed.

joscha commented 8 years ago

I believe it has something to do with how the configuration was amended in 2.4.x (most likely changed in 2.5.x): https://github.com/joscha/play-authenticate/blob/master/samples/java/play-authenticate-usage/test/test/GoogleOAuth2Base.java#L15-L17

however as the tests won't run for me locally at the moment, I can't reproduce it.

oexza commented 8 years ago

@pdolega i think part of the problem is the build.sbt files of both the core and play-authenticate-usage example should have "com.feth" %% "play-easymail" % "0.8.0-SNAPSHOT" not "com.feth" %% "play-easymail" % "0.7.0"

pdolega commented 8 years ago

@joscha @oexza my mistake. You are right I screwed amending of these props (I tested them by simply putting them into configuration file, not via environment vars).

Missed that, fixing now.

pdolega commented 8 years ago

@joscha @oexza fix here: https://github.com/joscha/play-authenticate/pull/299

joscha commented 8 years ago

@oexza I tried upgrading easymail to 0.8.0-SNAPSHOT, there was a compile time error because of a missing symbol, I reverted the change. I think we changed something accidentally when doing the upgrade... On 4 Apr 2016 23:17, "oexza" notifications@github.com wrote:

@pdolega https://github.com/pdolega i think part of the problem is the build.sbt files of both the core and play-authenticate-usage example should have "com.feth" %% "play-easymail" % "0.8.0-SNAPSHOT" not "com.feth" %% "play-easymail" % "0.7.0"

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/joscha/play-authenticate/pull/298#issuecomment-205292731

oexza commented 8 years ago

@joscha no the change was not accidental, migrating easymail to a DI component required changing its structure, which will break any project that depends on it. lets merge in #299 in after which i'll submit a fix.

joscha commented 8 years ago

Sounds good, I think it would be great if we could keep backwards compatibility... On 5 Apr 2016 10:29 am, "oexza" notifications@github.com wrote:

@joscha https://github.com/joscha no the change was not accidental, migrating easymail to a DI component required changing its structure, which will break any project that depends on it. lets merge in #299 https://github.com/joscha/play-authenticate/pull/299 in after which i'll submit a fix.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/joscha/play-authenticate/pull/298#issuecomment-205558463

pdolega commented 8 years ago

@joscha Yeah that would be nice (especially as this is actually minor version number that changed). Unfortunately even play-framework itself doesn't follow semver - I think it's impossible to keep backward compatibility here as plugin class disappeared (and whole plugin mechanism).

oexza commented 8 years ago

yeah it would be nice to, unfortunately we can't for the reason @pdolega has given, the plugin system upon which Typesafe play-mailer module depended on is gone.

On 4/5/16, pdolega notifications@github.com wrote:

@joscha Yeah that would be nice (especially as this is actually minor version number that changed). Unfortunately even play-framework itself doesn't follow semver - I think it's impossible to keep backward compatibility here as plugin class disappeared (and whole plugin mechanism).

You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/joscha/play-authenticate/pull/298#issuecomment-205629161

pdolega commented 8 years ago

A thought - perhaps it may be worth to switch versioning scheme of play-authenticate and easymail to be aligned with play-framework versioning number. Would make compatibility more explicit...

joscha commented 8 years ago

Yes, deadbolt did that as well - there is nothing that keeps us from it I guess. On 5 Apr 2016 5:10 pm, "pdolega" notifications@github.com wrote:

A thought - perhaps it may be worth to switch versioning scheme of play-authenticate and easymail to be aligned with play-framework versioning number. Would make compatibility more explicit...

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/joscha/play-authenticate/pull/298#issuecomment-205689039

joscha commented 8 years ago

0.8.0-SNAPSHOT has been released.