souenzzo / eql-style-guide

36 stars 2 forks source link

Auth/Security in pathom #4

Open souenzzo opened 4 years ago

souenzzo commented 4 years ago

Related to #3

This thead is dedicated to anyone describe how handle security.

Some methods that I use:

1- Filter your query given a session, you can build a set of attributes that the user can reach

This method do not work alone!

(letfn [(cleanup-query [query allowed-key?]
(->> query
eql/query->ast
(eql/transduce-children (filter (comp allowed-key? :dispatch-key)))
eql/ast->query))]
(cleanup-query [:a
{:b [:c]}
{[:d 1] [:e]}
'(f {:g :h})
'{(i {:j :k}) [:l]}]
#{:a :b :c :d :e 'f 'i :l}))

2- Let it be. you your datasource (REST, DB, GQL) has some auth method, let it handle auth for you The world isn't perfect. you will probably need to compose with other methods

(let [user-db (d/filter db (session-filter env))]
(parser (assoc env ::db user-db)
query))

3- Env basead Just check if your env is authorized to call this handler Maybe it can be handled by pc/transform Maybe you can transform programmatically every resolve in your index

(pc/defresolver ... [env input]
{...}
(check-auth! env input)
...)
dvingo commented 4 years ago

I am still iterating on a solution, but I currently am enjoying combining pedestal interceptors with pathom-connect/transform. Here's the helper I wrote to enable this: https://github.com/dvingo/my-clj-utils/blob/master/src/main/dv/pathom.clj#L182

(:require    
   [dv.pathom :as pa] 
   [io.pedestal.interceptor.helpers :as ih])

(defn auth-user-interceptor
  "Takes a msg to set as the server error message if there is no user in the pathom environment."
  ([]
   (auth-user-interceptor "You must be logged in to perform this action."))
  ([msg]
   (fn auth-user-interceptor*
;;; the map "in" is the pedestal environment/context and contains:
;;  :opts - The definition-time pathom resolver or mutation map of options.
;;  :env - The pathom connect environment for the resolver or mutation passed by pathom at request-time.
;;  :params - The params to the resolver or the mutation.
     [{env :env :as in}]
     (let [{:keys [current-user]} env]
       (cond-> in (not current-user)
         (pa/assoc-response {:server/error? true :server/error-msg msg}))))))

(pc/defmutation create-thing-mutation
  [env props]
  {::pc/transform (pa/interceptors->pathom-transform 
      [(ih/before (user/auth-user-interceptor "You must be logged in to create a thing."))])}
  ;; usual body of mutate here    
)

the current user is assoced into the pathom environment from a session, like in this example: https://github.com/dvingo/dv.fulcro-template/blob/master/resources/clj/new/dv.fulcro_template/src/main/app/server/pathom_parser.clj#L24

I like that this is flexible enough to support arbitrary logic; for example, for resolvers you could add an "after" interceptor that inspects the return value of the resolver and checks that only certain keys are allowed given some account information or user role information that are passed as constructor arguments to the interceptor. It also lets the "body" of the mutation/resolver be a straight line of logic because validation and auth are taken care of in the interceptors.

lgessler commented 4 years ago

Here's my unsophisticated approach, inspired by some code from @dvingo. The basic strategy is to keep some permissions information available in env (probably a session) and use a transform on resolvers that'll error out and not execute the resolver if the user has insufficient permissions.

First some helper functions:

(defn server-error [msg]
  {:server/message msg
   :server/error?  true})

;; assume just two simple security levels: :admin and :user. If you want something more
;; fine-grained, this is the function to change
(defn authorized
  "Given a resolver's environment, say whether it is authorized for a given level"
  [env level]
  (let [{:keys [session/valid? user/admin?]} (get-in env [:ring/request :session])]
    (or (nil? level)
        (and (= level :admin) admin?)
        (and (= level :user) valid?))))

The transform maker, which is used to make a transform for each security level:

(defn make-auth-transform [level]
  "Make a transform for a Pathom resolver that checks whether the user has sufficient
  permissions for a given operation. mutate? is a bool that indicates whether this is
  for a resolver or a mutation, and level is one of :admin or :user indicating required
  permissions."
  (fn auth-transform [{::pc/keys [mutate resolve] :as outer-env}]
    (let [pathom-action (if mutate mutate resolve)
          pathom-action-kwd (if mutate ::pc/mutate ::pc/resolve)]
      (-> outer-env
          (assoc
            pathom-action-kwd
            (fn [env params]
              (if (authorized env level)
                          (pathom-action env params)
                          (server-error (str "Unauthorized pathom action: session "
                                             (get-in env [:ring/request :session])
                                             " does not satisfy authorization requirement "
                                             level)))))))))

(def admin-required (make-auth-transform :admin))
(def user-required (make-auth-transform :user))

Now all you need to do is use admin-required or user-required on a resolver, and you're good to go:

(pc/defresolver my-resolver [_ _]
  {::pc/output    [...]
   ::pc/transform admin-required}
  ...)

If the user has insufficient permissions, my-resolver will (1) prevent execution of the resolver (meaning no side effects will occur) and (2) return nothing. (It would be better for it to actually return the error message I make with server-error, but I'm still trying to figure out why it isn't making it out of the parser--I think Pathom is trimming it because it thinks it's superfluous output. I tried modifying ::pc/output to accommodate the server keys but that didn't seem to work. Oh well, the important bits of this work, and hackers don't need to know why they're failing ;))

If you wanted to make permissions more sophisticated, you could straightforwardly just modify the authorized function so that any additional information about the user is pulled in, and combine that with rules, etc.

posobin commented 3 years ago

Here is an approach that I think will work: make all resolvers that need a valid auth take an additional input (say :app/authed), and make a global resolver that outputs {:app/authed true} or {} based on the session data stored in env (like authorized above). Of course, you would need to filter your input query for :app/authed then, but it seems to solve the problem? Can also make several resolvers: :app/authed-admin, :app/authed-user, and then you get different access levels immediately. Not sure how to make the response say that the request failed due to authorization issues though.

souenzzo commented 3 years ago

I'm implementing this:

(pc/defresolver authed? [env input]
  {::private   #{:app.user/authed?}
   ::pc/output [:app.user/authed?]}
  {:app.user/authed? ...})

(pc/defresolver password [env input]
  {::pc/input #{:app.user/email
                :app.user/authed?}
   ::pc/output [:app.user/password]}
  {:app.user/password ...})
(defn private-keys
  "Returns a list of keys that should not be used as input on queries"
  [env]
 (set (mapcat ::private (vals  (::pc/index-resolvers (::pc/indexes env))))))

(defn remove-input-idents
  [env query idents]
  (let [idents (set idents)]
    (->> query
         eql/query->ast
         (eql/transduce-children (fn [{:keys [dispatch-key params]
                                       :as   node}]
                                   (cond (contains? idents dispatch-key)
                                         (throw (ex-info (str "You can't use '" dispatch-key "' as input")
                                                         {}))
                                         ;; I use placeholders to provide input, as in pathom3
                                         (p/placeholder-key? env dispatch-key) (assoc node :params (apply dissoc params idents))
                                         (contains? params :pathom/context) (assoc node :params (assoc params (apply dissoc (:pathom/context params) idents)))
                                         :else node)))
         eql/ast->query)))