GNOME Bugzilla – Bug 714643
Account identifier instead of email address
Last modified: 2016-07-25 00:42:54 UTC
---- Reported by jim@yorba.org 2013-02-08 17:59:00 -0800 ---- Original Redmine bug id: 6356 Original URL: http://redmine.yorba.org/issues/6356 Searchable id: yorba-bug-6356 Original author: Jim Nelson Original description: Up to now we've gotten by using the account's email address as an account identifier (in particular, see #5996). That, however, isn't viable over the long-term. Geary should assign a unique identifier to each generated account. That identifier could be used for the directory name as well as internally as a unique token for hash keys, over DBus, and so on. Since an account with the same email address could be created, destroyed, and created again, this identifier can also be persisted (it won't be confused with the new account in this scenario). A UUID is probably overkill, as it only needs to be unique for Geary, not worldwide, something similar would do the trick. Related issues: related to geary - 5996: Don't use data directory name as email address (Open) ---- Additional Comments From geary-maint@gnome.bugs 2013-09-04 11:53:00 -0700 ---- ### History #### #1 Updated by Eric Gregory 9 months ago I should mention the email string is used as a unique ID throughout the Accounts dialog, and in the Composer's "From:" combo box as well. #### #2 Updated by Jim Nelson 9 months ago * **Target version** changed from _0.3.0_ to _0.4.0_ #### #3 Updated by Jim Nelson 3 months ago * **Target version** changed from _0.4.0_ to _0.5.0_ --- Bug imported by chaz@yorba.org 2013-11-21 20:28 UTC --- This bug was previously known as _bug_ 6356 at http://redmine.yorba.org/show_bug.cgi?id=6356 Unknown milestone "unknown in product geary. Setting to default milestone for this product, "---". Setting qa contact to the default for this product. This bug either had no qa contact or an invalid one. Resolution set on an open status. Dropping resolution
I'm thinking that the name of the domain for the account with some numeric increment ("gmail.com_1", "gmail.com_2", "vee.net_1") might be appropriate. This can be guessed from either the email or the server hostnames. It provides a mnemonic for both users messing with their FS and for debugging, and if the hostname is used as the basis, perhaps less likely to change than if the primary email address is used. Either that or just use a simple "account_n" string, where "n" is an incrementing integer, but that then loses the affordance of the mnemonic. I wonder if we need to salt these (per FF/TB profile file names) to avoid attackers targeting the well-known/easily guessable names?
The difficulty here is that Geary doesn't store the primary email address in geary.ini at the moment, so there needs to be some config migration. This could perhaps be done in conjunction with migrating the ini file to ~/.config/geary/<id> (Bug 741883) and retaining the email address as the value of the id for migrated accounts: - Scan for accounts in ~/.local/share/geary, for each: - If `.config-migrated` does not exist in the account dir: - Load the old config for the account - Set the account's id to be the primary email address - Set the primary email address in the config - Save the updated config out to ~/.config... - Create `.config-migrated` as an empty file New accounts created subsequently would simply have their ids assigned per a scheme like in the comment above.
Created attachment 331071 [details] [review] Make use of the "primary_email" field in the config file This should get us started fixing this bug. Note that this depends on the patches posted in bug 741883.
Review of attachment 331071 [details] [review]: All good here as well. Will commit this with the patches from Bug 741883.
Created attachment 331239 [details] [review] Make use of the "primary_email" field in the config file Rebased against the new patches in bug 741883.
Review of attachment 331239 [details] [review]: Pushed to master as 7b5473c
Now that the primary email is being stored in the config file (thanks Oskar!), what remains here is: 1. Devise a decent id scheme per Jim's comments in the description and in those comment 2. 2. Add an "id" property to the AccountInformation class, set it using the scheme above for new accounts and load it from the account's config directory's basename for existing accounts 3. Replace all uses of AccountInformation::primary_email as something other than an email address with the new id property instead.
I've got a WIP branch in progress for this. Both the client and engine side has been updated: - AccountInformation::id now exists, which is immutable - string AccountInformation::email has been replaced with Geary.RFC822.MailboxAddress primary_mailbox, which can be changed. - AccountInformation::display_name now also exists, which is mutable and basically returns the primary address Some random notes: This hasn't been tested out much at all. In particular it needs to be tested when sending an email using STMP via Google, when the primary_mailbox is not the user's default GMail address. The display_name property provides a centralised way to determine what string should be displayed to the user so as to uniquely identify the account to them. Since it returns the primary_mailbox's address, it is not immutable, but is probably the best thing to present regardless. Also, this will basically replace the nickname in the future - see Bug 730707 and its duplicates. If we decide to migrate existing accounts to use the new id scheme rather than keeping their old primary email as their id's, we'd also have to migrate data in libsecret, so lets not do that.
Created attachment 331434 [details] [review] Convert from using AccountInformation.email to an id - engine changes.
Created attachment 331435 [details] [review] Convert from using AccountInformation.email to an id - client changes.
This looks promising! A few comments: - Showing the primary email in the UI was also what I had in mind in light of bug 730707 :) - I don't think it's worth migrating old accounts to the new id system, it adds unneeded complexity to the code. We won't have to check if the basename is in the form of an email address and what to do if it is the case etc. - What do we do with the "real_name" property? We've basically got mailboxes all over now which have a name property too. Do we want to merge that with primary_mailbox.name in AccountInformation?
Oh yeah, good point about real_name. After looking into removing it and just using the primary_mailbox's name prop instead, it looks pretty trivial, however in doing so I found a few wrinkles with the email/id change in add-edit-page.vala that need fixing. I'm going to be pretty busy over the next few days, so will fix it up and respin these patches next week.
Created attachment 331695 [details] [review] Convert from using AccountInformation.email to an id - engine changes
Created attachment 331696 [details] [review] Convert from using AccountInformation.email to an id - client changes
Created attachment 331697 [details] [review] Remove AccountInformation.real_name, use primary_mailbox.name instead
I tried the patches, they seem to work out. I renamed an account directory to a random one in ~/.config and ~/.local/share, Geary seems to pick that up without a hitch. But the account id is shown in the accounts list now, instead of the email address. And there seems to be some logic there for getting the selected account through the email shown in the treeview.. I guess we could change that to primary_mailbox.address.
Created attachment 331752 [details] [review] Convert from using AccountInformation.email to an id - client changes Ah, good catch, thanks. Updated patch fixes that.
*** Bug 714903 has been marked as a duplicate of this bug. ***
Created attachment 331803 [details] [review] Use an opaque string instead of primary email address as account id Final patch in the series. Will commit to master after a bit more testing.
So I guess this gives us a way to namespace things. I would imagine something like "account_" for usual Geary accounts, "goa_" and "uoa_" for accounts fetched from their respective services. Does that seem sane? This would give us an easy way to differentiate between different types of accounts.
Yeah, I was thinking about that. My original plan was to use server type as the prefix - "gmail_", "other_", etc., and that could also be used for GOA/UOA. This would be useful for debugging, as a mnemonic for users, and so on. However it struck me that by encoding this stuff in the id, I would be making the same mistake as using the primary email address did previously. What if Geary lets people change provider types in the future? In the end letting the engine just choose something and have the user and client treat it as an opaque string is the best way to go. So, to determine if an account is local/GOA/UOA, it means loading geary.ini file and looking at the config in there, per the scheme outlined in Bug 768975: If the ini file indicates a GoaServiceInformation class should be used, then it's a GOA account.
This have now been pushed to master.