GNOME Bugzilla – Bug 659075
IndividualAggregator shouldn't set the trust level for PersonaStores
Last modified: 2011-09-16 19:45:02 UTC
PersonaStore.trust-level is changed to be determined entirely by the store. This is needed so that we link personas from different stores by their linkable properties, which (correctly) requires that we fully trust those stores. It doesn't make sense, conceptually, to tie trust into our choice of primary store — instead it should be determined by how much control the user has over the store, which the store itself is in a better position to determine than the IndividualAggregator.
To follow-up on the description (which was posted accidentally - sorry), this is a follow up of Philip's email about tidying up writeability and primary persona stores.
Created attachment 196625 [details] [review] patch
Review of attachment 196625 [details] [review]: The Tracker backend also needs its trust level setting (to FULL, I think). ::: backends/eds/lib/edsf-persona-store.vala @@ +798,3 @@ if (do_initial_query == false && this._is_quiescent == false) { + this.trust_level = PersonaStoreTrust.PARTIAL; What's the reason behind adding this one? If it's to make LDAP stores only partially-trusted, I think we need to more explicitly check that the address book's actually coming from LDAP (e.g. by checking the type of the address book or something similar). If not, it needs a comment. @@ +1769,3 @@ + + private void _update_trust_level () + { Stick a comment in here along the lines of: This isn't perfect, since we want to base our trust of the address book on whether *other people* can write to it (and potentially maliciously affect the linking our aggregator does). However, since we can't know that, we assume that if we can write to the address book we're probably in full control of it. If we can't, either nobody/a sysadmin is (e.g. LDAP) or somebody else (who we can't trust) is (e.g. a read-only view of someone else's WebDAV address book).
Created attachment 196702 [details] [review] patch Updated version.
Review of attachment 196702 [details] [review]: A couple of small issues to fix, then please commit! ::: backends/eds/lib/edsf-persona-store.vala @@ +1777,3 @@ + private void _update_trust_level () + { + unowned SourceGroup? group = (SourceGroup) this._source.peek_group (); Should the case be to (SourceGroup?) instead? @@ +1781,3 @@ + { + var base_uri = group.peek_base_uri (); + if (Regex.match_simple ("^ldap", base_uri)) Might it be faster to do base_uri.has_prefix("ldap")? Would be useful to have a comment showing an example of the kind of base URI we expect to see/match.
Merged: commit 44f9a8316082e5f86ac96e5018f11713767ecb9d Author: Raul Gutierrez Segales <rgs@collabora.co.uk> Date: Wed Sep 14 20:48:47 2011 +0100 Let each PersonaStore manage its trust-level Previously, the IndividualAggregator was in charge of downgrading and upgrading the trust-level of each PersonaStore. From now on, we let each Store manage its trust-level since it's the natural way to do it. Fixes: https://bugzilla.gnome.org/show_bug.cgi?id=659075