search

spring-attic / spring-social

Allows you to connect your applications with SaaS providers such as Facebook and Twitter.
http://projects.spring.io/spring-social
Apache License 2.0
619 stars 351 forks source link

ProviderSignInAttempt is not Serializable and ProviderSignInUtils does not respect session strategy correctly #126

Closed michaellavelle closed 9 years ago

michaellavelle commented 10 years ago

Modified ProviderSignInAttempt to ensure it is Serializable, and fixed ProviderSignInUtils so it correctly respects an injected SessionStrategy when retrieving ProviderSignInAttempt. Removed deprecated static methods which are incompatible with SessionStrategy work.

michaellavelle commented 10 years ago

I've been using the SessionStrategy feature introduced in the recent builds and have created a DynamoDB-backed SessionStrategy ( https://github.com/michaellavelle/spring-social-spring-data-dynamodb ).

When wiring the SessionStrategy into ProviderSignInController, at runtime when creating a new connection and navigating to sign up form, a stack trace results because an attempt is made to serialiaze a ProviderSignInAttempt which although Serializable is declared is not actually Serializable due to non-transient non-serializable service references.

trautonen commented 10 years ago

Yea, this was very very odd implementation of the sign in attempt. Although there is a fix that can be applied without code changes. If you make the ConnectionFactoryLocator and UserConnectionRepository Spring AOP proxies, they can be serialized. I think this approach is now in the documentation too. But I also consider that this is wrong way to do it.

cemo commented 10 years ago

This is definitely a blocker issue. Can someone review this PR please?

cemo commented 10 years ago

ping @habuma

habuma commented 10 years ago

This constitutes a breaking change. As has been suggested, there is a workaround with proxies and there are those using it successfully that way. If this PR is applied, however, then those apps will be broken.

My focus, up until recently, was to push a 1.1.0 release.I considered this PR prior to 1.1.0, but since it is a breaking change, I could not include it in 1.1.0. Therefore, I decided to defer it to 2.0.0.

In short: I am not ignoring this PR. I must, however, carefully consider what goes into each release and breaking changes cannot go into a point release. I am now working through planning for Spring Social 2.0.0, and this PR is likely to be included (after a more thorough review, of course).

cemo commented 10 years ago

@habuma would you mind showing me workaround? I had tried a few tricks but could not be successful.

elw00d commented 10 years ago

+1 Vote for it !

cemo commented 9 years ago

@habuma Is there any timeline for 2.0.0?

okohub commented 9 years ago

+1

we have in trouble with this problem. will you solve this? @habuma

HenokT commented 9 years ago

I'm having the same problem with ProviderSignInAttempt containing some non-serializable references and causing NotSerializableException when my session is getting persisted. I know this is a braking PR but is there a time line when it might be included in a future release?

habuma commented 9 years ago

This has been merged and is now available in the latest 1.1.1.BUILD-SNAPSHOT. You'll need to be sure to add Spring's snapshot repository (http://repo.spring.io/snapshot) to your build to try it.

Please take a moment and test this with the snapshot build and confirm that it addresses your needs. It will be included in a 1.1.1.RELEASE build soon.

cemo commented 9 years ago

@habuma thanks

HenokT commented 9 years ago

@habuma, I tried the 1.1.1.BUILD-SNAPSHOT and now a NullPointerException is thrown every time a request is processed by the SocialAuthenticationFilter. I believe this was a bug introduced in commit: https://github.com/spring-projects/spring-social/commit/e674f149fc9c538c39062269687d75d5b5c99e3e

SocialAuthenticationFilter was updated to store and use it's own copy of filterProcessesUrl instead of using the one from the super class but the problem is that the subclass's copy is never initialized.

Here is the stacktrace:

java.lang.NullPointerException: null
    at java.lang.String.startsWith(String.java:1393)
    at java.lang.String.startsWith(String.java:1422)
    at org.springframework.social.security.SocialAuthenticationFilter.getRequestedProviderId(SocialAuthenticationFilter.java:274)
    at org.springframework.social.security.SocialAuthenticationFilter.requiresAuthentication(SocialAuthenticationFilter.java:190)
    at org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter.doFilter(AbstractAuthenticationProcessingFilter.java:198)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:342)
    at org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:110)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:342)
    at org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter.doFilter(AbstractAuthenticationProcessingFilter.java:199)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:342)
    at com.potluckily.auth.TokenAuthenticationFilter.doFilter(TokenAuthenticationFilter.java:35)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:342)
    at org.springframework.security.web.header.HeaderWriterFilter.doFilterInternal(HeaderWriterFilter.java:57)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:342)
    at org.springframework.security.web.FilterChainProxy.doFilterInternal(FilterChainProxy.java:192)
    at org.springframework.security.web.FilterChainProxy.doFilter(FilterChainProxy.java:160)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:239)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206)
    at org.springframework.boot.actuate.trace.WebRequestTraceFilter.doFilterInternal(WebRequestTraceFilter.java:100)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:239)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206)
    at org.springframework.session.web.http.SessionRepositoryFilter.doFilterInternal(SessionRepositoryFilter.java:119)
    at org.springframework.session.web.http.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:65)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:239)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206)
    at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:88)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:239)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206)
    at org.springframework.boot.actuate.autoconfigure.MetricFilterAutoConfiguration$MetricsFilter.doFilterInternal(MetricFilterAutoConfiguration.java:90)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:239)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206)
    at com.potluckily.RewriteFilter.doFilter(RewriteFilter.java:49)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:239)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206)
    at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:219)
    at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:106)
    at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:501)
    at org.apache.catalina.valves.RemoteIpValve.invoke(RemoteIpValve.java:673)
    at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:142)
    at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:79)
    at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:88)
    at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:537)
    at org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1085)
    at org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:658)
    at org.apache.coyote.http11.Http11NioProtocol$Http11ConnectionHandler.process(Http11NioProtocol.java:222)
    at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1556)
    at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:1513)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
    at java.lang.Thread.run(Thread.java:745)
habuma commented 9 years ago

@HenokT : I'll have a look at this tomorrow. Admittedly, I only tested this change through ProviderSignInController. It sounds like your issue is when using SocialAuthenticationFilter, so I'll need to give that a try. On the surface, it sounds like a simple fix, though. Thanks for reporting it.

HenokT commented 9 years ago

@habuma I already submitted PR #157 with the fix. Please merge it with when you get a chance. Thanks in advance.