Skip to content

Commit

Permalink
Revert "Enforce rules entitlement checks on isEntitledToSpace" (#9)
Browse files Browse the repository at this point in the history 
Reverts #7

There's an issue in this change running go mod tidy I missed by not
building the docker image. Will resubmit after I fix it.
  • Loading branch information
@brianathere
brianathere authored May 20, 2024
1 parent 0f711d7 commit 97465bc
Show file tree
Hide file tree
Showing 25 changed files with 240 additions and 282 deletions.
2 changes: 1 addition & 1 deletion core/Dockerfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ COPY --from=builder /bin/xchain_node /usr/bin/xchain_node
RUN setcap 'cap_net_bind_service=+ep' /usr/bin/stream_node

COPY --from=builder /build/node/node/default_config.yaml /riveruser/stream_node/config/config.yaml
COPY --from=builder /build/node/node/default_config.yaml /riveruser/xchain_node/config/config.yaml
COPY --from=builder /build/xchain/xchain/default_config.yaml /riveruser/xchain_node/config/config.yaml

RUN mkdir -p /riveruser/stream_node/logs
RUN mkdir -p /riveruser/xchain_node/logs
Expand Down
105 changes: 30 additions & 75 deletions core/node/auth/auth_impl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package auth
import (
"context"
"fmt"
"strings"
"sync"
"time"

Expand All @@ -14,13 +13,12 @@ import (
"github.com/river-build/river/core/node/infra"
. "github.com/river-build/river/core/node/protocol"
"github.com/river-build/river/core/node/shared"
"github.com/river-build/river/core/xchain/entitlement"

"github.com/ethereum/go-ethereum/common"
)

type ChainAuth interface {
IsEntitled(ctx context.Context, cfg *config.Config, args *ChainAuthArgs) error
IsEntitled(ctx context.Context, args *ChainAuthArgs) error
}

var everyone = common.HexToAddress("0x1") // This represents an Ethereum address of "0x1"
Expand Down Expand Up @@ -59,12 +57,11 @@ const (
)

type ChainAuthArgs struct {
kind chainAuthKind
spaceId shared.StreamId
channelId shared.StreamId
principal common.Address
permission Permission
linkedWallets string // a serialized list of linked wallets to comply with the cache key constraints
kind chainAuthKind
spaceId shared.StreamId
channelId shared.StreamId
principal common.Address
permission Permission
}

// Replaces principal with given wallet and returns new copy of args.
Expand All @@ -74,19 +71,6 @@ func (args *ChainAuthArgs) withWallet(wallet common.Address) *ChainAuthArgs {
return &ret
}

func (args *ChainAuthArgs) withLinkedWallets(linkedWallets []common.Address) *ChainAuthArgs {
ret := *args
var builder strings.Builder
for i, addr := range linkedWallets {
if i > 0 {
builder.WriteString(",")
}
builder.WriteString(addr.Hex())
}
ret.linkedWallets = builder.String()
return &ret
}

func newArgsForEnabledSpace(spaceId shared.StreamId) *ChainAuthArgs {
return &ChainAuthArgs{
kind: chainAuthKindSpaceEnabled,
Expand Down Expand Up @@ -179,11 +163,10 @@ func NewChainAuth(
}, nil
}

func (ca *chainAuth) IsEntitled(ctx context.Context, cfg *config.Config, args *ChainAuthArgs) error {
func (ca *chainAuth) IsEntitled(ctx context.Context, args *ChainAuthArgs) error {
// TODO: counter for cache hits here?
result, _, err := ca.entitlementCache.executeUsingCache(
ctx,
cfg,
args,
ca.checkEntitlement,
)
Expand All @@ -207,29 +190,28 @@ func (ca *chainAuth) IsEntitled(ctx context.Context, cfg *config.Config, args *C
return nil
}

func (ca *chainAuth) isWalletEntitled(ctx context.Context, cfg *config.Config, args *ChainAuthArgs) (bool, error) {
func (ca *chainAuth) isWalletEntitled(ctx context.Context, args *ChainAuthArgs) (bool, error) {
log := dlog.FromCtx(ctx)
if args.kind == chainAuthKindSpace {
log.Debug("isWalletEntitled", "kind", "space", "args", args)
return ca.isEntitledToSpace(ctx, cfg, args)
return ca.isEntitledToSpace(ctx, args)
} else if args.kind == chainAuthKindChannel {
log.Debug("isWalletEntitled", "kind", "channel", "args", args)
return ca.isEntitledToChannel(ctx, cfg, args)
return ca.isEntitledToChannel(ctx, args)
} else {
return false, RiverError(Err_INTERNAL, "Unknown chain auth kind").Func("isWalletEntitled")
}
}

func (ca *chainAuth) isSpaceEnabledUncached(ctx context.Context, cfg *config.Config, args *ChainAuthArgs) (CacheResult, error) {
func (ca *chainAuth) isSpaceEnabledUncached(ctx context.Context, args *ChainAuthArgs) (CacheResult, error) {
// This is awkward as we want enabled to be cached for 15 minutes, but the API returns the inverse
isDisabled, err := ca.spaceContract.IsSpaceDisabled(ctx, args.spaceId)
return &boolCacheResult{allowed: !isDisabled}, err
}

func (ca *chainAuth) checkSpaceEnabled(ctx context.Context, cfg *config.Config, spaceId shared.StreamId) error {
func (ca *chainAuth) checkSpaceEnabled(ctx context.Context, spaceId shared.StreamId) error {
isEnabled, cacheHit, err := ca.entitlementCache.executeUsingCache(
ctx,
cfg,
newArgsForEnabledSpace(spaceId),
ca.isSpaceEnabledUncached,
)
Expand All @@ -249,21 +231,19 @@ func (ca *chainAuth) checkSpaceEnabled(ctx context.Context, cfg *config.Config,
}
}

func (ca *chainAuth) isChannelEnabledUncached(ctx context.Context, cfg *config.Config, args *ChainAuthArgs) (CacheResult, error) {
func (ca *chainAuth) isChannelEnabledUncached(ctx context.Context, args *ChainAuthArgs) (CacheResult, error) {
// This is awkward as we want enabled to be cached for 15 minutes, but the API returns the inverse
isDisabled, err := ca.spaceContract.IsChannelDisabled(ctx, args.spaceId, args.channelId)
return &boolCacheResult{allowed: !isDisabled}, err
}

func (ca *chainAuth) checkChannelEnabled(
ctx context.Context,
cfg *config.Config,
spaceId shared.StreamId,
channelId shared.StreamId,
) error {
isEnabled, cacheHit, err := ca.entitlementCache.executeUsingCache(
ctx,
cfg,
newArgsForEnabledChannel(spaceId, channelId),
ca.isChannelEnabledUncached,
)
Expand Down Expand Up @@ -300,7 +280,6 @@ func (scr *entitlementCacheResult) IsAllowed() bool {
// If the call fails or the space is not found, the allowed flag is set to false so the negative caching time applies.
func (ca *chainAuth) getSpaceEntitlementsForPermissionUncached(
ctx context.Context,
cfg *config.Config,
args *ChainAuthArgs,
) (CacheResult, error) {
log := dlog.FromCtx(ctx)
Expand All @@ -322,21 +301,11 @@ func (ca *chainAuth) getSpaceEntitlementsForPermissionUncached(
return &entitlementCacheResult{allowed: true, entitlementData: entitlementData, owner: owner}, nil
}

func deserializeWallets(serialized string) []common.Address {
addressStrings := strings.Split(serialized, ",")
linkedWallets := make([]common.Address, len(addressStrings))
for i, addrStr := range addressStrings {
linkedWallets[i] = common.HexToAddress(addrStr)
}
return linkedWallets
}

func (ca *chainAuth) isEntitledToSpaceUncached(ctx context.Context, cfg *config.Config, args *ChainAuthArgs) (CacheResult, error) {
func (ca *chainAuth) isEntitledToSpaceUncached(ctx context.Context, args *ChainAuthArgs) (CacheResult, error) {
log := dlog.FromCtx(ctx)
log.Debug("isEntitledToSpaceUncached", "args", args)
result, cacheHit, err := ca.entitlementManagerCache.executeUsingCache(
ctx,
cfg,
args,
ca.getSpaceEntitlementsForPermissionUncached,
)
Expand Down Expand Up @@ -364,25 +333,12 @@ func (ca *chainAuth) isEntitledToSpaceUncached(ctx context.Context, cfg *config.

entitlementData := temp.(*entitlementCacheResult) // Assuming result is of *entitlementCacheResult type
log.Debug("entitlementData", "args", args, "entitlementData", entitlementData)
for _, ent := range entitlementData.entitlementData {
log.Debug("entitlement", "entitlement", ent)
if ent.entitlementType == "RuleEntitlement" {
re := ent.ruleEntitlement
log.Debug("RuleEntitlement", "ruleEntitlement", re)
result, err := entitlement.EvaluateRuleData(ctx, cfg, deserializeWallets(args.linkedWallets), re)

if err != nil {
return &boolCacheResult{allowed: false}, AsRiverError(err).Func("isEntitledToSpace")
}
if result {
log.Debug("rule entitlement is true", "spaceId", args.spaceId)
return &boolCacheResult{allowed: true}, nil
} else {
log.Debug("rule entitlement is false", "spaceId", args.spaceId)
return &boolCacheResult{allowed: false}, nil
}
} else if ent.entitlementType == "UserEntitlement" {
for _, user := range ent.userEntitlement {
for _, entitlement := range entitlementData.entitlementData {
log.Debug("entitlement", "entitlement", entitlement)
if entitlement.entitlementType == "RuleEntitlement" {
// TODO implement rule entitlment
} else if entitlement.entitlementType == "UserEntitlement" {
for _, user := range entitlement.userEntitlement {
if user == everyone {
log.Debug("everyone is entitled to space", "spaceId", args.spaceId)
return &boolCacheResult{allowed: true}, nil
Expand All @@ -392,19 +348,19 @@ func (ca *chainAuth) isEntitledToSpaceUncached(ctx context.Context, cfg *config.
}
}
} else {
log.Warn("Invalid entitlement type", "entitlement", ent)
log.Warn("Invalid entitlement type", "entitlement", entitlement)
}
}

return &boolCacheResult{allowed: false}, nil
}

func (ca *chainAuth) isEntitledToSpace(ctx context.Context, cfg *config.Config, args *ChainAuthArgs) (bool, error) {
func (ca *chainAuth) isEntitledToSpace(ctx context.Context, args *ChainAuthArgs) (bool, error) {
if args.kind != chainAuthKindSpace {
return false, RiverError(Err_INTERNAL, "Wrong chain auth kind")
}

isEntitled, cacheHit, err := ca.entitlementCache.executeUsingCache(ctx, cfg, args, ca.isEntitledToSpaceUncached)
isEntitled, cacheHit, err := ca.entitlementCache.executeUsingCache(ctx, args, ca.isEntitledToSpaceUncached)
if err != nil {
return false, err
}
Expand All @@ -417,7 +373,7 @@ func (ca *chainAuth) isEntitledToSpace(ctx context.Context, cfg *config.Config,
return isEntitled.IsAllowed(), nil
}

func (ca *chainAuth) isEntitledToChannelUncached(ctx context.Context, cfg *config.Config, args *ChainAuthArgs) (CacheResult, error) {
func (ca *chainAuth) isEntitledToChannelUncached(ctx context.Context, args *ChainAuthArgs) (CacheResult, error) {
allowed, err := ca.spaceContract.IsEntitledToChannel(
ctx,
args.spaceId,
Expand All @@ -428,12 +384,12 @@ func (ca *chainAuth) isEntitledToChannelUncached(ctx context.Context, cfg *confi
return &boolCacheResult{allowed: allowed}, err
}

func (ca *chainAuth) isEntitledToChannel(ctx context.Context, cfg *config.Config, args *ChainAuthArgs) (bool, error) {
func (ca *chainAuth) isEntitledToChannel(ctx context.Context, args *ChainAuthArgs) (bool, error) {
if args.kind != chainAuthKindChannel {
return false, RiverError(Err_INTERNAL, "Wrong chain auth kind")
}

isEntitled, cacheHit, err := ca.entitlementCache.executeUsingCache(ctx, cfg, args, ca.isEntitledToChannelUncached)
isEntitled, cacheHit, err := ca.entitlementCache.executeUsingCache(ctx, args, ca.isEntitledToChannelUncached)
if err != nil {
return false, err
}
Expand Down Expand Up @@ -496,19 +452,19 @@ func (ca *chainAuth) checkMembership(
* If any of the operations fail before getting positive result, the whole operation fails.
* A prerequisite for this function is that one of the linked wallets is a member of the space.
*/
func (ca *chainAuth) checkEntitlement(ctx context.Context, cfg *config.Config, args *ChainAuthArgs) (CacheResult, error) {
func (ca *chainAuth) checkEntitlement(ctx context.Context, args *ChainAuthArgs) (CacheResult, error) {
log := dlog.FromCtx(ctx)

ctx, cancel := context.WithTimeout(ctx, time.Millisecond*time.Duration(ca.contractCallsTimeoutMs))
defer cancel()

if args.kind == chainAuthKindSpace {
err := ca.checkSpaceEnabled(ctx, cfg, args.spaceId)
err := ca.checkSpaceEnabled(ctx, args.spaceId)
if err != nil {
return &boolCacheResult{allowed: false}, nil
}
} else if args.kind == chainAuthKindChannel {
err := ca.checkChannelEnabled(ctx, cfg, args.spaceId, args.channelId)
err := ca.checkChannelEnabled(ctx, args.spaceId, args.channelId)
if err != nil {
return &boolCacheResult{allowed: false}, nil
}
Expand All @@ -524,7 +480,6 @@ func (ca *chainAuth) checkEntitlement(ctx context.Context, cfg *config.Config, a

// Add the root key to the list of wallets.
wallets = append(wallets, args.principal)
args = args.withLinkedWallets(wallets)

isMemberCtx, isMemberCancel := context.WithCancel(ctx)
defer isMemberCancel()
Expand Down Expand Up @@ -579,7 +534,7 @@ func (ca *chainAuth) checkEntitlement(ctx context.Context, cfg *config.Config, a
wg.Add(1)
go func(address common.Address) {
defer wg.Done()
result, err := ca.isWalletEntitled(ctx, cfg, args.withWallet(address))
result, err := ca.isWalletEntitled(ctx, args.withWallet(address))
resultsChan <- entitlementCheckResult{allowed: result, err: err}
}(wallet)
}
Expand Down
5 changes: 2 additions & 3 deletions core/node/auth/auth_impl_cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,8 @@ func newEntitlementManagerCache(ctx context.Context, cfg *config.ChainConfig) (*

func (ec *entitlementCache) executeUsingCache(
ctx context.Context,
cfg *config.Config,
key *ChainAuthArgs,
onMiss func(context.Context, *config.Config, *ChainAuthArgs) (CacheResult, error),
onMiss func(context.Context, *ChainAuthArgs) (CacheResult, error),
) (CacheResult, bool, error) {
// Check positive cache first
if val, ok := ec.positiveCache.Get(*key); ok {
Expand All @@ -168,7 +167,7 @@ func (ec *entitlementCache) executeUsingCache(
}

// Cache miss, execute the closure
result, err := onMiss(ctx, cfg, key)
result, err := onMiss(ctx, key)
if err != nil {
return nil, false, err
}
Expand Down
8 changes: 2 additions & 6 deletions core/node/auth/auth_impl_cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ func TestCache(t *testing.T) {
ctx, cancel := test.NewTestContext()
defer cancel()

cfg := &config.Config{}

c, err := newEntitlementCache(
ctx,
&config.ChainConfig{
Expand All @@ -43,9 +41,8 @@ func TestCache(t *testing.T) {
var cacheMissForReal bool
result, cacheHit, err := c.executeUsingCache(
ctx,
cfg,
NewChainAuthArgsForChannel(spaceId, channelId, "3", PermissionWrite),
func(context.Context, *config.Config, *ChainAuthArgs) (CacheResult, error) {
func(context.Context, *ChainAuthArgs) (CacheResult, error) {
cacheMissForReal = true
return &simpleCacheResult{allowed: true}, nil
},
Expand All @@ -58,9 +55,8 @@ func TestCache(t *testing.T) {
cacheMissForReal = false
result, cacheHit, err = c.executeUsingCache(
ctx,
cfg,
NewChainAuthArgsForChannel(spaceId, channelId, "3", PermissionWrite),
func(context.Context, *config.Config, *ChainAuthArgs) (CacheResult, error) {
func(context.Context, *ChainAuthArgs) (CacheResult, error) {
cacheMissForReal = true
return &simpleCacheResult{allowed: false}, nil
},
Expand Down
4 changes: 1 addition & 3 deletions core/node/auth/fake_auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package auth

import (
"context"

"github.com/river-build/river/core/node/config"
)

// This checkers always returns true, used for some testing scenarios.
Expand All @@ -15,6 +13,6 @@ type fakeChainAuth struct{}

var _ ChainAuth = (*fakeChainAuth)(nil)

func (a *fakeChainAuth) IsEntitled(ctx context.Context, cfg *config.Config, args *ChainAuthArgs) error {
func (a *fakeChainAuth) IsEntitled(ctx context.Context, args *ChainAuthArgs) error {
return nil
}
5 changes: 3 additions & 2 deletions core/node/auth/space_contract.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ package auth
import (
"context"

v3 "github.com/river-build/river/core/xchain/contracts/v3"

"github.com/ethereum/go-ethereum/common"
"github.com/river-build/river/core/node/shared"
"github.com/river-build/river/core/xchain/contracts"
)

type SpaceEntitlements struct {
entitlementType string
ruleEntitlement *contracts.IRuleData
ruleEntitlement v3.IRuleEntitlementRuleData
userEntitlement []common.Address
}

Expand Down
Loading

0 comments on commit 97465bc

Please sign in to comment.