Incorporate SecurityManager into Spacelift

[smiklosovic]

It could be done simply just on CommandTool / Task or on Spacelift object.

Examples:

Spacelift.task(CommandTool.class)
    .programName("echo")
    .paramaters("hello world")
    .withPermission(Permisson p) // by default used java.security.AllPermission
    .withSecurityManager(SecurityManager sm) // used default when not set
    .execute()
    .await()

on Spacelift object

Spacelift.withPermission(Permission p).withSecurityManager(SecurityManager sm)
    .task(CommandTool.class)
    .programName("echo")
    .paramaters("hello world")
    .execute()
    .await()

Spacelift.security(Permission p)
Spacelift.security(Permission p, SecurityManager sm)

Spacelift.task(MyTask.class).security(Permission p, SecurityManager sm)
Spacelift.task(MyTask.class).security(Permission p)

I would go for adding it to Task class instead of Spacelift because you could specify security managers and permissions on every (possibly chained) task and not globally on a Spacelift object.

Internally, it would check these permissions and execute() would be carried out only in case it passes, throwing SecurityException otherwise.

By doing this, developer of some Task does not need to do anything in order to secure his task, all it would take would be to set these permissions as suggested above, or directly in the Task definition by overloading class scoped security getters / setters

While this could be implemented by wrapping SecurityManager logic around actual Spacelift execution, for the readability / reusability this approach is more clear and explicit to me.

[kpiwko]

That would work only if we make sure that a task does not execute other task internally. Any ideas how to ensure that?

[smiklosovic]

heh, I didnt think about that. We could kind of hack it, something like counting the number of process() method invocations before eventual then(). process() invocation would increase counter by one, then() would decrease it. So if you ever reach then() with non zero counter, it means you had to invoke another process() while you are inside a process(). Or in other words, you have to have counter set to 0 before every process() method invocation().

However I do not know what about the case when you are chaining it inside a process() method. Maybe the same principle would be done, but kind of recursively.

[kpiwko]

Counter would not work reliably. Think about parallel executions and such. I would say that we can make security manager flat - that is keep it only for Spacelift object and inherit it all subsequent calls.

Do you have a real example what your trying to solve? It might help to define API better.

[smiklosovic]

I agree that it is clunky solution so doing it on Spacelift level would be better.

I do not have any particular problem to solve with this. It is a good question if we really want to implement it. However when you look at it plainly practically, I do not see the reason why not.

For example, your Task could read some properties and you do not want to allow them being read. Or some networking stuff, writing to files and so on ...

But it is true that you could do it directly in the process() method so the question is what is the added value we want to provide here.

The goal is to put developer away from dealing with that logic in his process() method, all he needs to do is to override some security related methods, returning security manager and a permission. And that checking voodoo would be done for him.