Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions examples/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,49 @@ def token_gen_call(username, password):
if mockpassword == password and mockusername == username: # This is an example. Don't do that.
return {"token" : jwt.encode({'user': username, 'data': 'mydata'}, secret_key, algorithm='HS256')}
return 'Invalid username and/or password for user: {0}'.format(username)

# JWT AUTH EXAMPLE #
replace_this = False
config = {
'jwt_secret': 'super-secret-key-please-change',
# token will expire in 3600 seconds if it is not refreshed and the user will be required to log in again
Copy link

Choose a reason for hiding this comment

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

Your sentences do not begin with a Capital letter and don't end with a dot(.), is this intended ?

Copy link
Author

Choose a reason for hiding this comment

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

I'm pretty new to collaboration. Is that the standard for comments? Begin with a capital letter and end with a dot?

Copy link

Choose a reason for hiding this comment

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

Well, I mean normally sentences have a Capital letter at the start and ends with a dot 😝. But I can see that this projects sentences don't end with a dot xD

Ex: https://github.com/timothycrosley/hug/blob/a2dfdd71dc5cb809107ca710fd6c0629c6802cfc/hug/api.py#L77

So lets stick with the project's standard and use a Capital letter for the begging of the sentence. 😉

Copy link
Author

Choose a reason for hiding this comment

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

PEP seems to indicate that "If a comment is short, the period at the end can be omitted."

I usually start comments with lower case, but yeah, that doesn't seem compliant. I'll change this then.

Copy link

Choose a reason for hiding this comment

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

Ah yea that is true, guess we have different standards for short comments 😁 .

'token_expiration_seconds': 3600,
# if a request is made at a time less than 1000 seconds before expiry, a new jwt is sent in the response header
'token_refresh_seconds': 1000
}
# enable authenticated endpoints, example @authenticated.get('/users/me')
authenticated = hug.http(requires=hug.authentication.json_web_token(hug.authentication.verify_jwt, config['jwt_secret']))

# check the token and issue a new one if it is about to expire (within token_refresh_seconds from expiry)
@hug.response_middleware()
def refresh_jwt(request, response, resource):
authorization = request.get_header('Authorization')
if authorization:
token = hug.authentication.refresh_jwt(authorization, config['token_refresh_seconds'],
config['token_expiration_seconds'], config['jwt_secret'])
if token:
response.set_header('token', token)

@hug.post('/login')
Copy link

Choose a reason for hiding this comment

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

☝️

def login(request, response,
email: fields.Email(),
password: fields.String()
):
response.status = falcon.HTTP_400

user = replace_this # store.get_user_by_email(email)
if not user:
return {'errors': {'Issue': "User not found."}}
elif 'password_hash' in user:
if replace_this: # if bcrypt.checkpw(password.encode('utf8'), user.password_hash):
response.status = falcon.HTTP_201
token = hug.authentication.new_jwt(
str(user['_id']),
config['token_expiration_seconds'],
config['jwt_secret'])
response.set_header('token', token)
else:
return {'errors': {'Issue': "Password hash mismatch."}}
else:
return {'errors': {'Issue': "Please check your email to complete registration."}}
# END - JWT AUTH EXAMPLE #
89 changes: 89 additions & 0 deletions hug/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@
"""
from __future__ import absolute_import

import jwt
import base64
import binascii

from falcon import HTTPUnauthorized
from datetime import datetime, timedelta


def authenticator(function, challenges=()):
Expand Down Expand Up @@ -138,3 +140,90 @@ def verify_user(user_name, user_password):
return user_name
return False
return verify_user

# JWT AUTH #
def jwt_authenticator(function, challenges=()):
"""Wraps authentication logic, verify_token through to the authentication function.

The verify_token function passed in should accept the authorization header and the jwt secret
and return a user id to store in the request context if authentication succeeded.
"""
challenges = challenges or ('{} realm="simple"'.format(function.__name__), )

def wrapper(verify_token, jwt_secret):
def authenticate(request, response, **kwargs):
result = function(request, response, verify_token, jwt_secret)

def jwt_authenticator_name():
try:
return function.__doc__.splitlines()[0]
except AttributeError:
return function.__name__

if result is None:
raise HTTPUnauthorized('Authentication Required',
'Please provide valid {0} credentials'.format(jwt_authenticator_name()),
challenges=challenges)

if result is False:
raise HTTPUnauthorized('Invalid Authentication',
'Provided {0} credentials were invalid'.format(jwt_authenticator_name()),
challenges=challenges)

request.context['user_id'] = result
return True

authenticate.__doc__ = function.__doc__
return authenticate

return wrapper

@jwt_authenticator
Copy link

Choose a reason for hiding this comment

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

☝️ 2 new lines between functions

def json_web_token(request, response, verify_token, jwt_secret):
"""JWT verification

