Issue #6205 - Fix issues with OpenID redirecting to wrong URI #6211

lachlan-roberts · 2021-04-26T05:22:52Z

Closes #6205

This change uses the CSRF token to index into a Map of CSRF_TOKEN to RedirectInfo. By doing this we can get the URI and method of the original request which started the OpenID authentication process. This will allow it to find the correct URI and not get confused with other resources requested by the browser, or by some calls made by javascript running on the page.

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

jmcc0nn3ll

I cherrypicked this over to jetty-11.0.x and it no longer tries to redirect to cometd internal urls.

However, I enabled the session-store-file and it is failing to persist:

java.io.NotSerializableException: org.eclipse.jetty.security.openid.OpenIdAuthenticator$UriRedirectInfo

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

janbartel

Make session attribute serializable and change name of method as indicated in comments.

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

janbartel

LGTM

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

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

Use the OpenID state param to map to the redirect URI.

jmcc0nn3ll · 2021-05-12T13:55:59Z

@lachlan-roberts I still see persist issues on Jetty 11.

2021-05-12 08:49:01.707:WARN :oejss.SessionHandler:qtp99451533-38: Unable to renew Session Id node01rgo03lt33jlxemrsf2crziju0:node01rgo03lt33jlxemrsf2crziju0.node0 -> node01lo2cluwb6gdl1dd42sp3pzjj1:node01lo2cluwb6gdl1dd42sp3pzjj1.node0
org.eclipse.jetty.server.session.UnwriteableSessionDataException: Unwriteable session node01lo2cluwb6gdl1dd42sp3pzjj1 for node0__0.0.0.0
	at org.eclipse.jetty.server.session.FileSessionDataStore.doStore(FileSessionDataStore.java:333)
	at org.eclipse.jetty.server.session.AbstractSessionDataStore.lambda$store$2(AbstractSessionDataStore.java:220)
	at org.eclipse.jetty.server.handler.ContextHandler.handle(ContextHandler.java:1425)
	at org.eclipse.jetty.server.handler.ContextHandler.handle(ContextHandler.java:1462)
	at org.eclipse.jetty.server.session.SessionContext.run(SessionContext.java:87)
	at org.eclipse.jetty.server.session.AbstractSessionDataStore.store(AbstractSessionDataStore.java:230)
	at org.eclipse.jetty.server.session.AbstractSessionCache.renewSessionId(AbstractSessionCache.java:787)
	at org.eclipse.jetty.server.session.AbstractSessionCache.renewSessionId(AbstractSessionCache.java:752)
	at org.eclipse.jetty.server.session.SessionHandler.renewSessionId(SessionHandler.java:1108)
	at org.eclipse.jetty.server.session.DefaultSessionIdManager.renewSessionId(DefaultSessionIdManager.java:450)
	at org.eclipse.jetty.server.session.Session.renewId(Session.java:811)
	at org.eclipse.jetty.security.authentication.LoginAuthenticator.renewSession(LoginAuthenticator.java:126)
	at org.eclipse.jetty.security.authentication.LoginAuthenticator.login(LoginAuthenticator.java:65)
	at org.eclipse.jetty.security.openid.OpenIdAuthenticator.login(OpenIdAuthenticator.java:167)
	at org.eclipse.jetty.security.openid.OpenIdAuthenticator.validateRequest(OpenIdAuthenticator.java:308)
	at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:515)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:122)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:223)
	at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1571)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:221)
	at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1358)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:176)
	at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:463)
	at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1544)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:174)
	at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1280)
	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:129)
	at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:192)
	at org.eclipse.jetty.server.handler.HandlerList.handle(HandlerList.java:51)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:122)
	at org.eclipse.jetty.server.Server.handle(Server.java:562)
	at org.eclipse.jetty.server.HttpChannel.lambda$handle$0(HttpChannel.java:399)
	at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:656)
	at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:391)
	at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:282)
	at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:319)
	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:100)
	at org.eclipse.jetty.io.SocketChannelEndPoint$1.run(SocketChannelEndPoint.java:101)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:333)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:310)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:168)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:126)
	at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:378)
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:894)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1038)
	at java.base/java.lang.Thread.run(Thread.java:829)
