Skip to content

Commit 2918f46

Browse files
authored
make sure only system and org admins can assign/delete roles from users (#2521)
1 parent dce108f commit 2918f46

2 files changed

Lines changed: 66 additions & 15 deletions

File tree

internal/httpserve/handlers/roles.go

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -130,20 +130,7 @@ func (h *Handler) handleRoleMutation(ctx echo.Context, openapi *OpenAPIContext,
130130
return h.BadRequest(ctx, ErrInvalidInput, openapi)
131131
}
132132

133-
allowed, err := h.DBClient.Authz.CheckAccess(reqCtx, fgax.AccessCheck{
134-
SubjectType: caller.SubjectType(),
135-
SubjectID: caller.SubjectID,
136-
Relation: fgax.CanEdit,
137-
ObjectID: orgID,
138-
ObjectType: fgax.Kind(generated.TypeOrganization),
139-
Context: utils.NewOrganizationContextKey(caller.SubjectEmail),
140-
})
141-
if err != nil {
142-
logx.FromContext(reqCtx).Error().Err(err).Str("organization_id", orgID).Msg("error checking organization role management access")
143-
return h.InternalServerError(ctx, ErrProcessingRequest, openapi)
144-
}
145-
146-
if !allowed {
133+
if !isCallerAdmin(caller) {
147134
return h.BadRequest(ctx, ErrInvalidInput, openapi)
148135
}
149136

@@ -167,6 +154,19 @@ func (h *Handler) handleRoleMutation(ctx echo.Context, openapi *OpenAPIContext,
167154
}, openapi)
168155
}
169156

