chore(chaos-center): Updated MyHub Rbacs and optimized filetype for fetching hub data #3444
Conversation
Signed-off-by: Amit Kumar Das <[email protected]>
| func (r *mutationResolver) SyncHub(ctx context.Context, id string, projectID string) ([]*model.MyHubStatus, error) { | ||
| err := authorization.ValidateRole(ctx, projectID, | ||
| authorization.MutationRbacRules[authorization.UpdateChaosWorkflow], | ||
| model.InvitationAccepted.String()) | ||
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
Will not work creates attack vector. I can send a random project id where I have enough permission and an unrelated hub id where I don't have enough permission, this code will allow me to make changes to the hub even though I shouldn't have access to that. The project id should be derived from the hub id
There was a problem hiding this comment.
A simple solution would be to query the hub with the hub id + project id, this would mean if the user sends unrelated hub and project ids db will not return the hub data.
| func (r *mutationResolver) DeleteMyHub(ctx context.Context, hubID string, projectID string) (bool, error) { | ||
| err := authorization.ValidateRole(ctx, projectID, | ||
| authorization.MutationRbacRules[authorization.DeleteMyHub], | ||
| model.InvitationAccepted.String()) | ||
| if err != nil { | ||
| return false, err | ||
| } |
| err := authorization.ValidateRole(ctx, experimentInput.ProjectID, | ||
| authorization.MutationRbacRules[authorization.GetYAMLData], | ||
| model.InvitationAccepted.String()) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| return myhub.GetYAMLData(ctx, experimentInput) | ||
| } | ||
|
|
||
| func (r *queryResolver) GetPredefinedWorkflowList(ctx context.Context, hubName string, projectID string) ([]string, error) { | ||
| err := authorization.ValidateRole(ctx, projectID, | ||
| authorization.MutationRbacRules[authorization.PredefinedWorkflowOperations], | ||
| model.InvitationAccepted.String()) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return myhub.GetPredefinedWorkflowList(hubName, projectID) | ||
| } | ||
|
|
||
| func (r *queryResolver) GetPredefinedExperimentYaml(ctx context.Context, experimentInput model.ExperimentInput) (string, error) { | ||
| err := authorization.ValidateRole(ctx, experimentInput.ProjectID, | ||
| authorization.MutationRbacRules[authorization.PredefinedWorkflowOperations], | ||
| model.InvitationAccepted.String()) | ||
| if err != nil { | ||
| return "", err | ||
| } |
There was a problem hiding this comment.
Make sure the ProjectIDs passed here are actually used to get the required data else, if they are just redundant information the same attack vector exists here too.
There was a problem hiding this comment.
@gdsoumya For the above 2 cases, the Project ID is being used to fetch the chaos-charts data from the hub directories. I believe it wont cause any issue here. Have updated the code for other cases.
Signed-off-by: Amit Kumar Das <[email protected]>
|
Thanks @gdsoumya for the review 🚀 |
Signed-off-by: Amit Kumar Das [email protected]
Proposed changes
This PR fixes the following issues:
enumref: litmuschaos/litmus-e2e#362
Types of changes
What types of changes does your code introduce to Litmus? Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Dependency
Special notes for your reviewer: