737955 - Filelink: deleting a Filelink Dropbox account should delete the corresponding credentials

Status: RESOLVED FIXED

(Thunderbird :: Preferences, defect)

Description

[Jb Piacentino]

STR 

(with Dropbox)

1 - Set up a Filelink Dropbox account with proper credentials
2 - Remove it (no other LFP registered)
3 - Add a Filelink to an email
4 - Choose Dropbox
5 - File gets uploaded

Expected behaviour
- Credentials should have been deleted in step 2 and Dropbox login/signup page should be launched in step 5

Dropbox username & password remain in the saved password

This problem does not appear with YouSendIt.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1

So here's what I've done:

Initially I talked to bienvenu about adding a "destroy" method to the provider interface, letting them manage things like cleanup...

Instead, I found that we could simply clear out any secret tokens stored in nsILoginManager from within cloudFileAccounts.jsm.  This seemed a bit simpler - does it seem like the right choice?

So, that clears out the auth tokens when removing an account.  It *still* doesn't fix our Dropbox OAuth cookie persistence problem....

What I did for that, was just fire out an XHRequest to https://www.dropbox.com/logout once the OAuth procedure has successfully completed.  This, essentially, ends the session opened in order to get the OAuth tokens.

I've also added a test for our Dropbox backend to make sure the logout request is sent.

Thoughts?  Is this the right way to go about things?

Comment 2

[David :Bienvenu]

Comment on attachment 608087 [details] [diff] [review]
Patch v1

I definitely prefer this, in that I think this belongs more to the account manager, and the providers don't have to duplicate this. But we should document this a bit, so that providers know to use the login manager, and that if they do, the login info will get cleaned up automatically when accounts are removed. We should probably have a page on devmo about implementing a cloud file provider. I can start one.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

David:

Is the XHRequest to https://www.dropbox.com/logout a reasonable solution?

-Mike

Comment 4

[David :Bienvenu]

(In reply to Mike Conley (:mconley) from comment #3)
> David:
> 
> Is the XHRequest to https://www.dropbox.com/logout a reasonable solution?
> 
> -Mike

I don't know - they don't define that in their REST api, so I really don't know if it will work or if there are downsides.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

(In reply to David :Bienvenu from comment #4)
> (In reply to Mike Conley (:mconley) from comment #3)
> > David:
> > 
> > Is the XHRequest to https://www.dropbox.com/logout a reasonable solution?
> > 
> > -Mike
> 
> I don't know - they don't define that in their REST api, so I really don't
> know if it will work or if there are downsides.

From local testing, it does "work", in that the created session for getting the OAuth tokens is destroyed.

Unsure if it's the best way though.  Open to suggestions.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 608087 [details] [diff] [review]
Patch v1

How do we feel about this?  Worth landing?

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Committed to comm-central as http://hg.mozilla.org/comm-central/rev/46bed17a19a9

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Committed to comm-aurora as http://hg.mozilla.org/releases/comm-aurora/rev/57bfc276a84b