fix(provisionerd): correctly mutate default tags for PSK auth

johnstcn · johnstcn · commit 95b51edef1ce · 2023-08-30T14:13:13.000Z
diff --git a/coderd/provisionerdserver/provisionertags.go b/coderd/provisionerdserver/provisionertags.go
@@ -24,7 +24,9 @@ func MutateTags(userID uuid.UUID, tags map[string]string) map[string]string {
 	}
 	switch tags[TagScope] {
 	case ScopeUser:
-		tags[TagOwner] = userID.String()
+		if userID != uuid.Nil {
+			tags[TagOwner] = userID.String()
+		}
 	case ScopeOrganization:
 	default:
 		tags[TagScope] = ScopeOrganization
diff --git a/coderd/provisionerdserver/provisionertags_test.go b/coderd/provisionerdserver/provisionertags_test.go
@@ -0,0 +1,80 @@
+package provisionerdserver_test
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/require"
+
+	"github.com/coder/coder/v2/coderd/provisionerdserver"
+)
+
+func TestMutateTags(t *testing.T) {
+	t.Parallel()
+
+	testUserID := uuid.New()
+
+	for _, tt := range []struct {
+		name   string
+		userID uuid.UUID
+		tags   map[string]string
+		want   map[string]string
+	}{
+		{
+			name:   "nil tags",
+			userID: uuid.Nil,
+			tags:   nil,
+			want: map[string]string{
+				provisionerdserver.TagScope: provisionerdserver.ScopeOrganization,
+			},
+		},
+		{
+			name:   "empty tags",
+			userID: uuid.Nil,
+			tags:   map[string]string{},
+			want: map[string]string{
+				provisionerdserver.TagScope: provisionerdserver.ScopeOrganization,
+			},
+		},
+		{
+			name:   "user scope",
+			tags:   map[string]string{provisionerdserver.TagScope: provisionerdserver.ScopeUser},
+			userID: testUserID,
+			want: map[string]string{
+				provisionerdserver.TagScope: provisionerdserver.ScopeUser,
+				provisionerdserver.TagOwner: testUserID.String(),
+			},
+		},
+		{
+			name:   "organization scope",
+			tags:   map[string]string{provisionerdserver.TagScope: provisionerdserver.ScopeOrganization},
+			userID: testUserID,
+			want: map[string]string{
+				provisionerdserver.TagScope: provisionerdserver.ScopeOrganization,
+			},
+		},
+		{
+			name:   "invalid scope",
+			tags:   map[string]string{provisionerdserver.TagScope: "360noscope"},
+			userID: testUserID,
+			want: map[string]string{
+				provisionerdserver.TagScope: provisionerdserver.ScopeOrganization,
+			},
+		},
+	} {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			// make a copy of the map because the function under test
+			// mutates the map
+			bytes, err := json.Marshal(tt.tags)
+			require.NoError(t, err)
+			var tags map[string]string
+			err = json.Unmarshal(bytes, &tags)
+			require.NoError(t, err)
+			got := provisionerdserver.MutateTags(tt.userID, tags)
+			require.Equal(t, tt.want, got)
+		})
+	}
+}
diff --git a/enterprise/cli/provisionerdaemons.go b/enterprise/cli/provisionerdaemons.go
@@ -83,6 +83,9 @@ func (r *RootCmd) provisionerDaemonStart() *clibase.Cmd {
 			}()
 
 			logger := slog.Make(sloghuman.Sink(inv.Stderr))
+			if ok, _ := inv.ParsedFlags().GetBool("verbose"); ok {
+				logger = logger.Leveled(slog.LevelDebug)
+			}
 			errCh := make(chan error, 1)
 			go func() {
 				defer cancel()
diff --git a/enterprise/coderd/provisionerdaemons.go b/enterprise/coderd/provisionerdaemons.go
@@ -117,7 +117,7 @@ func (p *provisionerDaemonAuth) authorize(r *http.Request, tags map[string]strin
 	if p.psk != "" {
 		psk := r.Header.Get(codersdk.ProvisionerDaemonPSK)
 		if subtle.ConstantTimeCompare([]byte(p.psk), []byte(psk)) == 1 {
-			return tags, true
+			return provisionerdserver.MutateTags(uuid.Nil, tags), true
 		}
 	}
 	return nil, false
@@ -172,10 +172,12 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
 
 	tags, authorized := api.provisionerDaemonAuth.authorize(r, tags)
 	if !authorized {
+		api.Logger.Warn(ctx, "unauthorized provisioner daemon serve request", slog.F("tags", tags))
 		httpapi.Write(ctx, rw, http.StatusForbidden,
 			codersdk.Response{Message: "You aren't allowed to create provisioner daemons"})
 		return
 	}
+	api.Logger.Debug(ctx, "provisioner authorized", slog.F("tags", tags))
 
 	provisioners := make([]database.ProvisionerType, 0)
 	for p := range provisionersMap {
@@ -196,6 +198,12 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
 		Tags:         tags,
 	})
 	if err != nil {
+		api.Logger.Error(ctx, "write provisioner daemon",
+			slog.F("name", name),
+			slog.F("provisioners", provisioners),
+			slog.F("tags", tags),
+			slog.Error(err),
+		)
 		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
 			Message: "Internal error writing provisioner daemon.",
 			Detail:  err.Error(),
@@ -205,6 +213,12 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
 
 	rawTags, err := json.Marshal(daemon.Tags)
 	if err != nil {
+		api.Logger.Error(ctx, "marshal provisioner tags",
+			slog.F("name", name),
+			slog.F("provisioners", provisioners),
+			slog.F("tags", tags),
+			slog.Error(err),
+		)
 		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
 			Message: "Internal error marshaling daemon tags.",
 			Detail:  err.Error(),
@@ -222,6 +236,12 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
 		CompressionMode: websocket.CompressionDisabled,
 	})
 	if err != nil {
+		api.Logger.Error(ctx, "accept provisioner websocket conn",
+			slog.F("name", name),
+			slog.F("provisioners", provisioners),
+			slog.F("tags", tags),
+			slog.Error(err),
+		)
 		httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
 			Message: "Internal error accepting websocket connection.",
 			Detail:  err.Error(),
@@ -267,6 +287,12 @@ func (api *API) provisionerDaemonServe(rw http.ResponseWriter, r *http.Request)
 		},
 	)
 	if err != nil {
+		api.Logger.Error(ctx, "create provisioner daemon server",
+			slog.F("name", name),
+			slog.F("provisioners", provisioners),
+			slog.F("tags", tags),
+			slog.Error(err),
+		)
 		_ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("create provisioner daemon server: %s", err))
 		return
 	}
