Possibility to add OneLogin config items like Contacts

Looks like a valid feature request to me.

This information is exposed in the SP's metadata XML. So it looks like this is good for IdPs that fetch the metadata and make use of it.

This is not just a custom extension for OneLogin IdPs, given that a ContactPerson is defined in the saml-schema-metadata-2.0.xsd definition. There's more info: the library's Metadata::builder() code has details on what exactly is added to the metadata - which is not exactly all details that are documented in the XSD. And on where to add this into the initial settings array.

I guess there's a case to be made for a 'additional_options' or 'metadata_add' configuration that can be set in settings,php, then merged (or deep-merged, with or without some checks).... combined with a mention in the README on how to use this + "use at your own risk",

Given that the SAML spec only supports specific contactPerson and organization attributes, and that the php-saml library further only supports certain values within those attributes (see https://github.com/SAML-Toolkits/php-saml?tab=readme-ov-file#saml-php-to... ) -- in other words, there's no reason to support *arbitrary* values being passed into the metadata, I propose that we provide a UI for entering technical contact, support contact, and organization information.

MR created.

1)

Thanks for your opinion / the MR. I think it's good that this is implemented by someone who is using the feature.

Given that the SAML spec only supports specific contactPerson and organization attributes, and that the php-saml library further only supports certain values within those attributes

That's true. I think it's remarkable that php-saml only outputs GivenName and not e.g. SurName, but who am I to second guess it...

Also I see there's more than technical and support ContactType, but I'm not going to second guess your implicit assertion that (only) these are the best to implement.

There seems to be a bit of redundant info there though, and I have a small question in the MR too.

2)

I'm not sure about the "attributeConsumingService(s)" thing and whether it belongs here.

Also I see there's more than technical and support ContactType, but I'm not going to second guess your implicit assertion that (only) these are the best to implement.

1. Actually, I don't think I have the depth of experience with the API to assert this, so I think it's best to include configuration options for all 5 available contact types. Done in the latest code changes!

In the theoretical chance that we make this extensible (which should not be that much work, with configuration translation) it fits Drupal better.

2. Maybe the best (most Drupally) thing to do is make this configurable? That's been done in the latest changes.

3. Yes, there was some test code in there that I've removed. Thanks for catching that!

4. Test coverage added, and tests are passed on previous major, current major, and next minor.

Ready for further review!

Thanks for adding tests! (I haven't looked at the exact reason why they fail for next-PHP-version; I think that happened in the main branch too, and it went away somehow. I'm also hoping to clean up the bogus warnings sometime soon.)

I was doubting whether to cram this through right now, because I'm going to do a new release and this is the only outstanding thing. Alas, I feel uneasy about one thing that I don't want to hack now, so it'll have to wait; I'm giving that back as/for feedback.

Thanks for the review, input, and communication about the timeline for this. I'll take a look at the suggestions & rework as I have time. I have no expectation/need for this to be fast-tracked for the upcoming release. Thanks!