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

add support for token based access #8

Merged
merged 4 commits into from
Apr 20, 2017
Merged

Conversation

jafferli
Copy link
Contributor

add support for token based access api

add support for token based access api
@zmallen zmallen self-requested a review April 19, 2017 12:40
@@ -4,6 +4,11 @@
from urlparse import urlparse
class GraylogAPI(object):
def __init__(self, url, username=None, password=None, api_key=None):

# check if the username / password vs. api_key cannot be provided at same time
if username is not None and api_key is not None:
Copy link
Owner

Choose a reason for hiding this comment

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

This check is raising an error in the unit tests. My question is: should it not matter if the username, password and api_key are set? We could just prefer the API key if it is provided, which is what seems like you added on line 42.


# use the api_key if it is been provided
if self.api_key is not None:
payload = self.api_key + ':token'
Copy link
Owner

Choose a reason for hiding this comment

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

👍 this looks like the way Graylog constructs the token header, http://docs.graylog.org/en/2.2/pages/configuration/rest_api.html

Copy link
Owner

@zmallen zmallen left a comment

Choose a reason for hiding this comment

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

I think the authorization header looks correct, and it matches documentation. Thanks for the P/R!

Could you please add some unit tests and fix the current broken unit test? It throws an exception since we generate a Graylog Object w/ username/password & an API key. I would split that unit test into a username/password one and a token based one. Lastly, could you increment setup.py version of pygraylog so I can do a pypi release?

Thank you!

@zmallen
Copy link
Owner

zmallen commented Apr 19, 2017

One last observation - update readme to show token based auth as well!

@zmallen zmallen mentioned this pull request Apr 19, 2017
update version from 0.3.1 to 0.3.2
@jafferli
Copy link
Contributor Author

update the patch-1 branch for

  1. setup.py for version change to 0.3.2
  2. unit test item

@zmallen
Copy link
Owner

zmallen commented Apr 20, 2017

Thank you @jafferli ! I'll get this pushed to pypi asap

@zmallen zmallen merged commit 22c8cd3 into zmallen:master Apr 20, 2017
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.

2 participants