869951 - Add support for an HMAC+SHA1 authentication scheme on payloads

Status: RESOLVED FIXED

(Webmaker Graveyard :: MakeAPI, enhancement)

Description

[David Humphrey (:humph)]

From email:

"I think the one thing we should all be aware of is that the payload for each request isn't being signed. This means that if someone can sniff any authenticated call, they'll be able to issue authenticated requests.

I'd like to see an HMAC+SHA1 authentication scheme done. This ensures that the payload for the request is signed and immutable. It is also something we have to do in order to support external clients of the MakeAPI. -- Something that I think is coming very soon."

Comment 1

[Jon Buckley]

We should look at https://github.com/hueniverse/hawk/, it got a recommendation from Ozten on the Identity team: http://ozten.com/psto/2013/05/07/proposal-for-securing-mozilla-web-services-with-hawk/ and a Python implementation too: http://ozten.com/psto/2013/05/07/pyhawk-hawk-for-python/

Comment 2

[Simon C. Wex]

I was just about to paste that in. +1.

Comment 3

[Simon C. Wex]

Hey :cade, let me know if you've got any questions on this one? -- I'm happy to be a beta tester.

Comment 4

[Chris DeCairos (:cade)]

:wex Sure thing!

Over the next few weeks I'm going to move into an API Key authentication model for API consuming Apps. Those authenticated requests will probably make use of Hawk.

Comment 5

[Chris DeCairos (:cade)]

So I took a (successful) first stab at this:

https://github.com/cadecairos/MakeAPI/commits/bug869951
https://github.com/cadecairos/make-api-client/commits/bug869951

The Create, Update and Delete Routes on the makeapi in this patch are no longer protected with basic auth - they're using Hawk.

