Skip to content

[Cookbook] Use configured user provider instead of injection #5255

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

Closed
wants to merge 3 commits into from
Closed

[Cookbook] Use configured user provider instead of injection #5255

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5552cbe
Use configured user provider instead of injection
mvar May 13, 2015
b9b23b3
Check user provider type
mvar May 23, 2015
539fc50
Improve invalid user provider exception message
mvar May 23, 2015
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
82 changes: 44 additions & 38 deletions cookbook/security/api_key_authentication.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,6 @@ value and then a User object is created::

class ApiKeyAuthenticator implements SimplePreAuthenticatorInterface
{
protected $userProvider;

public function __construct(ApiKeyUserProvider $userProvider)
{
$this->userProvider = $userProvider;
}

public function createToken(Request $request, $providerKey)
{
// look for an apikey query parameter
Expand All @@ -66,16 +59,22 @@ value and then a User object is created::

public function authenticateToken(TokenInterface $token, UserProviderInterface $userProvider, $providerKey)
{
if (!$userProvider instanceof ApiKeyUserProvider) {
throw new \InvalidArgumentException(
'$userProvider must be an instance of "ApiKeyUserProvider".'
Copy link
Member

Choose a reason for hiding this comment

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

I think we can give some more information here like:

throw new \InvalidArgumentException(
    sprintf(
        'The user provider must be an instance of ApiKeyUserProvider (%s was given).',
        get_class($userProvider)
    )
);

);
}

$apiKey = $token->getCredentials();
$username = $this->userProvider->getUsernameForApiKey($apiKey);
$username = $userProvider->getUsernameForApiKey($apiKey);
Copy link
Member

Choose a reason for hiding this comment

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

Don't we need to check first if the ApiKeyAuthenticator actually supports the given token? So, something like the following at the beginning of the method:

if (!$this->supportsToken($token)) {
    return;
}

Copy link
Member

Choose a reason for hiding this comment

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

And we also have to ensure that the given $userProvider actually is an instance of ApiKeyUserProvider.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xabbuh I like your first question, but I think it is out of scope of this topic. Even without my changes, same question could be asked. The cookbook referenced in this PR says If supportsToken() returns true, Symfony will now call authenticateToken() (in explanation how authentication process works). So Symfony checks that automatically. Would be strange if it wouldn't check. IMHO when method is defined in interface it's so not because you should use it internally.

Thanks for the note about user provider, I agree that we should add check here. I will update my PR later.

Copy link
Member

Choose a reason for hiding this comment

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

I thought about this again and I think we actually do not need to check for the token type. The authenticator doesn't do anything special with the token and so it should work just fine.


if (!$username) {
throw new AuthenticationException(
sprintf('API Key "%s" does not exist.', $apiKey)
);
}

$user = $this->userProvider->loadUserByUsername($username);
$user = $userProvider->loadUserByUsername($username);

return new PreAuthenticatedToken(
$user,
Expand Down Expand Up @@ -192,7 +191,7 @@ The ``$userProvider`` might look something like this::
}
}

Now register your user provider as service:
Now register your user provider as a service:

.. configuration-block::

Expand Down Expand Up @@ -258,7 +257,7 @@ exception in ``refreshUser()``.
Handling Authentication Failure
-------------------------------

In order for your ``ApiKeyAuthentication`` to correctly display a 403
In order for your ``ApiKeyAuthenticator`` to correctly display a 403
http status when either bad credentials or authentication fails you will
need to implement the :class:`Symfony\\Component\\Security\\Http\\Authentication\\AuthenticationFailureHandlerInterface` on your
Authenticator. This will provide a method ``onAuthenticationFailure`` which
Expand Down Expand Up @@ -290,11 +289,9 @@ you can use to create an error ``Response``.
Configuration
-------------

Once you have your ``ApiKeyAuthentication`` all setup, you need to register
Once you have your ``ApiKeyAuthenticator`` all setup, you need to register
it as a service and use it in your security configuration (e.g. ``security.yml``).
First, register it as a service. This assumes that you have already setup
your custom user provider as a service called ``your_api_key_user_provider``
(see :doc:`/cookbook/security/custom_provider`).
First, register it as a service.

.. configuration-block::

Expand All @@ -305,8 +302,8 @@ your custom user provider as a service called ``your_api_key_user_provider``
# ...

apikey_authenticator:
class: AppBundle\Security\ApiKeyAuthenticator
arguments: ["@api_key_user_provider"]
class: AppBundle\Security\ApiKeyAuthenticator
public: false

.. code-block:: xml

Expand All @@ -321,9 +318,7 @@ your custom user provider as a service called ``your_api_key_user_provider``

<service id="apikey_authenticator"
class="AppBundle\Security\ApiKeyAuthenticator"
>
<argument type="service" id="api_key_user_provider" />
</service>
public="false" />
</services>
</container>

Expand All @@ -335,13 +330,13 @@ your custom user provider as a service called ``your_api_key_user_provider``

// ...

$container->setDefinition('apikey_authenticator', new Definition(
'AppBundle\Security\ApiKeyAuthenticator',
array(new Reference('api_key_user_provider'))
));
$definition = new Definition('AppBundle\Security\ApiKeyAuthenticator');
$definition->setPublic(false);
$container->setDefinition('apikey_authenticator', $definition);
Copy link
Member

Choose a reason for hiding this comment

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

Imho we can simplify this a bit:

$definition = $container->setDefinition('apikey_authenticator', new Definition(
    'AppBundle\Security\ApiKeyAuthenticator',
    array(new Reference('api_key_user_provider'))
));
$definition->setPublic(false);

Copy link
Member

Choose a reason for hiding this comment

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

-1, I think the current code is clearer.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, then let's keep it as is.


Now, activate it in the ``firewalls`` section of your security configuration
using the ``simple_preauth`` key:
Now, activate it and your custom user provider (see :doc:`/cookbook/security/custom_provider`)
in the ``firewalls`` section of your security configuration
using the ``simple_preauth`` and ``provider`` keys respectively:

.. configuration-block::

Expand All @@ -357,6 +352,7 @@ using the ``simple_preauth`` key:
stateless: true
simple_preauth:
authenticator: apikey_authenticator
provider: api_key_user_provider

providers:
api_key_user_provider:
Expand All @@ -377,6 +373,7 @@ using the ``simple_preauth`` key:
<firewall name="secured_area"
pattern="^/admin"
stateless="true"
provider="api_key_user_provider"
>
<simple-preauth authenticator="apikey_authenticator" />
</firewall>
Expand All @@ -399,6 +396,7 @@ using the ``simple_preauth`` key:
'simple_preauth' => array(
'authenticator' => 'apikey_authenticator',
),
'provider' => 'api_key_user_provider',
),
),
'providers' => array(
Expand All @@ -408,7 +406,7 @@ using the ``simple_preauth`` key:
),
));

That's it! Now, your ``ApiKeyAuthentication`` should be called at the beginning
That's it! Now, your ``ApiKeyAuthenticator`` should be called at the beginning
of each request and your authentication process will take place.

The ``stateless`` configuration parameter prevents Symfony from trying to
Expand Down Expand Up @@ -444,6 +442,7 @@ configuration or set it to ``false``:
stateless: false
simple_preauth:
authenticator: apikey_authenticator
provider: api_key_user_provider

providers:
api_key_user_provider:
Expand All @@ -464,6 +463,7 @@ configuration or set it to ``false``:
<firewall name="secured_area"
pattern="^/admin"
stateless="false"
provider="api_key_user_provider"
>
<simple-preauth authenticator="apikey_authenticator" />
</firewall>
Expand All @@ -485,6 +485,7 @@ configuration or set it to ``false``:
'simple_preauth' => array(
'authenticator' => 'apikey_authenticator',
),
'provider' => 'api_key_user_provider',
),
),
'providers' => array(
Expand All @@ -507,8 +508,14 @@ to see if the stored token has a valid User object that can be used::
// ...
public function authenticateToken(TokenInterface $token, UserProviderInterface $userProvider, $providerKey)
{
if (!$userProvider instanceof ApiKeyUserProvider) {
throw new \InvalidArgumentException(
'$userProvider must be an instance of "ApiKeyUserProvider".'
);
}

$apiKey = $token->getCredentials();
$username = $this->userProvider->getUsernameForApiKey($apiKey);
$username = $userProvider->getUsernameForApiKey($apiKey);

// User is the Entity which represents your user
$user = $token->getUser();
Expand All @@ -527,7 +534,7 @@ to see if the stored token has a valid User object that can be used::
);
}

$user = $this->userProvider->loadUserByUsername($username);
$user = $userProvider->loadUserByUsername($username);

return new PreAuthenticatedToken(
$user,
Expand Down Expand Up @@ -601,13 +608,10 @@ current URL is before creating the token in ``createToken()``::

class ApiKeyAuthenticator implements SimplePreAuthenticatorInterface
{
protected $userProvider;

protected $httpUtils;

public function __construct(UserProviderInterface $userProvider, HttpUtils $httpUtils)
public function __construct(HttpUtils $httpUtils)
{
$this->userProvider = $userProvider;
$this->httpUtils = $httpUtils;
}

Expand Down Expand Up @@ -642,7 +646,8 @@ service:

apikey_authenticator:
class: AppBundle\Security\ApiKeyAuthenticator
arguments: ["@api_key_user_provider", "@security.http_utils"]
arguments: ["@security.http_utils"]
public: false

.. code-block:: xml

Expand All @@ -657,8 +662,8 @@ service:

<service id="apikey_authenticator"
class="AppBundle\Security\ApiKeyAuthenticator"
public="false"
>
<argument type="service" id="api_key_user_provider" />
<argument type="service" id="security.http_utils" />
</service>
</services>
Expand All @@ -672,12 +677,13 @@ service:

// ...

$container->setDefinition('apikey_authenticator', new Definition(
$definition = new Definition(
'AppBundle\Security\ApiKeyAuthenticator',
array(
new Reference('api_key_user_provider'),
new Reference('security.http_utils')
)
));
);
$definition->setPublic(false);
$container->setDefinition('apikey_authenticator', $definition);

That's it! Have fun!