Skip to content

Commit a99b96c

Browse files
KN4CK3RKN4CK3R
authored
Refactor secrets modification logic (#26873)
- Share code between web and api - Add some tests
1 parent e9f5067 commit a99b96c

File tree

10 files changed

+344
-204
lines changed

10 files changed

+344
-204
lines changed

‎models/secret/secret.go‎

Lines changed: 23 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,6 @@ type ErrSecretNotFound struct {
3333
Name string
3434
}
3535

36-
// IsErrSecretNotFound checks if an error is a ErrSecretNotFound.
37-
func IsErrSecretNotFound(err error) bool {
38-
_, ok := err.(ErrSecretNotFound)
39-
return ok
40-
}
41-
4236
func (err ErrSecretNotFound) Error() string {
4337
return fmt.Sprintf("secret was not found [name: %s]", err.Name)
4438
}
@@ -47,23 +41,18 @@ func (err ErrSecretNotFound) Unwrap() error {
4741
return util.ErrNotExist
4842
}
4943

50-
// newSecret Creates a new already encrypted secret
51-
func newSecret(ownerID, repoID int64, name, data string) *Secret {
52-
return &Secret{
53-
OwnerID: ownerID,
54-
RepoID: repoID,
55-
Name: strings.ToUpper(name),
56-
Data: data,
57-
}
58-
}
59-
6044
// InsertEncryptedSecret Creates, encrypts, and validates a new secret with yet unencrypted data and insert into database
6145
func InsertEncryptedSecret(ctx context.Context, ownerID, repoID int64, name, data string) (*Secret, error) {
6246
encrypted, err := secret_module.EncryptSecret(setting.SecretKey, data)
6347
if err != nil {
6448
return nil, err
6549
}
66-
secret := newSecret(ownerID, repoID, name, encrypted)
50+
secret := &Secret{
51+
OwnerID: ownerID,
52+
RepoID: repoID,
53+
Name: strings.ToUpper(name),
54+
Data: encrypted,
55+
}
6756
if err := secret.Validate(); err != nil {
6857
return secret, err
6958
}
@@ -83,8 +72,10 @@ func (s *Secret) Validate() error {
8372

8473
type FindSecretsOptions struct {
8574
db.ListOptions
86-
OwnerID int64
87-
RepoID int64
75+
OwnerID int64
76+
RepoID int64
77+
SecretID int64
78+
Name string
8879
}
8980

9081
func (opts *FindSecretsOptions) toConds() builder.Cond {
@@ -95,6 +86,12 @@ func (opts *FindSecretsOptions) toConds() builder.Cond {
9586
if opts.RepoID > 0 {
9687
cond = cond.And(builder.Eq{"repo_id": opts.RepoID})
9788
}
89+
if opts.SecretID != 0 {
90+
cond = cond.And(builder.Eq{"id": opts.SecretID})
91+
}
92+
if opts.Name != "" {
93+
cond = cond.And(builder.Eq{"name": strings.ToUpper(opts.Name)})
94+
}
9895

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

118115
// UpdateSecret changes org or user reop secret.
119-
func UpdateSecret(ctx context.Context, orgID, repoID int64, name, data string) error {
120-
sc := new(Secret)
121-
name = strings.ToUpper(name)
122-
has, err := db.GetEngine(ctx).
123-
Where("owner_id=?", orgID).
124-
And("repo_id=?", repoID).
125-
And("name=?", name).
126-
Get(sc)
127-
if err != nil {
128-
return err
129-
} else if !has {
130-
return ErrSecretNotFound{Name: name}
131-
}
132-
116+
func UpdateSecret(ctx context.Context, secretID int64, data string) error {
133117
encrypted, err := secret_module.EncryptSecret(setting.SecretKey, data)
134118
if err != nil {
135119
return err
136120
}
137121

138-
sc.Data = encrypted
139-
_, err = db.GetEngine(ctx).ID(sc.ID).Cols("data").Update(sc)
140-
return err
141-
}
142-
143-
// DeleteSecret deletes secret from an organization.
144-
func DeleteSecret(ctx context.Context, orgID, repoID int64, name string) error {
145-
sc := new(Secret)
146-
has, err := db.GetEngine(ctx).
147-
Where("owner_id=?", orgID).
148-
And("repo_id=?", repoID).
149-
And("name=?", strings.ToUpper(name)).
150-
Get(sc)
151-
if err != nil {
152-
return err
153-
} else if !has {
154-
return ErrSecretNotFound{Name: name}
155-
}
156-
157-
if _, err := db.GetEngine(ctx).ID(sc.ID).Delete(new(Secret)); err != nil {
158-
return fmt.Errorf("Delete: %w", err)
159-
}
160-
161-
return nil
162-
}
163-
164-
// CreateOrUpdateSecret creates or updates a secret and returns true if it was created
165-
func CreateOrUpdateSecret(ctx context.Context, orgID, repoID int64, name, data string) (bool, error) {
166-
sc := new(Secret)
167-
name = strings.ToUpper(name)
168-
has, err := db.GetEngine(ctx).
169-
Where("owner_id=?", orgID).
170-
And("repo_id=?", repoID).
171-
And("name=?", name).
172-
Get(sc)
173-
if err != nil {
174-
return false, err
122+
s := &Secret{
123+
Data: encrypted,
175124
}
176-
177-
if !has {
178-
_, err = InsertEncryptedSecret(ctx, orgID, repoID, name, data)
179-
if err != nil {
180-
return false, err
181-
}
182-
return true, nil
125+
affected, err := db.GetEngine(ctx).ID(secretID).Cols("data").Update(s)
126+
if affected != 1 {
127+
return ErrSecretNotFound{}
183128
}
184-
185-
if err := UpdateSecret(ctx, orgID, repoID, name, data); err != nil {
186-
return false, err
187-
}
188-
189-
return false, nil
129+
return err
190130
}

‎routers/api/v1/org/action.go‎

Lines changed: 32 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,16 @@
44
package org
55

66
import (
7+
"errors"
78
"net/http"
89

910
secret_model "code.gitea.io/gitea/models/secret"
1011
"code.gitea.io/gitea/modules/context"
1112
api "code.gitea.io/gitea/modules/structs"
13+
"code.gitea.io/gitea/modules/util"
1214
"code.gitea.io/gitea/modules/web"
1315
"code.gitea.io/gitea/routers/api/v1/utils"
14-
"code.gitea.io/gitea/routers/web/shared/actions"
16+
secret_service "code.gitea.io/gitea/services/secrets"
1517
)
1618

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

42-
listActionsSecrets(ctx)
43-
}
44-
45-
// listActionsSecrets list an organization's actions secrets
46-
func listActionsSecrets(ctx *context.APIContext) {
4744
opts := &secret_model.FindSecretsOptions{
4845
OwnerID: ctx.Org.Organization.ID,
4946
ListOptions: utils.GetListOptions(ctx),
@@ -104,25 +101,28 @@ func CreateOrUpdateSecret(ctx *context.APIContext) {
104101
// description: response when updating a secret
105102
// "400":
106103
// "$ref": "#/responses/error"
107-
// "403":
108-
// "$ref": "#/responses/forbidden"
109-
secretName := ctx.Params(":secretname")
110-
if err := actions.NameRegexMatch(secretName); err != nil {
111-
ctx.Error(http.StatusBadRequest, "CreateOrUpdateSecret", err)
112-
return
113-
}
104+
// "404":
105+
// "$ref": "#/responses/notFound"
106+
114107
opt := web.GetForm(ctx).(*api.CreateOrUpdateSecretOption)
115-
isCreated, err := secret_model.CreateOrUpdateSecret(ctx, ctx.Org.Organization.ID, 0, secretName, opt.Data)
108+
109+
_, created, err := secret_service.CreateOrUpdateSecret(ctx, ctx.Org.Organization.ID, 0, ctx.Params("secretname"), opt.Data)
116110
if err != nil {
117-
ctx.Error(http.StatusInternalServerError, "CreateOrUpdateSecret", err)
111+
if errors.Is(err, util.ErrInvalidArgument) {
112+
ctx.Error(http.StatusBadRequest, "CreateOrUpdateSecret", err)
113+
} else if errors.Is(err, util.ErrNotExist) {
114+
ctx.Error(http.StatusNotFound, "CreateOrUpdateSecret", err)
115+
} else {
116+
ctx.Error(http.StatusInternalServerError, "CreateOrUpdateSecret", err)
117+
}
118118
return
119119
}
120-
if isCreated {
120+
121+
if created {
121122
ctx.Status(http.StatusCreated)
122-
return
123+
} else {
124+
ctx.Status(http.StatusNoContent)
123125
}
124-
125-
ctx.Status(http.StatusNoContent)
126126
}
127127

128128
// DeleteSecret delete one secret of the organization
@@ -148,22 +148,20 @@ func DeleteSecret(ctx *context.APIContext) {
148148
// responses:
149149
// "204":
150150
// description: delete one secret of the organization
151-
// "403":
152-
// "$ref": "#/responses/forbidden"
153-
secretName := ctx.Params(":secretname")
154-
if err := actions.NameRegexMatch(secretName); err != nil {
155-
ctx.Error(http.StatusBadRequest, "DeleteSecret", err)
156-
return
157-
}
158-
err := secret_model.DeleteSecret(
159-
ctx, ctx.Org.Organization.ID, 0, secretName,
160-
)
161-
if secret_model.IsErrSecretNotFound(err) {
162-
ctx.NotFound(err)
163-
return
164-
}
151+
// "400":
152+
// "$ref": "#/responses/error"
153+
// "404":
154+
// "$ref": "#/responses/notFound"
155+
156+
err := secret_service.DeleteSecretByName(ctx, ctx.Org.Organization.ID, 0, ctx.Params("secretname"))
165157
if err != nil {
166-
ctx.Error(http.StatusInternalServerError, "DeleteSecret", err)
158+
if errors.Is(err, util.ErrInvalidArgument) {
159+
ctx.Error(http.StatusBadRequest, "DeleteSecret", err)
160+
} else if errors.Is(err, util.ErrNotExist) {
161+
ctx.Error(http.StatusNotFound, "DeleteSecret", err)
162+
} else {
163+
ctx.Error(http.StatusInternalServerError, "DeleteSecret", err)
164+
}
167165
return
168166
}
169167

‎routers/api/v1/repo/action.go‎

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,14 @@
44
package repo
55

66
import (
7+
"errors"
78
"net/http"
89

9-
secret_model "code.gitea.io/gitea/models/secret"
1010
"code.gitea.io/gitea/modules/context"
1111
api "code.gitea.io/gitea/modules/structs"
12+
"code.gitea.io/gitea/modules/util"
1213
"code.gitea.io/gitea/modules/web"
13-
"code.gitea.io/gitea/routers/web/shared/actions"
14+
secret_service "code.gitea.io/gitea/services/secrets"
1415
)
1516

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

5556
owner := ctx.Repo.Owner
5657
repo := ctx.Repo.Repository
5758

58-
secretName := ctx.Params(":secretname")
59-
if err := actions.NameRegexMatch(secretName); err != nil {
60-
ctx.Error(http.StatusBadRequest, "CreateOrUpdateSecret", err)
61-
return
62-
}
6359
opt := web.GetForm(ctx).(*api.CreateOrUpdateSecretOption)
64-
isCreated, err := secret_model.CreateOrUpdateSecret(ctx, owner.ID, repo.ID, secretName, opt.Data)
60+
61+
_, created, err := secret_service.CreateOrUpdateSecret(ctx, owner.ID, repo.ID, ctx.Params("secretname"), opt.Data)
6562
if err != nil {
66-
ctx.Error(http.StatusInternalServerError, "CreateOrUpdateSecret", err)
63+
if errors.Is(err, util.ErrInvalidArgument) {
64+
ctx.Error(http.StatusBadRequest, "CreateOrUpdateSecret", err)
65+
} else if errors.Is(err, util.ErrNotExist) {
66+
ctx.Error(http.StatusNotFound, "CreateOrUpdateSecret", err)
67+
} else {
68+
ctx.Error(http.StatusInternalServerError, "CreateOrUpdateSecret", err)
69+
}
6770
return
6871
}
69-
if isCreated {
72+
73+
if created {
7074
ctx.Status(http.StatusCreated)
71-
return
75+
} else {
76+
ctx.Status(http.StatusNoContent)
7277
}
73-
74-
ctx.Status(http.StatusNoContent)
7578
}
7679

7780
// DeleteSecret delete one secret of the repository
@@ -102,26 +105,23 @@ func DeleteSecret(ctx *context.APIContext) {
102105
// responses:
103106
// "204":
104107
// description: delete one secret of the organization
105-
// "403":
106-
// "$ref": "#/responses/forbidden"
108+
// "400":
109+
// "$ref": "#/responses/error"
110+
// "404":
111+
// "$ref": "#/responses/notFound"
107112

108113
owner := ctx.Repo.Owner
109114
repo := ctx.Repo.Repository
110115

111-
secretName := ctx.Params(":secretname")
112-
if err := actions.NameRegexMatch(secretName); err != nil {
113-
ctx.Error(http.StatusBadRequest, "DeleteSecret", err)
114-
return
115-
}
116-
err := secret_model.DeleteSecret(
117-
ctx, owner.ID, repo.ID, secretName,
118-
)
119-
if secret_model.IsErrSecretNotFound(err) {
120-
ctx.NotFound(err)
121-
return
122-
}
116+
err := secret_service.DeleteSecretByName(ctx, owner.ID, repo.ID, ctx.Params("secretname"))
123117
if err != nil {
124-
ctx.Error(http.StatusInternalServerError, "DeleteSecret", err)
118+
if errors.Is(err, util.ErrInvalidArgument) {
119+
ctx.Error(http.StatusBadRequest, "DeleteSecret", err)
120+
} else if errors.Is(err, util.ErrNotExist) {
121+
ctx.Error(http.StatusNotFound, "DeleteSecret", err)
122+
} else {
123+
ctx.Error(http.StatusInternalServerError, "DeleteSecret", err)
124+
}
125125
return
126126
}
127127

0 commit comments

Comments
 (0)