I would lay it out something like this:
interface AuthenticableInterface
{
public function getName(): string;
/**
* Based on auth type, return the right format of credentials to be sent to the server
*
* @param Authenticable $credentials
* @return mixed
*/
public function encodedCredentials(): string;
}
class AbstractAuthenticable implements AuthenticableInterface
{
protected $name = 'PLAIN';
protected $options;
public function __construct(Options $options)
{
$this->options = $options;
}
abstract public function encodedCredentials(): string;
public function getName(): string
{
return $this->name;
}
}
class Plain extends AbstractAuthenticable{
public function encodedCredentials(): string
{
$credentials = "\x00{$this->options->getUsername()}\x00{$this->options->getPassword()}";
return XML::quote(base64_encode($credentials));
}
}
I kept the interface, a lot of people get confused between using abstract and interface. I should add that the interface is inherited through the extending of the abstract class, so there is no need to declare (implement) it again.
Interfaces basically just outline the classes API (application program interface, or I like to say public interface), Abstract classes do the same thing (basically) as an interface but you can have concrete implementation of common functionality within the class. Interface doesn't allow you to implement any functionality.
By keeping the interface you could in theory create a Authenticable that uses a base other then the Abstract class. This is useful especially when type casting.
class foo{
public function __construct(AuthenticableInterface $Authenticable){
//...
}
}
So instead of hinting the Abstract class or the Concrete class you can type hint the interface which can be satisfied even when using a completely different base class. In this case it may not be strictly necessary to carry the interface but it does give you that little bit of extra flexibility for the hierarchy.
A good example of the above would be to make a token based login for something like REST where you don't have a username and password, and so that class could be quite different then the base class that uses a user and a password.
Another example I can think of is if you have a hierarchy in your user system, such as a Parent and Subusers, or Admins and non-admins. You may want something like a passwordless Login As functionality that relies more on the user's role and relationship to decide if it should authenticate. Something like a button you just click to login as one of your associated subusers etc. We use subusers a lot in our user systems.
Rather or not you should decouple the username and password depends really on how you want to pass that data. Obviously username and password are going to be really important so it may be worth adding them as function arguments. But as I mentioned above you may create authentication that does not use a username and password.
I would say the biggest argument for it is, then the password and username is not "hidden" or buried in some kind of BLOB or array data. But on the same token (pun intended) you may have some classes that don't need them. So the biggest disadvantage is then requiring it as part of the class, which may break the single interface design.
For example consider this interface and class:
interface Authenticable
{
public function setPassword(string $password);
}
class LoginAs
{
//public function setPassword(string $password);
}
So you can see setting a password for "LoginAs" doesn't really make sense and this would stretch what we can do with our interface. By having them as options they are not exposed to the interface because they are wrapped in another object.
I'll end by saying if you keep them in the options you will want to do some more robust error checking such as this:
public function encodedCredentials(): string
{
$credentials = "\x00{$this->options->getUsername()}\x00{$this->options->getPassword()}";
return XML::quote(base64_encode($credentials));
}
I modified your getUsername
function in Options
. I like to set them up this way.
class options{
public function getUsername($default=null){
if(isset($this->username)) return $this->username;
if(!is_null($default)) return $default;
throw new OptionException("Option {$username} is not set");
}
}
By doing this we can control what we get back when its not set. By setting a default other than null, when default is not set we throw an error. Typically I do this using the magic __call
method etc.. Basically I showed this to show how you can implement some error checking on those values. Obviously you could check this in each encodedCredentials()
but you can also bake it into the options class so it happens when we try to get an options etc...
Hope that helps!
Options
will always be easily accessible or not. Otherwise it looks good. The only thing I would add is there may be room for an Abstract class ( probably keep the interface too ). For things likepublic function getName()
etc. Which may need to be duplicated in all children etc... It never hurts to have an interface even it you have an Abstract class, sometimes you can do away with it, if you know you won't need to extend something other then your base class etc. \$\endgroup\$