tynamo/tapestry-security

Allow to globally disable security via a symbol

Closed this issue · 11 comments

For testing environments, it is sometimes desirable to disable all security restrictions. A symbol could be introduced to control whether security is enabled (defaulting to true of course).

Closed in favor of #10.

I've never really had a need for this but following your aborted attempt in #10, perhaps the @marker syntax itself could be expanded to something like @marker(excludes = {@core, @Builltin})? (and includes as well for completeness...).

Another thought I had is that maybe you could disable security per test (or write a filter that could be conditionally included) simply by unbinding the securityManager after it's set (similar to SecurityService.invokeWithSecurityDisabled() ). I'll support the goal if you need to incorporate changes to the library to make this happen but you'd have to come up with at least an initial implementation yourself.

I've had another look at #10, maybe it's possible after all. I still can't seem to get the tests to run, I keep getting

Failed tests:   startContainer(org.tynamo.security.TapestrySecurityIntegrationTest): Couldn't find an available port to run the functional test server

The test wasn't able to find a free port to bind the server to. That's interesting, there's even some code for trying to find the next free port - want to debug AbstractContainerTest (in tynamo-test) to see what's up?

Ah, I found the problem, in AbstractContainerTest:86: e.getMessage().contains("refused") won't work if the message is localized.

I'm getting closer. I'll need to change SecurityServiceImpl to bypass Shiro and return fixed values if security is disabled. I'm not entirely sure which ones (e.g. should isUser() return true or false? And what about isGuest())?
I can probably make it work if you'd like to have this feature in tapestry-security. I don't exactly need it anymore so I'm okay if you wouldn't.

Who could have thought anybody would ever use anything else but en_US JDK for development? :). Thanks, any suggestions for fixing? Doesn't have to be bullet proof, just need to work for you and me for starters.

If you are close and want to complete the work, send the pull request and let's record it here. I'm not sure at this point whether I'd merge that in because I've never had anybody else asking for it. If it was for testing, I'd assume that users would want to control the returned values of isUser() and isGuest() but making that happen would open another can of worms entirely.

Thanks, any suggestions for fixing?

Just just setting the LANG environment variable for the tests should do the trick. I've done that with Gradle before, it's a single line. I have no idea how to do that with Maven though. It's probably also just a few hundred lines of XML...

If it was for testing, I'd assume that users would want to control the returned values of isUser() and isGuest() but making that happen would open another can of worms entirely.

If anyone wants to do that, they can always override the SecurityService service with their own implementation.
I'll reopen the PR so we can continue the discussion there.

I took look at your pull request and even wrote a test for it but I'm not going to merge this in. Simply disabling the security is not very useful for testing because real world applications are often full of programmatic security logic using the SecurityService. Instead, it might be useful to be able to put the security in some kind of test mode which would allow you to use a simulated subject that can be changed at runtime through a special security testing page. In any case, that deserves a separate issue (contributions welcome!).