joy icon indicating copy to clipboard operation
joy copied to clipboard

Auth middleware

Open swlkr opened this issue 5 years ago • 6 comments

swlkr avatar Mar 23 '20 04:03 swlkr

Any thoughts on what shape this might take? The main reason for the issue I reported earlier (https://github.com/joy-framework/joy/issues/69) is because I was attempting to roll my own auth middleware for the time being.

I think the before/after middlewares can be used to accomplish what I am after. I want to be able to have a middleware that can be placed in front of specific routes that require auth. If a user is not logged in the after middleware will redirect the user to a login page. Ideally, after the user is done logging in they will be redirected back to their original destination route.

Apart from the issue I reported about the before/after middlewares, I'm struggling to come up with a middleware solution. I tried setting up a a regular middleware that looks something like this:

(defn require-login [handler]
  (fn [req]
    (def dest (req :uri))
    (def logged-in (get-in req [:session :account]))
    (if (and (not logged-in) (= (req :uri) "/reports"))
      (-> (redirect (string "/login?dest=" dest)))
      (handler req))))

And is consumed like this:

(def app (-> (app {:layout layout})
             (require-auth)))

The trouble I am running into is that although the require-login middleware above is capable of redirecting a user, it does not appear to be called at the right point in the middleware stack. As a result, it cannot see :session because :session has not been decoded and added to the request yet.

I'm not sure how to go about creating a middleware that is called after the session middleware is called. Any thoughts?

jcreager avatar Aug 03 '20 21:08 jcreager

As far as the middleware stack goes, that was why I made before/after (of course they aren't working currently, but that's another thing).

The only option right now if you want to insert something into a certain point in the middleware stack is to copy it and put it in your app:

(-> (handler routes)
    (layout)
    (with-before-middleware)
    (with-after-middleware)
    (csrf-token)
    (session)
    (extra-methods)
    (query-string)
    (body-parser)
    (json-body-parser)
    (server-error)
    (x-headers)
    (static-files)
    (not-found)
    (logger))

I feel like it's pretty common though with more advanced apps, so I should do something like joy eject which will output that middleware stack or attempt to update main.janet maybe.

In the mean time yeah it's just adding the stack to your app

swlkr avatar Aug 07 '20 03:08 swlkr

I’ve been thinking more about the auth middleware and the before/after middleware(s)? as well.

The wildcard stuff is good, but I think it’s better to be more explicit, something like whole controller and individual routes too, maybe:

(before {:include [:things/thing1 :things/thing2 :todos] :exclude [:accounts]} :before-fn)

and then similarly for auth.

As for auth, I think it might be better to have an auth middleware but only for restricting which routes get accessed and setting the currently logged in account or team or whatever table you decide represents that.

The other part of the auth stuff will be generated, similar to route generation.

The ultimate goal is to have something like devise/omniauth where you can just set your providers, api keys and have at it.

swlkr avatar Aug 08 '20 17:08 swlkr

Now that you mention just setting up your own middleware stack if you need something more than the defaults I'm not sure why I didn't consider doing that myself. That seems reasonable to me.

Regarding include/exclude for the before/after middlewares, the only downside I see to that is cases where you really do want to cover a route with a wildcard. If I had a route that was /projects and it had sub routes like /projects/:id, etc, and I wanted to require login to view any of the sub routes, /projects/* is convenient in that case. On the other hand I like having an explicit list for applying the same function and I think this would work well in cases where you have different controllers for different routes that need the same middleware function. It gives you a lot of granularity in one place. I would advocate for being able to supply one or the other. Before/after middlewares can accept a string for route matching, or a map with the keys :include and exclude.

Another thought regarding middlewares is to allow middlewares to be injected into route declarations. In this case the last argument will always be the controller. Some examples below.

Instead of only accepting a controller.

(route :get "/" :foo)

Also allow an optional middlewares like.

(route :get "/" :middleware :controller)

Or maybe an optional list of middlewares.

(route :get "/" [:middlewares] :controller)

These types of middlewares would need to be set up like a regular middleware that acts on the request and response like the default middleware stack rather than ones that act on the request or the response like the before/after middlewares.

jcreager avatar Aug 08 '20 18:08 jcreager

The middleware in the route thing is interesting, I started down that path in coast and it’s coming up again here.

I like it and it could be even more useful if you had a macro that represented crud routes like this:

(resource :accounts [:mw1 :mw2])

which would do what you’re saying there:

(route :get "/accounts" [:mw1 :mw2] :accounts/index)
(route :get "/accounts/:id" [:mw1 :mw2] :accounts/show)
; rest of the 7 crud routes go here

It’s probably better to put the middleware in the routes than the routes in different middleware stacks which is what happens now.

swlkr avatar Aug 08 '20 23:08 swlkr

Also if the function signature were [] vs var args you could def different middleware stacks and put that in the route definitions

swlkr avatar Aug 08 '20 23:08 swlkr