Make Wire options more robust

acme/api/order.go

@@ -282,13 +282,16 @@ func newAuthorization(ctx context.Context, az *acme.Authorization) error {
 			if err != nil {
 				return acme.WrapError(acme.ErrorMalformedType, err, "failed parsing ClientID")
 			}
-
+			wireOptions, err := prov.GetOptions().GetWireOptions()
+			if err != nil {
+				return acme.WrapErrorISE(err, "failed getting Wire options")
+			}
 			var targetProvider interface{ EvaluateTarget(string) (string, error) }
 			switch typ {
 			case acme.WIREOIDC01:
-				targetProvider = prov.GetOptions().GetWireOptions().GetOIDCOptions()
+				targetProvider = wireOptions.GetOIDCOptions()
 			case acme.WIREDPOP01:
-				targetProvider = prov.GetOptions().GetWireOptions().GetDPOPOptions()
+				targetProvider = wireOptions.GetDPOPOptions()
 			default:
 				return acme.NewError(acme.ErrorMalformedType, "unsupported type %q", typ)
 			}

acme/api/order_test.go

@@ -1720,7 +1720,7 @@ func TestHandler_NewOrder(t *testing.T) {
 				Wire: &wire.Options{
 					OIDC: &wire.OIDCOptions{
 						Provider: &wire.Provider{
-							IssuerURL:   "",
+							IssuerURL:   "https://issuer.example.com",
 							AuthURL:     "",
 							TokenURL:    "",
 							JWKSURL:     "",

acme/api/wire_integration_test.go

@@ -55,7 +55,7 @@ func TestWireIntegration(t *testing.T) {
 		Wire: &wire.Options{
 			OIDC: &wire.OIDCOptions{
 				Provider: &wire.Provider{
-					IssuerURL:   "",
+					IssuerURL:   "https://issuer.example.com",
 					AuthURL:     "",
 					TokenURL:    "",
 					JWKSURL:     "",

acme/challenge.go

@@ -370,7 +370,12 @@ func wireOIDC01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSO
 			"error unmarshalling Wire challenge payload"))
 	}
 
-	oidcOptions := prov.GetOptions().GetWireOptions().GetOIDCOptions()
+	wireOptions, err := prov.GetOptions().GetWireOptions()
+	if err != nil {
+		return WrapErrorISE(err, "failed getting Wire options")
+	}
+
+	oidcOptions := wireOptions.GetOIDCOptions()
 	idToken, err := oidcOptions.GetProvider(ctx).Verifier(oidcOptions.GetConfig()).Verify(ctx, oidcPayload.IDToken)
 	if err != nil {
 		return storeError(ctx, db, ch, false, WrapError(ErrorRejectedIdentifierType, err,
@@ -474,8 +479,12 @@ func wireDPOP01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSO
 		return WrapErrorISE(err, "error parsing device id")
 	}
 
-	dpopOptions := prov.GetOptions().GetWireOptions().GetDPOPOptions()
+	wireOptions, err := prov.GetOptions().GetWireOptions()
+	if err != nil {
+		return WrapErrorISE(err, "failed getting Wire options")
+	}
 
+	dpopOptions := wireOptions.GetDPOPOptions()
 	issuer, err := dpopOptions.EvaluateTarget(clientID.DeviceID)
 	if err != nil {
 		return WrapErrorISE(err, "invalid Go template registered for 'target'")

authority/provisioner/options.go

@@ -2,6 +2,7 @@ package provisioner
 
 import (
 	"encoding/json"
+	"fmt"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -54,11 +55,14 @@ func (o *Options) GetSSHOptions() *SSHOptions {
 }
 
 // GetWireOptions returns the SSH options.
-func (o *Options) GetWireOptions() *wire.Options {
+func (o *Options) GetWireOptions() (*wire.Options, error) {
 	if o == nil {
-		return nil
+		return nil, errors.New("no Wire options available")
 	}
-	return o.Wire
+	if err := o.Wire.Validate(); err != nil {
+		return nil, fmt.Errorf("failed validating Wire options: %w", err)
+	}
+	return o.Wire, nil
 }
 
 // GetWebhooks returns the webhooks options.

authority/provisioner/wire/dpop_options.go

@@ -32,14 +32,20 @@ func (o *DPOPOptions) EvaluateTarget(deviceID string) (string, error) {
 	if o == nil {
 		return "", errors.New("misconfigured target template configuration")
 	}
-	targetTemplate := o.GetTarget()
-	tmpl, err := template.New("DeviceId").Parse(targetTemplate)
+	tmpl, err := template.New("DeviceID").Parse(o.GetTarget())
 	if err != nil {
 		return "", fmt.Errorf("failed parsing dpop template: %w", err)
 	}
 	buf := new(bytes.Buffer)
-	if err = tmpl.Execute(buf, struct{ DeviceId string }{DeviceId: deviceID}); err != nil { //nolint:revive,stylecheck // TODO(hs): this requires changes in configuration
+	if err = tmpl.Execute(buf, struct{ DeviceID string }{DeviceID: deviceID}); err != nil {
 		return "", fmt.Errorf("failed executing dpop template: %w", err)
 	}
 	return buf.String(), nil
 }
+
+func (o *DPOPOptions) validate() error {
+	if _, err := template.New("DeviceID").Parse(o.GetTarget()); err != nil {
+		return fmt.Errorf("failed parsing template: %w", err)
+	}
+	return nil
+}

authority/provisioner/wire/oidc_options.go

@@ -59,27 +59,40 @@ func (o *OIDCOptions) GetConfig() *oidc.Config {
 	}
 }
 
+func (o *OIDCOptions) validate() error {
+	if o.Provider == nil {
+		return errors.New("provider not set")
+	}
+	if o.Provider.IssuerURL == "" {
+		return errors.New("issuer URL must not be empty")
+	}
+	if _, err := url.Parse(o.Provider.IssuerURL); err != nil {
+		return fmt.Errorf("failed parsing issuer URL: %w", err)
+	}
+	if _, err := template.New("DeviceID").Parse(o.Provider.IssuerURL); err != nil {
+		return fmt.Errorf("failed parsing template: %w", err)
+	}
+
+	return nil
+}
+
 func (o *OIDCOptions) EvaluateTarget(deviceID string) (string, error) {
 	if o == nil {
 		return "", errors.New("misconfigured target template configuration")
 	}
-	targetTemplate := o.Provider.IssuerURL
-	tmpl, err := template.New("DeviceId").Parse(targetTemplate)
+	tmpl, err := template.New("DeviceID").Parse(o.Provider.IssuerURL)
 	if err != nil {
-		return "", fmt.Errorf("failed parsing oidc template: %w", err)
+		return "", fmt.Errorf("failed parsing OIDC template: %w", err)
 	}
 	buf := new(bytes.Buffer)
-	if err = tmpl.Execute(buf, struct{ DeviceId string }{DeviceId: deviceID}); err != nil { //nolint:revive,stylecheck // TODO(hs): this requires changes in configuration
-		return "", fmt.Errorf("failed executing oidc template: %w", err)
+	if err = tmpl.Execute(buf, struct{ DeviceID string }{DeviceID: deviceID}); err != nil {
+		return "", fmt.Errorf("failed executing OIDC template: %w", err)
 	}
 	return buf.String(), nil
 }
 
 func toOIDCProviderConfig(in *Provider) *oidc.ProviderConfig {
-	issuerURL, err := url.Parse(in.IssuerURL)
-	if err != nil {
-		panic(err) // config error, it's ok to panic here
-	}
+	issuerURL, _ := url.Parse(in.IssuerURL) // NOTE: validation is performed in validate()
 	// Removes query params from the URL because we use it as a way to notify client about the actual OAuth ClientId
 	// for this provisioner.
 	// This URL is going to look like: "https://idp:5556/dex?clientid=foo"

authority/provisioner/wire/wire_options.go

@@ -1,9 +1,18 @@
 package wire
 
+import (
+	"errors"
+	"fmt"
+	"sync"
+)
+
 // Options holds the Wire ACME extension options
 type Options struct {
 	OIDC *OIDCOptions `json:"oidc,omitempty"`
 	DPOP *DPOPOptions `json:"dpop,omitempty"`
+
+	validateOnce  sync.Once
+	validationErr error
 }
 
 // GetOIDCOptions returns the OIDC options.
@@ -21,3 +30,33 @@ func (o *Options) GetDPOPOptions() *DPOPOptions {
 	}
 	return o.DPOP
 }
+
+func (o *Options) Validate() error {
+	o.validateOnce.Do(
+		func() {
+			o.validationErr = validate(o)
+		},
+	)
+
+	return o.validationErr
+}
+
+func validate(o *Options) error {
+	if oidc := o.GetOIDCOptions(); oidc != nil {
+		if err := oidc.validate(); err != nil {
+			return fmt.Errorf("failed validating OIDC options: %w", err)
+		}
+	} else {
+		return errors.New("no OIDC options available")
+	}
+
+	if dpop := o.GetDPOPOptions(); dpop != nil {
+		if err := dpop.validate(); err != nil {
+			return fmt.Errorf("failed validating DPoP options: %w", err)
+		}
+	} else {
+		return errors.New("no DPoP options available")
+	}
+
+	return nil
+}

authority/provisioner/wire/wire_options_test.go

@@ -0,0 +1,124 @@
+package wire
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestOptions_Validate(t *testing.T) {
+	type fields struct {
+		OIDC *OIDCOptions
+		DPOP *DPOPOptions
+	}
+	tests := []struct {
+		name        string
+		fields      fields
+		expectedErr error
+	}{
+		{
+			name: "ok",
+			fields: fields{
+				OIDC: &OIDCOptions{
+					Provider: &Provider{
+						IssuerURL: "https://example.com",
+					},
+					Config: &Config{},
+				},
+				DPOP: &DPOPOptions{},
+			},
+			expectedErr: nil,
+		},
+		{
+			name: "fail/no-oidc-options",
+			fields: fields{
+				OIDC: nil,
+				DPOP: &DPOPOptions{},
+			},
+			expectedErr: errors.New("no OIDC options available"),
+		},
+		{
+			name: "fail/empty-issuer-url",
+			fields: fields{
+				OIDC: &OIDCOptions{
+					Provider: &Provider{
+						IssuerURL: "",
+					},
+					Config: &Config{},
+				},
+				DPOP: &DPOPOptions{},
+			},
+			expectedErr: errors.New("failed validating OIDC options: issuer URL must not be empty"),
+		},
+		{
+			name: "fail/invalid-issuer-url",
+			fields: fields{
+				OIDC: &OIDCOptions{
+					Provider: &Provider{
+						IssuerURL: "\x00",
+					},
+					Config: &Config{},
+				},
+				DPOP: &DPOPOptions{},
+			},
+			expectedErr: errors.New(`failed validating OIDC options: failed parsing issuer URL: parse "\x00": net/url: invalid control character in URL`),
+		},
+		{
+			name: "fail/issuer-url-template",
+			fields: fields{
+				OIDC: &OIDCOptions{
+					Provider: &Provider{
+						IssuerURL: "https://issuer.example.com/{{}",
+					},
+					Config: &Config{},
+				},
+				DPOP: &DPOPOptions{},
+			},
+			expectedErr: errors.New(`failed validating OIDC options: failed parsing template: template: DeviceID:1: unexpected "}" in command`),
+		},
+		{
+			name: "fail/no-dpop-options",
+			fields: fields{
+				OIDC: &OIDCOptions{
+					Provider: &Provider{
+						IssuerURL: "https://example.com",
+					},
+					Config: &Config{},
+				},
+				DPOP: nil,
+			},
+			expectedErr: errors.New("no DPoP options available"),
+		},
+		{
+			name: "fail/target-template",
+			fields: fields{
+				OIDC: &OIDCOptions{
+					Provider: &Provider{
+						IssuerURL: "https://example.com",
+					},
+					Config: &Config{},
+				},
+				DPOP: &DPOPOptions{
+					Target: "{{}",
+				},
+			},
+			expectedErr: errors.New(`failed validating DPoP options: failed parsing template: template: DeviceID:1: unexpected "}" in command`),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			o := &Options{
+				OIDC: tt.fields.OIDC,
+				DPOP: tt.fields.DPOP,
+			}
+			err := o.Validate()
+			if tt.expectedErr != nil {
+				assert.EqualError(t, err, tt.expectedErr.Error())
+				return
+			}
+
+			assert.NoError(t, err)
+		})
+	}
+}