Add private_key auth method to snowflake query runner

[adrianoesch]

What type of PR is this?

• [ ] Refactor
• [x] Feature
• [ ] Bug Fix
• [ ] New Query Runner (Data Source)
• [ ] New Alert Destination
• [ ] Other

Description

Snowflake will soon require private key authentication for service users without multi factor authentication (see announcement). This PR makes snowflake configurable with a private_key.

How is this tested?

I would need guidance on how you want this tested.

• [ ] Unit tests (pytest, jest)
• [ ] E2E Tests (Cypress)
• [x] Manually

Related Tickets & Documents

I could only find this discussion.

[adrianoesch]

i've tested it locally, let me know what kind of tests you want for this.

[nijave]

Thanks!! Just updated our instance to this branch and it's working perfectly

[sh47267]

@adrianoesch Does it support passphrase-protected private keys?

[adrianoesch]

hi @sh47267

Does it support passphrase-protected private keys?

it doesn't yet, because i don't understand the motivation behind storing an encrypted string with the password in the same place. but it could easily be added to load_pem_private_key (see here)

[sh47267]

@adrianoesch

You're right — I realize now that without separating the passphrase from the key file, it doesn't really provide any additional security. There might still be demand for it, but thanks for the clarification for now.

[uofifox]

Any updates here? We're using redash in my org and trying to get ahead of the snowflake requirements/mandates. We're on an older version of redash anyway and we're holding until a version of redash has this fix on it.

[adrianoesch]

hi @uofifox , we need the attention of one of the maintainers to merge this into the main branch. as this is an open-source project, one can expect this to take some time. in the meantime, @nijave seems to have had success deploying this branch in his org. also, snowflake allows you to continue working with passwords for user with type LEGACY_SERVICE until nov 2025 (see here).

[uofifox]

Thanks for the update, and while the delay in enforcement is not until November, these kinds of things have a way of slipping your mind until its too late. Also note, I noticed you don't have the ability to support encrypted private keys. Most of the other platforms I used actually won't accept the private keys unless it is encrypted. Agree its a bit overkill, but to keep it standard for our group we've just by default encrypted the private key.

[adrianoesch]

any chance you can unblock us here @arikfr ?

[yoshiokatsuneo]

@adrianoesch

Thank you very much for your contribution! I'm a new maintainer. Personally, I don't use Snowflake, but want to check this PR because authentication issue looks affecting people using Redash with Snowflake.

• I just think we may not need _clean_newlines, and need not add "BEGIN PRIVATE KEY" on _get_private_key. We can simply load provided private key with "BEGIN PRIVATE KEY" so that we can keep code base simple.

• There are two snowflake.connector.connect lines and I think that is redundant. I think we can make only one function call as before.

Thank you!

[arikfr]

@yoshiokatsuneo I think they add the headers in case they are missing .. as you can't be sure what type of private key the user will upload.

[yoshiokatsuneo]

@yoshiokatsuneo I think they add the headers in case they are missing .. as you can't be sure what type of private key the user will upload.

@arikfr

