samlsp: remove deprecated fields

README.md

@@ -13,31 +13,6 @@ 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
-
-**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, but will be in future.
-
-In particular we have deprecated the following fields in
-_samlsp.Options_:
-
-- `Logger` - This was used to emit errors while validating, which is an anti-pattern.
-- `IDPMetadataURL` - Instead use `FetchMetadata()`
-- `HTTPClient` - Instead pass httpClient to FetchMetadata
-- `CookieMaxAge` - Instead assign a custom CookieRequestTracker or CookieSessionProvider
-- `CookieName` - Instead assign a custom CookieRequestTracker or CookieSessionProvider
-- `CookieDomain` - Instead assign a custom CookieRequestTracker or CookieSessionProvider
-- `CookieDomain` - Instead assign a custom CookieRequestTracker or CookieSessionProvider
-
 ## 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.
@@ -71,6 +46,7 @@ We will use `samlsp.Middleware` to wrap the endpoint we want to protect. Middlew
 package main
 
 import (
+	"context"
 	"crypto/rsa"
 	"crypto/tls"
 	"crypto/x509"
@@ -99,6 +75,11 @@ func main() {
 	if err != nil {
 		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("http://localhost:8000")
 	if err != nil {
@@ -109,7 +90,7 @@ func main() {
 		URL:            *rootURL,
 		Key:            keyPair.PrivateKey.(*rsa.PrivateKey),
 		Certificate:    keyPair.Leaf,
-		IDPMetadataURL: idpMetadataURL,
+		IDPMetadata: idpMetadata,
 	})
 	app := http.HandlerFunc(hello)
 	http.Handle("/hello", samlSP.RequireAccount(app))

samlsp/error.go

@@ -5,7 +5,6 @@ import (
 	"net/http"
 
 	"github.com/crewjam/saml"
-	"github.com/crewjam/saml/logger"
 )
 
 // ErrorFunction is a callback that is invoked to return an error to the
@@ -24,18 +23,3 @@ func DefaultOnError(w http.ResponseWriter, r *http.Request, err error) {
 	}
 	http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden)
 }
-
-// defaultOnErrorWithLogger is like DefaultOnError but accepts a custom logger.
-// This is a bridge for backward compatibility with people use provide the
-// deprecated Logger options field to New().
-func defaultOnErrorWithLogger(log logger.Interface) ErrorFunction {
-	return func(w http.ResponseWriter, r *http.Request, err error) {
-		if parseErr, ok := err.(*saml.InvalidResponseError); ok {
-			log.Printf("WARNING: received invalid saml response: %s (now: %s) %s",
-				parseErr.Response, parseErr.Now, parseErr.PrivateErr)
-		} else {
-			log.Printf("ERROR: %s", err)
-		}
-		http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden)
-	}
-}

samlsp/new.go

@@ -2,17 +2,14 @@
 package samlsp
 
 import (
-	"context"
 	"crypto/rsa"
 	"crypto/x509"
 	"net/http"
 	"net/url"
-	"time"
 
 	dsig "github.com/russellhaering/goxmldsig"
 
 	"github.com/crewjam/saml"
-	"github.com/crewjam/saml/logger"
 )
 
 // Options represents the parameters for creating a new middleware
@@ -27,69 +24,29 @@ type Options struct {
 	SignRequest       bool
 	ForceAuthn        bool // TODO(ross): this should be *bool
 	CookieSameSite    http.SameSite
-
-	// The following fields exist <= 0.3.0, but are superceded by the new
-	// SessionProvider and RequestTracker interfaces.
-	Logger         logger.Interface // DEPRECATED: this field will be removed, instead provide a custom OnError function to handle errors
-	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   bool             // DEPRECATED: this field will be removed, the Secure flag is set on cookies when the root URL uses the https scheme
 }
 
 // DefaultSessionCodec returns the default SessionCodec for the provided options,
 // a JWTSessionCodec configured to issue signed tokens.
 func DefaultSessionCodec(opts Options) JWTSessionCodec {
-	// for backwards compatibility, support CookieMaxAge
-	maxAge := defaultSessionMaxAge
-	if opts.CookieMaxAge > 0 {
-		maxAge = opts.CookieMaxAge
-	}
-
 	return JWTSessionCodec{
 		SigningMethod: defaultJWTSigningMethod,
 		Audience:      opts.URL.String(),
 		Issuer:        opts.URL.String(),
-		MaxAge:        maxAge,
+		MaxAge:        defaultSessionMaxAge,
 		Key:           opts.Key,
 	}
 }
 
 // DefaultSessionProvider returns the default SessionProvider for the provided options,
 // a CookieSessionProvider configured to store sessions in a cookie.
 func DefaultSessionProvider(opts Options) CookieSessionProvider {
-	// for backwards compatibility, support CookieMaxAge
-	maxAge := defaultSessionMaxAge
-	if opts.CookieMaxAge > 0 {
-		maxAge = opts.CookieMaxAge
-	}
-
-	// for backwards compatibility, support CookieName
-	cookieName := defaultSessionCookieName
-	if opts.CookieName != "" {
-		cookieName = opts.CookieName
-	}
-
-	// for backwards compatibility, support CookieDomain
-	cookieDomain := opts.URL.Host
-	if opts.CookieDomain != "" {
-		cookieDomain = opts.CookieDomain
-	}
-
-	// for backwards compatibility, support CookieDomain
-	cookieSecure := opts.URL.Scheme == "https"
-	if opts.CookieSecure {
-		cookieSecure = true
-	}
-
 	return CookieSessionProvider{
-		Name:     cookieName,
-		Domain:   cookieDomain,
-		MaxAge:   maxAge,
+		Name:     defaultSessionCookieName,
+		Domain:   opts.URL.Host,
+		MaxAge:   defaultSessionMaxAge,
 		HTTPOnly: true,
-		Secure:   cookieSecure,
+		Secure:   opts.URL.Scheme == "https",
 		SameSite: opts.CookieSameSite,
 		Codec:    DefaultSessionCodec(opts),
 	}
@@ -157,29 +114,10 @@ func DefaultServiceProvider(opts Options) saml.ServiceProvider {
 // replacing and/or changing Session, RequestTracker, and ServiceProvider
 // in the returned Middleware.
 func New(opts Options) (*Middleware, error) {
-	// for backwards compatibility, support Logger
-	onError := DefaultOnError
-	if opts.Logger != nil {
-		onError = defaultOnErrorWithLogger(opts.Logger)
-	}
-
-	// for backwards compatibility, support IDPMetadataURL
-	if opts.IDPMetadataURL != nil && opts.IDPMetadata == nil {
-		httpClient := opts.HTTPClient
-		if httpClient == nil {
-			httpClient = http.DefaultClient
-		}
-		metadata, err := FetchMetadata(context.TODO(), httpClient, *opts.IDPMetadataURL)
-		if err != nil {
-			return nil, err
-		}
-		opts.IDPMetadata = metadata
-	}
-
 	m := &Middleware{
 		ServiceProvider: DefaultServiceProvider(opts),
 		Binding:         "",
-		OnError:         onError,
+		OnError:         DefaultOnError,
 		Session:         DefaultSessionProvider(opts),
 	}
 	m.RequestTracker = DefaultRequestTracker(opts, &m.ServiceProvider)