jungsoft / rajska Goto Github PK

absinthe 1.5 changed schema pipeline and now it executes at compile time making use of rajska impossible

Sometimes is useful to have a way to reset the RateLimiter, e.g. rate limiting the login and be able to clear the rate limit when user resets his password.

For this we can add a function reset(identifier) to the Rajska.RateLimiter module that calls the https://hexdocs.pm/hammer/Hammer.html#delete_buckets/2.

Besides this low level function, we could also have a middleware for this.

Also, another suggestion is to improve the RateLimiter identifier config. Today we have 3 ways of specifying a identifier:

1. specifying an id option
2. specifying a keys option
3. not specifying anything and fallbacks to the IP

It would be nice, given the idea to have a middleware for limiting and reseting, do something like this:

field :login, :session do
  ...
  middleware Rajska.RateLimiter, namespace: :login, limit_by: :ip
  resolve &AccountsResolver.login/2
end

field :reset_password, :session do
  ...
  resolve &AccountsResolver.reset_password/2
  middleware Rajska.RateLimiter.Reset, namespace: :login, limit_by: :ip
end

With this, we are identifying by both an namespace and user IP. I see this useful for readability: we use a namespace to better uderstand that we are limiting in one mutation and reseting in another. Without this, it's still possible to implement, but it's harder to see why that reset is there:

field :login, :session do
  ...
  middleware Rajska.RateLimiter
  resolve &AccountsResolver.login/2
end

field :reset_password, :session do
  ...
  resolve &AccountsResolver.reset_password/2
  middleware Rajska.RateLimiter.Reset
end

The limit_by could be:

• :ip -> uses user IP
• {:keys, list} -> same way keys option today works
• {:id, any_fixed_id} -> same way id option works

The Hammer id built with this, would be something like "query:#{namespace}:#{limit_by}"

What do you guys think?

The Object Scope Authorization currently does not support nested associations, because when the middleware is executed, the fields are still Ecto.Association.NotLoaded

Closes #19

Current callback:

@callback has_user_access?(
    current_user,
    scope :: module(),
    {field :: any(), field_value :: any()},
    rule :: any()
  ) :: boolean()

• Merge scope module and {field, field_value} arguments into a struct, since scope module must define a struct
• Pass entire objects from field and query scope authorization to new has_user_access?/3
• Rename scope_by, scope_object_by and scope_field_by metas to scope?, scope_object? and scope_field?
• Set default scope? value to true

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.

Query authorization supports a custom message for unauthorized error:

  defp update_result(false, %{context: context} = resolution) do
    Resolution.put_result(resolution, {:error, Rajska.apply_auth_mod(context, :unauthorized_message, [resolution])})
  end

But object authorization does not:

  defp put_result(false, _fields, resolution, object) do
    Resolution.put_result(resolution, {:error, "Not authorized to access object #{object.identifier}"})
  end

We should follow the same pattern of having an overridable function in Rajska that should be called in the case of unauthorized in ObjectAuthorization

While working on this, I was thinking of a possibility of having a command that outputs all the objects/queries that a role has access to. This might not be completely possible, because some rules might be dependent on the object/scope/actor. Do you internally have a way to audit roles and authorization?

Add examples:

• Query scope authorization optional parameter
• Query scope authorization nested parameter
• :source scoping
• Query authorization without scope
• Query scope authorization with custom rule

General:

• Clarify which middleware calls which function
• Maybe add a full example

with (KeyError) key :type not found

in Rajska.ObjectAuthorization.find_associations/2 there is

authorize(schema_node.type, selections ++ tail, resolution)

but union type do not have :type key (as the type of this type is dynamic)

any workarounds, pointers?

• get_scope_by_field/2 should have bang: https://github.com/rschef/rajska/blob/master/lib/middlewares/field_authorization.ex#L44
• I think the usage doc for each middleware should include adding it to the schema
• Does scope: :source still makes sense in scope authorization? I added it in PM, but I'm not using it anymore and can't think a use case for it right now.
• Does a global config makes sense? It looks like all configuration can be made through the use Rajska call
• I think a behaviour should be implemented for the overridable functions, so the compiler can check them. See https://elixirforum.com/t/behaviours-defoverridable-and-implementations/3338 and https://elixir-lang.org/getting-started/typespecs-and-behaviours.html
• get_user_role and is_super_user? are overridable functions and take the resolution as an argument. My question here is if there is an use case for overriding this functionality. Can't we achieve the same level of flexibility by just overriding get_current_user and get_user_role(that would only take the user as argument)?