967049 - Remove or augment 'wipe credentials' functionality from passwords sync

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Richard Newman [:rnewman]]

See Bug 966004. The client has logic to make sure that Weave credentials don't stay on the server. This is being tickled under FxA, and the client is making a malformed request on first sync.

This should probably be removed (it seems like very old "oops" recovery code to me), or augmented to handle FxA credentials.

This will probably touch the same code as Bug 967047.

Comment 1

[Mark Hammond [:markh] [:mhammond] - PTO until April 20]

(In reply to Richard Newman [:rnewman] from comment #0)
> See Bug 966004. The client has logic to make sure that Weave credentials
> don't stay on the server. This is being tickled under FxA, and the client is
> making a malformed request on first sync.

hmm - what I see is:

1391751090495   Sync.Collection DEBUG   mesg: DELETE fail 400 https://sync-1-us-east-1.sync.services.mozilla.com/1.5/8989/storage/passwords?ids=

which I believe comes from the fact that no IDs match the password it is trying to delete.  ie, I believe the same "malformed" request would exist every time the old sync enters that block (eg, after sync prefs are reset).

Is that what you mean by the malformed request, or is there something else I'm missing?

> This will probably touch the same code as Bug 967047.

Yeah, I'll probably just include the fix for this in that bug.

Comment 2

[Richard Newman [:rnewman]]

(In reply to Mark Hammond [:markh] from comment #1)

> which I believe comes from the fact that no IDs match the password it is
> trying to delete.  ie, I believe the same "malformed" request would exist
> every time the old sync enters that block (eg, after sync prefs are reset).

For Old Sync, it'll make the same request, but it'll have IDs, because it'll find the old Sync credentials. They shouldn't exist on the server (because it filtered them out before uploading, since 2009 or so), but the deletion will succeed regardless.

For New Sync, there are no matching passwords locally, so the deletion will have an empty ID list, and will fail with a 400. 

... but it will have uploaded your FxA credentials, and it won't delete them. Which is the exact same bug that caused this block of code to have been written in the first place, I'd guess :P


> Is that what you mean by the malformed request, or is there something else
> I'm missing?

That's the malformed request, yes. The fix will be to include FxA credentials in the list of IDs to delete. The related bug is to not upload those, just as we don't upload Weave credentials.

Comment 3

[Mark Hammond [:markh] [:mhammond] - PTO until April 20]

(In reply to Richard Newman [:rnewman] from comment #2)
> For Old Sync, it'll make the same request, but it'll have IDs, because it'll
> find the old Sync credentials.

Ah, yes - it gets them from the login manager, so they should exist.  Sorry for the noise.

Comment 4

[Tracy Walker [:tracy]]

Is there anything QA will be able to do to verify this, once it is fixed?

Comment 5

[Richard Newman [:rnewman]]

(In reply to [:tracy] Tracy Walker - QA Mentor from comment #4)
> Is there anything QA will be able to do to verify this, once it is fixed?

On the first sync from a client, make sure there's no 400 response for a DELETE with the passwords log tag.

Comment 6

[Mark Hammond [:markh] [:mhammond] - PTO until April 20]

Fixed by the patch in bug 967047