ring

jmromrell 2019-04-26T16:31:00.000900Z

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

jmromrell 2019-04-26T16:32:55.002900Z

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

jmromrell 2019-04-26T16:37:40.007800Z

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.

jmromrell 2019-04-26T16:40:19.010500Z

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)

jmromrell 2019-04-26T16:41:59.011700Z

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

seancorfield 2019-04-26T17:39:25.015600Z

@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.

seancorfield 2019-04-26T17:40:05.015900Z

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)
  ...

seancorfield 2019-04-26T17:41:18.016600Z

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

seancorfield 2019-04-26T17:41:48.016800Z

(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)))

jmromrell 2019-04-26T17:51:43.019800Z

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.

jmromrell 2019-04-26T17:52:01.020400Z

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

seancorfield 2019-04-26T17:53:14.021100Z

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.

jmromrell 2019-04-26T17:58:05.023400Z

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.

jmromrell 2019-04-26T17:59:18.024100Z

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.

ikitommi 2019-04-26T18:49:38.026100Z

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

ikitommi 2019-04-26T19:08:01.036700Z

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.

jmromrell 2019-04-26T19:30:48.037500Z

I'll check out reitit, thank you!

seancorfield 2019-04-26T19:35:48.040600Z

@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.

seancorfield 2019-04-26T19:36:49.041800Z

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.

seancorfield 2019-04-26T19:38:05.043Z

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.

seancorfield 2019-04-26T19:45:07.046400Z

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.

jmromrell 2019-04-26T19:45:40.047Z

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).

jmromrell 2019-04-26T19:46:49.048100Z

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.

jmromrell 2019-04-26T19:48:00.049500Z

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?

jmromrell 2019-04-26T19:48:09.049700Z

That way it is passed through the call tree?

seancorfield 2019-04-26T19:49:49.051200Z

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.

seancorfield 2019-04-26T19:50:15.051800Z

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

jmromrell 2019-04-26T19:51:02.052500Z

I see. Thank you for the advice!

seancorfield 2019-04-26T19:51:11.052800Z

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

seancorfield 2019-04-26T19:51:25.053200Z

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

jmromrell 2019-04-26T19:51:51.053700Z

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

seancorfield 2019-04-26T19:55:52.055800Z

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.

jmromrell 2019-04-26T19:56:38.057Z

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

seancorfield 2019-04-26T19:58:01.058900Z

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).

seancorfield 2019-04-26T19:58:21.059400Z

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

jmromrell 2019-04-26T19:59:15.060300Z

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. 🙂

jmromrell 2019-04-26T19:59:24.060600Z

Thank you for all the help and advice!

2
ikitommi 2019-04-26T21:51:12.069900Z

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.