Improving --signer-fulcio-token flag to accept both path and raw to…

…ken string (#82) * modified `--signer-fulcio-token` flag to accept either a path to a token or a raw token string * modified `--signer-fulcio-token` flag to accept either a path to a token or a raw token string Signed-off-by: chaosinthecrd <[email protected]> * adding an unhappy path to the tests Signed-off-by: chaosinthecrd <[email protected]> * updated function and description * updated function and description Signed-off-by: chaosinthecrd <[email protected]> * removing ineffectual assignment in test Signed-off-by: chaosinthecrd <[email protected]> * updated to add token path flag and remove idToken function magic Signed-off-by: chaosinthecrd <[email protected]> * removing ineffectual assignments Signed-off-by: chaosinthecrd <[email protected]> * removing whitespace Signed-off-by: chaosinthecrd <[email protected]> * fixing small issue and adding test to makefile to speed things up Signed-off-by: chaosinthecrd <[email protected]> --------- Signed-off-by: chaosinthecrd <[email protected]>
in-toto · Dec 11, 2023 · 70efbcf · 70efbcf
1 parent b11e25f
commit 70efbcf
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 5 deletions.
diff --git a/Makefile b/Makefile
@@ -18,3 +18,9 @@ controller-gen: $(CONTROLLER_GEN) ## Download controller-gen locally if necessar
 $(CONTROLLER_GEN): $(LOCALBIN)
 	test -s $(LOCALBIN)/controller-gen && $(LOCALBIN)/controller-gen --version | grep -q $(CONTROLLER_TOOLS_VERSION) || \
 	GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-tools/cmd/controller-gen@$(CONTROLLER_TOOLS_VERSION)
+
+test: ## Run the go unit tests
+	go test -v -coverprofile=profile.cov -covermode=atomic ./...
+
+help: ## Display this help screen
+	@grep -h -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'
diff --git a/hack/test.token b/hack/test.token
@@ -0,0 +1 @@
+eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJmb29iYXIiLCJuYW1lIjoiSm9obiBEb2UiLCJpYXQiOjE1MTYyMzkwMjIsIkVtYWlsIjoidGVzdEBpbi10b3RvLmlvIn0.IswtNc6aJL3zAf-lSGvuz7Okf2tBr-I3ulJ_SRUMt0k
diff --git a/signer/fulcio/fulcio.go b/signer/fulcio/fulcio.go
@@ -93,7 +93,7 @@ func init() {
 		),
 		registry.StringConfigOption(
 			"token",
-			"Raw token to use for authentication",
+			"Raw token string to use for authentication to fulcio (cannot be used in conjunction with --fulcio-token-path)",
 			"",
 			func(sp signer.SignerProvider, token string) (signer.SignerProvider, error) {
 				fsp, ok := sp.(FulcioSignerProvider)
@@ -105,6 +105,20 @@ func init() {
 				return fsp, nil
 			},
 		),
+		registry.StringConfigOption(
+			"token-path",
+			"Path to the file containing a raw token to use for authentication to fulcio (cannot be used in conjunction with --fulcio-token)",
+			"",
+			func(sp signer.SignerProvider, tokenPath string) (signer.SignerProvider, error) {
+				fsp, ok := sp.(FulcioSignerProvider)
+				if !ok {
+					return sp, fmt.Errorf("provided signer provider is not a fulcio signer provider")
+				}
+
+				WithTokenPath(tokenPath)(&fsp)
+				return fsp, nil
+			},
+		),
 	)
 }
 
@@ -113,6 +127,7 @@ type FulcioSignerProvider struct {
 	OidcIssuer   string
 	OidcClientID string
 	Token        string
+	TokenPath    string
 }
 
 type Option func(*FulcioSignerProvider)
@@ -141,6 +156,12 @@ func WithToken(tokenOption string) Option {
 	}
 }
 
