refactor samlsp package to be more modular (crewjam#230)

This change splits the Middleware component into three interfaces (in addition to the saml.ServiceProvider):

* RequestTracker which handles tracking pending authn requests.
* Session which handles issuing session cookies.
* OnError which handles reporting errors to both the user and any logging.

The default implementations of RequestTracker and Session use http cookie to encode JWTs, but further delegate the encoding/verification of the JWTs to codec interfaces which allow further customization, again with default implementations.

This should make the *samlsp* package easier to extend to fit more use cases.

Fixes crewjam#231

README.md

@@ -1,4 +1,5 @@
 # SAML
+
 [![](https://godoc.org/github.com/crewjam/saml?status.svg)](http://godoc.org/github.com/crewjam/saml)
 
 [![Build Status](https://travis-ci.org/crewjam/saml.svg?branch=master)](https://travis-ci.org/crewjam/saml)
@@ -12,30 +13,82 @@ In SAML parlance an **Identity Provider** (IDP) is a service that knows how to a
 
 The core package contains the implementation of SAML. The package samlsp provides helper middleware suitable for use in Service Provider applications. The package samlidp provides a rudimentary IDP service that is useful for testing or as a starting point for other integrations.
 
-## Breaking Changes 
+## Breaking Changes
+
+Version 0.4.0 introduces a few breaking changes to the _samlsp_ package in order to make the package more extensible, and to clean up the interfaces a bit. The default behavior remains the same, but you can now provide interface implementations of _RequestTracker_ (which tracks pending requests), _Session_ (which handles maintaining a session) and _OnError_ which handles reporting errors.
+
+Public fields of _samlsp.Middleware_ have changed, so some usages may require adjustment. See [issue 231](https://github.com/crewjam/saml/issues/231) for details.
+
+The option to provide an IDP metadata **URL** has been deprecated. Instead, we recommend that you use the `FetchMetadata()` function, or fetch the metadata yourself and use the new `ParseMetadata()` function, and pass the metadata in _samlsp.Options.IDPMetadata_.
+
+Similarly, the _HTTPClient_ field is now deprecated because it was only used for fetching metdata, which is no longer directly implemented.
+
+The fields that manage how cookies are set are deprecated as well. To customize how cookies are managed, provide custom implementation of _RequestTracker_ and/or _Session_, perhaps by extending the default implementations.
+
+The deprecated fields have not been removed from the Options structure,
+
+don't need it any more other )
+
+We have
+
+In particular we have deprecated the following fields in
+_samlsp.Options_:
+
+- _Logger_ -- this was used to emit errors while
+
+IDPMetadataURL *url.URL // DEPRECATED: this field will be removed, instead use FetchMetadata
+HTTPClient *http.Client // DEPRECATED: this field will be removed, instead pass httpClient to FetchMetadata
+CookieMaxAge time.Duration // DEPRECATED: this field will be removed. Instead, assign a custom CookieRequestTracker or CookieSessionProvider
+CookieName string // DEPRECATED: this field will be removed. Instead, assign a custom CookieRequestTracker or CookieSessionProvider
+CookieDomain string // DEPRECATED: this field will be removed. Instead, assign a custom CookieRequestTracker or CookieSessionProvider
+CookieSecure
+
+URL url.URL
+Key *rsa.PrivateKey
+Certificate *x509.Certificate
+Intermediates []*x509.Certificate
+AllowIDPInitiated bool
+IDPMetadata *saml.EntityDescriptor
+ForceAuthn bool // TODO(ross): this should be \*bool
+
+    URL               url.URL
+    Key               *rsa.PrivateKey
+    Logger            logger.Interface
+    Certificate       *x509.Certificate
+    Intermediates     []*x509.Certificate
+    AllowIDPInitiated bool
+    IDPMetadata       *saml.EntityDescriptor
+    IDPMetadataURL    *url.URL
+    HTTPClient        *http.Client
+    CookieMaxAge      time.Duration
+    CookieName        string
+    CookieDomain      string
+    CookieSecure      bool
+    ForceAuthn        bool
 
 Note: between version 0.2.0 and the current master include changes to the API
 that will break your existing code a little.
 
 This change turned some fields from pointers to a single optional struct into
 the more correct slice of struct, and to pluralize the field name. For example,
-`IDPSSODescriptor *IDPSSODescriptor` has become 
-`IDPSSODescriptors []IDPSSODescriptor`. This more accurately reflects the 
+`IDPSSODescriptor *IDPSSODescriptor` has become
+`IDPSSODescriptors []IDPSSODescriptor`. This more accurately reflects the
 standard.
 
-The struct `Metadata` has been renamed to `EntityDescriptor`. In 0.2.0 and before, 
-every struct derived from the standard has the same name as in the standard, 
-*except* for `Metadata` which should always have been called `EntityDescriptor`. 
+The struct `Metadata` has been renamed to `EntityDescriptor`. In 0.2.0 and before,
+every struct derived from the standard has the same name as in the standard,
+_except_ for `Metadata` which should always have been called `EntityDescriptor`.
 
 In various places `url.URL` is now used where `string` was used <= version 0.1.0.
 
-In various places where keys and certificates were modeled as `string` 
-<= version 0.1.0 (what was I thinking?!) they are now modeled as 
+In various places where keys and certificates were modeled as `string`
+<= version 0.1.0 (what was I thinking?!) they are now modeled as
 `*rsa.PrivateKey`, `*x509.Certificate`, or `crypto.PrivateKey` as appropriate.
 
 ## Getting Started as a Service Provider
 
 Let us assume we have a simple web application to protect. We'll modify this application so it uses SAML to authenticate users.
+
 ```golang
 package main
 
@@ -54,6 +107,7 @@ func main() {
     http.ListenAndServe(":8000", nil)
 }
 ```
+
 Each service provider must have an self-signed X.509 key pair established. You can generate your own with something like this:
 
     openssl req -x509 -newkey rsa:2048 -keyout myservice.key -out myservice.cert -days 365 -nodes -subj "/CN=myservice.example.com"
@@ -146,9 +200,9 @@ The package can produce signed SAML assertions, and can validate both signed and
 
 ## RelayState
 
-The *RelayState* parameter allows you to pass user state information across the authentication flow. The most common use for this is to allow a user to request a deep link into your site, be redirected through the SAML login flow, and upon successful completion, be directed to the originally requested link, rather than the root.
+The _RelayState_ parameter allows you to pass user state information across the authentication flow. The most common use for this is to allow a user to request a deep link into your site, be redirected through the SAML login flow, and upon successful completion, be directed to the originally requested link, rather than the root.
 
-Unfortunately, *RelayState* is less useful than it could be. Firstly, it is **not** authenticated, so anything you supply must be signed to avoid XSS or CSRF. Secondly, it is limited to 80 bytes in length, which precludes signing. (See section 3.6.3.1 of SAMLProfiles.)
+Unfortunately, _RelayState_ is less useful than it could be. Firstly, it is **not** authenticated, so anything you supply must be signed to avoid XSS or CSRF. Secondly, it is limited to 80 bytes in length, which precludes signing. (See section 3.6.3.1 of SAMLProfiles.)
 
 ## References
 

example/service.go

@@ -3,6 +3,7 @@ package main
 
 import (
 	"bytes"
+	"context"
 	"crypto/rsa"
 	"crypto/tls"
 	"crypto/x509"
@@ -18,7 +19,6 @@ import (
 	"github.com/zenazn/goji"
 	"github.com/zenazn/goji/web"
 
-	"github.com/crewjam/saml/logger"
 	"github.com/crewjam/saml/samlsp"
 )
 
@@ -100,7 +100,6 @@ OwJlNCASPZRH/JmF8tX0hoHuAQ==
 )
 
 func main() {
-	logr := logger.DefaultLogger
 	rootURLstr := flag.String("url", "https://962766ce.ngrok.io", "The base URL of this service")
 	idpMetadataURLstr := flag.String("idp", "https://516becc2.ngrok.io/metadata", "The metadata URL for the IDP")
 	flag.Parse()
@@ -119,6 +118,12 @@ func main() {
 		panic(err) // TODO handle error
 	}
 
+	idpMetadata, err := samlsp.FetchMetadata(context.Background(), http.DefaultClient,
+		*idpMetadataURL)
+	if err != nil {
+		panic(err) // TODO handle error
+	}
+
 	rootURL, err := url.Parse(*rootURLstr)
 	if err != nil {
 		panic(err) // TODO handle error
@@ -127,13 +132,12 @@ func main() {
 	samlSP, err := samlsp.New(samlsp.Options{
 		URL:               *rootURL,
 		Key:               keyPair.PrivateKey.(*rsa.PrivateKey),
-		Logger:            logr,
 		Certificate:       keyPair.Leaf,
 		AllowIDPInitiated: true,
-		IDPMetadataURL:    idpMetadataURL,
+		IDPMetadata:       idpMetadata,
 	})
 	if err != nil {
-		logr.Fatalf("%s", err)
+		panic(err) // TODO handle error
 	}
 
 	// register with the service provider

example/trivial/trivial.go

@@ -1,20 +1,19 @@
 package main
 
 import (
+	"context"
+	"crypto/rsa"
+	"crypto/tls"
+	"crypto/x509"
 	"fmt"
 	"net/http"
 	"net/url"
 
-	"crypto/tls"
-	"crypto/x509"
-
-	"crypto/rsa"
-
 	"github.com/crewjam/saml/samlsp"
 )
 
 func hello(w http.ResponseWriter, r *http.Request) {
-	fmt.Fprintf(w, "Hello, %s!", samlsp.Token(r.Context()).Attributes.Get("cn"))
+	fmt.Fprintf(w, "Hello, %s!", samlsp.AttributeFromContext(r.Context(), "cn"))
 }
 
 func main() {
@@ -27,22 +26,27 @@ func main() {
 		panic(err) // TODO handle error
 	}
 
-	idpMetadataURL, err := url.Parse("https://www.testshib.org/metadata/testshib-providers.xml")
+	rootURL, _ := url.Parse("http://localhost:8000")
+	idpMetadataURL, _ := url.Parse("https://www.testshib.org/metadata/testshib-providers.xml")
+
+	idpMetadata, err := samlsp.FetchMetadata(
+		context.Background(),
+		http.DefaultClient,
+		*idpMetadataURL)
 	if err != nil {
 		panic(err) // TODO handle error
 	}
 
-	rootURL, err := url.Parse("http://localhost:8000")
+	samlSP, err := samlsp.New(samlsp.Options{
+		URL:         *rootURL,
+		IDPMetadata: idpMetadata,
+		Key:         keyPair.PrivateKey.(*rsa.PrivateKey),
+		Certificate: keyPair.Leaf,
+	})
 	if err != nil {
 		panic(err) // TODO handle error
 	}
 
-	samlSP, _ := samlsp.New(samlsp.Options{
-		IDPMetadataURL: idpMetadataURL,
-		URL:            *rootURL,
-		Key:            keyPair.PrivateKey.(*rsa.PrivateKey),
-		Certificate:    keyPair.Leaf,
-	})
 	app := http.HandlerFunc(hello)
 	http.Handle("/hello", samlSP.RequireAccount(app))
 	http.Handle("/saml/", samlSP)

go.mod

@@ -4,10 +4,13 @@ go 1.13
 
 require (
 	github.com/beevik/etree v1.1.0
+	github.com/crewjam/httperr v0.0.0-20190612203328-a946449404da
+	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/dchest/uniuri v0.0.0-20160212164326-8902c56451e9
 	github.com/dgrijalva/jwt-go v3.2.0+incompatible
 	github.com/jonboulle/clockwork v0.1.0 // indirect
 	github.com/kr/pretty v0.1.0
+	github.com/pkg/errors v0.8.1 // indirect
 	github.com/russellhaering/goxmldsig v0.0.0-20180430223755-7acd5e4a6ef7
 	github.com/stretchr/testify v1.4.0
 	github.com/zenazn/goji v0.9.1-0.20160507202103-64eb34159fe5

go.sum

@@ -1,7 +1,11 @@
 github.com/beevik/etree v1.1.0 h1:T0xke/WvNtMoCqgzPhkX2r4rjY3GDZFi+FjpRZY2Jbs=
 github.com/beevik/etree v1.1.0/go.mod h1:r8Aw8JqVegEf0w2fDnATrX9VpkMcyFeM0FhwO62wh+A=
+github.com/crewjam/httperr v0.0.0-20190612203328-a946449404da h1:WXnT88cFG2davqSFqvaFfzkSMC0lqh/8/rKZ+z7tYvI=
+github.com/crewjam/httperr v0.0.0-20190612203328-a946449404da/go.mod h1:+rmNIXRvYMqLQeR4DHyTvs6y0MEMymTz4vyFpFkKTPs=
 github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
 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 v0.0.0-20160212164326-8902c56451e9 h1:74lLNRzvsdIlkTgfDSMuaPjBr4cf6k7pwQQANm/yLKU=
 github.com/dchest/uniuri v0.0.0-20160212164326-8902c56451e9/go.mod h1:GgB8SF9nRG+GqaDtLcwJZsQFhcogVCJ79j4EdT0c2V4=
 github.com/dgrijalva/jwt-go v3.2.0+incompatible h1:7qlOGliEKZXTDg6OTjfoBKDXWrumCAMpl/TFQ4/5kLM=
@@ -13,6 +17,8 @@ github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORN
 github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
+github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I=
+github.com/pkg/errors v0.8.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/russellhaering/goxmldsig v0.0.0-20180430223755-7acd5e4a6ef7 h1:J4AOUcOh/t1XbQcJfkEqhzgvMJ2tDxdCVvmHxW5QXao=

metadata.go

@@ -8,10 +8,10 @@ import (
 )
 
 // HTTPPostBinding is the official URN for the HTTP-POST binding (transport)
-var HTTPPostBinding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
+const HTTPPostBinding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
 
 // HTTPRedirectBinding is the official URN for the HTTP-Redirect binding (transport)
-var HTTPRedirectBinding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect"
+const HTTPRedirectBinding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect"
 
 // EntitiesDescriptor represents the SAML object of the same name.
 //

samlidp/samlidp_test.go

@@ -119,7 +119,6 @@ OwJlNCASPZRH/JmF8tX0hoHuAQ==
 		MetadataURL: mustParseURL("https://sp.example.com/saml2/metadata"),
 		AcsURL:      mustParseURL("https://sp.example.com/saml2/acs"),
 		IDPMetadata: &saml.EntityDescriptor{},
-		Logger:      logger.DefaultLogger,
 	}
 	test.Key = mustParsePrivateKey("-----BEGIN RSA PRIVATE KEY-----\nMIICXgIBAAKBgQDU8wdiaFmPfTyRYuFlVPi866WrH/2JubkHzp89bBQopDaLXYxi\n3PTu3O6Q/KaKxMOFBqrInwqpv/omOGZ4ycQ51O9I+Yc7ybVlW94lTo2gpGf+Y/8E\nPsVbnZaFutRctJ4dVIp9aQ2TpLiGT0xX1OzBO/JEgq9GzDRf+B+eqSuglwIDAQAB\nAoGBAMuy1eN6cgFiCOgBsB3gVDdTKpww87Qk5ivjqEt28SmXO13A1KNVPS6oQ8SJ\nCT5Azc6X/BIAoJCURVL+LHdqebogKljhH/3yIel1kH19vr4E2kTM/tYH+qj8afUS\nJEmArUzsmmK8ccuNqBcllqdwCZjxL4CHDUmyRudFcHVX9oyhAkEA/OV1OkjM3CLU\nN3sqELdMmHq5QZCUihBmk3/N5OvGdqAFGBlEeewlepEVxkh7JnaNXAXrKHRVu/f/\nfbCQxH+qrwJBANeQERF97b9Sibp9xgolb749UWNlAdqmEpmlvmS202TdcaaT1msU\n4rRLiQN3X9O9mq4LZMSVethrQAdX1whawpkCQQDk1yGf7xZpMJ8F4U5sN+F4rLyM\nRq8Sy8p2OBTwzCUXXK+fYeXjybsUUMr6VMYTRP2fQr/LKJIX+E5ZxvcIyFmDAkEA\nyfjNVUNVaIbQTzEbRlRvT6MqR+PTCefC072NF9aJWR93JimspGZMR7viY6IM4lrr\nvBkm0F5yXKaYtoiiDMzlOQJADqmEwXl0D72ZG/2KDg8b4QZEmC9i5gidpQwJXUc6\nhU+IVQoLxRq0fBib/36K9tcrrO5Ba4iEvDcNY+D8yGbUtA==\n-----END RSA PRIVATE KEY-----\n")
 	test.Certificate = mustParseCertificate("-----BEGIN CERTIFICATE-----\nMIIB7zCCAVgCCQDFzbKIp7b3MTANBgkqhkiG9w0BAQUFADA8MQswCQYDVQQGEwJV\nUzELMAkGA1UECAwCR0ExDDAKBgNVBAoMA2ZvbzESMBAGA1UEAwwJbG9jYWxob3N0\nMB4XDTEzMTAwMjAwMDg1MVoXDTE0MTAwMjAwMDg1MVowPDELMAkGA1UEBhMCVVMx\nCzAJBgNVBAgMAkdBMQwwCgYDVQQKDANmb28xEjAQBgNVBAMMCWxvY2FsaG9zdDCB\nnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEA1PMHYmhZj308kWLhZVT4vOulqx/9\nibm5B86fPWwUKKQ2i12MYtz07tzukPymisTDhQaqyJ8Kqb/6JjhmeMnEOdTvSPmH\nO8m1ZVveJU6NoKRn/mP/BD7FW52WhbrUXLSeHVSKfWkNk6S4hk9MV9TswTvyRIKv\nRsw0X/gfnqkroJcCAwEAATANBgkqhkiG9w0BAQUFAAOBgQCMMlIO+GNcGekevKgk\nakpMdAqJfs24maGb90DvTLbRZRD7Xvn1MnVBBS9hzlXiFLYOInXACMW5gcoRFfeT\nQLSouMM8o57h0uKjfTmuoWHLQLi6hnF+cvCsEFiJZ4AbF+DgmO6TarJ8O05t8zvn\nOwJlNCASPZRH/JmF8tX0hoHuAQ==\n-----END CERTIFICATE-----\n")