Allow IdP authprocs to be configured in authsources.php

[nathanjrobertson]

Currently, for SPs you can configure authprocs in metadata, config.php or authsources.php. However, for an IdP you can only configure in metadata or config.php. This PR addresses this, such that regardless of whether you're configuring an IdP or an SP, you can configure in all three places.

The main driver for the desire for authsources.php to be supported as an IdP authproc configuration source is when using multiauth with multiple different authentication sources. When using multiauth, it would be really handy to be able to apply different authprocs based on which authentication mechanism was delegated to (as defined in authsources.php).

• You can't do this in saml20-sp-remote, as SP's are likely used by multiple underlying authentication mechanisms, which likely return quite different raw attributes (hence the need for authprocs to make it more transparent to the SP which underlying authenication mechanism was used).
• The only other option currently would be the global config.php. Yes, you could do this with pre-conditional filters, but it is nothing like as clean, configuration wise (and more prone to error) as just adding support for authsources.php authprocs for IdP.

[tvdijen]

This is hackish and I don't agree with it

[nathanjrobertson]

The "hackishness" of this particular part of this PR is fixed in #2003

[tvdijen]

As per Slack discussion, I find this a hackish solution for an issue that I don't understand.

[pradtke]

So my IdP's authsource is an SP. Won't this change result in the authsource authprocs running twice? once when the SP authsource is used for login, and a second time when run inside of the idp. That would be a big surprise to people, especially since not all filters can be run in both places.

A second reason doing this "somewhere else" may be the desired location is that the IdP class is just one way of integrating with apps. There is an OIDC module, a casserver module and adfs module. One challenge in enhancing only the SAML IdP side is when people go to use the other modules they get a different behavior. e.g. you add an OIDC app and now the authproc filters configured this new way don't run, even though the authsource is the same.

[nathanjrobertson]

The second point I understand and agree with. It must apply to all backends, not just SAML - I hadn't considered ADFS or OIDC. Thanks for the feedback.

WRT the first point, I think I understand what you mean - the case where we're bridging (https://simplesamlphp.org/docs/stable/simplesamlphp-advancedfeatures.html#bridging-between-protocols)? In that case we're running as both an SP and an IdP, and the "authproc" could be run on the SP side then re-run on the IdP side. Perhaps adding an extra key authproc_idp might be acceptable to avoid that clash but still allow IdP authprocs in authsources.php?

[nathanjrobertson]

As per Slack discussion, I find this a hackish solution for an issue that I don't understand.

This issue arises when using multiauth for authentication. By definition, for a common set of SPs it combines together several different authentication and authorization technologies, and as a result what each auth/auth backend returns as attributes is different. We need to transform the attributes that each returns into what the SPs are expecting - that is the role of authproc - post processing of attributes returned by any number of incompatible authentication plugins to be in a format expected by the SPs.

A sensible approach to authprocs in configuration here would be to have a set of per-authentication plugin (authsource) transforms run first, then after that run the transforms that apply to all authsources. In our case, SQL, LDAP and OAuth2/MSGraph all return quite different attributes.

From a configuration perspective, the cleanest way get per-authsource authprocs is to add support for IdP authprocs in authsources.php. Having a single huge list of rules in config.php with '%precondition' filters set becomes unmaintainable and messy, and the ability to keep your per-authsource transform configuration with the authsource itself makes configuration more maintainable. Restated, per-authsource authproc for each (in authsources.php), followed by common rules applied to all (in config.php) is what I'm looking to do. Because they get merged together, ordered by the integer ID, that is workable as long as the implementer follows a convention for the IDs.

Like I said, I'm fine with reworking the technical aspects. I accept @pradtke's feedback that it's implemented in the wrong place, causing the two issues he identified. Hence, the current code in the PR isn't mergeable. However, the problem definitely exists, and the configuration aspect of the proposed PR I believe makes multiauth useable and is a big step forward for configuring that module. Without it, it's a mess of '%precondition' filters in a global file, which in non-trivial deployments ends up being more difficult to maintain than it needs to be.

It's far tidier configuration wise to store global authprocs in a global place (config.php), per-SP in it's place (SP metadata), and per-authsource authprocs in its place (authsources.php). That final case is only necessary when you have multiple authsources (the multiauth case) for a given SP or set of SPs.

So, I'm happy to put the time into reworking it. The most important thing to me at the moment is that the project is open to the concept I'm presenting and is open reviewing and potentially merging the change once it's technically up to scratch.

[pradtke]

I've found a variety of situations where I want to run authproc filters on things defined in authsources.php that do not involve being a saml IDP at all. For example, we use SSP as a CAS client to Slate. Or the support I added in the authoauth2 module to run authproc filters as part of the authsource.

Maybe a path forward is a generalized way of running authproc filters as part of the authentication? So instead of the SP authsource or the OAuth2 authsource interacting with the processing chain it could be handled by the Auth\Source class, perhaps in Source::completeAuth. I think that would:

• alleviate my concerns about a solution that only worked for SAML IdPs
• make the SP authsource less "special" since currently it is the only one where you can define authproc filters
• perhaps feels less hackish/be something useful outside of the multiauth module use case.

I'm unsure how feasible it is, where the code should live, or what sort of odd behaviors we could run into (e.g. does multiauth run both its own filters and that of the target auth source; do filters run on reauthentication, etc).

[nathanjrobertson]

I've found a variety of situations where I want to run authproc filters on things defined in authsources.php that do not involve being a saml IDP at all. For example, we use SSP as a CAS client to Slate. Or the support I added in the authoauth2 module to run authproc filters as part of the authsource.

Yeah, I saw that a few days ago (https://github.com/cirrusidentity/simplesamlphp-module-authoauth2/blob/924636059d3359f5fa8e07d19c362ce109a2236c/src/Auth/Source/OAuth2.php#L297). It's one of the modules that's in our multiauth setup. (As an aside, I have a subclass of MicrosoftHybridAuth.php which implements MS Graph support for getting attributes using the token returned - if you're open to a PR to add that support, I'll carve that out of our code and submit it to you).

When I read that code I thought "that would be better in the core SSphp", and you'd put it there because of the lack of support for IdP per-authsource authprocs. That code actually convinced me the problem existed (so, partly inspired me to write this PR) - it was obvious it wasn't a problem unique to that authsource, but implemented in authoauth2 to cover up a missing feature in the core SSphp.

In the multiauth case, the authprocs only run for the delegated module, not multiauth itself. The multiauth module itself doesn't authenticate anyone, and doesn't produce any attributes itself - it's job is to find the right authsource to delegate to and hand off to that module. By the time authprocs are run, the authenticating module has been fully delegated to, and that delegated module has no idea it was redirected to in a multiauth way.

I like your suggestion of moving authprocs all together to make them more symmetrical between the IdP and SP cases. As you say, there's no real justification for the SP authsource being more special than an IdP one. Looking forward to @tvdijen's feedback on that idea.