Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auth/Security in pathom #4

Open
souenzzo opened this issue Jul 5, 2020 · 4 comments
Open

Auth/Security in pathom #4

souenzzo opened this issue Jul 5, 2020 · 4 comments

Comments

@souenzzo
Copy link
Owner

souenzzo commented Jul 5, 2020

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
Copy link

dvingo commented Jul 9, 2020

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
Copy link

lgessler commented Jul 27, 2020

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
Copy link

posobin commented Dec 31, 2020

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
Copy link
Owner Author

I'm implementing this:

  • Every resolver can declare with keys it consider as a "private key"
  • before call the parser, I cleanup the query, removing any attempt to use this private key as input
  • we will allow that key to be returned. But we may change this in the future, with a "elide-private-keys" plugins
(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)))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants