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.

Sign up for GitHub

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

Absinthe v1.5, Relay support #27

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Absinthe v1.5, Relay support #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c60f88c
Only compile RateLimiter when Hammer is available
jeroenvisser101 Jul 15, 2020
760d3b2
Allow Absinthe 1.5.0
jeroenvisser101 Jul 15, 2020
3320148
Project fields to support named fragment spreads
jeroenvisser101 Jul 15, 2020
2e59556
Add updated Introspection exception
jeroenvisser101 Jul 15, 2020
18c91b6
Don't require QueryAuthorization to be first
jeroenvisser101 Jul 15, 2020
22edb56
Add optional default_authorize callback
jeroenvisser101 Jul 15, 2020
f0de230
Allow custom errors in unauthorized_message
jeroenvisser101 Jul 15, 2020
a5d8a06
Add plugin implementation
jeroenvisser101 Jul 16, 2020
9c9ac9c
Add 1.6 compatibility
jeroenvisser101 Feb 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions lib/authorization.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,15 @@ defmodule Rajska.Authorization do

@callback has_user_access?(current_user, scoped_struct, rule) :: boolean()

@callback unauthorized_message(resolution :: Resolution.t()) :: String.t()
@callback unauthorized_message(resolution :: Resolution.t()) ::
Absinthe.Type.Field.error_value()

@callback context_role_authorized?(context, allowed_role :: role) :: boolean()

@callback context_user_authorized?(context, scoped_struct, rule) :: boolean()

@callback default_authorize(context, scoped_struct) :: role() | nil

@optional_callbacks get_current_user: 1,
get_ip: 1,
get_user_role: 1,
Expand All @@ -38,5 +41,6 @@ defmodule Rajska.Authorization do
has_user_access?: 3,
unauthorized_message: 1,
context_role_authorized?: 2,
context_user_authorized?: 3
context_user_authorized?: 3,
default_authorize: 2
end
17 changes: 16 additions & 1 deletion lib/middlewares/object_authorization.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ defmodule Rajska.ObjectAuthorization do
def call(%Resolution{state: :resolved} = resolution, _config), do: resolution

def call(%Resolution{definition: definition} = resolution, _config) do
authorize(definition.schema_node.type, definition.selections, resolution)
fields = Resolution.project(resolution)
authorize(definition.schema_node.type, fields, resolution)
end

defp authorize(type, fields, resolution) do
Expand All @@ -87,10 +88,17 @@ defmodule Rajska.ObjectAuthorization do
defp authorize_object(object, fields, resolution) do
object
|> Type.meta(:authorize)
|> default_authorize(resolution.context, object)
|> authorized?(resolution.context, object)
|> put_result(fields, resolution, object)
end

defp default_authorize(nil, context, object) do
Rajska.apply_auth_mod(context, :default_authorize, [context, object])
end

defp default_authorize(authorize, _context, _object), do: authorize

defp authorized?(nil, _, object), do: raise "No meta authorize defined for object #{inspect object.identifier}"

defp authorized?(permission, context, _object) do
Expand All @@ -114,6 +122,13 @@ defmodule Rajska.ObjectAuthorization do
authorize(schema_node, selections ++ tail, resolution)
end

