486145 - Sharing API: Login from Extension

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Collector Extension, defect, P1)

Description

[David McNamara [:mackers]]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10_5_6; en-gb) AppleWebKit/525.27.1 (KHTML, like Gecko) Version/3.2.1 Safari/525.27.1
Build Identifier: 

We need to allow the user to login to AMO directly from the extension. For this we will need to add a Login call to the API.

Input: username and password
Return: yay or nay.

Does this call set a cookie or does the extension? How do we indicate we're authenticated for the remainder of the session?

I presume we'll also be needing a log out.



Reproducible: Always

Comment 1

[Justin Scott [:fligtar]]

Les mentioned that he will try to work on the spec for this soon.

Comment 2

[Les Orchard [:lorchard]]

Basically, the idea I have is that you hit POST to a URL in the API using Basic auth or some POST parameters, and the response will contain an auth token string on success.  

That token string can then be used as auth for the rest of the API by being included as a custom header.  No cookies involved.  The extension retains the token as long as it wants.  Logout in the extension means that it discards the token string.

If we decide at some future point to implement an explicit logout / deauth feature on the server side, the API would just stop accepting the previously-issued token.  At that point, 401 Unauthorized responses from the API would be a hint to the extension to request another login.  But, this would only be in response to a user action on the AMO site.  The token won't otherwise expire on its own.

Comment 3

[Justin Scott [:fligtar]]

That sounds good to me. I think we would want the token to expire anytime the user changes their e-mail address or password on the site, and the extension should then force re-authentication.

Will you have time to work on an implementation of this next week, Les?

Comment 4

[Les Orchard [:lorchard]]

Attachment: implementation of auth token factory resource and changes to accept auth tokens

I'll update the bandwagon API docs I have in the wiki soon, but this patch should work for introducing a new auth token factory resource.

When an unauth'd request is made to the API, the error response will include the URL of the auth factory resource as href along with the "unauthorized" reason in XML.

This auth factory URL accepts a POST (no parameters), using HTTP basic auth with the username and password presumably requested from the user via login dialog.

A successful POST to the factory URL yields a response containing a new token value and URL.  A valid token value or URL will be accepted as authentication in a custom header named X-API-Auth.  

An authenticated DELETE request to the token URL will invalidate the token - this should allow for forced "logout".

Comment 5

[Fred Wenzel [:wenzel]]

For some reason, I get a lot of fails in the tests:

FAIL: testEmptyCollectionsWithModel -> Logged in with test account @ line 65 (8990540)
FAIL: testEmptyCollectionsWithModel -> Service document should be found at http://localhost:8888/amo/en-US/firefox/api/1.3/sharing/ @ line 141 (8861488)
FAIL: testCollectionAddonCrud -> Error reading socket [Cannot open [localhost:80] with [Connection refused] within [15] seconds]

Something makes very strong assumptions about my dev instance setup here, it seems :(

Comment 6

[Les Orchard [:lorchard]]

Odd... the tests actually try not to assume anything beyond the initial sharing API service doc URL - and that's arrived at using $this->actionURI('/api/1.3/sharing/') from the WebTestHelper super-class.

I've seen these things pass on my laptop, khan, and (iirc) bandwagon staging.  Wonder what's different here. :/

Comment 7

[Fred Wenzel [:wenzel]]

(In reply to comment #6)
> I've seen these things pass on my laptop, khan, and (iirc) bandwagon staging. 
> Wonder what's different here. :/

Thanks. I'll try to figure out what's going wrong here. Probably I am missing some setting somewhere. Do you run the tests on the remora test data, or on a prod snapshot?

Comment 8

[Les Orchard [:lorchard]]

Just the test data.  I have it pointed at a remora_test database with site/app/config/sql/remora.sql and site/app/tests/data/remora-test-data.sql dumped in.

I'm trying to remember if there was any other config that needed setting - or if the test class doesn't like using a different port in HTTP requests.

Comment 9

[Fred Wenzel [:wenzel]]

Comment on attachment 371345 [details] [diff] [review]
implementation of auth token factory resource and changes to accept auth tokens

now that I finally got the tests working, this looks good to me.

Comment 10

[Les Orchard [:lorchard]]

checked in as r24422 - will need some SQL changes :/

Comment 11

[Fred Wenzel [:wenzel]]

(In reply to comment #10)
> checked in as r24422 - will need some SQL changes :/

Make sure to add the SQL to config/sql/remora.sql as well, please.

Comment 12

[Fred Wenzel [:wenzel]]

(In reply to comment #11)
> Make sure to add the SQL to config/sql/remora.sql as well, please.

Oops, my fault: You already did that. Sorry.

Comment 13

[David McNamara [:mackers]]

There is no separate bug for implementing this in the extension, so I'll comment here.

The API is now working great.

In that regard, I have implemented both login and "logout" in the extension.

r24513

Comment 14

[Brian King [:kinger]]

Dave, great, just tested it and it works fine.

A few questions though:

1) Did we not discuss the UI being handled in the extension, as opposed to redirecting to the site?

2) When I first synced up, all my subscriptions were present despite me not being logged in. Or at least in Settings I was told I was not logged in. Could this be a legacy of the old cookie system?

3) Are we getting a notification after login/logout, and if so can it bubble up to Settings so that the status can be changed immediately? Right now the Settings window has to be reopened.

Comment 15

[Brian King [:kinger]]

And I am still getting annoying login prompts for every subscription refresh, but I guess that is a stage thing.

Comment 16

[Brian King [:kinger]]

More strange behaviour. Steps:

1) I shut down Firefox
2) Removed bandwagon.sqlite
3) Restart and open Add-ons
4) In Subscriptions I am shown the login deck, so I do and do so
5) Shutdown and restart Add-ons window
6) I'm shown the Find Collections deck layer, even though I am subscribed to collections on the site. Closing and reopening Add-ons again does not help.

No error output in console

Comment 17

[Brian King [:kinger]]

The subs appear now after a Firefox restart, but it is still a bug that it is necessary. Maybe this is covered in another bug, I am not sure.

Comment 18

[Brian King [:kinger]]

Sorry, ignore my last 4 comments. And sorry all for the spam. I obviously hadn't rebuilt. I now see the in-extension UI and all is well. Nice work.

Comment 19

[Justin Scott [:fligtar]]

Moving to revised M5 milestone (4/28).

Comment 20

[Justin Scott [:fligtar]]

Hmm. I can type anything at all into the login box and hit login and it still logs me in and shows my subscriptions.

Comment 21

[David McNamara [:mackers]]

(In reply to comment #20)
> Hmm. I can type anything at all into the login box and hit login and it still
> logs me in and shows my subscriptions.

Hmm. Very odd. I can't reproduce.

For me, I enter junk and hit login and get an error saying my login is incorrect.

Any more details? Does it manifest on a clean profile?

Comment 22

[David McNamara [:mackers]]

(In reply to comment #20)
> Hmm. I can type anything at all into the login box and hit login and it still
> logs me in and shows my subscriptions.

Aha! I can reproduce this now.

I think it was due to the fact that you were logged in to AMO and it was authentication with that cookie instead of the auth token.

Fixed in r24534