Skip to content

Refactor secrets modification logic #26873

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 23 additions & 83 deletions models/secret/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,6 @@ type ErrSecretNotFound struct {
Name string
}

// IsErrSecretNotFound checks if an error is a ErrSecretNotFound.
func IsErrSecretNotFound(err error) bool {
_, ok := err.(ErrSecretNotFound)
return ok
}

func (err ErrSecretNotFound) Error() string {
return fmt.Sprintf("secret was not found [name: %s]", err.Name)
}
Expand All @@ -47,23 +41,18 @@ func (err ErrSecretNotFound) Unwrap() error {
return util.ErrNotExist
}

// newSecret Creates a new already encrypted secret
func newSecret(ownerID, repoID int64, name, data string) *Secret {
return &Secret{
OwnerID: ownerID,
RepoID: repoID,
Name: strings.ToUpper(name),
Data: data,
}
}

// InsertEncryptedSecret Creates, encrypts, and validates a new secret with yet unencrypted data and insert into database
func InsertEncryptedSecret(ctx context.Context, ownerID, repoID int64, name, data string) (*Secret, error) {
encrypted, err := secret_module.EncryptSecret(setting.SecretKey, data)
if err != nil {
return nil, err
}
secret := newSecret(ownerID, repoID, name, encrypted)
secret := &Secret{
OwnerID: ownerID,
RepoID: repoID,
Name: strings.ToUpper(name),
Data: encrypted,
}
if err := secret.Validate(); err != nil {
return secret, err
}
Expand All @@ -83,8 +72,10 @@ func (s *Secret) Validate() error {

type FindSecretsOptions struct {
db.ListOptions
OwnerID int64
RepoID int64
OwnerID int64
RepoID int64
SecretID int64
Name string
}

func (opts *FindSecretsOptions) toConds() builder.Cond {
Expand All @@ -95,6 +86,12 @@ func (opts *FindSecretsOptions) toConds() builder.Cond {
if opts.RepoID > 0 {
cond = cond.And(builder.Eq{"repo_id": opts.RepoID})
}
if opts.SecretID != 0 {
cond = cond.And(builder.Eq{"id": opts.SecretID})
}
if opts.Name != "" {
cond = cond.And(builder.Eq{"name": strings.ToUpper(opts.Name)})
}

return cond
}
Expand All @@ -116,75 +113,18 @@ func CountSecrets(ctx context.Context, opts *FindSecretsOptions) (int64, error)
}

// UpdateSecret changes org or user reop secret.
func UpdateSecret(ctx context.Context, orgID, repoID int64, name, data string) error {
sc := new(Secret)
name = strings.ToUpper(name)
has, err := db.GetEngine(ctx).
Where("owner_id=?", orgID).
And("repo_id=?", repoID).
And("name=?", name).
Get(sc)
if err != nil {
return err
} else if !has {
return ErrSecretNotFound{Name: name}
}

func UpdateSecret(ctx context.Context, secretID int64, data string) error {
encrypted, err := secret_module.EncryptSecret(setting.SecretKey, data)
if err != nil {
return err
}

sc.Data = encrypted
_, err = db.GetEngine(ctx).ID(sc.ID).Cols("data").Update(sc)
return err
}

// DeleteSecret deletes secret from an organization.
func DeleteSecret(ctx context.Context, orgID, repoID int64, name string) error {
sc := new(Secret)
has, err := db.GetEngine(ctx).
Where("owner_id=?", orgID).
And("repo_id=?", repoID).
And("name=?", strings.ToUpper(name)).
Get(sc)
if err != nil {
return err
} else if !has {
return ErrSecretNotFound{Name: name}
}

if _, err := db.GetEngine(ctx).ID(sc.ID).Delete(new(Secret)); err != nil {
return fmt.Errorf("Delete: %w", err)
}

return nil
}

// CreateOrUpdateSecret creates or updates a secret and returns true if it was created
func CreateOrUpdateSecret(ctx context.Context, orgID, repoID int64, name, data string) (bool, error) {
sc := new(Secret)
name = strings.ToUpper(name)
has, err := db.GetEngine(ctx).
Where("owner_id=?", orgID).
And("repo_id=?", repoID).
And("name=?", name).
Get(sc)
if err != nil {
return false, err
s := &Secret{
Data: encrypted,
}

if !has {
_, err = InsertEncryptedSecret(ctx, orgID, repoID, name, data)
if err != nil {
return false, err
}
return true, nil
affected, err := db.GetEngine(ctx).ID(secretID).Cols("data").Update(s)
if affected != 1 {
return ErrSecretNotFound{}
}

if err := UpdateSecret(ctx, orgID, repoID, name, data); err != nil {
return false, err
}

return false, nil
return err
}
66 changes: 32 additions & 34 deletions routers/api/v1/org/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@
package org

import (
"errors"
"net/http"

secret_model "code.gitea.io/gitea/models/secret"
"code.gitea.io/gitea/modules/context"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/routers/api/v1/utils"
"code.gitea.io/gitea/routers/web/shared/actions"
secret_service "code.gitea.io/gitea/services/secrets"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't use 007 as import name 😢

)

// ListActionsSecrets list an organization's actions secrets
Expand Down Expand Up @@ -39,11 +41,6 @@ func ListActionsSecrets(ctx *context.APIContext) {
// "200":
// "$ref": "#/responses/SecretList"

listActionsSecrets(ctx)
}

// listActionsSecrets list an organization's actions secrets
func listActionsSecrets(ctx *context.APIContext) {
opts := &secret_model.FindSecretsOptions{
OwnerID: ctx.Org.Organization.ID,
ListOptions: utils.GetListOptions(ctx),
Expand Down Expand Up @@ -104,25 +101,28 @@ func CreateOrUpdateSecret(ctx *context.APIContext) {
// description: response when updating a secret
// "400":
// "$ref": "#/responses/error"
// "403":
// "$ref": "#/responses/forbidden"
secretName := ctx.Params(":secretname")
if err := actions.NameRegexMatch(secretName); err != nil {
ctx.Error(http.StatusBadRequest, "CreateOrUpdateSecret", err)
return
}
// "404":
// "$ref": "#/responses/notFound"

opt := web.GetForm(ctx).(*api.CreateOrUpdateSecretOption)
isCreated, err := secret_model.CreateOrUpdateSecret(ctx, ctx.Org.Organization.ID, 0, secretName, opt.Data)

_, created, err := secret_service.CreateOrUpdateSecret(ctx, ctx.Org.Organization.ID, 0, ctx.Params("secretname"), opt.Data)
if err != nil {
ctx.Error(http.StatusInternalServerError, "CreateOrUpdateSecret", err)
if errors.Is(err, util.ErrInvalidArgument) {
ctx.Error(http.StatusBadRequest, "CreateOrUpdateSecret", err)
} else if errors.Is(err, util.ErrNotExist) {
ctx.Error(http.StatusNotFound, "CreateOrUpdateSecret", err)
} else {
ctx.Error(http.StatusInternalServerError, "CreateOrUpdateSecret", err)
}
return
}
if isCreated {

if created {
ctx.Status(http.StatusCreated)
return
} else {
ctx.Status(http.StatusNoContent)
}

ctx.Status(http.StatusNoContent)
}

// DeleteSecret delete one secret of the organization
Expand All @@ -148,22 +148,20 @@ func DeleteSecret(ctx *context.APIContext) {
// responses:
// "204":
// description: delete one secret of the organization
// "403":
// "$ref": "#/responses/forbidden"
secretName := ctx.Params(":secretname")
if err := actions.NameRegexMatch(secretName); err != nil {
ctx.Error(http.StatusBadRequest, "DeleteSecret", err)
return
}
err := secret_model.DeleteSecret(
ctx, ctx.Org.Organization.ID, 0, secretName,
)
if secret_model.IsErrSecretNotFound(err) {
ctx.NotFound(err)
return
}
// "400":
// "$ref": "#/responses/error"
// "404":
// "$ref": "#/responses/notFound"

err := secret_service.DeleteSecretByName(ctx, ctx.Org.Organization.ID, 0, ctx.Params("secretname"))
if err != nil {
ctx.Error(http.StatusInternalServerError, "DeleteSecret", err)
if errors.Is(err, util.ErrInvalidArgument) {
ctx.Error(http.StatusBadRequest, "DeleteSecret", err)
} else if errors.Is(err, util.ErrNotExist) {
ctx.Error(http.StatusNotFound, "DeleteSecret", err)
} else {
ctx.Error(http.StatusInternalServerError, "DeleteSecret", err)
}
return
}

Expand Down
60 changes: 30 additions & 30 deletions routers/api/v1/repo/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
package repo

import (
"errors"
"net/http"

secret_model "code.gitea.io/gitea/models/secret"
"code.gitea.io/gitea/modules/context"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/routers/web/shared/actions"
secret_service "code.gitea.io/gitea/services/secrets"
)

// create or update one secret of the repository
Expand Down Expand Up @@ -49,29 +50,31 @@ func CreateOrUpdateSecret(ctx *context.APIContext) {
// description: response when updating a secret
// "400":
// "$ref": "#/responses/error"
// "403":
// "$ref": "#/responses/forbidden"
// "404":
// "$ref": "#/responses/notFound"

owner := ctx.Repo.Owner
repo := ctx.Repo.Repository

secretName := ctx.Params(":secretname")
if err := actions.NameRegexMatch(secretName); err != nil {
ctx.Error(http.StatusBadRequest, "CreateOrUpdateSecret", err)
return
}
opt := web.GetForm(ctx).(*api.CreateOrUpdateSecretOption)
isCreated, err := secret_model.CreateOrUpdateSecret(ctx, owner.ID, repo.ID, secretName, opt.Data)

_, created, err := secret_service.CreateOrUpdateSecret(ctx, owner.ID, repo.ID, ctx.Params("secretname"), opt.Data)
if err != nil {
ctx.Error(http.StatusInternalServerError, "CreateOrUpdateSecret", err)
if errors.Is(err, util.ErrInvalidArgument) {
ctx.Error(http.StatusBadRequest, "CreateOrUpdateSecret", err)
} else if errors.Is(err, util.ErrNotExist) {
ctx.Error(http.StatusNotFound, "CreateOrUpdateSecret", err)
} else {
ctx.Error(http.StatusInternalServerError, "CreateOrUpdateSecret", err)
}
return
}
if isCreated {

if created {
ctx.Status(http.StatusCreated)
return
} else {
ctx.Status(http.StatusNoContent)
}

ctx.Status(http.StatusNoContent)
}

// DeleteSecret delete one secret of the repository
Expand Down Expand Up @@ -102,26 +105,23 @@ func DeleteSecret(ctx *context.APIContext) {
// responses:
// "204":
// description: delete one secret of the organization
// "403":
// "$ref": "#/responses/forbidden"
// "400":
// "$ref": "#/responses/error"
// "404":
// "$ref": "#/responses/notFound"

owner := ctx.Repo.Owner
repo := ctx.Repo.Repository

secretName := ctx.Params(":secretname")
if err := actions.NameRegexMatch(secretName); err != nil {
ctx.Error(http.StatusBadRequest, "DeleteSecret", err)
return
}
err := secret_model.DeleteSecret(
ctx, owner.ID, repo.ID, secretName,
)
if secret_model.IsErrSecretNotFound(err) {
ctx.NotFound(err)
return
}
err := secret_service.DeleteSecretByName(ctx, owner.ID, repo.ID, ctx.Params("secretname"))
if err != nil {
ctx.Error(http.StatusInternalServerError, "DeleteSecret", err)
if errors.Is(err, util.ErrInvalidArgument) {
ctx.Error(http.StatusBadRequest, "DeleteSecret", err)
} else if errors.Is(err, util.ErrNotExist) {
ctx.Error(http.StatusNotFound, "DeleteSecret", err)
} else {
ctx.Error(http.StatusInternalServerError, "DeleteSecret", err)
}
return
}

Expand Down
Loading