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

Fix invalid credential error handling in authentication process #962

Merged
merged 9 commits into from
Jan 18, 2023

Conversation

Didainius
Copy link
Collaborator

@Didainius Didainius commented Dec 15, 2022

This PR builds on top of vmware/go-vcloud-director#536 and only adds additional tests as the real fix is in go-vcloud-director SDK.

We had an unhandled HTTP 401 (Unauthorized) error in the password authentication flow and as a result it would not return an error when user supplies invalid credentials. The errors then become very confusing and the user is left helpless to do his guesswork.

Before, this would cause very confusing errors similar to:

Error: could not find any extended Provider VDC with name ext-net-name: error retrieving query: API Error: 406: The request has invalid accept header: Invalid API version requested. Supported API versions are: [37.0.0-alpha, 36.3, 36.2, 36.1, 36.0, 35.2, 35.0, 34.0, 33.0 [D], 32.0 [D], 31.0 [D]] ([D] indicates deprecated versions)
│
│   with data.vcd_provider_vdc.nsxt-pvdc,
│   on Org-creation-Main.tf line 47, in data "vcd_provider_vdc" "nsxt-pvdc":
│   47: data "vcd_provider_vdc" "nsxt-pvdc" {
│
╵
╷
│ Error: could not find external network V2 by name 'ext-net-name': could not find external network by name: error getting all pages for endpoint https://HOSTNAME/cloudapi/1.0.0/externalNetworks/: error in HTTP GET request: NOT_ACCEPTABLE - The request has invalid accept header: Invalid API version requested. Supported API versions are: [37.0.0-alpha, 36.3, 36.2, 36.1, 36.0, 35.2, 35.0, 34.0, 33.0 [D], 32.0 [D], 31.0 [D]] ([D] indicates deprecated versions)
│
│   with data.vcd_external_network_v2.nsxt-ext-net,
│   on Org-creation-Main.tf line 98, in data "vcd_external_network_v2" "nsxt-ext-net":
│   98: data "vcd_external_network_v2" "nsxt-ext-net" {

After the fix, the user would get similar error which directly hints that one should check his credentials:

   auth_test.go:310: Step 1/1 error: Error running pre-apply refresh: exit status 1

        Error: something went wrong during authentication: error authorizing: received response HTTP 401 (Unauthorized). Please check if your credentials are valid

          with provider["registry.terraform.io/hashicorp/vcd"],
          on terraform_plugin_test.tf line 2, in provider "vcd":
           2:                   provider "vcd" {

Three additional subtests are added to TestAccAuth that expect an error:

  • TestAccAuth/InvalidSystemUserAndPasswordWithDefaultOrgAndVdc
  • TestAccAuth/InvalidPassword,AuthType=integrated
  • TestAccAuth/InvalidSystemUserAndPassword,AuthType=integrated

Signed-off-by: Dainius Serplis <dserplis@vmware.com>
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
@Didainius Didainius changed the title Fix invalid credential error in authentication process Fix invalid credential error handling in authentication process Dec 15, 2022
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
@Didainius Didainius marked this pull request as ready for review January 16, 2023 12:31
Copy link
Collaborator

@adambarreiro adambarreiro left a comment

Choose a reason for hiding this comment

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

LGTM!

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.

Just one suggestion for changelog.

.changes/v3.9.0/962-bug-fixes.md Outdated Show resolved Hide resolved
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
Signed-off-by: Dainius Serplis <dserplis@vmware.com>
@Didainius Didainius merged commit 9c3aba5 into vmware:main Jan 18, 2023
@Didainius Didainius deleted the fix-auth-error branch January 18, 2023 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants