New issue
Advanced search Search tips

Issue 767730 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 712444
issue 756224
issue 756229



Sign in to add a comment

Store current account email in LUCI_CONTEXT and expose it via common/auth API

Project Member Reported by vadimsh@chromium.org, Sep 22 2017

Issue description

When using LUCI service account authentication, some tools need to know an email associated with the access token:
 * Git requires it for 'user.email' config property, otherwise Gerrit rejects pushes.
 * gsutil requires it for some unknown mysterious reasons.

We currently can always grab an email associated with a token through an RPC to Userinfo service. But this is wasteful, since there's no natural place to "cache" the result and different tools will redo this call (perhaps multiple times, e.g. each time 'git' is invoked, gitwrapper need to get an email from somewhere).

The proposal is to add 'email' field to Account message in LUCI_CONTEXT['local_auth'] (see https://chromium.googlesource.com/infra/luci/luci-py/+/master/client/LUCI_CONTEXT.md), and make Swarming (and other implementers of LUCI_CONTEXT server protocol) populate it accordingly. 

Swarming almost always knows the email associated with the token. The only time it doesn't is when using 'service_account': 'bot' setting. In this case Swarming can grab the email _once_ during bot startup and cache it in memory forever.
 
Blocking: 756229 712444
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 22 2017

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

commit a4b81caa7496ffb6d92be79c82cd84273b0cc303
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Fri Sep 22 23:27:40 2017

Add doc for 'email' field in LUCI_CONTEXT["local_auth"].

R=iannucci@chromium.org
BUG= 767730 

Change-Id: I9e6fa4c286b2887155a26ff2db92021b0051c343
Reviewed-on: https://chromium-review.googlesource.com/679079
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/a4b81caa7496ffb6d92be79c82cd84273b0cc303/client/LUCI_CONTEXT.md

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 23 2017

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

commit c228bdc7d3213f4e3ca614178564fcfc6e85ca73
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Sat Sep 23 01:08:32 2017

auth: Add GetEmail method to Authenticator.

It returns an email associated with the token. It stores this email alongside
the token in the cache to avoid doing round trips to the token info endpoint
all the time.

It also uses different strategies for grabbing this email for different auth
providers. For example, on GCE we just ask the metadata server, which is faster
and more reliable than using token info endpoint.

LUCI_CONTEXT-based method for grabbing an email is not implemented yet. It will
be done in a separate CL (by storing the email in local_auth struct).

Implementation consists of replacing *oauth2.Token with *internal.Token
everywhere inside auth library guts, where internal.Token is just a pair
(oauth.Token, email). Public API still operates in terms of *oauth2.Tokens.

R=iannucci@chromium.org
BUG= 767730 

Change-Id: I350f9244521ec645ddc1d3376322943831114d95
Reviewed-on: https://chromium-review.googlesource.com/679439
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>

[modify] https://crrev.com/c228bdc7d3213f4e3ca614178564fcfc6e85ca73/common/auth/auth.go
[modify] https://crrev.com/c228bdc7d3213f4e3ca614178564fcfc6e85ca73/common/auth/auth_test.go
[modify] https://crrev.com/c228bdc7d3213f4e3ca614178564fcfc6e85ca73/common/auth/internal/common.go
[modify] https://crrev.com/c228bdc7d3213f4e3ca614178564fcfc6e85ca73/common/auth/internal/common_test.go
[modify] https://crrev.com/c228bdc7d3213f4e3ca614178564fcfc6e85ca73/common/auth/internal/disk_cache.go
[modify] https://crrev.com/c228bdc7d3213f4e3ca614178564fcfc6e85ca73/common/auth/internal/disk_cache_test.go
[modify] https://crrev.com/c228bdc7d3213f4e3ca614178564fcfc6e85ca73/common/auth/internal/gce.go
[modify] https://crrev.com/c228bdc7d3213f4e3ca614178564fcfc6e85ca73/common/auth/internal/iam.go
[modify] https://crrev.com/c228bdc7d3213f4e3ca614178564fcfc6e85ca73/common/auth/internal/luci_ctx.go
[modify] https://crrev.com/c228bdc7d3213f4e3ca614178564fcfc6e85ca73/common/auth/internal/luci_ctx_test.go
[modify] https://crrev.com/c228bdc7d3213f4e3ca614178564fcfc6e85ca73/common/auth/internal/proc_cache.go
[modify] https://crrev.com/c228bdc7d3213f4e3ca614178564fcfc6e85ca73/common/auth/internal/proc_cache_test.go
[modify] https://crrev.com/c228bdc7d3213f4e3ca614178564fcfc6e85ca73/common/auth/internal/service.go
[modify] https://crrev.com/c228bdc7d3213f4e3ca614178564fcfc6e85ca73/common/auth/internal/user.go

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 26 2017

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 26 2017

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

commit de3fc344c2c849e54e6f300a9ac9b5b65201f2f3
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Tue Sep 26 01:51:40 2017

auth: Avoid doing RPCs when getting an email, if possible.

When building a LUCI_CONTEXT with emails we need to get an email before we
actually get the token (email is needed to populate the environment, and the
token is needed only later when clients make RPCs).

Optimize this case to avoid any RPCs, if possible. This is not possible only
when using User provider (the one with manual login), and there's no token
cached, or it is in old format (without the email).

R=iannucci@chromium.org
BUG= 767730 

Change-Id: I73df79c259b0331ea76ff4c6d1a3a17d837d62ac
Reviewed-on: https://chromium-review.googlesource.com/683253
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/de3fc344c2c849e54e6f300a9ac9b5b65201f2f3/common/auth/auth.go
[modify] https://crrev.com/de3fc344c2c849e54e6f300a9ac9b5b65201f2f3/common/auth/auth_test.go
[modify] https://crrev.com/de3fc344c2c849e54e6f300a9ac9b5b65201f2f3/common/auth/internal/common.go
[modify] https://crrev.com/de3fc344c2c849e54e6f300a9ac9b5b65201f2f3/common/auth/internal/gce.go
[modify] https://crrev.com/de3fc344c2c849e54e6f300a9ac9b5b65201f2f3/common/auth/internal/iam.go
[modify] https://crrev.com/de3fc344c2c849e54e6f300a9ac9b5b65201f2f3/common/auth/internal/luci_ctx.go
[modify] https://crrev.com/de3fc344c2c849e54e6f300a9ac9b5b65201f2f3/common/auth/internal/service.go
[modify] https://crrev.com/de3fc344c2c849e54e6f300a9ac9b5b65201f2f3/common/auth/internal/user.go

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 26 2017

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

commit 9840cd357106a5169e05b13ccdf65f5e12ff269d
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Tue Sep 26 13:17:27 2017

Let the bot know its identity, as seen by the server.

This will later be used to figure out what email corresponds to tokens that
are handed out to tasks that specify 'service_account: bot'.

An alternative would be to try to resolve the token into email locally on the
bot, but this turns out to be more complicated (in particular error handling),
and more wasteful (the server knows the email anyway, let's just send it back
to the bot).

R=maruel@chromium.org
BUG= 767730 

Change-Id: If1488ca8f1bd5e011f9df0f4bf45380d801424af
Reviewed-on: https://chromium-review.googlesource.com/683321
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/9840cd357106a5169e05b13ccdf65f5e12ff269d/appengine/swarming/handlers_bot.py
[modify] https://crrev.com/9840cd357106a5169e05b13ccdf65f5e12ff269d/appengine/swarming/handlers_bot_test.py

Project Member

Comment 9 by bugdroid1@chromium.org, Sep 26 2017

Deployed swarming changed to chromium-swarm-dev.
Project Member

Comment 11 by bugdroid1@chromium.org, Sep 26 2017

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

commit 9c63809842a277ce10a86afd51b61c639a665d11
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Tue Sep 26 23:49:06 2017

Roll luci-go DEPS.

infra/go/src/go.chromium.org/luci:
24a04f85f [milo] use custom JSON marshaling for buildbot logs
61f6abfc9 [milo] use custom JSON marshaling for buildbot results
f2e8ef2ff [luci-notify] Pull settings.cfg during config update
98678574c scheduler: fix bug GetVisibleInvocation.
f26624529 [logdog-stream-view] Change state when paused.
15d8dc02e [logdog-stream-view] More UI fixes.
a5867acd7 [buildbucket] add limit parameter to Fetch and Run
4b99d1bf5 [logdog] Fix viewer bug.
5040ef006 [milo] Move source_manifest.proto to common/proto/milo.
69cf6dc7d [logdog] Add warmup handlers.
588cc8180 [buildbucket] add Fetch and Run
57b025e75 auth: Propagate the email through LUCI_CONTEXT["local_auth"].
de3fc344c auth: Avoid doing RPCs when getting an email, if possible.
08d79c295 auth: Remove code paths that support old LUCI_CONTEXT["local_auth"] protocol.

R=iannucci@chromium.org
BUG= 767730 

Change-Id: Ia8cb1a394583f2dd43c5405713c606fd6c5aea47
Reviewed-on: https://chromium-review.googlesource.com/685412
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>

[modify] https://crrev.com/9c63809842a277ce10a86afd51b61c639a665d11/DEPS

Project Member

Comment 12 by bugdroid1@chromium.org, Sep 27 2017

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

commit 8366c99f026bc2bb1ef3a2955e782b1fa37e2afc
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Wed Sep 27 00:51:32 2017

kitchen: Report emails of default and system accounts.

Mostly to verify GetEmail() works on Swarming, but also for better visibility
into what exactly is being used for authentication by Kitchen.

R=iannucci@chromium.org, nodir@chromium.org
BUG= 767730 

Change-Id: Id0ac2167ce58b41284dd1778857af907b412bae0
Reviewed-on: https://chromium-review.googlesource.com/685542
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/8366c99f026bc2bb1ef3a2955e782b1fa37e2afc/go/src/infra/tools/kitchen/cook.go

Status: Fixed (was: Assigned)
This should work now on main Swarming instance too.

The API is https://godoc.org/go.chromium.org/luci/common/auth#Authenticator.GetEmail

Sign in to add a comment