broadinstitute / cromwell-tools

A collection of Python clients and accessory scripts for interacting with the Cromwell
https://cromwell-tools.readthedocs.io
BSD 3-Clause "New" or "Revised" License
22 stars 9 forks source link

Make authentication optional? #35

Closed CarlosBorroto closed 5 years ago

CarlosBorroto commented 6 years ago

Hi, would it be possible to make authentication optional? We run cromwell in a walled environment and would prefer not to deal with authentication. Currently if no form of authentication is passed to start_workflow this exception is raised from `harmonize_credentials:

 File "envs/pipeflow/src/cromwell-tools/cromwell_tools/cromwell_tools.py", line 184, in start_workflow
    auth, headers = _get_auth_credentials(cromwell_user=user, cromwell_password=password, caas_key=caas_key)
  File "envs/pipeflow/src/cromwell-tools/cromwell_tools/cromwell_tools.py", line 59, in _get_auth_credentials
    cromwell_user, cromwell_password = harmonize_credentials(secrets_file, cromwell_user, cromwell_password)
  File "envs/pipeflow/src/cromwell-tools/cromwell_tools/cromwell_tools.py", line 43, in harmonize_credentials
    raise ValueError('One form of cromwell authentication must be provided, please pass '
ValueError: One form of cromwell authentication must be provided, please pass either cromwell_user and cromwell_password or a secrets_file.
ambrosejcarr commented 6 years ago

Hi Carlos,

This seems like a reasonable request to me. We've never run cromwell without Auth on my team, so it'll take a bit of sleuthing to figure out all the corner cases of doing this, but I think we should be able to make it work.

Auth in general is in a messy state on master. We have a big refactor slated to fix a lot of it (see #31). I think we can add this feature to that PR when we pull this work in.

CarlosBorroto commented 6 years ago

@ambrosejcarr great, thanks for considering the request. I'll work on getting a proxy in front of our cromwell instance for now. Looking at all the work in #31 it seems wise to wait for that PR.

rexwangcc commented 6 years ago

We merged in the refactor work with https://github.com/broadinstitute/cromwell-tools/pull/43 without enabling No-Auth option. Since the Auth is standardized and more modular now, we will add this feature (which is reasonable to me) as a follow-up PR shortly.