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.

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

Scopes and security #402

Open
NathanVss opened this issue Mar 21, 2016 · 12 comments
Open

Scopes and security #402

NathanVss opened this issue Mar 21, 2016 · 12 comments

Comments

@NathanVss
Copy link

Hello everyone.

One concept of this bundle is a bit confusing me. When dealing with scope, why can we get any scope without permissions given by the client ?
I mean, any user can log ( with grant_type password ) and request any scope they want.
I'm facing this problem because of my current situation :
I one hand I have multiple users that log with grant_type password with the client. In the other hand I must configure a external trusted server access to certain restricted API routes that the lambda users should not be able to request. So I wanted to give a special role to my external server with scopes. Then the problem is that any user can get any scope they want.

How to deal with that ?
Thank you.

@Spomky
Copy link

Spomky commented Mar 21, 2016

OAuth2 is about delegation, not authentication. When a client uses the resource owner password credentials grant, it just means that the resource owner allow it to access on its resources.
The authorization is never given by the client, but by the resource owner through the authorization server.

The confusion is not in the use of scopes or grant types, but in the fact that this bundle is used to authenticate user while It should be use to protect a resource server.

Regarding the resource owner password credentials grant, you should also keep in mind that it is suitable in cases where the resource owner has a trust relationship with the client (see RFC6749 section 4.3).

@NathanVss
Copy link
Author

Thank you for your quick response.

Yes, the resources owner has a trust relationship with the client. But how can I restrict certains scopes to some client with this bundle actually ?

I didn't clearly understood this sentence " but in the fact that this bundle is used to authenticate user while It should be use to protect a resource server". Do you mean I use this bundle I a uncorrect way ?

Thanks.

@Spomky
Copy link

Spomky commented Mar 21, 2016

Yes, the resources owner has a trust relationship with the client. But how can I restrict certains scopes to some client with this bundle actually ?

I am sorry but this feature is not yet implemented (see that -closed- PR). And I am not sure it will be implemented within the next weeks.

Do you mean I use this bundle I a uncorrect way ?

As you noted, the problem with the resource owner password credentials grant is that the client can issue access tokens at any time with any scopes. That is why you should avoid its use unless you have absolute confidence in it (which is the case as I understand).

For me the main confusion comes from the fact that this bundle is often used to authenticate users, but OAuth2 is not designed for that purpose.
If you really want to authenticate users using OAuth2, then the OpenID Connect extension is a good alternative... but not supported at the moment by this bundle.

@NathanVss
Copy link
Author

Thanks.
The client is a mobile application, so any user can modify the token request.

So the only solution to deal with scopes with this bundle now is to create users with wanted roles and then use password grant type ?

Thank you again for your advices.

@ihorsamusenko
Copy link

Ran into the same problem, we are going to use client_credential grant.

According to the doc (https://tools.ietf.org/html/rfc6749#section-3.3) :

The authorization server MAY fully or partially ignore the scope
requested by the client, based on the authorization server policy or
the resource owner's instructions. If the issued access token scope
is different from the one requested by the client, the authorization
server MUST include the "scope" response parameter to inform the
client of the actual scope granted.

  1. I extended my Client entity with fied allowed_scopes
  2. I specified fos_oauth_server.service.storage service to my own Storage class, with different createAccessToken method.

What beats me is that OAuth2\Oauth2::createAccessToken() does not care what I save to database.
Can I somehow change behavior of Oauth2\OAuth2 class methods ?

@pronata
Copy link

pronata commented Nov 30, 2016

@samusenkoiv I guess my comment is late but I also encountered with the need to change the Oauth2\OAuth2 class methods.
Here I found the answer: https://github.com/FriendsOfSymfony/FOSOAuthServerBundle/issues/337#issuecomment-247557225
It's possible to extend OAuth2/OAuth2 class and override methods, then override the default package class OAuth2 in services.yml:

parameters:
    fos_oauth_server.server.class: My\Bundle\OAuth2\OAuth2

@ihorsamusenko
Copy link

@pronata this is how I made it work =) anyway, thanks for trying to help.

@pronata
Copy link

pronata commented Dec 1, 2016

What is the status of updating the scope policy? Let it set scopes for concrete client and set default scope?

@chmielot
Copy link

I just wanted to share my (alternative/cleaner?) solution to overwrite the createAccessToken method of the OAuth2 library class:

  1. Create a compiler pass: http://symfony.com/doc/current/service_container/compiler_passes.html and http://symfony.com/doc/current/bundles/override.html#services-configuration
<?php

namespace AppBundle\DependencyInjection\Compiler;

use AppBundle\AppBundle;
use AppBundle\Services\OAuth2;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;

