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

Automatic ID resolver generators too tightly coupled to RAD Authorization #12

Closed
cjsauer opened this issue Apr 3, 2021 · 3 comments
Closed

Comments

@cjsauer
Copy link

cjsauer commented Apr 3, 2021

I'm working on RAD authorization for my project, and I've noticed that the built-in RAD Auth is depended upon by the Datomic/Cloud automatic ID resolver generators, namely here and here.

This is a bit unfortunate, because it "blesses" RAD's authorization implementation with no means to work around it.

I'm wondering if this is still necessary following the work done by @tylernisonoff in #10, which adds generic transform capability. It seems likely that RAD's redact routine could be moved out into a transform, and could then be opted into/out of at will by library users.

I'm willing to make this change, but I want to make sure I'm not missing something obvious.

Thanks for taking a look.

@cjsauer cjsauer changed the title ID resolvers too tightly couple to RAD Authorization ID resolvers too tightly coupled to RAD Authorization Apr 3, 2021
@cjsauer cjsauer changed the title ID resolvers too tightly coupled to RAD Authorization Automatic ID resolver generators too tightly coupled to RAD Authorization Apr 3, 2021
@awkay
Copy link
Member

awkay commented Apr 3, 2021

So, there is a do/wrap-resolve (it was there but undocumented) that can be used to transform the autogenerated resolvers.

So, something like:

(def all-attributes (mapv 
                                  wrap-my-special-transform
                                  list-of-attributes))

is possible.

Is that sufficient, or should we consider something else?

@awkay
Copy link
Member

awkay commented Apr 3, 2021

And yes, RAD's auth is definitely not meant to be there still (in db adapter)...should be deprecated at best.

@cjsauer
Copy link
Author

cjsauer commented Apr 3, 2021

I'm currently working on generalizing RAD's authorization, and am building on a lot of what exists already (only minor changes so far). What's there is a great start in my opinion.

So, something like...is possible

Yeah that's what I'm currently doing in my authorization work. You can put it right into the parser constructor next to the other pathom plugins.

(defstate parser
  :start
  (pathom/new-parser config
    [(attr/pathom-plugin all-attributes)
     (form/pathom-plugin save/middleware delete/middleware)
     (datomic/pathom-plugin (fn [_env] {:production (:main datomic-connections)}))
     (blob/pathom-plugin bs/temporary-blob-store {:files         bs/file-blob-store
                                                  :avatar-images bs/image-blob-store})]
    auth/pathom-plugin
    [
     automatic-resolvers
     form/resolvers
     (blob/resolvers all-attributes)
     index-explorer
     ,,,
     (auth/generate-authorization-resolvers all-attributes)]))

RAD's auth is definitely not meant to be there still

I'll open a PR with it yanked.

@cjsauer cjsauer closed this as completed Apr 3, 2021
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

2 participants