mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2025-02-22 11:25:47 -05:00
fix(sec): web route update and delete runner variables
The web route to update and delete variables of runners did not check if the ID that was given belonged to the context it was requested in, this made it possible to update and delete every existing runner variable of a instance for any authenticated user. The code has been reworked to always take into account the context of the request (owner and repository ID). (cherry picked from commit 5cb8fdfc8b9213cc368cd074aac93a1327ea20b0)
This commit is contained in:
parent
0e82cf121d
commit
4c8c215b75
5 changed files with 31 additions and 25 deletions
|
@ -74,7 +74,7 @@ func FindVariables(ctx context.Context, opts FindVariablesOpts) ([]*ActionVariab
|
|||
}
|
||||
|
||||
func UpdateVariable(ctx context.Context, variable *ActionVariable) (bool, error) {
|
||||
count, err := db.GetEngine(ctx).ID(variable.ID).Cols("name", "data").
|
||||
count, err := db.GetEngine(ctx).ID(variable.ID).Where("owner_id = ? AND repo_id = ?", variable.OwnerID, variable.RepoID).Cols("name", "data").
|
||||
Update(&ActionVariable{
|
||||
Name: variable.Name,
|
||||
Data: variable.Data,
|
||||
|
@ -82,11 +82,9 @@ func UpdateVariable(ctx context.Context, variable *ActionVariable) (bool, error)
|
|||
return count != 0, err
|
||||
}
|
||||
|
||||
func DeleteVariable(ctx context.Context, id int64) error {
|
||||
if _, err := db.DeleteByID[ActionVariable](ctx, id); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
func DeleteVariable(ctx context.Context, variableID, ownerID, repoID int64) (bool, error) {
|
||||
count, err := db.GetEngine(ctx).Table("action_variable").Where("id = ? AND owner_id = ? AND repo_id = ?", variableID, ownerID, repoID).Delete()
|
||||
return count != 0, err
|
||||
}
|
||||
|
||||
func GetVariablesOfRun(ctx context.Context, run *ActionRun) (map[string]string, error) {
|
||||
|
|
|
@ -3743,6 +3743,7 @@ variables.deletion.description = Removing a variable is permanent and cannot be
|
|||
variables.description = Variables will be passed to certain actions and cannot be read otherwise.
|
||||
variables.id_not_exist = Variable with ID %d does not exist.
|
||||
variables.edit = Edit Variable
|
||||
variables.not_found = Failed to find the variable.
|
||||
variables.deletion.failed = Failed to remove variable.
|
||||
variables.deletion.success = The variable has been removed.
|
||||
variables.creation.failed = Failed to add variable.
|
||||
|
|
|
@ -127,7 +127,7 @@ func VariableUpdate(ctx *context.Context) {
|
|||
return
|
||||
}
|
||||
|
||||
shared.UpdateVariable(ctx, vCtx.RedirectLink)
|
||||
shared.UpdateVariable(ctx, vCtx.OwnerID, vCtx.RepoID, vCtx.RedirectLink)
|
||||
}
|
||||
|
||||
func VariableDelete(ctx *context.Context) {
|
||||
|
@ -136,5 +136,5 @@ func VariableDelete(ctx *context.Context) {
|
|||
ctx.ServerError("getVariablesCtx", err)
|
||||
return
|
||||
}
|
||||
shared.DeleteVariable(ctx, vCtx.RedirectLink)
|
||||
shared.DeleteVariable(ctx, vCtx.OwnerID, vCtx.RepoID, vCtx.RedirectLink)
|
||||
}
|
||||
|
|
|
@ -39,25 +39,33 @@ func CreateVariable(ctx *context.Context, ownerID, repoID int64, redirectURL str
|
|||
ctx.JSONRedirect(redirectURL)
|
||||
}
|
||||
|
||||
func UpdateVariable(ctx *context.Context, redirectURL string) {
|
||||
func UpdateVariable(ctx *context.Context, ownerID, repoID int64, redirectURL string) {
|
||||
id := ctx.ParamsInt64(":variable_id")
|
||||
form := web.GetForm(ctx).(*forms.EditVariableForm)
|
||||
|
||||
if ok, err := actions_service.UpdateVariable(ctx, id, form.Name, form.Data); err != nil || !ok {
|
||||
log.Error("UpdateVariable: %v", err)
|
||||
ctx.JSONError(ctx.Tr("actions.variables.update.failed"))
|
||||
if ok, err := actions_service.UpdateVariable(ctx, id, ownerID, repoID, form.Name, form.Data); err != nil || !ok {
|
||||
if !ok {
|
||||
ctx.JSONError(ctx.Tr("actions.variables.not_found"))
|
||||
} else {
|
||||
log.Error("UpdateVariable: %v", err)
|
||||
ctx.JSONError(ctx.Tr("actions.variables.update.failed"))
|
||||
}
|
||||
return
|
||||
}
|
||||
ctx.Flash.Success(ctx.Tr("actions.variables.update.success"))
|
||||
ctx.JSONRedirect(redirectURL)
|
||||
}
|
||||
|
||||
func DeleteVariable(ctx *context.Context, redirectURL string) {
|
||||
func DeleteVariable(ctx *context.Context, ownerID, repoID int64, redirectURL string) {
|
||||
id := ctx.ParamsInt64(":variable_id")
|
||||
|
||||
if err := actions_service.DeleteVariableByID(ctx, id); err != nil {
|
||||
log.Error("Delete variable [%d] failed: %v", id, err)
|
||||
ctx.JSONError(ctx.Tr("actions.variables.deletion.failed"))
|
||||
if ok, err := actions_model.DeleteVariable(ctx, id, ownerID, repoID); err != nil || !ok {
|
||||
if !ok {
|
||||
ctx.JSONError(ctx.Tr("actions.variables.not_found"))
|
||||
} else {
|
||||
log.Error("Delete variable [%d] failed: %v", id, err)
|
||||
ctx.JSONError(ctx.Tr("actions.variables.deletion.failed"))
|
||||
}
|
||||
return
|
||||
}
|
||||
ctx.Flash.Success(ctx.Tr("actions.variables.deletion.success"))
|
||||
|
|
|
@ -31,7 +31,7 @@ func CreateVariable(ctx context.Context, ownerID, repoID int64, name, data strin
|
|||
return v, nil
|
||||
}
|
||||
|
||||
func UpdateVariable(ctx context.Context, variableID int64, name, data string) (bool, error) {
|
||||
func UpdateVariable(ctx context.Context, variableID, ownerID, repoID int64, name, data string) (bool, error) {
|
||||
if err := secret_service.ValidateName(name); err != nil {
|
||||
return false, err
|
||||
}
|
||||
|
@ -41,16 +41,14 @@ func UpdateVariable(ctx context.Context, variableID int64, name, data string) (b
|
|||
}
|
||||
|
||||
return actions_model.UpdateVariable(ctx, &actions_model.ActionVariable{
|
||||
ID: variableID,
|
||||
Name: strings.ToUpper(name),
|
||||
Data: util.ReserveLineBreakForTextarea(data),
|
||||
ID: variableID,
|
||||
Name: strings.ToUpper(name),
|
||||
Data: util.ReserveLineBreakForTextarea(data),
|
||||
OwnerID: ownerID,
|
||||
RepoID: repoID,
|
||||
})
|
||||
}
|
||||
|
||||
func DeleteVariableByID(ctx context.Context, variableID int64) error {
|
||||
return actions_model.DeleteVariable(ctx, variableID)
|
||||
}
|
||||
|
||||
func DeleteVariableByName(ctx context.Context, ownerID, repoID int64, name string) error {
|
||||
if err := secret_service.ValidateName(name); err != nil {
|
||||
return err
|
||||
|
@ -69,7 +67,8 @@ func DeleteVariableByName(ctx context.Context, ownerID, repoID int64, name strin
|
|||
return err
|
||||
}
|
||||
|
||||
return actions_model.DeleteVariable(ctx, v.ID)
|
||||
_, err = actions_model.DeleteVariable(ctx, v.ID, ownerID, repoID)
|
||||
return err
|
||||
}
|
||||
|
||||
func GetVariable(ctx context.Context, opts actions_model.FindVariablesOpts) (*actions_model.ActionVariable, error) {
|
||||
|
|
Loading…
Add table
Reference in a new issue