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

Proposal: add new special has_user_access? for QueryAuthorization #37

Open
jotaviobiondo opened this issue Nov 10, 2021 · 2 comments
Open

Comments

@jotaviobiondo
Copy link
Member

Today, the has_user_access?/3 receives the user, some struct and the rule. This is nice for the Rajska.ObjectScopeAuthorization where the struct received is "complete" with all data, since it's coming from the source object that is being authorized.

But, for the Rajska.QueryAuthorization it's a bit strange, since we receive an "incomplete" struct, created using the scope and args option. This can sometimes be confusing, since we need to know if this struct is coming from the query authorization or the object authorization.

This can even limit (or force creation of workarounds), when the arguments received in the query or mutation is not directly related to the scope module. Consider this example:

Post and a Comment are Ecto structs, with Post having a has_many :comments, Comment.

object :post do
  field :id, :integer
  field :name, :string
  field :comments, list_of(:comments)
end

object :comment do
  field :id, :integer
  field :text, :string
  field :post, :post
end

field :list_posts_by_comments, list_of(:post) do
  arg :comment_ids, list_of(:integer)

  middleware Rajska.QueryAuthorization, [
    permit: :user,
    scope: Post,
    args: %{?????????: :comment_ids},
  ]
  resolve &BlogResolver.list_posts/2
end

In this case, the Post struct does not contains a comment_ids field, so we have to use the most related field form post, like this: args: %{comments: :comment_ids} or create a virtual field comment_ids in Post.

It's doable, but seems like a workaround.

My suggestions:

  1. have another function (besides has_user_access?) just for the QueryAuthorization, with a signature more or less like query_authorized?(%User{}, Post = _just_the_module, %{comment_ids: comment_ids} = args, rule)
  2. use the same has_user_access?, but add a fourth argument, acting like a metadata and pass the empty struct: has_user_access?(%User{}, %Post{} = _no_field_filled, rule, %{comment_ids: comment_ids} = metadata)

I don't think these 2 suggestions are the best API, but couldn't think a better one right now. If you guys agree with the proposal, we should probably iterate to find a more suitable API for the QueryAuthorization.

@gabrielpra1
Copy link
Member

I agree that it's annoying, but I also quite like having the same API for Query and Object authorization. However, now it's a "same-but-not-really" API, so I would also be in favor of changing it, but aiming for something that will work for both middlewares. Personally, the first option seems more intuitive since it matches the middleware declaration more closely.

@jotaviobiondo
Copy link
Member Author

Good point! Having the same API for both is better.

If we go with the first approach, what should the third argument (the parameters arg) have as data?
I was thinking something like:

%{
  authorization_type: :query | :object,
  query_or_mutation: :list_users,
  object: %Post{}, 
  query_args: %{comment_ids: ...}, 
}
  • authorization_type: useful if the user needs to know from where the function is being called
  • query_or_mutation: the query/mutation name -> same from above, if user needs information from where it's coming from
  • object: the object when the function is being called for object authorization,
  • query_args: the args that come from the query when doing query authorization. We can keep the API we have today that selects which args the struct is build with, so we standardize parameters. For example, the query arg could come as id in some queries/mutation and post_id in others, so we can have something like args: %{id: :post_id} to "transform" arguments

Is something like this you have in mind?

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