Skip to content
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

Secrets storage with SecretKey encrypted #22142

Merged
merged 103 commits into from
Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from 97 commits
Commits
Show all changes
103 commits
Select commit Hold shift + click to select a range
6b24ada
Add simple master key provider for secret encryption
lafriks Jan 5, 2021
47c472a
Add AES GCM encryption provider
lafriks Jan 7, 2021
64a91b4
add models and services
lunny Oct 7, 2022
9f2d204
improve UI
lunny Oct 9, 2022
368c97e
Merge main branch
lunny Oct 9, 2022
d39f7c4
finish adding and deleting secret for repository settting
lunny Oct 9, 2022
9465c13
org secrets
lunny Oct 10, 2022
eb2b139
fix show add secret panel
wxiaoguang Oct 13, 2022
f4df973
org secrets
lunny Oct 13, 2022
abde871
Fix duplicated alert
lunny Oct 14, 2022
b1e8915
delete modal
wxiaoguang Oct 14, 2022
a077ee2
Almost finished
lunny Oct 14, 2022
5b1044a
merge main branch
lunny Oct 14, 2022
834c7c1
Fix lint
lunny Oct 14, 2022
8e1291d
Some improvements
lunny Oct 15, 2022
1788de5
Merge branch 'main' into lafriks-fork-feat/secrets
lunny Oct 15, 2022
0a3be05
Fix bug
lunny Oct 15, 2022
a0ebf78
Merge branch 'main' into lafriks-fork-feat/secrets
lunny Oct 15, 2022
319da15
Fix bug
lunny Oct 16, 2022
b08114b
Merge branch 'main' into lafriks-fork-feat/secrets
lunny Oct 16, 2022
073656c
merge main branch
lunny Oct 17, 2022
f89bd80
merge main branch
lunny Oct 20, 2022
850d936
encrypt master key
lunny Oct 20, 2022
41310a7
merge main branch
lunny Oct 21, 2022
e54785a
merge main branch
lunny Oct 22, 2022
ccb57c8
merge main branch
lunny Oct 24, 2022
e30f532
merge main branch
lunny Nov 7, 2022
29e4f6b
merge main branch
lunny Nov 21, 2022
b01b2a9
merge main branch
lunny Nov 26, 2022
a1aee64
Merge branch 'main' into lafriks-fork-feat/secrets
lunny Nov 26, 2022
4c8f590
Merge branch 'main' into feature/secrets
wolfogre Dec 7, 2022
bc999bd
chore: add SPDX-License-Identifier
wolfogre Dec 7, 2022
41e9be0
fix: use .locale.Tr
wolfogre Dec 7, 2022
2c7ae0c
Merge branch 'main' into feature/secrets
wolfogre Dec 8, 2022
dd84d07
fix: use LONGTEXT
wolfogre Dec 9, 2022
f5effc1
fix: update nameRE
wolfogre Dec 9, 2022
c08fc15
fix: ErrSecretInvalidValue
wolfogre Dec 9, 2022
44ca6bf
fix: secret_deletion_failed
wolfogre Dec 9, 2022
6fcb7bf
fix: in_error
wolfogre Dec 9, 2022
b79b156
fix: remove PullRequest field
wolfogre Dec 9, 2022
acc0c12
fix: FindUserSecrets
wolfogre Dec 9, 2022
c754525
Merge pull request #4 from wolfogre/feature/secrets
lafriks Dec 9, 2022
eb5bcec
Merge branch 'main' into feat/secrets
wolfogre Dec 13, 2022
3183368
Update templates/org/settings/navbar.tmpl
wolfogre Dec 13, 2022
e86e30f
Update options/locale/locale_en-US.ini
wolfogre Dec 13, 2022
e6cee41
Update options/locale/locale_en-US.ini
wolfogre Dec 13, 2022
641d37a
fix: use web.Bind
wolfogre Dec 13, 2022
7c82f7a
fix: remove PullRequestRead
wolfogre Dec 13, 2022
f9d58d4
fix: rename to secret_name
wolfogre Dec 13, 2022
aa10928
Update templates/org/settings/secrets.tmpl
wolfogre Dec 13, 2022
f738069
Update routers/web/org/setting.go
wolfogre Dec 13, 2022
5103f1d
Update routers/web/org/setting.go
wolfogre Dec 13, 2022
a23241f
Update services/secrets/encryption_aes.go
wolfogre Dec 13, 2022
9f8fdaa
Update templates/install.tmpl
wolfogre Dec 13, 2022
b32bb7a
Update templates/repo/settings/secrets.tmpl
wolfogre Dec 13, 2022
23dd7a7
Merge branch 'main' into feat/secrets
wolfogre Dec 14, 2022
a8c192d
fix: rename to owner
wolfogre Dec 14, 2022
f1ef5ae
fix: remove FindObjects
wolfogre Dec 14, 2022
4a2676e
fix: add unique index
wolfogre Dec 14, 2022
5aa55fe
fix: delete secrets
wolfogre Dec 14, 2022
d1a729b
fix: generate master key with 64 chars
wolfogre Dec 14, 2022
00f305f
chore: revert files about master key
wolfogre Dec 14, 2022
b1a1926
fix: make it compile
wolfogre Dec 14, 2022
6352031
Merge branch 'main' into feature/secrets
wolfogre Dec 15, 2022
402c8aa
chore: remove In
wolfogre Dec 15, 2022
caecad3
fix: delete secrets
wolfogre Dec 15, 2022
9139775
chore: remove locale
wolfogre Dec 15, 2022
91a3048
chore: simplify template
wolfogre Dec 15, 2022
343c3b4
fix: add index
wolfogre Dec 15, 2022
b828e3b
chore: remove RequireTribute
wolfogre Dec 15, 2022
df8ad92
fix: post repo secrets
wolfogre Dec 15, 2022
ab58816
feat: new locale
wolfogre Dec 15, 2022
2b745d0
fix: use new locale
wolfogre Dec 15, 2022
674fced
fix: creation.failed
wolfogre Dec 15, 2022
7b241ca
fix: check reg
wolfogre Dec 15, 2022
031ac08
chore: remove tooltip
wolfogre Dec 15, 2022
4753a9b
feat: trim space
wolfogre Dec 15, 2022
6c11bd8
test: debug
wolfogre Dec 15, 2022
b4f6063
Revert "test: debug"
wolfogre Dec 15, 2022
7bc76de
chore: update comment
wolfogre Dec 15, 2022
b398215
Apply suggestions from code review
lunny Dec 16, 2022
5562518
Update templates/repo/settings/secrets.tmpl
wolfogre Dec 19, 2022
6ebc38e
Update templates/org/settings/secrets.tmpl
wolfogre Dec 19, 2022
3c26505
fix: label for
wolfogre Dec 19, 2022
e74572f
Merge branch 'main' into feature/secrets
wolfogre Dec 19, 2022
752d8e5
fix: template
wolfogre Dec 19, 2022
eba6dc5
fix: use different routes
wolfogre Dec 19, 2022
762a23c
fix: update name rule
wolfogre Dec 19, 2022
1bf0e15
Merge branch 'main' into feature/secrets
wolfogre Dec 19, 2022
1c415f9
Add a simple description for secrets
lunny Dec 19, 2022
0f9e2a6
remove wrong description
lunny Dec 19, 2022
0ae9a05
Update docs/content/doc/secrets/overview.en-us.md
lunny Dec 19, 2022
5395a56
Apply suggestions from code review
lunny Dec 19, 2022
524561d
Update docs/content/doc/secrets/overview.en-us.md
lunny Dec 19, 2022
4478b5e
Fix lint
lunny Dec 19, 2022
32cae8e
Follow reviewers' requirements
lunny Dec 19, 2022
5652a6d
introduce InsertEncryptedSecret function
lunny Dec 19, 2022
8b9837a
Update docs/content/doc/secrets/overview.en-us.md
lunny Dec 19, 2022
4f266c1
Fix lint
lunny Dec 19, 2022
aceb513
Merge branch 'main' into feature/secrets
wolfogre Dec 20, 2022
e587971
chore: retrigger CI
wolfogre Dec 20, 2022
401b6a7
Remove duplicated sentence
lunny Dec 20, 2022
0529b1c
follow @KN4CK3R's review
lunny Dec 20, 2022
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
35 changes: 35 additions & 0 deletions docs/content/doc/secrets/overview.en-us.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
date: "2022-12-19T21:26:00+08:00"
title: "Encrypted secrets"
slug: "secrets/overview"
draft: false
toc: false
menu:
sidebar:
parent: "secrets"
name: "Overview"
weight: 1
identifier: "overview"
---

# Encrypted secrets

Encrypted secrets allow you to store sensitive information in your organization, repository, or repository environments.
lunny marked this conversation as resolved.
Show resolved Hide resolved

# Naming your secrets

The following rules apply to secret names:

Secret names can only contain alphanumeric characters (`[a-z]`, `[A-Z]`, `[0-9]`) or underscores (`_`). Spaces are not allowed.

Secret names must not start with the `GITHUB_` and `GITEA_` prefix.

Secret names must not start with a number.

Secret names are not case-sensitive.

Secret names must be unique at the level they are created at.

For example, a secret created at the repository level must have a unique name in that repository, and a secret created at the organization level must have a unique name at that level.

If a secret with the same name exists at multiple levels, the secret at the lowest level takes precedence. For example, if an organization-level secret has the same name as a repository-level secret, then the repository-level secret takes precedence. Similarly, if an organization and repository both have a secret with the same name, the repository-level secret takes precedence.
lunny marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,8 @@ var migrations = []Migration{
NewMigration("Add package cleanup rule table", v1_19.CreatePackageCleanupRuleTable),
// v235 -> v236
NewMigration("Add index for access_token", v1_19.AddIndexForAccessToken),
// v236 -> v237
NewMigration("Create secrets table", v1_19.CreateSecretsTable),
}

// GetCurrentDBVersion returns the current db version
Expand Down
23 changes: 23 additions & 0 deletions models/migrations/v1_19/v236.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2022 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package v1_19 //nolint

import (
"code.gitea.io/gitea/modules/timeutil"

"xorm.io/xorm"
)

func CreateSecretsTable(x *xorm.Engine) error {
type Secret struct {
ID int64
OwnerID int64 `xorm:"INDEX UNIQUE(owner_repo_name) NOT NULL"`
RepoID int64 `xorm:"INDEX UNIQUE(owner_repo_name) NOT NULL DEFAULT 0"`
Name string `xorm:"UNIQUE(owner_repo_name) NOT NULL"`
Data string `xorm:"LONGTEXT"`
CreatedUnix timeutil.TimeStamp `xorm:"created NOT NULL"`
}

return x.Sync(new(Secret))
}
2 changes: 2 additions & 0 deletions models/organization/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/perm"
repo_model "code.gitea.io/gitea/models/repo"
secret_model "code.gitea.io/gitea/models/secret"
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/log"
Expand Down Expand Up @@ -370,6 +371,7 @@ func DeleteOrganization(ctx context.Context, org *Organization) error {
&TeamUser{OrgID: org.ID},
&TeamUnit{OrgID: org.ID},
&TeamInvite{OrgID: org.ID},
&secret_model.Secret{OwnerID: org.ID},
); err != nil {
return fmt.Errorf("DeleteBeans: %w", err)
}
Expand Down
2 changes: 2 additions & 0 deletions models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
access_model "code.gitea.io/gitea/models/perm/access"
project_model "code.gitea.io/gitea/models/project"
repo_model "code.gitea.io/gitea/models/repo"
secret_model "code.gitea.io/gitea/models/secret"
system_model "code.gitea.io/gitea/models/system"
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
Expand Down Expand Up @@ -150,6 +151,7 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error {
&admin_model.Task{RepoID: repoID},
&repo_model.Watch{RepoID: repoID},
&webhook.Webhook{RepoID: repoID},
&secret_model.Secret{RepoID: repoID},
); err != nil {
return fmt.Errorf("deleteBeans: %w", err)
}
Expand Down
124 changes: 124 additions & 0 deletions models/secret/secret.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
// Copyright 2022 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package secret

