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

OAuth breaks if a bad .netrc file is present #107

Closed
susodapop opened this issue May 12, 2023 · 0 comments · Fixed by #108
Closed

OAuth breaks if a bad .netrc file is present #107

susodapop opened this issue May 12, 2023 · 0 comments · Fixed by #108

Comments

@susodapop
Copy link
Contributor

susodapop commented May 12, 2023

Description

There is a hidden behaviour in requests where a .netrc will silently override provided authentication headers unless manually overridden.

We first observed this in dbt-databricks because of databricks/dbt-databricks#337. Fixing it however required two changes: one to dbt-databricks and one to databricks-sdk-py. The fix is simple: override the default behaviour of requests by supplying a custom AuthBase class.

  1. The fix to dbt-databricks has to do with Python models which use requests and a REST API (instead of thrift server) to check if the all-purpose cluster is running.
  2. The fix to databricks-sdk-py (this issue) is around OAuth, because the SDK uses requests to perform the OAuth handshake. This step is only needed where OAuth is used to authenticate dbt-databricks

Steps to Reproduce

There is probably an easier way to reproduce this with a short script. But I'm using what I found while developing dbt-databricks.

  1. Checkout this code from dbt-databricks Stop requestslib from overriding auth headers with data from .netrc file dbt-databricks#338. This branch incorporates the first fix I described above.

  2. Add an intentionally bad ~/.netrc to your workstation, like this:

machine <my-workspace>.cloud.databricks.com
login token
password <expired_token>
  1. Try to run the Python test_python_uc_sql_endpoint integration test after updating _build_databricks_cluster_target in tests/profiles.py to comment out the "token" key. This forces dbt-databricks to use OAuth instead.
  2. Observe that the test fails with this error:
E           ValueError: b'{"errorCode":"invalid_client","errorSummary":"Invalid value for \'client_id\' parameter.","errorLink":"invalid_client","errorId":"oaeLJQz1r35SrSNVtVcJUig0A","errorCauses":[]}'

This happens because without the override applied, the SDK includes authentication headers for a REST API request that doesn't require authentication and the server kicks back an invalid value for 'client_id' error.

I'm about to open a PR that fixes this.

There is a related issue on databricks-sql-python (which implements its own oauth process). That fix is the same as this one.

@nfx nfx closed this as completed in #108 May 17, 2023
nfx pushed a commit that referenced this issue May 17, 2023
…ke (#108)

## Changes
<!-- Summary of your changes that are easy to understand -->

This PR closes #107 by applying the fix described there. 

## Tests
<!-- 
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->

Tested manually while developing the same fix for dbt-databricks (see
linked issue).

- [ ] `make test` run locally
- [ ] `make fmt` applied
- [ ] relevant integration tests applied

---------

Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant