-
Notifications
You must be signed in to change notification settings - Fork 915
Don't send empty credentials to SR in Authorization Header #863
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's testconf.json.swp?
tests/schema_registry/conftest.py
Outdated
headers = request._request.headers | ||
|
||
# Pass request to downstream matchers | ||
if 'Authorization' not in headers.keys(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how this works under the hood, but this seems like it needs to scan keys() for 'Authorization', would suggest using ..keys().get('Authorization', None)
tests/schema_registry/conftest.py
Outdated
if 'Authorization' not in headers.keys(): | ||
return None | ||
|
||
userinfo = headers.get('Authorization').split(" ")[1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here's a second lookup. Just do one lookup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent use of single/double quotes on the same line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will throw an IndexError if the Authorization header does not contain a space, but maybe that's okay here.
tests/schema_registry/conftest.py
Outdated
# Authenticated, pass downstream | ||
return None | ||
|
||
unauhtorized = {'error_code': 401, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo
tests/schema_registry/conftest.py
Outdated
'message': "401 Unauthorized"} | ||
return create_response(request=request, | ||
status_code=401, | ||
json=unauhtorized) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo
|
||
Verify that the package works, should print the expected Python client | ||
and librdkafka versions: | ||
|
||
$ python -c 'import confluent_kafka as ck ; print "py:", ck.version(), "c:", ck.libversion()' | ||
$ python -c 'import confluent_kafka as ck ; print "py: {} c: {}" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not work for python3
No description provided.