Fix some provisioner prompt issues related to SCEP and policies

This PR fixes the following issues:

 - SCEP provisioners not detected in admin token flows
 - Invalid provisioner selection logic when managing provisioner policies
 - Unexpected error messages showing "issuer" instead or "provisioner" flag

Author: hslatman

command/ca/policy/acme/acme.go

@@ -8,6 +8,7 @@ import (
 	"github.com/smallstep/cli/command/ca/policy/actions"
 	"github.com/smallstep/cli/command/ca/policy/policycontext"
 	"github.com/smallstep/cli/command/ca/policy/x509"
+	"github.com/smallstep/cli/internal/provisionerflag"
 )
 
 // Command returns the ACME account policy subcommand.
@@ -27,5 +28,9 @@ Please note that certificate issuance policies for ACME accounts are currently o
 			actions.RemoveCommand(ctx),
 			x509.Command(ctx),
 		},
+		Before: func(ctx *cli.Context) error {
+			provisionerflag.Ignore()
+			return nil
+		},
 	}
 }

command/ca/policy/provisioner/provisioner.go

@@ -9,6 +9,7 @@ import (
 	"github.com/smallstep/cli/command/ca/policy/policycontext"
 	"github.com/smallstep/cli/command/ca/policy/ssh"
 	"github.com/smallstep/cli/command/ca/policy/x509"
+	"github.com/smallstep/cli/internal/provisionerflag"
 )
 
 // Command returns the policy subcommand.
@@ -29,5 +30,9 @@ Please note that certificate issuance policies on the provisioner level are curr
 			x509.Command(ctx),
 			ssh.Command(ctx),
 		},
+		Before: func(ctx *cli.Context) error {
+			provisionerflag.Ignore()
+			return nil
+		},
 	}
 }

flags/flags.go

@@ -673,18 +673,24 @@ func parseCaURL(ctx *cli.Context, caURL string) (string, error) {
 
 // FirstStringOf returns the value of the first defined flag from the input list.
 // If no defined flags, returns first flag with non-empty default value.
-func FirstStringOf(ctx *cli.Context, flags ...string) string {
+func FirstStringOf(ctx *cli.Context, flags ...string) (string, string) {
 	// Return first defined flag.
 	for _, f := range flags {
 		if ctx.IsSet(f) {
-			return ctx.String(f)
+			return ctx.String(f), f
 		}
 	}
 	// Return first non-empty, default, flag value.
 	for _, f := range flags {
 		if val := ctx.String(f); val != "" {
-			return val
+			return val, f
 		}
 	}
-	return ""
+
+	var name = "<unknown>"
+	if len(flags) > 0 {
+		name = flags[0]
+	}
+
+	return "", name
 }

flags/flags_test.go

@@ -11,6 +11,7 @@ import (
 	"testing"
 
 	"github.com/smallstep/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/urfave/cli"
 	"go.step.sm/crypto/fingerprint"
 )
@@ -242,6 +243,7 @@ func TestFirstStringOf(t *testing.T) {
 		getContext func() *cli.Context
 		inputs     []string
 		want       string
+		wantName   string
 	}{
 		{
 			name: "no-flags-empty",
@@ -250,8 +252,9 @@ func TestFirstStringOf(t *testing.T) {
 				//_ = set.String("ca-url", "", "")
 				return cli.NewContext(app, set, nil)
 			},
-			inputs: []string{"foo", "bar"},
-			want:   "",
+			inputs:   []string{"foo", "bar"},
+			want:     "",
+			wantName: "foo",
 		},
 		{
 			name: "return-first-set-flag",
@@ -265,8 +268,9 @@ func TestFirstStringOf(t *testing.T) {
 				ctx.Set("baz", "test2")
 				return ctx
 			},
-			inputs: []string{"foo", "bar", "baz"},
-			want:   "test1",
+			inputs:   []string{"foo", "bar", "baz"},
+			want:     "test1",
+			wantName: "bar",
 		},
 		{
 			name: "return-first-default-flag",
@@ -278,8 +282,9 @@ func TestFirstStringOf(t *testing.T) {
 				ctx := cli.NewContext(app, set, nil)
 				return ctx
 			},
-			inputs: []string{"foo", "bar", "baz"},
-			want:   "test1",
+			inputs:   []string{"foo", "bar", "baz"},
+			want:     "test1",
+			wantName: "baz",
 		},
 		{
 			name: "all-empty",
@@ -291,17 +296,17 @@ func TestFirstStringOf(t *testing.T) {
 				ctx := cli.NewContext(app, set, nil)
 				return ctx
 			},
-			inputs: []string{"foo", "bar", "baz"},
-			want:   "",
+			inputs:   []string{"foo", "bar", "baz"},
+			want:     "",
+			wantName: "foo",
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			ctx := tt.getContext()
-			val := FirstStringOf(ctx, tt.inputs...)
-			if val != tt.want {
-				t.Errorf("expected %v, but got %v", tt.want, val)
-			}
+			val, name := FirstStringOf(ctx, tt.inputs...)
+			require.Equal(t, tt.want, val)
+			require.Equal(t, tt.wantName, name)
 		})
 	}
 }

internal/provisionerflag/ignore.go

@@ -0,0 +1,17 @@
+package provisionerflag
+
+import (
+	"sync/atomic"
+)
+
+var disabled atomic.Bool
+
+// Ignore marks the provisionerflag to be ignored
+func Ignore() {
+	disabled.Store(true)
+}
+
+// ShouldBeIgnored returns true if the provisioner flag should be ignored
+func ShouldBeIgnored() bool {
+	return disabled.Load()
+}

utils/cautils/offline.go

@@ -12,15 +12,17 @@ import (
 	"time"
 
 	"github.com/pkg/errors"
+	"github.com/urfave/cli"
+	"golang.org/x/crypto/ssh"
+
 	"github.com/smallstep/certificates/api"
 	"github.com/smallstep/certificates/authority"
 	"github.com/smallstep/certificates/authority/config"
 	"github.com/smallstep/certificates/authority/provisioner"
-	"github.com/smallstep/cli/utils"
-	"github.com/urfave/cli"
 	"go.step.sm/crypto/pemutil"
 	"go.step.sm/crypto/x509util"
-	"golang.org/x/crypto/ssh"
+
+	"github.com/smallstep/cli/utils"
 )
 
 // OfflineCA is a wrapper on top of the certificates authority methods that is
@@ -585,6 +587,8 @@ func (c *OfflineCA) GenerateToken(ctx *cli.Context, tokType int, subject string,
 		return p.GetIdentityToken(subject, c.CaURL())
 	case *provisioner.ACME: // Return an error with the provisioner ID.
 		return "", &ACMETokenError{p.GetName()}
+	case *provisioner.SCEP:
+		return "", &SCEPTokenError{p.GetName()}
 	default: // Default is assumed to be a standard JWT.
 		jwkP, ok := p.(*provisioner.JWK)
 		if !ok {

utils/cautils/token_flow.go

@@ -15,6 +15,7 @@ import (
 	"github.com/smallstep/cli-utils/ui"
 
 	"github.com/smallstep/cli/flags"
+	"github.com/smallstep/cli/internal/provisionerflag"
 	"github.com/smallstep/cli/utils"
 )
 
@@ -86,6 +87,17 @@ func (e *ACMETokenError) Error() string {
 	return "step ACME provisioners do not support token auth flows"
 }
 
+// SCEPTokenError is the error type returned when the user attempts a Token Flow
+// while using a SCEP provisioner.
+type SCEPTokenError struct {
+	Name string
+}
+
+// Error implements the error interface.
+func (e *SCEPTokenError) Error() string {
+	return "step SCEP provisioners do not support token auth flows"
+}
+
 // NewTokenFlow implements the common flow used to generate a token
 func NewTokenFlow(ctx *cli.Context, tokType int, subject string, sans []string, caURL, root string, notBefore, notAfter time.Time, certNotBefore, certNotAfter provisioner.TimeDuration, opts ...Option) (string, error) {
 	// Apply options to shared context
@@ -164,6 +176,8 @@ func NewTokenFlow(ctx *cli.Context, tokType int, subject string, sans []string,
 		return p.GetIdentityToken(subject, caURL)
 	case *provisioner.ACME: // Return an error with the provisioner ID.
 		return "", &ACMETokenError{p.GetName()}
+	case *provisioner.SCEP:
+		return "", &SCEPTokenError{p.GetName()}
 	default:
 		return "", errors.Errorf("unknown provisioner type %T", p)
 	}
@@ -212,13 +226,13 @@ func OfflineTokenFlow(ctx *cli.Context, typ int, subject string, sans []string,
 	}
 
 	kid := ctx.String("kid")
-	issuer := flags.FirstStringOf(ctx, "provisioner", "issuer")
+	issuer, flag := flags.FirstStringOf(ctx, "provisioner", "issuer")
 
 	// Require issuer and keyFile if ca.json does not exists.
 	// kid can be passed or created using jwk.Thumbprint.
 	switch {
 	case issuer == "":
-		return "", errs.RequiredWithFlag(ctx, "offline", "issuer")
+		return "", errs.RequiredWithFlag(ctx, "offline", flag)
 	case ctx.String("key") == "":
 		return "", errs.RequiredWithFlag(ctx, "offline", "key")
 	}
@@ -293,7 +307,7 @@ func provisionerPrompt(ctx *cli.Context, provisioners provisioner.List) (provisi
 		provisioners = provisionerFilter(provisioners, func(p provisioner.Interface) bool {
 			switch p.GetType() {
 			case provisioner.TypeJWK, provisioner.TypeOIDC,
-				provisioner.TypeACME, provisioner.TypeK8sSA,
+				provisioner.TypeACME, provisioner.TypeSCEP, provisioner.TypeK8sSA,
 				provisioner.TypeX5C, provisioner.TypeSSHPOP, provisioner.TypeNebula:
 				return true
 			case provisioner.TypeGCP, provisioner.TypeAWS, provisioner.TypeAzure:
@@ -325,23 +339,27 @@ func provisionerPrompt(ctx *cli.Context, provisioners provisioner.List) (provisi
 		}
 	}
 
-	// Filter by issuer (provisioner name)
-	if issuer := flags.FirstStringOf(ctx, "provisioner", "issuer"); issuer != "" {
+	// Filter by admin-provisioner (provisioner name)
+	if issuer := ctx.String("admin-provisioner"); issuer != "" {
 		provisioners = provisionerFilter(provisioners, func(p provisioner.Interface) bool {
 			return p.GetName() == issuer
 		})
 		if len(provisioners) == 0 {
-			return nil, errs.InvalidFlagValue(ctx, "issuer", issuer, "")
+			return nil, errs.InvalidFlagValue(ctx, "admin-provisioner", issuer, "")
 		}
 	}
 
-	// Filter by admin-provisioner (provisioner name)
-	if issuer := ctx.String("admin-provisioner"); issuer != "" {
+	// Filter by provisioner / issuer (provisioner name)
+	if issuer, flag := flags.FirstStringOf(ctx, "provisioner", "issuer"); issuer != "" {
 		provisioners = provisionerFilter(provisioners, func(p provisioner.Interface) bool {
+			if provisionerflag.ShouldBeIgnored() {
+				return true // fake match; effectively skipping provisioner flag value for provisioner-dependent policy commands
+			}
+
 			return p.GetName() == issuer
 		})
 		if len(provisioners) == 0 {
-			return nil, errs.InvalidFlagValue(ctx, "admin-provisioner", issuer, "")
+			return nil, errs.InvalidFlagValue(ctx, flag, issuer, "")
 		}
 	}
 
@@ -364,7 +382,7 @@ func provisionerPrompt(ctx *cli.Context, provisioners provisioner.List) (provisi
 				Name:        fmt.Sprintf("%s (%s) [tenant: %s]", p.Name, p.GetType(), p.TenantID),
 				Provisioner: p,
 			})
-		case *provisioner.GCP, *provisioner.AWS, *provisioner.X5C, *provisioner.SSHPOP, *provisioner.ACME, *provisioner.Nebula:
+		case *provisioner.GCP, *provisioner.AWS, *provisioner.X5C, *provisioner.SSHPOP, *provisioner.ACME, *provisioner.SCEP, *provisioner.Nebula:
 			items = append(items, &provisionersSelect{
 				Name:        fmt.Sprintf("%s (%s)", p.GetName(), p.GetType()),
 				Provisioner: p,