flyio: add missing validations on Access fields

superfly · Jul 2, 2024 · 67e0e26 · 67e0e26
1 parent b4fab9f
commit 67e0e26
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 21 deletions.
diff --git a/flyio/access.go b/flyio/access.go
@@ -2,6 +2,7 @@ package flyio
 
 import (
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/superfly/macaroon"
@@ -53,15 +54,22 @@ func (f *Access) Validate() error {
 		return fmt.Errorf("%w: app, org-feature", resset.ErrResourcesMutuallyExclusive)
 	}
 
-	// app-level resources = machines, volumes
-	if f.Machine != nil || f.Volume != nil {
-		if f.AppID == nil {
-			return fmt.Errorf("%w app if app-owned resource is specified", resset.ErrResourceUnspecified)
-		}
-
-		if f.Machine != nil && f.Volume != nil {
-			return fmt.Errorf("%w: volume, machine", resset.ErrResourcesMutuallyExclusive)
-		}
+	// app-level resources = machines, volumes, app-features
+	var appResources []string
+	if f.Machine != nil {
+		appResources = append(appResources, "machine")
+	}
+	if f.Volume != nil {
+		appResources = append(appResources, "volume")
+	}
+	if f.AppFeature != nil {
+		appResources = append(appResources, *f.AppFeature)
+	}
+	if len(appResources) != 0 && f.AppID == nil {
+		return fmt.Errorf("%w app if app-owned resource is specified", resset.ErrResourceUnspecified)
+	}
+	if len(appResources) > 1 {
+		return fmt.Errorf("%w: %s", resset.ErrResourcesMutuallyExclusive, strings.Join(appResources, ", "))
 	}
 
 	// lfsc feature-level resource = clusters
@@ -76,9 +84,19 @@ func (f *Access) Validate() error {
 	}
 
 	// machine feature requires machine
-	if f.MachineFeature != nil && f.Machine == nil {
+	var machineResources []string
+	if f.Command != nil {
+		machineResources = append(machineResources, "command-execution")
+	}
+	if f.MachineFeature != nil {
+		machineResources = append(machineResources, *f.MachineFeature)
+	}
+	if len(machineResources) != 0 && f.Machine == nil {
 		return fmt.Errorf("%w machine ", resset.ErrResourceUnspecified)
 	}
+	if len(machineResources) > 1 {
+		return fmt.Errorf("%w: %s", resset.ErrResourcesMutuallyExclusive, strings.Join(machineResources, ", "))
+	}
 
 	return nil
 }

diff --git a/flyio/caveats_test.go b/flyio/caveats_test.go
@@ -117,63 +117,82 @@ func TestCommands(t *testing.T) {
 
 	yes(cs, &Access{
 		OrgID:   uptr(1),
-		Action:  resset.ActionAll,
+		AppID:   uptr(1),
+		Machine: ptr("machine"),
 		Command: []string{"cmd1", "arg1"},
 	})
 
 	yes(cs, &Access{
 		OrgID:   uptr(1),
 		Action:  resset.ActionAll,
+		AppID:   uptr(1),
+		Machine: ptr("machine"),
 		Command: []string{"cmd1", "arg1", "arg2"},
 	})
 
 	yes(cs, &Access{
 		OrgID:   uptr(1),
+		AppID:   uptr(1),
+		Machine: ptr("machine"),
 		Action:  resset.ActionAll,
 		Command: []string{"cmd2", "arg1"},
 	})
 
 	no(cs, &Access{
 		OrgID:   uptr(1),
+		AppID:   uptr(1),
+		Machine: ptr("machine"),
 		Action:  resset.ActionAll,
 		Command: []string{"cmd2", "arg1", "arg2"},
 	}, resset.ErrUnauthorizedForResource)
 
 	no(cs, &Access{
 		OrgID:   uptr(1),
+		AppID:   uptr(1),
+		Machine: ptr("machine"),
 		Action:  resset.ActionAll,
 		Command: []string{"cmd3", "arg1"},
 	}, resset.ErrUnauthorizedForResource)
 
 	no(cs, &Access{
-		OrgID:  uptr(1),
-		Action: resset.ActionAll,
+		OrgID:   uptr(1),
+		AppID:   uptr(1),
+		Machine: ptr("machine"),
+		Action:  resset.ActionAll,
 	}, resset.ErrResourceUnspecified)
 
 	csNone := macaroon.NewCaveatSet(&Commands{})
 
 	no(csNone, &Access{
 		OrgID:   uptr(1),
+		AppID:   uptr(1),
+		Machine: ptr("machine"),
 		Action:  resset.ActionAll,
 		Command: []string{"cmd2", "arg1", "arg2", "arg3"},
 	}, resset.ErrUnauthorizedForResource)
 
 	no(csNone, &Access{
-		OrgID:  uptr(1),
-		Action: resset.ActionAll,
+		OrgID:   uptr(1),
+		AppID:   uptr(1),
+		Machine: ptr("machine"),
+		Action:  resset.ActionAll,
 	}, resset.ErrResourceUnspecified)
 
 	csAny := macaroon.NewCaveatSet(&Commands{Command{}})
 
 	yes(csAny, &Access{
 		OrgID:   uptr(1),
+		AppID:   uptr(1),
+		Machine: ptr("machine"),
 		Action:  resset.ActionAll,
 		Command: []string{"cmd2", "arg1", "arg2", "arg3"},
 	})
 
 	no(csAny, &Access{
-		OrgID:  uptr(1),
-		Action: resset.ActionAll,
+		OrgID:   uptr(1),
+		AppID:   uptr(1),
+		Machine: ptr("machine"),
+		Action:  resset.ActionAll,
 	}, resset.ErrResourceUnspecified)
 
 	csIf := macaroon.NewCaveatSet(
@@ -185,17 +204,23 @@ func TestCommands(t *testing.T) {
 
 	yes(csIf, &Access{
 		OrgID:   uptr(1),
+		AppID:   uptr(1),
+		Machine: ptr("machine"),
 		Action:  resset.ActionNone,
 		Command: []string{"uname", "arg"},
 	})
 
 	yes(csIf, &Access{
-		OrgID:  uptr(1),
-		Action: resset.ActionDelete,
+		OrgID:   uptr(1),
+		AppID:   uptr(1),
+		Machine: ptr("machine"),
+		Action:  resset.ActionDelete,
 	})
 
 	no(csIf, &Access{
-		OrgID:  uptr(1),
-		Action: resset.ActionWrite,
+		OrgID:   uptr(1),
+		AppID:   uptr(1),
+		Machine: ptr("machine"),
+		Action:  resset.ActionWrite,
 	}, resset.ErrUnauthorizedForAction)
 }