simplesamlphp / simplesamlphp-module-metarefresh

The metarefresh module will download and parse metadata documents and store them locally
GNU Lesser General Public License v2.1
7 stars 13 forks source link

Allow Cron outputFormat to be a "PDO" type #24

Closed tamil-rss closed 1 year ago

tamil-rss commented 1 year ago

Problem

Currently, SimpleSAML Metarefresh module supports only flatfile and serialize output formats for the cron. Would like to have PDO output format to have the metadata upserted in the database.

We are planning to use PDO type for storing metadata and have it managed from one of our own internal services. We are able to achieve this for statically stored metadata. But for the cron, PDO isn't supported as an outputFormat.

I wonder why this feature is missing in this module, since PDO is one of key SimpleSAMLphp metadata handler or is there any specific reasons to avoid it. Really curious to know!!

If no specific reason, can this be taken forward as an enhancement? Also, I have some draft changes and happy to collaborate!!

Proposed Solution

It would be great to have a method that writes a metadata entry to database named as writeMetadataPdo in MetaLoader.php and use the addEntry method from MetaDataStorageHandlerPdo.php. There needs a mechanism for removing expired entry too.

tvdijen commented 1 year ago

I wonder why this feature is missing in this module, since PDO is one of key SimpleSAMLphp metadata handler or is there any specific reasons to avoid it. Really curious to know!!

This module is as old as time itself.. I don't think there is any particular reason. I'd be happy to take your contribution!

tamil-rss commented 1 year ago

Thank you for the quick response! I tried pushing my commit on to a feature branch but throws 403 due to insufficient permissions. Can you please guide me on the next steps ?

thijskh commented 1 year ago

Hi! You fork the repository first. Then you create the branch in your own fork. From there you can create a pull request. Here's a step-by-step tutorial