Should plugs also run for subcontrollers?

[TWith2Sugars]

I had a scenario where I moved my auth from the pipe_through of the router to a controller because I only needed to authenticate parts of my api. I had two top level controllers, secured and public. I only added the plug to the secured controller (and none of it's sub controllers). This only secured the controller it self.

For this to work I had to add the auth plug to every controller/subcontroller (and there where a lot).

If plugs shouldn't run on subcontrollers should there be a mechanism for this? E.G. subplugs?

[Krzysztof-Cieslak]

I think we can add special case Subcontroller to the types of actions that will specify that the plug needs to run for subcontroller calls. However it shouldn't be included in All case (i.e. to run it for everything and subcontroller user should need to pass [All;Subcontroller] to the plug).

The starting point for implementation - https://github.com/SaturnFramework/Saturn/blob/master/src/Saturn/Controller.fs#L15-L25

[TWith2Sugars]

For deeply nested controllers would you expect this to propagate all the way down or just 1 level?

[Krzysztof-Cieslak]

I think the most straightforward implementation (i.e. just adding same code that we do for all actions also in the place where we add subcontroller to the routing) will mean that the plug will fire for the all calls to subcontroller, including the nested calls...

[NinoFloris]

@Krzysztof-Cieslak So I was thinking about this a bit, we would however get two ways of doing the same thing...

either you do:

let projects = requireAuth >=> controller {
    // ...
}

or new way would be:

let projects = controller {
    plugs [All; SubController] requireAuth
    // ...
}

I'm not sure I see the main advantage yet. And if you don't want to apply it to All actions/subControllers you could always define it like this

let projects = controller {
    subController "/items" (fun id -> requireAuth >=> items id)
    // ...
}

[Krzysztof-Cieslak]

So, after this loooong consideration I think I agree with @NinoFloris here - this is easily solvable with normal 🚀operator (or pipeline CE). So I think we’ll keep current design. Closing this one.