Show Signer in commit lists and add basic trust #10425

zeripath · 2020-02-23T12:03:07Z

Show the avatar of the signer in the commit list pages as we do not
enforce that the signer is an author or committer. This makes it
clearer who has signed the commit.

Also display commits signed by non-members differently from
members and in particular make it clear when a non-member signer
is different from the committer to help reduce the risk of
spoofing.

Signed-off-by: Andrew Thornton [email protected]

Screenshots

Verified Commits

Untrusted Commits (signed by committer who is not a member of the repository)

Unmatched Commits (signed by committer who is not a member of the repository and the committer and the signer are not matched)

Bad signature

Show the avatar of the signer in the commit list pages as we do not enforce that the signer is an author or committer. This makes it clearer who has signed the commit. Also display commits signed by non-members differently from members and in particular make it clear when a non-member signer is different from the committer to help reduce the risk of spoofing. Signed-off-by: Andrew Thornton <[email protected]>

6543 · 2020-02-23T14:25:06Z

can you add a screenshot before/after?

web_src/less/_repository.less

Signed-off-by: Andrew Thornton <[email protected]>

zeripath · 2020-02-23T15:50:11Z

@6543 Screenshots are up

silverwind · 2020-02-23T16:02:22Z

Dark theme will probably need a few fixes as this introduces a lot of bright color rules.

codecov-io · 2020-02-23T16:14:02Z

Codecov Report

Merging #10425 into master will decrease coverage by 0.01%.
The diff coverage is 34.65%.

@@            Coverage Diff             @@
##           master   #10425      +/-   ##
==========================================
- Coverage   43.68%   43.67%   -0.02%     
==========================================
  Files         586      586              
  Lines       81467    81473       +6     
==========================================
- Hits        35591    35584       -7     
- Misses      41465    41478      +13     
  Partials     4411     4411

Impacted Files	Coverage Δ
models/gpg_key.go	`53.42% <ø> (+0.09%)`	⬆️
routers/api/v1/repo/pull.go	`34.63% <ø> (ø)`	⬆️
modules/auth/admin.go	`0% <0%> (ø)`	⬆️
routers/admin/admin.go	`10.29% <0%> (ø)`	⬆️
routers/api/v1/repo/issue_subscription.go	`0% <0%> (ø)`	⬆️
models/login_source.go	`25.45% <0%> (ø)`	⬆️
routers/repo/view.go	`36.82% <0%> (ø)`	⬆️
modules/auth/pam/pam_stub.go	`0% <0%> (ø)`	⬆️
routers/api/v1/admin/user.go	`30.09% <0%> (ø)`	⬆️
routers/user/auth.go	`11.4% <0%> (ø)`	⬆️
... and 53 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af172ef...ab8e196. Read the comment docs.

Signed-off-by: Andrew Thornton <[email protected]>

zeripath · 2020-02-23T19:51:02Z

I've updated colors for dark theme and adjusted the less

models/gpg_key.go

models/repo_collaboration.go

routers/repo/view.go

Signed-off-by: Andrew Thornton <[email protected]>

silverwind · 2020-02-23T22:24:52Z

Small request: Can you make the red/green/yellow badges' text color either match the border color (except yellow on light theme, go orange there)? I think they would look better with a text color instead of dull grey.

zeripath · 2020-02-23T23:00:36Z

@silverwind I don't understand.

models/gpg_key.go

silverwind · 2020-02-23T23:16:24Z

I don't understand

Badge text color is always grey (same as unsigned)

While I think they would look better with some color (same as border here):

Can of course be done in a separate PR.

silverwind · 2020-02-23T23:22:26Z

There is a slight vertical centering error with the hash text here:

Can't investigate now cause the build fails, but I think you can probably just do flexbox centering inside the tag.

models/gpg_key.go

zeripath · 2020-02-23T23:28:51Z

So I'm not a fan of changing the colour of the shas - I think they would stand out too much.

