Skip to content

fix: resolve GCP service-account vs oauth credentials correctly (#3328)#4026

Open
Pawansingh3889 wants to merge 1 commit into
dlt-hub:develfrom
Pawansingh3889:fix/3328-gcp-credentials-type-and-path
Open

fix: resolve GCP service-account vs oauth credentials correctly (#3328)#4026
Pawansingh3889 wants to merge 1 commit into
dlt-hub:develfrom
Pawansingh3889:fix/3328-gcp-credentials-type-and-path

Conversation

@Pawansingh3889

Copy link
Copy Markdown
Contributor

Closes #3328.

What's going on

GCS / filesystem credentials are typed as Union[GcpServiceAccountCredentials, GcpOAuthCredentials]. When dlt resolves a union it only falls through to the next member if a spec raises NativeValueError. The problem: neither GCP spec checked that the JSON actually matched its own type.

  • GcpOAuthCredentials.parse_native_representation happily json.loads-es a service-account JSON (a service-account file even has a client_id), so resolution can land on oauth.
  • When things do fail, the union re-raises the last member's error, which is why people see GcpOAuthCredentials cannot parse the configuration value provided... even though they supplied a service account. That's both the error in dlt tries OAuth in GCP Instead of Service Account Credentials when given Service Account Credentials via credential file #3328 and the "redirects to the OAuth consent screen instead of using the service account" symptom from the google_sheets source.

Changes

  • GcpOAuthCredentials rejects a JSON that looks like a service account (type == "service_account" or a private_key present) by raising InvalidGoogleOauth2Json.
  • GcpServiceAccountCredentials rejects oauth client-secret / user info (client_secret / installed / web / type == "authorized_user") by raising InvalidGoogleServicesJson.
  • Both are NativeValueError subclasses, so a credentials union now falls through to the correct spec instead of mis-parsing.
  • credentials may now also be a path to a json file — if the value is an existing file it's read from disk, so credentials = "/path/to/service_account.json" works (the literal case in the issue).

Tests

Added to tests/common/configuration/test_credentials.py:

  • each spec rejects the other's JSON,
  • a Union[...] resolves a service-account JSON to the service-account spec and oauth info to the oauth spec,
  • credentials are read from a file path.

The GCP/oauth credential tests pass locally (9 passed, including the existing ones), black and flake8 are clean, and no existing credential test changed behaviour.

…hub#3328)

The GCS/filesystem credentials are a Union[GcpServiceAccountCredentials,
GcpOAuthCredentials], but neither spec checked that the supplied JSON matched
its own type. A service-account JSON was therefore happily ingested by the
oauth spec, so resolution could pick oauth and the union surfaced the oauth
parse error ("GcpOAuthCredentials cannot parse...") instead of using the
service account.

Each spec now rejects the other's JSON (raising the existing NativeValueError
subclasses) so a credentials union falls through to the correct spec. Also
read the credentials from disk when the value is a path to a json file.
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 this pull request may close these issues.

dlt tries OAuth in GCP Instead of Service Account Credentials when given Service Account Credentials via credential file

1 participant