From 14540c4b929fc181462bb3fc7945e64db6c566cd Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 21 Feb 2024 14:29:36 -0800 Subject: [PATCH] Improve error handling when TTL is invalid (#106) * Include a better error message if TTL is too large * Don't print usage on error * Allow ambiguous TTL error --- cli/consts.go | 2 +- cli/error.go | 78 +++++++++++++++++++++++++++++++++++++---------- cli/error_test.go | 39 ++++++++++++++++++++++++ cli/get.go | 14 +++++---- cli/main.go | 6 ++-- cli/root.go | 1 + go.mod | 2 ++ go.sum | 5 +++ 8 files changed, 122 insertions(+), 25 deletions(-) create mode 100644 cli/error_test.go diff --git a/cli/consts.go b/cli/consts.go index 4866d86f..7dfd8e32 100644 --- a/cli/consts.go +++ b/cli/consts.go @@ -12,7 +12,7 @@ var ( const ( // DefaultTTL for requested credentials in hours - DefaultTTL uint = 8 + DefaultTTL uint = 1 // DefaultTimeRemaining for new key requests in minutes DefaultTimeRemaining uint = 5 LinuxAmd64BinaryName string = "keyconjurer-linux-amd64" diff --git a/cli/error.go b/cli/error.go index c47dc9ab..eb214217 100644 --- a/cli/error.go +++ b/cli/error.go @@ -1,18 +1,22 @@ package main import ( + "errors" "fmt" "strings" + "time" + + "github.com/aws/aws-sdk-go/aws/awserr" ) const ( - ExitCodeTokensExpiredOrAbsent uint8 = 0x1 - ExitCodeUndisclosedOktaError = 0x2 - ExitCodeAuthenticationError = 0x3 - ExitCodeConnectivityError = 0x4 - ExitCodeValueError = 0x5 - ExitCodeAWSError = 0x6 - ExitCodeUnknownError = 0x7D + ExitCodeTokensExpiredOrAbsent int = 0x1 + ExitCodeUndisclosedOktaError int = 0x2 + ExitCodeAuthenticationError int = 0x3 + ExitCodeConnectivityError int = 0x4 + ExitCodeValueError int = 0x5 + ExitCodeAWSError int = 0x6 + ExitCodeUnknownError int = 0x7D ) var ( @@ -25,25 +29,25 @@ var ( type genericError struct { Message string - ExitCode uint8 + ExitCode int } func (e genericError) Error() string { return e.Message } -func (e genericError) Code() uint8 { +func (e genericError) Code() int { return e.ExitCode } type codeError interface { Error() string - Code() uint8 + Code() int } // UsageError indicates that the user used the program incorrectly type UsageError struct { - ExitCode uint8 + ExitCode int Description string DebugMessage string } @@ -52,7 +56,7 @@ func (u UsageError) Error() string { return u.Description } -func (u UsageError) Code() uint8 { +func (u UsageError) Code() int { return u.ExitCode } @@ -85,7 +89,7 @@ func (v ValueError) Error() string { return fmt.Sprintf("provided value %s was not valid (accepted values: %s)", v.Value, acceptable) } -func (v ValueError) Code() uint8 { +func (v ValueError) Code() int { return ExitCodeValueError } @@ -102,7 +106,7 @@ func (o OktaError) Error() string { return o.Message } -func (o OktaError) Code() uint8 { +func (o OktaError) Code() int { return ExitCodeUndisclosedOktaError } @@ -116,9 +120,51 @@ func (o AWSError) Unwrap() error { } func (o AWSError) Error() string { - return o.Message + return fmt.Sprintf("%s: %s", o.Message, o.InnerError) } -func (o AWSError) Code() uint8 { +func (o AWSError) Code() int { return ExitCodeAWSError } + +type TimeToLiveError struct { + MaxDuration time.Duration + RequestedDuration time.Duration +} + +func (o TimeToLiveError) Code() int { + return ExitCodeValueError +} + +func (o TimeToLiveError) Error() string { + if o.MaxDuration == 0 && o.RequestedDuration == 0 { + // Duration is ambiguous/was not specified by AWS, so we return a generic message instead. + return "the TTL you requested exceeds the maximum TTL for this configuration" + } + + // We cast to int to discard decimal places + return fmt.Sprintf("you requested a TTL of %d hours, but the maximum for this configuration is %d hours", int(o.RequestedDuration.Hours()), int(o.MaxDuration.Hours())) +} + +// tryParseTimeToLiveError attempts to parse an error related to the DurationSeconds field in the STS request. +// +// If the given error does relate to the specified DurationSeconds being larger than MaxDurationSeconds, this function will return a more specific error than the one the AWS SDK provides, and returns true. +// Returns nil and false in all other situations. +func tryParseTimeToLiveError(err error) (error, bool) { + var awsErr awserr.Error + if errors.As(err, &awsErr) && awsErr.Code() == "ValidationError" { + var providedValue, maxValue time.Duration + // This is no more specific type than this, and yes, unfortunately the error message includes the count. + formatOne := "1 validation error detected: Value '%d' at 'durationSeconds' failed to satisfy constraint: Member must have value less than or equal to %d" + if n, parseErr := fmt.Sscanf(awsErr.Message(), formatOne, &providedValue, &maxValue); parseErr == nil && n == 2 { + return TimeToLiveError{MaxDuration: maxValue * time.Second, RequestedDuration: providedValue * time.Second}, true + } + + formatAmbiguousMaximum := "The requested DurationSeconds exceeds the MaxSessionDuration set for this role." + if strings.Compare(awsErr.Message(), formatAmbiguousMaximum) == 0 { + return TimeToLiveError{MaxDuration: 0, RequestedDuration: 0}, true + } + } + + return nil, false +} diff --git a/cli/error_test.go b/cli/error_test.go new file mode 100644 index 00000000..356eddde --- /dev/null +++ b/cli/error_test.go @@ -0,0 +1,39 @@ +package main + +import ( + "testing" + "time" + + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/stretchr/testify/require" +) + +func Test_tryParseTimeToLiveError(t *testing.T) { + t.Run("UnambiguousAmount", func(t *testing.T) { + validationError := awserr.New("ValidationError", "1 validation error detected: Value '86400' at 'durationSeconds' failed to satisfy constraint: Member must have value less than or equal to 43200", nil) + err, ok := tryParseTimeToLiveError(validationError) + + require.True(t, ok) + require.NotNil(t, err) + require.Equal(t, err.Error(), "you requested a TTL of 24 hours, but the maximum for this configuration is 12 hours") + var ttlError TimeToLiveError + require.ErrorAs(t, err, &ttlError) + require.Equal(t, ttlError.MaxDuration, 43200*time.Second) + require.Equal(t, ttlError.RequestedDuration, 86400*time.Second) + require.Equal(t, ttlError.Code(), ExitCodeValueError) + }) + + t.Run("AmbiguousAmount", func(t *testing.T) { + validationError := awserr.New("ValidationError", "The requested DurationSeconds exceeds the MaxSessionDuration set for this role.", nil) + err, ok := tryParseTimeToLiveError(validationError) + + require.True(t, ok) + require.NotNil(t, err) + require.Equal(t, err.Error(), "the TTL you requested exceeds the maximum TTL for this configuration") + var ttlError TimeToLiveError + require.ErrorAs(t, err, &ttlError) + require.Equal(t, ttlError.MaxDuration, time.Duration(0)) + require.Equal(t, ttlError.RequestedDuration, time.Duration(0)) + require.Equal(t, ttlError.Code(), ExitCodeValueError) + }) +} diff --git a/cli/get.go b/cli/get.go index f985559b..f5052d8a 100644 --- a/cli/get.go +++ b/cli/get.go @@ -107,11 +107,6 @@ A role must be specified when using this command through the --role flag. You ma return ValueError{Value: shellType, ValidValues: permittedShellTypes} } - // make sure we enforce limit - if ttl > 8 { - ttl = 8 - } - var accountID string if len(args) > 0 { accountID = args[0] @@ -177,8 +172,15 @@ A role must be specified when using this command through the --role flag. You ma SAMLAssertion: &assertionStr, }) + if err, ok := tryParseTimeToLiveError(err); ok { + return err + } + if err != nil { - return AWSError{InnerError: err, Message: "failed to exchange credentials"} + return AWSError{ + InnerError: err, + Message: "failed to exchange credentials", + } } credentials = CloudCredentials{ diff --git a/cli/main.go b/cli/main.go index ba011e36..7f5f43da 100644 --- a/cli/main.go +++ b/cli/main.go @@ -5,6 +5,7 @@ import ( "os" "strings" + "github.com/spf13/cobra" "golang.org/x/exp/slog" ) @@ -28,10 +29,11 @@ func main() { err := rootCmd.Execute() var codeErr codeError if errors.As(err, &codeErr) { - rootCmd.PrintErrf("keyconjurer: %s\n", codeErr.Error()) + cobra.CheckErr(codeErr) os.Exit(int(codeErr.Code())) } else if err != nil { - rootCmd.PrintErrf("keyconjurer: %s\n", err.Error()) + // Probably a cobra error. + cobra.CheckErr(err) os.Exit(ExitCodeUnknownError) } } diff --git a/cli/root.go b/cli/root.go index 14522f67..07838e85 100644 --- a/cli/root.go +++ b/cli/root.go @@ -102,4 +102,5 @@ To get started run the following commands: return config.Write(file) }, SilenceErrors: true, + SilenceUsage: true, } diff --git a/go.mod b/go.mod index 88573377..cb2deed1 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ require ( github.com/RobotsAndPencils/go-saml v0.0.0-20170520135329-fb13cb52a46b github.com/aws/aws-lambda-go v1.19.1 github.com/aws/aws-sdk-go v1.34.19 + github.com/aws/aws-sdk-go-v2/service/ec2 v1.148.2 github.com/coreos/go-oidc v2.2.1+incompatible github.com/go-ini/ini v1.61.0 github.com/hashicorp/go-rootcerts v1.0.2 @@ -24,6 +25,7 @@ require ( ) require ( + github.com/aws/smithy-go v1.20.1 // indirect github.com/cenkalti/backoff/v4 v4.1.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/golang/protobuf v1.5.2 // indirect diff --git a/go.sum b/go.sum index f6af4ac3..f9aecef1 100644 --- a/go.sum +++ b/go.sum @@ -10,6 +10,11 @@ github.com/aws/aws-lambda-go v1.19.1/go.mod h1:jJmlefzPfGnckuHdXX7/80O3BvUUi12XO github.com/aws/aws-sdk-go v1.34.10/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= github.com/aws/aws-sdk-go v1.34.19 h1:x3MMvAJ1nfWviixEduchBSs65DgY5Y2pA2/NAcxVGOo= github.com/aws/aws-sdk-go v1.34.19/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= +github.com/aws/aws-sdk-go-v2 v1.25.1 h1:P7hU6A5qEdmajGwvae/zDkOq+ULLC9tQBTwqqiwFGpI= +github.com/aws/aws-sdk-go-v2/service/ec2 v1.148.2 h1:1oOlVyfM5Lzn/XKjqoVyy2i4OQhqOPaqYg3Jk+cZ4FE= +github.com/aws/aws-sdk-go-v2/service/ec2 v1.148.2/go.mod h1:7MUTgVVnC1GAxx4SNQqzQalrm1n4v1HYa/R/LEB3CKo= +github.com/aws/smithy-go v1.20.1 h1:4SZlSlMr36UEqC7XOyRVb27XMeZubNcBNN+9IgEPIQw= +github.com/aws/smithy-go v1.20.1/go.mod h1:krry+ya/rV9RDcV/Q16kpu6ypI4K2czasz0NC3qS14E= github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs= github.com/cenkalti/backoff/v4 v4.1.0 h1:c8LkOFQTzuO0WBM/ae5HdGQuZPfPxp7lqBRwQRm4fSc= github.com/cenkalti/backoff/v4 v4.1.0/go.mod h1:scbssz8iZGpm3xbr14ovlUdkxfGXNInqkPWOWmG2CLw=