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

http remote backend initialization fails due to unescaped username #28302

Closed
wants to merge 2 commits into from
Closed

http remote backend initialization fails due to unescaped username #28302

wants to merge 2 commits into from

Conversation

FalcoSuessgott
Copy link

Backend initialization fails if the username contains a .

The problem is the setting of the basic auth header without escaping the parameters:

if c.Username != "" {
req.SetBasicAuth(c.Username, c.Password)
}

In that PR I escaped the url, username and password using url.EscapeQuery, now it works:

$> terraform init \
-backend-config="address=url" \
-backend-config="lock_address=url" \
-backend-config="unlock_address=url" \
-backend-config="password=foo" \
-backend-config="lock_method=POST" \
-backend-config="unlock_method=DELETE" \
-backend-config="retry_wait_min=5" \
-backend-config="username=user.name"
Terraform initialized in an empty directory!

related Issues:

@hashicorp-cla
Copy link

hashicorp-cla commented Apr 7, 2021

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Apr 7, 2021

Codecov Report

Merging #28302 (69a1fef) into main (b7fb533) will not change coverage.
The diff coverage is 50.00%.

Impacted Files Coverage Δ
backend/remote-state/http/client.go 50.42% <50.00%> (ø)

@apparentlymart
Copy link
Member

apparentlymart commented Apr 15, 2021

Hi @FalcoSuessgott! Thanks for working on this.

This SetBasicAuth function populates an Authorization header in the request, and such headers are not subject to URL encoding. Instead, the encoding of this would be Authorization: Basic followed by a base64-encoded version of the username and password concatenated together using a colon, as we can see in RFC 7617 section 2.

If your particular target system is requiring the username to be encoded then this seems to be a quirk of that particular system rather than a general HTTP requirement, because RFC 7617 and RFC 7235 don't call for any encoding other than octet encoding (as UTF-8) and base64 encoding for these values.

For that reason, I don't think it would be appropriate to force this on all users of the http backend. Instead, this seems better addressed by you configuring the HTTP backend to send the username in the form your server expects, which I guess in this case would involve using percent-encoding for that period:

-backend-config="username=user%2Ename"

Please let me know if I've missed something here! Thanks,

@FalcoSuessgott
Copy link
Author

@apparentlymart thanks for that explanation, your right! :) 👍

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants