Fork me on GitHub
#pedestal
<
2020-11-03
>
simongray15:11:39

In case the user is trying to access some path that they’re not allowed into, I want my service to short-circuit and return HTTP status 403 immediately. What is the right way to do this? Should I throw an exception in one interceptor and then handle it in another?

simongray15:11:48

Also, based on http://pedestal.io/guides/hello-world-content-types > One important note here: As soon as any interceptor attaches a response to the context map, Pedestal considers the request handled. Remaining interceptors won’t be called, and only the ones that are already on the stack will have their `:leave` functions called. This is a “short-circuit” behavior like an early-exit in code. I would expect an interceptor chain consisting of two interceptors where the first one attaches a 403 response to short-circuit the chain, but it doesn’t seem to do so? This is the code for the interceptor:

(ic/interceptor
    {:name  ::session-guard
     :enter (fn [ctx]
              (assoc ctx :response {:status 403}))})
It is the first interceptor in a vector containing two of them. If it is the only interceptor in the chain I get my 403 response, but otherwise it just continues on the to the next interceptor. Kind of unexpected?

simongray15:11:06

This is the relevant route:

#{["/guarded" :get [(ic/session-guard conf) (guarded-page)] :route-name ::guarded-page]}
(ic/session-guard conf) creates the interceptor that attaches a 403 response, while (guarded-page) creates a ring handler that makes a regular 200 response. When both are present I always get the 200 and if only the first interceptor is there I get the 403. No short-circuiting in sight?

simongray16:11:45

So I followed the Pedestal source code all the way to

(defn- terminator-inject
  [context]
  (interceptor.chain/terminate-when context #(ring-response/response? (:response %))))
Which checks whether the response is a valid Ring response:
(defn response?
  "True if the supplied value is a valid response map."
  {:added "1.1"}
  [resp]
  (and (map? resp)
       (integer? (:status resp))
       (map? (:headers resp))))
And then I noticed that :headers must be set, so I went and modified my interceptor to also return a headers map:
(defn session-guard
  [{:keys [users]}]
  (ic/interceptor
    {:name  ::session-guard
     :enter (fn [ctx]
              (assoc ctx :response {:status 403
                                    :headers {}}))}))
and now it short-circuits as expected 🙂 I would still like to know if it makes more sense to do this as part of error-handling though. Any thoughts? Also, sorry for a wall of text.

souenzzo16:11:50

there is a io.pedestal.http/response?, that is used by the standard not-found-interceptor 😉

simongray21:11:33

seems kind of sloppy that two only slightly different predicate functions are in use for termination and not-found… wonder why?

Louis Kottmann18:11:04

actually I discovered that if you just assoc a response to the context in an interceptor, it stops the chain right there and sends your response

Louis Kottmann18:11:11

(assoc context :response (u/forbidden msg))

Louis Kottmann18:11:33

but I wonder if that's as valid as I tested

Louis Kottmann18:11:51

yeah I did see terminate, but why not just set the response if it's enough?

chrisulloa18:11:45

Yeah the :response value should trigger the short-circuiting. I wasn’t aware of the header requirement, but in our services we always attach content-type headers.

👌 3
chrisulloa18:11:51

To me throwing an error in your interceptor, then catching it in another error interceptor and returning a 403 seems like a good way to do it. Especially if you are in a place where it’s easier to throw an exception than return a response. But would be interesting to know how other people handle that.

👍 3
chrisulloa18:11:15

We usually check permissions at the very beginning of the interceptor stack.

chrisulloa18:11:20

So don’t find myself needing to do that.

Louis Kottmann18:11:49

for the header, I use the :leave interceptor io.pedestal.http/json-body and never had an issue (I never set them manually)

chrisulloa18:11:08

json-body docs

Set the Content-Type header to "application/json" and convert the body to
JSON if the body is a collection and a type has not been set.