bshaffer / oauth2-server-php

A library for implementing an OAuth2 Server in php
http://bshaffer.github.io/oauth2-server-php-docs
MIT License
3.26k stars 953 forks source link

Scope verification is not correct #604

Open TarasHots opened 9 years ago

TarasHots commented 9 years ago

Found this issue on the latest 1.7.1 version, and zf-mvc-auth's latest of 1.1.3. Problem is in false approving access for authorized users but with smaller scope access. I have an ACL roles called e.g. 'test' and 'live'. I have some resource called A_Resource, and allowed 'live' users access to it. This issue allows 'test' users to be accessed to A_Resource. I debugged a bit and this strange behaviour starts at:

public function authenticate(Request $request, Response $response, MvcAuthEvent $mvcAuthEvent)
    {
        $content       = $request->getContent();
        $oauth2request = new OAuth2Request(
            $_GET,
            $_POST,
            array(),
            $_COOKIE,
            $_FILES,
            $_SERVER,
            $content,
            $request->getHeaders()->toArray()
        );
        if (! $this->oauth2Server->verifyResourceRequest($oauth2request)) {//here
            return false;
        }
        $token    = $this->oauth2Server->getAccessTokenData($oauth2request);
        $identity = new Identity\AuthenticatedIdentity($token);
        $identity->setName($token['user_id']);
        return $identity;
    }

the method's signature is

public function verifyResourceRequest(RequestInterface $request, ResponseInterface $response = null, $scope = null)

As a result, here:

public function verifyResourceRequest(RequestInterface $request, ResponseInterface $response, $scope = null)
    {
        $token = $this->getAccessTokenData($request, $response);
        // Check if we have token data
        if (is_null($token)) {
            return false;
        }
        /**
         * Check scope, if provided
         * If token doesn't have a scope, it's null/empty, or it's insufficient, then throw 403
         * @see http://tools.ietf.org/html/rfc6750#section-3.1
         */
        //problem in $scope
        if ($scope && (!isset($token["scope"]) || !$token["scope"] || !$this->scopeUtil->checkScope($scope, $token["scope"]))) {
            $response->setError(403, 'insufficient_scope', 'The request requires higher privileges than provided by the access token');
            $response->addHttpHeaders(array(
                'WWW-Authenticate' => sprintf('%s realm="%s", scope="%s", error="%s", error_description="%s"',
                    $this->tokenType->getTokenType(),
                    $this->config['www_realm'],
                    $scope,
                    $response->getParameter('error'),
                    $response->getParameter('error_description')
                )
            ));
            return false;
        }
        // allow retrieval of the token
        $this->token = $token;
        return (bool) $token;
    }

we got problem with $scope variable, because it's isn't false, it is null, and this breaks the statement and not sending 403 error. The rest of checks in that if statement are fine. But seems that scope should be provided, and appropriate logic for default values also need to be implemented... Probably the problem is in authenticate method, passing role, all works great, but didn't find way to get requested scope there...

Looking forward for your response

weierophinney commented 9 years ago

@TarasHots This issue needs to be reported against zfcampus/zf-mvc-auth. As you note, $scope is not being passed, and oauth2-server-php is then correctly skipping the resource check. What you're essentially requesting is a way to specify the scope for zf-mvc-auth to send to oauth2-server-php, which is why you need to open the issue against that repository.