Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add caching support to IDPMetadata() #102

Merged
merged 6 commits into from
Sep 13, 2023

Conversation

remilapeyre
Copy link
Contributor

@remilapeyre remilapeyre commented Sep 9, 2023

Caching the metadata document will avoid an additional round-trip to the IDP for every connection.

The Metadata for the OASIS Security Assertion Markup Language says regarding caching:

4.3 Post-Processing of Metadata
The following sections describe the post-processing of metadata.

4.3.1 Metadata Instance Caching
[E94] Document caching MUST be based on the duration indicated by the cacheDuration attribute of
the subject element(s). If metadata elements have parent elements which contain caching policies, the
parent element takes precedence. To properly process the cacheDuration attribute, consumers must
retain the date and time when an instance was obtained.

Note that cache expiration does not imply a lack of validity in the absence of a validUntil attribute or
other information; failure to update a cached instance (e.g., due to network failure) need not render
metadata invalid, although implementations may offer such controls to deployers.
When a document or element has expired, the consumer MUST retrieve a fresh copy, which may require
a refresh of the document location(s). Consumers SHOULD process document cache processing
according to [RFC2616] Section 13, and MAY request the Last-Modified date and time from the HTTP
server. Publishers SHOULD ensure acceptable cache processing as described in [RFC2616] (Section
10.3.5 304 Not Modified).

4.3.2 [E94] Metadata Instance Validity
Metadata MUST be considered invalid upon reaching the time specified in a validUntil attribute of the
subject element(s). The effective expiration may be adjusted downward by parent element(s) with earlier
expirations. Invalid metadata MUST NOT be used. This contrasts with "stale" metadata that may be
beyond its optimum cache duration but is not explicitly invalid. Such metadata remains valid and MAY be
used at the discretion of the implementation.

With this change the cached metadata is used until it expires. This behavior can be disabled using WithCache().

Using a stale document when refreshing it fails is disabled by default and users can opt-in using WithStale().

This patch also fixes parsing the cacheDuration attribute by using the github.com/crewjam/saml library.

Caching the metadata document will avoid an additional round-trip to the
IDP for every connection.

The Metadata for the OASIS Security Assertion Markup Language says
regarding caching:

	4.3 Post-Processing of Metadata
	The following sections describe the post-processing of metadata.

	4.3.1 Metadata Instance Caching
	[E94] Document caching MUST be based on the duration indicated by the cacheDuration attribute of
	the subject element(s). If metadata elements have parent elements which contain caching policies, the
	parent element takes precedence. To properly process the cacheDuration attribute, consumers must
	retain the date and time when an instance was obtained.

	Note that cache expiration does not imply a lack of validity in the absence of a validUntil attribute or
	other information; failure to update a cached instance (e.g., due to network failure) need not render
	metadata invalid, although implementations may offer such controls to deployers.
	When a document or element has expired, the consumer MUST retrieve a fresh copy, which may require
	a refresh of the document location(s). Consumers SHOULD process document cache processing
	according to [RFC2616] Section 13, and MAY request the Last-Modified date and time from the HTTP
	server. Publishers SHOULD ensure acceptable cache processing as described in [RFC2616] (Section
	10.3.5 304 Not Modified).

	4.3.2 [E94] Metadata Instance Validity
	Metadata MUST be considered invalid upon reaching the time specified in a validUntil attribute of the
	subject element(s). The effective expiration may be adjusted downward by parent element(s) with earlier
	expirations. Invalid metadata MUST NOT be used. This contrasts with "stale" metadata that may be
	beyond its optimum cache duration but is not explicitly invalid. Such metadata remains valid and MAY be
	used at the discretion of the implementation.

With this change the cached metadata is used until it expires. This behavior
can be disabled using WithCache().

Using a stale document when refreshing it fails is disabled by default
and users can opt-in using WithStale().
saml/sp_test.go Outdated Show resolved Hide resolved
saml/models/metadata/duration.go Show resolved Hide resolved
saml/sp.go Outdated Show resolved Hide resolved
saml/sp.go Outdated Show resolved Hide resolved
saml/sp.go Outdated Show resolved Hide resolved
saml/sp_test.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@jimlambrt jimlambrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! One super minor comment.

Co-authored-by: Jim <jlambert@hashicorp.com>
@austingebauer austingebauer self-requested a review September 11, 2023 17:00
Copy link
Contributor

@austingebauer austingebauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @remilapeyre!

@austingebauer
Copy link
Contributor

Going to merge this. Can address any additional comments in follow up PRs.

@austingebauer austingebauer merged commit b0ed5aa into hashicorp:saml-lib Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants