Merge pull request #38 from owncloud/fix/defensive-code

Defensive Code on Type Assertion

Author: kulmann

changelog/unreleased/hardening-type-assertion.md

@@ -0,0 +1,6 @@
+Bugfix: Fix runtime error when type asserting on nil value
+
+Fixed the case where an account UUID present in the context is nil, and type asserting it as a string would produce a runtime error.
+
+https://github.com/owncloud/ocis-settings/pull/38
+https://github.com/owncloud/ocis-settings/issues/37

pkg/service/v0/service.go

@@ -115,13 +115,8 @@ func getFailsafeIdentifier(c context.Context, identifier *proto.Identifier) *pro
 		identifier = &proto.Identifier{}
 	}
 	if identifier.AccountUuid == "me" {
-		ownAccountUUID := c.Value(middleware.UUIDKey).(string)
-		if len(ownAccountUUID) > 0 {
+		if ownAccountUUID, ok := c.Value(middleware.UUIDKey).(string); ok {
 			identifier.AccountUuid = ownAccountUUID
-		} else {
-			// might be valid for the request not having an AccountUuid in the identifier.
-			// but clear it, instead of passing on `me`.
-			identifier.AccountUuid = ""
 		}
 	}
 	identifier.AccountUuid = strings.ToLower(identifier.AccountUuid)

pkg/service/v0/service_test.go

@@ -0,0 +1,65 @@
+package svc
+
+import (
+	"context"
+	"testing"
+
+	"github.com/owncloud/ocis-pkg/v2/middleware"
+	"github.com/owncloud/ocis-settings/pkg/proto/v0"
+	"github.com/stretchr/testify/assert"
+)
+
+var (
+	ctxWithUUID      = context.WithValue(context.Background(), middleware.UUIDKey, "61445573-4dbe-4d56-88dc-88ab47aceba7")
+	ctxWithEmptyUUID = context.WithValue(context.Background(), middleware.UUIDKey, "")
+	emptyCtx         = context.Background()
+
+	scenarios = []struct {
+		name       string
+		identifier *proto.Identifier
+		ctx        context.Context
+		expect     *proto.Identifier
+	}{
+		{
+			name: "context with UUID; identifier = 'me'",
+			ctx:  ctxWithUUID,
+			identifier: &proto.Identifier{
+				AccountUuid: "me",
+			},
+			expect: &proto.Identifier{
+				AccountUuid: ctxWithUUID.Value(middleware.UUIDKey).(string),
+			},
+		},
+		{
+			name: "context without UUID; identifier = 'me'",
+			ctx:  ctxWithEmptyUUID,
+			identifier: &proto.Identifier{
+				AccountUuid: "me",
+			},
+			expect: &proto.Identifier{
+				AccountUuid: "",
+			},
+		},
+		{
+			name:       "context with UUID; identifier not 'me'",
+			ctx:        ctxWithUUID,
+			identifier: &proto.Identifier{},
+			expect: &proto.Identifier{
+				AccountUuid: "",
+			},
+		},
+	}
+)
+
+func TestGetFailsafeIdentifier(t *testing.T) {
+	for _, s := range scenarios {
+		scenario := s
+		t.Run(scenario.name, func(t *testing.T) {
+			got := getFailsafeIdentifier(scenario.ctx, scenario.identifier)
+			assert.NotPanics(t, func() {
+				getFailsafeIdentifier(emptyCtx, scenario.identifier)
+			})
+			assert.Equal(t, scenario.expect, got)
+		})
+	}
+}