I created an ApiUser Database model for mongo that stores an API key, an app id (username) and a few other things. keys and user names can be generated using a really crude tool I added to the Make Editor (its at the bottom of the page) API keys are generated using node-uuid ( https://github.com/shtylman/node-uuid ).

Hawk uses Mongoose to find the API key for a given user, and attempts to authenticate the call.

The makeapi-client code can ONLY do create/update/delete calls if in a node context. Along with the apiURL, you must pass the Make Constructor a hawk object that contains the API key, username and algorithm.

Comment 6

[Chris DeCairos (:cade)]

Attachment: https://github.com/mozilla/MakeAPI/pull/113

Time to get some eyes on this.

NOTE: This doesn't mean we're ready for third party access.  We still need to figure out how to handle the identity of creator and tag filtering.

Comment 7

[Chris DeCairos (:cade)]

Attachment: https://github.com/mozilla/makeapi-client/pull/6

Comment 8

[Matthew Schranz [:mjschranz]]

Comment on attachment 778527 [details] [review]
https://github.com/mozilla/MakeAPI/pull/113

See pull request comments.

Comment 9

[Matthew Schranz [:mjschranz]]

Comment on attachment 778531 [details] [review]
https://github.com/mozilla/makeapi-client/pull/6

See pull request comments.

Comment 10

[Matthew Schranz [:mjschranz]]

Comment on attachment 778531 [details] [review]
https://github.com/mozilla/makeapi-client/pull/6

Again, get others to review.

Comment 11

[Matthew Schranz [:mjschranz]]

Comment on attachment 778527 [details] [review]
https://github.com/mozilla/MakeAPI/pull/113

There seems to be an issue with your hawk error handling. The error object I get in my callbacks is null for some reason.

Comment 12

[Chris DeCairos (:cade)]

STRs?

Comment 13

[Matthew Schranz [:mjschranz]]

I was just trying to send a new make to my local make server with these patches running.

Now, the error itself isn't important. It happened because it wasn't getting passed new hawk authorization credentials. However the error object I got back was null, making the make itself null and then causing the popcorn maker server to crash.

Comment 14

[Chris DeCairos (:cade)]

(In reply to Matthew Schranz [:mjschranz] from comment #13)
> I was just trying to send a new make to my local make server with these
> patches running.

How were you sending it? pastebin plz

> Now, the error itself isn't important. It happened because it wasn't getting
> passed new hawk authorization credentials. However the error object I got

"getting passed new hawk autorization credentials" <- wat?

> back was null, making the make itself null and then causing the popcorn
> maker server to crash.

Comment 15

[Chris DeCairos (:cade)]

Attachment: https://github.com/mozilla/thimble.webmaker.org/pull/182

This patch updates Thimble to use the new API Key Authentication model in the MakeAPI

Comment 16

[Chris DeCairos (:cade)]

Attachment: https://github.com/mozilla/webmaker.org/pull/282

Updates webmaker.org with API Key Authentication (for delete operations)

Comment 17

[Matthew Schranz [:mjschranz]]

(In reply to Chris DeCairos (:cade) from comment #14)
> (In reply to Matthew Schranz [:mjschranz] from comment #13)
> > I was just trying to send a new make to my local make server with these
> > patches running.
> 
> How were you sending it? pastebin plz

https://github.com/mozilla/popcorn.webmaker.org/blob/master/lib/middleware.js#L64-L81

I used an untouched Popcorn Maker. It wasn't passing any of the new "hawk" credentials. It fails your middleware check (as expected) and hits https://github.com/mozilla/MakeAPI/pull/113/files#L1R96. That responding function doesn't return a proper error object. If you look back at the link for how Popcorn Maker sends new makes to the MakeAPI, it should hit the error check. However it does and crashes on line 96.

Comment 18

[Chris DeCairos (:cade)]

It shouldn't even be tested with an out of date makeapi-client version.

I'm working on a patch for popcorn maker. lets test with up to date code.

Comment 19

[Chris DeCairos (:cade)]

Attachment: https://github.com/mozilla/popcorn.webmaker.org/pull/127

MakeAPI ... API Key Authentication for Popcorn Maker

Comment 20

[Chris DeCairos (:cade)]

NOTE: All of thimble, popcorn maker and webmaker.org will need the hawk supporting branch of makeapi-client.

I discovered the awesomeness of npm link today, https://npmjs.org/doc/link.html (props to @daleee) use the docs there to easily set up each to use a local checked out version of makeapi-client

Also, you'll have to go into the admin console on the MakeAPI and issue a set of keys. Pass those keys to your app .env's or configs (for popcorn maker)

Comment 21

[Jon Buckley [:jbuck]]

Comment on attachment 778527 [details] [review]
https://github.com/mozilla/MakeAPI/pull/113

Some suggestions in the patch, but this looks pretty close

Comment 22

[Jon Buckley [:jbuck]]

Comment on attachment 778531 [details] [review]
https://github.com/mozilla/makeapi-client/pull/6

Three things to look at

Comment 23

[Chris DeCairos (:cade)]

Comment on attachment 778531 [details] [review]
https://github.com/mozilla/makeapi-client/pull/6

updated!

Comment 24

[Chris DeCairos (:cade)]

Comment on attachment 778527 [details] [review]
https://github.com/mozilla/MakeAPI/pull/113

updated too!

Comment 25

[Jon Buckley [:jbuck]]

Comment on attachment 778531 [details] [review]
https://github.com/mozilla/makeapi-client/pull/6

One nit about an API path in https://github.com/mozilla/makeapi-client/pull/6#discussion-diff-5376781, r+ with addressing it either way

Comment 26

[Jon Buckley [:jbuck]]

Comment on attachment 778527 [details] [review]
https://github.com/mozilla/MakeAPI/pull/113

r+, with the potential nit that if you change the path for `makes` vs `make` then it'll need to be updated here

Comment 27

[[github robot]]

Commit pushed to master at https://github.com/mozilla/MakeAPI

https://github.com/mozilla/MakeAPI/commit/daccbfa273503fef9a2b8267bb0bb7066a32ebd5
Bug 869951 - API Key Authentication using Hawk

Comment 28

[Chris DeCairos (:cade)]

Attachment: https://github.com/mozilla/MakeAPI/pull/117

I forgot that I tied up a loose end when POSTing a new make. no longer is it 

{ maker: "", make: { /* make data */ } }

it's just:

{ /* make data */ }

I've added some code to make sure we handle both kinds (for now)

Comment 29

[[github robot]]

Commit pushed to master at https://github.com/mozilla/MakeAPI

https://github.com/mozilla/MakeAPI/commit/71a92a555c9874fd18b13acc9001b386baffb52a
Bug 869951 - enable backwards compatible tag filtering

Comment 30

[[github robot]]

Commit pushed to master at https://github.com/mozilla/MakeAPI

https://github.com/mozilla/MakeAPI/commit/14611866efe9121911f13fefa2183a907f96f167
Bug 869951 - Port override for Hawk if SSL is on

Comment 31

[Chris DeCairos (:cade)]

Attachment: https://github.com/mozilla/MakeAPI/pull/118

Fix declaring a variable out of scope.

Comment 32

[[github robot]]

Commit pushed to master at https://github.com/mozilla/MakeAPI

https://github.com/mozilla/MakeAPI/commit/dc912248464f8216d0492730842ce6c60620d7e6
Bug 869951 - Declare hawkOptions in scope of it's use

Comment 33

[[github robot]]

Commit pushed to master at https://github.com/mozilla/popcorn.webmaker.org

https://github.com/mozilla/popcorn.webmaker.org/commit/e0bac3b004b883ed89930d285398ecee4bd0ba06
Bug 869951 - API Key Authentication with MakeAPI

Comment 34

[[github robot]]

Commit pushed to master at https://github.com/mozilla/webmaker.org

https://github.com/mozilla/webmaker.org/commit/8fd030c5ca52b5039f1fed3a1c96b48c4d44b571
Bug 869951 - API Key Authentication with MakeAPI

Comment 35

[[github robot]]

Commit pushed to master at https://github.com/mozilla/thimble.webmaker.org

https://github.com/mozilla/thimble.webmaker.org/commit/a6608a6ed576677e682db59b5e012add3ef0f8b5
Bug 869951 - API Key Authentication with MakeAPI

Comment 36

[Chris DeCairos (:cade)]

Shipped MakeAPI, Popcorn Maker, Thimble and Webmaker.org to Production and Enabled API key Authentication.

Filed Bug 898138 to remove the deprecated API
Filed Bug 898139 to add in Response validation in makeapi-client