Skip to content

Commit

Permalink
Improve error handling when TTL is invalid (#106)
Browse files Browse the repository at this point in the history
* Include a better error message if TTL is too large

* Don't print usage on error

* Allow ambiguous TTL error
  • Loading branch information
punmechanic authored Feb 21, 2024
1 parent a8c2d4b commit 14540c4
Show file tree
Hide file tree
Showing 8 changed files with 122 additions and 25 deletions.
2 changes: 1 addition & 1 deletion cli/consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
78 changes: 62 additions & 16 deletions cli/error.go
Original file line number Diff line number Diff line change
@@ -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 (
Expand All @@ -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
}
Expand All @@ -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
}

Expand Down Expand Up @@ -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
}

Expand All @@ -102,7 +106,7 @@ func (o OktaError) Error() string {
return o.Message
}

func (o OktaError) Code() uint8 {
func (o OktaError) Code() int {
return ExitCodeUndisclosedOktaError
}

Expand All @@ -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
}
39 changes: 39 additions & 0 deletions cli/error_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
14 changes: 8 additions & 6 deletions cli/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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{
Expand Down
6 changes: 4 additions & 2 deletions cli/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os"
"strings"

"github.com/spf13/cobra"
"golang.org/x/exp/slog"
)

Expand All @@ -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)
}
}
1 change: 1 addition & 0 deletions cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,5 @@ To get started run the following commands:
return config.Write(file)
},
SilenceErrors: true,
SilenceUsage: true,
}
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
5 changes: 5 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down

0 comments on commit 14540c4

Please sign in to comment.