Fork me on GitHub
#fulcro
<
2020-07-01
>
Jakub Holý16:07:13

Hi! Is there a way to add ::pc/errors automatically to the query of each component? I guess :global-eql-transform is the way, but an example would be extremely helpful 🙂 I would then use the app's :render-middleware to wrap it and show an error message instead of the component if there actually have been any errors.

Jakub Holý17:07:19

For Fulcro, what is the best practice for putting pathom errors on the component level? Is it (p/post-process-parser-plugin p/raise-errors) on the server or should/can I use raise-errors on the client?

dvingo17:07:12

I'm interested in the approaches for this as well - right now I'm adding my server error keys to the component queries individually

Jakub Holý17:07:05

I am trying this global EQL transform

(defn eql-transform-components-query-pathom-error [ast]
  (let [errors-query-child (eql/expr->ast ::pc/errors)]
    (clojure.walk/postwalk
      (fn [node]
        ;; ?? And not (-> node :query first #{:com.fulcrologic.fulcro.routing.dynamic-routing/id}) ?
        (if ((every-pred map? :component :children) node)
          (update node :children conj errors-query-child)
          node))
      ast)))
which seems OK though I still a struggling with some issues...

thosmos19:07:52

I just use a global tx-result and handler that displays success/fail across my app. However, I have some modal popups that obscure the root handler, so I just do something like this in my component’s query that needs access to the global tx-result:

{[:root/tx-result '_] (comp/get-query TxResult)}
And then in the render section:
(ui-modal-content {}
            (when (not (empty? tx-result))
              (ui-tx-result tx-result))
Obviously that’s only useful in special cases. It sounds like you might want more specific error handling across each comp

Jakub Holý19:07:29

Thanks! Yes, I want error handling per component. But this is good to know as well 🙂

dvingo17:07:50

so you're trying to add the properties on the client side so every component will query for them?

Jakub Holý17:07:29

Yes. And then the render-middleware checks for it and displays the error instead of the component when any.

zilti18:07:44

Also, implementing an auth system as pathom middleware is hard. Wow.

JAtkins18:07:36

Maybe we are in the same position - I actually just finished my middleware system for auth. Which part are you talking about?

dvingo18:07:04

I am working on one using the pc/transform hook, liking it so far

JAtkins18:07:11

(assuming that our apps even are the same style and need the same security, which they may not)

JAtkins18:07:37

Ah, you took a bit different path. Is that for managing df/loads?

dvingo18:07:14

(pc/defmutation create-task-mutation
  [_ {:goal/keys [id] :as props}]
  {::pc/sym       `create-task
   ::pc/transform (user/auth-user "You must be logged in to create a task.")} 
  
  ;; normal body here
  )

dvingo18:07:37

but you could pass anything to the auth helper - roles the user must have etc

JAtkins18:07:28

Gacha. That was the easier part [for my use case]. The hard part is making sure that your pathom endpoint is safe, since anyone can pass any eql.

JAtkins18:07:46

I still have some changes to make there.

dvingo18:07:26

ahh interesting

dvingo18:07:46

what are the scenarios where you need to sanitize the query? in my mind csrf token + being logged in is a good start

JAtkins18:07:06

Yep, but I'm just assuming that some person can figure out how to make requests from the dev tools of their browser. I'm just using uuids for entities, so I need to make sure that they are either (a: private (like the uuid of your address entity, or b: protected (user uuid)). I'm using jwt so I have to assume that the user uuid is public information at some level. I then have to check the eql to make sure there aren't any random joins to account.

tony.kay18:07:01

Might I suggest a policy-style approach: one way I’ve done it that works well is to put a lambda in the resolver’s map (all resolvers are just maps), then we wrote our own defresolver/defmutation macro that looks for that entry. The macro can surround the body with the security check (which can use env as input, and you can put request and other stuff in there). We then add links from most entities to their “owning firm/account”, which makes the whole thing much easier to check.

tony.kay18:07:54

that way it doesn’t matter how they traverse the graph to get to something. Some “component” children don’t need anything extra, since they are not accessible without going through their parent anyway.

JAtkins18:07:33

That is where I am going next actually. I'm not terribly happy with the current approach. It requires me to dynamically compile datomic queries to find the owner of entities during edits.

tony.kay18:07:33

but the policy lambda can do any amount of logic to do the check at a resolver level. And save is easy enough to augment to auto-add the link to the “current account/firm” on new entities.

JAtkins18:07:53

yup, I'll end up doing that pretty soon. I like that idea much better than having to add different graph traversals.

dvingo19:07:21

I believe that's the same approach as the transform - it will not call the resolver/mutation body based on the logic of the helper - is there a user - their role etc? is that what you mean by policy?

tony.kay20:07:02

yeah…a function that implements the logic…does the user “own” the entity being queried? Do they have rights to see it, etc.

zilti20:07:58

> one way I’ve done it that works well is to put a lambda in the resolver’s map (all resolvers are just maps), then we wrote our own defresolver/defmutation macro that looks for that entry. So, not a middleware?

zilti20:07:27

What I am working on right now is a pathom parser middleware. I added keys to the defattrs with a map that says what kind of access rights there are. Then in wrap-read, wrap-resolve and wrap-mutate, I have a function that checks if all the requested attributes actually are in permission scope for the currently active ring session.

zilti20:07:44

I do this by getting all requested attributes from the AST, plus the Pathom entity (::p/entity env) because I have to get certain attributes from the latter if existing.

zilti20:07:06

I feel like I am creating an uncontrollable monster...

tony.kay20:07:08

the middleware in mine just sets up the env to include things like session info, a db, etc. You could of course do middlware that tries to do security, but that seems harder to keep in sync with resolvers because not co-located

zilti20:07:06

I can't really rely on resolvers due to using Fulcro RAD, can I? Since Fulcro RAD can load things without using any of my self-written resolvers?

tony.kay20:07:59

Rembmer that RAD is generating those for convenience. Look at the auto-gen code….trivial.

tony.kay20:07:12

don’t convolute convenience with a requirement to use

zilti20:07:02

I also think it is nice when I have security on an attribute-level though, since that way, I can write resolvers and all without having to think about neatly integrating it there

tony.kay20:07:02

so, you could easily ape that code and mix in your own security system. don’t stick with the auto-gen’d stuff and then do backflips to “back into” it…that’d be a waste of time

tony.kay20:07:43

sure, attributes are the perfect place to hang security things….and if you see a middleware approach that is reasonable, sure, go for it

zilti20:07:15

Yea. I am just not sure if I catch everything. Also, I guess this way I cannot go more granular than dividing rights into no access, read access and write access.

tony.kay20:07:44

the main thing to be careful of is over-resolving (optimization) in a given resolver…since that could cause other resolvers to not be called. General graph security, as you said, can get hard.

tony.kay20:07:10

My experience, though, is that doing a combo of adding ownership into to the entities in the db and rules on the resolvers works quite well. However, when you get down to finer-grained access (who can read this field?) then you need more code….a given resolver coud return a lot of things.

tony.kay20:07:35

so at that point you kind of need to keep contextual info at each entity and then post-analyze the response before sending it.

zilti20:07:49

I haven't fully understood these yet anyway I think. Lots of magic going on with Pathom, and when it is so neatly hidden behind lots of autogenerated resolvers from Fulcro RAD Datomic and all...

zilti20:07:39

Who knows, maybe my stubbornness pays off, and I end up releasing that thing as a library.

dvingo22:07:01

not sure about RAD - but pc/transform is a nice place to add arbitrary middleware to a resolver or mutation - so you can inspect the before state and post state. I have some code that allows the use of pedestal interceptors on any resolver/mutation: https://github.com/dvingo/my-clj-utils/blob/master/src/main/dv/pathom.clj#L194

👀 3
zilti22:07:37

Thanks, this could be handy for other situations 🙂 Well, one additional reason why I don't want to write all the resolvers myself when I have fulcro-rad-datomic is that this would mean I'd also have to write all the database queries as well. So much additional code.

JAtkins22:07:04

If you follow what tony said you can do one of 2 things 1. copy and modify the rad resolvers. I imagine the interface is relatively stable for that part of the api 2. (my plan) make a new field on everything to be resolved (:owning-account/id or something) and make sure that value is verified in pathom middleware.

JAtkins22:07:52

There may be another one, but I'm not able to think of it right now.

zilti22:07:58

Yes, I went with solution 2

Jakub Holý18:07:16

How is it possible that comp/get-computed inside a router body return nil when I pass them in? I use the correct factory:

(def ui-details-display-router (comp/computed-factory DetailsDisplayRouter))
and in the calling code:
(ui-details-display-router details-router {:organization-number organization-number})
so even if organization-number was nil, the computed props should be a map with a key, not nil?! What have I screwed up this time and how to troubleshoot it? Solved: Computed props are always nil if the normal props are nil.

6