search

okta / okta-oidc-js

okta-oidc-js
https://github.com/okta/okta-oidc-js
Other
395 stars 232 forks source link

[jwt-verifier] Travis build not working in PRs from forked repos #501

Open heidemn opened 5 years ago

heidemn commented 5 years ago

I'm submitting this issue for the package(s):

I'm submitting a:

Current behavior

This nop PR fails the Travis build in the jwt-verifier module: https://github.com/okta/okta-oidc-js/pull/500 It is equal to the master branch, except whitespace.

See comment below - the reason is probably that builds in PRs from forked repos are configured differently.

(This prevents me to contribute an improvement to oidc-middleware: https://github.com/okta/okta-oidc-js/issues/497 )

Expected behavior

The nop PR should pass the Travis build. https://github.com/okta/okta-oidc-js/pull/500

Minimal reproduction of the problem with instructions

Extra information about the use case/user story you are trying to implement

Environment

heidemn commented 5 years ago

@aarongranick-okta can you maybe help out here? I'm trying to contribute in PR #498, but already my "nop" PR #500 fails on Travis.

Is it possible that the Travis build only works for branches in the Okta repo? The PR from my forked repo seems to be configured differently. The build in my PR only sets the ISSUER env var. The build in master sets 7 additional env vars (e.g. CLIENT_SECRET).

How can I make my PR build on Travis, and successfully contribute?

okta:master grafik

My PR: grafik

swiftone commented 5 years ago

Is it possible that the Travis build only works for branches in the Okta repo?

Yes - our CI runs automation tests that require an active okta organization - Travis is configured with one for our team accounts, but PRs from outside will not get access to these credentials.

We've been investigating options for this but have not yet come up with a good solution - generally the team can grab a PR from outside and add it as coming from inside (we'll tag the author in a comment so they are aware) and then the tests can run. It's not ideal, but it's what we have for now.

Thanks for the PR, BTW! Have you signed a CLA for us? https://developer.okta.com/cla/

heidemn commented 5 years ago

@swiftone thanks for the explanation. True, it would be tricky to allow contributors to specify their own Okta org, but at the same time not require them to make the credentials public. I also understand that you don't want to propagate bad practices, and check in secrets to Git.

I guess my PR is not an "Obvious Fix", so I'll check with my employer if I'm allowed to sign the CLA. I will also run the tests on my machine, and then update the PR.

heidemn commented 5 years ago

@swiftone our legal department requested some modifications to the CLA. We sent this request to CLA@okta.com, but never received an answer.

Do you have any suggestion how to proceed? (Our company is a paying Okta customer.)