866832 - Have webmaker id passed to create/update/delete for tag verification

Status: RESOLVED FIXED

(Webmaker Graveyard :: MakeAPI, defect)

Description

[Chris DeCairos (:cade)]

In order for tag "domains" ( "webmaker.org", "popcorn.webmaker.org", etc ) to be applied only by authenticated users, we need to have the API authenticate accounts against the webmaker login server.

Admins should be able to apply the reserved domains on top of their own email. This will allow tagged approval for promotion publicly.

Apps ( like Thimble or Popcorn Maker) should be able to update makes on behalf of logged in users, OR, logged in users can directly update certain fields of makes. i.e. tag a project as favourite or add a hashtag to a project of their own.

Comment 1

[Simon C. Wex]

Relevant tag documentation:

https://wex.etherpad.mozilla.org/MakeAPI-tags

Comment 2

[Chris DeCairos (:cade)]

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

I did a first pass at this. 

Basically, Create update and delete will be secured with basic authentication. Apps/tools that we trust will authenticate users and make API calls on their behalf, passing the Make API their id and a flag indicating if they're an administrator (AFAIK, that information will be in the encrypted session cookie)

This patch adds a new piece of middleware that checks if tags are being added/changed and applies the security logic that was in the requirements for application tags. i.e. admin can add any application tag, user can only add and application tag that contains their own webmakerid before the ':'

Comment 3

[Kieran Sedgwick (:sedge)]

:cade - so in other words, you're relying on the other apps NEVER making a call to the makeAPI unless they know there's an authenticated user?

Comment 4

[Chris DeCairos (:cade)]

Yup. All actions that'd trigger a call to Create/update/delete on the Make API in Popcorn Maker or Thimble already require authentication with persona/webmaker, so there's not much to do there right now, just have to get those apps passing the id and admin flag with their API calls.

Comment 5

[Chris DeCairos (:cade)]

Altering to title to better reflect the work done.

Comment 6

[Jon Buckley]

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

r-, notes in the pull request.

What's a good way to test this? The localdata generator doesn't add admin tags, right?

Comment 7

[Chris DeCairos (:cade)]

No, I don't believe it does at this point in time.

Comment 9

[Chris DeCairos (:cade)]

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

This patch supports tag filtering based on the rules we've set out for tags in https://wex.etherpad.mozilla.org/MakeAPI-tags

Regular users can add raw tags, and user tags (userid:tag) all other user tags and application tags are stripped from makes.

Admins can add any tag.

I still have to add in logic that allows apps to add application tags, so that they can apply tags with it's their own specific application tags. (popcorn.webmaker.org:project,thimble.webmaker.org:project)

Lets double check that all tag rules are being enforced.

Comment 10

[David Humphrey (:humph)]

Chris, README looks pretty out of sync with the state of the code.  Can you confirm this is right?

Comment 11

[David Humphrey (:humph)]

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

Comments in PR

Comment 13

[Chris DeCairos (:cade)]

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

I think I got all of issues fixed from last week. next round of review!

Comment 14

[Chris DeCairos (:cade)]

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

I've asked :jbuck for a review

Comment 15

[Chris DeCairos (:cade)]

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

one step closer!

Comment 16

[David Humphrey (:humph)]

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

Few things in the PR, r+ with that.

Comment 17

[Chris DeCairos (:cade)]

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

Landing....

Comment 18

[[github robot]]

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

https://github.com/mozilla/MakeAPI/commit/2e89dc4010ee260c5df9337126c4900634f8a951
Fixes Bug 866832 - Tag Filtering for Make Creation and Updating