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 addon #356

Merged
merged 11 commits into from
Jul 28, 2016
Merged

JWT addon #356

merged 11 commits into from
Jul 28, 2016

Conversation

sebv
Copy link

@sebv sebv commented Dec 12, 2014

JWT token authentication for PR.

Users need to add this to .travis.yml.

addons:
  jwt: 
     secure: <SAUCE_ACCESS_KEY=123456789 encrypted> 

The JWT addon will then create a time-limited JWT token and export it to the SAUCE_ACCESS_KEY environment variable.

This is not specific to sauce labs several services may be configured, for instance:

addons:
  jwt:
     saucelabs:
        secure: <SAUCE_ACCESS_KEY=123456789 encrypted>
     moonwalk:
        secure: <MOONWALK_SECRET=123456789 encrypted>

In this case 2 tokens will be generated, one in the SAUCE_ACCESS_KEY env variable, the other in MOONWALK_SECRET.

@sebv sebv changed the title Added JWT addon JWT addon Dec 12, 2014
@sebv
Copy link
Author

sebv commented Dec 12, 2014

depends on travis-ci/travis-core#418

@sebv
Copy link
Author

sebv commented Dec 12, 2014

ping @joshk @sso @sourishkrout

@BanzaiMan BanzaiMan self-assigned this Jan 5, 2015
BanzaiMan added a commit that referenced this pull request Jan 6, 2015
@BanzaiMan
Copy link
Contributor

Could you add documentation on how to use it, and security implications thereof, in the form of a PR to https://github.com/travis-ci/docs-travis-ci-com?

@BanzaiMan
Copy link
Contributor

This has been deployed to staging. Please go to https://staging.travis-ci.org/profile, sync data with GitHub and enable a repository to test this.

The staging environment might be refreshed with a different code without prior notice, so if you don't see Initializing JWT in the build log when you have the addon set, do let us know, so that we can get the correct code out.

@sourishkrout
Copy link
Contributor

Thank you @BanzaiMan for moving this along. The JWT addon is not expect to work without the changes submitted as part of the PR in travis-ci/travis-core#418. - I now realize we should have included that in the initial description. Is it possible to get the travis-core changes deployed to staging as well?

@BanzaiMan
Copy link
Contributor

@sourishkrout The PR to travis-core has been deployed to staging. Could you try it? (Make sure that you use no critically sensitive data.)

@sourishkrout
Copy link
Contributor

Unfortunately it doesn't look like it worked.

