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

Support cookies in http ldap connections (#237) #239

Merged
merged 3 commits into from
May 25, 2024

Conversation

airosa
Copy link
Contributor

@airosa airosa commented May 14, 2024

Support cookies in http ldap connections.

We have been running code from then forked package since beginning of the year and we have not seen issues related to this patch.

closes #237

airosa and others added 2 commits May 14, 2024 13:42
Support cookies in http ldap connections

---------

Co-authored-by: Airo, Sami <sami.airo@ecb.europa.eu>
@airosa
Copy link
Contributor Author

airosa commented May 17, 2024

Really nice integration test!

I looked briefly at the output and it looks like this change could be breaking the kerberos authentication support when using HTTP - bummer! We are using kerberos+binary and simple+http so I have not tested this combination before.

Here are the lines that I saw erroring out - they point out that the cookie has a problem:

2024-05-17T03:43:09,512  INFO [HiveServer2-HttpHandler-Pool: Thread-61] thrift.ThriftHttpServlet: Could not validate cookie sent, will try to generate a new cookie
2024-05-17T03:43:09,516 ERROR [HiveServer2-HttpHandler-Pool: Thread-61] thrift.ThriftHttpServlet: Failed to authenticate with hive/_HOST kerberos principal

@airosa
Copy link
Contributor Author

airosa commented May 20, 2024

I changed the approach a bit and as a mitigation it now just deduplicates the request cookies before the request is sent. The cookies are not reset any more when the cursor is closed.

The deduplication is done in the following Go http "roundtripper" (https://pkg.go.dev/net/http#RoundTripper)

type CookieDedupTransport struct {
	http.RoundTripper
}

// RoundTrip removes duplicate cookies (cookies with the same name) from the request
// This is a mitigation for the issue where Hive/Impala cookies get duplicated in the response
func (d *CookieDedupTransport) RoundTrip(req *http.Request) (*http.Response, error) {
	cookieMap := map[string]string{}
	for _, cookie := range req.Cookies() {
		cookieMap[cookie.Name] = cookie.Value
	}

	req.Header.Set("Cookie", "")

	for key, value := range cookieMap {
		req.AddCookie(&http.Cookie{Name: key, Value: value})
	}

	resp, err := d.RoundTripper.RoundTrip(req)

	return resp, err
}

The roundtripper is added to the http client in the getHTTPClient function.

httpClient.Transport = &CookieDedupTransport{httpClient.Transport}

@beltran
Copy link
Owner

beltran commented May 25, 2024

Thank you for writing this PR and following up on the failing test.

@beltran beltran merged commit 9d317a0 into beltran:master May 25, 2024
1 check passed
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.

HTTP(S) plain/ldap authentication connection does not support cookies and issue with the cookie handling
3 participants