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

JWT token only #73

Merged
merged 11 commits into from
Aug 21, 2023
Merged

JWT token only #73

merged 11 commits into from
Aug 21, 2023

Conversation

sverhoeven
Copy link
Member

@sverhoeven sverhoeven commented Jul 18, 2023

Replaces the storing of user in db and social logins with asymmetric JWT token.

Fixes #54
Fixes #58
Fixes #57
Fixes #25
Fixes #51

TODO

  • get rid of user management
  • add asymmetric JWT auth
  • dont alter existing alembic versions, but add new one fastapi_users_db_sqlalchemy.generics was imported, so we needed to change version anyway as we dont have that dep anymore.
  • add rsa key pair to Docker compose and document

Should be merged together with i-VRESSE/haddock3-webapp#47

@sverhoeven sverhoeven marked this pull request as ready for review July 19, 2023 12:06
@sverhoeven sverhoeven mentioned this pull request Jul 19, 2023
22 tasks
@sverhoeven sverhoeven requested a review from Peter9192 August 10, 2023 10:58
Copy link
Contributor

@Peter9192 Peter9192 left a comment

Choose a reason for hiding this comment

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

Nice clean up of the bar. I've tested running bartender on my local machine and through docker-compose. It all seems to work nicely. Some of the explanations were a bit unclear to me. I'll continue to review i-VRESSE/haddock3-webapp#47.

Comment on lines 45 to 51
The private key of the RSA key pair is used to generate a token in
an another web application or with the `bartender generate-token` command.

The public key of the RSA key pair is used to verify that the token comes
from a trusted source.
The public key file location is `public_key.pem`
or value of `BARTENDER_PUBLIC_KEY` environment variable.
Copy link
Contributor

Choose a reason for hiding this comment

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

In this description, who is the owner of the keypair? Is it the bartender or the consumer?

Copy link
Contributor

Choose a reason for hiding this comment

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

If my understanding is correct, in this case, the "sender" should be the owner of the keypair. The sender uses their private key to create a token and the "recipient" uses the senders' public key to verify that it could only have been the sender (i.e. the only one knowing their private key) to have generated the token. Correct?

So does that mean that bartender could potentially store multiple public keys for various senders?

Copy link
Member Author

Choose a reason for hiding this comment

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

Create #75 for multiple public keys. Will implement when we have a concrete use case.

Copy link
Member Author

Choose a reason for hiding this comment

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

If my understanding is correct, in this case, the "sender" should be the owner of the keypair. The sender uses their private key to create a token and the "recipient" uses the senders' public key to verify that it could only have been the sender (i.e. the only one knowing their private key) to have generated the token. Correct?

Yep, correct. That is how https://en.wikipedia.org/wiki/Digital_signature works. The signer owns the private key and gives a public key to the verifier.

So does that mean that bartender could potentially store multiple public keys for various senders?

egi_redirect_url: Optional[str] = None
egi_environment: Literal["production", "development", "demo"] = "production"
# RSA public key used to verify JWT tokens
public_key: Path = Path("public_key.pem")
Copy link
Contributor

Choose a reason for hiding this comment

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

How does that work when the issuer is not bartender itself?

Copy link
Member Author

Choose a reason for hiding this comment

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

In i-VRESSE/haddock3-webapp@441c87d changed issuer to haddock3 webapp as it is the one generating and signing the token with its private key.

Copy link
Member Author

Choose a reason for hiding this comment

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

Bartender does not care who is the issuer is. Only that the signature in the token can be verified with its public key. The public key is generated from a private key so there is a pairing up of signer (with private key ) and verifier.

Comment on lines +3 to +5
Create config file `config.yaml` as described at [configuration.md](configuration.md).
The `job_root_dir` property should be set to `/tmp/jobs`
which is a Docker compose volume.
Copy link
Contributor

Choose a reason for hiding this comment

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

At first I couldn't find that setting on the linked page. It would be helpful if the job_root_dir section on that page was closer to this header. Perhaps just right above "applications"

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in a20d098

docs/deploy.md Outdated
Comment on lines 20 to 21
To use `bartender generate-token` command inside container you need make
the private key available in the container.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps add an instruction on how to do that (make private key available to container)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added in 1505b9c

Copy link
Member Author

@sverhoeven sverhoeven left a comment

Choose a reason for hiding this comment

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

Thanks for reviewing and learning about asym jwt during the process.

docs/deploy.md Outdated
Comment on lines 20 to 21
To use `bartender generate-token` command inside container you need make
the private key available in the container.
Copy link
Member Author

Choose a reason for hiding this comment

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

Added in 1505b9c

Comment on lines +3 to +5
Create config file `config.yaml` as described at [configuration.md](configuration.md).
The `job_root_dir` property should be set to `/tmp/jobs`
which is a Docker compose volume.
Copy link
Member Author

Choose a reason for hiding this comment

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

Done in a20d098

@sverhoeven sverhoeven merged commit 7db53ef into main Aug 21, 2023
@sverhoeven sverhoeven deleted the asym-jwt-only branch August 21, 2023 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants