New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 806080 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 781611



Sign in to add a comment

buildbucket access: does not initialize auth context

Project Member Reported by no...@chromium.org, Jan 25 2018

Issue description

Today buildbucket's access service does not initialize auth context, so all requests, authenticated or not, are seen as sent from "anonymous:anonymous". In other words, buildbucket's Access service is currently useless.
 

Comment 1 by no...@chromium.org, Jan 25 2018

Status: Available (was: Untriaged)
Vadim suggested that 

https://chromium.googlesource.com/infra/luci/luci-py/+/64a565f50162fe43b6e9d2dac83ed9d13714f3cb/appengine/components/components/auth/endpoints_support.py#224

could be extracted into something reusable.

Comment 2 by no...@chromium.org, Feb 9 2018

Cc: -mknyszek@chromium.org mknyszek@google.com
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/a4421c9b2912a3ab352185d97d8716f4e7d22f20

commit a4421c9b2912a3ab352185d97d8716f4e7d22f20
Author: Nodir Turakulov <nodir@chromium.org>
Date: Mon Feb 12 18:05:05 2018

Revert "[milo] Add ACL checks for builder page."

This reverts commit 963d2795099a574ab459c1ecc71a13541ebc09d9.

Reason for revert: incorrectly prevents users from accessing chromium/luci.chromium.findit/findit_variable builder which is restricted to googlers. Root cause:  crbug.com/806080 

Original change's description:
> [milo] Add ACL checks for builder page.
>
> Bug: 781611
> Change-Id: I14f3caf416a2dcc68cbb96d9c16249bff20ce996
> Reviewed-on: https://chromium-review.googlesource.com/820294
> Commit-Queue: Michael Knyszek <mknyszek@google.com>
> Reviewed-by: Nodir Turakulov <nodir@chromium.org>

TBR=vadimsh@chromium.org,nodir@chromium.org,mknyszek@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 781611, 806080 
Change-Id: Iffcbb44d3731ab1e3a31f3ec39d450116444789e
Reviewed-on: https://chromium-review.googlesource.com/914168
Commit-Queue: Nodir Turakulov <nodir@chromium.org>
Reviewed-by: Nodir Turakulov <nodir@chromium.org>

[modify] https://crrev.com/a4421c9b2912a3ab352185d97d8716f4e7d22f20/milo/frontend/view_builder.go

Comment 4 by estaab@chromium.org, Feb 22 2018

Cc: vadimsh@chromium.org
Labels: LUCI-Blocker-ChromeInternal
Vadim, do you think you can take a look at this? We will need it to unblock internal migrations.
Owner: vadimsh@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 26 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/53783f3e4ea7c15f43244f36a402bca655cb464d

commit 53783f3e4ea7c15f43244f36a402bca655cb464d
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Mon Feb 26 21:56:27 2018

[auth] Allow passing AuthDB to check_bearer_delegation_token explicitly.

This will be somewhat useful in subsequent refactoring that deduplicates some
auth related code in auth.ApiHandler, Cloud Endpoints wrapper and (soon to
be added) pRPC auth interceptor.

BUG= 806080 
R=nodir@chromium.org

Change-Id: I6bb4b9761c18ebbacc28397dc9470ebd0d72e405
Reviewed-on: https://chromium-review.googlesource.com/938397
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/53783f3e4ea7c15f43244f36a402bca655cb464d/appengine/components/components/auth/delegation.py
[modify] https://crrev.com/53783f3e4ea7c15f43244f36a402bca655cb464d/appengine/components/components/auth/delegation_test.py

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/7cd2310f71f8e574db3dfb3ba297e2eb4e75ab09

commit 7cd2310f71f8e574db3dfb3ba297e2eb4e75ab09
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Tue Feb 27 00:01:08 2018

[prpc] Add support for interceptors.

Will be used for auth and monitoring. The design vaguely resembles
grpc.ServerInterceptor, except we use a callback instead of an abstract class,
and it returns the response (and not grpc.RpcMethodHandler).

Also fix few minor bugs while at it:
  * Make None response with OK code cause INTERNAL error.
  * set_code is always expecting a StatusCode.*, not just an int. Add assert.

R=nodir@chromium.org, mknyszek@google.com
BUG= 806080 

Change-Id: Iab220d1fc10a5acbe8ebd86e089cdaa26111a91b
Reviewed-on: https://chromium-review.googlesource.com/936261
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>
Reviewed-by: Nodir Turakulov <nodir@chromium.org>

