Data output agent

[cantino]

This pull request adds a new DataOutputAgent that returns a RSS or JSON feed of incoming events. In order to do this, the EventHooksController will now accept all HTTP verbs, not just POSTs. (Perhaps the controller should now be renamed to WebRequestsController, or similar, since webhooks are traditionally limited to POST requests?)

This is a solution to the output half of #11. Appreciate feedback @albertsun, @alias1, @Fluffums, and @snicker!

[ixti]

IMHO this example is not very needed as route uses default Rails behavior and it was tested in Rails :D

[cantino]

Fair enough-- match goes away in Rails 4, though, so this is somewhat of a paranoid guard.

[ixti]

AFAIK it will not be removed completely. In Rails 4, match without :via will be prohibited, so

match "/users/:user_id/webhooks/:agent_id/:secret" => \
  "webhooks#handle_request",
    :as => :webhooks

will be replaced with:

match "/users/:user_id/webhooks/:agent_id/:secret" => \
  "webhooks#handle_request",
    :as => :webhooks,
    :via => :all

[0xdevalias]

Just had a very brief scan over it all and it looks kinda cool to me. Without getting deep into the guts of it I feel I can't really comment further though.

I'd support changing the name to WebRequestsController. Also, I wonder if the webhooks part of the route is the best name now, given it's expanded uses?

[cantino]

Yup, just did that now!

On Sat, Apr 5, 2014 at 6:43 PM, alias1 notifications@github.com wrote:

Just had a very brief scan over it all and it looks kinda cool to me.
Without getting deep into the guts of it I feel I can't really comment
further though.

I'd support changing the name to WebRequestsController. Also, I wonder if
the webhooks part of the route is the best name now, given it's expanded
uses?

Reply to this email directly or view it on GitHubhttps://github.com//pull/203#issuecomment-39656165
.

[albertsun]

I'd agree about changing the name to WebRequestsController.

For the url, I wonder if instead of having secret as a URL segment it should be passed as a query string parameter instead? Would make it look a bit better for un-protected routes. And in the future the url won't need to change to support oauth style access_key=?&secret_key=? tokens.

[0xdevalias]

Query string isn't really RESTful though. Under what circumstances would it not be protected? Even if you end up publishing the 'secret' url somewhere public, the design should still be such that it's kept secret (eg. how do you differentiate different endpoints without the varying key?)

For OAUTH, as I understand it, access/secret should only ever be used as part of the url between servers over SSL. I would imagine that most if not all usecases for this would call for something more public/flexible than this, in which case you would be passing the access_token (usually) via Auth header.

Basically just braindumping, so feel free to question/correct me.

[albertsun]

Ah, fair enough point about OAUTH. I don't really fully understand how that part works.

As far as query string being RESTful though... isn't the agent_id enough of an identifier? Whatever secret is in the url won't change the data being delivered, right? It'll always just be all the events that the DataOutputAgent has. The secret feels like secondary information to me.

The most common case I can imagine for using this will be to set up a DataOutputAgent to a WebsiteAgent to make an ersatz RSS feed and then publishing a public URL that anyone can point their RSS feed reader at to subscribe to.

[cantino]

It's true that :secret doesn't really need to be in the path. I guess I could add an alternative route that doesn't require it, and move it to a query param (as per @albertsun's suggestion). That would give Agent's maximum flexibility. My original thought was to include it, forcing Agents to be as secure as possible.

[0xdevalias]

I'm definitely a fan of secure by design. Given that you can still do all of the 'public' user cases as is, i'd be inclined to keep it in the path.

(But to answer the above, yeah the agent id is sufficient to uniquely identify them, didn't really think about that aspect)