feat(VpcInstanceAuthenticator): add support for new VPC authentication flow #153
Conversation
…n flow This commit introduces the VpcInstanceAuthenticator. This authenticator implements the authentication flow within a VPC-managed compute resource that is configured to use the compute resource identity feature. This involves the use of the compute resource's local VPC Instance Metadata Service API to retrieve an instance identity token, and then exchange that token for an IAM access token. The IAM access token is then used to authenticate outbound REST API requests by adding an Authorization containing the access token.
| - Bearer Token Authentication | ||
| - Identity and Access Management (IAM) Authentication | ||
| - Container Authentication | ||
| - VPC Instance Authentication |
There was a problem hiding this comment.
In addition to adding the docs for the VPC authenticator, I also made changes throughout the document to bring it more in sync with the Go Authentication.md file, specifically to make the code examples more consistent.
| */ | ||
| private OkHttpClient setLoggingLevel(OkHttpClient client, LoggingLevel loggingLevel) { | ||
| HttpLoggingInterceptor loggingInterceptor = new HttpLoggingInterceptor(); | ||
| // First check to see if the client already has the http logging interceptor registered. |
There was a problem hiding this comment.
While testing the VpcInstanceAuthenticator, I noticed that we had a nasty bug here where we would register a new instance of the logging interceptor EACH TIME the setLoggingLevel method is called, even if the client already had the logging interceptor registered. This would result in multiple copies of the debug output for each request/response.
| * Converts a VpcTokenResponse instance to an IamToken instance. | ||
| * @param vpcResponse the VpcTokenResponse instance to be converted. | ||
| */ | ||
| public IamToken(VpcTokenResponse vpcResponse) { |
There was a problem hiding this comment.
Added this constructor to make it easy to convert a VpcTokenResponse instance (the response received from the VPC Instance Metadata Service) into an IamToken instance (the class we used to cache the iam access token and related info).
| | ||
| /** | ||
| * Create a MockResponse with JSON content type and the object serialized to JSON as body. | ||
| * Create a MockResponse with JSON content type and "body" serialized to JSON as the response body. |
There was a problem hiding this comment.
Couldn't stop myself from fixing up some of the javadocs in this class :)
| // Tests involving the Builder class and fromConfiguration() method. | ||
| // | ||
| | ||
| @Test() |
There was a problem hiding this comment.
I noticed we didn't have a "success" test for the BasicAuthenticator.Builder class.
| @dpopp07 @pyrooka I've pushed a commit to this PR that refactors the VpcInstanceAuthenticator class so that it does in fact subclass the TokenRequestBasedAuthenticator base class, similar to the CP4D authenticator. |
| OkHttpClient updatedClient = client; | ||
| if (loggingInterceptor == null) { | ||
| loggingInterceptor = new HttpLoggingInterceptor(); | ||
| loggingInterceptor.redactHeader(HttpHeaders.AUTHORIZATION); |
There was a problem hiding this comment.
Added this so that we don't end up logging Authorization header values when debug logging is turned on :)
| try { | ||
| // Create a PUT request to retrieve the instance identity token. | ||
| RequestBuilder builder = RequestBuilder | ||
| .put(RequestBuilder.resolveRequestUrl(getImdsEndpoint(), operationPathCreateAccessToken)); |
There was a problem hiding this comment.
@padamstx Do you think we need to canonicalize the URL if we are unconditionally adding the operation path to it?
There was a problem hiding this comment.
Actually no, because:
- This is a new authenticator and so we've never supported a scenario where we allow the user to specify more than just the base endpoint URL.
- I'm not sure how we'd canonicalize it because there are two potential operation paths that the user could mistakenly include in the url string.
- I doubt that a user would really ever need to specify the "url" property because there is zero flexibility to use anything other than the link local address (the default). We really just allow the url to be set for testing purposes.
There was a problem hiding this comment.
Sounds good, just wanted to double check!
src/main/java/com/ibm/cloud/sdk/core/security/VpcInstanceAuthenticator.java Outdated Show resolved Hide resolved
src/main/java/com/ibm/cloud/sdk/core/security/VpcInstanceAuthenticator.java Outdated Show resolved Hide resolved
# [9.14.0](9.13.4...9.14.0) (2021-11-08) ### Features * **VpcInstanceAuthenticator:** add support for new VPC authentication flow ([#153](#153)) ([815031d](815031d))
| 🎉 This PR is included in version 9.14.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
4 participants
This commit introduces the VpcInstanceAuthenticator.
This authenticator implements the authentication flow
within a VPC-managed compute resource that is configured to
use the compute resource identity feature.
This involves the use of the compute resource's local
VPC Instance Metadata Service API to retrieve an instance identity
token, and then exchange that token for an IAM access token.
The IAM access token is then used to authenticate outbound REST
API requests by adding an Authorization containing the access token.