gregorip02 / restql

📦 A data resolution package for your Laravel models.
https://github.com/gregorip02/restql/wiki
GNU General Public License v3.0
128 stars 6 forks source link

Guards and Authentication #4

Closed VenatusSimpleX closed 4 years ago

VenatusSimpleX commented 4 years ago

Hi, I am wondering if your package supports guards and authorization, because I understand that we can prevent unauthorized users from using the endpoint via middleware, though what if we have parts of it is public (unauthed), and parts of it is only for authed users only?

oddvalue commented 4 years ago

I feel like this feature would be essential for a large production app. Something similar to the way the Spatie query builder package (https://github.com/spatie/laravel-query-builder) allows you to define what columns can be selected, what relations can be included, etc... This package looks awesome, though. Definitely gonna give it a try later.

gregorip02 commented 4 years ago

Hi @VenatusSimpleX Your point of view makes a lot of sense, we could assume that various middlewares protect our endpoint from unauthorized users. However, some properties of our models may not be accessible to a certain group of users

We could implement something like what @oddvalue mentions and add an array of attributes that can be queried by RestQL.

I will consider this for further changes.

VenatusSimpleX commented 4 years ago

What about implementing the guards in the config?

Where the current config syntax is

'allowed_models' => [
    'users' => 'App\User',
    'posts' => 'App\Post',
    'comments' => 'App\Comment'
]

What if we modify it like

'allowed_models' => [
    'users' => [
        'model' => 'App\User',
        'middleware' => [
            // Typical ones for BREAD that I can think of
            // list, index, update, store, delete
            'update' => 'role:admin,self', // same syntax as in routes
            'store' => 'role:admin',
            'delete' => 'role:admin'
        ]
    ],
    // other models etc...
]

We can keep the flavor of this package where it places the schema in config and parses based on that if we modify like above.

And the above is assuming we can POST/PUT/DELETE requests as well (I've not tried, and I don't think docs mentioned anything other than GETing model information).

What do you think?

Also, I think this can be another issue (let me know and I'll open a new one), since there were no documentation on doing anything else other than GETing, does this package allow for doing the rest of BREAD aside from Reading?

gregorip02 commented 4 years ago

I think your idea is great and I had something similar in mind, but this means releasing a larger version of the package due to the fact that the old configuration structure would no longer be compatible.

RestQL was initially designed to get data easily, I plan to make a solid and scalable code base to later include operations such as creating, modifying and deleting data. This is indeed a new issue.

VenatusSimpleX commented 4 years ago

What if we have two types of "modes" for parsing the configuration? We can keep the current format as is and have it as an almost out of the box API generator for getting data easily from the models and the relationships?

The "advanced" or "deeper" configuration would be something like the proposed config- though we can probably have further discussion on how to make this more scalable so even if there are many models to be configured the settings won't be a mess or an eye sore to handle.

oddvalue commented 4 years ago

I like the idea of adding middleware to the allowed model config. That could easily be made backwards compatible with a simple if.

e.g.

if (is_array($classOrConfig)) {
    // new style config set up
} else {
    // old style config setup
}
gregorip02 commented 4 years ago

I will consider it strongly, until today I have been working on some repairs with the implicit values of the clauses for v[1.3]. And I want to write a set of unit tests, which as you can see has not yet.

Remember this is open source, if you have a good idea you have a pull request and I would be happy to review and add it.

zippoxer commented 4 years ago

Maybe we combine Nova's Resource and Laravel's Resource? Take a look:

// An ambassador for a Post living in RestQL.
class Post extends Restable {
    public $model = \App\Post::class;

    // Restrict or augment SELECT queries.
    public function select(Request $request, Builder $query) {
        if($request->user()->isAdmin()) {
            // Can see anything.
            return;
        }

        // Can see anything he posted and public posts by others.
        return $query->where('status', 'public')
                     ->orWhere('user_id', $request->user()->id);
    }

    // Authorize an update to the Restable.
    public function canUpdate(Request $request) {
        return $this->author_id === $request->user()->id;
    }

    public function canDelete(...) { ... }

    // Transform Restable for the JSON response.
    public function toArray(Request $request)
    {
        return [
            'id' => $this->id,
            'title' => $this->title,
            'content' => $this->content,
            'published_at' => $this->published_at,

            // A hidden column, only visible for admins.
            'created_at' => $this->mergeWhen($request->user()->isAdmin(), $this->created_at),
        ];
    }
}

Obviously not complete and still has security holes, but what do you think about this approach?

gregorip02 commented 4 years ago

Hi @zippoxer,

I think this implementation would overload the eloquent models. The idea is to make the package "ready to go" just by running composer require.

I will start working on the next minor version adding verb based authorization functionality. Then, I'll try to create a configuration scheme that doesn't break things.

The package tries to adapt as best as possible to Laravel and its architectural concepts. I will try to adapt concepts like FormRequest and fillable attributes to create / update / delete.

zippoxer commented 4 years ago

@gregorip02 Sure, makes sense.

However, do you have an alternative to handle security & access control? Cases like when an app needs to limit query-able columns depending on the role of the authenticated user? Or restrict/augment the query like in my example?

I realize I'm talking Firebase-level stuff, but I'm just talking :-) (not expecting to have that in RestQL)

VenatusSimpleX commented 4 years ago

Hey @zippoxer ,

I think the proposed method should cover the cases you had mentioned- in the form of placing middlewares that grants appropriate permissions to the users.

I also feel that managing roles and permissions itself are outside of this package's scope. We've got the wonderful spatie/laravel-permission to manage this already, in fact, if we implement middleware integration with the models in the config itself, it'll be one of the more perfect companions to the aforementioned permissions package while converting our CRUD to GQL-styled queries

gregorip02 commented 4 years ago

Hi guys, I am making significant changes and I would like you to take a look at it. As imagined the new configuration will break version 1.x, but will make the package more extensible.

See #6

Regarding this issue, the structure of the new configuration will allow the creation of new keys within the schema definition as the following.

<?php

return [
  'schema' => [
    'authors' => [
      'class' => 'App\Author',
      // Features for middlewares have not yet been implemented.
    ]
  ]
];

I'm also working on the concept "resolvers", just basic functions to return specific data from the application. For example, if I wanted to create a resolver to return the authenticated user it would be something like this.

return [ 'resolvers' => [ 'whoami' => [ 'class' => 'Restql\Resolvers\WhoamiResolver', ] ] ];

- The internal code
```php
<?php

namespace Restql\Resolvers;

use Restql\Resolver;
use Restql\SchemaDefinition;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\Auth;
use Restql\Contracts\SchemaHandlerContract;

final class WhoamiResolver extends Resolver implements SchemaHandlerContract
{
    /**
     * Implement the model|resolver handler.
     *
     * @param  \Restql\SchemaDefinition $schema
     * @return \Illuminate\Support\Collection
     */
    public function handle(SchemaDefinition $schema): Collection
    {
        return Collection::make(Auth::user());
    }
}
gregorip02 commented 4 years ago

Guards and Authentication added in #6