Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable usage of bearer tokens #341

Merged
merged 11 commits into from
Nov 11, 2020
Merged

Conversation

dataclouder
Copy link
Contributor

@dataclouder dataclouder commented Nov 9, 2020

The SDK now accepts both the old (deprecated) authorization token and the bearer token.
The usage is transparent: it doesn't require new options or configuration. If we enter a bearer token instead of an authorization token, it gets used.

How to test:

  1. run ./scripts/get_token.sh
  2. Copy the bearer token
  3. Paste the token into the configuration file
  4. Set the environment variable GOVCD_LOG_PASSWORDS=1
  5. Run a quick test (such as Test_GetVcdVersion)
  6. Check the logs, making sure that the Authorization header now contains the bearer token and that the X-Vmware-Vcloud-Token-Type header contains "Bearer".
  7. Change the token to the authorization token (also from step 1)
  8. Run steps 4-6 (the token used is the short one)
  9. Remove the token from the configuration file
  10. Run steps 4-5
  11. Check the logs, making sure that Authorization header contains the bearer token.

This PR also adds a transparent fallback connection using cloudapi/1.0.0/sessions when the call to /api/session has been disabled. No action is needed to activate it. When the fallback call happens, we'll find trace information in the logs:

2020/11/10 15:35:16 [TRACE] Connecting to VCD using cloudapi
2020/11/10 15:35:16 [TRACE] URL https://bos1-vcloud-static-xxx-xxx.eng.vmware.com/cloudapi/1.0.0/sessions/provider

To test the fallback functionality, disable access to api/sessions following the instructions in this page and then run any test without token.

Giuseppe Maxia added 3 commits November 9, 2020 21:50
Add support for bearer token
* The default token is now a bearer token
* The library accepts both authorization and bearer tokens when passed
  as user option
@dataclouder dataclouder marked this pull request as ready for review November 10, 2020 08:38
@@ -79,7 +79,7 @@ var (
LogHttpResponse bool = true

// List of tags to be excluded from logging
skipTags = []string{"SupportedVersions", "ovf:License"}
skipTags = []string{"ovf:License"}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the SupportedVersions call in recent releases has become short and bearable, while up to 9.5 it was 600+ lines.

Copy link
Contributor

@vbauzys vbauzys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Maybe we already can update sample configuration and state that

    # The vCD address, in the format https://vCD_IP/api
    # or https://vCD_host_name/api
    url: https://11.111.1.111/api

should be

    url: https://11.111.1.111/openapi

@dataclouder
Copy link
Contributor Author

LGTM.
Maybe we already can update sample configuration and state that

    # The vCD address, in the format https://vCD_IP/api
    # or https://vCD_host_name/api
    url: https://11.111.1.111/api

should be

    url: https://11.111.1.111/openapi

Not yet.
This may work for the initial connection, and we would get a valid token, but there are three problems:

  • It won't work in 9.7
  • the connection call is different for provider and tenant (see get_token.sh for an example)
  • many calls in our SDK still require the /api format.

@dataclouder dataclouder marked this pull request as draft November 10, 2020 13:10
Giuseppe Maxia added 2 commits November 10, 2020 15:31
If a VCD has disabled authentication using /api/sessions
we can automatically attempt a connection using /cloudapi/1.0.0/sessions
@dataclouder dataclouder marked this pull request as ready for review November 10, 2020 14:40
Copy link
Collaborator

@Didainius Didainius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My testing worked out well.

CHANGELOG.md Outdated
* Added methods `adminVdc.UpdateStorageProfile` [#340](https://github.com/vmware/go-vcloud-director/pull/340)

BREAKING CHANGES:

* type.VdcConfiguration (used for creation) changed the type for storage profile from `[]*VdcStorageProfile` to `[]*VdcStorageProfileConfiguration`
>>>>>>> master
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leftover

vcdCli.Client.VCDToken = resp.Header.Get(AuthorizationHeader)
vcdCli.Client.VCDAuthHeader = AuthorizationHeader
vcdCli.Client.VCDToken = resp.Header.Get(BearerTokenHeader)
vcdCli.Client.VCDAuthHeader = BearerTokenHeader
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused by this part. Have we removed the authorization token completely in favor of the bearer token?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No.
We have changed the token that we use to keep the client working. Previously, we injected the authorization token at connection time in the client, so that it would be used for further operations. Now we use the bearer token for the same purpose.

However, if you connect using the old authorization token, it will be used like before.

Copy link
Collaborator

@lvirbalas lvirbalas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@dataclouder dataclouder merged commit e4834af into vmware:master Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants