SAML2 v1.0 Technical Design

Participants:

• BB: Boy Baukema, SURFnet
• OM: Olav Morken, UNINETT
• PM: Pieter van der Meulen, SURFnet

Coding Standard: PSR-0, PSR-1, PSR-2.

Proposed by: BB

Resolution: Approved

BB: Using well established coding standards makes interaction

OM: Switching does incur a moment in history where 'everything changed'.

Requirements

OM:

• Relatively low-level classes to parse and generate the XML. Something like what you can find in SAML2/XML/.
• Helper-classes for the SAML 2.0 bindings.
• Helper-classes for implementing a SAML 2.0 SP and IdP.

BB:

• XML marshalling / unmarshalling.
• Object model with static guarantees (so I can do $response->getAssertion(0)->getIssuer() ).
• XML DSig / Enc helpers (never having to touch signature verification for instance)
• Ideally: a large amount of configuration freedom (preferably using Dependency Injection or a similar technique).

Subclassing IdP or SP objects

Proposed by: OM

Resolution: Rejected

OM:

    // Class implementation
    class MyIdP extends BasicIdP {

        protected function processAuthnRequest(..._AuthnRequest $request) {
            $response = $this->buildSimpleAuthnResponse($request);
            // Add our response data here. Then send it:
            $this->sendAuthnResponse($response);
        }

        // Other functions to process other message types.
    }

    // Some PHP handler script somewhere.
    $idp = new MyIdP();
    $idp->processMessage();

BB: I don't think a SP or IdP class will ever work for us. We're hosting a proxy with is a little bit of both (SP and IdP) and a lot of magic in between 😄. In fact I believe you can't model all the behaviour required for an SP or IdP in a single class, what makes an SP / IdP or Proxy is the behaviour (like does it do metadata refresh?), it is much more a system than a single class. Also it's way too easy to break LSP with such high level classes. I'd much rather favour composition over inheritance and have a lot of classes that have a single responsibility (like verifying the NotBefore and NotAfters).

Should mirror the SAML2 spec

Proposed by: OM

Resolution: Approved

OM: Creating an Assertion with attributes with a different NameIDFormat should be possible.

PM: Not really a requirement for us, as most SAML2 software doesn't support this.

BB: Not a requirement, but no impediment either.

API

Proposed by: OM

OM:

    $acs = new ...AttributeConsumingService();
    $acs->index = 0;
    $acs->ServiceName['en'] = 'testSP';
    $acs->ServiceDescription['en'] = 'A SP used to test stuff.';

    $eppn = new ...RequestedAttribute();
    $eppn->Name = 'urn:oid:1.3.6.1.4.1.5923.1.1.1.6';
    $eppn->NameFormat = 'urn:oasis:names:tc:SAML:2.0:attrname-format:uri';
    $eppn->isRequired = true;
    $acs->RequestedAttribute[] = $eppn;

BB:

AttributeConsumingService::create(
    ServiceNameCollection::create(array(ServiceName::create(Lang::ENGLISH, 'Name'))),
    RequestedAttributeCollection::create(array(
        RequestedAttribute::create('urn:oid:1.3.6.1.4.1.5923.1.1.1.6')
            ->betterKnownAs('EduPersonPrincipalName')
            ->useNameFormat(Constants::NAMEFORMAT_URI)
            ->require()
        ))
    )
    ->setServiceDescriptions(
        ServiceDescriptionCollection::create(array(ServiceDescription::create(LANG::ENGLISH, 'Consume attributes')))
    );

You could even have a Builder that trades IDE autocompletion over convenience, but at least the underlying model would have type checking. Nice thing here too is that you emulate the XML nesting, so you won't be tempted to write one large swath of code, you'll hit a point pretty soon where you hit your limit on the number of indents you can live with and be forced to extract part of this building to another method.

Actually OpenSAML goes even further and has the creation done by injected factories so you can inject your own factory at any point and have it create a "MyCustom\RequestedAttribute" instead of a 'normal' one. That was very helpful in testing the signature validation vulnerability (being able to trick it to add assertions in an assertion with extensions). But very boilerplate heavy though.

Favor Exceptions over assertions

Proposed by: BB

Resolution: Approved

BB: When something unexpected happens on production (like a method is given an array where it expected a string) we want to know. The overhead is negligible.

OM: I'm not against that change. I would however wait with it for a bit :)