Skip to content

Commit d99c5e5

Browse files
committed
Added action level rbac access checks
1 parent 54b6fd5 commit d99c5e5

File tree

12 files changed

+232
-45
lines changed

12 files changed

+232
-45
lines changed

internal/app/action/action.go

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package action
55

66
import (
7+
"context"
78
"embed"
89
"encoding/json"
910
"fmt"
@@ -36,8 +37,10 @@ var embedHtml embed.FS
3637
var embedFS = hashfs.NewFS(embedHtml)
3738

3839
type ActionLink struct {
39-
Name string
40-
Path string
40+
Name string
41+
Path string
42+
Permits []string
43+
Authorized bool
4144
}
4245

4346
// Action represents a single action that is exposed by the App. Actions
@@ -69,13 +72,16 @@ type Action struct {
6972
esmLibs []types.JSLibrary
7073
appPathDomain types.AppPathDomain
7174
serverConfig *types.ServerConfig
75+
permit []string
76+
authorizer types.AuthorizerFunc // can be null
7277
}
7378

7479
// NewAction creates a new action
7580
func NewAction(logger *types.Logger, sourceFS *appfs.SourceFs, isDev bool, name, description, apath string, run, suggest starlark.Callable,
7681
params []apptype.AppParam, paramValuesStr map[string]string, paramDict starlark.StringDict,
7782
appPath string, styleType types.StyleType, containerProxyUrl string, hidden []string, showValidate bool,
78-
auditInsert func(*types.AuditEvent) error, containerManager any, jsLibs []types.JSLibrary, appPathDomain types.AppPathDomain, serverConfig *types.ServerConfig) (*Action, error) {
83+
auditInsert func(*types.AuditEvent) error, containerManager any, jsLibs []types.JSLibrary, appPathDomain types.AppPathDomain,
84+
serverConfig *types.ServerConfig, permit []string, authorizer types.AuthorizerFunc) (*Action, error) {
7985

8086
funcMap := system.GetFuncMap()
8187

@@ -151,13 +157,16 @@ func NewAction(logger *types.Logger, sourceFS *appfs.SourceFs, isDev bool, name,
151157
// Links, AppTemplate and Theme names are initialized later
152158
appPathDomain: appPathDomain,
153159
serverConfig: serverConfig,
160+
permit: permit,
161+
authorizer: authorizer,
154162
}, nil
155163
}
156164

157165
func (a *Action) GetLink() ActionLink {
158166
return ActionLink{
159-
Name: a.name,
160-
Path: a.pagePath,
167+
Name: a.name,
168+
Path: a.pagePath,
169+
Permits: a.permit,
161170
}
162171
}
163172

@@ -203,7 +212,27 @@ func (a *Action) validateAction(w http.ResponseWriter, r *http.Request) {
203212
a.execAction(w, r, false, true, "validate")
204213
}
205214

215+
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)
218+
if err != nil {
219+
http.Error(w, err.Error(), http.StatusInternalServerError)
220+
return false
221+
}
222+
if !authorized {
223+
userId := system.GetContextUserId(r.Context())
224+
http.Error(w, fmt.Sprintf("Unauthorized : %s does not have access to action %s", userId, a.name), http.StatusUnauthorized)
225+
return false
226+
}
227+
}
228+
return true
229+
}
230+
206231
func (a *Action) execAction(w http.ResponseWriter, r *http.Request, isSuggest, isValidate bool, op string) {
232+
if !a.authorizeAction(w, r) {
233+
return
234+
}
235+
207236
if isSuggest && a.suggest == nil {
208237
http.Error(w, "suggest not supported for this action", http.StatusNotImplemented)
209238
return
@@ -406,7 +435,7 @@ func (a *Action) execAction(w http.ResponseWriter, r *http.Request, isSuggest, i
406435
}
407436

408437
if isSuggest {
409-
a.handleSuggestResponse(w, qsParams.Encode(), ret)
438+
a.handleSuggestResponse(r.Context(), w, qsParams.Encode(), ret)
410439
return
411440
}
412441

@@ -487,7 +516,7 @@ func (a *Action) execAction(w http.ResponseWriter, r *http.Request, isSuggest, i
487516
}
488517

489518
if len(a.Links) > 1 {
490-
linksWithQS := a.getLinksWithQS(qsParams.Encode())
519+
linksWithQS := a.getLinksWithQS(r.Context(), qsParams.Encode())
491520
input := map[string]any{"links": linksWithQS}
492521
err = a.actionTemplate.ExecuteTemplate(w, "dropdown", input)
493522
if err != nil {
@@ -730,6 +759,10 @@ const (
730759
)
731760

732761
func (a *Action) getForm(w http.ResponseWriter, r *http.Request) {
762+
if !a.authorizeAction(w, r) {
763+
return
764+
}
765+
733766
queryParams := r.URL.Query()
734767
params := make([]ParamDef, 0, len(a.params))
735768

@@ -810,7 +843,7 @@ func (a *Action) getForm(w http.ResponseWriter, r *http.Request) {
810843
params = append(params, param)
811844
}
812845

813-
linksWithQS := a.getLinksWithQS(r.URL.RawQuery)
846+
linksWithQS := a.getLinksWithQS(r.Context(), r.URL.RawQuery)
814847
input := map[string]any{
815848
"dev": a.isDev,
816849
"name": a.name,
@@ -833,20 +866,29 @@ func (a *Action) getForm(w http.ResponseWriter, r *http.Request) {
833866
}
834867
}
835868

836-
func (a *Action) getLinksWithQS(qs string) []ActionLink {
869+
func (a *Action) getLinksWithQS(ctx context.Context, qs string) []ActionLink {
837870
linksWithQS := make([]ActionLink, 0, len(a.Links))
838871
for _, link := range a.Links {
872+
authorized := true
873+
if a.authorizer != nil && len(link.Permits) > 0 {
874+
var err error
875+
authorized, err = a.authorizer(ctx, link.Permits)
876+
if err != nil {
877+
a.Error().Msgf("error authorizing link %s: %s", link.Name, err)
878+
}
879+
}
839880
if link.Path != a.pagePath { // Don't add self link
840881
if qs != "" {
841882
link.Path = link.Path + "?" + qs
842883
}
884+
link.Authorized = authorized // whether this user has access to this action
843885
linksWithQS = append(linksWithQS, link)
844886
}
845887
}
846888
return linksWithQS
847889
}
848890

849-
func (a *Action) handleSuggestResponse(w http.ResponseWriter, paramQS string, retVal starlark.Value) {
891+
func (a *Action) handleSuggestResponse(ctx context.Context, w http.ResponseWriter, paramQS string, retVal starlark.Value) {
850892
ret, err := starlark_type.UnmarshalStarlark(retVal)
851893
if err != nil {
852894
http.Error(w, fmt.Sprintf("error unmarshalling suggest response: %s", err), http.StatusInternalServerError)
@@ -865,7 +907,7 @@ func (a *Action) handleSuggestResponse(w http.ResponseWriter, paramQS string, re
865907
}
866908

867909
if len(a.Links) > 1 {
868-
linksWithQS := a.getLinksWithQS(paramQS)
910+
linksWithQS := a.getLinksWithQS(ctx, paramQS)
869911
input := map[string]any{"links": linksWithQS}
870912
err = a.actionTemplate.ExecuteTemplate(w, "dropdown", input)
871913
if err != nil {

internal/app/action/form.go.html

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,13 @@
6464
tabindex="0"
6565
class="dropdown-content menu bg-base-100 rounded-box z-[1] w-52 p-2 shadow">
6666
{{ range .links }}
67-
<li><a href="{{ .Path }}">{{ .Name }}</a></li>
67+
{{ if .Authorized }}
68+
<li><a class="link" href="{{ .Path }}">{{ .Name }}</a></li>
69+
{{ else }}
70+
<li class="disabled menu-disabled">
71+
<a aria-disabled="true">{{ .Name }}</a>
72+
</li>
73+
{{ end }}
6874
{{ end }}
6975
</ul>
7076
{{ end }}

internal/app/app.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,8 @@ type App struct {
8585
lastRequestTime atomic.Int64
8686
secretEvalFunc func([][]string, string, string) (string, error)
8787
auditInsert func(*types.AuditEvent) error
88-
AppRunPath string // path to the app run directory
88+
AppRunPath string // path to the app run directory
89+
authorizer types.AuthorizerFunc // the authorizer function to use, can be null
8990
}
9091

9192
type starlarkCacheEntry struct {
@@ -102,7 +103,7 @@ func NewApp(sourceFS *appfs.SourceFs, workFS *appfs.WorkFs, logger *types.Logger
102103
appEntry *types.AppEntry, systemConfig *types.SystemConfig,
103104
plugins map[string]types.PluginSettings, appConfig types.AppConfig, notifyClose chan<- types.AppPathDomain,
104105
secretEvalFunc func([][]string, string, string) (string, error),
105-
auditInsert func(*types.AuditEvent) error, serverConfig *types.ServerConfig) (*App, error) {
106+
auditInsert func(*types.AuditEvent) error, serverConfig *types.ServerConfig, authorizer types.AuthorizerFunc) (*App, error) {
106107
newApp := &App{
107108
sourceFS: sourceFS,
108109
Logger: logger,
@@ -114,6 +115,7 @@ func NewApp(sourceFS *appfs.SourceFs, workFS *appfs.WorkFs, logger *types.Logger
114115
appStyle: &dev.AppStyle{},
115116
auditInsert: auditInsert,
116117
serverConfig: serverConfig,
118+
authorizer: authorizer,
117119
}
118120
newApp.plugins = NewAppPlugins(newApp, plugins, appEntry.Metadata.Accounts)
119121
newApp.AppConfig = appConfig

internal/app/apptype/builtins.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -310,17 +310,20 @@ func createLibraryBuiltin(_ *starlark.Thread, _ *starlark.Builtin, args starlark
310310
func createActionBuiltin(_ *starlark.Thread, _ *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
311311
var name, desc, path starlark.String
312312
var suggest, executor starlark.Callable
313-
var hidden *starlark.List
313+
var hidden, permit *starlark.List
314314
var showValidate starlark.Bool
315315
if err := starlark.UnpackArgs(ACTION, args, kwargs, "name", &name, "path", &path,
316316
"run", &executor, "suggest?", &suggest, "description?", &desc, "hidden?", &hidden,
317-
"show_validate?", &showValidate); err != nil {
317+
"show_validate?", &showValidate, "permit?", &permit); err != nil {
318318
return nil, fmt.Errorf("error unpacking action args: %w", err)
319319
}
320320

321321
if hidden == nil {
322322
hidden = starlark.NewList([]starlark.Value{})
323323
}
324+
if permit == nil {
325+
permit = starlark.NewList([]starlark.Value{})
326+
}
324327

325328
fields := starlark.StringDict{
326329
"name": name,
@@ -329,6 +332,7 @@ func createActionBuiltin(_ *starlark.Thread, _ *starlark.Builtin, args starlark.
329332
"run": executor,
330333
"hidden": hidden,
331334
"show_validate": showValidate,
335+
"permit": permit,
332336
}
333337

334338
if suggest != nil {

internal/app/setup.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,7 @@ func (a *App) addAction(count int, val starlark.Value, router *chi.Mux) (err err
521521

522522
var name, path, description string
523523
var run, suggest starlark.Callable
524-
var hidden []string
524+
var hidden, permit []string
525525
var showValidate bool
526526
if name, err = apptype.GetStringAttr(actionDef, "name"); err != nil {
527527
return err
@@ -538,6 +538,11 @@ func (a *App) addAction(count int, val starlark.Value, router *chi.Mux) (err err
538538
if hidden, err = apptype.GetListStringAttr(actionDef, "hidden", true); err != nil {
539539
return err
540540
}
541+
542+
if permit, err = apptype.GetListStringAttr(actionDef, "permit", true); err != nil {
543+
return err
544+
}
545+
541546
if showValidate, err = apptype.GetBoolAttr(actionDef, "show_validate"); err != nil {
542547
return err
543548
}
@@ -557,7 +562,7 @@ func (a *App) addAction(count int, val starlark.Value, router *chi.Mux) (err err
557562
}
558563
action, err := action.NewAction(a.Logger, a.sourceFS, a.IsDev, name, description, path, run, suggest,
559564
slices.Collect(maps.Values(a.paramInfo)), a.paramValuesStr, a.paramDict, a.Path, a.appStyle.GetStyleType(),
560-
containerProxyUrl, hidden, showValidate, a.auditInsert, a.containerManager, a.jsLibs, a.AppPathDomain(), a.serverConfig)
565+
containerProxyUrl, hidden, showValidate, a.auditInsert, a.containerManager, a.jsLibs, a.AppPathDomain(), a.serverConfig, permit, a.authorizer)
561566
if err != nil {
562567
return fmt.Errorf("error creating action %s: %w", name, err)
563568
}

internal/app/tests/app_test_helper.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ func CreateTestAppInt(logger *types.Logger, path string, fileData map[string]str
107107
workFS := appfs.NewWorkFs("", &TestWriteFS{TestReadFS: &TestReadFS{fileData: map[string]string{}}})
108108
a, err := app.NewApp(sourceFS, workFS, logger,
109109
createTestAppEntry(id, path, isDev, metadata), &systemConfig, pluginConfig, *appConfig,
110-
nil, secretManager.AppEvalTemplate, nil, &types.ServerConfig{})
110+
nil, secretManager.AppEvalTemplate, nil, &types.ServerConfig{}, nil)
111111
if err != nil {
112112
return nil, nil, err
113113
}

internal/app/tests/appaction_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -939,21 +939,21 @@ app = ace.app("testApp",
939939

940940
testutil.AssertEqualsInt(t, "code", 200, response.Code)
941941
body := response.Body.String()
942-
if strings.Contains(body, `<li><a href="/test/test1">test1Action</a></li>`) {
942+
if strings.Contains(body, `<li><a class="link" href="/test/test1">test1Action</a></li>`) {
943943
t.Errorf("actions switcher should not have current action, got %s", body)
944944
}
945-
testutil.AssertStringContains(t, body, `<li><a href="/test/test2">test2Action</a></li>`)
945+
testutil.AssertStringContains(t, body, `<li><a class="link" href="/test/test2">test2Action</a></li>`)
946946

947947
request = httptest.NewRequest("GET", "/test/test2?param1=abc", nil)
948948
response = httptest.NewRecorder()
949949
a.ServeHTTP(response, request)
950950

951951
testutil.AssertEqualsInt(t, "code", 200, response.Code)
952952
body = response.Body.String()
953-
if strings.Contains(body, `<li><a href="/test/test2">test2Action</a></li>`) {
953+
if strings.Contains(body, `<li><a class="link" href="/test/test2">test2Action</a></li>`) {
954954
t.Errorf("actions switcher should not have current action, got %s", body)
955955
}
956-
testutil.AssertStringContains(t, body, `<li><a href="/test/test1?param1=abc">test1Action</a></li>`)
956+
testutil.AssertStringContains(t, body, `<li><a class="link" href="/test/test1?param1=abc">test1Action</a></li>`)
957957

958958
values := url.Values{
959959
"param1": {"p1val"},
@@ -977,7 +977,7 @@ app = ace.app("testApp",
977977
tabindex="0"
978978
class="dropdown-content menu bg-base-100 rounded-box z-[1] w-52 p-2 shadow">
979979
980-
<li><a href="/test/test2?param1=p1val">test2Action</a></li>
980+
<li><a class="link" href="/test/test2?param1=p1val">test2Action</a></li>
981981
982982
</ul>
983983
<div

internal/server/app_apis.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ func (s *Server) setupApp(appEntry *types.AppEntry, tx types.Transaction) (*app.
355355
})
356356
return app.NewApp(sourceFS, workFS, &appLogger, appEntry, &s.config.System,
357357
s.config.Plugins, s.config.AppConfig, s.notifyClose, s.secretsManager.AppEvalTemplate,
358-
s.InsertAuditEvent, s.config)
358+
s.InsertAuditEvent, s.config, s.AuthorizeAny)
359359
}
360360

361361
func (s *Server) GetAppApi(ctx context.Context, appPath string) (*types.AppGetResponse, error) {
@@ -513,7 +513,7 @@ func (s *Server) authenticateAndServeApp(w http.ResponseWriter, r *http.Request,
513513
}
514514

515515
s.Trace().Msgf("Authenticated user %s, doing authorization check", userId)
516-
authorized, err := s.rbacManager.Authorize(userId, app.AppEntry.AppPathDomain(), string(appAuth), types.PermissionAccess, groups)
516+
authorized, err := s.rbacManager.Authorize(userId, app.AppEntry.AppPathDomain(), string(appAuth), types.PermissionAccess, groups, false)
517517
if err != nil {
518518
http.Error(w, err.Error(), http.StatusInternalServerError)
519519
return
@@ -527,6 +527,8 @@ func (s *Server) authenticateAndServeApp(w http.ResponseWriter, r *http.Request,
527527
// Create a new context with the user ID
528528
ctx := context.WithValue(r.Context(), types.USER_ID, userId)
529529
ctx = context.WithValue(ctx, types.APP_ID, string(app.Id))
530+
ctx = context.WithValue(ctx, types.APP_PATH_DOMAIN, app.AppEntry.AppPathDomain())
531+
ctx = context.WithValue(ctx, types.APP_AUTH, appAuth)
530532
ctx = context.WithValue(ctx, types.GROUPS, groups)
531533

532534
contextShared := ctx.Value(types.SHARED)

internal/server/rbac.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func NewRBACHandler(logger *types.Logger, rbacConfig *types.RBACConfig, serverCo
4141
}
4242

4343
func (h *RBACManager) Authorize(user string, appPathDomain types.AppPathDomain,
44-
appAuthSetting string, permission types.RBACPermission, groups []string) (bool, error) {
44+
appAuthSetting string, permission types.RBACPermission, groups []string, isAppLevelPermission bool) (bool, error) {
4545
h.mu.RLock()
4646
defer h.mu.RUnlock()
4747

@@ -55,7 +55,7 @@ func (h *RBACManager) Authorize(user string, appPathDomain types.AppPathDomain,
5555
return true, nil
5656
}
5757

58-
if !strings.HasPrefix(appAuthSetting, RBAC_AUTH_PREFIX) && permission == types.PermissionAccess {
58+
if !strings.HasPrefix(appAuthSetting, RBAC_AUTH_PREFIX) && (permission == types.PermissionAccess || isAppLevelPermission) {
5959
// if app auth does not have rbac enabled, authorize access for Access permission
6060
// If authenticated, then app access is allowed
6161
return true, nil

0 commit comments

Comments
 (0)