defp find_associations(
[%Absinthe.Blueprint.Document.Fragment.Spread{} | tail],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just did a PR for this and only now realized you already did something similar. But I think this implementation is not the best because we simply ignore the fragments, which could contain further associations.

What do you think about the implementation in #29?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey 👋 , good catch, however, I think I added line 66, which should project the fields down. If you look at the docs for Absinthe.Resolution.project/2, they mention it's non-trivial. The fragments might not be simple, and might not always apply to this type (for interfaces for instance).

I wonder what would happen if I added the same tests you did, if this would still be passing 🤔

rajska/lib/middlewares/object_authorization.ex

Line 66 in a5d8a06

fields = Resolution.project(resolution)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't ☹️

I believe the problem is "one layer down":

Project one layer down from where we are right now.

In the tests we have a fragment within a fragment and so the inner fragment is ignored by the authorization and the query returns successfully when it should return an error.

So I think the best option here is for me to merge #29 and then you can rebase with it. Do you agree?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gabrielpra1 noticed it yesterday yeah, have been trying to get it to work but haven't been succesful yet. The thing is, not all fragments are named:

... on Type {
  field
}

I don't think this one would work, would it?

I still think projection should be the solution here, and as long as we project at the correct level, it should error appropriately, even in nested fragments.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gabrielpra1 the thing for us here, is that we'd really like to be able to use the same query for users with different permissions, and those permissions would only check actual access. For instance, in relay, you might see queries like this:

query NodeQuery($id: ID!) {
  node(id: $id) {
    ...Product
    ...Order
    ...Customer
  }
}

As long as the user only receives what they have access to (e.g. only passes global IDs of types that are allowed), this is fine. The thing is, node is defined as an interface, and only after resolution can we be sure that the user isn't accessing anything it doesn't have access to. I naively assumed that the middleware was being created for each object returned, but it only runs once, before resolution, meaning the details required to determine what the node interface resolves to, are not available yet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this one would work, would it?

It should work, we already support inline fragments. Here is the query being used in the test case:

    {
      unionQuery {
        ... on User {
          name
        }
        ... on WalletBalance {
          total
        }
      }
    }

Absinthe's projection only takes the entire resolution as argument, so I don't think we could use it recursively. Seeing the source code, though, I think we are already covering all cases, except the skip directive, which I have never seen being used.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do use description @include(if: $includeProductDescription) which is similar to @skip.

I can try your PR in our project to see if any issues arise.

[https://hexdocs.pm/absinthe/Absinthe.Resolution.html#project/2](The example from the docs) seem to suggest it would be able to run at any level, I think because of the path in the resolution, but again, this would not make it work with interface types (it would work only post-resolution)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll merge it for now since it is covering our use cases, but we can continue that discussion in another PR if you still have troubles with it.

I'm not sure how to deal with interface types, it looks like we would need to rely only on the ObjectScopeAuthorization for this case, since it is the only one that runs after resolution

resolution
) do
find_associations(tail, resolution)
end

defp find_associations(
[%{schema_node: schema_node, selections: selections} | tail],
resolution
Expand Down
11 changes: 11 additions & 0 deletions lib/middlewares/object_scope_authorization.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,17 @@ defmodule Rajska.ObjectScopeAuthorization do
alias Absinthe.{Blueprint, Phase, Type}
alias Rajska.Introspection
use Absinthe.Phase
@behaviour Absinthe.Plugin

@spec run(Blueprint.t() | Phase.Error.t(), Keyword.t()) :: {:ok, map}
def run(%Blueprint{execution: execution} = bp, _options \\ []) do
{:ok, %{bp | execution: process(execution)}}
end

def pipeline(pipeline, _execution), do: pipeline
def before_resolution(execution), do: execution
def after_resolution(execution), do: process(execution)

defp process(%{validation_errors: [], result: result} = execution), do: %{execution | result: result(result, execution.context)}
defp process(execution), do: execution

Expand All @@ -76,6 +81,12 @@ defmodule Rajska.ObjectScopeAuthorization do
when identifier in [:query_type, nil] do
result
end
defp result(%{emitter: %{schema_node: %{definition: Absinthe.Phase.Schema.Introspection}}} = result, _context) do
result
end

# No fields because of non_null violation further down the tree
defp result(%{fields: nil} = result, _context), do: result

# Root
defp result(%{fields: fields, emitter: %{schema_node: %{identifier: identifier}}} = result, context)
Expand Down
102 changes: 52 additions & 50 deletions lib/middlewares/rate_limiter.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,71 +1,73 @@
defmodule Rajska.RateLimiter do
@moduledoc """
Rate limiter absinthe middleware. Uses [Hammer](https://github.com/ExHammer/hammer).
if Code.ensure_loaded?(Hammer) do
defmodule Rajska.RateLimiter do
@moduledoc """
Rate limiter absinthe middleware. Uses [Hammer](https://github.com/ExHammer/hammer).

## Usage
## Usage

First configure Hammer, following its documentation. For example:
First configure Hammer, following its documentation. For example:

config :hammer,
backend: {Hammer.Backend.ETS, [expiry_ms: 60_000 * 60 * 4,
cleanup_interval_ms: 60_000 * 10]}
config :hammer,
backend: {Hammer.Backend.ETS, [expiry_ms: 60_000 * 60 * 4,
cleanup_interval_ms: 60_000 * 10]}

Add your middleware to the query that should be limited:
Add your middleware to the query that should be limited:

field :default_config, :string do
middleware Rajska.RateLimiter
resolve fn _, _ -> {:ok, "ok"} end
end
field :default_config, :string do
middleware Rajska.RateLimiter
resolve fn _, _ -> {:ok, "ok"} end
end

You can also configure it and use multiple rules for limiting in one query:
You can also configure it and use multiple rules for limiting in one query:

field :login_user, :session do
arg :email, non_null(:string)
arg :password, non_null(:string)
field :login_user, :session do
arg :email, non_null(:string)
arg :password, non_null(:string)

middleware Rajska.RateLimiter, limit: 10 # Using the default identifier (user IP)
middleware Rajska.RateLimiter, keys: :email, limit: 5 # Using the value provided in the email arg
resolve &AccountsResolver.login_user/2
end
middleware Rajska.RateLimiter, limit: 10 # Using the default identifier (user IP)
middleware Rajska.RateLimiter, keys: :email, limit: 5 # Using the value provided in the email arg
resolve &AccountsResolver.login_user/2
end

The allowed configuration are:
The allowed configuration are:

* `scale_ms`: The timespan for the maximum number of actions. Defaults to 60_000.
* `limit`: The maximum number of actions in the specified timespan. Defaults to 10.
* `id`: An atom or string to be used as the bucket identifier. Note that this will always be the same, so by using this the limit will be global instead of by user.
* `keys`: An atom or a list of atoms to get a query argument as identifier. Use a list when the argument is nested.
* `error_msg`: The error message to be displayed when rate limit exceeds. Defaults to `"Too many requests"`.
* `scale_ms`: The timespan for the maximum number of actions. Defaults to 60_000.
* `limit`: The maximum number of actions in the specified timespan. Defaults to 10.
* `id`: An atom or string to be used as the bucket identifier. Note that this will always be the same, so by using this the limit will be global instead of by user.
* `keys`: An atom or a list of atoms to get a query argument as identifier. Use a list when the argument is nested.
* `error_msg`: The error message to be displayed when rate limit exceeds. Defaults to `"Too many requests"`.

Note that when neither `id` or `keys` is provided, the default is to use the user's IP. For that, the default behaviour is to use
`c:Rajska.Authorization.get_ip/1` to fetch the IP from the absinthe context. That means you need to manually insert the user's IP in the
absinthe context before using it as an identifier. See the [absinthe docs](https://hexdocs.pm/absinthe/context-and-authentication.html#content)
for more information.
"""
@behaviour Absinthe.Middleware
Note that when neither `id` or `keys` is provided, the default is to use the user's IP. For that, the default behaviour is to use
`c:Rajska.Authorization.get_ip/1` to fetch the IP from the absinthe context. That means you need to manually insert the user's IP in the
absinthe context before using it as an identifier. See the [absinthe docs](https://hexdocs.pm/absinthe/context-and-authentication.html#content)
for more information.
"""
@behaviour Absinthe.Middleware

alias Absinthe.Resolution
alias Absinthe.Resolution

def call(%Resolution{state: :resolved} = resolution, _config), do: resolution
def call(%Resolution{state: :resolved} = resolution, _config), do: resolution

def call(%Resolution{} = resolution, config) do
scale_ms = Keyword.get(config, :scale_ms, 60_000)
limit = Keyword.get(config, :limit, 10)
identifier = get_identifier(resolution, config[:keys], config[:id])
error_msg = Keyword.get(config, :error_msg, "Too many requests")
def call(%Resolution{} = resolution, config) do
scale_ms = Keyword.get(config, :scale_ms, 60_000)
limit = Keyword.get(config, :limit, 10)
identifier = get_identifier(resolution, config[:keys], config[:id])
error_msg = Keyword.get(config, :error_msg, "Too many requests")

case Hammer.check_rate("query:#{identifier}", scale_ms, limit) do
{:allow, _count} -> resolution
{:deny, _limit} -> Resolution.put_result(resolution, {:error, error_msg})
case Hammer.check_rate("query:#{identifier}", scale_ms, limit) do
{:allow, _count} -> resolution
{:deny, _limit} -> Resolution.put_result(resolution, {:error, error_msg})
end
end
end

defp get_identifier(%Resolution{context: context}, nil, nil),
do: Rajska.apply_auth_mod(context, :get_ip, [context])
defp get_identifier(%Resolution{context: context}, nil, nil),
do: Rajska.apply_auth_mod(context, :get_ip, [context])

defp get_identifier(%Resolution{arguments: arguments}, keys, nil),
do: get_in(arguments, List.wrap(keys)) || raise "Invalid configuration in Rate Limiter. Key not found in arguments."
defp get_identifier(%Resolution{arguments: arguments}, keys, nil),
do: get_in(arguments, List.wrap(keys)) || raise "Invalid configuration in Rate Limiter. Key not found in arguments."

defp get_identifier(%Resolution{}, nil, id), do: id
defp get_identifier(%Resolution{}, nil, id), do: id

defp get_identifier(%Resolution{}, _keys, _id), do: raise "Invalid configuration in Rate Limiter. If key is defined, then id must not be defined"
defp get_identifier(%Resolution{}, _keys, _id), do: raise "Invalid configuration in Rate Limiter. If key is defined, then id must not be defined"
end
end
5 changes: 4 additions & 1 deletion lib/rajska.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ defmodule Rajska do
defmacro __using__(opts \\ []) do
super_role = Keyword.get(opts, :super_role, :admin)
valid_roles = Keyword.get(opts, :valid_roles, [super_role])
default_rule = Keyword.get(opts, :default_rule, :default)
default_rule = Keyword.get(opts, :default_rule, :default)
default_authorize = Keyword.get(opts, :default_authorize, nil)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add this option to the docs on this file (line 40) and on readme?


quote do
@behaviour Authorization
Expand Down Expand Up @@ -130,6 +131,8 @@ defmodule Rajska do
|> get_current_user()
|> has_user_access?(scoped_struct, rule)
end

def default_authorize(_context, _object), do: unquote(default_authorize)
rschef marked this conversation as resolved.
Show resolved Hide resolved

defoverridable Authorization
end
Expand Down
Loading