sdk/logical: handle empty token type string values as TokenTypeDefault (#7273)

* sdk/logical: handle empty token type string values as TokenTypeDefault

* add test case for missing token_type value

builtin/credential/approle/path_role_test.go

@@ -3,6 +3,7 @@ package approle
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"reflect"
 	"strings"
 	"testing"
@@ -1843,6 +1844,32 @@ func createRole(t *testing.T, b *backend, s logical.Storage, roleName, policies
 // TestAppRole_TokenutilUpgrade ensures that when we read values out that are
 // values with upgrade logic we see the correct struct entries populated
 func TestAppRole_TokenutilUpgrade(t *testing.T) {
+	tests := []struct {
+		name              string
+		storageValMissing bool
+		storageVal        string
+		expectedTokenType logical.TokenType
+	}{
+		{
+			"token_type_missing",
+			true,
+			"",
+			logical.TokenTypeDefault,
+		},
+		{
+			"token_type_empty",
+			false,
+			"",
+			logical.TokenTypeDefault,
+		},
+		{
+			"token_type_service",
+			false,
+			"service",
+			logical.TokenTypeService,
+		},
+	}
+
 	s := &logical.InmemStorage{}
 
 	config := logical.TestBackendConfig()
@@ -1861,15 +1888,25 @@ func TestAppRole_TokenutilUpgrade(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+
+			// Construct the storage entry object based on our test case.
+			tokenTypeKV := ""
+			if !tt.storageValMissing {
+				tokenTypeKV = fmt.Sprintf(`, "token_type": "%s"`, tt.storageVal)
+			}
+			entryVal := fmt.Sprintf(`{"policies": ["foo"], "period": 300000000000, "token_bound_cidrs": ["127.0.0.1", "10.10.10.10/24"]%s}`, tokenTypeKV)
+
 			// Hand craft JSON because there is overlap between fields
 			if err := s.Put(ctx, &logical.StorageEntry{
-		Key:   "role/foo",
-		Value: []byte(`{"policies": ["foo"], "period": 300000000000, "token_bound_cidrs": ["127.0.0.1", "10.10.10.10/24"], "token_type": "service"}`),
+				Key:   "role/" + tt.name,
+				Value: []byte(entryVal),
 			}); err != nil {
 				t.Fatal(err)
 			}
 
-	fooEntry, err := b.roleEntry(ctx, s, "foo")
+			resEntry, err := b.roleEntry(ctx, s, tt.name)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1881,11 +1918,16 @@ func TestAppRole_TokenutilUpgrade(t *testing.T) {
 				TokenParams: tokenutil.TokenParams{
 					TokenPolicies: []string{"foo"},
 					TokenPeriod:   300 * time.Second,
-			TokenBoundCIDRs: []*sockaddr.SockAddrMarshaler{&sockaddr.SockAddrMarshaler{SockAddr: sockaddr.MustIPAddr("127.0.0.1")}, &sockaddr.SockAddrMarshaler{SockAddr: sockaddr.MustIPAddr("10.10.10.10/24")}},
-			TokenType:       logical.TokenTypeService,
+					TokenBoundCIDRs: []*sockaddr.SockAddrMarshaler{
+						{SockAddr: sockaddr.MustIPAddr("127.0.0.1")},
+						{SockAddr: sockaddr.MustIPAddr("10.10.10.10/24")},
+					},
+					TokenType: tt.expectedTokenType,
 				},
 			}
-	if diff := deep.Equal(fooEntry, exp); diff != nil {
+			if diff := deep.Equal(resEntry, exp); diff != nil {
 				t.Fatal(diff)
 			}
+		})
+	}
 }

sdk/logical/token.go

@@ -38,7 +38,7 @@ func (t *TokenType) UnmarshalJSON(b []byte) error {
 	// Handle upgrade from pre-1.2 where we were serialized as string:
 	s := string(b)
 	switch s {
-	case `"default"`:
+	case `"default"`, `""`:
 		*t = TokenTypeDefault
 	case `"service"`:
 		*t = TokenTypeService

sdk/logical/token_test.go

@@ -30,4 +30,14 @@ func TestJSONSerialization(t *testing.T) {
 	if tt != utt {
 		t.Fatalf("expected %v, got %v", tt, utt)
 	}
+
+	// Test on an empty value, which should unmarshal into TokenTypeDefault
+	tt = TokenTypeDefault
+	err = json.Unmarshal([]byte(`""`), &utt)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if tt != utt {
+		t.Fatalf("expected %v, got %v", tt, utt)
+	}
 }