Repository Identity "Automatically Detected User": don't trust unverified emails
ClosedPublic
Actions

Authored by valerio.bozzolan on Thu, Dec 5, 09:58.

Details

Reviewers

speck
20after4

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15965: Repository Identity "Automatically Detected User": it reads unverified emails, with spam concerns

Commits

rP7429da91d293: Repository Identity "Automatically Detected User": don't trust unverified emails

Summary

Make sure that Repository Diffusion Identities "Automatically Detected User " are not created from unverified emails.

Closes T15965

Test Plan

Find at least one identity that is assigned to nobody:

http://phorge.localhost/diffusion/identity/

(For example, you may easily find an identity of "GitHub <noreply@github.com>")

(Double check that its "Assigned To" is unset or make sure it's unset for this test)

Be evil: add *that* email in your Profile → Settings → Email addresses. So, for example add "noreply@github.com", like a rogue. The email can stay unverified.

Run this command to immediately cause an effect:

./bin/repository rebuild-identities --all-identities

before this change, you can reproduce that you successfully steal that identity and you become "GitHub" or whoever
after this change, you see that "Automatically Detected User" is unset again
after this change, any other identity manually assigned, is still assigned to that value
after this change, any other identity automatically assigned to verified emails, are still "Automatically Detected User"

Diff Detail

Repository

rP Phorge

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

valerio.bozzolan created this revision.Thu, Dec 5, 09:58

Owners added a reviewer: O1: Blessed Committers.Thu, Dec 5, 09:58

Herald added subscribers: Cigaryno, Matthew, tobiaswiese. · View Herald TranscriptThu, Dec 5, 09:58

valerio.bozzolan requested review of this revision.Thu, Dec 5, 09:58

valerio.bozzolan changed the edit policy from "All Users" to "Custom Policy".

valerio.bozzolan changed the visibility from "Public (No Login Required)" to "Custom Policy".Thu, Dec 5, 10:02

valerio.bozzolan changed the edit policy from "Custom Policy" to "All Users".

valerio.bozzolan added a subscriber: aklapper.

run unit test

Harbormaster completed remote builds in B1640: Diff 2555.Thu, Dec 5, 15:35

valerio.bozzolan retitled this revision from Repository Identity: don't trust unverified emails to Repository Identity "Automatically Detected User": don't trust unverified emails.Mon, Dec 9, 08:57

valerio.bozzolan edited the summary of this revision. (Show Details)

speck accepted this revision.Wed, Dec 11, 02:23

This revision is now accepted and ready to land.Wed, Dec 11, 02:23

20after4 accepted this revision.Wed, Dec 11, 03:27

valerio.bozzolan edited the test plan for this revision. (Show Details)Wed, Dec 11, 08:03

Thanks folks 💃 Let's land and open visibility, so other people can read more and cherry-pick in their stable if they need.

P.S. sorry for late test plan completion :D :D I've tested in my production btw

Closed by commit rP7429da91d293: Repository Identity "Automatically Detected User": don't trust unverified emails. · Explain WhyWed, Dec 11, 08:31

This revision was automatically updated to reflect the committed changes.

valerio.bozzolan added a commit: rP7429da91d293: Repository Identity "Automatically Detected User": don't trust unverified emails.

valerio.bozzolan changed the visibility from "Custom Policy" to "Public (No Login Required)".Wed, Dec 11, 08:31

valerio.bozzolan mentioned this in Next Up.Wed, Dec 11, 08:39