OpenID Connect Authentication by lachlan-roberts · Pull Request #4038 · jetty/jetty.project

lachlan-roberts · 2019-08-29T23:41:28Z

Allow webapps to authenticate users using the OpenId Connect (OIDC) protocol based on OAuth 2.0. This can be done with any OpenId Provider supporting OIDC. For example by using googles OIDC service you can authenticate with your webapp using your google account.

I have tested authentication with Google, Microsoft and Yahoo so far.

See #137

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

improved comments Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

lachlan-roberts · 2019-09-02T03:32:41Z

OpenID Configuration

To configure OpenID Authentication with Jetty you need to specify the OpenID Provider, Client ID and Client Secret. These can be set as properties in the start.ini or start.d/openid.ini files.

OpenID Provider

The OpenID Provider must be an OAuth 2.0 Authentication Server which implements OpenID Connect. To use the jetty-openid module you must input the URL for the OpenID Provider you wish to use.

Examples:

Registering App with OpenID Provider

You must register an app with the OpenID provider which will give you a Client ID and Client Secret. Once set up you must also register all the possible URI's for your webapp with the path /j_security_check so that the OpenId provider will allow redirection back to the webapp.

for example these may look like

http://localhost:8080/openid-webapp/j_security_check
https://mywebsite.com/j_security_check

WebApp Specific Configuration

The webapp should be deployed with a jetty deployable descriptor XML file to configure the security handler with the error page and OpenIdAuthenticatorFactory.

The security handler should be given an init param for "org.eclipse.jetty.security.openid.error_page" with a path relative to the webapp where authentication errors should be redirected.

<Configure class="org.eclipse.jetty.webapp.WebAppContext">
    <Set name="contextPath">/myapp</Set>
    <Set name="war"><SystemProperty name="myapp.home"/>/myapp.war</Set>
    <Get name="securityHandler">
        <Call name="setInitParameter">
            <Arg>org.eclipse.jetty.security.openid.error_page</Arg>
            <Arg>/error</Arg>
        </Call>
        <Set name="authenticatorFactory">
            <New class="org.eclipse.jetty.security.openid.OpenIdAuthenticatorFactory"/>
        </Set>
    </Get>
</Configure>

Claims

Claims provide the application with details about the user, such as sub (unique id), name and email.

Once a user has been authenticated you can retrieve these claims can be accessed via a Session attribute.

Map<String, Object> userInfo = (Map)request.getSession().getAttribute("org.eclipse.jetty.security.openid.user_claims");

Scopes

Scopes can be used to request additional resources it needs about the user such as additional user claims.

For the Google OpenID Provider it can be useful to request the scopes profile and email which can give you additional user claims such as email, email_verified and picture.

web.xml

To use OpenID authentication the web.xml file must have the login-config element with an auth method value of OPENID.

The realm name must be set to the exact URL string used to set the OpenID Provider.

Example:

<login-config>
  <auth-method>OPENID</auth-method>
  <realm-name>https://accounts.google.com/</realm-name>
</login-config>

lachlan-roberts · 2019-09-02T03:37:20Z

@WalkerWatch can you help with the documentation for this

jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java

jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdConfiguration.java

jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java

jetty-openid/src/main/config/modules/openid/openid-baseloginservice.xml

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

sbordet · 2019-09-06T10:23:16Z

jetty-openid/src/main/config/modules/openid.mod

+# jetty.openid.openIdProvider=https://accounts.google.com/
+
+## The Client Identifier
+# jetty.openid.clientId=1051168419525-5nl60mkugb77p9j194mrh287p1e0ahfi.apps.googleusercontent.com


Is this some fake ID or a real one? You don't want to put here a real ID!

These were real for my testing app, but I will change to fake ones.
The client secret can be reset easily so didn't think it was a big deal if it was real while testing.

... and now that a real one has been published, you will need to revoke that from your account, otherwise anybody can start using your account for auth! Low risk I know, but good practise to always cancel any credentials that have been published accidentally.

Client secrets have been reset.
Except for on Yahoo because the site is so bugged I cannot reset the secret, delete the app, or contact support without errors.

sbordet · 2019-09-06T10:23:48Z

jetty-openid/src/main/config/modules/openid.mod

+# jetty.openid.clientId=1051168419525-5nl60mkugb77p9j194mrh287p1e0ahfi.apps.googleusercontent.com
+
+## The Client Secret
+# jetty.openid.clientSecret=XT_MIsSv_aUCGollauCaJY8S


Fake or real?

changed to fake

sbordet · 2019-09-06T10:26:37Z

jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java

+    public static final String __J_POST = "org.eclipse.jetty.security.openid.POST";
+    public static final String __J_METHOD = "org.eclipse.jetty.security.openid.METHOD";
+    public static final String __CSRF_TOKEN = "org.eclipse.jetty.security.openid.csrf_token";
+    public static final String __J_SECURITY_CHECK = "/j_security_check";


Get rid of the __ in front of public constants.
Why are these public?

I have removed the __ parts.
This was based off the FormAuthenticator where all the constants are prefixed with __ and are public.

sbordet · 2019-09-06T10:27:53Z

jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java

+     * If true, uris that cause a redirect to a login page will always
+     * be remembered. If false, only the first uri that leads to a login
+     * page redirect is remembered.
+     * See https://bugs.eclipse.org/bugs/show_bug.cgi?id=379909


What's this URL to a old issue tracking system? Remove it.

sbordet · 2019-09-06T10:28:35Z

jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java

+                }
+            }
+
+


Too many empty lines!

jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java

sbordet · 2019-09-06T10:45:47Z

jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java

+        }
+
+        InputStream content = (InputStream)connection.getContent();
+        return (Map)JSON.parse(IO.toString(content));


You are leaking the connection open.

I have changed the InputStream to be used with a try.
Or do I need to explicitly call connection.disconnect()?

sbordet · 2019-09-06T10:48:22Z

jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdUserPrincipal.java

+import java.io.Serializable;
+import java.security.Principal;
+
+public class OpenIdUserPrincipal implements Principal, Serializable


No point in making this class Serializable when its only field _credentials is not serializable.

Removed the Serializable.

So how is this auth going to work in a cluster? If we have a non sticky load balancer, we need the authentication to move with the session - hence it needs to be serializable.

I made OpenIdUserPrincipal, OpenIdCredentials and OpenIdConfiguration all Serializable. So now the SessionAuthentication used will allow the authentication to move with the session in a clustered environment.

jetty-openid/src/test/resources/jetty-logging.properties

jetty-util/src/main/java/org/eclipse/jetty/util/security/Constraint.java

jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationDemo.java

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

gregw

I think we need to consider the clustered behaviour of this. @janbartel can you assist.

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

lachlan-roberts · 2019-09-13T00:09:30Z

@sbordet can I get a re-review

Review and code cleanups. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>

lachlan-roberts added 4 commits August 29, 2019 08:03

add OpenId module to support OpenId Connect authentication

d33b96f

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

add additional validation, renaming and cleanups

1936963

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

fix build problems

2323dff

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

add configuration to authenticate new users with a base LoginService

f592e63

improved comments Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

lachlan-roberts requested a review from janbartel September 2, 2019 03:34

lachlan-roberts commented Sep 4, 2019

View reviewed changes

jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java Show resolved Hide resolved

janbartel requested changes Sep 6, 2019

View reviewed changes

fix claim validation

2770afb

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

sbordet requested changes Sep 6, 2019

View reviewed changes

sbordet reviewed Sep 6, 2019

View reviewed changes

jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationDemo.java Outdated Show resolved Hide resolved

lachlan-roberts added 3 commits September 9, 2019 14:57

add javadoc

be69598

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

changes from review

dc26739

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

OpenIdAuthenticator javadoc update

9134def

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

gregw requested changes Sep 9, 2019

View reviewed changes

implement serializable for auth in cluster

c67ac73

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

joakime added the Enhancement label Sep 10, 2019

gregw approved these changes Sep 10, 2019

View reviewed changes

lachlan-roberts requested review from janbartel and sbordet September 10, 2019 23:38

janbartel approved these changes Sep 10, 2019

View reviewed changes

lachlan-roberts added 2 commits September 11, 2019 12:14

Reworked OpenId demo into test using a local test OpenIdProvider

85cdc0d

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

add missing licence header

fbd1ba0

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

gregw mentioned this pull request Sep 11, 2019

Support OAuth #137

Closed

remove empty lines

eb1c77d

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

Fixes #137 - Support OAuth.

b3912d5

Review and code cleanups. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>

sbordet approved these changes Sep 13, 2019

View reviewed changes

joakime merged commit e013c24 into jetty-9.4.x Sep 13, 2019

joakime deleted the jetty-9.4.x-OpenId branch September 13, 2019 21:42

lachlan-roberts mentioned this pull request Sep 26, 2019

Need documentation for the jetty-openid module #4123

Closed

lachlan-roberts assigned lachlan-roberts and unassigned lachlan-roberts Oct 9, 2019

Conversation

lachlan-roberts commented Aug 29, 2019 • edited by gregw Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lachlan-roberts commented Sep 2, 2019

OpenID Configuration

OpenID Provider

Registering App with OpenID Provider

WebApp Specific Configuration

Claims

Scopes

web.xml

Uh oh!

lachlan-roberts commented Sep 2, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

gregw left a comment

Choose a reason for hiding this comment

Uh oh!

lachlan-roberts commented Sep 13, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lachlan-roberts commented Aug 29, 2019 •

edited by gregw

Loading