use rsa key for jwt

[rmb938]

Fixes #26

[codecov-io]

Codecov Report

Merging #27 into master will decrease coverage by 3.78%.
The diff coverage is 77.27%.

@@            Coverage Diff            @@
##           master     #27      +/-   ##
=========================================
- Coverage   37.28%   33.5%   -3.79%     
=========================================
  Files           9      13       +4     
  Lines         692     991     +299     
=========================================
+ Hits          258     332      +74     
- Misses        400     622     +222     
- Partials       34      37       +3

Impacted Files	Coverage Δ
handlers/User.go	27.77% <70%> (ø)
handlers/handler.go	83.07% <83.33%> (ø)
handlers/pipeline_run.go	0% <0%> (ø)
handlers/pipeline.go	0% <0%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59bb2bf...9cfb17a. Read the comment docs.

[Skarlso]

So, I advice against this because... :)

First off, your key would be visible in your bash history even if it's a dev environment that would be a bad idea. Unless you would like to use a vault and pass in an environment property which is a hassle.

Second, the signing should happen via a public / private key-pair to be honest rather than a passed in OR generated jws key. So instead of doing this, try focusing on a key exchange solution?

[rmb938]

I can also add an environment variable for the key.

Also why not support both a key and a public/private key-pair? If someone wants to spin up gaia just to try out or quickly dev with having to first generate a keypair is kinda annoying.

[Skarlso]

Because providing the ability to circumvent security because of convenience is just asking for people to use it in production and they definitely will because it's easier. :)

Instead do this:

• Load in a private key ( for which the name could be passed in from command line )
• And use the content as a security key
• Use the public part of that key as a verify key

Shouldn't be too difficult and would remove the need for yet another flag.

EDIT jwt-go already provides examples and you can use ParseRSAPrivateKeyFromPEM and the accompanying things for this.

[rmb938]

Ok, no problem. I can make those changes later today.

[Skarlso]

...
privateKey, _ := ioutil.ReadFile(privateKeyPath)
signKey, _ = jwt.ParseRSAPrivateKeyFromPEM(privateKey)
...
tokenString, err := t.SignedString(signKey)
...

Something like this should work juuuust nicely. :) with err handling ofc. :P

[rmb938]

Yup, that's what I was planning.

[Skarlso]

Cool. And here https://github.com/gaia-pipeline/gaia/blob/master/handlers/handler.go#L144 you can use the opposite ParseRSAPublicKeyFromPEM.

Alright, I shuddup. It's just an interesting PR. :)

[michelvocks]

Thanks a lot @rmb938 for your contribution. 🤗

I personally think that the initial startup of Gaia should be easy and simple.
If I understand your code correctly then currently the user is forced to set the parameter jwtkey and like you already said: This will be a bad first experience for all new users.
I also totally agree with @Skarlso point that this is an security issue.

So what do you think about providing a parameter to set a public/private key and if this parameter is empty we generate a token (like we do it now). Do you think this is still an security issue? 😉

[rmb938]

Yup that makes sense. What do you think about printing a warning message if the auto generated key is being used? That way users know they messed up something if they actually wanted to use a private key.

[Skarlso]

@rmb938 @michelvocks Sounds cool! 👍

[michelvocks]

Awesome! 🤗
Looking forward to it!

[rmb938]

@michelvocks How does that look?

[Skarlso]

Oh cool, you can do .Public. Nice.

[Skarlso]

This is looking good. :) You'll have to write tests though. ;) :)

[rmb938]

@Skarlso @michelvocks Added tests for everything I added so we should be good to go :)

[Skarlso]

Looking good! Good job with the tests! 👍 :)

[michelvocks]

@rmb938 can you have a short look at my review comment? ☺️

Would be nice if you could change this one thing.
The rest looks really really good. Good job 🤗

[michelvocks]

Do me a favor and move this variable to the gaia.Cfg struct.
I think it's a bit more tidy and makes more sense (in my opinion). 😄

[michelvocks]

LGTM 🤗 Thanks!