Fork me on GitHub
#reitit
<
2020-04-20
>
ikitommi14:04:35

Question: should the (auto-)generated :options endpoint be validated against route-data specs?

ikitommi14:04:31

This fails:

(ring/router
  ["/api" {:get {:handler identity
                 :roles #{:admin}}}]
  {:validate rrs/validate
   :exception pretty/exception
   :data {:middleware [{:spec (s/keys :req-un [::roles])
                        :wrap (fn [handler]
                                (fn [request]
                                  (handler request)))}]}})
… as the :roles is only defined for :get, but not for the auto-generated (empty) :options endpoint. Feature or a bug?

ikitommi14:04:06

there lurks the :options endpoint, not conforming to the route-data spec.

bartuka14:04:29

I would say that would be better to let users define the validations to :options explicitly if they want to. seems like some "magic in the background" is happening, would not be clear to me why is that so. (the argument against this default would be increase in security? ... )

ikitommi15:04:53

the validation effects all methods currently. I one defines “I want all routes to have :role defined”, endpoints like {:get {:handler …, :roles #{:admin}} fail, as the :role is not defined for the generated :options. Less magic is better, but I feel generating the :options is already bit magical. Forcing it to have all the required route data is a surprise, but not requiring those might be a security bug: one can ask OPTIONS for a route that is otherwise forbidden. :thinking_face:

ikitommi15:04:50

I’d better ask @malcolmsparks, as he’s the expert on these.

👍 4
jaime18:04:31

Hello! When doing coercion, isn't it expected that string can be transformed into int automatically? So I have this route (notice that every :parameters are int?

(ns limeray.api.handler
  (:require [mount.core :refer [defstate]]
            [clojure.spec.alpha :as s]
            [spec-tools.core :as st]
            [reitit.ring :as ring]
            [reitit.ring :as ring]
            [reitit.dev.pretty :as pretty]
            [muuntaja.core :as m]
            [reitit.coercion :as coercion]
            [reitit.ring.coercion :as rrc]
            [reitit.ring.middleware.parameters :as parameters]
            [reitit.ring.middleware.muuntaja :as muuntaja]
            [reitit.ring.middleware.exception :as exception]
            [reitit.swagger :as swagger]
            [reitit.swagger-ui :as swagger-ui]
            [reitit.coercion.spec :as rcs]
            [ring.util.response :as resp]
            [ring.adapter.jetty :as jetty]))

(def router-options {:exception pretty/exception
                     :data {:coercion rcs/coercion
                            :muuntaja m/instance
                            :middleware [parameters/parameters-middleware
                                         muuntaja/format-negotiate-middleware
                                         muuntaja/format-response-middleware
                                         exception/exception-middleware
                                         muuntaja/format-request-middleware
                                         rrc/coerce-exceptions-middleware
                                         rrc/coerce-response-middleware
                                         rrc/coerce-request-middleware]}})

(defn make-app []
  (ring/ring-handler
   (ring/router
    [["/swagger.json" {:get (swagger/create-swagger-handler)}]
     ["/api-docs/*" {:get (swagger-ui/create-swagger-ui-handler)}]
     ["/api"
      ["/plus/:z"
       {:post {:parameters {:query {:x int?}
                            :body {:y int?}
                            :path {:z int?}}
               :responses {200 {:body {:total int?}}}
               :handler (fn [{:keys [parameters]}]
                          (println "params " parameters)
                          (let [total (+ (-> parameters :query :x)
                                         (-> parameters :body :y)
                                         (-> parameters :path :z))]
                            {:status 200
                             :body {:total total}}))}}]]]
    router-options)))
But when I make a request with the the :body-params {:y "2"} (quoted as string), the request returns 400
(app {:request-method :post
      :uri "/api/plus/3"
      :query-params {"x" "1"}
      :body-params {:y "2"}})
I was expecting that :y "2" will be accepted and converted into int. Is it a common practice to be strict against string to int? Or is it the order of the middleware that I messed up?

bartuka18:04:15

I think your premise it not correct. The contract defined in your handler is to validate against all input such as (s/valid? int? <input>) has to be true. and if you try (s/valid? int? "2") ;;=> false

jaime06:04:57

I think I need to make custom transformer as mentioned by ikitommi. Thanks!