Changing the credentials plugin API.#4154
Conversation
- The plugin is now passed more information that it can use to create
auth metadata:
- service_url (as before)
- method name
- channel_auth_context
|
@jtattermusch @murgatroid99 @yang-g @tbetbetbe @stanley-cheung @ctiller @soltanmm @nathanielmanistaatgoogle please have a look. Comments are welcome. This is the API change only so that you can see what's coming. I'm starting to work on the implementation and will adjust if you want me to change the API. |
The purpose of this is to be able to install a composition policy that describes which types are incompatible and that will be enforced during call creds composition. If this functionality is wanted it will be done in an additive function in the API like : void grpc_call_credentials_set_composite_policy( grpc_call_credentials_composite_policy policy);
|
An example of how the new API looks in C++ is here: 114f394. |
There was a problem hiding this comment.
Why do we need all of this information in the metadata plugin? Why do we need the auth context?
There was a problem hiding this comment.
You may have some credentials that want to bind some token to the server's channel identity.
|
@mvrable may want to have a look as well. |
|
The changes look alright to me, but it looks like we might need to adjust the metadata plugin API in wrapped languages so that the argument passed to the auth plugin callback is an object (of class "AuthMetadataContext" or some other fitting name). We won't expose the newly added fields like "method" or "auth_context" because currently we don't have use for them (correct?), but we need to make sure we can expose them with only additive API change if we need make them available in the future. Once this PR is merged, can you pls file issues for all wrapped languages to cover this (or comment on the existing issues for languages where the auth work hasn't been done yet)? I'd still like a review for the additions to the C core public api (grpc_security.h). @yang-g , can you please take a look? Btw, I'm OOO next week, so I will reassign to @murgatroid99 . |
|
Assigning @ctiller for the core API review. Will give it back to @murgatroid99 after we're in agreement. |
|
LGTM |
Also passing empty string as opposed to NULL when the method name is not found. This is much less error-prone.
|
for @yang-g Re: fully qualified method name. I added some documentation that should make things clearer. |
|
The errors are due to jenkins's internal flakyness. Merging. |
Changing the credentials plugin API.
The plugin is now passed more information that it can use to create
auth metadata:
- service_url (as before)
- method name
- channel_auth_context
Also adding a type field that can be used later if we want to set policies for which credentials are incompatible during composition.