search

benlucchesi / grails-cookie-session

cookie sessions for grails applications
28 stars 32 forks source link

Fix for issue #40, session not present for Security Context persistence #41

Closed alesbukovsky closed 9 years ago

alesbukovsky commented 10 years ago

If an application did not use a session yet (i.e. none is created), the Security Context is not persisted causing issue with subsequent request authentication. This is sometimes hard to debug since some other components may create a session under the hood (e.g. flash scope). This patch causes response wrapper to create a new session if none exists and if compatibility with Spring Security is enabled (in fact, the filter checks for Spring Security listener presence and passed true/false flag down to the wrapper).

benlucchesi commented 10 years ago

Hi Ales,

Thanks for the contribution. I'll review it shortly and see if there's any possible conflicts.

Just so you know, there was an earlier fix made to cookie-sessions because it aggressively created cookies regardless of whether they were used or not. This was a problem because it broke grails' normal behavior of delaying creation of cookies until they are needed. Hopefully your fix does this!

Also, have you tested it with 1.x and 2.x spring security plugins? 2.x breaks compatibility with 1.x in that it changes where the spring security context is stored.

Ben Lucchesi | Chief Software Architect Granicus, Inc.work: 415.357.3618 x1300 [X]http://www.granicus.com/ Follow Us! Bloghttp://blog.granicus.com/ | Twitterhttp://twitter.com/granicus | Facebookhttp://www.facebook.com/pages/Granicus/134633056573520

From: Ales Bukovsky [notifications@github.com] Sent: Thursday, June 05, 2014 9:08 AM To: benlucchesi/grails-cookie-session-v2 Subject: [grails-cookie-session-v2] Fix for issue #40, session not present for Security Context persistence (#41)

If an application did not use a session yet (i.e. none is created), the Security Context is not persisted causing issue with subsequent request authentication. This is sometimes hard to debug since some other components may create a session under the hood (e.g. flash scope). This patch causes response wrapper to create a new session if none exists and if compatibility with Spring Security is enabled (in fact, the filter checks for Spring Security listener presence and passed true/false flag down to the wrapper).

You can merge this Pull Request by running

git pull https://github.com/alesbukovsky/grails-cookie-session-v2 fix-40

Or view, comment on, or merge it at:

https://github.com/benlucchesi/grails-cookie-session-v2/pull/41

Commit Summary

File Changes

Patch Links:

— Reply to this email directly or view it on GitHubhttps://github.com/benlucchesi/grails-cookie-session-v2/pull/41.

alesbukovsky commented 10 years ago

The fix only enforces new session if compatibility with Spring Security is enabled. I do not know if there is any further way to trim it down (presence of Security Context, maybe?)

I did not test older Spring Security plugins. But there might be no need to do so. I did not touch anything about the security context processing. I have only changed hard-coded false in the step that gets a session in response wrapper to a flag based on existence of a security listener bean.

benlucchesi commented 10 years ago

Thanks for the clarification. I'll look into creating a session late which covers your scenario. If nothing presents itself, Ill publish your fix. thanks!

Ben Lucchesi | Chief Software Architect Granicus, Inc.work: 415.357.3618 x1300 [X]http://www.granicus.com/ Follow Us! Bloghttp://blog.granicus.com/ | Twitterhttp://twitter.com/granicus | Facebookhttp://www.facebook.com/pages/Granicus/134633056573520

From: Ales Bukovsky [notifications@github.com] Sent: Thursday, June 05, 2014 11:47 AM To: benlucchesi/grails-cookie-session-v2 Cc: Benjamin Lucchesi Subject: Re: [grails-cookie-session-v2] Fix for issue #40, session not present for Security Context persistence (#41)

The fix only enforces new session if compatibility with Spring Security is enabled. I do not know if there is any further way to trim it down (presence of Security Context, maybe?)

I did not test older Spring Security plugins. But there might be no need to do so. I did not touch anything about the security context processing. I have only changed hard-coded false in the step that gets a session in response wrapper to a flag based on existence of a security listener bean.

— Reply to this email directly or view it on GitHubhttps://github.com/benlucchesi/grails-cookie-session-v2/pull/41#issuecomment-45258671.