Adding public report support.

Adding public report support.

For a public report, the user needs to SetSinglePrivilegedRequest in order to access some row data.
To ensure that unauthorized users can't see data they shouldn't, I've also added a check that each TestMetaData is viewable.

BUG=catapult:#3141

Review-Url: https://codereview.chromium.org/2784983002
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/94d1bb9f4ab0a7fbbabe5018ffa0578b054fa89c

[jessimb, 2017-03-29 22:04:32]

Description was changed from

==========
Adding public report support.

For a public report, the user needs to SetSinglePrivilegedRequest in order to
access some row data.
To ensure that unauthorized users can't see data they shouldn't I've also added
a check that each TestMetaData is view-able.

BUG=catapult:#3141
==========

to

==========
Adding public report support.

For a public report, the user needs to SetSinglePrivilegedRequest in order to
access some row data.
To ensure that unauthorized users can't see data they shouldn't, I've also added
a check that each TestMetaData is viewable.

BUG=catapult:#3141
==========

[jessimb, 2017-03-29 22:05:30]

jessimb@chromium.org changed reviewers:
+ eakuefner@chromium.org, sullivan@chromium.org

[jessimb, 2017-03-29 22:05:30]

PTAL.

I'm not 100% sure I need the permissions check, but this is what I was thinking.
If we decide against it, this CL becomes a one-liner.

Thanks!

[eakuefner, 2017-03-30 00:22:14]

You're getting the TestMetadatas in _GetTestToUnitsMap so I feel like the check
isn't necessary. However, if it is (Annie?), please parallelize.

lgtm

[sullivan, 2017-03-30 13:22:54]

https://codereview.chromium.org/2784983002/diff/1/dashboard/dashboard/speed_r...
File dashboard/dashboard/speed_releasing.py (right):

https://codereview.chromium.org/2784983002/diff/1/dashboard/dashboard/speed_r...
dashboard/dashboard/speed_releasing.py:245: return True
Won't this throw an exception if it fails?

Also can do a get_multi() to speed it up a bit.

[jessimb, 2017-03-30 15:49:37]

On 2017/03/30 at 00:22:14, eakuefner wrote:
> You're getting the TestMetadatas in _GetTestToUnitsMap so I feel like the
check isn't necessary. However, if it is (Annie?), please parallelize.
> 
> lgtm

I'm only getting the TestMetaData for the first bot, not all of them. If the
first bot isn't internal only but the second one is, then maybe there is a
vulnerability. I'm not sure. Perhaps it doesn't matter anyway because the
table_config itself is protected by internal only.

[jessimb, 2017-03-30 17:10:58]

PTAL.

I removed the check because the internal_only on table_config should block out
any internal_only data.

[sullivan, 2017-03-30 17:46:46]

Is there already a test that verifies what is shown to users without access?

https://codereview.chromium.org/2784983002/diff/20001/dashboard/dashboard/spe...
File dashboard/dashboard/speed_releasing.py (right):

https://codereview.chromium.org/2784983002/diff/20001/dashboard/dashboard/spe...
dashboard/dashboard/speed_releasing.py:320:
datastore_hooks.SetSinglePrivilegedRequest()
Should add a comment explaining why it's okay to do this.

[jessimb, 2017-03-30 17:55:33]

PTAL.

Yes,
https://github.com/catapult-project/catapult/blob/master/dashboard/dashboard/...
tests what happens when you go to an internal_only report as a non-internal
user.

https://codereview.chromium.org/2784983002/diff/20001/dashboard/dashboard/spe...
File dashboard/dashboard/speed_releasing.py (right):

https://codereview.chromium.org/2784983002/diff/20001/dashboard/dashboard/spe...
dashboard/dashboard/speed_releasing.py:320:
datastore_hooks.SetSinglePrivilegedRequest()
On 2017/03/30 at 17:46:45, sullivan wrote:
> Should add a comment explaining why it's okay to do this.

Done

[sullivan, 2017-03-30 18:00:17]

lgtm

[jessimb, 2017-03-30 18:01:17]

The CQ bit was checked by jessimb@chromium.org

[jessimb, 2017-03-30 18:01:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
eakuefner@chromium.org
Link to the patchset: https://codereview.chromium.org/2784983002/#ps40001
(title: "adding a comment")

[commit-bot: I haz the power, 2017-03-30 18:01:24]

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, 2017-03-30 18:23:47]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490896877605440,
"parent_rev": "b8e22b236dfce4af0f9f584760003cfcf434d891", "commit_rev":
"94d1bb9f4ab0a7fbbabe5018ffa0578b054fa89c"}

[commit-bot: I haz the power, 2017-03-30 18:23:51]

Description was changed from

==========
Adding public report support.

For a public report, the user needs to SetSinglePrivilegedRequest in order to
access some row data.
To ensure that unauthorized users can't see data they shouldn't, I've also added
a check that each TestMetaData is viewable.

BUG=catapult:#3141
==========

to

==========
Adding public report support.

For a public report, the user needs to SetSinglePrivilegedRequest in order to
access some row data.
To ensure that unauthorized users can't see data they shouldn't, I've also added
a check that each TestMetaData is viewable.

BUG=catapult:#3141

Review-Url: https://codereview.chromium.org/2784983002
Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2017-03-30 18:23:52]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...