Use configuration classes instead of configuration arrays

Is your feature request related to a problem? Please describe.
SimpleSAMLphp is a great library that provides all sorts of configuration options so it does exactly what you want.
All options are (or at least, should be) documented on the website. However, all configuration is done using simple PHP arrays - which are very flexible, but don't have any typehints available and are not checked on runtime.

Describe the solution you'd like
It would be great, if it is possible to configure (some parts) with configuration classes - that are fully OOP and therefore typehinted.

This has a few advantages:

It is, while configuring in a code editor, clear what can be configured - typehints and errors are automatically shown
It is possible to make options required or provide clear defaults for options
It is possible to deprecate options, which will be shown in the code editor
It is possible to see (in PHPStorm for example) where a certain option is used in the codebase
The configuration classes are a clear overview of all available options, which can be (automatically) used to keep documentation up-to-date

This can be a backwards compatible change - for example, new options could be configured with classes that are still put into an array. And all classes could have an fromArray method that would make convertions easy (and that could be automatically called by simplesamlphp, so it is not a breaking change).

Describe alternatives you've considered
Keep everything as it is. But I think that with all new PHP stuff going on, arrays are not the best option anymore.

Additional context
Of course, there are a lot of different ways these classes could be structured.

For example, if PHP 8.1 is the minimum required PHP version, this would be possible:

<?php
namespace \SimpleSAML\Configuration;

class SpAuthsource {
  /**
   * Configure a new SAML service provider, that could be used to get data from
   * a SAML identity provider.
   */
  public function __construct(
    /**
     * This is a key that should be unique within the simplesamlphp
     * authsources.
     */
    public string $key = 'default-sp',

    /**
     * The entityID is the ID that will be used by other systems to identify
     * this service provider. If NULL, the URL where the metadata of this sp
     * can be downloaded will be used.
     */
    public ?string $entityID = NULL,

    /**
     * This is the certificate that will be used while decrypting ...
     */
    public ?SpCertificate $certificate = NULL,

    /**
     * If you want to use key rollover, you can provide your previous 
     * certificate here so ...
     */
    public ?SpCertificate $oldCertificate = NULL,
    ) {}

  public static function fromArray(array $config): SpAuthsource {
    $spAuthsource = new SpAuthsource($config['key']);
    $spAuthsource->entityID = $config['entityId'];

    if (!empty($config['certificate'])) {
      $spAuthsource->certificate = new SpCertificate(
        certificate: $arguments['certificate'],
        privatekey: $arguments['privatekey'] ?? NULL,
        privatekeyPass: $arguments['privatekey_pass'] ?? NULL,
      );
    }
    if (!empty($config['new_certificate'])) {
      $spAuthsource->oldCertificate = $spAuthsource->certificate;
      $spAuthsource->certificate = new SpCertificate(
        certificate: $arguments['new_certificate'],
        privatekey: $arguments['new_privatekey'] ?? NULL,
        privatekeyPass: $arguments['new_privatekey_pass'] ?? NULL,
      );
    }
    return $spAuthsource;
  }
}

and for the end-user, it would be configured like this:

<?php
// authsources.php

$authsources[] = new \SimpleSAML\Configuration\SpAuthsource(
  key: 'default-sp',
  certificate: new \SimpleSAML\Configuration\Certificate(
    certificate: __DIR__ . '../cert.crt',
  ),
);

Is this something that would be considered?

@bartlangelaan

@bartlangelaan Hi Bart! It's definitely an interesting thing to discuss! I do think however, that a minimum version of PHP 8.1 is still a long way ahead of us. For our next major release we're aiming at 7.4

I agree - but the classses could be built in a PHP 7.4 compatible way, where they could be used with named parameters (if the user uses 8.1) or by changing the propertyes (for 7.4 / 8.0).

<?php
namespace \SimpleSAML\Configuration;

class SpAuthsource {

    /**
     * This is a key that should be unique within the simplesamlphp
     * authsources.
     */
    public string $key = 'default-sp';

    /**
     * The entityID is the ID that will be used by other systems to identify
     * this service provider. If NULL, the URL where the metadata of this sp
     * can be downloaded will be used.
     */
    public ?string $entityID = NULL;

    /**
     * This is the certificate that will be used while decrypting ...
     */
    public ?SpCertificate $certificate = NULL;

    /**
     * If you want to use key rollover, you can provide your previous 
     * certificate here so ...
     */
    public ?SpCertificate $oldCertificate = NULL;

    /**
     * Configure a new SAML service provider, that could be used to get data from
     * a SAML identity provider.
     */
    public function __construct(
      string $key = 'default-sp',
      string $entityId = NULL,
      SpCertificate $certificate = NULL,
      SpCertificate $oldCertificate = NULL,
    ) {
      $this->key = $key;
      $this->entityId = $entityId;
      $this->certificate = $certificate;
      $this->oldCertificate = $oldCertificate
    }
}

It could be used in either of these ways:

<?php
// authsources.php

// PHP 8.1
$authsources[] = new \SimpleSAML\Configuration\SpAuthsource(
  key: 'default-sp',
  certificate: new \SimpleSAML\Configuration\Certificate(
    certificate: __DIR__ . '../cert.crt',
  ),
);

// PHP 7.4 / PHP 8.0
$certificate =  new \SimpleSAML\Configuration\Certificate();
$certificate->certifictate = __DIR__ . '../cert.crt';
$otherSp = new \SimpleSAML\Configuration\SpAuthsource('other-sp');
$otherSp->certificate = $certificate;
$authsources[] = $otherSp;

Both ways are still fully typed.

Agreed! I'm a bit worried about the complexity for deployers though.. They're not necessarily PHP programmers.
I was thinking about the possibility to have the actual configuration values in a YAML-file to reduce complexity

Hmm, that is still not typed and thus statically checked.

Also, in our case, the configuration is not for the deployers. It is part of a complex Drupal setup, where the configuration is automatically set based on environment variables and files being available on the server.

Having the option to configure things with yaml could be great, but I think that that could still be as complex as having a php array.

Making sure that every configuration class also has a ::fromArray() method, that could also be used for importing yaml, in the future.

@bartlangelaan

Hi @bartlangelaan ! I've made a small PoC of what I had in mind. I'm curious to hear what you think!

Navigation Menu

Repository navigation

Use configuration classes instead of configuration arrays #1550

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Use configuration classes instead of configuration arrays #1550

Description

Activity

tvdijen commented on Dec 18, 2021

bartlangelaan commented on Dec 20, 2021

tvdijen commented on Dec 20, 2021

bartlangelaan commented on Dec 21, 2021

tvdijen commented on Mar 10, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions