Add an optional IAP-enabled ID token when using the Nomulus tool

[gbrodman]

We can use the saved refresh token associated with the nomulus tool to request an ID token with an audience of the IAP client in order to satisfy IAP with with the Nomulus tool.

Note: this requires that the user of the Nomulus tool, e.g. "gbrodman@google.com" has a User object stored in SQL if using IAP.

Tested on QA

---

This change is 

[jianglai]

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gbrodman and @ptkach)

---

core/src/main/java/google/registry/config/files/default-config.yaml line 308 at r1 (raw file):

  # in this list. Client IDs are typically of the format
  # numbers-alphanumerics.apps.googleusercontent.com
  allowedOauthClientIds: []

Is my understanding correct that once we put every endpoint behind IAP, we will only need the iapClientId, but not this list, as client id checks would have been done by IAP before the request reaches our app?

Code quote:

  allowedOauthClientIds: []

---

core/src/main/java/google/registry/tools/RequestFactoryModule.java line 64 at r1 (raw file):

          .createRequestFactory(
              request -> {
                credentialsBundle.getHttpRequestInitializer().initialize(request);

I think what this does is to set the Credential as an interceptor for the request, which in turn refreshes (if necessary) and add the token to the request. So It should be part of the else clause below, as we only need this behavior (i.e. using the access token instead of ID token) if the IAP client ID is not set. In another word, it should not be necessary to do this, if we know we are going to overwrite the header with an ID token later.

Code quote:

credentialsBundle.getHttpRequestInitializer().initialize(request);

---

core/src/main/java/google/registry/tools/RequestFactoryModule.java line 68 at r1 (raw file):

                // that instead for authentication
                if (iapClientId.isPresent()) {
                  String idToken = getIdToken(credentialsBundle, iapClientId.get());

It probably doesn't matter much for the tool, where performance is not critical. But it would be better to cache the ID token instead of having to request one for each HTTP request, which involves an extra round trip to the token server for each HTTP request we send to nomulus.

It would be a must-have for the proxy, which currently does this:

https://cs.opensource.google/nomulus/nomulus/+/master:proxy/src/main/java/google/registry/proxy/ProxyModule.java;l=230;bpv=0;bpt=1

But my understanding is that we only have to manually create the ID token for local credentials, is that right? For the proxy which uses default service account credential derived from GKE cluster, the ID token can be obtained directly using existing libraries, is that correct? In that case the refresh-if-necessary behavior should be taken care of by the library.

Code quote:

String idToken = getIdToken(credentialsBundle, iapClientId.get());

[jianglai]

this requires that the user of the Nomulus tool, e.g. "gbrodman@google.com" has a User object stored in SQL if using IAP.
We should put this note in the code somewhere, maybe in the config file where the iapClientId field is defined?

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gbrodman and @ptkach)

[gbrodman]

good call

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @jianglai and @ptkach)

---

core/src/main/java/google/registry/config/files/default-config.yaml line 308 at r1 (raw file):

Previously, jianglai (Lai Jiang) wrote…

Is my understanding correct that once we put every endpoint behind IAP, we will only need the iapClientId, but not this list, as client id checks would have been done by IAP before the request reaches our app?

Possibly, hopefully! It seems like the other users of this are the "domain-watcher" and the proxy.

---

core/src/main/java/google/registry/tools/RequestFactoryModule.java line 64 at r1 (raw file):

Previously, jianglai (Lai Jiang) wrote…

I think what this does is to set the Credential as an interceptor for the request, which in turn refreshes (if necessary) and add the token to the request. So It should be part of the else clause below, as we only need this behavior (i.e. using the access token instead of ID token) if the IAP client ID is not set. In another word, it should not be necessary to do this, if we know we are going to overwrite the header with an ID token later.

Good call (we still want to keep the timeouts)

---

core/src/main/java/google/registry/tools/RequestFactoryModule.java line 68 at r1 (raw file):

Previously, jianglai (Lai Jiang) wrote…

It probably doesn't matter much for the tool, where performance is not critical. But it would be better to cache the ID token instead of having to request one for each HTTP request, which involves an extra round trip to the token server for each HTTP request we send to nomulus.

It would be a must-have for the proxy, which currently does this:

https://cs.opensource.google/nomulus/nomulus/+/master:proxy/src/main/java/google/registry/proxy/ProxyModule.java;l=230;bpv=0;bpt=1

But my understanding is that we only have to manually create the ID token for local credentials, is that right? For the proxy which uses default service account credential derived from GKE cluster, the ID token can be obtained directly using existing libraries, is that correct? In that case the refresh-if-necessary behavior should be taken care of by the library.

Yeah, if the proxy uses the default service account then we should just be able to do https://cloud.google.com/iap/docs/authentication-howto#obtaining_an_oidc_token_for_the_default_service_account using the standard libraries and that should hopefully take care of everything under the hood for us.

I'm a bit leery of ID token caching because it'd have to use the disk and (like you said) it's not a huge deal for one more HTTP round trip with the tool.

[jianglai]

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @gbrodman and @ptkach)

