Add Gitea secrets storage and management #14483

lafriks · 2021-01-27T11:35:55Z

Also needed for #13539

some screenshots:
repository level secrets

org level secrets

kdumontnu · 2021-03-22T23:59:47Z

Does this close #12065?

lafriks · 2021-03-23T00:14:05Z

Yes, this could be used to resolve that

InCogNiTo124 · 2022-03-30T22:15:32Z

Any update on this?

yctn · 2022-04-26T15:57:12Z

Any update on this?

lunny · 2022-09-25T14:20:10Z

@lafriks Could I pick up this PR?

Co-authored-by: delvh <dev.lh@web.de>

KN4CK3R · 2022-12-13T09:36:22Z

services/secrets/secrets.go

+	})
+}
+
+func InsertOrgSecret(ctx context.Context, userID int64, key, data string) error {


There is a name mismatch here: InsertOrgSecret and FindUserSecrets. I would rename both to Owner. Maybe change the structs member too.

I agree about the struct fields.
But we should find a name to stick with as OwnerSecret sounds abolutely wrong to me and doesn't clarify the difference to RepoSecrets enough.
What about UserSecrets instead, even if they can also be for an organization?

JakobDev · 2022-12-13T09:47:03Z

This PR has currently secrets on Repo and Organisation level. Could you also add Secrets to the User level, that would apply to all personal Projects?

delvh · 2022-12-13T09:48:42Z

I have thought about that as well, but what would the benefit be?
When do you really need user global secrets?

KN4CK3R · 2022-12-13T09:53:17Z

The same as org secrets? Available for every repo.

The package registry may need secrets at the owner (user/org) level too. The Debian registry for example uses a gpg key to sign data which could be stored in the secrets.

delvh · 2022-12-13T09:54:23Z

Okay, yes that's makes sense.
Then let's remove the differentiation between users and organizations, and add it for all owners.

JakobDev · 2022-12-13T09:57:45Z

When do you really need user global secrets?

I need them. On GitLab I have a lot of personal projects. Mostly little tools. Almost all have a CI (mostly just publish to e.g.g PyPI on a new Release). a lot of them share the same secrets (e.g. PyPI credentials). As GitLab does not allow secrets on User level, I need to add everything for each Repo new and when something changed, that means a lot of work for me, to change it for every single Repo. I'm currently moving to Codebreg, which uses Gitea. If you add add secrets management, it would be great to allow this on User level to save me a lot of Work. Of course, I could create a Org just for my personal projects, but that feels a little bit unnecessary.

delvh · 2022-12-13T10:09:06Z

services/secrets/secrets.go

+	})
+}
+
+func InsertOrgSecret(ctx context.Context, userID int64, key, data string) error {


I agree about the struct fields.
But we should find a name to stick with as OwnerSecret sounds abolutely wrong to me and doesn't clarify the difference to RepoSecrets enough.
What about UserSecrets instead, even if they can also be for an organization?

delvh · 2022-12-13T10:09:47Z

services/secrets/secrets.go

@@ -1,6 +1,5 @@
 // Copyright 2021 The Gitea Authors. All rights reserved.


Do we copyright the year the code was written, or the year it is merged?
I would have thought it is the year it is merged…

delvh · 2022-12-13T10:17:27Z

models/auth/secret.go

+	UserID      int64              `xorm:"index NOTNULL"`
+	RepoID      int64              `xorm:"index NOTNULL"`
+	Name        string             `xorm:"NOTNULL"`


Could it be that the suggestions in this file were missed?

delvh · 2022-12-13T10:22:49Z

services/secrets/secrets.go

+}
+
+func InsertRepoSecret(ctx context.Context, repoID int64, key, data string) error {
+	v, err := EncryptString(data)


I think I've also mentioned it above, but here it is again:
I think it would be a good idea to strip leading and trailing whitespace from secrets.
There is only a marginal benefit in not doing it (who has a secret that starts or ends with whitespace?).
However, there is a huge benefit for usability, since many copying programs add for example whitespace at the end, meaning you would get an invalid secret that you first have to debug why it isn't working.

Solved in the new PR.

delvh · 2022-12-13T10:24:17Z

options/locale/locale_en-US.ini

+settings.pull_request_read_hint = Allow pull request read the secret
+settings.add_secret = Add Secret
+settings.add_secret_success = The secret '%s' has been added.
+settings.secret_value_content_placeholder = Input any content


Assuming my suggestion above is implemented:

Suggested change

settings.secret_value_content_placeholder = Input any content

settings.secret_value_content_placeholder = Input any content. Whitespace at the start and end will be omitted.

Then suddenly this placeholder isn't redundant anymore.

wolfogre · 2022-12-14T06:07:04Z

Hi guys, I wonder if I can start a new PR with all commits kept?

I tried to fix problems ASAP to stop blocking #21937, and @lafriks kindly gave me access to push to his branch, but I still can't mark comments resolved or update the PR content to explain what I'm trying to do, because I'm not the poster. I think it's OK yet, but if we want to add more to this PR like "secrets for users" or "trimming whitespace", I think I can handle it faster with a new PR.

I know it will cause some inconvenience to reviewers, so I think it's also a good idea to merge this PR with some todos left, and start a new PR to finish them.

What do you think?

lunny · 2022-12-14T06:28:54Z

Hi guys, I wonder if I can start a new PR with all commits kept?

I tried to fix problems ASAP to stop blocking #21937, and @lafriks kindly gave me access to push to his branch, but I still can't mark comments resolved or update the PR content to explain what I'm trying to do, because I'm not the poster. I think it's OK yet, but if we want to add more to this PR like "secrets for users" or "trimming whitespace", I think I can handle it faster with a new PR.

I know it will cause some inconvenience to reviewers, so I think it's also a good idea to merge this PR with some todos left, and start a new PR to finish them.

What do you think?

I personally prefer to leave some todo like supporting user level secrets in next PRs and we can merge this one ASAP.

wolfogre · 2022-12-14T09:34:57Z

In the process of helping to fix problems, I took a close look at the code. Finally, to be honest, I am against this PR.

The point is that I think we don't need a new MasterKey, we should use SecretKey. These are my reasons:

SecretKey can do what MasterKey can do, and they have similar designs:
- SECRET_KEY vs MASTER_KEY
- SECRET_KEY_URI vs MASTER_KEY_PROVIDER
- EncryptString/DecryptString vs EncryptSecret/DecryptSecret
MasterKey will use SECRET_KEY when MASTER_KEY is empty:
- That means it's possible to treat SecretKey as MasterKey.
Response to "SECRET_KEY could be stored encrypted by MASTER_KEY in the future when we add server-wide secret storage we could than use 'internal' secrets that would be protected by master key"
- It doesn't make sense to move a key from the config file into encrypted storage and add a new one into the config file.
- SecretKey can also protect "internal" secrets, and it's already done something like that.
MasterKey isn't a better name:
- Come on guys, have you forgotten why we changed the default branch name to "main"?
- I know this is not where it came from, but SecretKey can be understood as a key to protect other secrets.

If we agree to give up MasterKey, some commits and comments will be out of topic.

So I will start a new PR as an alternative with necessary commits kept, and if I'm wrong, we can continue with this one.

See:

Secrets storage with SecretKey encrypted #22142

delvh · 2022-12-14T10:15:18Z

services/repository/repository.go

@@ -51,6 +52,10 @@ func DeleteRepository(ctx context.Context, doer *user_model.User, repo *repo_mod
 		return err
 	}

+	if err := secret_service.DeleteSecretsByRepoID(ctx, repo.ID); err != nil {


I thought you would simply use models.DeleteRepository#130-152 for it…

Solved in the new PR.

delvh · 2022-12-14T10:16:36Z

services/org/org.go

@@ -39,6 +40,10 @@ func DeleteOrganization(org *organization.Organization) error {
 		return models.ErrUserOwnPackages{UID: org.ID}
 	}

+	if err := secret_service.DeleteSecretsByOwnerID(ctx, org.ID); err != nil {


I thought you would simply use models/DeleteUser#72-91.

Solved in the new PR.

delvh · 2022-12-14T10:19:00Z

models/auth/secret.go

+	OwnerID     int64              `xorm:"UNIQUE(owner_repo_name) NOTNULL"`
+	RepoID      int64              `xorm:"UNIQUE(owner_repo_name) NOTNULL"`


I think we should not remove the indexes. They can still be valuable.

Solved in the new PR.

lunny · 2022-12-15T08:50:31Z

I think we can close this and switch to #22142

Fork of #14483, but [gave up MasterKey](#14483 (comment)), and fixed some problems. Close #12065. Needed by #13539. Featrues: - Secrets for repo and org, not user yet. - Use SecretKey to encrypte/encrypt secrets. - Trim spaces of secret value. - Add a new locale ini block, to make it easy to support secrets for user. Snapshots: Repo level secrets: ![image](https://user-images.githubusercontent.com/9418365/207823319-b8a4903f-38ca-4af7-9d05-336a5af906f3.png) Rrg level secrets ![image](https://user-images.githubusercontent.com/9418365/207823371-8bd02e93-1928-40d1-8c76-f48b255ace36.png) Co-authored-by: Lauris BH <lauris@nix.lv> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: delvh <dev.lh@web.de> Co-authored-by: KN4CK3R <admin@oldschoolhack.me>

lunny · 2022-12-20T09:07:47Z

replaced by #22142

lafriks mentioned this pull request Jan 27, 2021

Discussion of v1.15.x Roadmap #14477

Closed

12 tasks

lunny mentioned this pull request Feb 18, 2021

GPG Signing: let user provide signature #9723

Open

KN4CK3R mentioned this pull request Mar 10, 2021

Add LFS Migration and Mirror #14726

Merged

11 tasks

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 22, 2021

lafriks mentioned this pull request Jul 14, 2021

Plan of Gitea v1.16 #16429

Closed

ansemjo mentioned this pull request Aug 7, 2021

Migrate from SSH #1635

Open

5 tasks

lafriks mentioned this pull request Jan 21, 2022

Plan of Gitea v1.17 #18345

Closed

lunny mentioned this pull request Sep 17, 2022

Unable to update Personal Access Tokens for GitHub pull-mirror repositories #21112

Closed

lunny force-pushed the feat/secrets branch from e9cc59e to bcc8143 Compare September 26, 2022 07:48

lunny added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Sep 26, 2022

lunny added this to the 1.18.0 milestone Sep 26, 2022

lafriks added 2 commits October 6, 2022 12:02

Add simple master key provider for secret encryption

6b24ada

Add AES GCM encryption provider

47c472a

lunny force-pushed the feat/secrets branch from 3b122cd to 47c472a Compare October 6, 2022 04:02

lunny and others added 11 commits October 7, 2022 22:43

add models and services

64a91b4

improve UI

9f2d204

Merge main branch

368c97e

finish adding and deleting secret for repository settting

d39f7c4

org secrets

9465c13

fix show add secret panel

eb2b139

org secrets

f4df973

Fix duplicated alert

abde871

delete modal

b1e8915

Almost finished

a077ee2

merge main branch

5b1044a

wolfogre and others added 5 commits December 13, 2022 17:09

Update routers/web/org/setting.go

f738069

Co-authored-by: delvh <dev.lh@web.de>

Update routers/web/org/setting.go

5103f1d

Co-authored-by: delvh <dev.lh@web.de>

Update services/secrets/encryption_aes.go

a23241f

Co-authored-by: delvh <dev.lh@web.de>

Update templates/install.tmpl

9f8fdaa

Co-authored-by: delvh <dev.lh@web.de>

Update templates/repo/settings/secrets.tmpl

b32bb7a

Co-authored-by: delvh <dev.lh@web.de>

KN4CK3R reviewed Dec 13, 2022

View reviewed changes

delvh reviewed Dec 13, 2022

View reviewed changes

wolfogre added 6 commits December 14, 2022 15:10

Merge branch 'main' into feat/secrets

23dd7a7

fix: rename to owner

a8c192d

fix: remove FindObjects

f1ef5ae

fix: add unique index

4a2676e

fix: delete secrets

5aa55fe

fix: generate master key with 64 chars

d1a729b

delvh reviewed Dec 14, 2022

View reviewed changes

wolfogre mentioned this pull request Dec 15, 2022

Secrets storage with SecretKey encrypted #22142

Merged

lunny closed this Dec 20, 2022

lunny removed this from the 1.19.0 milestone Dec 20, 2022

go-gitea locked and limited conversation to collaborators May 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Gitea secrets storage and management #14483

Add Gitea secrets storage and management #14483

lafriks commented Jan 27, 2021 •

edited by lunny

Loading

kdumontnu commented Mar 22, 2021 •

edited

Loading

lafriks commented Mar 23, 2021

InCogNiTo124 commented Mar 30, 2022 •

edited

Loading

yctn commented Apr 26, 2022

lunny commented Sep 25, 2022

KN4CK3R Dec 13, 2022

delvh Dec 13, 2022

JakobDev commented Dec 13, 2022

delvh commented Dec 13, 2022

KN4CK3R commented Dec 13, 2022

delvh commented Dec 13, 2022

JakobDev commented Dec 13, 2022

delvh Dec 13, 2022

delvh Dec 13, 2022

delvh Dec 13, 2022

delvh Dec 13, 2022

wolfogre Dec 15, 2022

delvh Dec 13, 2022

wolfogre commented Dec 14, 2022

lunny commented Dec 14, 2022

wolfogre commented Dec 14, 2022 •

edited

Loading

delvh Dec 14, 2022

wolfogre Dec 15, 2022

delvh Dec 14, 2022

wolfogre Dec 15, 2022

delvh Dec 14, 2022

wolfogre Dec 15, 2022

lunny commented Dec 15, 2022

lunny commented Dec 20, 2022

		@@ -1,6 +1,5 @@
		// Copyright 2021 The Gitea Authors. All rights reserved.

	settings.secret_value_content_placeholder = Input any content
	settings.secret_value_content_placeholder = Input any content. Whitespace at the start and end will be omitted.

		OwnerID int64 `xorm:"UNIQUE(owner_repo_name) NOTNULL"`
		RepoID int64 `xorm:"UNIQUE(owner_repo_name) NOTNULL"`

Add Gitea secrets storage and management #14483

Add Gitea secrets storage and management #14483

Conversation

lafriks commented Jan 27, 2021 • edited by lunny Loading

kdumontnu commented Mar 22, 2021 • edited Loading

lafriks commented Mar 23, 2021

InCogNiTo124 commented Mar 30, 2022 • edited Loading

yctn commented Apr 26, 2022

lunny commented Sep 25, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JakobDev commented Dec 13, 2022

delvh commented Dec 13, 2022

KN4CK3R commented Dec 13, 2022

delvh commented Dec 13, 2022

JakobDev commented Dec 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wolfogre commented Dec 14, 2022

lunny commented Dec 14, 2022

wolfogre commented Dec 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lunny commented Dec 15, 2022

lunny commented Dec 20, 2022

lafriks commented Jan 27, 2021 •

edited by lunny

Loading

kdumontnu commented Mar 22, 2021 •

edited

Loading

InCogNiTo124 commented Mar 30, 2022 •

edited

Loading

wolfogre commented Dec 14, 2022 •

edited

Loading