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 python client example #156

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bytesandwich
Copy link

Demonstrates a python flask app to authenticate using keymaster
and store tokens in JWT. Uses a JWT rotation strategy.

Demonstrates a python flask app to authenticate using keymaster
and store tokens in JWT. Uses a JWT rotation strategy.
@cviecco cviecco self-requested a review September 10, 2018 02:41
Copy link
Contributor

@cviecco cviecco left a comment

Choose a reason for hiding this comment

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

I general: Thanks for starting this. But still needs some work. The docker dependency on the generator must be removed, the docker dependency on the app I think it should but please make your case. (I am inclined on asking the removal, but I want to hear reasoning).

@@ -0,0 +1,8 @@
FROM ubuntu:18.04
Copy link
Contributor

Choose a reason for hiding this comment

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

a docker container for a self-signing is overkill. Just add the Bash script.

cd /output
DOMAIN=localhost.$WEBSITE
ANY_INTEGER=$RANDOM
openssl genpkey -algorithm RSA -out key.pem -pkeyopt rsa_keygen_bits:2048
Copy link
Contributor

Choose a reason for hiding this comment

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

It is my guess that you added this because of the old version of openssl on macos, so convert this line into:
openssl genpkey -algorithm RSA -out key.pem -pkeyopt rsa_keygen_bits:2048 || openssl genrsa 2048

Copy link
Author

Choose a reason for hiding this comment

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

Can you help me get around the error I get on osx?

+ openssl genpkey -algorithm RSA -out key.pem -pkeyopt rsa_keygen_bits:2048
openssl:Error: 'genpkey' is an invalid command.

All keyword args will be signed into a JWT with the response.
"""
resp = make_response(redirect(url))
resp.set_cookie(jwt_cookie_key, jwt.encode(
Copy link
Contributor

Choose a reason for hiding this comment

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

Both the Cookie and the jwt token should have lifetimes. Which you should verify, for the redirection is should not be more than 300s and once authenticated the actual duration it should be the duration of the authentication request or if not possible, something in the order of hours (2-8).

Also if you plan to use the same cookie name for the redirect and the outh presence you should add a field to distinguish these two cases.

jwt = get_jwt_token(request.cookies)
if 'oauth_token' in jwt:
# User is authenticated, don't do the auth dance.
client = OAuth2Session(client_id, token=jwt['oauth_token'])
Copy link
Contributor

Choose a reason for hiding this comment

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

you should store the userinfo in the jwt contents, so that you dont pay a round-trip to the oauth2 provider at every request. (this is another reason to distinguish the two cookies)

# be rotated out.
for jwt_secret in jwt_secrets:
try:
return jwt.decode(
Copy link
Contributor

Choose a reason for hiding this comment

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

you need to add semantic validation... (probably here, problably somewhere else):

  1. Does the jwt return makes sense for this request?
  2. Is is still valid?
  3. It it valid yet?

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