broadinstitute/cromshell

Expose curl header for oauth

microbioticajon opened this issue · 7 comments

Hi Guys,

This is a feature request. We have fronted our Cromwell server with an oauth proxy that acts as a gate keeper to the cluster. In our case, authorisation is achieved by submitting a bearer token in the header of the request. Could I propose the following addition to cromshell?

Change this line from:
alias curl='curl --connect-timeout ${CURL_CONNECT_TIMEOUT} --max-time ${CURL_MAX_TIMEOUT}'

to something like:
alias curl='curl -H "${CROM_CURL_HEADER}" --connect-timeout ${CURL_CONNECT_TIMEOUT} --max-time ${CURL_MAX_TIMEOUT}'

CROM_CURL_HEADER can be set outside the script and is flexible to the users needs. This should be backward compatible but allow implementations incorporating oauth to function without having to make changes to the cromshell script.

Thoughts?

Best,
Jon

This seems like a very reasonable idea.

Great!

I have submitted a PR with the modification we are using: #175

I was having trouble injecting variables into the curl alias so I used an if block to add the header option - Bash is not really my language here.

If you find this useful please feel free to tweak/modify it.

Best,
Jon

I'll take a look sometime this week. I'm loathe to rely on environment variables - they make code unpredictable and difficult to debug. Configuration is the only place it can work, but it would be good to have a record of it somewhere.

@lbergelson @microbioticajon How would you feel about adding it to the cromshell.config file as an alternative?

Hi @jonn-smith

Thanks for the response. I appreciate where you are coming from, env variables do feel dirty.

My thinking is this. For our use case, oauth tokens expire after a relatively short period of time so would need to be updated frequently. Setting an env var outside the tool means I just need to source a small script that fetches and exports the oauth token to the environment variable and Cromshell is seamlessly happy.

Using a command line argument is not desirable as it puts the token in the shell history (even tho it does eventually expire).

I guess you could add an interactive app that lets the user set the header and in my case token, and store in config. This makes scripting a bit harder.

I also guess if I wanted to script this I could directly add the header string to the .cromshell/cromwell_server.config although at the moment this config file is pretty basic and might require a bit more structure (json perhaps?). The env variable approach can be implemented with a few lines of code and is backward compatible :-)

I dont mind really. My preference is something that is relatively simple for the user to refresh their token when it expires without having to execute multiple tools and copy paste values.

Thanks again,
Jon

@microbioticajon Good point. Yeah - this is one of the few situations where I could be convinced to use env vars.

What I was envisioning is a new command (cromshell auth or similar) which would prompt you and / or do the work in the background based on your setup (similar to doing export GCS_OAUTH_TOKEN=$(gcloud auth application-default print-access-token)).

It'll be trivial to go back and forth with it, so your PR is probably fine. @lbergelson or I can take a look at it at some point soon.

I believe this feature has now been implemented so will close this issue. Many thanks!

This is a wonderful feature, and as of just today, I kid you not, I am finding myself needing 2 headers.

I will submit either a PR or a new issue.