class OverrideOAuthServiceCompilerPass implements CompilerPassInterface
{
    public function process(ContainerBuilder $container)
    {
        $definition = $container->getDefinition('fos_oauth_server.server');
        $definition->setClass(OAuth2::class);
    }
}
  1. Extend your client with an allowedScopes field of type array (will be saved in allowed_scopes as a serialised string, similar to allowedGrantTypes)
  2. Overwrite the createAccessToken method of OAuth2\OAuth2. I know that we should extend the Client interface to be able to call getAllowedScopes but a pull request implementing that was closed without merging because of BC breaks: Scope Policy functionality oauth2-php#45
    So this can definitely be seen as an easy workaround until there's a better solution. The Client class is my own Client entity:
class OAuth2 extends OAuth2Base
{
    /**
     * @inheritdoc
     */
    public function createAccessToken(IOAuth2Client $client, $data, $scope = null, $access_token_lifetime = null,
                                      $issue_refresh_token = true, $refresh_token_lifetime = null)
    {
        if ($client instanceof Client) {
            $scope = implode(' ', $client->getAllowedScopes());
        }

        return parent::createAccessToken($client, $data, $scope, $access_token_lifetime, $issue_refresh_token, $refresh_token_lifetime);
    }
}

@thomas-negrault
Copy link

Thank you for your solution @chmielot. For anyone looking to reproduce this behaviour here is what I added to the previous code (use of OAuth2Base + string in $scope annotation in order to remove idea inspection)

<?php

namespace AppBundle\Services;

use OAuth2\Model\IOAuth2Client;
use OAuth2\OAuth2 as OAuth2Base;
use AppBundle\Document\Client;

class OAuth2 extends OAuth2Base
{
    /**
     * @param IOAuth2Client $client
     * @param mixed         $data
     * @param string|null   $scope
     * @param null          $access_token_lifetime
     * @param bool          $issue_refresh_token
     * @param null          $refresh_token_lifetime
     *
     * @return array
     */
    public function createAccessToken(
        IOAuth2Client $client,
        $data,
        $scope = null,
        $access_token_lifetime = null,
        $issue_refresh_token = true,
        $refresh_token_lifetime = null
    ) {
        if ($client instanceof Client) {
            $scope = implode(' ', $client->getAllowedScopes());
        }

        return parent::createAccessToken(
            $client,
            $data,
            $scope,
            $access_token_lifetime,
            $issue_refresh_token,
            $refresh_token_lifetime
        );
    }
}

And to my bundle file:

<?php

namespace AppBundle;

use AppBundle\DependencyInjection\Compiler\OverrideOAuthServiceCompilerPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\HttpKernel\Bundle\Bundle;

class AppBundle extends Bundle
{
    /**
     * Build Container
     *
     * @param ContainerBuilder $container
     */
    public function build(ContainerBuilder $container)
    {
        parent::build($container);
        $container->addCompilerPass(new OverrideOAuthServiceCompilerPass());
    }
}

@bpotard
Copy link

bpotard commented Jul 13, 2018

As an alternative / addition to the Client level scope restriction above - which can be useful on its own - I believe it could also be useful that the actual roles granted to the User (i.e. corresponding to the user credentials used for authentication) are enforced.

This can be done by overloading the checkUserCredentials method of the OAuthStorage service, e.g.:

<?php

namespace AppBundle\Service;

use AppBundle\Entity\User;
use FOS\OAuthServerBundle\Storage\OAuthStorage as OAuthStorageBase;
use OAuth2\Model\IOAuth2Client;

class OAuthStorage extends OAuthStorageBase
{
    public function checkUserCredentials(IOAuth2Client $client, $username, $password) {
        $stored = parent::checkUserCredentials($client, $username, $password);
        if ($stored == false) return $stored;
        // Otherwise $stored['data'] is the User entity
        /** @var User $user */
        $user = $stored["data"];
        // Get the roles of the user, convert it to the expected format for scopes
        $roles = strtolower(implode(" ", str_replace("ROLE_", "", $user->getRoles())));
        // Inject the scopes as  default requirements for the $user
        // N.B.: if the parent has already set a scope, we do not override it
        $stored += array('scope' => $roles);
        return $stored;
    }
}

... and again creating a compiler pass (or modifying the previous one if you are doing the Client-level role restriction):

<?php

namespace AppBundle\DependencyInjection\Compiler;

use AppBundle\Service\OAuthStorage;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;

class OverrideOAuthServiceCompilerPass implements CompilerPassInterface
{
    public function process(ContainerBuilder $container)
    {
        $definition = $container->getDefinition('fos_oauth_server.storage.default');
        $definition->setClass(OAuthStorage::class);
    }
}

Note that this will error rather than silently change the scope if a user requests a scope he is not entitled to. Hope that helps!

@AntoniusGolly
Copy link

@bpotard your solution is a life saver. You should write an article about it as the package is still lacking a native solution. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants