Skip to content

Commit 8d0f6c7

Browse files
akclaceakclace
committed
Refactor RBAC code to remove callback functions
1 parent 2e8946c commit 8d0f6c7

File tree

17 files changed

+245
-201
lines changed

17 files changed

+245
-201
lines changed

‎internal/app/action/action.go‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/openrundev/openrun/internal/app/appfs"
2727
"github.com/openrundev/openrun/internal/app/apptype"
2828
"github.com/openrundev/openrun/internal/app/starlark_type"
29+
"github.com/openrundev/openrun/internal/rbac"
2930
"github.com/openrundev/openrun/internal/system"
3031
"github.com/openrundev/openrun/internal/types"
3132
"go.starlark.net/starlark"
@@ -73,15 +74,15 @@ type Action struct {
7374
appPathDomain types.AppPathDomain
7475
serverConfig *types.ServerConfig
7576
permit []string
76-
authorizer types.AuthorizerFunc // can be null
77+
rbacApi rbac.RBACAPI
7778
}
7879

7980
// NewAction creates a new action
8081
func NewAction(logger *types.Logger, sourceFS *appfs.SourceFs, isDev bool, name, description, apath string, run, suggest starlark.Callable,
8182
params []apptype.AppParam, paramValuesStr map[string]string, paramDict starlark.StringDict,
8283
appPath string, styleType types.StyleType, containerProxyUrl string, hidden []string, showValidate bool,
8384
auditInsert func(*types.AuditEvent) error, containerManager any, jsLibs []types.JSLibrary, appPathDomain types.AppPathDomain,
84-
serverConfig *types.ServerConfig, permit []string, authorizer types.AuthorizerFunc) (*Action, error) {
85+
serverConfig *types.ServerConfig, permit []string, rbacApi rbac.RBACAPI) (*Action, error) {
8586

8687
funcMap := system.GetFuncMap()
8788

@@ -158,7 +159,7 @@ func NewAction(logger *types.Logger, sourceFS *appfs.SourceFs, isDev bool, name,
158159
appPathDomain: appPathDomain,
159160
serverConfig: serverConfig,
160161
permit: permit,
161-
authorizer: authorizer,
162+
rbacApi: rbacApi,
162163
}, nil
163164
}
164165

@@ -213,8 +214,8 @@ func (a *Action) validateAction(w http.ResponseWriter, r *http.Request) {
213214
}
214215

215216
func (a *Action) authorizeAction(w http.ResponseWriter, r *http.Request) bool {
216-
if a.authorizer != nil && len(a.permit) > 0 {
217-
authorized, err := a.authorizer(r.Context(), a.permit)
217+
if a.rbacApi != nil && len(a.permit) > 0 {
218+
authorized, err := a.rbacApi.AuthorizeAny(r.Context(), a.permit)
218219
if err != nil {
219220
http.Error(w, err.Error(), http.StatusInternalServerError)
220221
return false
@@ -872,9 +873,9 @@ func (a *Action) getLinksWithQS(ctx context.Context, qs string) []ActionLink {
872873
linksWithQS := make([]ActionLink, 0, len(a.Links))
873874
for _, link := range a.Links {
874875
authorized := true
875-
if a.authorizer != nil && len(link.Permits) > 0 {
876+
if a.rbacApi != nil && len(link.Permits) > 0 {
876877
var err error
877-
authorized, err = a.authorizer(ctx, link.Permits)
878+
authorized, err = a.rbacApi.AuthorizeAny(ctx, link.Permits)
878879
if err != nil {
879880
a.Error().Msgf("error authorizing link %s: %s", link.Name, err)
880881
}

‎internal/app/app.go‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"github.com/openrundev/openrun/internal/app/apptype"
3030
"github.com/openrundev/openrun/internal/app/dev"
3131
"github.com/openrundev/openrun/internal/app/starlark_type"
32+
"github.com/openrundev/openrun/internal/rbac"
3233
"github.com/openrundev/openrun/internal/system"
3334
"github.com/openrundev/openrun/internal/types"
3435
"go.starlark.net/starlark"
@@ -87,9 +88,8 @@ type App struct {
8788
lastRequestTime atomic.Int64
8889
secretEvalFunc func([][]string, string, string) (string, error)
8990
auditInsert func(*types.AuditEvent) error
90-
AppRunPath string // path to the app run directory
91-
authorizer types.AuthorizerFunc // the authorizer function to use, can be null
92-
customPermsFunc types.CustomPermsFunc // the custom permissions function to use, can be null
91+
AppRunPath string // path to the app run directory
92+
rbacApi rbac.RBACAPI // the rbac api to use
9393
}
9494

9595
type starlarkCacheEntry struct {
@@ -107,20 +107,19 @@ func NewApp(sourceFS *appfs.SourceFs, workFS *appfs.WorkFs, logger *types.Logger
107107
plugins map[string]types.PluginSettings, appConfig types.AppConfig, notifyClose chan<- types.AppPathDomain,
108108
secretEvalFunc func([][]string, string, string) (string, error),
109109
auditInsert func(*types.AuditEvent) error, serverConfig *types.ServerConfig,
110-
authorizer types.AuthorizerFunc, customPermsFunc types.CustomPermsFunc) (*App, error) {
110+
rbacApi rbac.RBACAPI) (*App, error) {
111111
newApp := &App{
112-
sourceFS: sourceFS,
113-
Logger: logger,
114-
AppEntry: appEntry,
115-
systemConfig: systemConfig,
116-
starlarkCache: map[string]*starlarkCacheEntry{},
117-
notifyClose: notifyClose,
118-
secretEvalFunc: secretEvalFunc,
119-
appStyle: &dev.AppStyle{},
120-
auditInsert: auditInsert,
121-
serverConfig: serverConfig,
122-
authorizer: authorizer,
123-
customPermsFunc: customPermsFunc,
112+
sourceFS: sourceFS,
113+
Logger: logger,
114+
AppEntry: appEntry,
115+
systemConfig: systemConfig,
116+
starlarkCache: map[string]*starlarkCacheEntry{},
117+
notifyClose: notifyClose,
118+
secretEvalFunc: secretEvalFunc,
119+
appStyle: &dev.AppStyle{},
120+
auditInsert: auditInsert,
121+
serverConfig: serverConfig,
122+
rbacApi: rbacApi,
124123
}
125124
newApp.plugins = NewAppPlugins(newApp, plugins, appEntry.Metadata.Accounts)
126125
newApp.AppConfig = appConfig

‎internal/app/setup.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,7 @@ func (a *App) addAction(count int, val starlark.Value, router *chi.Mux) (err err
561561
}
562562
action, err := action.NewAction(a.Logger, a.sourceFS, a.IsDev, name, description, path, run, suggest,
563563
slices.Collect(maps.Values(a.paramInfo)), a.paramValuesStr, a.paramDict, a.Path, a.appStyle.GetStyleType(),
564-
containerProxyUrl, hidden, showValidate, a.auditInsert, a.containerManager, a.jsLibs, a.AppPathDomain(), a.serverConfig, permit, a.authorizer)
564+
containerProxyUrl, hidden, showValidate, a.auditInsert, a.containerManager, a.jsLibs, a.AppPathDomain(), a.serverConfig, permit, a.rbacApi)
565565
if err != nil {
566566
return fmt.Errorf("error creating action %s: %w", name, err)
567567
}
@@ -861,8 +861,8 @@ func (a *App) addProxyConfig(count int, router *chi.Mux, proxyDef *starlarkstruc
861861
}
862862

863863
customPerms := make([]string, 0)
864-
if a.customPermsFunc != nil {
865-
customPerms, err = a.customPermsFunc(r.Context())
864+
if a.rbacApi != nil {
865+
customPerms, err = a.rbacApi.GetCustomPermissions(r.Context())
866866
}
867867
// Add the user and custom permissions to the request headers
868868
r.Header.Set(types.OPENRUN_HEADER_PERMS, strings.Join(customPerms, ","))

‎internal/app/tests/app_test_helper.go‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
"github.com/openrundev/openrun/internal/app"
1616
"github.com/openrundev/openrun/internal/app/appfs"
17+
"github.com/openrundev/openrun/internal/rbac"
1718
"github.com/openrundev/openrun/internal/system"
1819
"github.com/openrundev/openrun/internal/types"
1920

@@ -22,54 +23,54 @@ import (
2223
)
2324

2425
func CreateDevModeTestApp(logger *types.Logger, fileData map[string]string) (*app.App, *appfs.WorkFs, error) {
25-
return CreateTestAppInt(logger, "/test", fileData, true, nil, nil, nil, "app_dev_testapp", types.AppSettings{}, nil, nil, nil, nil)
26+
return CreateTestAppInt(logger, "/test", fileData, true, nil, nil, nil, "app_dev_testapp", types.AppSettings{}, nil, nil, nil)
2627
}
2728

2829
func CreateTestApp(logger *types.Logger, fileData map[string]string) (*app.App, *appfs.WorkFs, error) {
29-
return CreateTestAppInt(logger, "/test", fileData, false, nil, nil, nil, "app_prd_testapp", types.AppSettings{}, nil, nil, nil, nil)
30+
return CreateTestAppInt(logger, "/test", fileData, false, nil, nil, nil, "app_prd_testapp", types.AppSettings{}, nil, nil, nil)
3031
}
3132

3233
func CreateTestAppConfig(logger *types.Logger, fileData map[string]string, appConfig types.AppConfig) (*app.App, *appfs.WorkFs, error) {
33-
return CreateTestAppInt(logger, "/test", fileData, false, nil, nil, nil, "app_prd_testapp", types.AppSettings{}, nil, &appConfig, nil, nil)
34+
return CreateTestAppInt(logger, "/test", fileData, false, nil, nil, nil, "app_prd_testapp", types.AppSettings{}, nil, &appConfig, nil)
3435
}
3536

3637
func CreateTestAppParams(logger *types.Logger, fileData map[string]string, params map[string]string) (*app.App, *appfs.WorkFs, error) {
37-
return CreateTestAppInt(logger, "/test", fileData, false, nil, nil, nil, "app_prd_testapp", types.AppSettings{}, params, nil, nil, nil)
38+
return CreateTestAppInt(logger, "/test", fileData, false, nil, nil, nil, "app_prd_testapp", types.AppSettings{}, params, nil, nil)
3839
}
3940

4041
func CreateTestAppRoot(logger *types.Logger, fileData map[string]string) (*app.App, *appfs.WorkFs, error) {
41-
return CreateTestAppInt(logger, "/", fileData, false, nil, nil, nil, "app_prd_testapp", types.AppSettings{}, nil, nil, nil, nil)
42+
return CreateTestAppInt(logger, "/", fileData, false, nil, nil, nil, "app_prd_testapp", types.AppSettings{}, nil, nil, nil)
4243
}
4344

4445
func CreateTestAppPlugin(logger *types.Logger, fileData map[string]string,
4546
plugins []string, permissions []types.Permission, pluginConfig map[string]types.PluginSettings) (*app.App, *appfs.WorkFs, error) {
46-
return CreateTestAppInt(logger, "/test", fileData, false, plugins, permissions, pluginConfig, "app_prd_testapp", types.AppSettings{}, nil, nil, nil, nil)
47+
return CreateTestAppInt(logger, "/test", fileData, false, plugins, permissions, pluginConfig, "app_prd_testapp", types.AppSettings{}, nil, nil, nil)
4748
}
4849

4950
func CreateTestAppPluginRoot(logger *types.Logger, fileData map[string]string,
5051
plugins []string, permissions []types.Permission, pluginConfig map[string]types.PluginSettings) (*app.App, *appfs.WorkFs, error) {
51-
return CreateTestAppInt(logger, "/", fileData, false, plugins, permissions, pluginConfig, "app_prd_testapp", types.AppSettings{}, nil, nil, nil, nil)
52+
return CreateTestAppInt(logger, "/", fileData, false, plugins, permissions, pluginConfig, "app_prd_testapp", types.AppSettings{}, nil, nil, nil)
5253
}
5354

5455
func CreateDevAppPlugin(logger *types.Logger, fileData map[string]string, plugins []string,
5556
permissions []types.Permission, pluginConfig map[string]types.PluginSettings) (*app.App, *appfs.WorkFs, error) {
56-
return CreateTestAppInt(logger, "/test", fileData, true, plugins, permissions, pluginConfig, "app_dev_testapp", types.AppSettings{}, nil, nil, nil, nil)
57+
return CreateTestAppInt(logger, "/test", fileData, true, plugins, permissions, pluginConfig, "app_dev_testapp", types.AppSettings{}, nil, nil, nil)
5758
}
5859

5960
func CreateTestAppPluginId(logger *types.Logger, fileData map[string]string,
6061
plugins []string, permissions []types.Permission, pluginConfig map[string]types.PluginSettings, id string, settings types.AppSettings) (*app.App, *appfs.WorkFs, error) {
61-
return CreateTestAppInt(logger, "/test", fileData, false, plugins, permissions, pluginConfig, id, settings, nil, nil, nil, nil)
62+
return CreateTestAppInt(logger, "/test", fileData, false, plugins, permissions, pluginConfig, id, settings, nil, nil, nil)
6263
}
6364

6465
func CreateTestAppAuthorizer(logger *types.Logger, fileData map[string]string,
65-
plugins []string, permissions []types.Permission, pluginConfig map[string]types.PluginSettings, authorizer types.AuthorizerFunc, customPermsFunc types.CustomPermsFunc) (*app.App, *appfs.WorkFs, error) {
66-
return CreateTestAppInt(logger, "/test", fileData, false, plugins, permissions, pluginConfig, "app_prd_testapp", types.AppSettings{}, nil, nil, authorizer, customPermsFunc)
66+
plugins []string, permissions []types.Permission, pluginConfig map[string]types.PluginSettings, rbacApi rbac.RBACAPI) (*app.App, *appfs.WorkFs, error) {
67+
return CreateTestAppInt(logger, "/test", fileData, false, plugins, permissions, pluginConfig, "app_prd_testapp", types.AppSettings{}, nil, nil, rbacApi)
6768
}
6869

6970
func CreateTestAppInt(logger *types.Logger, path string, fileData map[string]string, isDev bool,
7071
plugins []string, permissions []types.Permission, pluginConfig map[string]types.PluginSettings,
7172
id string, settings types.AppSettings, params map[string]string, appConfig *types.AppConfig,
72-
authorizer types.AuthorizerFunc, customPermsFunc types.CustomPermsFunc) (*app.App, *appfs.WorkFs, error) {
73+
rbacApi rbac.RBACAPI) (*app.App, *appfs.WorkFs, error) {
7374
systemConfig := types.SystemConfig{TailwindCSSCommand: "", AllowedEnv: []string{"HOME"}}
7475
var fs appfs.ReadableFS
7576
if isDev {
@@ -113,7 +114,7 @@ func CreateTestAppInt(logger *types.Logger, path string, fileData map[string]str
113114
workFS := appfs.NewWorkFs("", &TestWriteFS{TestReadFS: &TestReadFS{fileData: map[string]string{}}})
114115
a, err := app.NewApp(sourceFS, workFS, logger,
115116
createTestAppEntry(id, path, isDev, metadata), &systemConfig, pluginConfig, *appConfig,
116-
nil, secretManager.AppEvalTemplate, nil, &types.ServerConfig{}, authorizer, customPermsFunc)
117+
nil, secretManager.AppEvalTemplate, nil, &types.ServerConfig{}, rbacApi)
117118
if err != nil {
118119
return nil, nil, err
119120
}

‎internal/app/tests/proxy_test.go‎

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,23 @@ import (
1515
"github.com/openrundev/openrun/internal/types"
1616
)
1717

18+
// testRBAC is a minimal RBACAPI implementation for tests
19+
type testRBAC struct {
20+
perms []string
21+
}
22+
23+
func (t *testRBAC) AuthorizeAny(ctx context.Context, permissions []string) (bool, error) {
24+
return true, nil
25+
}
26+
27+
func (t *testRBAC) Authorize(ctx context.Context, permission types.RBACPermission, isAppLevelPermission bool) (bool, error) {
28+
return true, nil
29+
}
30+
31+
func (t *testRBAC) GetCustomPermissions(ctx context.Context) ([]string, error) {
32+
return t.perms, nil
33+
}
34+
1835
func TestProxyBasics(t *testing.T) {
1936
testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
2037
if r.URL.Path != "/abc" {
@@ -677,21 +694,23 @@ permissions=[
677694
)`, testServer.URL),
678695
}
679696

680-
// Create custom authorizer and perms func
681-
authorizer := func(ctx context.Context, permissions []string) (bool, error) {
682-
// Always allow
683-
return true, nil
684-
}
697+
/*
698+
// Create custom authorizer and perms func
699+
_ := func(ctx context.Context, permissions []string) (bool, error) {
700+
// Always allow
701+
return true, nil
702+
}
685703
686-
customPermsFunc := func(ctx context.Context) ([]string, error) {
687-
// Return custom permissions
688-
return []string{"read:data", "write:data", "admin"}, nil
689-
}
704+
_ := func(ctx context.Context) ([]string, error) {
705+
// Return custom permissions
706+
return []string{"read:data", "write:data", "admin"}, nil
707+
}
708+
*/
690709

691710
a, _, err := CreateTestAppAuthorizer(logger, fileData, []string{"proxy.in"},
692711
[]types.Permission{
693712
{Plugin: "proxy.in", Method: "config"},
694-
}, map[string]types.PluginSettings{}, authorizer, customPermsFunc)
713+
}, map[string]types.PluginSettings{}, &testRBAC{perms: []string{"read:data", "write:data", "admin"}})
695714
if err != nil {
696715
t.Fatalf("Error %s", err)
697716
}
@@ -733,21 +752,23 @@ permissions=[
733752
)`, testServer.URL),
734753
}
735754

736-
// Create custom authorizer that sets a user in context
737-
authorizer := func(ctx context.Context, permissions []string) (bool, error) {
738-
// Always allow
739-
return true, nil
740-
}
755+
/*
756+
// Create custom authorizer that sets a user in context
757+
authorizer := func(ctx context.Context, permissions []string) (bool, error) {
758+
// Always allow
759+
return true, nil
760+
}
741761
742-
customPermsFunc := func(ctx context.Context) ([]string, error) {
743-
// Return empty custom permissions
744-
return []string{}, nil
745-
}
762+
customPermsFunc := func(ctx context.Context) ([]string, error) {
763+
// Return empty custom permissions
764+
return []string{}, nil
765+
}
766+
*/
746767

747768
a, _, err := CreateTestAppAuthorizer(logger, fileData, []string{"proxy.in"},
748769
[]types.Permission{
749770
{Plugin: "proxy.in", Method: "config"},
750-
}, map[string]types.PluginSettings{}, authorizer, customPermsFunc)
771+
}, map[string]types.PluginSettings{}, &testRBAC{perms: []string{}})
751772
if err != nil {
752773
t.Fatalf("Error %s", err)
753774
}

‎internal/server/pathglob.go‎ renamed to ‎internal/rbac/pathglob.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright (c) ClaceIO, LLC
22
// SPDX-License-Identifier: Apache-2.0
33

4-
package server
4+
package rbac
55

66
import (
77
"fmt"

‎internal/server/pathglob_test.go‎ renamed to ‎internal/rbac/pathglob_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright (c) ClaceIO, LLC
22
// SPDX-License-Identifier: Apache-2.0
33

4-
package server
4+
package rbac
55

66
import (
77
"fmt"

0 commit comments

Comments
 (0)