GNOME Bugzilla – Bug 685401
linking by email
Last modified: 2012-10-29 23:42:47 UTC
As discussed on IRC, folks only links Personas based on a few properties which are assumed to be unique for persons. For EDS, these are IM handles and web service addresses. The later are folks extensions and not normally found in vcards. folks does not link by telephone number or email address. I agree with the argument that phone numbers are too often shared between different persons to be used for linking, for example family members living in the same house. For email I am less sure. Both I and flo expected that to be a unique identifier for real-world persons and thus suitable for linking. Let's discuss the pros and cons of linking by email...
(In reply to comment #0) > As discussed on IRC, folks only links Personas based on a few properties which > are assumed to be unique for persons. For EDS, these are IM handles and web > service addresses. The later are folks extensions and not normally found in > vcards. > > folks does not link by telephone number or email address. > > I agree with the argument that phone numbers are too often shared between > different persons to be used for linking, for example family members living in > the same house. > > For email I am less sure. Both I and flo expected that to be a unique > identifier for real-world persons and thus suitable for linking. Let's discuss > the pros and cons of linking by email... Shared email addresses aren't entirely unheard of. They were somewhat common in the dial-up era, when people would get their email access through their ISP. I'm guessing that's very rare at this point, but I do know of two people who share an email account (I think they also share a Facebook account, which is even less common and not something I'd worry about supporting). But the main reason we didn't link by email, at least historically, was that at least one of the Personas providing it would be untrusted. Eg, Facebook contacts can set an arbitrary email address (and IM account, for that matter). So someone would be able to spoof another person's identity through their Facebook account. However, I think we should be able to add "email-addresses" to Edsf.Persona.linkable_properties (and this should be all the implementation needed to support this feature, I think). We only do linking if multiple personas contain the same value for the same property, both Personas have that property in their linkable_properties, and one Persona's PersonaStore.trust_level == FULL and the other's Persona.trust_level >= PARTIAL. We'll need to carefully check that all the linking this now supports is safe (eg, making sure we don't suddenly allow the Facebook-based spoofing I mentioned earlier), but I don't otherwise have a problem with it.
Created attachment 225865 [details] [review] make "email-addresses" linkable in EDS This works for me...
Review of attachment 225865 [details] [review]: Looks good to me. I agree with Travis that we should enable this. I’ve just quickly checked the code in the IndividualAggregator, and it does indeed only link two personas by their linkable properties if they both have FULL trust. Please commit this to master along with a suitable entry in NEWS. Thanks!
Review of attachment 225865 [details] [review]: Oh, actually, you should probably also change edsf-persona.vala to no longer perform lazy loading on the email-addresses property. If it’s a linkable property, its values will always be needed, so lazy loading is an unnecessary overhead. See the im-addresses property in edsf-persona.vala for an example.
Created attachment 227554 [details] [review] make "email-addresses" linkable in EDS + remove lazy loading As suggested, this revised patch also removes lazy loading of email addresses.
Thanks, merged to master.
Review of attachment 227554 [details] [review]: ::: backends/eds/lib/edsf-persona.vala @@ -288,3 @@ - private HashSet<EmailFieldDetails>? _email_addresses = null; - private Set<EmailFieldDetails>? _email_addresses_ro = null; Please don’t remove the read-only view of the set (_email_addresses_ro). folks exposes as many of its collection properties as possible as read-only collections. This prevents clients erroneously modifying the collections without going through folks. @@ +529,3 @@ + + Extraneous whitespace.
Created attachment 227598 [details] [review] Fix merged patch Oops, my bad. This patch should fix those two issues, right?
Also, should Edsf.Persona _web_service_addresses also have a corresponding read only Set?
Review of attachment 227598 [details] [review]: Looks good to me. > Also, should Edsf.Persona _web_service_addresses also have a corresponding read > only Set? libgee doesn’t provide any API like MultiMap.read_only_view, so this isn’t possible. I was sure there was a bug open against libgee about this, but it seems to have disappeared. Would you mind opening one please (and then opening a libfolks bug against it)?
Done.