The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in Thunderbird 14.0

Status

Thunderbird
Preferences
--
major
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Jb Piacentino, Assigned: mconley)

Tracking

13 Branch
Thunderbird 14.0
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird13 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
Assignee: nobody → mconley
Component: Account Manager → Preferences
QA Contact: account-manager → preferences
Blocks: 698925
Created attachment 608087 [details] [diff] [review]
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?
Attachment #608087 - Flags: feedback?(squibblyflabbetydoo)
Attachment #608087 - Flags: feedback?(dbienvenu)

Comment 2

5 years ago
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.
Attachment #608087 - Flags: feedback?(dbienvenu) → feedback+
David:

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

-Mike

Comment 4

5 years ago
(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.
(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 on attachment 608087 [details] [diff] [review]
Patch v1

How do we feel about this?  Worth landing?
Attachment #608087 - Flags: feedback?(squibblyflabbetydoo) → review?(dbienvenu)

Updated

5 years ago
Attachment #608087 - Flags: review?(dbienvenu) → review+
Attachment #608087 - Flags: approval-comm-aurora?
Committed to comm-central as http://hg.mozilla.org/comm-central/rev/46bed17a19a9
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 14.0

Updated

5 years ago
Attachment #608087 - Flags: approval-comm-aurora? → approval-comm-aurora+
Committed to comm-aurora as http://hg.mozilla.org/releases/comm-aurora/rev/57bfc276a84b
status-thunderbird13: --- → fixed
Depends on: 739986
You need to log in before you can comment on or make changes to this bug.