Skip to content

Show Signer in commit lists and add basic trust #10425

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

Merged
merged 19 commits into from
Feb 27, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
31 changes: 28 additions & 3 deletions models/gpg_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,7 @@ type CommitVerification struct {
CommittingUser *User
SigningEmail string
SigningKey *GPGKey
TrustStatus string
}

// SignCommit represents a commit with validation of signature.
Expand Down Expand Up @@ -759,17 +760,41 @@ func verifyWithGPGSettings(gpgSettings *git.GPGSettings, sig *packet.Signature,
}

// ParseCommitsWithSignature checks if signaute of commits are corresponding to users gpg keys.
func ParseCommitsWithSignature(oldCommits *list.List) *list.List {
func ParseCommitsWithSignature(oldCommits *list.List, repository *Repository) *list.List {
var (
newCommits = list.New()
e = oldCommits.Front()
)
memberMap := map[int64]bool{}

for e != nil {
c := e.Value.(UserCommit)
newCommits.PushBack(SignCommit{
signCommit := SignCommit{
UserCommit: &c,
Verification: ParseCommitWithSignature(c.Commit),
})
}

if signCommit.Verification.Verified {
signCommit.Verification.TrustStatus = "trusted"
if signCommit.Verification.SigningUser.ID != 0 {
isMember, has := memberMap[signCommit.Verification.SigningUser.ID]
if !has {
// We can ignore the error here as isMember would return false and so the user would be listed as untrusted
isMember, _ = repository.IsOwnerMemberCollaborator(signCommit.Verification.SigningUser.ID)
memberMap[signCommit.Verification.SigningUser.ID] = isMember
}
if !isMember {
signCommit.Verification.TrustStatus = "untrusted"
if signCommit.Verification.CommittingUser.ID != signCommit.Verification.SigningUser.ID {
// The committing user and the signing user are not the same and are not the default key
// This should be marked as questionable unless the signing user is a collaborator/team member etc.
signCommit.Verification.TrustStatus = "unmatched"
}
}
}
}

newCommits.PushBack(signCommit)
e = e.Next()
}
return newCommits
Expand Down
18 changes: 18 additions & 0 deletions models/repo_collaboration.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,3 +210,21 @@ func (repo *Repository) getRepoTeams(e Engine) (teams []*Team, err error) {
func (repo *Repository) GetRepoTeams() ([]*Team, error) {
return repo.getRepoTeams(x)
}

// IsOwnerMemberCollaborator checks if a provided user is the owner, a collaborator or a member of a team in a repository
func (repo *Repository) IsOwnerMemberCollaborator(userID int64) (bool, error) {
if repo.OwnerID == userID {
return true, nil
}
teamMember, err := x.Join("INNER", "team_repo", "team_repo.team_id = team_user.team_id").
Where("team_repo.repo_id = ?", repo.ID).
And("team_user.uid = ?", userID).Table("team_user").Exist(&TeamUser{})
if err != nil {
return false, err
}
if teamMember {
return true, nil
}

return x.Get(&Collaboration{RepoID: repo.ID, UserID: userID})
}
2 changes: 2 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,8 @@ commits.date = Date
commits.older = Older
commits.newer = Newer
commits.signed_by = Signed by
commits.signed_by_untrusted_user = Signed by untrusted user
commits.signed_by_untrusted_user_unmatched = Signed by untrusted user who does not match committer
commits.gpg_key_id = GPG Key ID

ext_issues = Ext. Issues
Expand Down
2 changes: 0 additions & 2 deletions routers/private/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,8 @@ func readAndVerifyCommit(sha string, repo *git.Repository, env []string) error {
if err != nil {
return err
}
log.Info("have commit %s", commit.ID.String())
verification := models.ParseCommitWithSignature(commit)
if !verification.Verified {
log.Info("unverified commit %s", commit.ID.String())
cancel()
return &errUnverifiedCommit{
commit.ID.String(),
Expand Down
25 changes: 21 additions & 4 deletions routers/repo/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func Commits(ctx *context.Context) {
return
}
commits = models.ValidateCommitsWithEmails(commits)
commits = models.ParseCommitsWithSignature(commits)
commits = models.ParseCommitsWithSignature(commits, ctx.Repo.Repository)
commits = models.ParseCommitsWithStatus(commits, ctx.Repo.Repository)
ctx.Data["Commits"] = commits

Expand Down Expand Up @@ -139,7 +139,7 @@ func SearchCommits(ctx *context.Context) {
return
}
commits = models.ValidateCommitsWithEmails(commits)
commits = models.ParseCommitsWithSignature(commits)
commits = models.ParseCommitsWithSignature(commits, ctx.Repo.Repository)
commits = models.ParseCommitsWithStatus(commits, ctx.Repo.Repository)
ctx.Data["Commits"] = commits

Expand Down Expand Up @@ -185,7 +185,7 @@ func FileHistory(ctx *context.Context) {
return
}
commits = models.ValidateCommitsWithEmails(commits)
commits = models.ParseCommitsWithSignature(commits)
commits = models.ParseCommitsWithSignature(commits, ctx.Repo.Repository)
commits = models.ParseCommitsWithStatus(commits, ctx.Repo.Repository)
ctx.Data["Commits"] = commits

Expand Down Expand Up @@ -269,12 +269,29 @@ func Diff(ctx *context.Context) {
setPathsCompareContext(ctx, parentCommit, commit, headTarget)
ctx.Data["Title"] = commit.Summary() + " · " + base.ShortSha(commitID)
ctx.Data["Commit"] = commit
ctx.Data["Verification"] = models.ParseCommitWithSignature(commit)
verification := models.ParseCommitWithSignature(commit)
ctx.Data["Verification"] = verification
ctx.Data["Author"] = models.ValidateCommitWithEmail(commit)
ctx.Data["Diff"] = diff
ctx.Data["Parents"] = parents
ctx.Data["DiffNotAvailable"] = diff.NumFiles() == 0

verification.TrustStatus = "trusted"
if verification.Verified && verification.SigningUser.ID != 0 {
trusted, err := ctx.Repo.Repository.IsOwnerMemberCollaborator(verification.SigningUser.ID)
if err != nil {
ctx.ServerError("IsOwnerMemberCollaborator", err)
return
}
if !trusted {
verification.TrustStatus = "untrusted"
if verification.CommittingUser.ID != verification.SigningUser.ID {
// The committing user and the signing user are not the same and are not the default key
// This should be marked as questionable unless the signing user is a collaborator/team member etc.
verification.TrustStatus = "unmatched"
}
}
}
note := &git.Note{}
err = git.GetNote(ctx.Repo.GitRepo, commitID, note)
if err == nil {
Expand Down
2 changes: 1 addition & 1 deletion routers/repo/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ func PrepareCompareDiff(
}

compareInfo.Commits = models.ValidateCommitsWithEmails(compareInfo.Commits)
compareInfo.Commits = models.ParseCommitsWithSignature(compareInfo.Commits)
compareInfo.Commits = models.ParseCommitsWithSignature(compareInfo.Commits, headRepo)
compareInfo.Commits = models.ParseCommitsWithStatus(compareInfo.Commits, headRepo)
ctx.Data["Commits"] = compareInfo.Commits
ctx.Data["CommitCount"] = compareInfo.Commits.Len()
Expand Down
2 changes: 1 addition & 1 deletion routers/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ func ViewPullCommits(ctx *context.Context) {
ctx.Data["Reponame"] = ctx.Repo.Repository.Name
commits = prInfo.Commits
commits = models.ValidateCommitsWithEmails(commits)
commits = models.ParseCommitsWithSignature(commits)
commits = models.ParseCommitsWithSignature(commits, ctx.Repo.Repository)
commits = models.ParseCommitsWithStatus(commits, ctx.Repo.Repository)
ctx.Data["Commits"] = commits
ctx.Data["CommitCount"] = commits.Len()
Expand Down
20 changes: 19 additions & 1 deletion routers/repo/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,25 @@ func renderDirectory(ctx *context.Context, treeLink string) {
// Show latest commit info of repository in table header,
// or of directory if not in root directory.
ctx.Data["LatestCommit"] = latestCommit
ctx.Data["LatestCommitVerification"] = models.ParseCommitWithSignature(latestCommit)
verification := models.ParseCommitWithSignature(latestCommit)

ctx.Data["LatestCommitVerification"] = verification
verification.TrustStatus = "trusted"
if verification.Verified && verification.SigningUser.ID != 0 {
trusted, err := ctx.Repo.Repository.IsOwnerMemberCollaborator(verification.SigningUser.ID)
if err != nil {
ctx.ServerError("IsOwnerMemberCollaborator", err)
return
}
if !trusted {
verification.TrustStatus = "untrusted"
if verification.CommittingUser.ID != verification.SigningUser.ID {
// The committing user and the signing user are not the same and are not the default key
// This should be marked as questionable unless the signing user is a collaborator/team member etc.
verification.TrustStatus = "unmatched"
}
}
}
ctx.Data["LatestCommitUser"] = models.ValidateCommitWithEmail(latestCommit)

statuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository, ctx.Repo.Commit.ID.String(), 0)
Expand Down
2 changes: 1 addition & 1 deletion routers/repo/wiki.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry)
return nil, nil
}
commitsHistory = models.ValidateCommitsWithEmails(commitsHistory)
commitsHistory = models.ParseCommitsWithSignature(commitsHistory)
commitsHistory = models.ParseCommitsWithSignature(commitsHistory, ctx.Repo.Repository)

ctx.Data["Commits"] = commitsHistory

Expand Down
10 changes: 8 additions & 2 deletions templates/repo/commit_page.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,14 @@
{{if .Verification.Verified }}
<div class="ui bottom attached positive message">
{{if ne .Verification.SigningUser.ID 0}}
<i class="green lock icon"></i>
<span>{{.i18n.Tr "repo.commits.signed_by"}}:</span>
<i class="{{if eq .Verification.TrustStatus "trusted"}}green{{else if eq .Verification.TrustStatus "untrusted"}}yellow{{else}}orange{{end}} lock icon"></i>
{{if eq .Verification.TrustStatus "trusted"}}
<span>{{.i18n.Tr "repo.commits.signed_by"}}:</span>
{{else if eq .Verification.TrustStatus "untrusted"}}
<span class="ui text yellow">{{.i18n.Tr "repo.commits.signed_by_untrusted_user"}}:</span>
{{else}}
<span class="ui text orange" title="{{.i18n.Tr "repo.commits.signed_by_untrusted_user_unmatched"}}">{{.i18n.Tr "repo.commits.signed_by_untrusted_user"}}:</span>
{{end}}
<img class="ui avatar image" src="{{.Verification.SigningUser.RelAvatarLink}}" />
<a href="{{.Verification.SigningUser.HomeLink}}"><strong>{{.Verification.SigningUser.Name}}</strong></a> <{{.Verification.SigningEmail}}>
<span class="pull-right"><span>{{.i18n.Tr "repo.commits.gpg_key_id"}}:</span> {{.Verification.SigningKey.KeyID}}</span>
Expand Down
28 changes: 19 additions & 9 deletions templates/repo/commits_list.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,13 @@
{{if .Signature}}
{{$class = (printf "%s%s" $class " isSigned")}}
{{if .Verification.Verified}}
{{$class = (printf "%s%s" $class " isVerified")}}
{{if eq .Verification.TrustStatus "trusted"}}
{{$class = (printf "%s%s" $class " isVerified")}}
{{else if eq .Verification.TrustStatus "untrusted"}}
{{$class = (printf "%s%s" $class " isVerifiedUntrusted")}}
{{else}}
{{$class = (printf "%s%s" $class " isVerifiedUnmatched")}}
{{end}}
{{else if .Verification.Warning}}
{{$class = (printf "%s%s" $class " isWarning")}}
{{end}}
Expand All @@ -42,14 +48,18 @@
{{if .Signature}}
<div class="ui detail icon button">
{{if .Verification.Verified}}
{{if ne .Verification.SigningUser.ID 0}}
<i title="{{.Verification.Reason}}" class="lock green icon"></i>
{{else}}
<i title="{{.Verification.Reason}}" class="icons">
<i class="green lock icon"></i>
<i class="tiny inverted cog icon centerlock"></i>
</i>
{{end}}
<div title="{{if eq .Verification.TrustStatus "trusted"}}{{else if eq .Verification.TrustStatus "untrusted"}}{{$.i18n.Tr "repo.commits.signed_by_untrusted_user"}}: {{else}}{{$.i18n.Tr "repo.commits.signed_by_untrusted_user_unmatched"}}: {{end}}{{.Verification.Reason}}">
{{if ne .Verification.SigningUser.ID 0}}
<i class="lock {{if eq .Verification.TrustStatus "trusted"}}green{{else if eq .Verification.TrustStatus "untrusted"}}yellow{{else}}orange{{end}} icon"></i>
<img class="ui signature avatar image" src="{{.Verification.SigningUser.RelAvatarLink}}" />
{{else}}
<i title="{{.Verification.Reason}}" class="icons">
<i class="green lock icon"></i>
<i class="tiny inverted cog icon centerlock"></i>
</i>
<img class="ui signature avatar image" src="{{AvatarLink .Verification.SigningEmail}}" />
{{end}}
</div>
{{else if .Verification.Warning}}
<i title="{{$.i18n.Tr .Verification.Reason}}" class="red unlock icon"></i>
{{else}}
Expand Down
25 changes: 18 additions & 7 deletions templates/repo/view_list.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,27 @@
<strong>{{.LatestCommit.Author.Name}}</strong>
{{end}}
{{end}}
<a rel="nofollow" class="ui sha label {{if .LatestCommit.Signature}} isSigned {{if .LatestCommitVerification.Verified }} isVerified {{end}}{{end}}" href="{{.RepoLink}}/commit/{{.LatestCommit.ID}}">
<a rel="nofollow" class="ui sha label {{if .LatestCommit.Signature}} isSigned {{if .LatestCommitVerification.Verified }} isVerified{{if eq .LatestCommitVerification.TrustStatus "trusted"}}{{else if eq .LatestCommitVerification.TrustStatus "untrusted"}}Untrusted{{else}}Unmatched{{end}}{{else if .LatestCommitVerification.Warning}} isWarning{{end}}{{end}}" href="{{.RepoLink}}/commit/{{.LatestCommit.ID}}">
{{ShortSha .LatestCommit.ID.String}}
{{if .LatestCommit.Signature}}
<div class="ui detail icon button">
{{if .LatestCommitVerification.Verified}}
<i title="{{.LatestCommitVerification.Reason}}" class="lock green icon"></i>
{{else}}
{{if .LatestCommitVerification.Verified}}
<div class="ui detail icon button" title="{{if eq .LatestCommitVerification.TrustStatus "trusted"}}{{else if eq .LatestCommitVerification.TrustStatus "untrusted"}}{{.i18n.Tr "repo.commits.signed_by_untrusted_user"}}: {{else}}{{.i18n.Tr "repo.commits.signed_by_untrusted_user_unmatched"}}: {{end}}{{.LatestCommitVerification.Reason}}">
<i class="lock {{if eq .LatestCommitVerification.TrustStatus "trusted"}}green{{else if eq .LatestCommitVerification.TrustStatus "untrusted"}}yellow{{else}}orange{{end}} icon"></i>
{{if ne .LatestCommitVerification.SigningUser.ID 0}}
<img class="ui signature avatar image" src="{{.LatestCommitVerification.SigningUser.RelAvatarLink}}" />
{{else}}
<img class="ui signature avatar image" src="{{AvatarLink .LatestCommitVerification.SigningEmail}}" />
{{end}}
</div>
{{else if .LatestCommitVerification.Warning}}
<div class="ui detail icon button">
<i title="{{$.i18n.Tr .LatestCommitVerification.Reason}}" class="red unlock icon"></i>
</div>
{{else}}
<div class="ui detail icon button">
<i title="{{$.i18n.Tr .LatestCommitVerification.Reason}}" class="unlock icon"></i>
{{end}}
</div>
</div>
{{end}}
{{end}}
</a>
{{template "repo/commit_status" .LatestCommitStatus}}
Expand Down
64 changes: 63 additions & 1 deletion web_src/less/_repository.less
Original file line number Diff line number Diff line change
Expand Up @@ -1234,7 +1234,7 @@
text-align: center;
}

width: 140px;
width: 175px;
}
}

Expand All @@ -1255,6 +1255,12 @@
#repo-files-table .sha.label {
border: 1px solid #bbbbbb;

.ui.signature.avatar {
height: 16px;
margin-bottom: 0;
width: auto;
}

.detail.icon {
background: #fafafa;
margin: -6px -10px -4px 0;
Expand Down Expand Up @@ -1285,6 +1291,32 @@
background: fade(#21ba45, 30%) !important;
}
}

&.isSigned.isVerifiedUntrusted {
border: 1px solid #fbbd08;
background: fade(#fbbd08, 10%);

.detail.icon {
border-left: 1px solid #fbbd08;
}

&:hover {
background: fade(#fbbd08, 30%) !important;
}
}

&.isSigned.isVerifiedUnmatched {
border: 1px solid #f2711c;
background: fade(#f2711c, 10%);

.detail.icon {
border-left: 1px solid #f2711c;
}

&:hover {
background: fade(#f2711c, 30%) !important;
}
}
}

.diff-detail-box {
Expand Down Expand Up @@ -1908,6 +1940,36 @@
}
}

.ui.attached.isSigned.isVerifiedUntrusted {
&:not(.positive) {
border-left: 1px solid #c2c193;
border-right: 1px solid #c2c193;
}

&.top:not(.positive) {
border-top: 1px solid #c2c193;
}

&:not(.positive):last-child {
border-bottom: 1px solid #c2c193;
}
}

.ui.attached.isSigned.isVerifiedUnmatched {
&:not(.positive) {
border-left: 1px solid #c2a893;
border-right: 1px solid #c2a893;
}

&.top:not(.positive) {
border-top: 1px solid #c2a893;
}

&:not(.positive):last-child {
border-bottom: 1px solid #c2a893;
}
}

.ui.segment.sub-menu {
padding: 7px;
line-height: 0;
Expand Down