[go: up one dir, main page]

Skip to content

Use author_email to verify UI-signed commits

This MR should be merged after the migration MR has been merged.

What does this MR do and why?

Related to: https://gitlab.com/gitlab-org/gitlab/-/issues/526568+

Problem

With the check_for_mailmapped_commit_emails](#481441) feature flag enabled, UI-signed commits were found to be incorrectly displaying an "orange verified" label for old commits created with the FF disabled. They should display a green "Verified" label.

Why it happened

We identified that GitLab compares the user and author to verify UI-signed commits when the feature flag is enabled. For UI-signed commits, GitLab itself is the signer — and GitLab is not a user. Therefore, we should not rely on the user_id for verifying UI-signed commits.

Instead, we should store the commit author’s email — which is returned by the getCommitSignatures RPC — in the SshSignature table and use that for verification.

Additionally, we discovered that the user_id was being overwritten with the commit author for UI-signed commits. These two concepts should remain separate.

New author_email column

Populating the new author_email column in the table is handled in a separate MR..

References

Screenshots or screen recordings

Before After
in_master in_my_branch

How to set up and validate locally

  1. Disable the mailmap feature flag in rails console: Feature.disable(:check_for_mailmapped_commit_emails)
  2. Get UI Signed Commits working with your GDK with an ssh key: (https://docs.gitlab.com/user/project/repository/signed_commits/web_commits/, https://docs.gitlab.com/administration/gitaly/configure_gitaly/#configure-commit-signing-for-gitlab-ui-commits)
  3. Add same ssh key used for creating UI commits to a user (user_1) with a verified email.
  4. Create a commit with a different user (user_2) and see that the commit is verified.
  5. Notice that the key_id for the ssh_signature table is populated and the user_id is that of user_1.
  6. Enable the mailmap feature flag: Feature.enable(:check_for_mailmapped_commit_emails) and Feature.enable(ui_signed_commit_verification)
  7. Refresh the page showing the verified commit, notice that the label has turned orange.

Query plan

 ModifyTable on public.ssh_signatures  (cost=0.43..3.45 rows=0 width=0) (actual time=2.032..2.032 rows=0 loops=1)
   Buffers: shared hit=54 read=3 dirtied=1
   WAL: records=7 fpi=1 bytes=8613
   I/O Timings: read=1.673 write=0.000
   ->  Index Scan using ssh_signatures_pkey on public.ssh_signatures  (cost=0.43..3.45 rows=1 width=38) (actual time=0.036..0.038 rows=1 loops=1)
         Index Cond: (ssh_signatures.id = 31)
         Buffers: shared hit=7
         I/O Timings: read=0.000 write=0.000
Settings: seq_page_cost = '4', work_mem = '100MB', effective_cache_size = '472585MB', jit = 'off', random_page_cost = '1.5'
Time: 2.614 ms
  - planning: 0.524 ms
  - execution: 2.090 ms
    - I/O read: 1.673 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 54 (~432.00 KiB) from the buffer pool
  - reads: 3 (~24.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 1 (~8.00 KiB)
  - writes: 0

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Emma Park

Merge request reports

Loading