From b0ed5aa1afa97916aaa27c87f797457fd917e245 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Lapeyre?= Date: Wed, 13 Sep 2023 18:59:40 +0200 Subject: [PATCH] Add caching support to IDPMetadata() (#102) * Add caching support to IDPMetadata() 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(). * Address code review comments * Run go mod tidy * Run go mod tidy * Update saml/sp_test.go Co-authored-by: Jim --------- Co-authored-by: Jim --- saml/authn_request.go | 2 + saml/go.mod | 1 + saml/go.sum | 34 ++++- saml/models/metadata/duration.go | 22 ++++ saml/models/metadata/duration_test.go | 56 +++++++++ saml/models/metadata/entity_descriptor.go | 6 +- saml/sp.go | 106 +++++++++++++++- saml/sp_test.go | 143 ++++++++++++++++++++++ 8 files changed, 362 insertions(+), 8 deletions(-) create mode 100644 saml/models/metadata/duration.go create mode 100644 saml/models/metadata/duration_test.go diff --git a/saml/authn_request.go b/saml/authn_request.go index 3873002..acfb4ff 100644 --- a/saml/authn_request.go +++ b/saml/authn_request.go @@ -125,6 +125,8 @@ func WithClock(clock clockwork.Clock) Option { opts.clock = clock case *parseResponseOptions: opts.clock = clock + case *idpMetadataOptions: + opts.clock = clock } } } diff --git a/saml/go.mod b/saml/go.mod index 091fdf2..684679b 100644 --- a/saml/go.mod +++ b/saml/go.mod @@ -5,6 +5,7 @@ go 1.19 require ( github.com/beevik/etree v1.2.0 github.com/crewjam/go-xmlsec v0.0.0-20200414151428-d2b1a58f7262 + github.com/crewjam/saml v0.4.13 github.com/hashicorp/go-uuid v1.0.3 github.com/jonboulle/clockwork v0.4.0 github.com/russellhaering/gosaml2 v0.9.1 diff --git a/saml/go.sum b/saml/go.sum index 664a69d..003b56e 100644 --- a/saml/go.sum +++ b/saml/go.sum @@ -6,9 +6,17 @@ github.com/crewjam/errset v0.0.0-20160219153700-f78d65de925c h1:dCJ9oZ0VgnzJHR5B github.com/crewjam/errset v0.0.0-20160219153700-f78d65de925c/go.mod h1:XhiWL7J86xoqJ8+x2OA+AM2l9skQP2DZ0UOXQYVg7uI= github.com/crewjam/go-xmlsec v0.0.0-20200414151428-d2b1a58f7262 h1:3V8RSsB1mxeAfxMb7lGSd0HlCHhc/ElJj1peaJMAkyk= github.com/crewjam/go-xmlsec v0.0.0-20200414151428-d2b1a58f7262/go.mod h1:M9eHnKpImgRwzOFdlFQnbgJRqFwW/eX1cKAVobv03uE= +github.com/crewjam/httperr v0.2.0/go.mod h1:Jlz+Sg/XqBQhyMjdDiC+GNNRzZTD7x39Gu3pglZ5oH4= +github.com/crewjam/saml v0.4.13 h1:TYHggH/hwP7eArqiXSJUvtOPNzQDyQ7vwmwEqlFWhMc= +github.com/crewjam/saml v0.4.13/go.mod h1:igEejV+fihTIlHXYP8zOec3V5A8y3lws5bQBFsTm4gA= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/dchest/uniuri v1.2.0/go.mod h1:fSzm4SLHzNZvWLvWJew423PhAzkpNQYq+uNLq4kxhkY= +github.com/golang-jwt/jwt/v4 v4.4.3 h1:Hxl6lhQFj4AnOX6MLrsCb/+7tCj7DxP7VA+2rDIq5AU= +github.com/golang-jwt/jwt/v4 v4.4.3/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= +github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= +github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/hashicorp/go-uuid v1.0.3 h1:2gKiV6YVmrJ1i2CKKa9obLvRieoRGviZFL26PcT/Co8= github.com/hashicorp/go-uuid v1.0.3/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/jonboulle/clockwork v0.2.2/go.mod h1:Pkfl5aHPm1nk2H9h0bjmnJD/BcgbGXUBGnn1kMkgxc8= @@ -17,8 +25,9 @@ github.com/jonboulle/clockwork v0.4.0 h1:p4Cf1aMWXnXAUh8lVfewRBx1zaTSYKrKMF2g3ST github.com/jonboulle/clockwork v0.4.0/go.mod h1:xgRqUGwRcjKCO1vbZUEtSLrqKoPSsUpK7fnezOII0kc= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= -github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0= github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk= +github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= @@ -28,30 +37,51 @@ github.com/mattermost/xml-roundtrip-validator v0.1.0/go.mod h1:qccnGMcpgwcNaBnxq github.com/moov-io/signedxml v1.1.1 h1:TQ2fK4DRCYv7agH+z6RjtnBTmEyYMAztFzuHIPtUJpg= github.com/moov-io/signedxml v1.1.1/go.mod h1:p+b4f/Wo/qKyew8fHW8VZOgsILWylyvvjdE68egzbwc= github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= +github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= -github.com/rogpeppe/go-internal v1.8.0 h1:FCbCCtXNOY3UtUuHUYaghJg4y7Fd14rXifAYUAtL9R8= github.com/rogpeppe/go-internal v1.8.0/go.mod h1:WmiCO8CzOY8rg0OYDC4/i/2WRWAB6poM+XZ2dLUbcbE= +github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8= +github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= github.com/russellhaering/gosaml2 v0.9.1 h1:H/whrl8NuSoxyW46Ww5lKPskm+5K+qYLw9afqJ/Zef0= github.com/russellhaering/gosaml2 v0.9.1/go.mod h1:ja+qgbayxm+0mxBRLMSUuX3COqy+sb0RRhIGun/W2kc= +github.com/russellhaering/goxmldsig v1.2.0/go.mod h1:gM4MDENBQf7M+V824SGfyIUVFWydB7n0KkEubVJl+Tw= github.com/russellhaering/goxmldsig v1.3.0/go.mod h1:gM4MDENBQf7M+V824SGfyIUVFWydB7n0KkEubVJl+Tw= github.com/russellhaering/goxmldsig v1.4.0 h1:8UcDh/xGyQiyrW+Fq5t8f+l2DLB1+zlhYzkPUJ7Qhys= github.com/russellhaering/goxmldsig v1.4.0/go.mod h1:gM4MDENBQf7M+V824SGfyIUVFWydB7n0KkEubVJl+Tw= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/zenazn/goji v1.0.1/go.mod h1:7S9M489iMyHBNxwZnk9/EHS098H4/F6TATF2mIxtB1Q= +golang.org/x/crypto v0.0.0-20220128200615-198e4374d7ed/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.10.0 h1:LKqV2xt9+kDzSTfOhx4FrkEBcMrAgHSYgzywV9zcGmM= golang.org/x/crypto v0.10.0/go.mod h1:o4eNf7Ede1fv+hwOwZsTHl9EsPFO6q6ZvYR8vYfY45I= +golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= +golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= +golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= +gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo= +gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw= diff --git a/saml/models/metadata/duration.go b/saml/models/metadata/duration.go new file mode 100644 index 0000000..f28aa35 --- /dev/null +++ b/saml/models/metadata/duration.go @@ -0,0 +1,22 @@ +package metadata + +import ( + "time" + + crewjamSaml "github.com/crewjam/saml" +) + +// Duration is a time.Duration that uses the xsd:duration format for text +// marshalling and unmarshalling. +type Duration time.Duration + +// MarshalText implements the encoding.TextMarshaler interface. +func (d Duration) MarshalText() ([]byte, error) { + return crewjamSaml.Duration(d).MarshalText() +} + +// UnmarshalText implements the encoding.TextUnmarshaler interface. +func (d *Duration) UnmarshalText(text []byte) error { + cp := (*crewjamSaml.Duration)(d) + return cp.UnmarshalText(text) +} diff --git a/saml/models/metadata/duration_test.go b/saml/models/metadata/duration_test.go new file mode 100644 index 0000000..9ae0853 --- /dev/null +++ b/saml/models/metadata/duration_test.go @@ -0,0 +1,56 @@ +package metadata + +import ( + "errors" + "strconv" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +var durationMarshalTests = []struct { + in time.Duration + expected []byte +}{ + {0, nil}, + {time.Hour, []byte("PT1H")}, + {-time.Hour, []byte("-PT1H")}, +} + +func TestDuration(t *testing.T) { + for i, testCase := range durationMarshalTests { + t.Run(strconv.Itoa(i), func(t *testing.T) { + actual, err := Duration(testCase.in).MarshalText() + require.NoError(t, err) + require.Equal(t, testCase.expected, actual) + }) + } +} + +var durationUnmarshalTests = []struct { + in []byte + expected time.Duration + err error +}{ + {nil, 0, nil}, + {[]byte("-PT1H"), -time.Hour, nil}, + {[]byte("P1D"), 24 * time.Hour, nil}, + {[]byte("P1M"), 720 * time.Hour, nil}, + {[]byte("PT1.S"), 0, errors.New("invalid duration (PT1.S)")}, +} + +func TestDurationUnmarshal(t *testing.T) { + for i, testCase := range durationUnmarshalTests { + t.Run(strconv.Itoa(i), func(t *testing.T) { + var actual Duration + err := actual.UnmarshalText(testCase.in) + if testCase.err == nil { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, testCase.err.Error()) + } + require.Equal(t, Duration(testCase.expected), actual) + }) + } +} diff --git a/saml/models/metadata/entity_descriptor.go b/saml/models/metadata/entity_descriptor.go index 0e8bb64..8ac8985 100644 --- a/saml/models/metadata/entity_descriptor.go +++ b/saml/models/metadata/entity_descriptor.go @@ -37,9 +37,9 @@ const ( // DescriptorCommon defines common fields used in Entity- and EntitiesDescriptor. type DescriptorCommon struct { - ID string `xml:",attr,omitempty"` - ValidUntil *time.Time `xml:"validUntil,attr,omitempty"` - CacheDuration time.Duration `xml:"cacheDuration,attr,omitempty"` + ID string `xml:",attr,omitempty"` + ValidUntil *time.Time `xml:"validUntil,attr,omitempty"` + CacheDuration *Duration `xml:"cacheDuration,attr,omitempty"` Signature *dsig.Signature } diff --git a/saml/sp.go b/saml/sp.go index 4b38b23..957402c 100644 --- a/saml/sp.go +++ b/saml/sp.go @@ -8,9 +8,12 @@ import ( "io" "net/http" "net/url" + "sync" + "time" "github.com/hashicorp/cap/saml/models/core" "github.com/hashicorp/cap/saml/models/metadata" + "github.com/jonboulle/clockwork" dsig "github.com/russellhaering/goxmldsig/types" ) @@ -81,6 +84,10 @@ func WithAdditionalACSEndpoint(b core.ServiceBinding, location url.URL) Option { // ServiceProvider defines a type for service providers type ServiceProvider struct { cfg *Config + + metadata *metadata.EntityDescriptorIDPSSO + metadataCachedUntil *time.Time + metadataLock sync.Mutex } // NewServiceProvider creates a new ServiceProvider. @@ -156,21 +163,103 @@ func (sp *ServiceProvider) CreateMetadata(opt ...Option) *metadata.EntityDescrip return &spsso } +type idpMetadataOptions struct { + cache bool + useStale bool + clock clockwork.Clock +} + +func idpMetadataOptionsDefault() idpMetadataOptions { + return idpMetadataOptions{ + cache: true, + useStale: false, + clock: clockwork.NewRealClock(), + } +} + +func getIDPMetadataOptions(opt ...Option) idpMetadataOptions { + opts := idpMetadataOptionsDefault() + ApplyOpts(&opts, opt...) + return opts +} + +// WithCache control whether we should cache IDP Metadata. +func WithCache(cache bool) Option { + return func(o interface{}) { + if o, ok := o.(*idpMetadataOptions); ok { + o.cache = cache + } + } +} + +// WithStale control whether we should use a stale IDP Metadata document if +// refreshing it fails. +func WithStale(stale bool) Option { + return func(o interface{}) { + if o, ok := o.(*idpMetadataOptions); ok { + o.useStale = stale + } + } +} + // IDPMetadata fetches the metadata XML document from the configured identity provider. -func (sp *ServiceProvider) IDPMetadata() (*metadata.EntityDescriptorIDPSSO, error) { +// Options: +// - WithClock +// - WithCache +// - WithStale +func (sp *ServiceProvider) IDPMetadata(opt ...Option) (*metadata.EntityDescriptorIDPSSO, error) { const op = "saml.ServiceProvider.FetchIDPMetadata" + opts := getIDPMetadataOptions(opt...) + var err error var ed *metadata.EntityDescriptorIDPSSO + isValid := func(md *metadata.EntityDescriptorIDPSSO) bool { + if md == nil { + return false + } + if md.ValidUntil == nil { + return true + } + return opts.clock.Now().Before(*md.ValidUntil) + } + + isAlive := func(md *metadata.EntityDescriptorIDPSSO, expireAt *time.Time) bool { + if md == nil || !opts.cache || expireAt == nil { + return false + } + + return opts.clock.Now().Before(*expireAt) + } + + if opts.cache { + // We only take the lock when caching is enabled so that requests can be + // done concurrently when it is not + sp.metadataLock.Lock() + defer sp.metadataLock.Unlock() + + switch { + case !isValid(sp.metadata): + sp.metadata = nil + sp.metadataCachedUntil = nil + case isValid(sp.metadata) && isAlive(sp.metadata, sp.metadataCachedUntil): + return sp.metadata, nil + } + } + // Order of switch case determines IDP metadata config precedence switch { case sp.cfg.MetadataURL != "": ed, err = fetchIDPMetadata(sp.cfg.MetadataURL) - if err != nil { + switch { + case err != nil && opts.useStale && isValid(sp.metadata): + // An error occurred but we have a cached metadata document that + // we can use + return sp.metadata, nil + case err != nil: return nil, fmt.Errorf("%s: %w", op, err) } - return ed, nil case sp.cfg.MetadataXML != "": ed, err = parseIDPMetadata([]byte(sp.cfg.MetadataXML)) @@ -188,6 +277,17 @@ func (sp *ServiceProvider) IDPMetadata() (*metadata.EntityDescriptorIDPSSO, erro return nil, fmt.Errorf("%s: no IDP metadata configuration set: %w", op, ErrInvalidParameter) } + if !isValid(ed) { + return nil, fmt.Errorf("the IDP configuration was only valid until %s", ed.ValidUntil.Format(time.RFC3339)) + } + + sp.metadata = ed + sp.metadataCachedUntil = nil + if sp.metadata.CacheDuration != nil { + cachedUntil := opts.clock.Now().Add(time.Duration(*sp.metadata.CacheDuration)) + sp.metadataCachedUntil = &cachedUntil + } + return ed, err } diff --git a/saml/sp_test.go b/saml/sp_test.go index f27939b..9c74bba 100644 --- a/saml/sp_test.go +++ b/saml/sp_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/jonboulle/clockwork" "github.com/stretchr/testify/require" "github.com/hashicorp/cap/saml" @@ -118,6 +119,118 @@ func Test_ServiceProvider_FetchMetadata_ErrorCases(t *testing.T) { } } +func Test_ServiceProvider_FetchMetadata_Cache(t *testing.T) { + type testServer struct { + fail bool + failOnRefresh bool + } + + newTestServer := func(t *testing.T, failOnRefresh bool) string { + t.Helper() + + ts := &testServer{false, failOnRefresh} + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if !ts.fail { + w.Write([]byte(exampleIDPSSODescriptorX)) + } + ts.fail = ts.fail || ts.failOnRefresh + })) + t.Cleanup(s.Close) + + return s.URL + } + + cases := []struct { + name string + newTime string + shouldBeCached bool + opts []saml.Option + failOnRefresh bool + expectErrorOnRefresh bool + }{ + { + name: "is cached", + shouldBeCached: true, + }, + { + name: "cache is disabled", + opts: []saml.Option{saml.WithCache(false)}, + shouldBeCached: false, + }, + { + name: "stale cached document should not be used", + newTime: "2017-07-26", + shouldBeCached: false, + }, + { + name: "is not cached once validUntil is reached", + newTime: "2018-07-25", + expectErrorOnRefresh: true, + }, + { + name: "a stale document should not be used if refreshing fails", + newTime: "2017-07-26", + failOnRefresh: true, + expectErrorOnRefresh: true, + }, + { + name: "use stale document", + opts: []saml.Option{saml.WithStale(true)}, + newTime: "2017-07-26", + failOnRefresh: true, + shouldBeCached: true, + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + r := require.New(t) + + url := newTestServer(t, tt.failOnRefresh) + metaURL := fmt.Sprintf("%s/saml/metadata", url) + cfg, err := saml.NewConfig( + metaURL, + metaURL, + metaURL, + ) + r.NoError(err) + + provider, err := saml.NewServiceProvider(cfg) + r.NoError(err) + + newTime, err := time.Parse("2006-01-02", "2017-07-25") + r.NoError(err) + + opts := append([]saml.Option{saml.WithClock(clockwork.NewFakeClockAt(newTime))}, tt.opts...) + + got1, err := provider.IDPMetadata(opts...) + r.NoError(err) + r.NotNil(got1) + + if tt.newTime != "" { + newTime, err = time.Parse("2006-01-02", tt.newTime) + r.NoError(err) + opts = append(opts, saml.WithClock(clockwork.NewFakeClockAt(newTime))) + } + + got2, err := provider.IDPMetadata(opts...) + if tt.expectErrorOnRefresh { + r.Error(err) + return + } + r.NoError(err) + r.NotNil(got2) + + if tt.shouldBeCached { + r.True(got1 == got2) + } else { + r.False(got1 == got2) + } + }) + } +} + func Test_ServiceProvider_CreateMetadata(t *testing.T) { t.Parallel() r := require.New(t) @@ -136,6 +249,7 @@ func Test_ServiceProvider_CreateMetadata(t *testing.T) { acs, meta, ) + r.NoError(err) cfg.ValidUntil = validUntil @@ -282,3 +396,32 @@ func Test_CreateMetadata_Options(t *testing.T) { ) }) } + +var exampleIDPSSODescriptorX = ` + + + + + + + https://registrar.example.net/category/self-certified + + + + + + ... + ... + https://www.example.info/ + + + SAML Technical Support + mailto:technical-support@example.info + + +`