+func WithTokenPath(tokenPathOption string) Option {
+	return func(fsp *FulcioSignerProvider) {
+		fsp.TokenPath = tokenPathOption
+	}
+}
+
 func New(opts ...Option) FulcioSignerProvider {
 	fsp := FulcioSignerProvider{}
 	for _, opt := range opts {
@@ -194,7 +215,7 @@ func (fsp FulcioSignerProvider) Signer(ctx context.Context) (cryptoutil.Signer,
 	var raw string
 
 	switch {
-	case fsp.Token == "" && os.Getenv("GITHUB_ACTIONS") == "true":
+	case fsp.Token == "" && fsp.TokenPath == "" && os.Getenv("GITHUB_ACTIONS") == "true":
 		tokenURL := os.Getenv("ACTIONS_ID_TOKEN_REQUEST_URL")
 		if tokenURL == "" {
 			return nil, errors.New("ACTIONS_ID_TOKEN_REQUEST_URL is not set")
@@ -209,10 +230,18 @@ func (fsp FulcioSignerProvider) Signer(ctx context.Context) (cryptoutil.Signer,
 		if err != nil {
 			return nil, err
 		}
-
-	case fsp.Token != "":
+	// we want to fail if both flags used (they're mutually exclusive)
+	case fsp.TokenPath != "" && fsp.Token != "":
+		return nil, errors.New("only one of --fulcio-token-path or --fulcio-raw-token can be used")
+	case fsp.Token != "" && fsp.TokenPath == "":
 		raw = fsp.Token
+	case fsp.TokenPath != "" && fsp.Token == "":
+		f, err := os.ReadFile(fsp.TokenPath)
+		if err != nil {
+			return nil, fmt.Errorf("failed to read fulcio token from filepath %s: %w", fsp.TokenPath, err)
+		}
 
+		raw = string(f)
 	case fsp.Token == "" && isatty.IsTerminal(os.Stdin.Fd()):
 		tok, err := oauthflow.OIDConnect(fsp.OidcIssuer, fsp.OidcClientID, "", "", oauthflow.DefaultIDTokenGetter)
 		if err != nil {
@@ -281,7 +310,7 @@ func (fsp FulcioSignerProvider) Signer(ctx context.Context) (cryptoutil.Signer,
 func getCert(ctx context.Context, key *rsa.PrivateKey, fc fulciopb.CAClient, token string) (*fulciopb.SigningCertificate, error) {
 	t, err := jwt.ParseSigned(token)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("Failed to parse jwt token for fulcio: %w", err)
 	}
 
 	var claims struct {

diff --git a/signer/fulcio/fulcio_test.go b/signer/fulcio/fulcio_test.go
@@ -24,6 +24,7 @@ import (
 	"fmt"
 	"log"
 	"net"
+	"os"
 	"strings"
 	"testing"
 	"time"
@@ -33,6 +34,7 @@ import (
 	fulciopb "github.com/sigstore/fulcio/pkg/generated/protobuf"
 	"github.com/stretchr/testify/require"
 	"go.step.sm/crypto/jose"
+	"path/filepath"
 
 	"google.golang.org/grpc"
 	"gopkg.in/square/go-jose.v2/jwt"
@@ -198,6 +200,25 @@ func TestSigner(t *testing.T) {
 	_, err = provider.Signer(ctx)
 	//this should be a tranport err since we cant actually test on 443 which is the default
 	require.ErrorContains(t, err, "lookup test")
+
+	// Test signer with token read from file
+	// NOTE: this function could be refactored to accept a fileSystem or io.Reader so reading the file can be mocked,
+	// but unsure if this is the way we want to go for now
+	wd, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("failed to get working directory: %v", err)
+	}
+	rootDir := filepath.Dir(filepath.Dir(wd))
+	tp := filepath.Join(rootDir, "hack", "test.token")
+
+	provider = New(WithFulcioURL(fmt.Sprintf("http://%v:%v", hostname, port)), WithTokenPath(tp))
+	_, err = provider.Signer(ctx)
+	require.NoError(t, err)
+
+	// Test signer with both token read from file and raw token
+	provider = New(WithFulcioURL(fmt.Sprintf("http://%v:%v", hostname, port)), WithTokenPath(tp), WithToken(token))
+	_, err = provider.Signer(ctx)
+	require.ErrorContains(t, err, "only one of --fulcio-token-path or --fulcio-raw-token can be used")
 }
 
 func generateCertChain(t *testing.T) []string {
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJmb29iYXIiLCJuYW1lIjoiSm9obiBEb2UiLCJpYXQiOjE1MTYyMzkwMjIsIkVtYWlsIjoidGVzdEBpbi10b3RvLmlvIn0.IswtNc6aJL3zAf-lSGvuz7Okf2tBr-I3ulJ_SRUMt0k