SAML Metadata parsing does not correctly parse valid duration values.

[lwjameson]

Bug report

• [x] I confirm this is a bug with Supabase, not with my own application.
• [x] I confirm I have searched the Docs, GitHub Discussions, and Discord.

Describe the bug

We have a customer that we are setting up with SAML authentication. Their metadata EntitiesDescriptor contains the following:

(Sanitized) <md:EntitiesDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" xmlns:ds="http://www.w3.org/2000/09/xmldsig#" xmlns:mdui="urn:oasis:names:tc:SAML:metadata:ui" xmlns:mdattr="urn:oasis:names:tc:SAML:metadata:attribute" xmlns:mdrpi="urn:oasis:names:tc:SAML:metadata:rpi" xmlns:shibmd="urn:mace:shibboleth:metadata:1.0" xmlns:xrd="http://docs.oasis-open.org/ns/xri/xrd-1.0" ....snip... validUntil="2024-08-09T23:11:46Z" cacheDuration="PT5H">

When adding this provider via supabase sso add we receive the following error:

Unexpected error adding identity provider: {"message":"Unexpected failure, please check server logs for more information"}

The server logs show: (Sanitized)

{"component":"api","error":"strconv.ParseInt: parsing \"PT5H\": invalid syntax","level":"error","method":"POST","msg":"Unhandled server error: strconv.ParseInt: parsing \"PT5H\": invalid syntax","path":"/admin/sso/providers","referer":"https://<removed>","remote_addr":"3.95.37.194","request_id":"8abf1e44f73505f5-IAD","time":"2024-07-31T16:55:49Z"}

According to the spec:

https://docs.oasis-open.org/security/saml/v2.0/saml-schema-metadata-2.0.xsd

This is the cacheDuration schema:

<attribute name="cacheDuration" type="duration" use="optional"/>

According to the XML schema:

https://www.w3.org/TR/xmlschema-2/#duration

3.2.6.1 Lexical representation The lexical representation for duration is the [[ISO 8601]](https://www.w3.org/TR/xmlschema-2/#ISO8601) extended format PnYn MnDTnH nMnS, where nY represents the number of years, nM the number of months, nD the number of days, 'T' is the date/time separator, nH the number of hours, nM the number of minutes and nS the number of seconds. The number of seconds can include decimal digits to arbitrary precision.

This is a valid way of specifying a duration.

To Reproduce

1. Create an XML metadata file that uses Lexical representation to specify cacheDuration
2. Attempt to add this to an instance using supabase sso add
3. Get error

Expected behavior

Valid SAML metadata is processed successfully.

[lwjameson]

Hi SB Auth. This is a real bug and needs to be addressed. We have a paying customer who cannot access their site.

[kangmingtay]

hi @lwjameson, can you please open a ticket with us at https://supabase.help and link this issue in the ticket so we can investigate further?

[hf]

Looks like this is a bug in the underlying library, the EntityDescriptor type correctly parses the Duration but not the EntitiesDescriptor (plural!): https://github.com/crewjam/saml/blob/main/metadata.go#L30-L103

Will take a while to fix given that the library has been known to slowly take on fixes.

This is because we use this method to parse the SAML Metadata: https://github.com/crewjam/saml/blob/bbccb7933d5f60512ebc6caec7120c604581983d/samlsp/fetch_metadata.go#L36

[hf]

Proposed a fix: https://github.com/crewjam/saml/pull/575

[hf]

If we don't see any movement on this in the library, the only option we have is to essentially fork.