Using getters in Provider class

[robinvdvleuten]

To make the Provider class more extendable, I've added getters for all injected arguments. In my case, I needed another user property (an api key) than the password. And instead of copying over the whole class, I just could rewrite the authenticate function.

[djoos]

Hi Robin,

thanks for the PR - I'll have a look at getting this merged in this week.

As I'm always interested in different use cases of the WSSE authentication-bundle: would you mind sharing a bit more about the issues you were experiencing?

Thanks in advance for your feedback! (oftewel: dank bij voorbaat! :-) )

Kind regards, David (een zuiderbuur ;-) )

[robinvdvleuten]

Ha zuiderbuur!

Het is altijd lastig om iemand zijn herkomst af te leiden uit nicknames, ook omdat je London als standplaats op je GH profiel hebt staan ;)

Back to the topic, I'm currently working on an app with Symfony as back- and AngularJS as frontend. Both using the same domain. To authenticate with the stateless backend, I'm generating WSSE headers with wsse.js. But wasn't very happy with the fact that I need to expose the user's plain password to generate the WSSE header correctly. So I choose to generate API keys for each user and use these to generate the WSSE headers. But because your bundle's code uses $user->getPassword() to validate the incoming header, I extended the class and changed it to $user->getApiKey(). With the help of the added getters, I just had to overwrite the authenticate method instead of the whole class.

Another option would be that your bundle provides some sort of interface for the User class that adds a method which let the User class return the correct property by itself.

Groeten, Robin

PS. If you do not have the time but like my last suggestion, I can implement this functionality in this PR.

[djoos]

Hey Robin,

nicknames zijn inderdaad geen goede indicatie; ik woon/werk in Londen, maar ben 100% Belg ;-)

Re: expose the user's plain password to generate the WSSE header I agree that this is not ideal, even though there's no real problem when sending the sha1 digest over the line, especially via HTTPS... Having said that: I've made some improvements (see the "dev_encoder_of_choice"-branch) in reply to issue #20, which might potentially help out?

Re: $user->getPassword() I agree that this could be improved as what the bundle actually needs is a secret, which could be the user's password, API key or any other user attribute depending on the actual implementation. An interface with a getSecret()-method seems interesting, with a fallback to user->getPassword() if the interface is not implemented?

Let me know your thoughts on the items above, thanks in advance for your feedback!

Groeten, David

[djoos]

Hi Robin,

have you already had the opportunity to give the "dev_encoder_of_choice"-branch a try? Thanks in advance for your feedback!

Kind regards, David

[robinvdvleuten]

Hi David,

I haven't found the time to test the code, but I'll like the idea. But I stiI would suggest letting the user of the bundle decide which property to use when matching the digest. I guess there are two good options; an User interface with a method and a method on the Provider class calling $user->getPassword() which can be overwritten so it can returns $user->getApiKey() or something else. I can create some code for this if you like?

Cheers,

Robin

[robinvdvleuten]

Hi David,

I've implemented a simple method for retrieving the user's 'secret' property. Now extending the Provider class for a custom secret property is as easy as;

<?php

namespace Rvdv\Bundle\TimesheetBundle\Security\Core\Authentication\Provider;

use Escape\WSSEAuthenticationBundle\Security\Core\Authentication\Provider\Provider as BaseProvider;

class Provider extends BaseProvider
{
    protected function getUserSecret($user)
    {
        return $user->getApiKey();
    }
}

[djoos]

Hi Robin,

I just added a line comment to the latest commit - do you still need the initial commit's (8a3b8283529b2ae96209c525e1781356f1ebf354) changes actually?

Thanks in advance for your feedback!

Kind regards, David

[robinvdvleuten]

These are indeed not needed anymore, I should remove them.

Best regards,

Robin

[djoos]

Thanks for your contribution @RobinvdVleuten!

[robinvdvleuten]

You're welcome! Thanks for the awesome bundle.

[djoos]

Thanks!