Authorization seems to not work as expected

[briets]

First of all thumbs up for all your impressive work on this boilerplate! A very simplified and quick way of setting up an API.

I've however come across an issue of which I'm not sure it is a bug or just idiotic use. Probably the latter... :-)

It seems in the \App\Resources\UserResource class it does not matter how I setup the ACL for ApiEndpoint::get and ApiEndpoint::post routes (I haven't tested this for other request methods). They will always be accessible no matter if a user is logged on or not (even on no Authorization key). Even if I deny AclRoles::ALL_ROLES the method will still be executed in stead of returning an access denied message. It doesn't seem to matter whether I deny access to the entire resource or just on all endpoints in the resource.

The strange thing is that the ACL will be enforced on ApiEndPoint::all routes.

I'm hoping someone can help me with this issue as I said before I really like this approach of setting up an API.

Kind regards, Rick

[briets]

I think I've found the issue. Nameless routes declared with $this->endpoint in a resource will be stored by name in the resource. If multiple routes have no name (null) they will overwrite each other and only the last declared endpoint will work as expected. I've made a fix so that an ApiEndpoint always will have a name.

<?php

namespace App\Api;

use PhalconApi\Constants\HttpMethods;
use PhalconRest\Api\ApiEndpoint as Endpoint;

/**
 * ApiCollection cannot handle nameless endpoints. To ensure every endpoint has a name 
 * one will be created on construct. This name can easily be overridden by 
 * calling the setName method.
 *
 * The original ApiEndpoint was created using self:: in stead of static::. This has 
 * also been overridden so that extended ApiEndpoint classes call themselves on
 * factory.
 * 
 * @author briets
 */
class ApiEndpoint extends Endpoint {
    
    public function __construct($path, $httpMethod = HttpMethods::GET, $handlerMethod = null)
    {
        parent::__construct($path, $httpMethod, $handlerMethod);
        $this->name = serialize(['endpoint' => $this->getIdentifier()]);
    }
    
    /**
     * Returns pre-configured all endpoint
     *
     * @return static
     */
    public static function all()
    {
        return static::factory('/', HttpMethods::GET, 'all')
            ->name(parent::ALL)
            ->description('Returns all items');
    }

    /**
     * Returns endpoint with default values
     *
     * @param string $path
     * @param string $httpMethod
     * @param string $handlerMethod
     *
     * @return static
     */
    public static function factory($path, $httpMethod = HttpMethods::GET, $handlerMethod = null)
    {
        return new static($path, $httpMethod, $handlerMethod);
    }

    /**
     * Returns pre-configured find endpoint
     *
     * @return static
     */
    public static function find()
    {
        return static::factory('/{id}', HttpMethods::GET, 'find')
            ->name(parent::FIND)
            ->description('Returns the item identified by {id}');
    }

    /**
     * Returns pre-configured create endpoint
     *
     * @return static
     */
    public static function create()
    {
        return static::factory('/', HttpMethods::POST, 'create')
            ->name(parent::CREATE)
            ->description('Creates a new item using the posted data');
    }

    /**
     * Returns pre-configured update endpoint
     *
     * @return static
     */
    public static function update()
    {
        return static::factory('/{id}', HttpMethods::PUT, 'update')
            ->name(parent::UPDATE)
            ->description('Updates an existing item identified by {id}, using the posted data');
    }

    /**
     * Returns pre-configured remove endpoint
     *
     * @return static
     */
    public static function remove()
    {
        return static::factory('/{id}', HttpMethods::DELETE, 'remove')
            ->name(parent::REMOVE)
            ->description('Removes the item identified by {id}');
    }

    /**
     * Returns pre-configured GET endpoint
     *
     * @param $path
     * @param string $handlerMethod
     *
     * @return ApiEndpoint
     */
    public static function get($path, $handlerMethod = null)
    {
        return static::factory($path, HttpMethods::GET, $handlerMethod);
    }

    /**
     * Returns pre-configured POST endpoint
     *
     * @param $path
     * @param string $handlerMethod
     *
     * @return ApiEndpoint
     */
    public static function post($path, $handlerMethod = null)
    {
        return static::factory($path, HttpMethods::POST, $handlerMethod);
    }

    /**
     * Returns pre-configured PUT endpoint
     *
     * @param $path
     * @param string $handlerMethod
     *
     * @return ApiEndpoint
     */
    public static function put($path, $handlerMethod = null)
    {
        return static::factory($path, HttpMethods::PUT, $handlerMethod);
    }

    /**
     * Returns pre-configured DELETE endpoint
     *
     * @param $path
     * @param string $handlerMethod
     *
     * @return ApiEndpoint
     */
    public static function delete($path, $handlerMethod = null)
    {
        return static::factory($path, HttpMethods::DELETE, $handlerMethod);
    }

    /**
     * Returns pre-configured HEAD endpoint
     *
     * @param $path
     * @param string $handlerMethod
     *
     * @return ApiEndpoint
     */
    public static function head($path, $handlerMethod = null)
    {
        return static::factory($path, HttpMethods::HEAD, $handlerMethod);
    }

    /**
     * Returns pre-configured OPTIONS endpoint
     *
     * @param $path
     * @param string $handlerMethod
     *
     * @return ApiEndpoint
     */
    public static function options($path, $handlerMethod = null)
    {
        return static::factory($path, HttpMethods::OPTIONS, $handlerMethod);
    }

    /**
     * Returns pre-configured PATCH endpoint
     *
     * @param $path
     * @param string $handlerMethod
     *
     * @return ApiEndpoint
     */
    public static function patch($path, $handlerMethod = null)
    {
        return static::factory($path, HttpMethods::PATCH, $handlerMethod);
    }
    
}

Now just be sure to use this ApiEndpoint for declaring endpoints in a resource or a collection. I'm not sure (didn't trace deep enough) if something else is not working anymore after this so use it at your own risk.

Kind regards, Rick

[cjaybayno]

I agree with you briets. I have been running this api boilerplate for more than a year and cannot find a proper solution for this bugs. My remediation is to create a resources for every single api witch turn to have a massive amount of resources files and I break the idea of using resource for group of api. Today I decided to find the root cause and made some fix

find the ApiCollection class and paste this code and replace the line 126

if ($endpoint->getName()) $this->endpointsByName[$endpoint->getName()] = $endpoint; else $this->endpointsByName[] = $endpoint;

if you endpoint doesn't have name, it will use the array index instead.

[briets]

The reason I created a new class that extends from the original ApiEndpoint class is that there is no need to change any code in the vendor folder. It's even possible to create a new package just for the solution as long as the solution is not implemented in the boilerplate.

The simplest solution would be to just always give a name to a route... ;) But let's act like that is not true....

[rmrmohan88]

Hi briets,

@briets Thanks for your solution it helps me a lot. I am a bit confused in which path i have to add this new class file or code in my project and when i have to call this class to overwrite the existing Endpoint class.