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

Only check at least one email gpg key #2266

Merged
merged 10 commits into from
Sep 5, 2017

Conversation

sapk
Copy link
Member

@sapk sapk commented Aug 6, 2017

Fix #2213 & #2187 by relaxing the control on import by allowing any key with one key at least matching one email of the user.

Commit validation should stilll be good because I use GetUserByEmail to get the user from commiter email and this method need that the email is activated. And add an extra checkup and optimization by checking before checking sign if the emails attached to the key can validate this particular email of commiter.

Still to do :

  • ~~~Add integration tests for validation of commit to check corner case.~~~
Extra : Command line to test this : go test -c code.gitea.io/gitea/integrations -o integrations.sqlite.test -tags 'sqlite' && GITEA_ROOT="$GOPATH/src/code.gitea.io/gitea" GITEA_CONF=integrations/sqlite.ini ./integrations.sqlite.test -test.v -test.run GPG

@sapk sapk force-pushed the only-check-at-least-on-email-gpg-key branch from 1e53d2a to 0492428 Compare August 7, 2017 00:32
@sapk
Copy link
Member Author

sapk commented Aug 7, 2017

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 7, 2017
@sapk sapk force-pushed the only-check-at-least-on-email-gpg-key branch from 0492428 to 9e19506 Compare August 7, 2017 00:40
@sapk sapk changed the title [WIP] Only check at least on email gpg key Only check at least on email gpg key Aug 7, 2017
@sapk sapk changed the title Only check at least on email gpg key Only check at least one email gpg key Aug 7, 2017
@jqs7
Copy link
Contributor

jqs7 commented Aug 19, 2017

the integration tests of pgsql failed because pgsql insert record start with ID 10001:

create pgp key: {"id":10001,"primary_key_id":"","key_id":"38EA3BCED732982C","public_key":"xsBNBFmGVsMBCACuxgZ7W7rI9xN08Y4M7B8yx/6/I4Slm94+wXf8YNRvAyqj30dWVJhyBcnfNRDLKSQp5o/hhfDkCgdqBjLa1PnH
lGS3PXJc0hP/FyYPD2BFvNMPpCYSeu3T1qKSNXm6X0XOWD2LIrdiDC8HaI9FqZVMI/srMK2CF8XCL2m67W1FuoPlWzod5ORy0IZB7spoF0xihmcgnEGElRmdo5w/vkGH8U7Zyn9Eb57UVFeafgeskf4wqB23BjbMdW2YaB+yzMRwYgOnD5lnBD4uqSmvja
V9C0kxn7x+oJkkiRV8/z1cNcO+BaeQAkh/yTTeTzYGSc/ZOqCX1O+NOPgSeixVlqenABEBAAE=","emails":[{"email":"user2@example.com","verified":true}],"subkeys":[{"id":10002,"primary_key_id":"38EA3BCED732982$
","key_id":"70D7C694D17D03AD","public_key":"zsBNBFmGVsMBCADAGba2L6NCOE1i3WIP6CPzbdOoN3gdTfTgccAx9fNeon9jor+3tgEjlo9/6cXiRoksOV6W4wFab/ZwWgwN6JO4CGvZWi7EQwMMMp1E36YTojKQJrcA9UvMnTHulqQQ88F5E8
45DhzFQM3erv42QZZMBAX3kXCgy1GNFocl6tLUvJdEqs+VcJGGANMpmzE4WLa8KhSYnxipwuQ62JBy9R+cHyKTOARk8znRqSu5bT3LtlrZ/HXu+6Oy4+2uCdNzZIh5J5tPS7CPA6ptl88iGVBte/CJ7cjgJWSQqeYp2Y5QvsWAivkQ4Ww9plHbbwV0A2ea
HsjjWzlUl3HoJ/snMOhBABEBAAE=","emails":null,"subkeys":null,"can_sign":true,"can_encrypt_comms":true,"can_encrypt_storage":true,"can_certify":true,"created_at":"2017-08-06T07:37:39+08:00","ex
pires_at":"2019-08-06T07:37:39+08:00"}],"can_sign":true,"can_encrypt_comms":true,"can_encrypt_storage":true,"can_certify":true,"created_at":"2017-08-06T07:37:39+08:00","expires_at":"2019-08-
06T07:37:39+08:00"}

@sapk sapk force-pushed the only-check-at-least-on-email-gpg-key branch from 2292961 to b3cc631 Compare August 20, 2017 23:30
@sapk
Copy link
Member Author

sapk commented Aug 20, 2017

Rebase and I use the ID returned in the list of key to try to get one specific key. So the test are not failing anymore on pgsql that have index not starting at 1.
Thanks @jqs7 for the insigth.

@lunny lunny added this to the 1.3.0 milestone Aug 21, 2017
@strk
Copy link
Member

strk commented Sep 5, 2017

Trusted LGTM, but I don't like those .git objects in the repository (any way to create those repositories from input files, with a script ?)

@tboerger 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 Sep 5, 2017
@lafriks
Copy link
Member

lafriks commented Sep 5, 2017

LGTM

@tboerger 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 Sep 5, 2017
@lunny lunny merged commit 7c417bb into go-gitea:master Sep 5, 2017
@lunny lunny added the type/bug label Sep 5, 2017
@lunny
Copy link
Member

lunny commented Sep 5, 2017

@strk any better idea than put the .git directory on the repo?

@lafriks
Copy link
Member

lafriks commented Sep 5, 2017

@strk Btw just saw that you allow adding gpg key even if no emails is validated/active. Why so why not force at least one activated email?

@lunny
Copy link
Member

lunny commented Sep 5, 2017

@sapk and seems this PR caused all other PRs' tests failed since wrong protected branch settings. I have corrected wrong protected branch, but maybe you can send a PR to fix the tests.

@sapk
Copy link
Member Author

sapk commented Sep 5, 2017

@lunny I will have a look at it.

@strk I could generate the repo. This will need some extra work because we need to have key for signing maybe done in another PR.

@sapk
Copy link
Member Author

sapk commented Sep 8, 2017

Should I backport these to 1.2.0 ? (with #2467)

sapk added a commit to sapk-fork/gitea that referenced this pull request Oct 26, 2017
* Only require one email (possibly not yet validated)

* Update message error and check validation of commit

* Add integrations tests

* Complete integration for import

* Add pre-check/optimization

* Add some test (not finished)

* Finish

* Fix fixtures

* Fix typo

* Don't guess key ID
@lafriks lafriks added backport/done All backports for this PR have been created backport/v1.2 labels Oct 26, 2017
lunny pushed a commit that referenced this pull request Oct 27, 2017
* Only check at least one email gpg key (#2266)

* Only require one email (possibly not yet validated)

* Update message error and check validation of commit

* Add integrations tests

* Complete integration for import

* Add pre-check/optimization

* Add some test (not finished)

* Finish

* Fix fixtures

* Fix typo

* Don't guess key ID

* Make repo private to no interfere with other tests (#2467)

* GPG key email verification no longer case sensitive (#2661) (#2663)

* GPG key email verification no longer case sensitive (#2661)

* case insensitive GPG key email verification now cached (#2661)

Signed-off-by: Julian Scholle <julian.scholle@googlemail.com>
@sapk sapk deleted the only-check-at-least-on-email-gpg-key branch February 13, 2019 08:41
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GPG keys with revoked email addresses cannot be verified
6 participants