Checks for the Authorization header and verifies it using the verify_token function
"""
authorization = request.get_header('Authorization')
if authorization:
verified_token = verify_token(authorization, response, jwt_secret)
if verified_token:
return verified_token
else:
return False
return None

Copy link

Choose a reason for hiding this comment

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

☝️

def verify_jwt(authorization, response, jwt_secret):
try:
token = authorization.split(' ')[1]
decoding = jwt.decode(token, jwt_secret, algorithm='HS256')
print(decoding)
Copy link

Choose a reason for hiding this comment

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

Is this print statement suppose to be there ?

Copy link
Author

@dsmurrell dsmurrell Aug 24, 2017

Choose a reason for hiding this comment

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

You are right. It's not.

return decoding['user_id']
except Exception as ex:
template = "An exception of type {0} occurred. Arguments:\n{1!r}"
print(template.format(type(ex).__name__, ex.args))
Copy link

Choose a reason for hiding this comment

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

Same as above ☝️

Copy link
Author

Choose a reason for hiding this comment

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

What is the standard way to report on something unexpected? I would normally use a logger, but it's not in this scope. @timothycrosley how do you normally handle reporting unexpected exceptions in hug?

return False
return None

def new_jwt(user_id, token_expiration_seconds, jwt_secret):
return jwt.encode({'user_id': user_id,
'exp': datetime.utcnow() + timedelta(seconds=token_expiration_seconds)},
jwt_secret, algorithm='HS256').decode("utf-8")

def refresh_jwt(authorization, token_refresh_seconds, token_expiration_seconds, jwt_secret):
Copy link

Choose a reason for hiding this comment

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

☝️

try:
token = authorization.split(' ')[1]
decoding = jwt.decode(token, jwt_secret, algorithm='HS256')
exp = decoding['exp']

if datetime.utcnow() > (datetime.utcfromtimestamp(exp) - timedelta(seconds=token_refresh_seconds)):
return jwt.encode({'user_id': decoding['user_id'],
'exp': datetime.utcnow() + timedelta(seconds=token_expiration_seconds)},
jwt_secret, algorithm='HS256').decode("utf-8")
else:
Copy link

Choose a reason for hiding this comment

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

Isn't this else statement redundant ? 🤔

Copy link
Author

@dsmurrell dsmurrell Aug 24, 2017

Choose a reason for hiding this comment

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

Does a Python function return None if it reaches its end? Even so, isn't it better to be explicit about where you return None and what it means?

Copy link

Choose a reason for hiding this comment

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

Wel, yes python automatically returns None but there is no harm in explicitly returning None yourself. My point here was the following:

if x:
    return x
else:
    return y

Can be rewritten to:

if x:
    return x

return y

As of there is no need to say else because the if statement exists the function if its true.

Copy link
Author

@dsmurrell dsmurrell Aug 24, 2017

Choose a reason for hiding this comment

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

Yes, I guess it is strictly redundant. It also takes the same space. I prefer the readability of the former though as it links the code in the else to if statement. Is there a general standard on the best way in Python?

Copy link

Choose a reason for hiding this comment

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

Well it is opinion based really as you can see here.

So i guess we should see what @timothycrosley thinks about this one. Or we can search in the project's code for an if else return block code and use that as reference to be consistent with the already written code.

Copy link
Author

Choose a reason for hiding this comment

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

Sounds good.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the feedback btw. I'm new to this open source collaboration stuff.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@dsmurrell, I want to stress that for being new to open source collaboration, this is a very good high quality first pull request, great work!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a slight preference for removing the return None in both places, and then remove the else and let it fall through

return None
except Exception as ex:
template = "An exception of type {0} occurred. Arguments:\n{1!r}"
print(template.format(type(ex).__name__, ex.args))
Copy link

Choose a reason for hiding this comment

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

Suppose to print ?

Copy link
Author

Choose a reason for hiding this comment

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

Same as above. Looking for the way hug reports or outputs unexpected exceptions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @dsmurrell,

The answer is that you should catch any errors you expect and want to use to change output, and let any errors you don't expect bubble up and be handled by code higher up.

Thanks!

~Timothy

return None
# END - JWT AUTH #