import (
"context"
"fmt"
"regexp"
"strings"

"code.gitea.io/gitea/models/db"
secret_module "code.gitea.io/gitea/modules/secret"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/util"

"xorm.io/builder"
)

type ErrSecretInvalidValue struct {
Name *string
Data *string
}

func (err ErrSecretInvalidValue) Error() string {
if err.Name != nil {
return fmt.Sprintf("secret name %q is invalid", *err.Name)
}
if err.Data != nil {
return fmt.Sprintf("secret data %q is invalid", *err.Data)
}
return util.ErrInvalidArgument.Error()
}

func (err ErrSecretInvalidValue) Unwrap() error {
return util.ErrInvalidArgument
}

// Secret represents a secret
type Secret struct {
ID int64
OwnerID int64 `xorm:"INDEX UNIQUE(owner_repo_name) NOT NULL"`
RepoID int64 `xorm:"INDEX UNIQUE(owner_repo_name) NOT NULL DEFAULT 0"`
Name string `xorm:"UNIQUE(owner_repo_name) NOT NULL"`
Data string `xorm:"LONGTEXT"` // encrypted data
CreatedUnix timeutil.TimeStamp `xorm:"created NOT NULL"`
}

// 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, strings.TrimSpace(data))
if err != nil {
return nil, err
}
secret := newSecret(ownerID, repoID, name, encrypted)
if err := secret.Validate(); err != nil {
return secret, err
}
return secret, db.Insert(ctx, secret)
}

func init() {
db.RegisterModel(new(Secret))
}

var (
secretNameReg = regexp.MustCompile("^[A-Z_][A-Z0-9_]*$")
forbiddenSecretPrefixReg = regexp.MustCompile("^GIT(EA|HUB)_")
)

// Validate validates the required fields and formats.
func (s *Secret) Validate() error {
switch {
case len(s.Name) == 0 || len(s.Name) > 50:
return ErrSecretInvalidValue{Name: &s.Name}
case len(s.Data) == 0:
return ErrSecretInvalidValue{Data: &s.Data}
case !secretNameReg.MatchString(s.Name) ||
forbiddenSecretPrefixReg.MatchString(s.Name):
return ErrSecretInvalidValue{Name: &s.Name}
default:
return nil
}
}

type FindSecretsOptions struct {
db.ListOptions
OwnerID int64
RepoID int64
}

func (opts *FindSecretsOptions) toConds() builder.Cond {
cond := builder.NewCond()
if opts.OwnerID > 0 {
cond = cond.And(builder.Eq{"owner_id": opts.OwnerID})
}
if opts.RepoID > 0 {
cond = cond.And(builder.Eq{"repo_id": opts.RepoID})
}

return cond
}

func FindSecrets(ctx context.Context, opts FindSecretsOptions) ([]*Secret, error) {
var secrets []*Secret
sess := db.GetEngine(ctx)
if opts.PageSize != 0 {
sess = db.SetSessionPagination(sess, &opts.ListOptions)
}
return secrets, sess.
Where(opts.toConds()).
Find(&secrets)
}
16 changes: 16 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -3212,3 +3212,19 @@ owner.settings.cleanuprules.remove.days = Remove versions older than
owner.settings.cleanuprules.remove.pattern = Remove versions matching
owner.settings.cleanuprules.success.update = Cleanup rule has been updated.
owner.settings.cleanuprules.success.delete = Cleanup rule has been deleted.

[secrets]
secrets = Secrets
description = Secrets will be passed to certain actions and cannot be read otherwise.
none = There are no secrets yet.
value = Value
name = Name
creation = Add Secret
creation.name_placeholder = case-insensitive, alphanumeric characters or underscores only, cannot start with GITEA_ or GITHUB_
creation.value_placeholder = Input any content. Whitespace at the start and end will be omitted.
creation.success = The secret '%s' has been added.
creation.failed = Failed to add secret.
deletion = Remove secret
deletion.description = Removing a secret will revoke its access to repositories. Continue?
deletion.success = The secret has been removed.
deletion.failed = Failed to remove secret.
51 changes: 51 additions & 0 deletions routers/web/org/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"code.gitea.io/gitea/models"
"code.gitea.io/gitea/models/db"
repo_model "code.gitea.io/gitea/models/repo"
secret_model "code.gitea.io/gitea/models/secret"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/models/webhook"
"code.gitea.io/gitea/modules/base"
Expand All @@ -37,6 +38,8 @@ const (
tplSettingsHooks base.TplName = "org/settings/hooks"
// tplSettingsLabels template path for render labels settings
tplSettingsLabels base.TplName = "org/settings/labels"
// tplSettingsSecrets template path for render secrets settings
tplSettingsSecrets base.TplName = "org/settings/secrets"
)

// Settings render the main settings page
Expand Down Expand Up @@ -246,3 +249,51 @@ func Labels(ctx *context.Context) {
ctx.Data["LabelTemplates"] = repo_module.LabelTemplates
ctx.HTML(http.StatusOK, tplSettingsLabels)
}

// Secrets render organization secrets page
func Secrets(ctx *context.Context) {
ctx.Data["Title"] = ctx.Tr("repo.secrets")
ctx.Data["PageIsOrgSettings"] = true
ctx.Data["PageIsOrgSettingsSecrets"] = true

secrets, err := secret_model.FindSecrets(ctx, secret_model.FindSecretsOptions{OwnerID: ctx.Org.Organization.ID})
if err != nil {
ctx.ServerError("FindSecrets", err)
return
}
ctx.Data["Secrets"] = secrets

ctx.HTML(http.StatusOK, tplSettingsSecrets)
}

// SecretsPost add secrets
func SecretsPost(ctx *context.Context) {
form := web.GetForm(ctx).(*forms.AddSecretForm)

_, err := secret_model.InsertEncryptedSecret(ctx, ctx.Org.Organization.ID, 0, form.Title, strings.TrimSpace(form.Content))
lunny marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
ctx.Flash.Error(ctx.Tr("secrets.creation.failed"))
log.Error("validate secret: %v", err)
ctx.Redirect(ctx.Org.OrgLink + "/settings/secrets")
return
}

log.Trace("Org %d: secret added", ctx.Org.Organization.ID)
ctx.Flash.Success(ctx.Tr("secrets.creation.success", form.Title))
ctx.Redirect(ctx.Org.OrgLink + "/settings/secrets")
}

// SecretsDelete delete secrets
func SecretsDelete(ctx *context.Context) {
id := ctx.FormInt64("id")
if _, err := db.DeleteByBean(ctx, &secret_model.Secret{ID: id}); err != nil {
ctx.Flash.Error(ctx.Tr("secrets.deletion.failed"))
log.Error("delete secret %d: %v", id, err)
} else {
ctx.Flash.Success(ctx.Tr("secrets.deletion.success"))
}

ctx.JSON(http.StatusOK, map[string]interface{}{
"redirect": ctx.Org.OrgLink + "/settings/secrets",
})
}
40 changes: 40 additions & 0 deletions routers/web/repo/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"code.gitea.io/gitea/models/organization"
"code.gitea.io/gitea/models/perm"
repo_model "code.gitea.io/gitea/models/repo"
secret_model "code.gitea.io/gitea/models/secret"
unit_model "code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/base"
Expand Down Expand Up @@ -1113,12 +1114,37 @@ func DeployKeys(ctx *context.Context) {
}
ctx.Data["Deploykeys"] = keys

