Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: activity web hook #11

Open
jakerella opened this issue May 9, 2013 · 7 comments
Open

Feature request: activity web hook #11

jakerella opened this issue May 9, 2013 · 7 comments

Comments

@jakerella
Copy link

I would like to see an integration point for the PT activity web hook (https://www.pivotaltracker.com/help/integrations#activity_web_hook). The idea would be that Node accepts a post, and then the app passes the request on to this library which processes it, send the response, and makes a callback for processing by the app.

In pseudo-code:

// note that this is an Express post handler, but it should not require Express
app.post('/pt-activity-hook', function(req, res){
    // pivotal library would extract data and fill the response appropriately
    pivotal.handleActivityhook(req, res, function(activity) {
        // "activity" is the data in the <activity> xml node as JSON
        console.log("Activity captured!".green, activity);
        // any other processing
    });
});

I may try to implement this myself and I'll contribute if anything comes of it.

@stelcheck
Copy link
Member

Sounds like a good idea! Would you be up to contribute that feature?

Just a bit of input here:

// note that this is an Express post handler, but it should not require Express
app.post('/pt-activity-hook', function(req, res){
    res.end()
    pivotal.parseActivityHookData(req, function (activity) { 
        console.log("activity: ", activity)
    });
});

Would probably make more sense, and simplify how testable the code would be.

Some other ideas:

  • since we are passing the request, we could check if the request comes from a valid PT server (do they have a list of this somewhere perhaps?)

@jakerella
Copy link
Author

I can work on it some time, but may be a while. My company is actually working on an FOSS PT cumulative flow diagram tracker (let me know if you're interested and I'll point you to the code when it's ready).

I'm still a bit new to Node and Express... why the res.end()? Wouldn't you want to respond to the server with a positive (200) status if it was good (and maybe a 401 or 500 if it wasn't)?

As for the other idea and the checking the request, we could check the originating server... but that can be spoofed. :) What I do for my current project is turn it right around - I hit the PT API using the provided token and get two things:

  1. I get the latest activities and make sure the received one is in there; and
  2. see if that story exists for that account.

I figure that without an actual "verification" process on the PT side this is as good as it gets. Let me know what you think.

@stelcheck
Copy link
Member

The only reason why i res.end() is that I doubt that the PT API will care if you return a 401 or a 500. On the other hand, you can always refer to your log to see the outcome of the call. But yes, for clarity, you could consider waiting until processing is done before sending an HTTP response; but you don't necessarily have to.

The originating server could be spoofed indeed. However, unless the request is signed (which I don't think it is) or authenticated (which also doesn't seem the case), you will be out of luck, and that will be the best you can do.

What you do is technically no safer than working with a list of IP - perhaps only a bit more obscure, but more easily brute-forceable I think, since project and story ID's are in the low 100,000 from what I have seen so far. On the other hand, with node you should be able to retrieve the remote IP by looking at the connection's socket (see http://stackoverflow.com/questions/8102535/remote-ip-from-request-on-http-server), which should not be fakeable (at the very least, if you can send back data successfully, you should be pretty safe).

@jakerella
Copy link
Author

I see what you're saying about the IP address of the originating server, but my issue there would be that they could change those IPs! Or even just add more servers, requiring a change to the library and all existing instances to break.

As for how I'm doing the verification now, you're right, the activity POST is not signed in any way, but that's not what I'm relying on. When I get an activity, I then hit the PT API and get recent activities for the given project, and I make sure that the activity I received is present. In other words, I make sure that the activity is the same both on my receiving end, and on the PT end. I don't think that could be spoofed - although you could have duplicates.

Does that make more sense? Do you see other concerns about the verification?

@stelcheck
Copy link
Member

According to https://www.pivotaltracker.com/help/integrations, the list is clear but indeed could change without notice. But I still think this would be safer than relying on double-checks (and more efficient) to rely on this. Even if that break, since they have a list of server announced, I think it would be a quick fix to update the list and then go online (and I assume they won't change ALL their servers at once; my experience with GitHub API, for instance, is that they would add a server every once in a while, and we would then see some errors pop up, but most of the calls would make it through).

@jakerella
Copy link
Author

Ok then... I'll see what I can do on this, but it may be a while. I'll share what I have as I go though. When I do get this going, do you want me to work in a separate branch and issue a pull there?

@stelcheck
Copy link
Member

Great! No rush, if spmeone else needs it more than you do then they just
need to code it themselves.

I work by pull requests, so yes, just fork/branch/commit/push/pr so that i
xan take a quick look at the code. And let me know if you have any further
question.

P.S. when you will do your PR, please paste the link to this thread in your
PR comment for reference.

Kind regards,
On Jun 4, 2013 5:05 AM, "Jordan Kasper" [email protected] wrote:

Ok then... I'll see what I can do on this, but it may be a while. I'll
share what I have as I go though. When I do get this going, do you want me
to work in a separate branch and issue a pull there?


Reply to this email directly or view it on GitHubhttps://github.com//issues/11#issuecomment-18867197
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants