Allow access to Google API regional endpoints via Google Default Credentials

[yihuazhang]

The self-signed JWT used to access Google API regional endpoints do not have a correct audience format dictated in https://google.aip.dev/auth/4111. That is, it includes a trailing RPC service name while the spec requires only the presence of hostname (or endpoint).

This should fix #27098 (comment)

---

This change is 

[markdroth]

The overall functionality looks good here. Just a few minor comments to address.

Please let me know if you have any questions. Thanks!

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

---

src/core/lib/security/credentials/jwt/jwt_credentials.h, line 85 at r1 (raw file):

//
// Exposed for testing purpose.
char* grpc_remove_service_name_from_uri(const char* peer_uri);

Instead of adding a new C-style function, let's make this a C++-style function. Please use something like this:

namespace grpc_core {

// Exposed for testing purposes only.
absl::StatusOr<std::string> RemoveServiceNameFromJwtUri(absl::string_view uri);

}  // namespace grpc_core

---

src/core/lib/security/credentials/jwt/jwt_credentials.cc, line 191 at r1 (raw file):

  }
  char* result = nullptr;
  gpr_asprintf(&result, "%s://%s/", uri->scheme().c_str(),

Instead, please use:

return absl::StrFormat("%s://%s/", uri->scheme(), uri->authority());

---

test/core/security/credentials_test.cc, line 1408 at r1 (raw file):

}

static void test_jwt_creds_uri_process(void) {

Please call this something like test_remove_service_from_jwt_uri().

[yihuazhang]

Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @markdroth and @ZhenLian)

---

src/core/lib/security/credentials/jwt/jwt_credentials.h, line 85 at r1 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Instead of adding a new C-style function, let's make this a C++-style function. Please use something like this:

namespace grpc_core {

// Exposed for testing purposes only.
absl::StatusOr<std::string> RemoveServiceNameFromJwtUri(absl::string_view uri);

}  // namespace grpc_core

Done.

---

src/core/lib/security/credentials/jwt/jwt_credentials.cc, line 191 at r1 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Instead, please use:

return absl::StrFormat("%s://%s/", uri->scheme(), uri->authority());

Done.

---

test/core/security/credentials_test.cc, line 1408 at r1 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

Please call this something like test_remove_service_from_jwt_uri().

Done.

[markdroth]

Just a couple of remaining nits. Feel free to merge once you've addressed them.

Thanks!

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @yihuazhang and @ZhenLian)

---

src/core/lib/security/credentials/jwt/jwt_credentials.cc, line 46 at r2 (raw file):

  GRPC_MDELEM_UNREF(cached_.jwt_md);
  cached_.jwt_md = GRPC_MDNULL;
  if (!cached_.service_url.empty()) {

No need for this conditional; you can just unconditionally call clear() here, since that's a no-op if it's already empty.

---

src/core/lib/security/credentials/jwt/jwt_credentials.cc, line 71 at r2 (raw file):

      grpc_core::RemoveServiceNameFromJwtUri(context.service_url);
  if (!uri.ok()) {
    *error = GRPC_ERROR_CREATE_FROM_STRING_VIEW(uri.status().message());

You can use absl_status_to_grpc_error() here:

grpc/src/core/lib/transport/error_utils.h

Line 50 in cb2e755

grpc_error_handle absl_status_to_grpc_error(absl::Status status);

[yihuazhang]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @markdroth and @ZhenLian)

---

src/core/lib/security/credentials/jwt/jwt_credentials.cc, line 46 at r2 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

No need for this conditional; you can just unconditionally call clear() here, since that's a no-op if it's already empty.

Ah yes, you are right. Done!

---

src/core/lib/security/credentials/jwt/jwt_credentials.cc, line 71 at r2 (raw file):

Previously, markdroth (Mark D. Roth) wrote…

You can use absl_status_to_grpc_error() here:

grpc/src/core/lib/transport/error_utils.h

Line 50 in cb2e755

grpc_error_handle absl_status_to_grpc_error(absl::Status status);

Nice suggestion! Done.

[yihuazhang]

Thanks much for detailed and prompt review, @markdroth!