Allow authenticated POST to get current status.

Allow authenticated POST to get current status.

Currently, status app doesn't allow authentication by bot for accessing /current . Private status apps don't give read access anonymously, and so gatekeeper-ng was not able to check current tree status. This patch fixes that.

BUG=453319
TEST=by deploying test version to AE and poking it, because unittests don't work at all.

Committed: https://chromium.googlesource.com/infra/infra/+/a0bcaf5ab4393261051f3f7fb6d51414d55ea361

[tandrii(chromium), 2015-02-02 15:54:58]

PTAL

with companion CL for gatekeeper-ng  https://codereview.chromium.org/891713003/

[tandrii(chromium), 2015-02-02 15:55:47]

sorry, the right url for gatekeeper CL is
https://codereview.chromium.org/895633002/

[tandrii(chromium), 2015-02-09 18:29:50]

tandrii@chromium.org changed reviewers:
+ stip@chromium.org

[tandrii(chromium), 2015-02-09 18:29:51]

PTAL

I almost forgot about this CL :(

[ghost stip (do not use), 2015-02-09 23:32:45]

https://chromiumcodereview.appspot.com/891713003/diff/1/appengine/chromium_st...
File appengine/chromium_status/appengine_module/chromium_status/status.py
(right):

https://chromiumcodereview.appspot.com/891713003/diff/1/appengine/chromium_st...
appengine/chromium_status/appengine_module/chromium_status/status.py:318: def
post(self):
wait, so you're overloading post() to mean 'bot access' while get() is for human
access? seems crazy.

could we try to detect the authentication token? if it's present, use bot auth,
and if it's not use requires_read_access?

[tandrii(chromium), 2015-02-10 14:18:34]

You are right. And after a fresh look, I see that no change is required at all.
The bot login is executed in _late_linit whenever read|write|bot access is
checked (as decorator). Thus, the existing @utils.requires_read_access will
execute bot login if possible.

The reason for my confusion was that bot login is allowed under https only.
Sorry for wasting your time :(

[tandrii(chromium), 2015-02-10 18:27:51]

And I am wrong yet again for closing this issue.

A new Patch Set is coming shortly.

[tandrii(chromium), 2015-02-10 18:45:45]

PTAL again.

The new patchset is simpler, but still needs POST method as explained below.

https://codereview.chromium.org/891713003/diff/1/appengine/chromium_status/ap...
File appengine/chromium_status/appengine_module/chromium_status/status.py
(right):

https://codereview.chromium.org/891713003/diff/1/appengine/chromium_status/ap...
appengine/chromium_status/appengine_module/chromium_status/status.py:318: def
post(self):
On 2015/02/09 23:32:45, stip wrote:
> wait, so you're overloading post() to mean 'bot access' while get() is for
human
> access? seems crazy.
> 
> could we try to detect the authentication token? if it's present, use bot
auth,
> and if it's not use requires_read_access?

So, I thought this is not necessary after testing with https.

However, urllib.urlopen(url, data=None,  ...) automatically uses POST if data is
not None. So we need "def post(self):", even though I don't like this.
Otherwise, we get "urllib2.HTTPError: HTTP Error 405: Method Not Allowed" .

https://codereview.chromium.org/891713003/diff/1/appengine/chromium_status/ap...
appengine/chromium_status/appengine_module/chromium_status/status.py:374: 
this is how updating status is done by gatekeeper-ng, and other tools.

[ghost stip (do not use), 2015-02-11 03:35:00]

I see. You're using POST so we can put the password in the body instead of the
URL. I guess ideally we'd send it as a header or something, but that's a bit
more of a change. lgtm

[tandrii(chromium), 2015-02-13 22:38:09]

The CQ bit was checked by tandrii@chromium.org

[commit-bot: I haz the power, 2015-02-13 22:38:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/891713003/20001

[commit-bot: I haz the power, 2015-02-13 22:41:32]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/infra/infra/+/a0bcaf5ab4393261051f3f7fb6d51...