[add] https://crrev.com/7cd2310f71f8e574db3dfb3ba297e2eb4e75ab09/appengine/components/components/prpc/codes.py
[modify] https://crrev.com/7cd2310f71f8e574db3dfb3ba297e2eb4e75ab09/appengine/components/components/prpc/context.py
[modify] https://crrev.com/7cd2310f71f8e574db3dfb3ba297e2eb4e75ab09/appengine/components/components/prpc/encoding.py
[modify] https://crrev.com/7cd2310f71f8e574db3dfb3ba297e2eb4e75ab09/appengine/components/components/prpc/headers.py
[modify] https://crrev.com/7cd2310f71f8e574db3dfb3ba297e2eb4e75ab09/appengine/components/components/prpc/headers_test.py
[modify] https://crrev.com/7cd2310f71f8e574db3dfb3ba297e2eb4e75ab09/appengine/components/components/prpc/server.py
[modify] https://crrev.com/7cd2310f71f8e574db3dfb3ba297e2eb4e75ab09/appengine/components/components/prpc/server_test.py

Project Member

Comment 11 by bugdroid1@chromium.org, Feb 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/aae6f01c114c3aa53b66d1531b00ef4a1aaf8d51

commit aae6f01c114c3aa53b66d1531b00ef4a1aaf8d51
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Tue Feb 27 03:35:16 2018

[auth] Make check_oauth_access_token accept only one header value.

It used to accept all headers, but then used only values for Authorization key.

pRPC handlers don't have headers dict (and have different convention for how to
encode header keys), so it would be cleaner just to pass the value of
Authorization header directly.

BUG= 806080 
R=nodir@chromium.org

Change-Id: If6b90b27314b1fca5e14f50ceca1b45253585c1a
Reviewed-on: https://chromium-review.googlesource.com/938977
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/aae6f01c114c3aa53b66d1531b00ef4a1aaf8d51/appengine/components/components/auth/api.py
[modify] https://crrev.com/aae6f01c114c3aa53b66d1531b00ef4a1aaf8d51/appengine/components/components/auth/endpoints_support.py
[modify] https://crrev.com/aae6f01c114c3aa53b66d1531b00ef4a1aaf8d51/appengine/components/components/auth/handler.py

Project Member

Comment 12 by bugdroid1@chromium.org, Feb 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/886d1c5e8b02df3ed771982802dae1dca93129df

commit 886d1c5e8b02df3ed771982802dae1dca93129df
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Wed Feb 28 04:01:25 2018

[auth] Add pRPC interceptor that sets up auth context.

Validates Authorization header, delegation tokens and checks IP whitelist.
On success updates the auth context in the thread-local storage. This makes
various components.auth functions work from inside pRPC handlers.

Additionally it catches auth.AuthenticationError and auth.AuthorizationError
and converts them to corresponding gRPC status codes, with logging matching
what we have for Cloud Endpoints and webapp2 handlers.

BUG= 806080 
R=nodir@chromium.org

Change-Id: I8eec19e1c0d52a3cb2847c59abb8c90f1961c3ce
Reviewed-on: https://chromium-review.googlesource.com/939048
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/886d1c5e8b02df3ed771982802dae1dca93129df/appengine/components/components/auth/__init__.py
[add] https://crrev.com/886d1c5e8b02df3ed771982802dae1dca93129df/appengine/components/components/auth/prpc.py
[add] https://crrev.com/886d1c5e8b02df3ed771982802dae1dca93129df/appengine/components/components/auth/prpc_test.py
[modify] https://crrev.com/886d1c5e8b02df3ed771982802dae1dca93129df/appengine/components/components/auth/version.py

Project Member

Comment 13 by bugdroid1@chromium.org, Feb 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/1c744baf9681aa48ef842f1adbda2dd166f342aa

commit 1c744baf9681aa48ef842f1adbda2dd166f342aa
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Wed Feb 28 04:20:02 2018

Roll luci DEPS.

infra/luci:
886d1c5e [auth] Add pRPC interceptor that sets up auth context.
5dc02ee6 [gerrit] make functions async
7d491717 Swarming: fix fix_python_cmd again
47af739c Swarming: fix the exception in tidy_stale
432378e2 Swarming: do not crash in _get_os_numbers() if cmd.exe is hosed.

TBR=nodir@chromium.org
BUG= 806080 

Change-Id: I9bb5404718136ce8271fe207199a265af89e7423
Reviewed-on: https://chromium-review.googlesource.com/940806
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/1c744baf9681aa48ef842f1adbda2dd166f342aa/DEPS

Project Member

Comment 14 by bugdroid1@chromium.org, Feb 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/500abb7b12d67e940d0f32c8b9f801d7ea49df0a

commit 500abb7b12d67e940d0f32c8b9f801d7ea49df0a
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Wed Feb 28 20:48:42 2018

[cr-buildbucket] Enable auth for pRPC APIs.

R=nodir@chromium.org
BUG= 806080 

Change-Id: I29ccc14b015c9d02ff2687be87d3da5278d3a4af
Reviewed-on: https://chromium-review.googlesource.com/940811
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/500abb7b12d67e940d0f32c8b9f801d7ea49df0a/appengine/cr-buildbucket/access/api.py

Status: Fixed (was: Assigned)

Sign in to add a comment