Merge pull request #6798 from jboeuf/fix_3803

Fix #3803

10 files changed, 35 insertions, 29 deletions

diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
index 9c99ff883a..9c125566d4 100644
--- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c
+++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
@@ -1557,17 +1557,15 @@ static void close_from_api(grpc_exec_ctx *exec_ctx,
       gpr_slice_buffer_add(&transport_global->qbuf,
                            gpr_slice_ref(*optional_message));
     }
-
     gpr_slice_buffer_add(
         &transport_global->qbuf,
         grpc_chttp2_rst_stream_create(stream_global->id, GRPC_CHTTP2_NO_ERROR,
                                       &stream_global->stats.outgoing));
-
-    if (optional_message) {
-      gpr_slice_ref(*optional_message);
-    }
   }
 
+  if (optional_message) {
+    gpr_slice_ref(*optional_message);
+  }
   grpc_chttp2_fake_status(exec_ctx, transport_global, stream_global, status,
                           optional_message);
   grpc_error *err = GRPC_ERROR_CREATE("Stream closed");
diff --git a/src/core/lib/security/credentials/composite/composite_credentials.c b/src/core/lib/security/credentials/composite/composite_credentials.c
index 07db8bfd75..850e41e646 100644
--- a/src/core/lib/security/credentials/composite/composite_credentials.c
+++ b/src/core/lib/security/credentials/composite/composite_credentials.c
@@ -72,11 +72,12 @@ static void composite_call_md_context_destroy(
 static void composite_call_metadata_cb(grpc_exec_ctx *exec_ctx, void *user_data,
                                        grpc_credentials_md *md_elems,
                                        size_t num_md,
-                                       grpc_credentials_status status) {
+                                       grpc_credentials_status status,
+                                       const char *error_details) {
   grpc_composite_call_credentials_metadata_context *ctx =
       (grpc_composite_call_credentials_metadata_context *)user_data;
   if (status != GRPC_CREDENTIALS_OK) {
-    ctx->cb(exec_ctx, ctx->user_data, NULL, 0, status);
+    ctx->cb(exec_ctx, ctx->user_data, NULL, 0, status, error_details);
     return;
   }
 
@@ -101,7 +102,7 @@ static void composite_call_metadata_cb(grpc_exec_ctx *exec_ctx, void *user_data,
 
   /* We're done!. */
   ctx->cb(exec_ctx, ctx->user_data, ctx->md_elems->entries,
-          ctx->md_elems->num_entries, GRPC_CREDENTIALS_OK);
+          ctx->md_elems->num_entries, GRPC_CREDENTIALS_OK, NULL);
   composite_call_md_context_destroy(ctx);
 }
 
diff --git a/src/core/lib/security/credentials/credentials.c b/src/core/lib/security/credentials/credentials.c
index 0eadaec191..029a357261 100644
--- a/src/core/lib/security/credentials/credentials.c
+++ b/src/core/lib/security/credentials/credentials.c
@@ -117,7 +117,7 @@ void grpc_call_credentials_get_request_metadata(
     grpc_credentials_metadata_cb cb, void *user_data) {
   if (creds == NULL || creds->vtable->get_request_metadata == NULL) {
     if (cb != NULL) {
-      cb(exec_ctx, user_data, NULL, 0, GRPC_CREDENTIALS_OK);
+      cb(exec_ctx, user_data, NULL, 0, GRPC_CREDENTIALS_OK, NULL);
     }
     return;
   }
diff --git a/src/core/lib/security/credentials/credentials.h b/src/core/lib/security/credentials/credentials.h
index ce235e3a1d..8e9d842ead 100644
--- a/src/core/lib/security/credentials/credentials.h
+++ b/src/core/lib/security/credentials/credentials.h
@@ -156,11 +156,10 @@ void grpc_credentials_md_store_unref(grpc_credentials_md_store *store);
 
 /* --- grpc_call_credentials. --- */
 
-typedef void (*grpc_credentials_metadata_cb)(grpc_exec_ctx *exec_ctx,
-                                             void *user_data,
-                                             grpc_credentials_md *md_elems,
-                                             size_t num_md,
-                                             grpc_credentials_status status);
+/* error_details must be NULL if status is GRPC_CREDENTIALS_OK. */
+typedef void (*grpc_credentials_metadata_cb)(
+    grpc_exec_ctx *exec_ctx, void *user_data, grpc_credentials_md *md_elems,
+    size_t num_md, grpc_credentials_status status, const char *error_details);
 
 typedef struct {
   void (*destruct)(grpc_call_credentials *c);
diff --git a/src/core/lib/security/credentials/fake/fake_credentials.c b/src/core/lib/security/credentials/fake/fake_credentials.c
index ee6d964de1..51cafd986f 100644
--- a/src/core/lib/security/credentials/fake/fake_credentials.c
+++ b/src/core/lib/security/credentials/fake/fake_credentials.c
@@ -100,7 +100,7 @@ static void on_simulated_token_fetch_done(grpc_exec_ctx *exec_ctx,
       (grpc_credentials_metadata_request *)user_data;
   grpc_md_only_test_credentials *c = (grpc_md_only_test_credentials *)r->creds;
   r->cb(exec_ctx, r->user_data, c->md_store->entries, c->md_store->num_entries,
-        GRPC_CREDENTIALS_OK);
+        GRPC_CREDENTIALS_OK, NULL);
   grpc_credentials_metadata_request_destroy(r);
 }
 
@@ -117,7 +117,7 @@ static void md_only_test_get_request_metadata(
         grpc_closure_create(on_simulated_token_fetch_done, cb_arg),
         GRPC_ERROR_NONE);
   } else {
-    cb(exec_ctx, user_data, c->md_store->entries, 1, GRPC_CREDENTIALS_OK);
+    cb(exec_ctx, user_data, c->md_store->entries, 1, GRPC_CREDENTIALS_OK, NULL);
   }
 }
 
