Fork me on GitHub
#ring
<
2019-04-26
>
jmromrell16:04:00

I'm running into a design problem with compojure. Given there is no compojure channel, might this be an appropriate place to discuss it?

jmromrell16:04:55

Specifically, I am trying to implement auth middleware. The desired behavior is that the middleware will prevent handler execution for unauthenticated or unauthorized users. However, I also do not want to force a user to be authenticated if the request does not match any of the routes that are wrapped by the middleware

jmromrell16:04:40

If I wrap routes in such middleware now, I have two options: 1. Ensure user is authenticated/authorized before passing request along. Problem is that at this point I do not know that the user's request will match any routes, so the authentication/authorization may be unnecessary and I might incorrectly return a HTTP 401 or 403 for a request when it should have been a 404 or a successful response from a route not in this middleware's context. 2. Ensure authentication/authorization after a response has been provided, ensuring a route has been matched in the middleware's context. Problem is this means that the handler has already been executed, which may mean the user just deleted something from a database they shouldn't have been authorized to do.

jmromrell16:04:19

I can address the second point by making every single endpoint return a [delay](https://clojuredocs.org/clojure.core/delay) which is realized by the auth middleware after ensuring the user is authenticated/authorized. But this potentially breaks middleware between the handler and the auth middleware which expected a realized response to inspect and modify. It also does not allow for auth middleware to be composed (for example, require authentication and basic permissions for an entire API, but more specific permissions for a particular set of routes)

jmromrell16:04:59

Is there a generally accepted solution to this problem? I feel others have to have run into this issue before.

seancorfield17:04:25

@jmromrell I often find I need middleware that runs only when routes are matched so I've created a variant of defroutes called defroutes-wrapped that takes an extra argument before the route definitions, which is middleware to wrap the handler in.

seancorfield17:04:05

So I'll have something like

(defroutes-wrapped api-routes #'api-middleware
  (POST  "/accept"             [] #'ctl-covalence/accept)
  (POST  "/block"              [] #'ctl-member/block)
  (GET   "/blocked"            [] #'ctl-search/blocked)
  ...

seancorfield17:04:18

And the api-middleware function can then check :compojure/route to see if it is exempt from auth checking.

seancorfield17:04:48

(defmacro defroutes-wrapped
  "Like Compojure's defroutes except it takes a middleware function and wraps
  all of the routes' handlers in that."
  [name middleware & routes]
  (concat [`compojure/defroutes name]
          (map (fn [[verb pattern args handler :as route]]
                 (if (= 4 (count route))
                   (list verb pattern args (list middleware handler))
                   route))
               routes)))

jmromrell17:04:43

I was actually just looking into an option similar to this. The auth middleware would push/pop the auth expectations onto a stack contained in a dynamic var which would be checked immediately prior to handler execution. I'm currently looking for a good hook in the compojure library to do a with-redefs around the handler execution rather than replacing an entire macro, if possible.

jmromrell17:04:01

Thanks for the helpful response! I'll definitely go this route (heh) if there isn't a good place to with-redefs

seancorfield17:04:14

with-redefs isn't something you want outside your test-only code. And I'd stay away from dynamic variables as much as possible too.

jmromrell17:04:05

Why would it be discouraged for me to do something like:

(defn my-create [path method info childs handler]
  (if (nil? childs)
    (->Route path method info childs (resolve-auth-requirement-stack handler))
    (->Route path method info childs handler)))

(with-redefs [compojure.api.routes/create my-create]
  ...)
It seems simpler than overriding their entire macro and replacing all places I reference it.

jmromrell17:04:18

And why should dynamic vars be avoided? I was inspired to this approach by the perseverance library, which uses a similar dynamic var approach to great effect.

ikitommi18:04:38

There is compojure.core/wrap-routes for that. But it doesn't help on mounting mw other than on enpoint.

ikitommi19:04:01

As you are using compojure-api, you could also check reitit-ring - has all the same features but it implements a route-first architecture: the mw-chain is only applied if there is a full match. You could most likely do same with bidi too.

jmromrell19:04:48

I'll check out reitit, thank you!

seancorfield19:04:48

@jmromrell with-redefs is not thread safe -- so make sure you're only doing this in one thread, presumably at startup, rather than using it on the fly; dynamic vars make it impossible to have multiple instances of things that depend on them if they run in the same thread -- so their use needs to be isolated to a single function call tree, i.e., keeping them tightly coupled with an internal implementation only. That's why you'll often hear people tell you to avoid them.

seancorfield19:04:49

I thought they were pretty cool when I got started with Clojure nearly nine years ago but I've learned to avoid them except in very specific situations -- and there are nearly always much cleaner ways of achieving similar things.

seancorfield19:04:05

I don't use with-redefs anywhere except in tests to mock certain lower-level functions. And I very rarely use dynamic variables at all, preferring to pass values through the call tree instead.

seancorfield19:04:07

In our 82,000 lines of Clojure at work, the only dynamic vars we use fall into two categories (with one exception): debug flags that can be dynamically rebound when testing/debugging; hooks for context that be dynamically rebound within tests. The one exception is a thread-specific context that is used entirely within a single call tree to simplify value passing.

jmromrell19:04:40

I've been reading into threading problems with with-redefs, and I can see why it should be avoided outside of testing or areas you can be certain are single-threaded (certainly not request handling on a busy web server).

jmromrell19:04:49

I'm still reading up on the concerns with dynamic vars, but appreciate you pointing me in that direction. I expect I will arrive at the same conclusions as you.

jmromrell19:04:00

You are saying in this case you would prefer to define a copy of the compojure macro(s), and instead of having the auth middleware add auth requirements to a stack in a dynamic var, to instead assoc that data in the request map?

jmromrell19:04:09

That way it is passed through the call tree?

seancorfield19:04:49

See my macro above -- it wraps the Compojure defroutes macro so that it can automatically apply middleware to routes that match (in terms of the number of elements in the route definitions). It's a syntactic convenience.

seancorfield19:04:15

In a smaller app, I've used let-routes from Compojure and explicitly wrapped the handlers in middleware.

jmromrell19:04:02

I see. Thank you for the advice!

seancorfield19:04:11

Here's an example of the latter, in a small web app designed to show beginners how to get started with common Clojure libs: https://github.com/seancorfield/usermanager-example/blob/master/src/usermanager/main.clj#L108-L132

seancorfield19:04:25

If there were a lot more routes, that would be painful.

jmromrell19:04:51

Yeah. In my case I'm implementing this for APIs with tens to hundreds of routes in various nested contexts

seancorfield19:04:52

If your routing is that complex, you might want to look at alternatives to Compojure. In my experience, it doesn't really scale to a large number of routes, and it's not particularly fast either.

jmromrell19:04:38

The largest API is essentially a team toolbox that we don't expose externally (even to other teams) and where performance is a non-issue. I have been looking into aleph/yada as possible alternatives for us to try later if needed

seancorfield19:04:01

Tommi's suggestion of reitit-ring is a good one to look at. We've used Bidi on one app and we've considered switching all our Compojure apps over to Bidi (partly because we could generate the routing data structure from other data/metadata which would make maintenance easier).

seancorfield19:04:21

I haven't looked at Yada but I know a lot of people love it.

jmromrell19:04:15

I'm definitely planning on looking into reitit at some point. I think I know someone at my company who uses it already. Just need to find time to do a deep dive on the available options. 🙂

jmromrell19:04:24

Thank you for all the help and advice!

ikitommi21:04:12

some rationale & benefits about the route-first (and reitit) here: https://www.slideshare.net/mobile/metosin/reitit-clojurenorth-2019-141438093/30. The example is actually the authorization middleware, so close. Disclaimer: maintainer in both compojure-api & reitit. I think similar things could be done with stacks like bidi+yada.