From 713d5d53079a89305db19415673009d722422972 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Sat, 7 Oct 2017 18:41:07 -0400 Subject: [PATCH] Don't swallow errors on token functions. --- api/secret.go | 156 +++----- api/secret_test.go | 688 ++++++++++++++++---------------- builtin/credential/token/cli.go | 36 +- command/login_test.go | 5 +- helper/parseutil/parseutil.go | 37 ++ 5 files changed, 461 insertions(+), 461 deletions(-) diff --git a/api/secret.go b/api/secret.go index 14962d92b..ef6d647c8 100644 --- a/api/secret.go +++ b/api/secret.go @@ -1,12 +1,12 @@ package api import ( - "encoding/json" + "fmt" "io" - "strconv" "time" "github.com/hashicorp/vault/helper/jsonutil" + "github.com/hashicorp/vault/helper/parseutil" ) // Secret is the structure returned for every secret within Vault. @@ -38,131 +38,125 @@ type Secret struct { } // TokenID returns the standardized token ID (token) for the given secret. -func (s *Secret) TokenID() string { +func (s *Secret) TokenID() (string, error) { if s == nil { - return "" + return "", nil } if s.Auth != nil && len(s.Auth.ClientToken) > 0 { - return s.Auth.ClientToken + return s.Auth.ClientToken, nil } if s.Data == nil || s.Data["id"] == nil { - return "" + return "", nil } id, ok := s.Data["id"].(string) if !ok { - return "" + return "", fmt.Errorf("token found but in the wrong format") } - return id + return id, nil } // TokenAccessor returns the standardized token accessor for the given secret. // If the secret is nil or does not contain an accessor, this returns the empty // string. -func (s *Secret) TokenAccessor() string { +func (s *Secret) TokenAccessor() (string, error) { if s == nil { - return "" + return "", nil } if s.Auth != nil && len(s.Auth.Accessor) > 0 { - return s.Auth.Accessor + return s.Auth.Accessor, nil } if s.Data == nil || s.Data["accessor"] == nil { - return "" + return "", nil } accessor, ok := s.Data["accessor"].(string) if !ok { - return "" + return "", fmt.Errorf("token found but in the wrong format") } - return accessor + return accessor, nil } // TokenRemainingUses returns the standardized remaining uses for the given -// secret. If the secret is nil or does not contain the "num_uses", this returns -// 0.. -func (s *Secret) TokenRemainingUses() int { +// secret. If the secret is nil or does not contain the "num_uses", this +// returns -1. On error, this will return -1 and a non-nil error. +func (s *Secret) TokenRemainingUses() (int, error) { if s == nil || s.Data == nil || s.Data["num_uses"] == nil { - return 0 + return -1, nil } - usesStr, ok := s.Data["num_uses"].(json.Number) - if !ok { - return 0 - } - - if string(usesStr) == "" { - return 0 - } - - uses, err := strconv.ParseInt(string(usesStr), 10, 64) + uses, err := parseutil.ParseInt(s.Data["num_uses"]) if err != nil { - return 0 + return 0, err } - return int(uses) + return int(uses), nil } // TokenPolicies returns the standardized list of policies for the given secret. // If the secret is nil or does not contain any policies, this returns nil. -// Policies are usually returned as []interface{}, but this function ensures -// they are []string. -func (s *Secret) TokenPolicies() []string { +func (s *Secret) TokenPolicies() ([]string, error) { if s == nil { - return nil + return nil, nil } if s.Auth != nil && len(s.Auth.Policies) > 0 { - return s.Auth.Policies + return s.Auth.Policies, nil } if s.Data == nil || s.Data["policies"] == nil { - return nil + return nil, nil + } + + sList, ok := s.Data["policies"].([]string) + if ok { + return sList, nil } list, ok := s.Data["policies"].([]interface{}) if !ok { - return nil + return nil, fmt.Errorf("unable to convert token policies to expected format") } policies := make([]string, len(list)) for i := range list { p, ok := list[i].(string) if !ok { - return nil + return nil, fmt.Errorf("unable to convert policy %v to string", list[i]) } policies[i] = p } - return policies + return policies, nil } // TokenMetadata returns the map of metadata associated with this token, if any // exists. If the secret is nil or does not contain the "metadata" key, this // returns nil. -func (s *Secret) TokenMetadata() map[string]string { +func (s *Secret) TokenMetadata() (map[string]string, error) { if s == nil { - return nil + return nil, nil } if s.Auth != nil && len(s.Auth.Metadata) > 0 { - return s.Auth.Metadata + return s.Auth.Metadata, nil } if s.Data == nil || (s.Data["metadata"] == nil && s.Data["meta"] == nil) { - return nil + return nil, nil } data, ok := s.Data["metadata"].(map[string]interface{}) if !ok { data, ok = s.Data["meta"].(map[string]interface{}) if !ok { - return nil + return nil, fmt.Errorf("unable to convert metadata field to expected format") } } @@ -170,99 +164,59 @@ func (s *Secret) TokenMetadata() map[string]string { for k, v := range data { typed, ok := v.(string) if !ok { - return nil + return nil, fmt.Errorf("unable to convert metadata value %v to string", v) } metadata[k] = typed } - return metadata + return metadata, nil } // TokenIsRenewable returns the standardized token renewability for the given // secret. If the secret is nil or does not contain the "renewable" key, this // returns false. -func (s *Secret) TokenIsRenewable() bool { +func (s *Secret) TokenIsRenewable() (bool, error) { if s == nil { - return false + return false, nil } if s.Auth != nil && s.Auth.Renewable { - return s.Auth.Renewable + return s.Auth.Renewable, nil } if s.Data == nil || s.Data["renewable"] == nil { - return false + return false, nil } - renewable, ok := s.Data["renewable"].(bool) - if !ok { - return false - } - - return renewable -} - -// TokenTTLInt returns the token's TTL as an integer number of seconds. -func (s *Secret) TokenTTLInt() int { - if s == nil { - return 0 - } - - if s.Auth != nil && s.Auth.LeaseDuration > 0 { - return s.Auth.LeaseDuration - } - - if s.Data == nil || s.Data["ttl"] == nil { - return 0 - } - - ttlStr, ok := s.Data["ttl"].(json.Number) - if !ok { - return 0 - } - - if string(ttlStr) == "" { - return 0 - } - - i, err := strconv.ParseInt(string(ttlStr), 0, 64) + renewable, err := parseutil.ParseBool(s.Data["renewable"]) if err != nil { - return 0 + return false, fmt.Errorf("could not convert renewable value to a boolean: %v", err) } - return int(i) + return renewable, nil } // TokenTTL returns the standardized remaining token TTL for the given secret. -// If the secret is nil or does not contain a TTL, this returns the 0. -func (s *Secret) TokenTTL() time.Duration { +// If the secret is nil or does not contain a TTL, this returns 0. +func (s *Secret) TokenTTL() (time.Duration, error) { if s == nil { - return 0 + return 0, nil } if s.Auth != nil && s.Auth.LeaseDuration > 0 { - return time.Duration(s.Auth.LeaseDuration) * time.Second + return time.Duration(s.Auth.LeaseDuration) * time.Second, nil } if s.Data == nil || s.Data["ttl"] == nil { - return 0 + return 0, nil } - ttlStr, ok := s.Data["ttl"].(json.Number) - if !ok { - return 0 - } - - if string(ttlStr) == "" { - return 0 - } - - ttl, err := time.ParseDuration(string(ttlStr) + "s") + ttl, err := parseutil.ParseDurationSecond(s.Data["ttl"]) if err != nil { - return 0 + return 0, err } - return ttl + return ttl, nil } // SecretWrapInfo contains wrapping information if we have it. If what is diff --git a/api/secret_test.go b/api/secret_test.go index 6ff12eea2..b4b0132bd 100644 --- a/api/secret_test.go +++ b/api/secret_test.go @@ -2,7 +2,6 @@ package api_test import ( "encoding/json" - "fmt" "reflect" "strings" "testing" @@ -69,11 +68,13 @@ func TestSecret_TokenID(t *testing.T) { name string secret *api.Secret exp string + err bool }{ { "nil", nil, "", + false, }, { "nil_auth", @@ -81,6 +82,7 @@ func TestSecret_TokenID(t *testing.T) { Auth: nil, }, "", + false, }, { "empty_auth_client_token", @@ -90,6 +92,7 @@ func TestSecret_TokenID(t *testing.T) { }, }, "", + false, }, { "real_auth_client_token", @@ -99,6 +102,7 @@ func TestSecret_TokenID(t *testing.T) { }, }, "my-token", + false, }, { "nil_data", @@ -106,6 +110,7 @@ func TestSecret_TokenID(t *testing.T) { Data: nil, }, "", + false, }, { "empty_data", @@ -113,6 +118,7 @@ func TestSecret_TokenID(t *testing.T) { Data: map[string]interface{}{}, }, "", + false, }, { "data_not_string", @@ -122,6 +128,7 @@ func TestSecret_TokenID(t *testing.T) { }, }, "", + true, }, { "data_string", @@ -131,6 +138,7 @@ func TestSecret_TokenID(t *testing.T) { }, }, "my-token", + false, }, } @@ -140,7 +148,10 @@ func TestSecret_TokenID(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - act := tc.secret.TokenID() + act, err := tc.secret.TokenID() + if err != nil && !tc.err { + t.Fatal(err) + } if act != tc.exp { t.Errorf("expected %q to be %q", act, tc.exp) } @@ -171,8 +182,12 @@ func TestSecret_TokenID(t *testing.T) { } token := secret.Auth.ClientToken - if secret.TokenID() != token { - t.Errorf("expected %q to be %q", secret.TokenID(), token) + tokenID, err := secret.TokenID() + if err != nil { + t.Fatal(err) + } + if tokenID != token { + t.Errorf("expected %q to be %q", tokenID, token) } }) @@ -190,8 +205,12 @@ func TestSecret_TokenID(t *testing.T) { } token := secret.Auth.ClientToken - if secret.TokenID() != token { - t.Errorf("expected %q to be %q", secret.TokenID(), token) + tokenID, err := secret.TokenID() + if err != nil { + t.Fatal(err) + } + if tokenID != token { + t.Errorf("expected %q to be %q", tokenID, token) } }) @@ -214,8 +233,12 @@ func TestSecret_TokenID(t *testing.T) { t.Fatal(err) } - if secret.TokenID() != token { - t.Errorf("expected %q to be %q", secret.TokenID(), token) + tokenID, err := secret.TokenID() + if err != nil { + t.Fatal(err) + } + if tokenID != token { + t.Errorf("expected %q to be %q", tokenID, token) } }) @@ -239,8 +262,12 @@ func TestSecret_TokenID(t *testing.T) { t.Fatal(err) } - if secret.TokenID() != token { - t.Errorf("expected %q to be %q", secret.TokenID(), token) + tokenID, err := secret.TokenID() + if err != nil { + t.Fatal(err) + } + if tokenID != token { + t.Errorf("expected %q to be %q", tokenID, token) } }) @@ -263,8 +290,12 @@ func TestSecret_TokenID(t *testing.T) { t.Fatal(err) } - if secret.TokenID() != token { - t.Errorf("expected %q to be %q", secret.TokenID(), token) + tokenID, err := secret.TokenID() + if err != nil { + t.Fatal(err) + } + if tokenID != token { + t.Errorf("expected %q to be %q", tokenID, token) } }) @@ -288,8 +319,12 @@ func TestSecret_TokenID(t *testing.T) { t.Fatal(err) } - if secret.TokenID() != token { - t.Errorf("expected %q to be %q", secret.TokenID(), token) + tokenID, err := secret.TokenID() + if err != nil { + t.Fatal(err) + } + if tokenID != token { + t.Errorf("expected %q to be %q", tokenID, token) } }) } @@ -301,11 +336,13 @@ func TestSecret_TokenAccessor(t *testing.T) { name string secret *api.Secret exp string + err bool }{ { "nil", nil, "", + false, }, { "nil_auth", @@ -313,6 +350,7 @@ func TestSecret_TokenAccessor(t *testing.T) { Auth: nil, }, "", + false, }, { "empty_auth_accessor", @@ -322,6 +360,7 @@ func TestSecret_TokenAccessor(t *testing.T) { }, }, "", + false, }, { "real_auth_accessor", @@ -331,6 +370,7 @@ func TestSecret_TokenAccessor(t *testing.T) { }, }, "my-accessor", + false, }, { "nil_data", @@ -338,6 +378,7 @@ func TestSecret_TokenAccessor(t *testing.T) { Data: nil, }, "", + false, }, { "empty_data", @@ -345,6 +386,7 @@ func TestSecret_TokenAccessor(t *testing.T) { Data: map[string]interface{}{}, }, "", + false, }, { "data_not_string", @@ -354,6 +396,7 @@ func TestSecret_TokenAccessor(t *testing.T) { }, }, "", + true, }, { "data_string", @@ -363,6 +406,7 @@ func TestSecret_TokenAccessor(t *testing.T) { }, }, "my-accessor", + false, }, } @@ -372,7 +416,10 @@ func TestSecret_TokenAccessor(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - act := tc.secret.TokenAccessor() + act, err := tc.secret.TokenAccessor() + if err != nil && !tc.err { + t.Fatal(err) + } if act != tc.exp { t.Errorf("expected %q to be %q", act, tc.exp) } @@ -403,8 +450,12 @@ func TestSecret_TokenAccessor(t *testing.T) { } _, accessor := secret.Auth.ClientToken, secret.Auth.Accessor - if secret.TokenAccessor() != accessor { - t.Errorf("expected %q to be %q", secret.TokenAccessor(), accessor) + newAccessor, err := secret.TokenAccessor() + if err != nil { + t.Fatal(err) + } + if newAccessor != accessor { + t.Errorf("expected %q to be %q", newAccessor, accessor) } }) @@ -422,8 +473,12 @@ func TestSecret_TokenAccessor(t *testing.T) { } _, accessor := secret.Auth.ClientToken, secret.Auth.Accessor - if secret.TokenAccessor() != accessor { - t.Errorf("expected %q to be %q", secret.TokenAccessor(), accessor) + newAccessor, err := secret.TokenAccessor() + if err != nil { + t.Fatal(err) + } + if newAccessor != accessor { + t.Errorf("expected %q to be %q", newAccessor, accessor) } }) @@ -446,8 +501,12 @@ func TestSecret_TokenAccessor(t *testing.T) { t.Fatal(err) } - if secret.TokenAccessor() != accessor { - t.Errorf("expected %q to be %q", secret.TokenAccessor(), accessor) + newAccessor, err := secret.TokenAccessor() + if err != nil { + t.Fatal(err) + } + if newAccessor != accessor { + t.Errorf("expected %q to be %q", newAccessor, accessor) } }) @@ -471,8 +530,12 @@ func TestSecret_TokenAccessor(t *testing.T) { t.Fatal(err) } - if secret.TokenAccessor() != accessor { - t.Errorf("expected %q to be %q", secret.TokenAccessor(), accessor) + newAccessor, err := secret.TokenAccessor() + if err != nil { + t.Fatal(err) + } + if newAccessor != accessor { + t.Errorf("expected %q to be %q", newAccessor, accessor) } }) @@ -495,8 +558,12 @@ func TestSecret_TokenAccessor(t *testing.T) { t.Fatal(err) } - if secret.TokenAccessor() != accessor { - t.Errorf("expected %q to be %q", secret.TokenAccessor(), accessor) + newAccessor, err := secret.TokenAccessor() + if err != nil { + t.Fatal(err) + } + if newAccessor != accessor { + t.Errorf("expected %q to be %q", newAccessor, accessor) } }) @@ -520,8 +587,12 @@ func TestSecret_TokenAccessor(t *testing.T) { t.Fatal(err) } - if secret.TokenAccessor() != accessor { - t.Errorf("expected %q to be %q", secret.TokenAccessor(), accessor) + newAccessor, err := secret.TokenAccessor() + if err != nil { + t.Fatal(err) + } + if newAccessor != accessor { + t.Errorf("expected %q to be %q", newAccessor, accessor) } }) } @@ -537,21 +608,21 @@ func TestSecret_TokenRemainingUses(t *testing.T) { { "nil", nil, - 0, + -1, }, { "nil_data", &api.Secret{ Data: nil, }, - 0, + -1, }, { "empty_data", &api.Secret{ Data: map[string]interface{}{}, }, - 0, + -1, }, { "data_not_json_number", @@ -560,7 +631,7 @@ func TestSecret_TokenRemainingUses(t *testing.T) { "num_uses": 123, }, }, - 0, + 123, }, { "data_json_number", @@ -579,7 +650,10 @@ func TestSecret_TokenRemainingUses(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - act := tc.secret.TokenRemainingUses() + act, err := tc.secret.TokenRemainingUses() + if tc.exp != -1 && err != nil { + t.Fatal(err) + } if act != tc.exp { t.Errorf("expected %d to be %d", act, tc.exp) } @@ -613,9 +687,13 @@ func TestSecret_TokenRemainingUses(t *testing.T) { } // Remaining uses is not returned from this API - uses = 0 - if secret.TokenRemainingUses() != uses { - t.Errorf("expected %d to be %d", secret.TokenRemainingUses(), uses) + uses = -1 + remaining, err := secret.TokenRemainingUses() + if err != nil { + t.Fatal(err) + } + if remaining != uses { + t.Errorf("expected %d to be %d", remaining, uses) } }) @@ -636,9 +714,13 @@ func TestSecret_TokenRemainingUses(t *testing.T) { } // /auth/token/create does not return the number of uses - uses = 0 - if secret.TokenRemainingUses() != uses { - t.Errorf("expected %d to be %d", secret.TokenRemainingUses(), uses) + uses = -1 + remaining, err := secret.TokenRemainingUses() + if err != nil { + t.Fatal(err) + } + if remaining != uses { + t.Errorf("expected %d to be %d", remaining, uses) } }) @@ -664,8 +746,12 @@ func TestSecret_TokenRemainingUses(t *testing.T) { t.Fatal(err) } - if secret.TokenRemainingUses() != uses { - t.Errorf("expected %d to be %d", secret.TokenRemainingUses(), uses) + remaining, err := secret.TokenRemainingUses() + if err != nil { + t.Fatal(err) + } + if remaining != uses { + t.Errorf("expected %d to be %d", remaining, uses) } }) @@ -693,8 +779,12 @@ func TestSecret_TokenRemainingUses(t *testing.T) { } uses = uses - 1 // we just used it - if secret.TokenRemainingUses() != uses { - t.Errorf("expected %d to be %d", secret.TokenRemainingUses(), uses) + remaining, err := secret.TokenRemainingUses() + if err != nil { + t.Fatal(err) + } + if remaining != uses { + t.Errorf("expected %d to be %d", remaining, uses) } }) @@ -721,9 +811,13 @@ func TestSecret_TokenRemainingUses(t *testing.T) { } // /auth/token/renew does not return the number of uses - uses = 0 - if secret.TokenRemainingUses() != uses { - t.Errorf("expected %d to be %d", secret.TokenRemainingUses(), uses) + uses = -1 + remaining, err := secret.TokenRemainingUses() + if err != nil { + t.Fatal(err) + } + if remaining != uses { + t.Errorf("expected %d to be %d", remaining, uses) } }) @@ -751,9 +845,13 @@ func TestSecret_TokenRemainingUses(t *testing.T) { } // /auth/token/renew-self does not return the number of uses - uses = 0 - if secret.TokenRemainingUses() != uses { - t.Errorf("expected %d to be %d", secret.TokenRemainingUses(), uses) + uses = -1 + remaining, err := secret.TokenRemainingUses() + if err != nil { + t.Fatal(err) + } + if remaining != uses { + t.Errorf("expected %d to be %d", remaining, uses) } }) } @@ -765,11 +863,13 @@ func TestSecret_TokenPolicies(t *testing.T) { name string secret *api.Secret exp []string + err bool }{ { "nil", nil, nil, + false, }, { "nil_auth", @@ -777,6 +877,7 @@ func TestSecret_TokenPolicies(t *testing.T) { Auth: nil, }, nil, + false, }, { "nil_auth_policies", @@ -786,6 +887,7 @@ func TestSecret_TokenPolicies(t *testing.T) { }, }, nil, + false, }, { "empty_auth_policies", @@ -795,6 +897,7 @@ func TestSecret_TokenPolicies(t *testing.T) { }, }, nil, + false, }, { "real_auth_policies", @@ -804,6 +907,7 @@ func TestSecret_TokenPolicies(t *testing.T) { }, }, []string{"foo"}, + false, }, { "nil_data", @@ -811,6 +915,7 @@ func TestSecret_TokenPolicies(t *testing.T) { Data: nil, }, nil, + false, }, { "empty_data", @@ -818,6 +923,7 @@ func TestSecret_TokenPolicies(t *testing.T) { Data: map[string]interface{}{}, }, nil, + false, }, { "data_not_slice", @@ -827,6 +933,7 @@ func TestSecret_TokenPolicies(t *testing.T) { }, }, nil, + true, }, { "data_slice", @@ -836,6 +943,7 @@ func TestSecret_TokenPolicies(t *testing.T) { }, }, []string{"foo"}, + false, }, } @@ -845,7 +953,10 @@ func TestSecret_TokenPolicies(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - act := tc.secret.TokenPolicies() + act, err := tc.secret.TokenPolicies() + if err != nil && !tc.err { + t.Fatal(err) + } if !reflect.DeepEqual(act, tc.exp) { t.Errorf("expected %#v to be %#v", act, tc.exp) } @@ -877,8 +988,12 @@ func TestSecret_TokenPolicies(t *testing.T) { t.Fatal(err) } - if !reflect.DeepEqual(secret.TokenPolicies(), policies) { - t.Errorf("expected %#v to be %#v", secret.TokenPolicies(), policies) + tPol, err := secret.TokenPolicies() + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(tPol, policies) { + t.Errorf("expected %#v to be %#v", tPol, policies) } }) @@ -897,8 +1012,12 @@ func TestSecret_TokenPolicies(t *testing.T) { t.Fatal(err) } - if !reflect.DeepEqual(secret.TokenPolicies(), policies) { - t.Errorf("expected %#v to be %#v", secret.TokenPolicies(), policies) + tPol, err := secret.TokenPolicies() + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(tPol, policies) { + t.Errorf("expected %#v to be %#v", tPol, policies) } }) @@ -923,8 +1042,12 @@ func TestSecret_TokenPolicies(t *testing.T) { t.Fatal(err) } - if !reflect.DeepEqual(secret.TokenPolicies(), policies) { - t.Errorf("expected %#v to be %#v", secret.TokenPolicies(), policies) + tPol, err := secret.TokenPolicies() + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(tPol, policies) { + t.Errorf("expected %#v to be %#v", tPol, policies) } }) @@ -950,8 +1073,12 @@ func TestSecret_TokenPolicies(t *testing.T) { t.Fatal(err) } - if !reflect.DeepEqual(secret.TokenPolicies(), policies) { - t.Errorf("expected %#v to be %#v", secret.TokenPolicies(), policies) + tPol, err := secret.TokenPolicies() + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(tPol, policies) { + t.Errorf("expected %#v to be %#v", tPol, policies) } }) @@ -976,8 +1103,12 @@ func TestSecret_TokenPolicies(t *testing.T) { t.Fatal(err) } - if !reflect.DeepEqual(secret.TokenPolicies(), policies) { - t.Errorf("expected %#v to be %#v", secret.TokenPolicies(), policies) + tPol, err := secret.TokenPolicies() + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(tPol, policies) { + t.Errorf("expected %#v to be %#v", tPol, policies) } }) @@ -1003,8 +1134,12 @@ func TestSecret_TokenPolicies(t *testing.T) { t.Fatal(err) } - if !reflect.DeepEqual(secret.TokenPolicies(), policies) { - t.Errorf("expected %#v to be %#v", secret.TokenPolicies(), policies) + tPol, err := secret.TokenPolicies() + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(tPol, policies) { + t.Errorf("expected %#v to be %#v", tPol, policies) } }) } @@ -1016,11 +1151,13 @@ func TestSecret_TokenMetadata(t *testing.T) { name string secret *api.Secret exp map[string]string + err bool }{ { "nil", nil, nil, + false, }, { "nil_auth", @@ -1028,6 +1165,7 @@ func TestSecret_TokenMetadata(t *testing.T) { Auth: nil, }, nil, + false, }, { "nil_auth_metadata", @@ -1037,6 +1175,7 @@ func TestSecret_TokenMetadata(t *testing.T) { }, }, nil, + false, }, { "empty_auth_metadata", @@ -1046,6 +1185,7 @@ func TestSecret_TokenMetadata(t *testing.T) { }, }, nil, + false, }, { "real_auth_metdata", @@ -1055,6 +1195,7 @@ func TestSecret_TokenMetadata(t *testing.T) { }, }, map[string]string{"foo": "bar"}, + false, }, { "nil_data", @@ -1062,6 +1203,7 @@ func TestSecret_TokenMetadata(t *testing.T) { Data: nil, }, nil, + false, }, { "empty_data", @@ -1069,6 +1211,7 @@ func TestSecret_TokenMetadata(t *testing.T) { Data: map[string]interface{}{}, }, nil, + false, }, { "data_not_map", @@ -1078,6 +1221,7 @@ func TestSecret_TokenMetadata(t *testing.T) { }, }, nil, + true, }, { "data_map", @@ -1087,6 +1231,7 @@ func TestSecret_TokenMetadata(t *testing.T) { }, }, map[string]string{"foo": "bar"}, + false, }, { "data_map_bad_type", @@ -1096,6 +1241,7 @@ func TestSecret_TokenMetadata(t *testing.T) { }, }, nil, + true, }, } @@ -1105,7 +1251,10 @@ func TestSecret_TokenMetadata(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - act := tc.secret.TokenMetadata() + act, err := tc.secret.TokenMetadata() + if err != nil && !tc.err { + t.Fatal(err) + } if !reflect.DeepEqual(act, tc.exp) { t.Errorf("expected %#v to be %#v", act, tc.exp) } @@ -1137,8 +1286,12 @@ func TestSecret_TokenMetadata(t *testing.T) { t.Fatal(err) } - if !reflect.DeepEqual(secret.TokenMetadata(), metadata) { - t.Errorf("expected %#v to be %#v", secret.TokenMetadata(), metadata) + tMeta, err := secret.TokenMetadata() + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(tMeta, metadata) { + t.Errorf("expected %#v to be %#v", tMeta, metadata) } }) @@ -1158,8 +1311,12 @@ func TestSecret_TokenMetadata(t *testing.T) { t.Fatal(err) } - if !reflect.DeepEqual(secret.TokenMetadata(), metadata) { - t.Errorf("expected %#v to be %#v", secret.TokenMetadata(), metadata) + tMeta, err := secret.TokenMetadata() + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(tMeta, metadata) { + t.Errorf("expected %#v to be %#v", tMeta, metadata) } }) @@ -1185,8 +1342,12 @@ func TestSecret_TokenMetadata(t *testing.T) { t.Fatal(err) } - if !reflect.DeepEqual(secret.TokenMetadata(), metadata) { - t.Errorf("expected %#v to be %#v", secret.TokenMetadata(), metadata) + tMeta, err := secret.TokenMetadata() + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(tMeta, metadata) { + t.Errorf("expected %#v to be %#v", tMeta, metadata) } }) @@ -1213,8 +1374,12 @@ func TestSecret_TokenMetadata(t *testing.T) { t.Fatal(err) } - if !reflect.DeepEqual(secret.TokenMetadata(), metadata) { - t.Errorf("expected %#v to be %#v", secret.TokenMetadata(), metadata) + tMeta, err := secret.TokenMetadata() + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(tMeta, metadata) { + t.Errorf("expected %#v to be %#v", tMeta, metadata) } }) @@ -1240,8 +1405,12 @@ func TestSecret_TokenMetadata(t *testing.T) { t.Fatal(err) } - if !reflect.DeepEqual(secret.TokenMetadata(), metadata) { - t.Errorf("expected %#v to be %#v", secret.TokenMetadata(), metadata) + tMeta, err := secret.TokenMetadata() + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(tMeta, metadata) { + t.Errorf("expected %#v to be %#v", tMeta, metadata) } }) @@ -1268,8 +1437,12 @@ func TestSecret_TokenMetadata(t *testing.T) { t.Fatal(err) } - if !reflect.DeepEqual(secret.TokenMetadata(), metadata) { - t.Errorf("expected %#v to be %#v", secret.TokenMetadata(), metadata) + tMeta, err := secret.TokenMetadata() + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(tMeta, metadata) { + t.Errorf("expected %#v to be %#v", tMeta, metadata) } }) } @@ -1333,7 +1506,16 @@ func TestSecret_TokenIsRenewable(t *testing.T) { "renewable": 123, }, }, - false, + true, + }, + { + "data_bool_string", + &api.Secret{ + Data: map[string]interface{}{ + "renewable": "true", + }, + }, + true, }, { "data_bool_true", @@ -1361,7 +1543,10 @@ func TestSecret_TokenIsRenewable(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - act := tc.secret.TokenIsRenewable() + act, err := tc.secret.TokenIsRenewable() + if err != nil { + t.Fatal(err) + } if act != tc.exp { t.Errorf("expected %t to be %t", act, tc.exp) } @@ -1393,8 +1578,12 @@ func TestSecret_TokenIsRenewable(t *testing.T) { t.Fatal(err) } - if secret.TokenIsRenewable() != renewable { - t.Errorf("expected %t to be %t", secret.TokenIsRenewable(), renewable) + tRenew, err := secret.TokenIsRenewable() + if err != nil { + t.Fatal(err) + } + if tRenew != renewable { + t.Errorf("expected %t to be %t", tRenew, renewable) } }) @@ -1414,8 +1603,12 @@ func TestSecret_TokenIsRenewable(t *testing.T) { t.Fatal(err) } - if secret.TokenIsRenewable() != renewable { - t.Errorf("expected %t to be %t", secret.TokenIsRenewable(), renewable) + tRenew, err := secret.TokenIsRenewable() + if err != nil { + t.Fatal(err) + } + if tRenew != renewable { + t.Errorf("expected %t to be %t", tRenew, renewable) } }) @@ -1441,8 +1634,12 @@ func TestSecret_TokenIsRenewable(t *testing.T) { t.Fatal(err) } - if secret.TokenIsRenewable() != renewable { - t.Errorf("expected %t to be %t", secret.TokenIsRenewable(), renewable) + tRenew, err := secret.TokenIsRenewable() + if err != nil { + t.Fatal(err) + } + if tRenew != renewable { + t.Errorf("expected %t to be %t", tRenew, renewable) } }) @@ -1469,8 +1666,12 @@ func TestSecret_TokenIsRenewable(t *testing.T) { t.Fatal(err) } - if secret.TokenIsRenewable() != renewable { - t.Errorf("expected %t to be %t", secret.TokenIsRenewable(), renewable) + tRenew, err := secret.TokenIsRenewable() + if err != nil { + t.Fatal(err) + } + if tRenew != renewable { + t.Errorf("expected %t to be %t", tRenew, renewable) } }) @@ -1496,8 +1697,12 @@ func TestSecret_TokenIsRenewable(t *testing.T) { t.Fatal(err) } - if secret.TokenIsRenewable() != renewable { - t.Errorf("expected %t to be %t", secret.TokenIsRenewable(), renewable) + tRenew, err := secret.TokenIsRenewable() + if err != nil { + t.Fatal(err) + } + if tRenew != renewable { + t.Errorf("expected %t to be %t", tRenew, renewable) } }) @@ -1524,262 +1729,12 @@ func TestSecret_TokenIsRenewable(t *testing.T) { t.Fatal(err) } - if secret.TokenIsRenewable() != renewable { - t.Errorf("expected %t to be %t", secret.TokenIsRenewable(), renewable) - } - }) -} - -func TestSecret_TokenTTLInt(t *testing.T) { - t.Parallel() - - cases := []struct { - name string - secret *api.Secret - exp int - }{ - { - "nil", - nil, - 0, - }, - { - "nil_auth", - &api.Secret{ - Auth: nil, - }, - 0, - }, - { - "nil_auth_lease_duration", - &api.Secret{ - Auth: &api.SecretAuth{ - LeaseDuration: 0, - }, - }, - 0, - }, - { - "real_auth_lease_duration", - &api.Secret{ - Auth: &api.SecretAuth{ - LeaseDuration: 3600, - }, - }, - 3600, - }, - { - "nil_data", - &api.Secret{ - Data: nil, - }, - 0, - }, - { - "empty_data", - &api.Secret{ - Data: map[string]interface{}{}, - }, - 0, - }, - { - "data_not_json_number", - &api.Secret{ - Data: map[string]interface{}{ - "ttl": 123, - }, - }, - 0, - }, - { - "data_json_number", - &api.Secret{ - Data: map[string]interface{}{ - "ttl": json.Number("3600"), - }, - }, - 3600, - }, - } - - for _, tc := range cases { - tc := tc - - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - act := tc.secret.TokenTTLInt() - if act != tc.exp { - t.Errorf("expected %d to be %d", act, tc.exp) - } - }) - } - - t.Run("auth", func(t *testing.T) { - t.Parallel() - - client, closer := testVaultServer(t) - defer closer() - - ttl := 3600 - - if err := client.Sys().EnableAuth("userpass", "userpass", ""); err != nil { - t.Fatal(err) - } - if _, err := client.Logical().Write("auth/userpass/users/test", map[string]interface{}{ - "password": "test", - "policies": "default", - "ttl": fmt.Sprintf("%ds", ttl), - "explicit_max_ttl": fmt.Sprintf("%ds", ttl), - }); err != nil { - t.Fatal(err) - } - - secret, err := client.Logical().Write("auth/userpass/login/test", map[string]interface{}{ - "password": "test", - }) - if err != nil || secret == nil { - t.Fatal(err) - } - - if secret.TokenTTLInt() == 0 || secret.TokenTTLInt() > ttl { - t.Errorf("expected %q to non-zero and less than %q", secret.TokenTTL(), ttl) - } - }) - - t.Run("token-create", func(t *testing.T) { - t.Parallel() - - client, closer := testVaultServer(t) - defer closer() - - ttl := 3600 - - secret, err := client.Auth().Token().Create(&api.TokenCreateRequest{ - Policies: []string{"default"}, - TTL: fmt.Sprintf("%ds", ttl), - ExplicitMaxTTL: fmt.Sprintf("%ds", ttl), - }) + tRenew, err := secret.TokenIsRenewable() if err != nil { t.Fatal(err) } - - if secret.TokenTTLInt() == 0 || secret.TokenTTLInt() > ttl { - t.Errorf("expected %q to non-zero and less than %q", secret.TokenTTLInt(), ttl) - } - }) - - t.Run("token-lookup", func(t *testing.T) { - t.Parallel() - - client, closer := testVaultServer(t) - defer closer() - - ttl := 3600 - - secret, err := client.Auth().Token().Create(&api.TokenCreateRequest{ - Policies: []string{"default"}, - TTL: fmt.Sprintf("%ds", ttl), - ExplicitMaxTTL: fmt.Sprintf("%ds", ttl), - }) - if err != nil { - t.Fatal(err) - } - token := secret.Auth.ClientToken - - secret, err = client.Auth().Token().Lookup(token) - if err != nil { - t.Fatal(err) - } - - if secret.TokenTTLInt() == 0 || secret.TokenTTLInt() > ttl { - t.Errorf("expected %q to non-zero and less than %q", secret.TokenTTLInt(), ttl) - } - }) - - t.Run("token-lookup-self", func(t *testing.T) { - t.Parallel() - - client, closer := testVaultServer(t) - defer closer() - - ttl := 3600 - - secret, err := client.Auth().Token().Create(&api.TokenCreateRequest{ - Policies: []string{"default"}, - TTL: fmt.Sprintf("%ds", ttl), - ExplicitMaxTTL: fmt.Sprintf("%ds", ttl), - }) - if err != nil { - t.Fatal(err) - } - token := secret.Auth.ClientToken - - client.SetToken(token) - secret, err = client.Auth().Token().LookupSelf() - if err != nil { - t.Fatal(err) - } - - if secret.TokenTTLInt() == 0 || secret.TokenTTLInt() > ttl { - t.Errorf("expected %q to non-zero and less than %q", secret.TokenTTLInt(), ttl) - } - }) - - t.Run("token-renew", func(t *testing.T) { - t.Parallel() - - client, closer := testVaultServer(t) - defer closer() - - ttl := 3600 - - secret, err := client.Auth().Token().Create(&api.TokenCreateRequest{ - Policies: []string{"default"}, - TTL: fmt.Sprintf("%ds", ttl), - ExplicitMaxTTL: fmt.Sprintf("%ds", ttl), - }) - if err != nil { - t.Fatal(err) - } - token := secret.Auth.ClientToken - - secret, err = client.Auth().Token().Renew(token, 0) - if err != nil { - t.Fatal(err) - } - - if secret.TokenTTLInt() == 0 || secret.TokenTTLInt() > ttl { - t.Errorf("expected %q to non-zero and less than %q", secret.TokenTTLInt(), ttl) - } - }) - - t.Run("token-renew-self", func(t *testing.T) { - t.Parallel() - - client, closer := testVaultServer(t) - defer closer() - - ttl := 3600 - - secret, err := client.Auth().Token().Create(&api.TokenCreateRequest{ - Policies: []string{"default"}, - TTL: fmt.Sprintf("%ds", ttl), - ExplicitMaxTTL: fmt.Sprintf("%ds", ttl), - }) - if err != nil { - t.Fatal(err) - } - token := secret.Auth.ClientToken - - client.SetToken(token) - secret, err = client.Auth().Token().RenewSelf(0) - if err != nil { - t.Fatal(err) - } - - if secret.TokenTTLInt() == 0 || secret.TokenTTLInt() > ttl { - t.Errorf("expected %q to non-zero and less than %q", secret.TokenTTLInt(), ttl) + if tRenew != renewable { + t.Errorf("expected %t to be %t", tRenew, renewable) } }) } @@ -1843,7 +1798,7 @@ func TestSecret_TokenTTL(t *testing.T) { "ttl": 123, }, }, - 0, + 123 * time.Second, }, { "data_json_number", @@ -1862,7 +1817,10 @@ func TestSecret_TokenTTL(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - act := tc.secret.TokenTTL() + act, err := tc.secret.TokenTTL() + if err != nil { + t.Fatal(err) + } if act != tc.exp { t.Errorf("expected %q to be %q", act, tc.exp) } @@ -1896,8 +1854,12 @@ func TestSecret_TokenTTL(t *testing.T) { t.Fatal(err) } - if secret.TokenTTL() == 0 || secret.TokenTTL() > ttl { - t.Errorf("expected %q to non-zero and less than %q", secret.TokenTTL(), ttl) + tokenTTL, err := secret.TokenTTL() + if err != nil { + t.Fatal(err) + } + if tokenTTL == 0 || tokenTTL > ttl { + t.Errorf("expected %q to non-zero and less than %q", tokenTTL, ttl) } }) @@ -1918,8 +1880,12 @@ func TestSecret_TokenTTL(t *testing.T) { t.Fatal(err) } - if secret.TokenTTL() == 0 || secret.TokenTTL() > ttl { - t.Errorf("expected %q to non-zero and less than %q", secret.TokenTTL(), ttl) + tokenTTL, err := secret.TokenTTL() + if err != nil { + t.Fatal(err) + } + if tokenTTL == 0 || tokenTTL > ttl { + t.Errorf("expected %q to non-zero and less than %q", tokenTTL, ttl) } }) @@ -1946,8 +1912,12 @@ func TestSecret_TokenTTL(t *testing.T) { t.Fatal(err) } - if secret.TokenTTL() == 0 || secret.TokenTTL() > ttl { - t.Errorf("expected %q to non-zero and less than %q", secret.TokenTTL(), ttl) + tokenTTL, err := secret.TokenTTL() + if err != nil { + t.Fatal(err) + } + if tokenTTL == 0 || tokenTTL > ttl { + t.Errorf("expected %q to non-zero and less than %q", tokenTTL, ttl) } }) @@ -1975,8 +1945,12 @@ func TestSecret_TokenTTL(t *testing.T) { t.Fatal(err) } - if secret.TokenTTL() == 0 || secret.TokenTTL() > ttl { - t.Errorf("expected %q to non-zero and less than %q", secret.TokenTTL(), ttl) + tokenTTL, err := secret.TokenTTL() + if err != nil { + t.Fatal(err) + } + if tokenTTL == 0 || tokenTTL > ttl { + t.Errorf("expected %q to non-zero and less than %q", tokenTTL, ttl) } }) @@ -2003,8 +1977,12 @@ func TestSecret_TokenTTL(t *testing.T) { t.Fatal(err) } - if secret.TokenTTL() == 0 || secret.TokenTTL() > ttl { - t.Errorf("expected %q to non-zero and less than %q", secret.TokenTTL(), ttl) + tokenTTL, err := secret.TokenTTL() + if err != nil { + t.Fatal(err) + } + if tokenTTL == 0 || tokenTTL > ttl { + t.Errorf("expected %q to non-zero and less than %q", tokenTTL, ttl) } }) @@ -2032,8 +2010,12 @@ func TestSecret_TokenTTL(t *testing.T) { t.Fatal(err) } - if secret.TokenTTL() == 0 || secret.TokenTTL() > ttl { - t.Errorf("expected %q to non-zero and less than %q", secret.TokenTTL(), ttl) + tokenTTL, err := secret.TokenTTL() + if err != nil { + t.Fatal(err) + } + if tokenTTL == 0 || tokenTTL > ttl { + t.Errorf("expected %q to non-zero and less than %q", tokenTTL, ttl) } }) } diff --git a/builtin/credential/token/cli.go b/builtin/credential/token/cli.go index 0bf59e0b5..b300128c2 100644 --- a/builtin/credential/token/cli.go +++ b/builtin/credential/token/cli.go @@ -95,15 +95,39 @@ func (h *CLIHandler) Auth(c *api.Client, m map[string]string) (*api.Secret, erro // Return an auth struct that "looks" like the response from an auth method. // lookup and lookup-self return their data in data, not auth. We try to // mirror that data here. + id, err := secret.TokenID() + if err != nil { + return nil, fmt.Errorf("Error accessing token ID: %s", err) + } + accessor, err := secret.TokenAccessor() + if err != nil { + return nil, fmt.Errorf("Error accessing token accessor: %s", err) + } + policies, err := secret.TokenPolicies() + if err != nil { + return nil, fmt.Errorf("Error accessing token policies: %s", err) + } + metadata, err := secret.TokenMetadata() + if err != nil { + return nil, fmt.Errorf("Error accessing token metadata: %s", err) + } + dur, err := secret.TokenTTL() + if err != nil { + return nil, fmt.Errorf("Error converting token TTL: %s", err) + } + renewable, err := secret.TokenIsRenewable() + if err != nil { + return nil, fmt.Errorf("Error checking if token is renewable: %s", err) + } return &api.Secret{ Auth: &api.SecretAuth{ - ClientToken: secret.TokenID(), - Accessor: secret.TokenAccessor(), - Policies: secret.TokenPolicies(), - Metadata: secret.TokenMetadata(), + ClientToken: id, + Accessor: accessor, + Policies: policies, + Metadata: metadata, - LeaseDuration: secret.TokenTTLInt(), - Renewable: secret.TokenIsRenewable(), + LeaseDuration: int(dur.Seconds()), + Renewable: renewable, }, }, nil diff --git a/command/login_test.go b/command/login_test.go index 38ae744a2..3776e2873 100644 --- a/command/login_test.go +++ b/command/login_test.go @@ -478,7 +478,10 @@ func TestLoginCommand_Run(t *testing.T) { // There was 1 use to start, make sure we didn't use it (verifying would // use it). - uses := lookup.TokenRemainingUses() + uses, err := lookup.TokenRemainingUses() + if err != nil { + t.Fatal(err) + } if uses != 1 { t.Errorf("expected %d to be %d", uses, 1) } diff --git a/helper/parseutil/parseutil.go b/helper/parseutil/parseutil.go index 957d5332e..ad5a96f64 100644 --- a/helper/parseutil/parseutil.go +++ b/helper/parseutil/parseutil.go @@ -56,6 +56,43 @@ func ParseDurationSecond(in interface{}) (time.Duration, error) { return dur, nil } +func ParseInt(in interface{}) (int64, error) { + var ret int64 + jsonIn, ok := in.(json.Number) + if ok { + in = jsonIn.String() + } + switch in.(type) { + case string: + inp := in.(string) + if inp == "" { + return 0, nil + } + var err error + left, err := strconv.ParseInt(inp, 10, 64) + if err != nil { + return ret, err + } + ret = left + case int: + ret = int64(in.(int)) + case int32: + ret = int64(in.(int32)) + case int64: + ret = in.(int64) + case uint: + ret = int64(in.(uint)) + case uint32: + ret = int64(in.(uint32)) + case uint64: + ret = int64(in.(uint64)) + default: + return 0, errors.New("could not parse value from input") + } + + return ret, nil +} + func ParseBool(in interface{}) (bool, error) { var result bool if err := mapstructure.WeakDecode(in, &result); err != nil {