Skip to content

Support for passing in Basic Auth tokens and resolve issue with HTTP/1.1 reverse proxies#118

Closed
sullivanmatt wants to merge 3 commits intosplunk:developfrom
sullivanmatt:basic_auth_support
Closed

Support for passing in Basic Auth tokens and resolve issue with HTTP/1.1 reverse proxies#118
sullivanmatt wants to merge 3 commits intosplunk:developfrom
sullivanmatt:basic_auth_support

Conversation

@sullivanmatt
Copy link
Copy Markdown
Contributor

Forgive these two minor fixes being in the same PR. I discovered the HTTP/1.1 proxy issue while working on testing my changes for Basic Auth.

Support for passing in Basic Auth tokens
This pull request modifies the way Splunk handles user-specified tokens. Currently, tokens that don't start with 'Splunk' get this prefix automatically inserted. This clobbers any Basic Auth tokens that the user has previously explicitly passed in:

from splunklib.client import Service

service = Service(host="splunk.myhost.net", port=8089, scheme="https", version=6.3,
                  token="Basic dGVzdHVzZXI6dGVzdHBhc3M=")

service.indexes['test'].submit("My message!", host="localhost", sourcetype="text")

Resolve issue with HTTP/1.1 reverse proxies
The Splunk SDK uses the built-in Python library httplib for making HTTP connections. This library does not support the use of HTTP/1.1 Connection: keep-alive. When used in its default mode, the request header Connection: close is automatically added to ensure compatibility with servers isn't broken. However the Splunk library passes in its own headers without a Connection declaration. Normally this isn't a problem, as the splunkd server running on 8089 does not support keep-alive either and just assumes the behavior to be close.

However if using a reverse proxy in front of this service, major issues arise because the SDK is not conforming to the HTTP/1.1 header spec, RFC2616 Section 14.10:

HTTP/1.1 applications that do not support persistent connections MUST include the "close" connection option in every message.

In my testing (nginx and Apache2), Splunk SDK would receive an empty response on all GET requests and fail. With this change, all requests proceed as expected.

@itay
Copy link
Copy Markdown
Contributor

itay commented Aug 25, 2015

@sullivanmatt overall, the change looks great. A few notes:

  1. splunkd (on 8089) actually does support keep-alive. However, we've found that the Python reliability with keep-alive is not high, and so we don't use it by default. That said, sending the close connection header is a good addiiton.
  2. Do you think you could add a couple of tests for the Basic Auth case? It could be something like:
    a. Create a user with a specific username and password
    b. Create a new service with a Basic Auth token
    c. Ensure it works
  3. I would consider making login() a no-op in this case, to make sure a new token doesn't come back.
  4. Another way to implement this (and I'm not saying you should change), is to force someone to pass in a username/password and then a basic_auth=True parameter to Context, and in turn we will internally do all the base64 book-keeping, etc. That may be more user-friendly.

@itay
Copy link
Copy Markdown
Contributor

itay commented Nov 21, 2015

@sullivanmatt do you think you're going to pick this up again?

@sullivanmatt
Copy link
Copy Markdown
Contributor Author

Yikes, it's been a while, hasn't it?

Yes, I do plan to pick this back up, if you don't mind leaving it open for a bit. Thanks!

@itay
Copy link
Copy Markdown
Contributor

itay commented Nov 21, 2015

Yep - no problem. Let's just rebase it on top of develop, which will pick up the automated CI now, and address the comments above.

By the way, there was a similar change in the Ruby SDK - you can take a look there what was done.

@sullivanmatt
Copy link
Copy Markdown
Contributor Author

Going to close this PR and issue the PRs for connection-close and the basic auth stuff as separate issues.

mateusz834 pushed a commit that referenced this pull request Apr 15, 2026
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.

3 participants