Skip to content

Commit 9c76193

Browse files
akclaceakclace
committed
Added auto group detection for SSO login using OIDC
1 parent da50ff6 commit 9c76193

File tree

8 files changed

+128
-54
lines changed

8 files changed

+128
-54
lines changed

‎internal/server/app_apis.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,7 @@ func (s *Server) authenticateAndServeApp(w http.ResponseWriter, r *http.Request,
469469
// Remove the RBAC_AUTH_PREFIX rbac: prefix
470470
strippedAuthStr := strings.TrimPrefix(string(appAuth), RBAC_AUTH_PREFIX)
471471
strippedAuth := types.AppAuthnType(strippedAuthStr)
472+
groups := make([]string, 0)
472473

473474
if strippedAuth == types.AppAuthnNone {
474475
// No authentication required
@@ -502,7 +503,7 @@ func (s *Server) authenticateAndServeApp(w http.ResponseWriter, r *http.Request,
502503
}
503504

504505
// Redirect to the auth provider if not logged in
505-
userId, err = s.ssoAuth.CheckAuth(w, r, strippedAuthStr, true)
506+
userId, groups, err = s.ssoAuth.CheckAuth(w, r, strippedAuthStr, true)
506507
if err != nil {
507508
http.Error(w, err.Error(), http.StatusInternalServerError)
508509
}
@@ -512,7 +513,7 @@ func (s *Server) authenticateAndServeApp(w http.ResponseWriter, r *http.Request,
512513
}
513514

514515
s.Trace().Msgf("Authenticated user %s, doing authorization check", userId)
515-
authorized, err := s.rbacManager.Authorize(userId, app.AppEntry.AppPathDomain(), string(appAuth), types.PermissionAccess)
516+
authorized, err := s.rbacManager.Authorize(userId, app.AppEntry.AppPathDomain(), string(appAuth), types.PermissionAccess, groups)
516517
if err != nil {
517518
http.Error(w, err.Error(), http.StatusInternalServerError)
518519
return
@@ -526,6 +527,7 @@ func (s *Server) authenticateAndServeApp(w http.ResponseWriter, r *http.Request,
526527
// Create a new context with the user ID
527528
ctx := context.WithValue(r.Context(), types.USER_ID, userId)
528529
ctx = context.WithValue(ctx, types.APP_ID, string(app.Id))
530+
ctx = context.WithValue(ctx, types.GROUPS, groups)
529531

530532
contextShared := ctx.Value(types.SHARED)
531533
if contextShared != nil {
@@ -917,9 +919,10 @@ func (s *Server) GetApps(ctx context.Context, appPathGlob string, internal bool)
917919
}
918920

919921
userId := system.GetContextUserId(ctx)
922+
groups := system.GetContextGroups(ctx)
920923
ret := make([]types.AppResponse, 0, len(filteredApps))
921924
for _, app := range filteredApps {
922-
authorized, err := s.AuthorizeList(userId, &app)
925+
authorized, err := s.AuthorizeList(userId, &app, groups)
923926
if err != nil {
924927
return nil, types.CreateRequestError(err.Error(), http.StatusInternalServerError)
925928
}

‎internal/server/openrun_plugin.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ func (c *openrunPlugin) listAppsImpl(thread *starlark.Thread, _ *starlark.Builti
7676
}
7777

7878
userId := system.GetRequestUserId(thread)
79+
groups := system.GetRequestGroups(thread)
7980
ret := starlark.List{}
8081
for _, app := range apps {
8182
// Filter out internal apps
@@ -116,7 +117,7 @@ func (c *openrunPlugin) listAppsImpl(thread *starlark.Thread, _ *starlark.Builti
116117
}
117118

118119
if permCheck {
119-
hasAccess, err := c.server.AuthorizeList(userId, &app)
120+
hasAccess, err := c.server.AuthorizeList(userId, &app, groups)
120121
if err != nil {
121122
return nil, err
122123
}

‎internal/server/rbac.go‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ func NewRBACHandler(logger *types.Logger, rbacConfig *types.RBACConfig, serverCo
4040
return rbacManager, nil
4141
}
4242

43-
func (h *RBACManager) Authorize(user string, appPathDomain types.AppPathDomain, appAuthSetting string, permission types.RBACPermission) (bool, error) {
43+
func (h *RBACManager) Authorize(user string, appPathDomain types.AppPathDomain,
44+
appAuthSetting string, permission types.RBACPermission, groups []string) (bool, error) {
4445
h.mu.RLock()
4546
defer h.mu.RUnlock()
4647

@@ -64,12 +65,13 @@ func (h *RBACManager) Authorize(user string, appPathDomain types.AppPathDomain,
6465
appPathDomain.Path = strings.TrimSuffix(appPathDomain.Path, types.STAGE_SUFFIX)
6566
appPathDomain.Path = strings.TrimSuffix(appPathDomain.Path, types.PREVIEW_SUFFIX)
6667

67-
return h.checkGrants(user, appPathDomain, permission)
68+
return h.checkGrants(user, appPathDomain, permission, groups)
6869
}
6970

70-
func (h *RBACManager) checkGrants(inputUser string, appPathDomain types.AppPathDomain, inputPermission types.RBACPermission) (bool, error) {
71+
func (h *RBACManager) checkGrants(inputUser string, appPathDomain types.AppPathDomain,
72+
inputPermission types.RBACPermission, groups []string) (bool, error) {
7173
for _, grant := range h.rbacConfig.Grants {
72-
match, err := h.checkGrant(grant, inputUser, appPathDomain, inputPermission)
74+
match, err := h.checkGrant(grant, inputUser, appPathDomain, inputPermission, groups)
7375
if err != nil {
7476
return false, err
7577
}
@@ -84,12 +86,19 @@ func (h *RBACManager) checkGrants(inputUser string, appPathDomain types.AppPathD
8486
return false, nil
8587
}
8688

87-
func (h *RBACManager) checkGrant(grant types.RBACGrant, inputUser string, appPathDomain types.AppPathDomain, inputPermission types.RBACPermission) (bool, error) {
89+
func (h *RBACManager) checkGrant(grant types.RBACGrant, inputUser string, appPathDomain types.AppPathDomain,
90+
inputPermission types.RBACPermission, groups []string) (bool, error) {
8891
userMatched := false
8992
for _, user := range grant.Users {
9093
if strings.HasPrefix(user, RBAC_GROUP_PREFIX) {
9194
refGroupName := user[len(RBAC_GROUP_PREFIX):]
92-
if slices.Contains(h.groups[refGroupName], inputUser) {
95+
if slices.Contains(groups, refGroupName) {
96+
// granted group name matched group as found from SSO login
97+
userMatched = true
98+
break
99+
}
100+
refGroup, ok := h.groups[refGroupName]
101+
if ok && slices.Contains(refGroup, inputUser) {
93102
userMatched = true
94103
break
95104
}
@@ -247,16 +256,7 @@ func (h *RBACManager) validateGrants(rbacConfig *types.RBACConfig) error {
247256
}
248257

249258
for i, grant := range rbacConfig.Grants {
250-
// Validate group references in Users
251-
for _, user := range grant.Users {
252-
if strings.HasPrefix(user, RBAC_GROUP_PREFIX) {
253-
groupName := user[len(RBAC_GROUP_PREFIX):]
254-
if _, exists := rbacConfig.Groups[groupName]; !exists {
255-
return fmt.Errorf("grant %d ('%s'): Users references undefined group '%s'", i, grant.Description, groupName)
256-
}
257-
}
258-
}
259-
259+
// groups can be passed dynamically (for SSO login), so we don't need to validate them
260260
// Validate role references in Roles
261261
for _, role := range grant.Roles {
262262
if _, exists := rbacConfig.Roles[role]; !exists {

‎internal/server/rbac_test.go‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,7 @@ func TestAuthorizeAccess(t *testing.T) {
500500
t.Fatalf("failed to create RBACManager: %v", err)
501501
}
502502

503-
result, err := rbacManager.Authorize(tt.user, tt.appPathDomain, tt.appAuthSetting, tt.permission)
503+
result, err := rbacManager.Authorize(tt.user, tt.appPathDomain, tt.appAuthSetting, tt.permission, []string{})
504504

505505
if tt.expectError {
506506
if err == nil {
@@ -600,7 +600,7 @@ func TestAuthorizeAccessWithGroupHierarchy(t *testing.T) {
600600
t.Fatalf("failed to create RBACManager: %v", err)
601601
}
602602

603-
result, err := rbacManager.Authorize(tt.user, tt.appPathDomain, "rbac:test", tt.permission)
603+
result, err := rbacManager.Authorize(tt.user, tt.appPathDomain, "rbac:test", tt.permission, []string{})
604604
if err != nil {
605605
t.Errorf("unexpected error: %v", err)
606606
return
@@ -710,7 +710,7 @@ func TestAuthorizeAccessWithRoleHierarchy(t *testing.T) {
710710
t.Fatalf("failed to create RBACManager: %v", err)
711711
}
712712

713-
result, err := rbacManager.Authorize(tt.user, tt.appPathDomain, "rbac:test", tt.permission)
713+
result, err := rbacManager.Authorize(tt.user, tt.appPathDomain, "rbac:test", tt.permission, []string{})
714714
if err != nil {
715715
t.Errorf("unexpected error: %v", err)
716716
return
@@ -833,7 +833,7 @@ func TestUpdateRBACConfig(t *testing.T) {
833833
}
834834

835835
// Test that the update worked by checking authorization
836-
result, err := rbacManager.Authorize("user1", types.AppPathDomain{Path: "/new", Domain: ""}, "rbac:test", types.PermissionList)
836+
result, err := rbacManager.Authorize("user1", types.AppPathDomain{Path: "/new", Domain: ""}, "rbac:test", types.PermissionList, []string{})
837837
if err != nil {
838838
t.Errorf("unexpected error during authorization test: %v", err)
839839
return
@@ -884,7 +884,7 @@ func TestAuthorizeAccessConcurrency(t *testing.T) {
884884
go func(user string) {
885885
defer func() { done <- true }()
886886

887-
result, err := rbacManager.Authorize(user, types.AppPathDomain{Path: "/test", Domain: ""}, "rbac:test", types.PermissionList)
887+
result, err := rbacManager.Authorize(user, types.AppPathDomain{Path: "/test", Domain: ""}, "rbac:test", types.PermissionList, []string{})
888888
if err != nil {
889889
t.Errorf("unexpected error: %v", err)
890890
return
@@ -987,7 +987,7 @@ func TestValidateGrants(t *testing.T) {
987987
expectError: false,
988988
},
989989
{
990-
name: "invalid grant - undefined group reference",
990+
name: "valid grant - undefined group reference (no longer validated)",
991991
rbacConfig: &types.RBACConfig{
992992
Enabled: true,
993993
Groups: map[string][]string{
@@ -998,15 +998,14 @@ func TestValidateGrants(t *testing.T) {
998998
},
999999
Grants: []types.RBACGrant{
10001000
{
1001-
Description: "invalid grant",
1001+
Description: "valid grant with undefined group",
10021002
Users: []string{"group:nonexistent"},
10031003
Roles: []string{"read"},
10041004
Targets: []string{"/test"},
10051005
},
10061006
},
10071007
},
1008-
expectError: true,
1009-
errorMsg: "grant 0 ('invalid grant'): Users references undefined group 'nonexistent'",
1008+
expectError: false,
10101009
},
10111010
{
10121011
name: "invalid grant - undefined role reference",
@@ -1031,7 +1030,7 @@ func TestValidateGrants(t *testing.T) {
10311030
errorMsg: "grant 0 ('invalid grant'): Roles references undefined role 'nonexistent'",
10321031
},
10331032
{
1034-
name: "invalid grant - multiple undefined group references",
1033+
name: "valid grant - multiple undefined group references (no longer validated)",
10351034
rbacConfig: &types.RBACConfig{
10361035
Enabled: true,
10371036
Groups: map[string][]string{
@@ -1042,15 +1041,14 @@ func TestValidateGrants(t *testing.T) {
10421041
},
10431042
Grants: []types.RBACGrant{
10441043
{
1045-
Description: "invalid grant",
1044+
Description: "valid grant with multiple undefined groups",
10461045
Users: []string{"group:nonexistent1", "group:nonexistent2"},
10471046
Roles: []string{"read"},
10481047
Targets: []string{"/test"},
10491048
},
10501049
},
10511050
},
1052-
expectError: true,
1053-
errorMsg: "grant 0 ('invalid grant'): Users references undefined group 'nonexistent1'",
1051+
expectError: false,
10541052
},
10551053
{
10561054
name: "invalid grant - multiple undefined role references",
@@ -1075,7 +1073,7 @@ func TestValidateGrants(t *testing.T) {
10751073
errorMsg: "grant 0 ('invalid grant'): Roles references undefined role 'nonexistent1'",
10761074
},
10771075
{
1078-
name: "invalid grant - multiple grants with errors",
1076+
name: "valid grants - multiple grants with undefined group (no longer validated)",
10791077
rbacConfig: &types.RBACConfig{
10801078
Enabled: true,
10811079
Groups: map[string][]string{
@@ -1092,15 +1090,14 @@ func TestValidateGrants(t *testing.T) {
10921090
Targets: []string{"/test"},
10931091
},
10941092
{
1095-
Description: "invalid grant",
1093+
Description: "valid grant with undefined group",
10961094
Users: []string{"group:nonexistent"},
10971095
Roles: []string{"read"},
10981096
Targets: []string{"/test"},
10991097
},
11001098
},
11011099
},
1102-
expectError: true,
1103-
errorMsg: "grant 1 ('invalid grant'): Users references undefined group 'nonexistent'",
1100+
expectError: false,
11041101
},
11051102
{
11061103
name: "empty grants - should be valid",

‎internal/server/server.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -845,11 +845,11 @@ func (s *Server) ParseGlob(appGlob string) ([]types.AppInfo, error) {
845845
// AuthorizeList checks if the user has access to perform list operation on the specified app
846846
// For RBAC mode, uses RBAC permissions. For non-RBAC mode, look at whether app is using
847847
// same authentication types as used by the caller
848-
func (s *Server) AuthorizeList(userId string, app *types.AppInfo) (bool, error) {
848+
func (s *Server) AuthorizeList(userId string, app *types.AppInfo, groups []string) (bool, error) {
849849
appAuthStr := string(app.Auth)
850850
if s.rbacManager.rbacConfig.Enabled {
851851
// RBAC auth is enabled, verify access
852-
return s.rbacManager.Authorize(userId, app.AppPathDomain, appAuthStr, types.PermissionList)
852+
return s.rbacManager.Authorize(userId, app.AppPathDomain, appAuthStr, types.PermissionList, groups)
853853
}
854854

855855
if userId != "" && userId == types.ADMIN_USER {

0 commit comments

Comments
 (0)