diff --git a/app/cli/cmd/auth_delete_account.go b/app/cli/cmd/auth_delete_account.go index 982066d13..4d6d00380 100644 --- a/app/cli/cmd/auth_delete_account.go +++ b/app/cli/cmd/auth_delete_account.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -31,7 +31,7 @@ func newAuthDeleteAccountCmd() *cobra.Command { Short: "delete your account", RunE: func(cmd *cobra.Command, args []string) error { // Get user information to make sure the user knows the account she is deleting - contextResp, err := action.NewConfigCurrentContext(ActionOpts).Run() + contextResp, err := action.NewConfigCurrentContext(ActionOpts).Run(cmd.Context()) if err != nil { return err } diff --git a/app/cli/cmd/auth_login.go b/app/cli/cmd/auth_login.go index 698cc7cd9..6897a687e 100644 --- a/app/cli/cmd/auth_login.go +++ b/app/cli/cmd/auth_login.go @@ -30,7 +30,6 @@ import ( pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1" "github.com/chainloop-dev/chainloop/internal/oauth" - jwt "github.com/golang-jwt/jwt/v5" "github.com/spf13/cobra" "github.com/spf13/viper" "golang.org/x/oauth2" @@ -90,7 +89,7 @@ func newAuthLoginCmd() *cobra.Command { Use: "login", Short: "authenticate the CLI with the Control Plane", RunE: func(cmd *cobra.Command, args []string) error { - return interactiveAuth(forceHeadlessLogin) + return interactiveAuth(cmd.Context(), forceHeadlessLogin) }, } @@ -98,7 +97,7 @@ func newAuthLoginCmd() *cobra.Command { return cmd } -func interactiveAuth(forceHeadless bool) error { +func interactiveAuth(ctx context.Context, forceHeadless bool) error { var a app listener, callbackURL, err := localListenerAndCallbackURL() @@ -122,7 +121,7 @@ func interactiveAuth(forceHeadless bool) error { serverLoginURL.RawQuery = q.Encode() if forceHeadless || isRemoteSession() { - return headlessAuth(serverLoginURL) + return headlessAuth(ctx, serverLoginURL) } a.callback = &loginCallback{ @@ -140,7 +139,7 @@ func interactiveAuth(forceHeadless bool) error { err = openbrowser(serverLoginURL.String()) if err != nil { logger.Debug().Err(err).Msg("falling back to manual login") - return headlessAuth(serverLoginURL) + return headlessAuth(ctx, serverLoginURL) } a.serverChan = make(chan error) @@ -202,7 +201,7 @@ func toHeadlessLoginURL(loginURL *url.URL) { loginURL.RawQuery = q.Encode() } -func headlessAuth(loginURL *url.URL) error { +func headlessAuth(ctx context.Context, loginURL *url.URL) error { toHeadlessLoginURL(loginURL) fmt.Printf("To authenticate, click on the following link and paste the result back here\n\n %s\n\n", loginURL.String()) @@ -212,22 +211,57 @@ func headlessAuth(loginURL *url.URL) error { return fmt.Errorf("retrieving password from stdin: %w", err) } - // We just want to check that it is a token, the actual verification will happen when it is sent to the server - // To be clear, this is just a best effort sanity check - if _, _, err := new(jwt.Parser).ParseUnverified(string(token), &jwt.MapClaims{}); err != nil { - return errors.New("invalid token") + fmt.Println("") + if err := verifyAndSaveAuthToken(ctx, string(token)); err != nil { + return err + } + + logger.Info().Msg("login successful!") + + return nil +} + +// loginVerifyTimeout bounds the call that checks the login token, so a stalled +// control plane does not block the login +const loginVerifyTimeout = 30 * time.Second + +// verifyAndSaveAuthToken saves the token to the config file only when the control plane +// accepts it, so that the login only reports success for a token that works +func verifyAndSaveAuthToken(ctx context.Context, token string) error { + if err := checkAuthToken(ctx, token, verifyAuthToken); err != nil { + return err } - if err := saveAuthToken(string(token)); err != nil { + if err := saveAuthToken(token); err != nil { return fmt.Errorf("storing token in config file: %w", err) } - fmt.Println("") - logger.Info().Msg("login successful!") + return nil +} + +// checkAuthToken makes sure that verify accepts the token. A rejected token keeps its +// gRPC status, so the CLI shows its usual authentication message +func checkAuthToken(ctx context.Context, token string, verify func(ctx context.Context, token string) error) error { + if token == "" { + return errors.New("no token was entered") + } + + if err := verify(ctx, token); err != nil { + return fmt.Errorf("verifying the token with the control plane: %w", err) + } return nil } +// verifyAuthToken asks the control plane for the session of the token +func verifyAuthToken(ctx context.Context, token string) error { + ctx, cancel := context.WithTimeout(ctx, loginVerifyTimeout) + defer cancel() + + _, err := fetchCurrentContext(ctx, token) + return err +} + // Save token to config file func saveAuthToken(token string) error { viper.Set(confOptions.authToken.viperKey, token) @@ -241,17 +275,16 @@ func (a *app) handleCallback(w http.ResponseWriter, r *http.Request) { }() token, err := a.callback.token(r) + if err == nil { + err = verifyAndSaveAuthToken(r.Context(), token) + } + if err != nil { w.WriteHeader(http.StatusExpectationFailed) fmt.Fprintln(w, "login failed, please check the terminal") return } - if err = saveAuthToken(token); err != nil { - w.WriteHeader(http.StatusInternalServerError) - return - } - logger.Info().Msg("login successful!") fmt.Fprintln(w, "login successful, you can now close this window and go back to the terminal") } @@ -259,6 +292,7 @@ func (a *app) handleCallback(w http.ResponseWriter, r *http.Request) { func openbrowser(url string) error { var cmd *exec.Cmd + //nolint:gosec // no shell is involved, the login URL from the control plane is a single argument switch runtime.GOOS { case "linux": cmd = exec.Command("xdg-open", url) diff --git a/app/cli/cmd/auth_login_test.go b/app/cli/cmd/auth_login_test.go index fe4265d9e..38e208d7e 100644 --- a/app/cli/cmd/auth_login_test.go +++ b/app/cli/cmd/auth_login_test.go @@ -23,10 +23,13 @@ import ( "net/url" "testing" + "github.com/chainloop-dev/chainloop/app/cli/pkg/action" "github.com/chainloop-dev/chainloop/internal/oauth" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/oauth2" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) func TestAddCodeChallenge(t *testing.T) { @@ -160,3 +163,61 @@ func TestLoginCallbackToken(t *testing.T) { }) } } + +func TestCheckAuthToken(t *testing.T) { + const token = "a-token" + + testCases := []struct { + name string + token string + verifyErr error + wantErr string + // whether the error renders as the CLI authentication message + wantAuthErr bool + }{ + { + name: "token accepted by the control plane", + token: token, + }, + { + name: "no token", + token: "", + wantErr: "no token was entered", + }, + { + name: "token rejected by the control plane", + token: token, + verifyErr: status.Error(codes.Unauthenticated, "token is invalid"), + wantErr: "verifying the token with the control plane", + wantAuthErr: true, + }, + { + name: "control plane error", + token: token, + verifyErr: errors.New("user not found"), + wantErr: "verifying the token with the control plane: user not found", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var verified bool + err := checkAuthToken(context.Background(), tc.token, func(_ context.Context, token string) error { + verified = true + assert.Equal(t, tc.token, token) + return tc.verifyErr + }) + + // Only a token that was entered goes to the control plane + assert.Equal(t, tc.token != "", verified) + if tc.wantErr != "" { + assert.ErrorContains(t, err, tc.wantErr) + _, isAuthErr := action.AuthErrorMessage(err) + assert.Equal(t, tc.wantAuthErr, isAuthErr) + return + } + + assert.NoError(t, err) + }) + } +} diff --git a/app/cli/cmd/organization_describe.go b/app/cli/cmd/organization_describe.go index 4153e5ee1..aef1a3464 100644 --- a/app/cli/cmd/organization_describe.go +++ b/app/cli/cmd/organization_describe.go @@ -31,7 +31,7 @@ func newOrganizationDescribeCmd() *cobra.Command { Aliases: []string{"current-context"}, Short: "Describe the current organization", RunE: func(cmd *cobra.Command, args []string) error { - res, err := action.NewConfigCurrentContext(ActionOpts).Run() + res, err := action.NewConfigCurrentContext(ActionOpts).Run(cmd.Context()) if err != nil { return err } diff --git a/app/cli/cmd/root.go b/app/cli/cmd/root.go index 5a46c90df..5a2e6eeb8 100644 --- a/app/cli/cmd/root.go +++ b/app/cli/cmd/root.go @@ -16,6 +16,7 @@ package cmd import ( + "context" "crypto/sha256" "encoding/hex" "errors" @@ -170,35 +171,15 @@ func ensureActionOpts(cmd *cobra.Command) error { } } - var opts = []grpcconn.Option{ - grpcconn.WithInsecure(apiInsecure()), - grpcconn.WithCLIVersion(fullVersion()), - } - - if maxRecv := apiMaxRecvMsgSize(); maxRecv > 0 { - opts = append(opts, grpcconn.WithMaxRecvMsgSize(maxRecv)) - } - - if caValue := viper.GetString(confOptions.controlplaneCA.viperKey); caValue != "" { - // Check if the value is a file path, if it is we read the content and encode it to base64, if not we assume it's the content already - if _, err := os.Stat(caValue); err == nil { - opts = append(opts, grpcconn.WithCAFile(caValue)) - } else { - opts = append(opts, grpcconn.WithCAContent(caValue)) - } - } + opts := controlPlaneConnOpts() controlplaneURL := viper.GetString(confOptions.controlplaneAPI.viperKey) // If no organization is selected, load it from the server and save it. orgName := effectiveOrganization(cmd) if orgName == "" { - conn, err := grpcconn.New(controlplaneURL, authToken, opts...) - if err != nil { - return err - } - - currentContext, err := action.NewConfigCurrentContext(newActionOpts(logger, conn, authToken)).Run() + // A connection error here also fails the connection below, which returns it + currentContext, err := fetchCurrentContext(cmd.Context(), authToken) if err == nil && currentContext.CurrentMembership != nil { if err := setLocalOrganization(currentContext.CurrentMembership.Org.Name); err != nil { return fmt.Errorf("writing config file: %w", err) @@ -235,6 +216,42 @@ func ensureActionOpts(cmd *cobra.Command) error { return nil } +// controlPlaneConnOpts returns the gRPC options for a control plane connection, +// without the organization +func controlPlaneConnOpts() []grpcconn.Option { + opts := []grpcconn.Option{ + grpcconn.WithInsecure(apiInsecure()), + grpcconn.WithCLIVersion(fullVersion()), + } + + if maxRecv := apiMaxRecvMsgSize(); maxRecv > 0 { + opts = append(opts, grpcconn.WithMaxRecvMsgSize(maxRecv)) + } + + if caValue := viper.GetString(confOptions.controlplaneCA.viperKey); caValue != "" { + // Check if the value is a file path, if it is we read the content and encode it to base64, if not we assume it's the content already + if _, err := os.Stat(caValue); err == nil { + opts = append(opts, grpcconn.WithCAFile(caValue)) + } else { + opts = append(opts, grpcconn.WithCAContent(caValue)) + } + } + + return opts +} + +// fetchCurrentContext asks the control plane who the token belongs to, on a connection +// without an organization, so it also works for a user that is not a member of one +func fetchCurrentContext(ctx context.Context, token string) (*action.ConfigContextItem, error) { + conn, err := grpcconn.New(viper.GetString(confOptions.controlplaneAPI.viperKey), token, controlPlaneConnOpts()...) + if err != nil { + return nil, err + } + defer conn.Close() + + return action.NewConfigCurrentContext(newActionOpts(logger, conn, token)).Run(ctx) +} + func NewRootCmd(l zerolog.Logger) *cobra.Command { // Seed the root logger here and not only in PersistentPreRunE: cobra // rejects unknown subcommands and invalid flags before that hook runs, and diff --git a/app/cli/pkg/action/config_current_context.go b/app/cli/pkg/action/config_current_context.go index 7cb7bf05c..aca29af32 100644 --- a/app/cli/pkg/action/config_current_context.go +++ b/app/cli/pkg/action/config_current_context.go @@ -74,9 +74,9 @@ func (u *UserItem) PrintUserProfileWithEmail() string { return result } -func (action *ConfigCurrentContext) Run() (*ConfigContextItem, error) { +func (action *ConfigCurrentContext) Run(ctx context.Context) (*ConfigContextItem, error) { client := pb.NewContextServiceClient(action.cfg.CPConnection) - resp, err := client.Current(context.Background(), &pb.ContextServiceCurrentRequest{}) + resp, err := client.Current(ctx, &pb.ContextServiceCurrentRequest{}) if err != nil { return nil, err } diff --git a/app/cli/pkg/action/trace_attestation.go b/app/cli/pkg/action/trace_attestation.go index 91507c905..739e657e0 100644 --- a/app/cli/pkg/action/trace_attestation.go +++ b/app/cli/pkg/action/trace_attestation.go @@ -159,12 +159,12 @@ func casCAPath() string { return viper.GetString("artifact-cas.api-ca") } func casInsecure() bool { return viper.GetBool("api-insecure") } // CheckAuth verifies the CLI can reach the control plane. -func (e *AttestationExecutor) CheckAuth(_ context.Context) error { +func (e *AttestationExecutor) CheckAuth(ctx context.Context) error { _, err := NewConfigCurrentContext(&ActionsOpts{ CPConnection: e.actionOpts.CPConnection, Logger: e.actionOpts.Logger, AuthTokenRaw: e.actionOpts.AuthTokenRaw, - }).Run() + }).Run(ctx) if err != nil { // The wrap keeps the gRPC status reachable with errors.As, so an // authentication failure still renders through AuthErrorMessage;