In terms of the slight vertical centering issue it's because of the height of the avatar. I think if we stick a few px padding-top that would sort it.

models/gpg_key.go

zeripath · 2020-02-26T22:15:12Z

I didn't adjust the view_list.tmpl - I've done that now. That should fix the problem.

zeripath · 2020-02-26T22:18:00Z

@lafriks I'm not sure how others do this - we should probably extend this functionality to include repository specific pubkeyrings +/- signed pubkeys.

lafriks · 2020-02-26T22:19:49Z

Why not use server wide keyring with also historical keys?

silverwind · 2020-02-26T22:31:16Z

I didn't adjust the view_list.tmpl - I've done that now. That should fix the problem.

It does not seem to have helped. HTML on repo page:

zeripath · 2020-02-26T22:34:29Z

Why not use server wide keyring with also historical keys?

It's an option, we probably want per repository settings though.

I think these are for another PR - this one just establishes a very simple trust model - we can codify more complex ones in time.

zeripath · 2020-02-26T22:37:34Z

I didn't adjust the view_list.tmpl - I've done that now. That should fix the problem.

It does not seem to have helped. HTML on repo page:

Weird it seemed to fix things over here.

I'm out of time to look at this again tonight - it's bed time for me - feel free to send a pr to my pr or suggest changes if you figure out how to fix things.

6543

I'm fine with the go part - I'll leafe js/css part to silverwind

silverwind · 2020-02-26T22:58:39Z

Should really un-nest and share .sha.label styles. Maybe I'll have a look later.

guillep2k · 2020-02-27T00:17:54Z

I think this PR is a huge improvement, but I see the problem with the updated keys. I agree it can be refined in a later PR.

We've got a commit table: could that help to hold the "historic"/calculated value? For migrations, the user could choose what to do with previous commits (mark them invalid/orange or "undefined"/gray).

guillep2k

Only a couple of nits. 👍

models/gpg_key.go

routers/repo/view.go

silverwind · 2020-02-27T19:18:49Z

There's two <i> icons (lock and cog) here on the lock, is that intentional?

zeripath · 2020-02-27T19:25:38Z

@silverwind I fixed it! -- oops I think that this little fix didn't make it in! I'll open another PR for it.

zeripath · 2020-02-27T19:26:41Z

@silverwind yes that's intentional if that's the server key.

silverwind · 2020-02-27T19:28:18Z

The alignment issue with the icons still persists for me. It's because there is a extra <div> in the template on the commits list that is absent on the repo page.

I think the .sha.label needs to be extracted to its own template file and included in bothcommits_list.tmpl and view_list.tmpl. Afterwards, un-nest the CSS so it can be used in more places in the future.

zeripath · 2020-02-27T19:30:12Z

diff --git a/templates/repo/view_list.tmpl b/templates/repo/view_list.tmpl
index c296eb7be..a1c8c133e 100644
--- a/templates/repo/view_list.tmpl
+++ b/templates/repo/view_list.tmpl
@@ -16,10 +16,11 @@
                                        {{end}}
                                {{end}}
                                <a rel="nofollow" class="ui sha label {{if .LatestCommit.Signature}} isSi
