grails / grails-core

The Grails Web Application Framework
http://grails.org
Apache License 2.0
2.79k stars 948 forks source link

3.2.10; Session-scoped Service fails #10687

Open transentia opened 7 years ago

transentia commented 7 years ago

I created a new 3.2.10 application.

Added a session-scoped service. Added a controller into which the service was injected.

Ran the application.

Accessed the controller from Chrome. Accessed the controller from Edge.

Expected: each browser would access a separate instance of the session-scoped service and thus get a unique response. Actual: each browser saw an identical response.

I note the doco saying:

Starting with Grails 2.3, new applications are generated with configuration that defaults the scope of controllers to singleton. If singleton controllers interact with prototype scoped services, the services effectively behave as per-controller singletons. If non-singleton services are required, controller scope should be changed as well.

This does not appear to apply to this case. In any case, if I add static scope = "session" to the controller, I see no change in behaviour.

I also note the admonition to

never store state in a service.

But the whole point of having a session-scoped service is to simplify just this situation for shopping carts and the like (eg http://ldaley.com/post/436635056/scoped-services-proxies-in-grails). In times of yore, one could use static proxy = true but that seems to have gone away(?).

And the specification:

session - A service is created for the scope of a user session

Looking at the following image, you can clearly see that each request (from a totally different browser) gets the same instance of the service.

The following image should show everything.

ssstest error

Code follows.

Service:

package ssstest

import grails.transaction.Transactional

@Transactional
class SSSService {
    static scope = "session"

    final uuid = UUID.randomUUID()

    def randomUUID() {
        uuid
    }
}

Controller:

package ssstest

class SSSController {
    static scope = "session"

    def SSSService

    def index() { render text: "${SSSService}:${SSSService.randomUUID()}" }
}
"C:\Program Files\Java\jdk1.8.0_131\bin\java" -XX:+TieredCompilation -XX:TieredStopAtLevel=1 -XX:CICompilerCount=3 -Djline.WindowsTerminal.directConsole=false -Dfile.encoding=UTF-8 -classpath C:\Users\BOBBRO~1\AppData\Local\Temp\classpath.jar org.grails.cli.GrailsCli -version
|Grails Version: 3.2.10
|Groovy Version: 2.4.10
|JVM Version: 1.8.0_131

Process finished with exit code 0
transentia commented 7 years ago

Forgot to attach the example project...it's v. small.

SSSTest.zip

graemerocher commented 7 years ago

in Grails 3 since we mostly removed the proxy support for services you have to define the Proxy manually in your Spring config. Currently no plans to re-instate this feature unless there is massive demand.

transentia commented 7 years ago

So something like (from: https://stackoverflow.com/a/29414444):

testingsessionServiceProxy(ScopedProxyFactoryBean) {
    targetBeanName = 'TestingsessionService'
    proxyTargetClass = true
}

I agree with the improvement label here but maybe it should apply to the doco as much as anything. The doco should say what you just said a lot more explicitly, cos this confused the hell out of me and led me to ditching the session-scoped service I had....work I could have done without having to do :-(

Does little ole me constitute "massive demand"?

jeffscottbrown commented 7 years ago

I haven't done it but I think this might be somewhat trivial to implement in a plugin.

transentia commented 7 years ago

Well, there is: https://github.com/alkemist/grails-scoped-proxy but this is obsolete as far as Grails 3 goes and is really aimed at making testing easier, as I read things.

The more I think about this, the more perturbed I become.

From my point of view, this was a big undocumented change that I stumbled over.

Moreover it caused a major security breach in that session information was leaked across all users on the system. None of my tests caught this (mea culpa, admittedly, but hard to actually automate a test for anyway...multiple browser instances are needed, for example). Nothing in the system or documentation warned me that I had left/was going to leave the session's barn doors open as I upgraded.

Opinion I generally work at public Utililties and in the current hyper-security-paranoia environment and if I "went public" with this, it would result in Grails being added to the do-not-use list. Rightly or wrongly. A very conservative industry area that looks for ANY TRIVIAL excuse to NOT adopt a technology. /Opinion

I am also having a hard time working out the current intended use-case for Grails' scoped proxies. Clearly session-scoping isn't EVER useful as it stands....and it WAS very useful.

Sorry to be so down on this, but Grails is now (a little bit) broken IMHO.

graemerocher commented 7 years ago

Opinion Making threats on a public Open Source Software issue tracker is not the best way to get things done /Opinion

In addition, we cannot account for users who poorly test their applications. Grails 3.x is a new major version and a major version upgrade brings major changes, testing your applications thoroughly both in an automated way and a manual way would have trivially accounted for any security flaws without attempting to lay the blame at the door on the tools that you use.

As Jeff Brown said, this is implemented trivially in a plugin without adding overhead to every user out there in terms of startup time. Just because there is an existing plugin out there that isn't Grails 3 compatible doesn't change the fact that it would be trivial to implement. If you need help with that, let us know https://grails.org/support.html

transentia commented 7 years ago

Making threats on a public Open Source Software issue tracker is not the best way to get things done

SINCERE APOLOGIES. That was NOT my intention. Honest!

I'm a great promoter of Grails and have been since it was a wee small thing. I've been an advocate for you guys within many organisations and at many conferences, etc.

OK. I tested badly. I'll cop that on the chin. Not the first bruise I have gathered. Won't be the last.

Still, my point remains. How was I to know that this was going to be a pain point? It was not in the past. It didn't appear in any change notes, etc. and I am struggling to see what good the situation as it stands is.

If a fresh adopter comes along, uses a session scoped service according to the doco and doesn't know the secret sauce that I have just discovered (s)he will be creating a massive security hole. I didn't find that until I put the app into production. Maybe others won't discover the need for a not-currently-existing plugin either.

graemerocher commented 7 years ago

An example showing how trivial it is to set this up. session-scope-demo.zip

transentia commented 7 years ago

Thanks.

U47 commented 7 years ago

+1 to the demand, although if it's made clear in the docs (especially that Grails 3 has changed) just to define the proxy manually as a bean, I don't see a huge issue with that.

martinduris commented 6 years ago

Hi, i wall really confused to setup 'request' scope for my service. Demo (above) really help me (i did not know that i need to setup proxy).

I suggest, place here http://docs.grails.org/3.2.9/ref/Services/scope.html some 'note' about proxy: when you put into google 'grails 3 service scope' -> you will go to docs, try to set as ssggested 'static scope = "session"' -> and you will stuck - nothing happens :-/

Thanks

vishwanath-openprise commented 6 years ago

+1