550732 - Allow read-only JSON-RPC methods to be called with GET

Status: RESOLVED FIXED

(Bugzilla :: WebService, enhancement)

Description

[Max Kanat-Alexander]

This is the first part of JSONP support, which is allowing JSON-RPC methods to be called with methods other than POST. (Although GET is the only reasonable method.) See the various security concerns in bug 550727.

Comment 1

[Max Kanat-Alexander]

Attachment: v1

Wow, that was tremendously complicated! Here's a few notes on how this patch works:

  * The JSON-RPC 1.1 HTTP GET specification is bad. The JSON-RPC 2.0 HTTP GET spec is better, so I went with that. However, I don't require people to specify &id= in the URL, because that's ridiculous and unnecessary. People *can* specify it if they want, though. I also don't require "params" to be Base64-encoded, because that's just stupid and pointless.

  * In order to throw errors early enough (within retrieve_json_from_get), I had to modify Bugzilla::Error slightly.

  * Both Cookie and Env auth are denied during GET requests (for future JSONP protection and because I just don't like the idea of external websites being able to get private bug data even now when they can't do anything with it). I did this by adding a new item to Bugzilla::Auth::Login, is_automatic, which should be relnoted.

  * I added a mechanism to specify which methods are read-only, and thus safe to call via GET. It's true that there are actually far more read-only methods than read-write methods, but because this is a security restriction, I want to be *explicit* about which methods we allow access to, so I want to always have to list them there.

Comment 2

[Max Kanat-Alexander]

  This patch requires the patch from bug 550618 in order to function (but that's because the JSON-RPC WebService in general requires that patch in order to function).

Comment 3

[David Lawrence [:dkl]]

Comment on attachment 430906 [details] [diff] [review]
v1

As typical when the reviewer for a patch takes too long to review it, slight bit-rot can occur :)

patching file Bugzilla/Auth/Login.pm
patching file Bugzilla/Auth/Login/Cookie.pm
patching file Bugzilla/Auth/Login/Env.pm
patching file Bugzilla/Auth/Login/Stack.pm
patching file Bugzilla/Error.pm
Hunk #1 FAILED at 127.
1 out of 1 hunk FAILED -- saving rejects to file Bugzilla/Error.pm.rej
patching file Bugzilla/WebService.pm
patching file Bugzilla/WebService/Bug.pm
patching file Bugzilla/WebService/Bugzilla.pm
patching file Bugzilla/WebService/Constants.pm
Hunk #1 FAILED at 112.
1 out of 1 hunk FAILED -- saving rejects to file Bugzilla/WebService/Constants.pm.rej
patching file Bugzilla/WebService/Product.pm
patching file Bugzilla/WebService/Server/JSONRPC.pm
Hunk #2 succeeded at 77 with fuzz 2 (offset 19 lines).
Hunk #3 succeeded at 189 (offset 19 lines).
Hunk #4 succeeded at 264 (offset 19 lines).
Hunk #5 succeeded at 317 (offset 19 lines).
Hunk #6 succeeded at 345 (offset 19 lines).
Hunk #7 succeeded at 384 (offset 19 lines).
patching file Bugzilla/WebService/User.pm
patching file template/en/default/global/user-error.html.tmpl
Hunk #1 succeeded at 979 (offset 1 line).

I will make the fixes on my end and continue to review but please submit a new version of the patch when you can.

Dave

Comment 4

[David Lawrence [:dkl]]

Comment on attachment 430906 [details] [diff] [review]
v1

I have completed my review and testing and everything works as expected and code looks good to me. You can fix the bit-rot on checkin. r=dkl

Comment 5

[Max Kanat-Alexander]

Thank you for the review! Bitrot fixed on checkin.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/                       
modified Bugzilla/WebService.pm
modified Bugzilla/Auth/Login.pm
modified Bugzilla/Auth/Login/Cookie.pm
modified Bugzilla/Auth/Login/Env.pm
modified Bugzilla/Auth/Login/Stack.pm
modified Bugzilla/WebService/Bug.pm
modified Bugzilla/WebService/Bugzilla.pm
modified Bugzilla/WebService/Constants.pm
modified Bugzilla/WebService/Product.pm                                        
modified Bugzilla/WebService/User.pm
modified Bugzilla/WebService/Server/JSONRPC.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 7142.

Comment 6

[Max Kanat-Alexander]

Added to the release notes in bug 604256.