-                                               <span class="shortsha">{{ShortSha .LatestCommit.ID.String
+                                       <span class="shortsha">{{ShortSha .LatestCommit.ID.String}}</span
+                                       <div class="ui detail icon button">
                                                {{if .LatestCommit.Signature}}
                                                        {{if .LatestCommitVerification.Verified}}
-                                                               <div class="ui detail icon button" title=
+                                                               <div title="{{if eq .LatestCommitVerifica
                                                                        {{if ne .LatestCommitVerification
                                                                                <i class="lock icon"></i>
                                                                                <img class="ui signature 
@@ -32,11 +33,10 @@
                                                                        {{end}}
                                                                </div>
                                                        {{else}}
-                                                               <div class="ui detail icon button">
-                                                                       <i title="{{$.i18n.Tr .LatestComm
-                                                               </div>
+                                                               <i title="{{$.i18n.Tr .LatestCommitVerifi
                                                        {{end}}
                                                {{end}}
+                                       </div>
                                </a>
                                {{template "repo/commit_status" .LatestCommitStatus}}
                                {{ $commitLink:= printf "%s/commit/%s" .RepoLink .LatestCommit.ID }}

silverwind · 2020-02-27T19:34:22Z

Yes, that missing <div class="ui detail icon button"> is the issue.

@silverwind yes that's intentional if that's the server key.

Would move the cog 1px (or .5px) to the top so it's "more" centered.

Backport go-gitea#10425 Backport go-gitea#10511 * Show Signer in commit lists and add basic trust (go-gitea#10425) Show the avatar of the signer in the commit list pages as we do not enforce that the signer is an author or committer. This makes it clearer who has signed the commit. Also display commits signed by non-members differently from members and in particular make it clear when a non-member signer is different from the committer to help reduce the risk of spoofing. Signed-off-by: Andrew Thornton <[email protected]> Fix the signing icon in the view_list.tmpl page (go-gitea#10511) Co-Authored-By: silverwind <[email protected]> Co-authored-by: guillep2k <[email protected]>

Backport #10425 Backport #10511 * Show Signer in commit lists and add basic trust (#10425) Show the avatar of the signer in the commit list pages as we do not enforce that the signer is an author or committer. This makes it clearer who has signed the commit. Also display commits signed by non-members differently from members and in particular make it clear when a non-member signer is different from the committer to help reduce the risk of spoofing. Signed-off-by: Andrew Thornton <[email protected]> Fix the signing icon in the view_list.tmpl page (#10511) Co-Authored-By: silverwind <[email protected]> Co-authored-by: guillep2k <[email protected]>

zeripath added type/bug type/feature Completely new functionality. Can only be merged if feature freeze is not active. labels Feb 23, 2020

zeripath added this to the 1.12.0 milestone Feb 23, 2020

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 23, 2020

silverwind reviewed Feb 23, 2020

View reviewed changes

web_src/less/_repository.less Outdated Show resolved Hide resolved

ensure orange text and background is available

04eda8b

Signed-off-by: Andrew Thornton <[email protected]>

zeripath added 4 commits February 23, 2020 19:10

adjust colors

ed3ee73

Signed-off-by: Andrew Thornton <[email protected]>

Merge branch 'master' into show-signer

173a073

placate latest ci

669bb72

Signed-off-by: Andrew Thornton <[email protected]>

Merge branch 'show-signer' of github.com:zeripath/gitea into show-signer

9b3f985

guillep2k reviewed Feb 23, 2020

View reviewed changes

models/gpg_key.go Outdated Show resolved Hide resolved

models/repo_collaboration.go Show resolved Hide resolved

routers/repo/view.go Outdated Show resolved Hide resolved

reduce code copying

148bab6

Signed-off-by: Andrew Thornton <[email protected]>

Update gpg_key.go

6944077

zeripath commented Feb 23, 2020

View reviewed changes

models/gpg_key.go Show resolved Hide resolved

Update models/gpg_key.go

c6f7264

zeripath commented Feb 23, 2020

View reviewed changes

models/gpg_key.go Outdated Show resolved Hide resolved

zeripath commented Feb 23, 2020

View reviewed changes

models/gpg_key.go Outdated Show resolved Hide resolved

Apply suggestions from code review

1257233

zeripath commented Feb 23, 2020

View reviewed changes

models/gpg_key.go Outdated Show resolved Hide resolved

6543 approved these changes Feb 26, 2020

View reviewed changes

GiteaBot 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 Feb 26, 2020

guillep2k approved these changes Feb 27, 2020

View reviewed changes

models/gpg_key.go Show resolved Hide resolved

routers/repo/view.go Outdated Show resolved Hide resolved

GiteaBot 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 Feb 27, 2020

zeripath added 2 commits February 27, 2020 18:16

Merge remote-tracking branch 'origin/master' into show-signer

d1badf8

as per @guillep2k

ab8e196

guillep2k merged commit 90919bb into go-gitea:master Feb 27, 2020

zeripath mentioned this pull request Feb 27, 2020

Fix the signing icon in the view_list.tmpl page #10511

Merged

zeripath mentioned this pull request Feb 28, 2020

Show Signer in commit lists and add basic trust (#10425 & #10511) #10524

Merged

zeripath added backport/v1.11 backport/done All backports for this PR have been created labels Feb 28, 2020

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show Signer in commit lists and add basic trust #10425

Show Signer in commit lists and add basic trust #10425

zeripath commented Feb 23, 2020 •

edited

Loading

6543 commented Feb 23, 2020

zeripath commented Feb 23, 2020

silverwind commented Feb 23, 2020

codecov-io commented Feb 23, 2020 •

edited

Loading

zeripath commented Feb 23, 2020

silverwind commented Feb 23, 2020

zeripath commented Feb 23, 2020

silverwind commented Feb 23, 2020 •

edited

Loading

silverwind commented Feb 23, 2020 •

edited

Loading

zeripath commented Feb 23, 2020

zeripath commented Feb 26, 2020

zeripath commented Feb 26, 2020

lafriks commented Feb 26, 2020

silverwind commented Feb 26, 2020

zeripath commented Feb 26, 2020

zeripath commented Feb 26, 2020

6543 left a comment

silverwind commented Feb 26, 2020

guillep2k commented Feb 27, 2020

guillep2k left a comment

silverwind commented Feb 27, 2020

zeripath commented Feb 27, 2020 •

edited

Loading

zeripath commented Feb 27, 2020

silverwind commented Feb 27, 2020 •

edited

Loading

zeripath commented Feb 27, 2020

silverwind commented Feb 27, 2020 •

edited

Loading

Show Signer in commit lists and add basic trust #10425

Show Signer in commit lists and add basic trust #10425

Conversation

zeripath commented Feb 23, 2020 • edited Loading

Screenshots

Verified Commits

Untrusted Commits (signed by committer who is not a member of the repository)

Unmatched Commits (signed by committer who is not a member of the repository and the committer and the signer are not matched)

Bad signature

6543 commented Feb 23, 2020

zeripath commented Feb 23, 2020

silverwind commented Feb 23, 2020

codecov-io commented Feb 23, 2020 • edited Loading

Codecov Report

zeripath commented Feb 23, 2020

silverwind commented Feb 23, 2020

zeripath commented Feb 23, 2020

silverwind commented Feb 23, 2020 • edited Loading

silverwind commented Feb 23, 2020 • edited Loading

zeripath commented Feb 23, 2020

zeripath commented Feb 26, 2020

zeripath commented Feb 26, 2020

lafriks commented Feb 26, 2020

silverwind commented Feb 26, 2020

zeripath commented Feb 26, 2020

zeripath commented Feb 26, 2020

6543 left a comment

Choose a reason for hiding this comment

silverwind commented Feb 26, 2020

guillep2k commented Feb 27, 2020

guillep2k left a comment

Choose a reason for hiding this comment

silverwind commented Feb 27, 2020

zeripath commented Feb 27, 2020 • edited Loading

zeripath commented Feb 27, 2020

silverwind commented Feb 27, 2020 • edited Loading

zeripath commented Feb 27, 2020

silverwind commented Feb 27, 2020 • edited Loading

zeripath commented Feb 23, 2020 •

edited

Loading

codecov-io commented Feb 23, 2020 •

edited

Loading

silverwind commented Feb 23, 2020 •

edited

Loading

silverwind commented Feb 23, 2020 •

edited

Loading

zeripath commented Feb 27, 2020 •

edited

Loading

silverwind commented Feb 27, 2020 •

edited

Loading

silverwind commented Feb 27, 2020 •

edited

Loading