Implement GPG api #710

sapk · 2017-01-20T16:30:42Z

In order to implement #425.

I added all the api call that gitea should implement.
This is this a WIP. I will change the db format to extract all data and registrer sub-keys in db in order to not have to rely on parsing again the armored key after. For the moment the armored key is stored in db and parse at presentation.

For the last part (validation of commit ), I will do it in a separate PR.

Related to go-gitea/go-sdk#36

Bug : ~~The public_key part in the respond is not in the good format.~~

bkcsoft · 2017-01-21T02:26:48Z

Will need some tests before it's done as well ;) (We should have a checklist in the PR_Template ;)

lunny · 2017-01-23T01:20:57Z

build failed

sapk · 2017-01-23T01:57:58Z

Not yet ready.
I will stash all the commit when ready for a cleaner history.

bkcsoft

Thoughts?

bkcsoft · 2017-01-23T23:51:25Z

models/gpg_key.go

+	Added             time.Time `xorm:"-"`
+	AddedUnix         int64
+	SubsKey           []*GPGKey `xorm:"-"`
+	Emails            []string


Maybe use []models.EmailAddress ? (even though we don't use all the fields) Since that could also be used to map users to keys

The email storing is the last part to do now.
I was thinking of something like that but didn't know if it is possible that multiple user have the same email ? Maybe a weird case could be that someone import a gpg key with a commun email ...

I'm fairly certain that multiple users can not have the same email (Unique constraint), though there might be a lingering bug there 😉

After searching and testing, I found that github only accept key with a allready verified email for your account. So []models.EmailAddress is definitly perfect for that.

ethantkoenig · 2017-01-29T04:25:35Z

models/gpg_key.go

@@ -0,0 +1,297 @@
+// Copyright 2014 The Gogs Authors. All rights reserved.


2017 The Gitea Authors.

ethantkoenig · 2017-01-29T04:27:36Z

models/gpg_key_test.go

+-----END PGP PUBLIC KEY BLOCK-----`
+
+	key, err := checkArmoredGPGKeyString(testGPGArmor)
+	if err != nil {


If you expect err to be nil, use assert.NoError(t, err) 😃

Will use directly assert.Nil(t, err, "Could not parse a valid GPG armored key")

ethantkoenig · 2017-01-29T04:28:15Z

routers/api/v1/convert/convert.go

@@ -73,6 +73,41 @@ func ToPublicKey(apiLink string, key *models.PublicKey) *api.PublicKey {
 	}
 }

+// ToGPGKey converts models.PublicGPGKey to api.GPGKey


Should the comment say models.GPGKey?

It should be a APIFormat-function on the model itself :)

ethantkoenig · 2017-01-29T04:28:30Z

routers/api/v1/user/gpg_key.go

@@ -0,0 +1,93 @@
+// Copyright 2015 The Gogs Authors. All rights reserved.


lunny · 2017-02-05T14:26:14Z

Conflicted and any update?

lunny · 2017-02-11T07:37:54Z

Just move this to v1.2

lunny · 2017-03-08T00:59:49Z

models/gpg_key.go

@@ -0,0 +1,273 @@
+// Copyright 2017 The Gogs Authors. All rights reserved.


The Gitea :)

sapk · 2017-03-08T01:14:40Z

So i think i have done all the API part. I will rebase/squash every things to make it less bad for complete review ^^.

lunny · 2017-03-08T01:27:16Z

So cool!

sapk · 2017-03-08T01:36:24Z

Oups i must I done something wrong with my squash ...

sapk · 2017-03-10T02:02:49Z

Doing a own review of my code, I did some adjustment (very little cleaning and refactor) and found that i forgot expiration date of key.

lunny · 2017-03-10T03:06:11Z

It's ready to review?

lunny · 2017-03-11T01:19:51Z

models/gpg_key.go

+	defer sessionRelease(sess)
+	sess.Begin()
+	for _, subkey := range key.SubsKey {
+		if err := addGPGKey(sess, subkey); err != nil {


sess.Inert(key.SubsKey) just ok?

Yes that was because I started with the same structure has https://github.com/go-gitea/gitea/blob/master/models/ssh_key.go#L396 but all checks on data are made in parseGPGKey so no need for addGPGKey.

Or maybe a other solution could be to not use AfterInsert but addGPGKey recursively in addGPGKey for subkeys ? This would be more simple.

I have gone with that even for delete case this permit that all the database logic are separated. Let me know if you want a other/better solution.

lunny · 2017-03-11T01:21:32Z

models/gpg_key.go

+	}
+	return base64.StdEncoding.EncodeToString(w.Bytes()), nil
+}
+func parseSubGPGKey(ownerID int64, primaryID string, pubkey *packet.PublicKey, expiry time.Time) (*GPGKey, error) {


lunny · 2017-03-11T01:21:52Z

models/gpg_key.go

+		CanCertify:        pubkey.PubKeyAlgo.CanSign(),
+	}, nil
+}
+func parseGPGKey(ownerID int64, e *openpgp.Entity) (*GPGKey, error) {


lunny · 2017-03-11T01:22:57Z

models/gpg_key.go

+
+	//Add subkeys to remove
+	subkeys := make([]*GPGKey, 0, 5)
+	x.Where("primary_key_id=?", key.KeyID).Find(&subkeys)


err handler

lunny · 2017-03-11T01:24:33Z

models/gpg_key.go

+	}
+
+	//Add subkeys to remove
+	subkeys := make([]*GPGKey, 0, 5)


var subkeyIDs []int64 err = x.Table("gpgkey").Where("primary_key_id=?", key.KeyID).Find(& subkeyIDs)

lunny · 2017-03-13T08:36:34Z

routers/api/v1/convert/convert.go

+			Created:      k.Created,
+			Expires:      k.Expired,
+			//Emails:            emails,
+			//SubsKey:           subkeys,


Why comment these?

Those are empy in subkeys so they can be remove. I was keeping them commented to show they are not set explicitly but not really usefull.

sapk · 2017-03-13T21:04:42Z

rebase

bkcsoft · 2017-03-16T00:31:50Z

LGTM

bkcsoft · 2017-03-16T00:32:43Z

@sapk Is the new go-sdk vendored?

lunny · 2017-03-16T00:47:10Z

LGTM except @bkcsoft 's comment. @ethantkoenig need your confirmation since you have reviewed this.

lunny · 2017-03-16T01:28:22Z

:( forgot. @sapk please take a look @bkcsoft 's comment.

sapk · 2017-03-16T10:15:38Z

It was allready vendor in master if I remember.

sapk · 2017-03-16T10:29:16Z

To be clear go-gitea/go-sdk@f5de21c is vendor. It's the only needed for this part.

go-gitea/go-sdk@4878372 only add missing request but don't change data format.

go-gitea/go-sdk@9da3bab Add commit verification but is not needed for api managing keys. It will be needed in commit verification.

Adds `[repository].DOWNLOAD_OR_CLONE_METHODS` (defaulting to "download-zip,download-targz,download-bundle,vscode-clone"), which lets an instance administrator override the additional clone methods displayed on the repository home view. This is purely display-only, the clone methods not listed here are still available, unless disabled elsewhere. They're just not displayed. Fixes go-gitea#710. Signed-off-by: Gergely Nagy <forgejo@gergo.csillger.hu> (cherry picked from commit 2aadcf4946e48ee43800568fe705d00a062c42bf) (cherry picked from commit 42ac34fbf9105eed27ee687b305a85515270f0cc) (cherry picked from commit bd231b02450212aca6be775663c3d24ddf19f990) (cherry picked from commit 3d3366dbbee37621fc665e557a4a87bf08104375) (cherry picked from commit 0157fb9b88fd50832c07b06c11c8dba6e059a465) (cherry picked from commit bee88f6a8309c6f9aeba1522383d77f08e5a4d2d)

bkcsoft added modifies/api This PR adds API routes or modifies them type/feature Completely new functionality. Can only be merged if feature freeze is not active. labels Jan 21, 2017

tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 21, 2017

lunny added this to the 1.1.0 milestone Jan 21, 2017

lunny added the pr/wip This PR is not ready for review label Jan 23, 2017

bkcsoft mentioned this pull request Jan 23, 2017

[WIP] GPG Verification (#425) #476

Closed

bkcsoft reviewed Jan 24, 2017

View reviewed changes

ethantkoenig suggested changes Jan 29, 2017

View reviewed changes

lunny modified the milestones: 1.2.0, 1.1.0 Feb 11, 2017

lunny reviewed Mar 8, 2017

View reviewed changes

models/gpg_key.go Outdated

@@ -0,0 +1,273 @@

// Copyright 2017 The Gogs Authors. All rights reserved.

Copy link

Member

lunny Mar 8, 2017

Choose a reason for hiding this comment

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

The Gitea :)

sapk changed the title ~~WIP - Implement GPG api~~ Implement GPG api Mar 8, 2017

lunny removed the pr/wip This PR is not ready for review label Mar 8, 2017

sapk closed this Mar 8, 2017

sapk force-pushed the add-gpg-api branch from c24adaa to 7ed00b6 Compare March 8, 2017 01:35

sapk reopened this Mar 8, 2017

sapk force-pushed the add-gpg-api branch from 9943538 to 06314a3 Compare March 8, 2017 01:56

This was referenced Mar 8, 2017

Add missing ListGPGKeys of a user go-gitea/go-sdk#44

Merged

GPG commit validation #1150

Merged

sapk force-pushed the add-gpg-api branch from ad2928d to 8f0e653 Compare March 10, 2017 02:05

lunny reviewed Mar 11, 2017

View reviewed changes

lunny requested changes Mar 11, 2017

View reviewed changes

lunny requested changes Mar 13, 2017

View reviewed changes

lunny approved these changes Mar 13, 2017

View reviewed changes

sapk and others added 4 commits March 13, 2017 21:59

Implement GPG API

44c85b9

Better handle error

46c43cd

Apply review recommendation + simplify database operations

8ed52ed

Remove useless comments

47f46c4

sapk force-pushed the add-gpg-api branch from b594263 to 47f46c4 Compare March 13, 2017 21:00

tboerger added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 16, 2017

tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 16, 2017

ethantkoenig approved these changes Mar 16, 2017

View reviewed changes

lunny merged commit ca1c3f1 into go-gitea:master Mar 16, 2017

sapk mentioned this pull request Mar 17, 2017

Rework SSH key management UI to add GPG #1293

Merged

5 tasks

sapk deleted the add-gpg-api branch April 28, 2017 10:04

guyzmo mentioned this pull request May 9, 2017

Implement GPG keys management guyzmo/git-repo#155

Open

go-gitea locked and limited conversation to collaborators Nov 23, 2020

		@@ -0,0 +1,297 @@
		// Copyright 2014 The Gogs Authors. All rights reserved.

		@@ -0,0 +1,93 @@
		// Copyright 2015 The Gogs Authors. All rights reserved.

		@@ -0,0 +1,273 @@
		// Copyright 2017 The Gogs Authors. All rights reserved.

Implement GPG api #710

Implement GPG api #710

Conversation

sapk commented Jan 20, 2017 • edited Loading

bkcsoft commented Jan 21, 2017

lunny commented Jan 23, 2017

sapk commented Jan 23, 2017

bkcsoft left a 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

sapk Mar 7, 2017 • 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

Choose a reason for hiding this comment

lunny commented Feb 5, 2017

lunny commented Feb 11, 2017

Choose a reason for hiding this comment

sapk commented Mar 8, 2017

lunny commented Mar 8, 2017

sapk commented Mar 8, 2017

sapk commented Mar 10, 2017

lunny commented Mar 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sapk Mar 11, 2017 • 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

Choose a reason for hiding this comment

sapk commented Mar 13, 2017

bkcsoft commented Mar 16, 2017

bkcsoft commented Mar 16, 2017

lunny commented Mar 16, 2017

lunny commented Mar 16, 2017

sapk commented Mar 16, 2017

sapk commented Mar 16, 2017

sapk commented Jan 20, 2017 •

edited

Loading

sapk Mar 7, 2017 •

edited

Loading

sapk Mar 11, 2017 •

edited

Loading