Add endpoint for permissions

Add endpoint for permissions

BUG=631047
Committed: https://github.com/luci/luci-py/commit/0be022d38c8efbb5add3933ba1cc89cc22340db3

[kjlubick, 2016-08-25 17:15:05]

kjlubick@google.com changed reviewers:
+ maruel@chromium.org, vadimsh@chromium.org

[M-A Ruel, 2016-08-25 18:22:23]

I was more thinking about inquiring about the groups the current user is member
of as part of the auth_service API, instead of querying the capabilities on
swarming. I'd like Vadim's opinion about this. I don't mind much either way, I
just want to know if he has any strong opinion about this.

https://codereview.chromium.org/2272193004/diff/1/appengine/swarming/handlers...
File appengine/swarming/handlers_endpoints.py (right):

https://codereview.chromium.org/2272193004/diff/1/appengine/swarming/handlers...
appengine/swarming/handlers_endpoints.py:148: can_get_bootstrap_py =
acl.is_user(),
bootstrap and bot_config are redundant

[kjlubick, 2016-08-25 18:29:33]

On 2016/08/25 at 18:22:23, maruel wrote:
> I was more thinking about inquiring about the groups the current user is
member of as part of the auth_service API, instead of querying the capabilities
on swarming. I'd like Vadim's opinion about this. I don't mind much either way,
I just want to know if he has any strong opinion about this.
> 
>
https://codereview.chromium.org/2272193004/diff/1/appengine/swarming/handlers...
> File appengine/swarming/handlers_endpoints.py (right):
> 
>
https://codereview.chromium.org/2272193004/diff/1/appengine/swarming/handlers...
> appengine/swarming/handlers_endpoints.py:148: can_get_bootstrap_py =
acl.is_user(),
> bootstrap and bot_config are redundant

From my chat with Vadim yesterday: 
Vadim: "instead of Javascript calling chrome-infra-auth.appspot.com API
directly, it should just use some (new) field from Swarming API response
"can_delete_bot" or something.
Perhaps it requires new Swarming API endpoint. that way name of
"swarming-admins" group (and the entier logic of who can delete bots etc) stays
in the single place - Swarming backend code"

Me: "Okay.  So new endpoint swarming.server.permissions will return an object
like {can_delete_bots:false, can_terminate_bot:true, can_cancel_task:true,...}
And then if we decide swarming-admins should be the only ones that can terminate
bots, we only need to change the backend instead of backend and frontend. "

Vadim: "yes"

[Vadim Sh., 2016-08-25 18:59:12]

I think duplicating ACL checks between Python and Javascript code is bad. At
some point they may become more complicated (e.g. bot owners can terminate bots
that they own, and not some other bots), and so it's better to keep them in
single place.

Another way to implement this is to modify existing responses to include
permission related fields. For example, bot dicts in bot listing API can include
a field "permissions" set to ["terminate", "delete"]. Task dicts: "permissions":
["cancel"]. Server details dict: ["modify_bot_config", "bootstrap"].

This approach may save HTTP calls, and is more flexible (we can implement bot
owners simpler in the future that way).

But if it's easier to have at as separate call for now, I don't mind.

M-A, what do you think?

[kjlubick, 2016-08-25 19:33:37]

https://codereview.chromium.org/2272193004/diff/1/appengine/swarming/handlers...
File appengine/swarming/handlers_endpoints.py (right):

https://codereview.chromium.org/2272193004/diff/1/appengine/swarming/handlers...
appengine/swarming/handlers_endpoints.py:148: can_get_bootstrap_py =
acl.is_user(),
On 2016/08/25 at 18:22:22, M-A Ruel wrote:
> bootstrap and bot_config are redundant

Consolidated to can_get_configs and can_put_configs

[M-A Ruel, 2016-08-25 19:49:47]

On 2016/08/25 18:59:12, Vadim Sh. wrote:
> I think duplicating ACL checks between Python and Javascript code is bad.

This is the main issue and I agree. I'm fine with keeping it simple and revisit
later if we need something more complex like Vadim noted.

Few nits about naming.

https://codereview.chromium.org/2272193004/diff/20001/appengine/swarming/hand...
File appengine/swarming/handlers_endpoints.py (right):

https://codereview.chromium.org/2272193004/diff/20001/appengine/swarming/hand...
appengine/swarming/handlers_endpoints.py:145: return
swarming_rpcs.ServerPermissions(
Server is kind of odd, it's ClientPermissions ?

https://codereview.chromium.org/2272193004/diff/20001/appengine/swarming/hand...
appengine/swarming/handlers_endpoints.py:146: can_delete_bot = acl.is_admin(),
Can we remove the "can_" prefixes? :)

https://codereview.chromium.org/2272193004/diff/20001/appengine/swarming/swar...
File appengine/swarming/swarming_rpcs.py (right):

https://codereview.chromium.org/2272193004/diff/20001/appengine/swarming/swar...
appengine/swarming/swarming_rpcs.py:75: """Reports the server version."""
version? :)

[kjlubick, 2016-08-25 20:21:03]

https://codereview.chromium.org/2272193004/diff/20001/appengine/swarming/hand...
File appengine/swarming/handlers_endpoints.py (right):

https://codereview.chromium.org/2272193004/diff/20001/appengine/swarming/hand...
appengine/swarming/handlers_endpoints.py:145: return
swarming_rpcs.ServerPermissions(
On 2016/08/25 at 19:49:47, M-A Ruel wrote:
> Server is kind of odd, it's ClientPermissions ?

Done.

https://codereview.chromium.org/2272193004/diff/20001/appengine/swarming/hand...
appengine/swarming/handlers_endpoints.py:146: can_delete_bot = acl.is_admin(),
On 2016/08/25 at 19:49:47, M-A Ruel wrote:
> Can we remove the "can_" prefixes? :)

Done.

https://codereview.chromium.org/2272193004/diff/20001/appengine/swarming/swar...
File appengine/swarming/swarming_rpcs.py (right):

https://codereview.chromium.org/2272193004/diff/20001/appengine/swarming/swar...
appengine/swarming/swarming_rpcs.py:75: """Reports the server version."""
On 2016/08/25 at 19:49:47, M-A Ruel wrote:
> version? :)
Fixed.

[M-A Ruel, 2016-08-26 00:32:14]

lgtm

[Vadim Sh., 2016-08-26 17:45:32]

lgtm

[kjlubick, 2016-08-26 17:45:45]

The CQ bit was checked by kjlubick@google.com

[commit-bot: I haz the power, 2016-08-26 17:45:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-26 17:49:12]

Description was changed from

==========
Add endpoint for permissions

BUG=631047
==========

to

==========
Add endpoint for permissions

BUG=631047

Committed:
https://github.com/luci/luci-py/commit/0be022d38c8efbb5add3933ba1cc89cc22340db3
==========

[commit-bot: I haz the power, 2016-08-26 17:49:13]

Committed patchset #3 (id:40001) as
https://github.com/luci/luci-py/commit/0be022d38c8efbb5add3933ba1cc89cc22340db3