diff --git a/src/core/lib/security/credentials/iam/iam_credentials.c b/src/core/lib/security/credentials/iam/iam_credentials.c
index 64d5871844..370a384d0e 100644
--- a/src/core/lib/security/credentials/iam/iam_credentials.c
+++ b/src/core/lib/security/credentials/iam/iam_credentials.c
@@ -55,7 +55,7 @@ static void iam_get_request_metadata(grpc_exec_ctx *exec_ctx,
                                      void *user_data) {
   grpc_google_iam_credentials *c = (grpc_google_iam_credentials *)creds;
   cb(exec_ctx, user_data, c->iam_md->entries, c->iam_md->num_entries,
-     GRPC_CREDENTIALS_OK);
+     GRPC_CREDENTIALS_OK, NULL);
 }
 
 static grpc_call_credentials_vtable iam_vtable = {iam_destruct,
diff --git a/src/core/lib/security/credentials/jwt/jwt_credentials.c b/src/core/lib/security/credentials/jwt/jwt_credentials.c
index f4dd920861..6323eb9393 100644
--- a/src/core/lib/security/credentials/jwt/jwt_credentials.c
+++ b/src/core/lib/security/credentials/jwt/jwt_credentials.c
@@ -113,10 +113,11 @@ static void jwt_get_request_metadata(grpc_exec_ctx *exec_ctx,
 
   if (jwt_md != NULL) {
     cb(exec_ctx, user_data, jwt_md->entries, jwt_md->num_entries,
-       GRPC_CREDENTIALS_OK);
+       GRPC_CREDENTIALS_OK, NULL);
     grpc_credentials_md_store_unref(jwt_md);
   } else {
-    cb(exec_ctx, user_data, NULL, 0, GRPC_CREDENTIALS_ERROR);
+    cb(exec_ctx, user_data, NULL, 0, GRPC_CREDENTIALS_ERROR,
+       "Could not generate JWT.");
   }
 }
 
diff --git a/src/core/lib/security/credentials/oauth2/oauth2_credentials.c b/src/core/lib/security/credentials/oauth2/oauth2_credentials.c
index 1102553dd3..c22ea5c468 100644
--- a/src/core/lib/security/credentials/oauth2/oauth2_credentials.c
+++ b/src/core/lib/security/credentials/oauth2/oauth2_credentials.c
@@ -235,10 +235,11 @@ static void on_oauth2_token_fetcher_http_response(grpc_exec_ctx *exec_ctx,
     c->token_expiration =
         gpr_time_add(gpr_now(GPR_CLOCK_REALTIME), token_lifetime);
     r->cb(exec_ctx, r->user_data, c->access_token_md->entries,
-          c->access_token_md->num_entries, status);
+          c->access_token_md->num_entries, GRPC_CREDENTIALS_OK, NULL);
   } else {
     c->token_expiration = gpr_inf_past(GPR_CLOCK_REALTIME);
-    r->cb(exec_ctx, r->user_data, NULL, 0, status);
+    r->cb(exec_ctx, r->user_data, NULL, 0, status,
+          "Error occured when fetching oauth2 token.");
   }
   gpr_mu_unlock(&c->mu);
   grpc_credentials_metadata_request_destroy(r);
@@ -266,7 +267,7 @@ static void oauth2_token_fetcher_get_request_metadata(
   }
   if (cached_access_token_md != NULL) {
     cb(exec_ctx, user_data, cached_access_token_md->entries,
-       cached_access_token_md->num_entries, GRPC_CREDENTIALS_OK);
+       cached_access_token_md->num_entries, GRPC_CREDENTIALS_OK, NULL);
     grpc_credentials_md_store_unref(cached_access_token_md);
   } else {
     c->fetch_func(
@@ -404,7 +405,8 @@ static void access_token_get_request_metadata(
     grpc_polling_entity *pollent, grpc_auth_metadata_context context,
     grpc_credentials_metadata_cb cb, void *user_data) {
   grpc_access_token_credentials *c = (grpc_access_token_credentials *)creds;
-  cb(exec_ctx, user_data, c->access_token_md->entries, 1, GRPC_CREDENTIALS_OK);
+  cb(exec_ctx, user_data, c->access_token_md->entries, 1, GRPC_CREDENTIALS_OK,
+     NULL);
 }
 
 static grpc_call_credentials_vtable access_token_vtable = {
diff --git a/src/core/lib/security/credentials/plugin/plugin_credentials.c b/src/core/lib/security/credentials/plugin/plugin_credentials.c
index 9fb55e8466..824ff081dc 100644
--- a/src/core/lib/security/credentials/plugin/plugin_credentials.c
+++ b/src/core/lib/security/credentials/plugin/plugin_credentials.c
@@ -67,7 +67,8 @@ static void plugin_md_request_metadata_ready(void *request,
       gpr_log(GPR_ERROR, "Getting metadata from plugin failed with error: %s",
               error_details);
     }
-    r->cb(&exec_ctx, r->user_data, NULL, 0, GRPC_CREDENTIALS_ERROR);
+    r->cb(&exec_ctx, r->user_data, NULL, 0, GRPC_CREDENTIALS_ERROR,
+          error_details);
   } else {
     size_t i;
     grpc_credentials_md *md_array = NULL;
@@ -79,7 +80,7 @@ static void plugin_md_request_metadata_ready(void *request,
             gpr_slice_from_copied_buffer(md[i].value, md[i].value_length);
       }
     }
-    r->cb(&exec_ctx, r->user_data, md_array, num_md, GRPC_CREDENTIALS_OK);
+    r->cb(&exec_ctx, r->user_data, md_array, num_md, GRPC_CREDENTIALS_OK, NULL);
     if (md_array != NULL) {
       for (i = 0; i < num_md; i++) {
         gpr_slice_unref(md_array[i].key);
@@ -107,7 +108,7 @@ static void plugin_get_request_metadata(grpc_exec_ctx *exec_ctx,
     c->plugin.get_metadata(c->plugin.state, context,
                            plugin_md_request_metadata_ready, request);
   } else {
-    cb(exec_ctx, user_data, NULL, 0, GRPC_CREDENTIALS_OK);
+    cb(exec_ctx, user_data, NULL, 0, GRPC_CREDENTIALS_OK, NULL);
   }
 }
 
diff --git a/src/core/lib/security/transport/client_auth_filter.c b/src/core/lib/security/transport/client_auth_filter.c
index 76be2acd72..399b92c8e1 100644
--- a/src/core/lib/security/transport/client_auth_filter.c
+++ b/src/core/lib/security/transport/client_auth_filter.c
@@ -91,14 +91,16 @@ static void bubble_up_error(grpc_exec_ctx *exec_ctx, grpc_call_element *elem,
                             grpc_status_code status, const char *error_msg) {
   call_data *calld = elem->call_data;
   gpr_log(GPR_ERROR, "Client side authentication failure: %s", error_msg);
-  grpc_transport_stream_op_add_cancellation(&calld->op, status);
+  gpr_slice error_slice = gpr_slice_from_copied_string(error_msg);
+  grpc_transport_stream_op_add_close(&calld->op, status, &error_slice);
   grpc_call_next_op(exec_ctx, elem, &calld->op);
 }
 
 static void on_credentials_metadata(grpc_exec_ctx *exec_ctx, void *user_data,
                                     grpc_credentials_md *md_elems,
                                     size_t num_md,
-                                    grpc_credentials_status status) {
+                                    grpc_credentials_status status,
+                                    const char *error_details) {
   grpc_call_element *elem = (grpc_call_element *)user_data;
   call_data *calld = elem->call_data;
   grpc_transport_stream_op *op = &calld->op;
@@ -107,7 +109,9 @@ static void on_credentials_metadata(grpc_exec_ctx *exec_ctx, void *user_data,
   reset_auth_metadata_context(&calld->auth_md_context);
   if (status != GRPC_CREDENTIALS_OK) {
     bubble_up_error(exec_ctx, elem, GRPC_STATUS_UNAUTHENTICATED,
-                    "Credentials failed to get metadata.");
+                    (error_details != NULL && strlen(error_details) > 0)
+                        ? error_details
+                        : "Credentials failed to get metadata.");
     return;
   }
   GPR_ASSERT(num_md <= MAX_CREDENTIALS_METADATA_COUNT);