Thank you ! And, I see ! ( I just thought we normally copy whole pem file like ssh key. But I'm just not sure for this use case. )

[adrianoesch]

hi @yoshiokatsuneo & @arikfr, and thanks for taking this up!

• copy-pasting strings with newlines can be messy in my experience. we're storing credentials in a secret manager and retrieve it via browser so cleaning it here seemed to be useful to me
• added a params object to remove duplicate snowflake.connector.connect calls
• added the optional private_key_pwd property as requested by some above

[yoshiokatsuneo]

@adrianoesch @arikfr

Thank you !

I noticed that the code assume private key header is "-----BEGIN PRIVATE KEY-----", but it is not always true.

• The private key without encryption have header: "-----BEGIN PRIVATE KEY-----".
• The private key with encryption have header: "-----BEGIN ENCRYPTED PRIVATE KEY-----".

Also, private key generated by some old tool looks have header "-----BEGIN RSA PRIVATE KEY-----". And future version or other variants may have different header.

Again, I just feel that handling those all cases make code base complex compare to the benefit, as the setting private key in only once.

Thank you !

[yoshiokatsuneo]

@adrianoesch

Also, I think the private key input field is input field that accept only one line while the private key have multiple lines. And, it is better if the secret key is not visible.

I suggest to load private key from file as BigQuery dataset's JSON Key file ("jsonKeyFile").

Thank you !

[yoshiokatsuneo]

@adrianoesch

we're storing credentials in a secret manager

Just for your information but I think here is a sample implementation that load private key from AWS secret manager. ( I'm not saying that this PR should handle the case. Just as a information.)

https://zenn.dev/dataheroes/articles/20250411-redash-snowflake-keypair-authn ( It is Japanese.... But, maybe you can use google translate ...)

Thank you !

[adrianoesch]

@yoshiokatsuneo

i wasnt aware of the different rsa key headers, that does indeed complicate things.

I suggest to load private key from file as BigQuery dataset's JSON Key file

you mean as base64 encoded string like here? but then i'd suggest renaming the property to private_key_b64 to make it explicit what we expect.

thanks for the hint towards the integration with the aws secret manager. this is neat for secret rotation i guess, but then you one would also have to take care of authentication towards aws. even though in an aws deployment that's rather easy, but let's leave this out of scope for now.

[adrianoesch]

@yoshiokatsuneo

is better if the secret key is not visible.

i would think that the secret array in the configuration (see here) would take care of that. is that not the case?

[yoshiokatsuneo]

@adrianoesch

is better if the secret key is not visible.

i would think that the secret array in the configuration (see here) would take care of that. is that not the case?

I think we also need to set type to "password" instead of "string" to set input field type to password like below.

                "private_key_pwd": {"type": "password"},

[adrianoesch]

@yoshiokatsuneo

i've added base64 encoded private key handling to simplify code.

unfortunately {"type":"password"} doesn't work out of the box. i think there's some type mapping going on between backend database type and frontend field type here but i'm hesitant to touch frontend code for now. also i don't see type password being used in any other query runner (see query).

this is the error on create datasource with type password:

jsonschema.exceptions.SchemaError: 'password' is not valid under any of the given schemas Failed validating 'anyOf' in metaschema['properties']['properties']['additionalProperties']['properties']['type']: {'anyOf': [{'$ref': '#/definitions/simpleTypes'}, {'items': {'$ref': '#/definitions/simpleTypes'}, 'minItems': 1, 'type': 'array', 'uniqueItems': True}]} On schema['properties']['private_key_pwd']['type']: 'password'

adding the attribute to the secret array in the runner config will replace the string with dashs on subsequent config loads which seems good enough to me:

[yoshiokatsuneo]

@adrianoesch

unfortunately {"type":"password"} doesn't work out of the box.

Ah, you are right !

I just noticed that the properties in which key have suffix "password" or "passwd" are handled as password type, and properties in which key have suffix "File" is handled as file type. This is implemented at normalizeSchema() function below

https://github.com/getredash/redash/blob/5ae1f70d9ec76d27332921ab8093e36f4f63ab6c/client/app/components/dynamic-form/dynamicFormHelper.js#L32

So, how about changing property key name following the convention like below ?

                "private_key_File": {"type": "string"},
                "private_key_password": {"type": "string"},

[adrianoesch]

@yoshiokatsuneo

we don't store private keys on disk and other services like dbt also use base64 encoded keys for snowflake auth (see here). if you want, i can add the file option additionally, but i don't think it's necessary.

"private_key_password": {"type": "string"},

this would not change the field type in normalizeSchema as it checks the fullstrings, not just the suffix:

https://github.com/getredash/redash/blob/5ae1f70d9ec76d27332921ab8093e36f4f63ab6c/client/app/components/dynamic-form/dynamicFormHelper.js#L34

[yoshiokatsuneo]

@adrianoesch

this would not change the field type in normalizeSchema as it checks the fullstrings, not just the suffix:

Ah I see, you are right again !

[yoshiokatsuneo]

@adrianoesch

we don't store private keys on disk and other services like dbt also use base64 encoded keys for snowflake auth (see here). if you want, i can add the file option additionally, but i don't think it's necessary.

I think this code asks users to input base64-encode PEM while dbt asks users to input PEM(or base64-DER). So, I think it is better to load PEM(not base64-encoded PEM) by loading file, to avoid to ask users to encode base64, again.

( It looks following dbt-snowflake code implementing authentication may be one reference.) https://github.com/dbt-labs/dbt-snowflake/blob/main/dbt/adapters/snowflake/auth.py#L18

Thank you.

[adrianoesch]

@yoshiokatsuneo added the file option

[yoshiokatsuneo]

@adrianoesch

Thank you !

And, when I tried to connect Snowflake using private key with password, I got error below. Do you have any idea ?

Connection Test Failed:
argument 'password': from_buffer() cannot return the address of a unicode object

[adrianoesch]

@yoshiokatsuneo load_pem_* function expects bytes-like object or None (see docs). fixed it!

[yoshiokatsuneo]

@adrianoesch

Thank you ! The PR looks good ! I tested and confirmed that it works.

[yoshiokatsuneo]

@arikfr

The PR looks good, I tested and and it looks working well. I think we can merge the PR.

Just to make sure, do you have any thought or comment ?

There is some discussion how to load the PEM file whether string or file. Current code(loading from file) looks good, but do you have any idea ?