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

Public key linkage between FlowAuth and FlowAPI #864

Merged
merged 94 commits into from
Jul 1, 2019
Merged

Conversation

greenape
Copy link
Member

@greenape greenape commented May 29, 2019

Closes #89 closes #779 closes #735

I have:

  • Formatted any Python files with black
  • Brought the branch up to date with master
  • Added any relevant Github labels
  • Added tests for any new additions
  • Added or updated any relevant documentation
  • Added an Architectural Decision Record (ADR), if appropriate
  • Added an MPLv2 License Header if appropriate
  • Updated the Changelog

Description

This changes the way JWTs are verified to use RSA public keys instead of a shared secret.

FlowAuth now expects to get a private key, with which it will sign all the JWTs it generates. FlowAPI now expects to get the corresponding public key, with which it can verify the JWTs. This means there are no longer any shared secrets, only a single key pair. The public key can be safely distributed, because it allows you only to verify JWTs.

Adds:

  • Two new env vars/secrets:
    • PRIVATE_JWT_SIGNING_KEY environment variable/secret added to FlowAuth, which should be a PEM encoded RSA private key, optionally base64 encoded if supplied as an environment variable.
    • PUBLIC_JWT_SIGNING_KEY environment variable/secret added to FlowAPI, which should be a PEM encoded RSA public key, optionally base64 encoded if supplied as an environment variable.
  • A new admin page to FlowAuth, which allows you to view/download the public key.

@greenape greenape added the ready-to-merge Label indicating a PR is OK to automerge label Jun 18, 2019
@greenape greenape mentioned this pull request Jun 19, 2019
docs/source/install.md Outdated Show resolved Hide resolved
docs/source/install.md Outdated Show resolved Hide resolved
greenape and others added 2 commits July 1, 2019 15:17
Co-Authored-By: maxalbert <maximilian.albert@gmail.com>
"""

if audience is None and "FLOWAPI_IDENTIFIER" in os.environ:
audience = os.environ["FLOWAPI_IDENTIFIER"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean if audience is None and FLOWAPI_IDENTIFIER is not set, then audience=None will be passed to generate_token() below? Is this an issue at all?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nah. Totally valid thing to do, and will just result in no audience key in the JWT - the resulting token wouldn't be usable for flowapi, but would by no means be invalid.

Copy link
Contributor

Choose a reason for hiding this comment

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

So what's a scenario where this may be useful?

@@ -273,32 +237,39 @@ def print_token(username, secret_key, lifetime, audience):

For example:


generate-jwt --username TEST_USER --private-key $PRIVATE_JWT_SIGNING_KEY --lifetime 1 --audience TEST_SERVER all-access http://localhost:9090
Copy link
Contributor

Choose a reason for hiding this comment

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

This is missing the --flowapi-url option:

Suggested change
generate-jwt --username TEST_USER --private-key $PRIVATE_JWT_SIGNING_KEY --lifetime 1 --audience TEST_SERVER all-access http://localhost:9090
generate-jwt --username TEST_USER --private-key $PRIVATE_JWT_SIGNING_KEY --lifetime 1 --audience TEST_SERVER all-access --flowapi-url http://localhost:9090

Copy link
Contributor

Choose a reason for hiding this comment

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

Strangely, the --flowapi-url doesn't show up though when running generate-jwt --help?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes. Little convoluted that - it'll only show up in the help for the subcommand (all-access). Annoyingly, click won't let you get to that bit unless you've successfully completed all the required named args, so you need to do something like enerate-jwt --username TEST_USER --private-key $PRIVATE_JWT_SIGNING_KEY --lifetime 1 --audience TEST_SERVER all-access --help to see it.

Copy link
Contributor

@maxalbert maxalbert left a comment

Choose a reason for hiding this comment

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

Great work! 🏅

This looks good from what I can tell by reading through the changes (I haven't done any manual testing of using this in practice though).

@greenape greenape merged commit f682c91 into master Jul 1, 2019
@greenape greenape deleted the public-key-auth branch July 1, 2019 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request FlowAPI Issues related to the FlowKit API FlowAuth Issues related to FlowAuth flowkit-jwt-generator ready-to-merge Label indicating a PR is OK to automerge security
Projects
None yet
2 participants