---

core/src/main/java/google/registry/config/files/default-config.yaml line 308 at r1 (raw file):

Previously, gbrodman wrote…

Possibly, hopefully! It seems like the other users of this are the "domain-watcher" and the proxy.

Yeah, we might to investigate how to let domain-watcher send requests that can pass IAP if we want to put everything behind IAP. But that's for another day.

---

core/src/main/java/google/registry/tools/RequestFactoryModule.java line 68 at r1 (raw file):

Previously, gbrodman wrote…

Yeah, if the proxy uses the default service account then we should just be able to do https://cloud.google.com/iap/docs/authentication-howto#obtaining_an_oidc_token_for_the_default_service_account using the standard libraries and that should hopefully take care of everything under the hood for us.

I'm a bit leery of ID token caching because it'd have to use the disk and (like you said) it's not a huge deal for one more HTTP round trip with the tool.

Not sure what you meant by "having to use the disk"? The refresh token is already saved in the disk, isn't it? Why is it a concern to use the disk? Plus, the caching itself doesn't need disk, per se. We can just use an in-memory memoizer with expiry that Guava provides, right?

I guess another concern (aside from performance) is that whether there's any adverse effect to refresh a token too frequently? Would the token server have any sort of rate limit? That's why I think if implementing caching is relatively trivial, we might as well do it.

[gbrodman]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jianglai and @ptkach)

---

core/src/main/java/google/registry/tools/RequestFactoryModule.java line 68 at r1 (raw file):

Previously, jianglai (Lai Jiang) wrote…

Not sure what you meant by "having to use the disk"? The refresh token is already saved in the disk, isn't it? Why is it a concern to use the disk? Plus, the caching itself doesn't need disk, per se. We can just use an in-memory memoizer with expiry that Guava provides, right?

I guess another concern (aside from performance) is that whether there's any adverse effect to refresh a token too frequently? Would the token server have any sort of rate limit? That's why I think if implementing caching is relatively trivial, we might as well do it.

There's no point in using an in-memory cache because this is client-side so the memory only lives as long as the nomulus command (unless, possibly, we're running the shell but I believe that is infrequent). We'd have to write the ID token to disk and check its expiration every time. Much simpler to just use the already-existing refresh token.

[jianglai]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ptkach)

---

core/src/main/java/google/registry/tools/RequestFactoryModule.java line 68 at r1 (raw file):

Previously, gbrodman wrote…

There's no point in using an in-memory cache because this is client-side so the memory only lives as long as the nomulus command (unless, possibly, we're running the shell but I believe that is infrequent). We'd have to write the ID token to disk and check its expiration every time. Much simpler to just use the already-existing refresh token.

I was thinking about just a fixed-timed cache, which would not require checking the expiration every time (but may result in some unnecessary refreshing). But yeah, I guess other then using the shell mode, an in-memory would not necessarily help much. I however think the library might be doing exactly what you described with the access token, i.e. persisting it to the disk and reusing it if possible.

Anyway, it doesn't seem worth the effort for us to implement the same. Let's do without caching for now.