Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 3 commits into from
May 10, 2021
Merged

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

merged 3 commits into from
May 10, 2021

Conversation

lachlan-roberts
Copy link
Contributor

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.

@lachlan-roberts lachlan-roberts mentioned this pull request Apr 26, 2021
Closed
jmcc0nn3ll
jmcc0nn3ll reviewed Apr 28, 2021
View reviewed changes
Copy link
Contributor

@jmcc0nn3ll jmcc0nn3ll left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

janbartel
janbartel requested changes Apr 29, 2021
View reviewed changes
Copy link
Contributor

@janbartel janbartel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@lachlan-roberts
Changes from review.
3995b85 
Signed-off-by: Lachlan Roberts <[email protected]>
janbartel
janbartel previously approved these changes Apr 29, 2021
View reviewed changes
Copy link
Contributor

@janbartel janbartel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lachlan-roberts
Changes from review.
d26c59f 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts lachlan-roberts merged commit e9f260f into jetty-10.0.x May 10, 2021
@lachlan-roberts 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
@lachlan-roberts
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 lachlan-roberts mentioned this pull request May 10, 2021
Merged
lachlan-roberts added a commit that referenced this pull request May 12, 2021
@lachlan-roberts
Issue #6205 - Fix issues with OpenID redirecting to wrong URI (#6211)
9d37dbd 
Use the OpenID state param to map to the redirect URI.
@jmcc0nn3ll
Copy link
Contributor

@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)

sbordet pushed a commit that referenced this pull request May 12, 2021
@lachlan-roberts @sbordet
Issue #6205 - Fix issues with OpenID redirecting to wrong URI (#6211)
2e7f5eb 
Use the OpenID state param to map to the redirect URI.
@jmcc0nn3ll
Copy link
Contributor

resolved!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregw gregw gregw left review comments

@jmcc0nn3ll jmcc0nn3ll jmcc0nn3ll left review comments

@janbartel janbartel janbartel left review comments

@sbordet sbordet sbordet approved these changes

Assignees

@lachlan-roberts lachlan-roberts

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

OpenIdAuthenticator may use incorrect redirect
5 participants
@lachlan-roberts @jmcc0nn3ll @gregw @janbartel @sbordet