Skip to content

Commit

Permalink
refactor(server/authz): support separate poll durations for policy an…
Browse files Browse the repository at this point in the history
…d data
  • Loading branch information
GeorgeMac committed May 24, 2024
1 parent 4152ea7 commit b78dfb3
Show file tree
Hide file tree
Showing 11 changed files with 101 additions and 76 deletions.
16 changes: 10 additions & 6 deletions config/flipt.schema.cue
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,18 @@ import "strings"

#authorization: {
#authorizationSource: {
backend: "filesystem"
filesystem: path: string
backend: "local"
local: path: string
poll_duration?: =~#duration
}

required?: bool | *false
policy?: #authorizationSource
data?: #authorizationSource
poll_duration?: =~#duration | "30s"
required?: bool | *false
policy?: #authorizationSource & {
poll_duration: =~#duration | *"5m"
}
data?: #authorizationSource & {
poll_duration: =~#duration | *"30s"
}
}

#cache: {
Expand Down
13 changes: 6 additions & 7 deletions config/flipt.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -333,11 +333,6 @@
"data": {
"type": ["object", "null"],
"$ref": "#/definitions/authorization/$defs/authorization_source"
},
"poll_duration": {
"type": "string",
"pattern": "^([0-9]+(ns|us|µs|ms|s|m|h))+$",
"default": "30s"
}
},
"$defs": {
Expand All @@ -347,14 +342,18 @@
"properties": {
"backend": {
"type": "string",
"enum": ["filesystem"]
"enum": ["local"]
},
"filesystem": {
"local": {
"type": "object",
"additionalProperties": false,
"properties": {
"path": {"type": "string"}
}
},
"poll_duration": {
"type": "string",
"pattern": "^([0-9]+(ns|us|µs|ms|s|m|h))+$"
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions internal/cmd/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,9 +364,9 @@ func NewGRPCServer(

if cfg.Authorization.Data != nil {
switch cfg.Authorization.Data.Backend {
case config.AuthorizationBackendFilesystem:
case config.AuthorizationBackendLocal:
engineOpts = append(engineOpts, authz.WithDataSource(
filesystem.DataSourceFromPath(cfg.Authorization.Data.Filesystem.Path),
filesystem.DataSourceFromPath(cfg.Authorization.Data.Local.Path),

Check failure on line 369 in internal/cmd/grpc.go

View workflow job for this annotation

GitHub Actions / Lint Go

not enough arguments in call to authz.WithDataSource

Check failure on line 369 in internal/cmd/grpc.go

View workflow job for this annotation

GitHub Actions / Lint Go

not enough arguments in call to authz.WithDataSource

Check failure on line 369 in internal/cmd/grpc.go

View workflow job for this annotation

GitHub Actions / Benchmark SQLite

not enough arguments in call to authz.WithDataSource
))
default:
return nil, fmt.Errorf("unexpected authz data backend type: %q", cfg.Authorization.Data.Backend)
Expand All @@ -375,8 +375,8 @@ func NewGRPCServer(

var source authz.PolicySource
switch cfg.Authorization.Policy.Backend {
case config.AuthorizationBackendFilesystem:
source = filesystem.PolicySourceFromPath(cfg.Authorization.Policy.Filesystem.Path)
case config.AuthorizationBackendLocal:
source = filesystem.PolicySourceFromPath(cfg.Authorization.Policy.Local.Path)
default:
return nil, fmt.Errorf("unexpected authz policy backend type: %q", cfg.Authorization.Policy.Backend)
}
Expand Down
46 changes: 29 additions & 17 deletions internal/config/authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,28 @@ var (
type AuthorizationConfig struct {
// Required designates whether authorization credentials are validated.
// If required == true, then authorization is required for all API endpoints.
Required bool `json:"required,omitempty" mapstructure:"required" yaml:"required,omitempty"`
Policy *AuthorizationSourceConfig `json:"policy,omitempty" mapstructure:"policy,omitempty" yaml:"policy,omitempty"`
Data *AuthorizationSourceConfig `json:"data,omitempty" mapstructure:"data,omitempty" yaml:"data,omitempty"`
PollDuration time.Duration `json:"pollDuration,omitempty" mapstructure:"poll_duration" yaml:"poll_duration,omitempty"`
Required bool `json:"required,omitempty" mapstructure:"required" yaml:"required,omitempty"`
Policy *AuthorizationSourceConfig `json:"policy,omitempty" mapstructure:"policy,omitempty" yaml:"policy,omitempty"`
Data *AuthorizationSourceConfig `json:"data,omitempty" mapstructure:"data,omitempty" yaml:"data,omitempty"`
}

func (c *AuthorizationConfig) setDefaults(v *viper.Viper) error {
v.SetDefault("authorization", map[string]interface{}{
"required": false,
"poll_duration": "30s",
})
auth := map[string]any{"required": false}
if v.GetBool("authorization.required") {
auth["policy"] = map[string]any{
"backend": "local",
"poll_duration": "5m",
}

if v.GetString("authorization.data.local.path") != "" {
auth["data"] = map[string]any{
"backend": "local",
"poll_duration": "30s",
}
}

v.SetDefault("authorization", auth)
}

return nil
}
Expand All @@ -50,8 +61,9 @@ func (c *AuthorizationConfig) validate() error {
}

type AuthorizationSourceConfig struct {
Backend AuthorizationBackend `json:"backend,omitempty" mapstructure:"backend" yaml:"backend,omitempty"`
Filesystem AuthorizationFilesystemConfig `json:"filesystem,omitempty" mapstructure:"filesystem" yaml:"filesystem,omitempty"`
Backend AuthorizationBackend `json:"backend,omitempty" mapstructure:"backend" yaml:"backend,omitempty"`
Local *AuthorizationLocalConfig `json:"local,omitempty" mapstructure:"local,omitempty" yaml:"local,omitempty"`
PollDuration time.Duration `json:"pollDuration,omitempty" mapstructure:"poll_duration" yaml:"poll_duration,omitempty"`
}

func (a *AuthorizationSourceConfig) validate() (err error) {
Expand All @@ -65,12 +77,12 @@ func (a *AuthorizationSourceConfig) validate() (err error) {
return nil
}

if a.Backend != AuthorizationBackendFilesystem {
return errors.New("backend must be one of [filesystem]")
if a.Backend != AuthorizationBackendLocal {
return errors.New("backend must be one of [local]")
}

if a.Filesystem.Path == "" {
return errors.New("filesystem: path must be non-empty string")
if a.Local == nil || a.Local.Path == "" {
return errors.New("local: path must be non-empty string")
}

return nil
Expand All @@ -81,11 +93,11 @@ func (a *AuthorizationSourceConfig) validate() (err error) {
type AuthorizationBackend string

const (
AuthorizationBackendFilesystem = "filesystem"
AuthorizationBackendLocal = "local"
)

// AuthorizationFilesystemConfig configures the filesystem backend source
// AuthorizationLocalConfig configures the local backend source
// for the authorization evaluation engines policies and data
type AuthorizationFilesystemConfig struct {
type AuthorizationLocalConfig struct {
Path string `json:"path,omitempty" mapstructure:"path" yaml:"path,omitempty"`
}
4 changes: 1 addition & 3 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -637,8 +637,6 @@ func Default() *Config {
},
},

Authorization: AuthorizationConfig{
PollDuration: 30 * time.Second,
},
Authorization: AuthorizationConfig{},
}
}
17 changes: 9 additions & 8 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -617,12 +617,12 @@ func TestLoad(t *testing.T) {
cfg.Authorization = AuthorizationConfig{
Required: true,
Policy: &AuthorizationSourceConfig{
Backend: AuthorizationBackendFilesystem,
Filesystem: AuthorizationFilesystemConfig{
Backend: AuthorizationBackendLocal,
Local: &AuthorizationLocalConfig{
Path: "/path/to/policy.rego",
},
PollDuration: 30 * time.Second,
},
PollDuration: 30 * time.Second,
}

cfg.Authentication = AuthenticationConfig{
Expand Down Expand Up @@ -860,18 +860,19 @@ func TestLoad(t *testing.T) {
cfg.Authorization = AuthorizationConfig{
Required: true,
Policy: &AuthorizationSourceConfig{
Backend: AuthorizationBackendFilesystem,
Filesystem: AuthorizationFilesystemConfig{
Backend: AuthorizationBackendLocal,
Local: &AuthorizationLocalConfig{
Path: "/path/to/policy.rego",
},
PollDuration: time.Minute,
},
Data: &AuthorizationSourceConfig{
Backend: AuthorizationBackendFilesystem,
Filesystem: AuthorizationFilesystemConfig{
Backend: AuthorizationBackendLocal,
Local: &AuthorizationLocalConfig{
Path: "/path/to/policy/data.json",
},
PollDuration: 30 * time.Second,
},
PollDuration: 30 * time.Second,
}

return cfg
Expand Down
9 changes: 5 additions & 4 deletions internal/config/testdata/advanced.yml
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,11 @@ authentication:
authorization:
required: true
policy:
backend: filesystem
filesystem:
backend: local
local:
path: "/path/to/policy.rego"
poll_duration: 1m
data:
backend: filesystem
filesystem:
backend: local
local:
path: "/path/to/policy/data.json"
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ authentication:
authorization:
required: true
policy:
backend: filesystem
filesystem:
backend: local
local:
path: "/path/to/policy.rego"
poll_duration: 30s
3 changes: 0 additions & 3 deletions internal/config/testdata/marshal/yaml/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,3 @@ db:
meta:
check_for_updates: true
telemetry_enabled: true

authorization:
poll_duration: 30s
51 changes: 31 additions & 20 deletions internal/server/authz/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
var (
_ Verifier = &Engine{}

defaultPollDuration = 30 * time.Second
defaultPolicyPollDuration = 5 * time.Minute

// ErrNotModified is returned from a source when the data has not
// been modified, identified based on the provided hash value
Expand Down Expand Up @@ -49,12 +49,14 @@ type Engine struct {
dataSource DataSource
dataHash []byte

pollDuration time.Duration
pollDuration time.Duration
dataSourcePollDuration time.Duration
}

func WithDataSource(source DataSource) containers.Option[Engine] {
func WithDataSource(source DataSource, pollDuration time.Duration) containers.Option[Engine] {
return func(e *Engine) {
e.dataSource = source
e.dataSourcePollDuration = pollDuration
}
}

Expand All @@ -69,17 +71,36 @@ func NewEngine(ctx context.Context, logger *zap.Logger, source PolicySource, opt
logger: logger,
policySource: source,
store: inmem.New(),
pollDuration: defaultPollDuration,
pollDuration: defaultPolicyPollDuration,
}

containers.ApplyAll(engine, opts...)

err := engine.update(ctx, storage.AddOp)
if err != nil {
// update data store with initial data if source is configured
if err := engine.updateData(ctx, storage.AddOp); err != nil {
return nil, err
}

// fetch policy and then compile and set query engine
if err := engine.updatePolicy(ctx); err != nil {
return nil, err
}

go engine.pollData(ctx)
// begin polling for updates for policy
go poll(ctx, engine.pollDuration, func() {
if err := engine.updatePolicy(ctx); err != nil {
engine.logger.Error("updating policy", zap.Error(err))
}
})

// being polling for updates to data if source configured
if engine.dataSource != nil {
go poll(ctx, engine.pollDuration, func() {
if err := engine.updateData(ctx, storage.ReplaceOp); err != nil {
engine.logger.Error("updating data", zap.Error(err))
}
})
}

return engine, nil
}
Expand All @@ -100,28 +121,18 @@ func (e *Engine) IsAllowed(ctx context.Context, input map[string]interface{}) (b
return results[0].Expressions[0].Value.(bool), nil
}

func (e *Engine) pollData(ctx context.Context) {
ticker := time.NewTicker(e.pollDuration)
func poll(ctx context.Context, d time.Duration, fn func()) {
ticker := time.NewTicker(d)
for {
select {
case <-ctx.Done():
return
case <-ticker.C:
if err := e.update(ctx, storage.ReplaceOp); err != nil {
e.logger.Error("updating policy and data", zap.Error(err))
}
fn()
}
}
}

func (e *Engine) update(ctx context.Context, op storage.PatchOp) error {
if err := e.updateData(ctx, op); err != nil {
return err
}

return e.updatePolicy(ctx)
}

func (e *Engine) updatePolicy(ctx context.Context) error {
e.mu.Lock()
defer e.mu.Unlock()
Expand Down
5 changes: 3 additions & 2 deletions internal/server/authz/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ import (
"context"
"encoding/json"
"testing"
"time"

"github.com/stretchr/testify/require"
"go.uber.org/zap/zaptest"
)

func TestEngine_NewEngine(t *testing.T) {
ctx := context.Background()
engine, err := NewEngine(ctx, zaptest.NewLogger(t), testRBACPolicy, WithDataSource(testRoleDefinitions))
engine, err := NewEngine(ctx, zaptest.NewLogger(t), testRBACPolicy, WithDataSource(testRoleDefinitions, 5*time.Second))
require.NoError(t, err)
require.NotNil(t, engine)
}
Expand Down Expand Up @@ -189,7 +190,7 @@ func TestEngine_IsAllowed(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx := context.Background()
engine, err := NewEngine(ctx, zaptest.NewLogger(t), testRBACPolicy, WithDataSource(testRoleDefinitions))
engine, err := NewEngine(ctx, zaptest.NewLogger(t), testRBACPolicy, WithDataSource(testRoleDefinitions, 5*time.Second))
require.NoError(t, err)

var input map[string]interface{}
Expand Down

0 comments on commit b78dfb3

Please sign in to comment.