Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resset: better error messages #36

Merged
merged 1 commit into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions flyio/caveats.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func (c *Apps) Prohibits(a macaroon.Access) error {
if !isFlyioAccess {
return fmt.Errorf("%w: access isnt AppIDGetter", macaroon.ErrInvalidAccess)
}
return c.Apps.Prohibits(f.GetAppID(), f.GetAction())
return c.Apps.Prohibits(f.GetAppID(), f.GetAction(), "app")
}

type Volumes struct {
Expand All @@ -118,7 +118,7 @@ func (c *Volumes) Prohibits(a macaroon.Access) error {
if !isFlyioAccess {
return fmt.Errorf("%w: access isnt VolumeGetter", macaroon.ErrInvalidAccess)
}
return c.Volumes.Prohibits(f.GetVolume(), f.GetAction())
return c.Volumes.Prohibits(f.GetVolume(), f.GetAction(), "volume")
}

type Machines struct {
Expand All @@ -134,7 +134,7 @@ func (c *Machines) Prohibits(a macaroon.Access) error {
if !isFlyioAccess {
return fmt.Errorf("%w: access isnt MachineGetter", macaroon.ErrInvalidAccess)
}
return c.Machines.Prohibits(f.GetMachine(), f.GetAction())
return c.Machines.Prohibits(f.GetMachine(), f.GetAction(), "machine")
}

type MachineFeatureSet struct {
Expand All @@ -150,7 +150,7 @@ func (c *MachineFeatureSet) Prohibits(a macaroon.Access) error {
if !isFlyioAccess {
return fmt.Errorf("%w: access isnt MachineFeatureGetter", macaroon.ErrInvalidAccess)
}
return c.Features.Prohibits(f.GetMachineFeature(), f.GetAction())
return c.Features.Prohibits(f.GetMachineFeature(), f.GetAction(), "machine feature")
}

// FeatureSet is a collection of organization-level "features" that are managed
Expand All @@ -171,7 +171,7 @@ func (c *FeatureSet) Prohibits(a macaroon.Access) error {
if !isFlyioAccess {
return fmt.Errorf("%w: access isnt FeatureGetter", macaroon.ErrInvalidAccess)
}
return c.Features.Prohibits(f.GetFeature(), f.GetAction())
return c.Features.Prohibits(f.GetFeature(), f.GetAction(), "org feature")
}

// Mutations is a set of GraphQL mutations allowed by this token.
Expand Down Expand Up @@ -238,7 +238,7 @@ func (c *Clusters) Prohibits(a macaroon.Access) error {
return fmt.Errorf("%w: access isnt ClusterGetter", macaroon.ErrInvalidAccess)
}

return c.Clusters.Prohibits(f.GetCluster(), f.GetAction())
return c.Clusters.Prohibits(f.GetCluster(), f.GetAction(), "cluster")
}

// Role is used by the AllowedRoles and IsMember caveats.
Expand Down Expand Up @@ -401,7 +401,7 @@ func (c *AppFeatureSet) Prohibits(a macaroon.Access) error {
if !isFlyioAccess {
return fmt.Errorf("%w: access isnt AppFeatureGetter", macaroon.ErrInvalidAccess)
}
return c.Features.Prohibits(f.GetAppFeature(), f.GetAction())
return c.Features.Prohibits(f.GetAppFeature(), f.GetAction(), "app feature")
}

// StorageObjects limits what storage objects can be accessed. Objects are
Expand All @@ -425,5 +425,5 @@ func (c *StorageObjects) Prohibits(a macaroon.Access) error {
if !isFlyioAccess {
return fmt.Errorf("%w: access isnt StorageObjectGetter", macaroon.ErrInvalidAccess)
}
return c.Prefixes.Prohibits(f.GetStorageObject(), f.GetAction())
return c.Prefixes.Prohibits(f.GetStorageObject(), f.GetAction(), "storage object")
}
2 changes: 1 addition & 1 deletion resset/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (c *Widgets) Prohibits(f macaroon.Access) error {
return macaroon.ErrInvalidAccess
}

return c.Widgets.Prohibits(wf.WidgetName, wf.Action)
return c.Widgets.Prohibits(wf.WidgetName, wf.Action, "widget")
}

// implements macaroon.Access; describes an attempt to access a widget
Expand Down
18 changes: 11 additions & 7 deletions resset/resource_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,38 +45,42 @@ func New[I ID](p Action, ids ...I) ResourceSet[I] {
return ret
}

func (rs ResourceSet[I]) Prohibits(id *I, action Action) error {
func (rs ResourceSet[I]) Prohibits(id *I, action Action, resourceType string) error {
if err := rs.validate(); err != nil {
return err
}
if id == nil {
return fmt.Errorf("%w resource", ErrResourceUnspecified)
return fmt.Errorf("%w %s", ErrResourceUnspecified, resourceType)
}

var (
foundPerm = false
perm = ActionAll
zeroID I
foundPerm = false
perm = ActionAll
zeroID I
allowedIDs []I
)

if zeroPerm, hasZero := rs[zeroID]; hasZero {
perm &= zeroPerm
foundPerm = true
allowedIDs = append(allowedIDs, zeroID)
}

for entryID, entryPerm := range rs {
allowedIDs = append(allowedIDs, entryID)

if match(entryID, *id) {
perm &= entryPerm
foundPerm = true
}
}

if !foundPerm {
return fmt.Errorf("%w %v", ErrUnauthorizedForResource, *id)
return fmt.Errorf("%w %s %v (only %v)", ErrUnauthorizedForResource, resourceType, *id, allowedIDs)
}

if !action.IsSubsetOf(perm) {
return fmt.Errorf("%w access %s (%s not allowed)", ErrUnauthorizedForAction, action, action.Remove(perm))
return fmt.Errorf("%w access %s on %s (%s not allowed)", ErrUnauthorizedForAction, action, resourceType, action.Remove(perm))
}

return nil
Expand Down
22 changes: 11 additions & 11 deletions resset/resource_set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,24 @@ func TestResourceSet(t *testing.T) {
"bar": ActionWrite,
}

assert.NoError(t, rs.Prohibits(ptr("foo"), ActionRead|ActionWrite))
assert.NoError(t, rs.Prohibits(ptr("bar"), ActionWrite))
assert.True(t, errors.Is(rs.Prohibits(nil, ActionWrite), ErrResourceUnspecified))
assert.True(t, errors.Is(rs.Prohibits(ptr("baz"), ActionWrite), ErrUnauthorizedForResource))
assert.True(t, errors.Is(rs.Prohibits(ptr(zero), ActionWrite), ErrUnauthorizedForResource))
assert.True(t, errors.Is(rs.Prohibits(ptr("foo"), ActionAll), ErrUnauthorizedForAction))
assert.NoError(t, rs.Prohibits(ptr("foo"), ActionRead|ActionWrite, "test resource"))
assert.NoError(t, rs.Prohibits(ptr("bar"), ActionWrite, "test resource"))
assert.True(t, errors.Is(rs.Prohibits(nil, ActionWrite, "test resource"), ErrResourceUnspecified))
assert.True(t, errors.Is(rs.Prohibits(ptr("baz"), ActionWrite, "test resource"), ErrUnauthorizedForResource))
assert.True(t, errors.Is(rs.Prohibits(ptr(zero), ActionWrite, "test resource"), ErrUnauthorizedForResource))
assert.True(t, errors.Is(rs.Prohibits(ptr("foo"), ActionAll, "test resource"), ErrUnauthorizedForAction))
}

func TestZeroID(t *testing.T) {
zero := ZeroID[string]()
rs := &ResourceSet[string]{zero: ActionRead}

assert.NoError(t, rs.Prohibits(ptr("foo"), ActionRead))
assert.NoError(t, rs.Prohibits(ptr(zero), ActionRead))
assert.NoError(t, rs.Prohibits(ptr("foo"), ActionRead, "test resource"))
assert.NoError(t, rs.Prohibits(ptr(zero), ActionRead, "test resource"))

assert.True(t, errors.Is(rs.Prohibits(nil, ActionRead), ErrResourceUnspecified))
assert.True(t, errors.Is(rs.Prohibits(ptr("foo"), ActionWrite), ErrUnauthorizedForAction))
assert.True(t, errors.Is(rs.Prohibits(ptr(zero), ActionWrite), ErrUnauthorizedForAction))
assert.True(t, errors.Is(rs.Prohibits(nil, ActionRead, "test resource"), ErrResourceUnspecified))
assert.True(t, errors.Is(rs.Prohibits(ptr("foo"), ActionWrite, "test resource"), ErrUnauthorizedForAction))
assert.True(t, errors.Is(rs.Prohibits(ptr(zero), ActionWrite, "test resource"), ErrUnauthorizedForAction))

rs = &ResourceSet[string]{
zero: ActionRead | ActionWrite,
Expand Down
Loading