Caused by: 
java.io.NotSerializableException: org.eclipse.jetty.security.openid.OpenIdAuthenticator
	at java.base/java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1185)
	at java.base/java.io.ObjectOutputStream.defaultWriteFields(ObjectOutputStream.java:1553)
	at java.base/java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1510)
	at java.base/java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1433)
	at java.base/java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1179)
	at java.base/java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:349)
	at org.eclipse.jetty.server.session.SessionData.serializeAttributes(SessionData.java:111)
	at org.eclipse.jetty.server.session.FileSessionDataStore.save(FileSessionDataStore.java:481)
	at org.eclipse.jetty.server.session.FileSessionDataStore.doStore(FileSessionDataStore.java:325)
	at org.eclipse.jetty.server.session.AbstractSessionDataStore.lambda$store$2(AbstractSessionDataStore.java:220)
	at org.eclipse.jetty.server.handler.ContextHandler.handle(ContextHandler.java:1425)
	at org.eclipse.jetty.server.handler.ContextHandler.handle(ContextHandler.java:1462)
	at org.eclipse.jetty.server.session.SessionContext.run(SessionContext.java:87)
	at org.eclipse.jetty.server.session.AbstractSessionDataStore.store(AbstractSessionDataStore.java:230)
	at org.eclipse.jetty.server.session.AbstractSessionCache.renewSessionId(AbstractSessionCache.java:787)
	at org.eclipse.jetty.server.session.AbstractSessionCache.renewSessionId(AbstractSessionCache.java:752)
	at org.eclipse.jetty.server.session.SessionHandler.renewSessionId(SessionHandler.java:1108)
	at org.eclipse.jetty.server.session.DefaultSessionIdManager.renewSessionId(DefaultSessionIdManager.java:450)
	at org.eclipse.jetty.server.session.Session.renewId(Session.java:811)
	at org.eclipse.jetty.security.authentication.LoginAuthenticator.renewSession(LoginAuthenticator.java:126)
	at org.eclipse.jetty.security.authentication.LoginAuthenticator.login(LoginAuthenticator.java:65)
	at org.eclipse.jetty.security.openid.OpenIdAuthenticator.login(OpenIdAuthenticator.java:167)
	at org.eclipse.jetty.security.openid.OpenIdAuthenticator.validateRequest(OpenIdAuthenticator.java:308)
	at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:515)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:122)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:223)
	at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1571)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:221)
	at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1358)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:176)
	at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:463)
	at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1544)
	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:174)
	at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1280)
	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:129)
	at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:192)
	at org.eclipse.jetty.server.handler.HandlerList.handle(HandlerList.java:51)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:122)
	at org.eclipse.jetty.server.Server.handle(Server.java:562)
	at org.eclipse.jetty.server.HttpChannel.lambda$handle$0(HttpChannel.java:399)
	at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:656)
	at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:391)
	at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:282)
	at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:319)
	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:100)
	at org.eclipse.jetty.io.SocketChannelEndPoint$1.run(SocketChannelEndPoint.java:101)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:333)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:310)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:168)
	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:126)
	at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:378)
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:894)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1038)
	at java.base/java.lang.Thread.run(Thread.java:829)

Use the OpenID state param to map to the redirect URI.

jmcc0nn3ll · 2021-05-12T20:14:46Z

resolved!

Issue #6205 - Use the OpenID state param to map to the redirect URI.

8481dda

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

lachlan-roberts mentioned this pull request Apr 26, 2021

Issue #6205 - Make OpenID alwaysSaveUri configuration available via init param. #6206

Closed

lachlan-roberts requested review from sbordet and jmcc0nn3ll April 27, 2021 06:09

jmcc0nn3ll assigned jmcc0nn3ll and lachlan-roberts and unassigned jmcc0nn3ll Apr 28, 2021

jmcc0nn3ll reviewed Apr 28, 2021

View reviewed changes

sbordet requested changes Apr 28, 2021

View reviewed changes

janbartel requested changes Apr 29, 2021

View reviewed changes

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

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

janbartel requested changes Apr 29, 2021

View reviewed changes

Changes from review.

3995b85

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

lachlan-roberts requested review from janbartel and sbordet April 29, 2021 06:01

janbartel previously approved these changes Apr 29, 2021

View reviewed changes

sbordet requested changes Apr 29, 2021

View reviewed changes

Changes from review.

d26c59f

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

lachlan-roberts dismissed janbartel’s stale review via d26c59f May 3, 2021 07:09

lachlan-roberts requested a review from sbordet May 3, 2021 07:10

sbordet approved these changes May 3, 2021

View reviewed changes

lachlan-roberts merged commit e9f260f into jetty-10.0.x May 10, 2021

lachlan-roberts deleted the jetty-10.0.x-6205-OpenID-Redirect2 branch May 10, 2021 05:19

lachlan-roberts added a commit that referenced this pull request May 10, 2021

Issue #6205 - Fix issues with OpenID redirecting to wrong URI (#6211)

0e0558f

Use the OpenID state param to map to the redirect URI.

lachlan-roberts mentioned this pull request May 10, 2021

Issue #6205 - Fix issues with OpenID redirecting to wrong URI (#6211) #6245

Merged

lachlan-roberts added this to In progress in Jetty 10.0.3/11.0.3 via automation May 12, 2021

lachlan-roberts moved this from In progress to Done in Jetty 10.0.3/11.0.3 May 12, 2021

lachlan-roberts added a commit that referenced this pull request May 12, 2021

Issue #6205 - Fix issues with OpenID redirecting to wrong URI (#6211)

9d37dbd

Use the OpenID state param to map to the redirect URI.

sbordet pushed a commit that referenced this pull request May 12, 2021

Issue #6205 - Fix issues with OpenID redirecting to wrong URI (#6211)

2e7f5eb

Use the OpenID state param to map to the redirect URI.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #6205 - Fix issues with OpenID redirecting to wrong URI #6211

Issue #6205 - Fix issues with OpenID redirecting to wrong URI #6211

lachlan-roberts commented Apr 26, 2021

jmcc0nn3ll left a comment •

edited

janbartel left a comment

janbartel left a comment

jmcc0nn3ll commented May 12, 2021

jmcc0nn3ll commented May 12, 2021

Issue #6205 - Fix issues with OpenID redirecting to wrong URI #6211

Issue #6205 - Fix issues with OpenID redirecting to wrong URI #6211

Conversation

lachlan-roberts commented Apr 26, 2021

jmcc0nn3ll left a comment • edited

Choose a reason for hiding this comment

janbartel left a comment

Choose a reason for hiding this comment

janbartel left a comment

Choose a reason for hiding this comment

jmcc0nn3ll commented May 12, 2021

jmcc0nn3ll commented May 12, 2021

jmcc0nn3ll left a comment •

edited