1. Atlassian
  2. Project: Atlassian
  3. atlassian-streams

Pull requests

#10 Merged
Repository
Branch
JRA-34744
Repository
Branch
master

JRA-34744 REST XSRF in JiraInlineActionResource

Author
  1. Piotr Stefan Stefaniak
Reviewers
Description

Check the https://jira.atlassian.com/browse/JRA-34744 for details.

Generally this pull-request is about wrapping the 'vulnerbable' methods (watchIssue and voteIssue) with consumes drojas annotation. Thus, forcing the clients to specify the Content-Type explicitly ("application/json"), which should prevent naive XSRF attempts.

Similarly I'm wrapping the 'url-proxy' resource with the same annotation (because the url-proxy will have from now execute requests with Content-Type set to application/json too). That change is also for preventing naive XSRF attempts using that ('url-proxy') endpoint.

  • Learn about pull requests

Comments (9)

  1. Michael Minns [Atlassian]

    I'm happy this is a consistent set of changes but looking at JRA-34744 it talks about including XSRF tokens in requests, this doesn't do that. Is this a deliberate change of plan or part of a wider fix?

  2. Piotr Stefan Stefaniak author

    Deliberate change. I don't think the XSRF tokens are necessary here (in fact, given how activity streams are embedded on one's application, I believe inserting proper xsrf/atl_tokens to the relevant requests on the fly would be tricky, if at all possible).

    Is my answer helpful to you?

  3. Piotr Stefan Stefaniak author

    No idea. :). I think that it's you who should decide - after all it's you guys there who will maintain this code in the future, aren't you?

    I didn't bother to write up such a test, because I don't really know where I should hook up with the testing. I mean, should it be the full-size functional test that would for example set up two JIRA instances and verify that you can't use hand-crafted HTML form to hit these ("watchIssue", "voteIssue", "url-proxy") endpoints? Or should it just unit test somehow the existence of the annotation existence in these endpoints? I'm not sure what would be the best trade-off here.

    Anyway, as you both hit 'approve' on this pull-request I'll assume I'm free to merge this stuff and release new version of atlassian-streams, ok? If you wish me to include tests to confirm that non-application/json requests are rejected - chime in. I'll see what I can do.

    Is that fair to you?