Mailpile icon indicating copy to clipboard operation
Mailpile copied to clipboard

Add Account should not permit duplicate email address

Open JackDca opened this issue 6 years ago • 16 comments

If the Add Account dialog is given the same name and email address as an existing account it will create a second account duplicating the email address of the existing account as requested. (In testing this the encryption key created during the setup of the first account was selected for the new account - the creation of a new key was for the new account was not tested.) Then, in the home screen on the line for the new account, if the Trash icon is clicked, the program appears to start removing the duplicate account, but the display stops updating.

Having the same email address attached to two different accounts in the same pile does not make sense and is sure to cause problems. The Add Account dialog should screen the supplied email address and reject it if it is the same as the email address for an existing account. Preferably the email addresses should be canonicalized before comparison.

JackDca avatar May 19 '18 02:05 JackDca

Agreed, thanks for filing this.

BjarniRunar avatar May 23 '18 05:05 BjarniRunar

Hey. A newbie here. Can I pick this up?

deepaknalore avatar Jul 11 '18 18:07 deepaknalore

@deepaknalore - Although I reported this issue, a fix would require mods to the front end, which is not my strong area, so I don't plan to work on it. If you can provide a fix, I say go for it!

JackDca avatar Jul 12 '18 02:07 JackDca

Since @deepaknalore hasn't commented anything yet I assume he doesn't currently work on this. I'd like to look a bit into the problem. Can somebody give me a hint where in the codebase I might have to look?

noahhaasis avatar Aug 02 '18 15:08 noahhaasis

I don't speak HTML/JS/CSS but here is a suggestion on how to get a grip on the code:

  • The Add Account pop-up that presents the "E-mail" field to be filled in has a subtitle "Basic Details" which is a good target for a search because it is specific enough to avoid a lot of irrelevant hits.
  • A global search of all the source for "Basic Details" finds default-theme/html/profiles/account-form.html.
  • A search of that file for "E-mail" finds this definition:
        <div class="left" style="margin-right: 1em; width: 29em;">
          <label>{{_("E-mail")}}</label>
          <input type="text" name="email" style="width: 100%"
                 class='fpa-email fpa-basic-username' data-next="#basics-next"
                 placeholder="[email protected]" value="{{ result.email }}"
                 onchange="javascript:_fpa.email_changed();">
        </div>

Hope this gets you started!

I use Eclipse to manage my source - it facilities various kinds of global and local searches. It integrates with git so you easily can compare against other versions. (There's also an Eclipse-based Python IDE which I have never learned to use!)

JackDca avatar Aug 03 '18 14:08 JackDca

Thanks @JackDca for the great comments, and thanks @noahhaasis for the offer to help! More hints:

A good solution to this would look something like this:

  1. Update ProfileVCard and/or AddProfile in .../mailpile/plugins/contacts.py
    1. Look up existing profiles, gather the e-mail addresses
    2. If creating a new profile, fail the API call if the e-mail address already exists
    3. If displaying the form, provide the list of e-mail addresses to the template...
  2. Update the template .../shared-data/default-theme/html/profiles/account-form.html
    1. Grab the e-mail list exposed by the fixed API methods, inject as a Javascript array somewhere
    2. Use that array to disable/enable the Save button and/or pop up warnings to the user

Note that because of the inheritance in contacts.py and the fact that the account-form.html template is used by multiple API endpoints, you probably want to gather and expose the list of e-mails in the parent class (ProfileVCard), but enforce that new accounts cannot be added with a dup e-mail in the subclass AddProfile. Since account-form.html is also used to edit existing contacts, the checks will need to accept an existing e-mail address if it's "unchanged" from before.

BjarniRunar avatar Aug 07 '18 16:08 BjarniRunar

I don't have time to work on the issue this week, so if anybody wants to work at it in the meantime go for it! If that isn't the case I'd try it next week.

noahhaasis avatar Aug 07 '18 17:08 noahhaasis

@noahhaasis Hey Noah. Do you want to work on it? I am free this week. I want to start looking at it.

deepaknalore avatar Aug 17 '18 05:08 deepaknalore

@BjarniRunar I started to code the changes, I wanted to understand the code by running test cases. But, the test case test_contacts has a problem and needs to be fixed.

Hence, I need your help.

  1. Can I do a remote-debug while running mailpile with my IDE( I am using pycharm)?
  2. How can I add a logger message?
  3. A good demo/ summary wiki which will point me in debugging whatever changes I make?

Thanks, des2

deepaknalore avatar Aug 22 '18 08:08 deepaknalore

@BjarniRunar Hey! Any inputs on this?

deepaknalore avatar Aug 26 '18 05:08 deepaknalore

I'm very sorry @deepaknalore, I just haven't had the time to give you the guidance you need on this. :disappointed:

BjarniRunar avatar Sep 25 '18 16:09 BjarniRunar

Is someone still working on this? if not please assign this to me. if this issue is already claimed please tell me some "low hanging fruit " labeled or easy issues to which I can contribute to . Thank you

faizan2700 avatar Sep 21 '19 12:09 faizan2700

Hi @faizan2700 - I don't speak for the Mailpile project - I'm just a contributor - but, I encourage you to dive in to this issue or any other that looks like a good way to get started. I don't think anyone is actively working on this issue. If I can help in any way I'm happy to do so.

JackDca avatar Sep 21 '19 20:09 JackDca

By testing Mailpile out, I have determined that this issue still has not been solved. Two of my colleagues and I will try to solve this issue in the coming week.

stefanb0305 avatar Dec 01 '20 14:12 stefanb0305

I am following the steps from below and have successfully fixed the bug in the frontend (updated the template). However, I cannot figure out exactly what to add in the Add Profile class. I was thinking of adding the following to _update_vcard_from_post after line 1217:

for vc in self.session.config.vcards.find_vcards([], kinds=['profile']):
    if vc.email == vcard.email or vc.fn == vcard.fn:
        if vc.random_uid != vcard.random_uid:
            # Should not accept duplicates
            return

Currently this seems to prevent a client from making any new account, although I do not see why. Any help would be appreciated!

Thanks @JackDca for the great comments, and thanks @noahhaasis for the offer to help! More hints:

A good solution to this would look something like this:

  1. Update ProfileVCard and/or AddProfile in .../mailpile/plugins/contacts.py

    1. Look up existing profiles, gather the e-mail addresses
    2. If creating a new profile, fail the API call if the e-mail address already exists
    3. If displaying the form, provide the list of e-mail addresses to the template...
  2. Update the template .../shared-data/default-theme/html/profiles/account-form.html

    1. Grab the e-mail list exposed by the fixed API methods, inject as a Javascript array somewhere
    2. Use that array to disable/enable the Save button and/or pop up warnings to the user

Note that because of the inheritance in contacts.py and the fact that the account-form.html template is used by multiple API endpoints, you probably want to gather and expose the list of e-mails in the parent class (ProfileVCard), but enforce that new accounts cannot be added with a dup e-mail in the subclass AddProfile. Since account-form.html is also used to edit existing contacts, the checks will need to accept an existing e-mail address if it's "unchanged" from before.

stefanb0305 avatar Dec 07 '20 00:12 stefanb0305

I would like to contribute to this open-source project so someone can please guide me through the code structure and the bug?

Vilayat-Ali avatar Aug 29 '21 14:08 Vilayat-Ali