add oauth #31

jackielii · 2018-10-10T15:32:40Z

No description provided.

tedeling · 2018-10-11T09:38:51Z

Thanks! This adds oauth with Slack? Is it finished?

jackielii · 2018-10-11T09:44:50Z

@tedeling this add OAuth2 capability to it. I have deployed internally and started using it. Sorry I had to add a few things unrelated. I can find some time later on to clean up and open another pull request.

If you want to test, here is my config file:

ehour.standalone.port=18080
# derby, mysql and postgresql are supported. When derby is selected you can ignore the ehour.database lines
ehour.database=postgresql

# for mysql uncomment the following lines (and make sure postgresql lines below are commented out)
#ehour.database.driver=com.mysql.jdbc.Driver
#ehour.database.url=jdbc:mysql://127.0.0.1:3306/ehour?zeroDateTimeBehavior=convertToNull&amp
#ehour.database.username=
#ehour.database.password=

# for postgresql uncomment the following lines (and make sure mysql lines above are commented out)
ehour.database.driver=org.postgresql.Driver
ehour.database.url=jdbc:postgresql://127.0.0.1:5432/ehour
ehour.database.username=ehour
ehour.database.password=ehour

ehour.configurationType=DEVELOPMENT

ehour.translations=../eHour-wicketweb/src/dist/i18n

#ehour.disableAuth=true
ehour.enableOAuth=true
ehour.oauth2.callbackURI=/oauth2/callback
ehour.oauth2.hostURL=http://localhost:18080/eh

ehour.oauth2.name=Slack
ehour.oauth2.userAuthorizationUri=https://slack.com/oauth/authorize
ehour.oauth2.accessTokenUri=https://slack.com/api/oauth.access
ehour.oauth2.clientID=xxxxxxxx
ehour.oauth2.clientSecret=xxxxxxxx
ehour.oauth2.scope=identity.basic identity.email

#ehour.oauth2.userAuthorizationUri=http://sso-dex:5556/dex/auth
#ehour.oauth2.accessTokenUri=http://sso-dex:5556/dex/token
#ehour.oauth2.clientID=xxxxxx
#ehour.oauth2.clientSecret=xxxxx
#ehour.oauth2.scope=profile email openid



#ehour.enableBookWholeWeek=false

# enable/disable mail, defaults to true
ehour.enableMail=false

So I tested with two OAuth providers: slack & dex, they all implements the standard OAuth2.0

tedeling · 2018-10-11T09:48:16Z

Yes if could split the unrelated additions in another PR, that would be great.

tedeling · 2018-10-23T11:27:11Z

eHour-common/src/main/java/net/rrm/ehour/domain/User.java


    @NotNull
-    @Column(name = "USERNAME", length = 64)
+    @Column(name = "USERNAME", length = 1024)


Why are they so lengthy, oauth standard? The datamodel needs changes as well to accommodate the longer username.

Hi, this change is for when login with oauth, I have to set the username to the email as that's the only reliable field that comes with the claims that is readable. It's probably not necessary to set it to something readable, for example use the "subject" claim. I was trying to let the user able to login with their emails as well after logged in with Oauth the first time.

So I think we should remove this change.

jackielii

In general this PR needs a bit improvement to handle different oauth servers, at least make it less error prone.

My comments are the ones I think you can take on. However, I really don't have time to move it forward at the moment.

Hope this helps if some one really needs a oauth adapter.

BTW, my oauth claims response handling is based on slack

jackielii · 2018-10-25T20:19:18Z

eHour-persistence/pom.xml

-            <artifactId>eHour-common</artifactId>
-            <type>test-jar</type>
-        </dependency>
+        <!--<dependency>-->


this is excluded because somehow if there the docker build won't pass

jackielii · 2018-10-25T20:20:12Z

eHour-persistence/src/main/resources/query/common/Project.queries.hbm.xml

 	<query name="Project.findAllProjectsForCustomers">
 		FROM Project prj
-		WHERE prj.customer IN (:customers)
+		<!--WHERE prj.customer IN (:customers)-->


this is a temporary workaround, the customers if empty will generate invalid SQL. didn't have time to look into it

jackielii · 2018-10-25T20:20:24Z

eHour-persistence/src/main/resources/query/common/Project.queries.hbm.xml

 		FROM Project prj
-		WHERE prj.customer IN (:customers) AND
+		WHERE
+		<!--prj.customer IN (:customers) AND-->


same workaround as above

jackielii · 2018-10-25T20:20:41Z

eHour-service/pom.xml

-            <scope>test</scope>
-            <classifier>tests</classifier>
-        </dependency>
+        <!--<dependency>-->


same reason about the docker build

jackielii · 2018-10-25T20:20:52Z

eHour-wicketweb/pom.xml

-            <scope>test</scope>
-            <classifier>tests</classifier>
-        </dependency>
+        <!--<dependency>-->


same reason about the docker build

jackielii · 2018-10-25T20:20:57Z

eHour-wicketweb/pom.xml

-            <scope>test</scope>
-            <classifier>tests</classifier>
-        </dependency>
+        <!--<dependency>-->


same reason about the docker build

jackielii · 2018-10-25T20:22:09Z

eHour-wicketweb/src/main/java/net/rrm/ehour/ui/manage/user/UserFormPanel.java

        RequiredTextField<String> usernameField = new RequiredTextField<>("user.username");
        form.add(usernameField);
-        usernameField.add(new StringValidator(0, 32));
+        usernameField.add(new StringValidator(0, 1024));


if this needs to be reverted. then the oauth create user logic needs to be updated

jackielii · 2018-10-25T20:23:44Z

eHour-wicketweb/src/main/java/net/rrm/ehour/ui/login/page/OAuthCallback.java

+
+        JSONObject jsonResponse = new JSONObject(result);
+
+        boolean success = jsonResponse.getBoolean("ok");


this bit should be re-examined. different oauth server returns different response. in OIDC case, it doesn't return "ok" in response

jackielii · 2018-10-25T20:25:00Z

eHour-wicketweb/src/main/java/net/rrm/ehour/ui/login/page/OAuthCallback.java

+            u = new User();
+            u.setLastName(name);
+            u.setEmail(email);
+            u.setUsername(email);


this needs changing, if signing in only with Oauth, then probably should use subject as username

Jackie Li added 3 commits October 10, 2018 16:31

add oauth

537b606

dockerfile

b208329

dockerfile +

3fa6c3a

increase username limit

c880787

tedeling self-requested a review October 11, 2018 09:39

tedeling reviewed Oct 23, 2018

View reviewed changes

jackielii commented Oct 25, 2018

View reviewed changes

Jackie Li added 3 commits April 29, 2019 23:26

render nothing is not reporter

cbebb8c

add disable individual report function

04d2a53

jdk8, cache dep

64713d6


		JSONObject jsonResponse = new JSONObject(result);

		boolean success = jsonResponse.getBoolean("ok");

add oauth #31

Are you sure you want to change the base?

add oauth #31

Uh oh!

Conversation

jackielii commented Oct 10, 2018

Uh oh!

tedeling commented Oct 11, 2018

Uh oh!

jackielii commented Oct 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tedeling commented Oct 11, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jackielii left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jackielii commented Oct 11, 2018 •

edited

Loading