Added jwt addon config to my .travis.yml (https://github.com/sourishkrout/osbdemo/blob/master/.travis.yml) according to this:

Repository slug: |travis-ci/travis-core| sourishkrout/osbdemo
Reading from stdin, press Ctrl+D when done
SAUCE_BLA=12345678
Please add the following to your .travis.yml file:

  secure: "eDQEcNFA0nQzTUj1HmnedM8+vWtaxLehmEgDzf/EEI/Bk6tYcnJMqpA0VMklUDsdmkpxpMRgspdrv1+T4LdYFGMJUOJ+4A7DjkEiTvpKammhgRRYja3VjSpBjx8svxvxwLOHY6l81xRglsdjyWrCx87gUNBr5jFZ5Pd0SJM3LqI="

Which didn't produce the JWT ENV var I expected (SAUCE_BLA=<jwt token>):

> export | grep SAUCE
export SAUCE_USERNAME='sourishkrout'
export npm_lifecycle_script='export | grep SAUCE'
export npm_package_scripts_test='export | grep SAUCE'

@BanzaiMan I wonder how we can debug this. I'm curious to find out if the decrypted JWT addon config from travis-core made it into travis-build at all?

Travis CI Job: https://staging.travis-ci.org/sourishkrout/osbdemo/builds/400203

@BanzaiMan
Copy link
Contributor

@sourishkrout The data format shown in https://github.com/sourishkrout/osbdemo/blob/7c4039ed7b22cd5e89411400f28e227459b8d5fa/.travis.yml#L12-L13 doesn't match the one shown in the original description of this PR.

I tested this earlier https://github.com/BanzaiMan/travis_staging_test/blob/c60885ce90cd6420ba557ac120b30650476a0cd1/.travis.yml#L10-L12 and the latter format seems to work; i.e., invokes JWT installation (https://staging.travis-ci.org/BanzaiMan/travis_staging_test#L103).

@BanzaiMan
Copy link
Contributor

Hi, @sourishkrout and @sebv,

We are itching to test this further. Could you advise us when we can coordinate our efforts to do so? You can respond here, or email support@travis-ci.com.

Thank you!

@sebv
Copy link
Author

sebv commented Apr 30, 2015

@BanzaiMan also rebased this one.

@sebv
Copy link
Author

sebv commented Apr 30, 2015

btw this yml format is correct.

@admc
Copy link

admc commented May 15, 2015

Hi @BanzaiMan, we are really interested in getting this project completed, Any chance you can reach out to me admc@saucelabs.com so I can work to arrange a live session where we can get everyone to work together to finish this? Thanks!

@BanzaiMan
Copy link
Contributor

Sorry for the delay. I'll look at this shortly!

BanzaiMan added a commit that referenced this pull request May 21, 2015
subject.should include_sexp [:echo, 'Initializing JWT', ansi: :yellow]
expected1 = "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJzbHVnIjoidHJhdmlzLWNpL3RyYXZpcy1jaSIsInB1bGwtcmVxdWVzdCI6IiIsImlhdCI6MH0.iO-I8CNXhQlgw_dL8R9CDaDPplSC3yf9J399Uumn6CE"
subject.should include_sexp [:export, ['MY_ACCESS_KEY_1', expected1]]
expected2 = "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJzbHVnIjoidHJhdmlzLWNpL3RyYXZpcy1jaSIsInB1bGwtcmVxdWVzdCI6IiIsImlhdCI6MH0.p1B9HnYCh-z5Igek5tr_QVPb1zV1ucwcPZNvY039WKg"

This comment was marked as spam.

@BanzaiMan
Copy link
Contributor

By the way, here's the build from the rebased try on Travis: https://travis-ci.org/travis-ci/travis-build/builds/63575373, which passes. (Note that I made slight changes to the spec syntax, to silence the warnings about the old "should" syntax.)

I apologize for not understanding the mechanism behind this encryption scheme enough to offer a better approach. How can we make these specs platform-independent? I see that a good portion of the encrypted data do match up in the beginning. Is that good enough for specs?

@sebv
Copy link
Author

sebv commented May 22, 2015

@BanzaiMan could you put some debug here: https://github.com/saucelabs/travis-build/blob/jwt/lib/travis/build/addons/jwt.rb#L25 and run the test for mac and linux, so we can see the difference before encoding.

We never ran the test on mac, not sure how to set those up.

@sourishkrout
Copy link
Contributor

I didn't to the digging to figure out why but the iat (issued at) is different for aboves spec run on Mac which will result in different payloads and signed hashes:

{
  "slug": "travis-ci/travis-ci",
  "pull-request": "",
  "iat": 18000
}

vs

{
  "slug": "travis-ci/travis-ci",
  "pull-request": "",
  "iat": 0
}

@sebv
Copy link
Author

sebv commented May 22, 2015

Looks like timezone issue, should probably initialize with GMT here https://github.com/saucelabs/travis-build/blob/jwt/spec/build/addons/jwt_spec.rb#L9 .

@joshk
Copy link
Contributor

joshk commented May 22, 2015

@BanzaiMan could you also please make sure you fully understand and grasp how JWT works and how it can be used with Sauce Labs.

@BanzaiMan
Copy link
Contributor

Confirmed that the specs are time (zone) dependent.

@BanzaiMan
Copy link
Contributor

I fixed the specs with e1e54d0

@BanzaiMan
Copy link
Contributor

This (along with travis-ci/travis-core#418) has been deployed to staging. My limited test seems reasonable (works as far as I can tell), and I would like to have a larger set of users test it.

To use this in staging, follow the following steps:

  1. Enable a repository at https://staging.travis-ci.org/profile (You can build against only one of Travis environment, so if you are currently using Travis CI production, i.e., https://travis-ci.org, you need to switch back to it when you are done with the experiment.)

  2. Encrypt the variable against the staging API endpoint:

    travis encrypt -r REPO/OWNER -e https://api-staging.travis-ci.org SUPER_SECRET_STUFF

  3. Create a PR against it. When you try dumping SUPER_SECRET_STUFF, you should see JWT-encoded string instead of the plain text value (https://staging.travis-ci.org/BanzaiMan/travis_staging_test/builds/430639#L101-L102):

    eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJzbHVnIjoiQmFuemFpTWFuL3RyYXZpc19zdGFnaW5nX3Rlc3QiLCJwdWxsLXJlcXVlc3QiOjksImlhdCI6MTQzMjgzNzA4OH0.7GBV_50n04rTwBHzxTpBMFBqWAkojdlsW5yMTnrzQ_Y

The staging environment may be used for other tests, so if you suspect it's not working, please check with me if the correct code is deployed. Thank you!

@BanzaiMan
Copy link
Contributor

With the suggested changes (e46ff5a), it would look like this: https://staging.travis-ci.org/travis-ci/travis_staging_test/builds/507077#L138

Gavin Mogan added 2 commits July 26, 2016 23:00
* Throw a warning when there is no secret available (just key=)
* Throw a warning when secret is null (related)

test for cases
@halkeye
Copy link
Contributor

halkeye commented Jul 27, 2016

Awesome

I figured out how to reproduce the testcase, and moved things slightly, but that should be everything.

I'll do another pass over of the docs tomorrow

@halkeye
Copy link
Contributor

halkeye commented Jul 27, 2016

I guess travis-ci/travis-core#418 needs reviewing too

BanzaiMan added a commit that referenced this pull request Jul 28, 2016
@BanzaiMan
Copy link
Contributor

@halkeye travis-core is getting deprecated, so that PR is replaced by other logic. I'll look at the PR and update it for the new logic.

@halkeye
Copy link
Contributor

halkeye commented Jul 28, 2016

Heh. That's what happens when a pr is over a year old :D

That's great. Let me know if you need anything else.

On Jul 28, 2016 7:22 AM, "Hiro Asari" notifications@github.com wrote:

@halkeye https://github.com/halkeye travis-core is getting deprecated,
so that PR is replaced by other logic. I'll look at the PR and update it
for the new logic.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#356 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGuBzwK9M7PvMnkstT1XOf3jvpW2ojrks5qaLsTgaJpZM4DHeYE
.

@BanzaiMan
Copy link
Contributor

For reference, travis-ci/travis-scheduler#29 is the PR.

I ran a test here: https://staging.travis-ci.org/travis-ci/travis_staging_test/builds/507127

It ran the JWT addon: https://github.com/travis-ci/travis_staging_test/blob/ae7a834f9e7b7122f9bdd31df1fe024cc9305a2c/.travis.yml#L5-L7
which encrypted the value: SAUCE_ACCESS_KEY=foobar.

As you can see on https://staging.travis-ci.org/travis-ci/travis_staging_test/builds/507127#L135, the token is:

eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJUcmF2aXMgQ0ksIEdtYkgiLCJzbHVnIjoidHJhdmlzLWNpL3RyYXZpc19zdGFnaW5nX3Rlc3QiLCJwdWxsLXJlcXVlc3QiOiIiLCJleHAiOjE0Njk3MjY1MjQsImlhdCI6MTQ2OTcyMTEyNH0.FDO_9iHOX5a1WcWUul-hshKYeJ2H7TIQFPbuA2mrcnk

Using https://jwt.io/, with the secret foobar, we see that the signature is correct.

json_web_tokens_-_jwt_io

@BanzaiMan
Copy link
Contributor

I see that travis-ci/travis-scheduler#29 needs more work, to always let jwt decrypt. (Which was a part of the old travis-core PR.) I'll work on this.

@BanzaiMan
Copy link
Contributor

Or, maybe no further work is necessary. In this PR build, you can see that the PR receives a valid JWT.

json_web_tokens_-_jwt_io

@halkeye
Copy link
Contributor

halkeye commented Jul 28, 2016

Yea, if foobar was your original secured token, that looks good to me.

@BanzaiMan
Copy link
Contributor

All right, then. I'll merge this (and the scheduler PR) and deploy to production shortly. I recommend using test data and credentials while we work out the rough edges.

Thanks for the efforts and patience, everyone!

@BanzaiMan
Copy link
Contributor

Once we have good results in production, we'll merge the docs PR and announce it.

@BanzaiMan BanzaiMan merged commit 712b29c into travis-ci:master Jul 28, 2016
@halkeye
Copy link
Contributor

halkeye commented Jul 29, 2016

@BanzaiMan how will we know when it's in production to start testing?

@BanzaiMan
Copy link
Contributor

It's there now.

@halkeye
Copy link
Contributor

halkeye commented Jul 29, 2016

@BanzaiMan I just put in a support request, but could the abuse detected/gatekeeper stuff have any relation to this PR?

https://travis-ci.org/saucelabs-sample-test-frameworks/Java-TestNG-Selenium/requests

@BanzaiMan
Copy link
Contributor

BanzaiMan commented Jul 29, 2016

@halkeye That should be lifted now. It was a bug in unrelated code.

@halkeye
Copy link
Contributor

halkeye commented Jul 29, 2016

@BanzaiMan Okay, looking at this test case.
it looks like the jwt stuff is running after install.

Should the function be changed to before_before_install (is that a thing)?

https://travis-ci.org/saucelabs-sample-test-frameworks/Java-TestNG-Selenium/jobs/148427092

@BanzaiMan
Copy link
Contributor

Yes, you can use before_before_install if it is the appropriate place to run the code.

@BanzaiMan
Copy link
Contributor

@halkeye Could you open a new issue in https://github.com/travis-ci/travis-ci to track further improvements (except code changes, of course)?

@halkeye
Copy link
Contributor

halkeye commented Jul 29, 2016

I updated this branch, but I can totally create a new issue if you want

@halkeye
Copy link
Contributor

halkeye commented Jul 29, 2016

@BanzaiMan new PR or new issue?

@halkeye
Copy link
Contributor

halkeye commented Jul 29, 2016

should I create a new PR for the change?

@halkeye
Copy link
Contributor

halkeye commented Jul 29, 2016

It wasn't picking up the new code and retesting, so just opened a new PR

halkeye pushed a commit to sauce-archives/docs-travis-ci-com that referenced this pull request Jul 30, 2016
halkeye pushed a commit to sauce-archives/docs-travis-ci-com that referenced this pull request Aug 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.