157+
func isCallerAdmin(caller *auth.Caller) bool {
158+
if caller.HasInLineage(auth.CapSystemAdmin) {
159+
return true
160+
}
161+
162+
switch caller.OrganizationRole {
163+
case auth.OwnerRole, auth.SuperAdminRole, auth.AdminRole:
164+
return true
165+
}
166+
167+
return false
168+
}
169+
170170
func convertOrgRolesToOpenAPI(roles []modelparse.OrganizationRole) []models.OrganizationRole {
171171
resp := make([]models.OrganizationRole, 0, len(roles))
172172
for _, role := range roles {

internal/httpserve/handlers/roles_test.go

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package handlers_test
22

33
import (
4+
"context"
45
"encoding/json"
56
"net/http"
67
"net/http/httptest"
@@ -10,8 +11,10 @@ import (
1011
"github.com/stretchr/testify/assert"
1112
"github.com/stretchr/testify/require"
1213
"github.com/theopenlane/httpsling"
14+
"github.com/theopenlane/iam/auth"
1315
"github.com/theopenlane/iam/fgax"
1416

17+
"github.com/theopenlane/core/common/enums"
1518
models "github.com/theopenlane/core/common/openapi"
1619
"github.com/theopenlane/core/internal/ent/generated"
1720
"github.com/theopenlane/core/internal/ent/generated/privacy"
@@ -57,17 +60,35 @@ func (suite *HandlerTestSuite) TestOrganizationRolesAssignmentHandler() {
5760
suite.registerRouteOnce("DELETE", "account/organization-roles", operation, suite.h.DeleteOrganizationRolesHandler)
5861

5962
ctx := privacy.DecisionContext(testUser1.UserCtx, privacy.Allow)
63+
ownerCtx := auth.NewTestContextWithOrgID(testUser1.ID, testUser1.OrganizationID, auth.WithOrganizationRole(auth.OwnerRole))
64+
ownerCtx = privacy.DecisionContext(ownerCtx, privacy.Allow)
65+
ownerCtx = generated.NewContext(ownerCtx, suite.db)
66+
6067
group, err := suite.db.Group.Create().
6168
SetName("Role Test Group " + testUser2.ID).
6269
SetDescription("Group for organization role assignment tests").
6370
SetOwnerID(testUser1.OrganizationID).
6471
Save(ctx)
6572
require.NoError(t, err)
6673

74+
member := suite.userBuilder(ctx)
75+
memberRole := enums.RoleMember
76+
err = suite.db.OrgMembership.Create().SetInput(generated.CreateOrgMembershipInput{
77+
UserID: member.ID,
78+
OrganizationID: testUser1.OrganizationID,
79+
Role: &memberRole,
80+
}).Exec(ctx)
81+
require.NoError(t, err)
82+
83+
memberCtx := auth.NewTestContextWithOrgID(member.ID, testUser1.OrganizationID, auth.WithOrganizationRole(auth.MemberRole))
84+
memberCtx = privacy.DecisionContext(memberCtx, privacy.Allow)
85+
memberCtx = generated.NewContext(memberCtx, suite.db)
86+
6787
cases := []struct {
6888
name string
6989
method string
7090
request models.OrganizationRolesRequest
91+
ctx context.Context
7192
statusCode int
7293
success bool
7394
}{
@@ -79,6 +100,7 @@ func (suite *HandlerTestSuite) TestOrganizationRolesAssignmentHandler() {
79100
Role: "policy_manager",
80101
UserIDs: []string{testUser2.ID},
81102
},
103+
ctx: ownerCtx,
82104
statusCode: http.StatusOK,
83105
success: true,
84106
},
@@ -90,6 +112,7 @@ func (suite *HandlerTestSuite) TestOrganizationRolesAssignmentHandler() {
90112
Role: "policy_manager",
91113
UserIDs: []string{testUser2.ID},
92114
},
115+
ctx: ownerCtx,
93116
statusCode: http.StatusOK,
94117
success: true,
95118
},
@@ -101,6 +124,7 @@ func (suite *HandlerTestSuite) TestOrganizationRolesAssignmentHandler() {
101124
Role: "risk_manager",
102125
GroupIDs: []string{group.ID},
103126
},
127+
ctx: ownerCtx,
104128
statusCode: http.StatusOK,
105129
success: true,
106130
},
@@ -112,6 +136,7 @@ func (suite *HandlerTestSuite) TestOrganizationRolesAssignmentHandler() {
112136
Role: "risk_manager",
113137
GroupIDs: []string{group.ID},
114138
},
139+
ctx: ownerCtx,
115140
statusCode: http.StatusOK,
116141
success: true,
117142
},
@@ -123,6 +148,7 @@ func (suite *HandlerTestSuite) TestOrganizationRolesAssignmentHandler() {
123148
Role: "policy_manager",
124149
UserIDs: []string{testUser2.ID},
125150
},
151+
ctx: ownerCtx,
126152
statusCode: http.StatusOK,
127153
success: true,
128154
},
@@ -134,6 +160,7 @@ func (suite *HandlerTestSuite) TestOrganizationRolesAssignmentHandler() {
134160
Role: "not_a_role",
135161
UserIDs: []string{testUser2.ID},
136162
},
163+
ctx: ownerCtx,
137164
statusCode: http.StatusBadRequest,
138165
},
139166
{
@@ -143,6 +170,7 @@ func (suite *HandlerTestSuite) TestOrganizationRolesAssignmentHandler() {
143170
OrganizationID: testUser1.OrganizationID,
144171
Role: "policy_manager",
145172
},
173+
ctx: ownerCtx,
146174
statusCode: http.StatusBadRequest,
147175
},
148176
{
@@ -153,6 +181,29 @@ func (suite *HandlerTestSuite) TestOrganizationRolesAssignmentHandler() {
153181
Role: "policy_manager",
154182
UserIDs: []string{testUser2.ID},
155183
},
184+
ctx: ownerCtx,
185+
statusCode: http.StatusBadRequest,
186+
},
187+
{
188+
name: "member cannot assign role to a user",
189+
method: http.MethodPost,
190+
request: models.OrganizationRolesRequest{
191+
OrganizationID: testUser1.OrganizationID,
192+
Role: "policy_manager",
193+
UserIDs: []string{testUser2.ID},
194+
},
195+
ctx: memberCtx,
196+
statusCode: http.StatusBadRequest,
197+
},
198+
{
199+
name: "member cannot assign role to a group",
200+
method: http.MethodPost,
201+
request: models.OrganizationRolesRequest{
202+
OrganizationID: testUser1.OrganizationID,
203+
Role: "risk_manager",
204+
GroupIDs: []string{group.ID},
205+
},
206+
ctx: memberCtx,
156207
statusCode: http.StatusBadRequest,
157208
},
158209
}
@@ -167,7 +218,7 @@ func (suite *HandlerTestSuite) TestOrganizationRolesAssignmentHandler() {
167218

168219
recorder := httptest.NewRecorder()
169220

170-
suite.e.ServeHTTP(recorder, req.WithContext(testUser1.UserCtx))
221+
suite.e.ServeHTTP(recorder, req.WithContext(tc.ctx))
171222

172223
response := recorder.Result()
173224
defer response.Body.Close()

0 commit comments

Comments
 (0)