secrets, err := secret_model.FindSecrets(ctx, secret_model.FindSecretsOptions{RepoID: ctx.Repo.Repository.ID})
if err != nil {
ctx.ServerError("FindSecrets", err)
return
}
ctx.Data["Secrets"] = secrets
Copy link
Member

Choose a reason for hiding this comment

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

As I said on the previous PR:
I think it would be a good idea to add ctx.Data["UserSecrets"] = FindSecrets(ctx, opts{OwnerID: ctx.Owner.ID}) (pseudo code)
so that we can also display read-only the user/org secrets that are already defined, and link to the corresponding settings page.
However, this can also be postponed for later if necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's a good idea, but let's do it later.


ctx.HTML(http.StatusOK, tplDeployKeys)
}

// SecretsPost response for creating a new secret
func SecretsPost(ctx *context.Context) {
form := web.GetForm(ctx).(*forms.AddSecretForm)

_, err := secret_model.InsertEncryptedSecret(ctx, 0, ctx.Repo.Repository.ID, form.Title, strings.TrimSpace(form.Content))
lunny marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
ctx.Flash.Error(ctx.Tr("secrets.creation.failed"))
log.Error("validate secret: %v", err)
ctx.Redirect(ctx.Repo.RepoLink + "/settings/keys")
return
}

log.Trace("Secret added: %d", ctx.Repo.Repository.ID)
ctx.Flash.Success(ctx.Tr("secrets.creation.success", form.Title))
ctx.Redirect(ctx.Repo.RepoLink + "/settings/keys")
}

// DeployKeysPost response for adding a deploy key of a repository
func DeployKeysPost(ctx *context.Context) {
form := web.GetForm(ctx).(*forms.AddKeyForm)

ctx.Data["Title"] = ctx.Tr("repo.settings.deploy_keys")
ctx.Data["PageIsSettingsKeys"] = true
ctx.Data["DisableSSH"] = setting.SSH.Disabled
Expand Down Expand Up @@ -1177,6 +1203,20 @@ func DeployKeysPost(ctx *context.Context) {
ctx.Redirect(ctx.Repo.RepoLink + "/settings/keys")
}

func DeleteSecret(ctx *context.Context) {
id := ctx.FormInt64("id")
Copy link
Member

@delvh delvh Dec 19, 2022

Choose a reason for hiding this comment

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

Alternatively, we could also delete by name, repo, and owner.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure, maybe we can provide batch deletion later.

Copy link
Member

@delvh delvh Dec 19, 2022

Choose a reason for hiding this comment

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

That wasn't what I meant:
I simply meant that deletion by id is certainly the easiest way to implement it, but we could also have implemented it by using the secret name, owner, and repo. That makes it at least for users easier to see that the correct secret will be deleted.
But it is such an unimportant detail that we can end this discussion now and keep the current approach.

Copy link
Member

Choose a reason for hiding this comment

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

@delvh While I agree with you, usually the user will never see this request because it is just a POST from the overview page. So the user will not see the called url.

if _, err := db.DeleteByBean(ctx, &secret_model.Secret{ID: id}); err != nil {
ctx.Flash.Error(ctx.Tr("secrets.deletion.failed"))
log.Error("delete secret %d: %v", id, err)
} else {
ctx.Flash.Success(ctx.Tr("secrets.deletion.success"))
}

ctx.JSON(http.StatusOK, map[string]interface{}{
"redirect": ctx.Repo.RepoLink + "/settings/keys",
})
}

// DeleteDeployKey response for deleting a deploy key
func DeleteDeployKey(ctx *context.Context) {
if err := asymkey_service.DeleteDeployKey(ctx.Doer, ctx.FormInt64("id")); err != nil {
Expand Down
Loading