sfc-gh-klochen / sfsnowsightextensions

Snowflake Snowsight Extensions enables manipulation of Snowsight features from command-line
6 stars 1 forks source link

Update login method as Snowflake changed the OAuth response #46

Closed joshuataylor closed 8 months ago

joshuataylor commented 9 months ago
  1. Allow redirects from GET requests
  2. Get cookies from all responses
  3. Workaround for the issue that the API now returns the client_id in the URL, and the apiGET function doesn't return the URL. So we need to return the final redirected URL as the result for start-oauth/snowflake.

Tested username+password and SSO.

Fixes #45

sfc-gh-mybarra commented 9 months ago

@lziegler @joshuataylor I can accept this pull request, but I need to ask you both to acknowledge the Snowflake Labs CLA. Please review the documentation at the following link and add your acceptance in a comment. An acceptable response would be _I accept Snowflake Open Source Contribution terms and conditions.

https://github.com/snowflakedb/CLA

joshuataylor commented 9 months ago

I, Josh Taylor, accept Snowflake Open Source Contribution terms and conditions, signed Sunday, 21st January 2024 (2024-01-21).

lziegler commented 9 months ago

I accept Snowflake Open Source Contribution terms and conditions.

danielodievich commented 9 months ago

I have access to 3 snowflake accounts, 2 behind MFA, and one just username/password.

~I seem to be getting authentication issues on the SnowflakeDriver.GetMasterTokenAndSessionTokenFromCredentials with the MFA protected one with the call just timing out.~ ok scratch that I just didn't see the MFA popup, once I acknowledged it worked just great

The username/password one works fine, although I had to use the actual account name and not the account locator before it let me proceed.

Thank you for fixing this! Much appreciated.

sfc-gh-mybarra commented 9 months ago

@joshuataylor I compiled your code and it doesn't work for me. Screenshot 2024-01-23 at 2 16 20 PM

danielodievich commented 9 months ago

Try hitting ora74440 instead of spfcogs-mybarra_aws_db, I'd be curious if it works.

danielodievich commented 9 months ago

And are you SURE you are running the code compiled from changes? Your path those 2023.2.8.0 version with binaries?

sfc-gh-mybarra commented 9 months ago

@danielodievich I initially used the account locator and received the same error. I cloned Joshua's repo and compiled the code without updating the version in the .csproj file. That is why 2023.2.8.0 is showing. Screenshot 2024-01-23 at 3 46 27 PM

sfc-gh-mybarra commented 9 months ago

I tried authenticating to another account in Azure and received the same error.

joshuataylor commented 9 months ago

I'll sign up for some trial accounts for Azure and Google Cloud, as it seems there might be a different cookie or something needed (they might not have rolled out the feature across all clouds perhaps yet?).

sfc-gh-mybarra commented 9 months ago

The screenshots that I shared were for an AWS account in the us west 2 region. That is the first region that Snowflake deployed their first accounts. I should have been able to authenticate. I'll keep trying.

joshuataylor commented 9 months ago

Could you email me your sanitised logs, the app does verbose logging (doesn't store passwords, but does store cookies, so remove those etc). joshuataylorx at gmail dot com

danielodievich commented 9 months ago

May I suggest that @sfc-gh-mybarra give @joshuataylor an account in his spfcogs-mybarra_aws_db tenant. I recognize is as a a personal test tenant and it should be possible to get a PUBLIC role that can do nothing but login and create worksheets.

sfc-gh-mybarra commented 9 months ago

I upgraded PowerShell to the latest version and ran the ZipReleases module. I'm am still receiving the same error with the newly generated binaries. I'm not sure what granting access to my account would accomplish. If all users can't compile and run, I can't approve the pull request yet.

danielodievich commented 9 months ago

I think if Joshua had access to your account, he can check if his logic works in your account and debug it from there.

sfc-gh-mybarra commented 9 months ago

That could be a solution, but how would my Enterprise Edition account with username/password differ from any other Enterprise Edition account with the same configuration? Debugging my account seems more like a bandaid measure. Additionally, would this be the solution to debug issues in other accounts?

joshuataylor commented 9 months ago

I think we need to take a step back and answer "When something goes wrong, how do we figure out what is going on?".

E.g:

  1. A user cannot authenticate (most common problem) using an authentication method (Username & Password, SSO, Private Key, MFA, etc).
  2. A user can authenticate, but gets an unexpected error on one the internal endpoints (worksheets, etc).

We should focus on #1 first, as this will be the most common issue - either due to user error, or Snowflake changing something on their end, adding a new authentication method etc. Authentication and other processes rarely change, especially at SF, but given this is more of an internal endpoint there will be changes.

When Snowflake changes something (rare)

If there is a change that Snowflake has made that breaks authentications we'll all know pretty quickly :-). I have recently setup a scheduled job which logs in using Username+Password and Private Key Auth twice a day to ensure everything is still valid for these endpoints.

Login issues on the client side

I can't login will be such a common and such an ambigious issue, and currently requires a lot of back<>forth to figure out.

I'm going to bet that most of the time it comes down to a user typing in their credentials wrong, HTTP Proxy issues (I bet Snowflake tends to be used a lot on enterprise/work computers :-)) and other transient issues that are hard to track down. We can become better at telling the user about this when it occurs, instead of a generic womp womp something went wrong message (which is fine for now btw).

Login Error Codes

Snowflake has done some great work to improve issues around the login process by adding better error codes for Private Keys, and then I think also MFA, OAuth and SSO. So if it is actually a user issue, then we should be able to give them a (hopefully relevant!) error code.

Diagnosing Issues

What would be good is to have a way for a user to share detailed enough diagnosis information in a format that is both easily understandable by a non-technical user but also be able to figure out what is going on.

This file should be terse enough so the user can understand that they are not going to be sharing any secrets (passwords, cookies, etc), company information, endpoint info, etc.

This way we can tell if there are multiple I can't login issues if they are the same thing or something different.

If the user can't login, we could print the line along the lines of To diagnose this issue, see file logs/login-log-YYYMMDD.log and create an issue <here> to the user.

Short term solution

Let's come up with a short term solution, with the intention of having it another draft PR which can be used to help those who currently can't login with this PR's change.

This is a quick idea dump (least amount of effort, with the intention to not merge it and come up with something better later):

Scenario 1: Someone who just wants to share a quick log:

  1. Have a new logger called LoginDebug (for lack of a better name)
  2. Log to a file with as little information we can for each step in the authentication process. Sanitise account information, login credentials etc. We just want to see what requests were sent (sanitised URL + body of the request) and what the response URL+relevant headers+sanitised body was. Each endpoint would have custom logging to only document as little as possible about the response (eg the code, if something was returned).
  3. User provides this file output in the issue.

Scenario 2: More technical user who wants to provide a debug log

Sometimes we're going to need more information, so having a way to get the sanitised request and response URL/Headers/Body will be good. More on this to come.

joshuataylor commented 9 months ago

Just to be clear, I'll create a follow up PR in the next day or so, it's a long weekend here and I finally have a chance to finish this.

sfc-gh-mybarra commented 9 months ago

That sounds great @joshuataylor. I appreciate you stepping in to help outside of your regular day to day. I'll look for the upcoming PR.

joshuataylor commented 9 months ago

Okay, so it looks like with release 8.4 there were some major changes made around authentication, which explains why we saw the authentication workflow break, and perhaps work for some but not others during the rollout.

I'm going to guess that those who couldn't log in were either:

a/ Still on the old version b/ On the new version

I know Snowflake does gradual rollouts, and some users can also request to be on the new version etc. This should hopefully be a change that isn't made often, but should be something we can hopefully adapt to if needed.

I'm seeing a new authentication flow for SSO now on both my AWS US instance (8.4.1, us-east-1) and my AWS Sydney (ap-southeast-2) instances. Could I get those that couldn't login to confirm their version & region?

select concat('Snowflake Version: ', coalesce(current_version(), '-'), ' | Region: ', coalesce(current_region(), '-'));

I'll get the code updated for this change, username/password seems to work fine still.

I also have a new branch I'll push out with what I'm calling Diagnostic Test, which should help diagnose these sorts of issues.

The log file output looks like this:

This file contains a Diagnostic Test, with sanitised information for API Endpoints accessed by SFSnowsightExtensions.
This should not contain any sensitive information about your account, but please double check before posting.
Please also include the output of the following SQL command, which will output the Snowflake version and Region, as this allows us to see if you're running on a newer or older version of Snowflake, and the region.
This should output a string similar to "Snowflake Version: 8.4.1 | Region: AWS_US_EAST_1".

"""
select concat('Snowflake Version: ', coalesce(current_version(), '-'), ' | Region: ', coalesce(current_region(), '-'));
"""

SFSnowsightExtensions Diagnostic Test - v2024.1.0.0 - Date: 2024-02-05
Environment: Darwin 23.3.0 Darwin Kernel Version 23.3.0: Wed Dec 20 21:31:00 PST 2023; root:xnu-10002.81.5~7/RELEASE_ARM64_T6020 - Arm64 | Dotnet Version: .NET 8.0.1 | PowerShell Version: 7.4.1 | HTTP_PROXY set: True | HTTPS_PROXY set: True
Passed Parameters: Account: True | UserName: True | Password: True | Credential: False | SSO: False | MainAppURL: default (https://app.snowflake.com)
--------------------
GET /v0/validate-snowflake-url
Response: 200 (OK)
Response Body: valid: VALID | account: EXISTS | appServerUrl: EXISTS | region: EXISTS | url: EXISTS
Request Cookies: N/A
Response Cookies: snowflake_deployment: EXISTS
--------------------
GET /start-oauth/snowflake
Response: 200 (OK)
Redirected to oauth/authorize: True
Request Cookies: N/A
Response Cookies: S8_SESSION: EXISTS | oauth-nonce-XXX: EXISTS
--------------------
POST /session/v1/login-request
Response: 200 (OK)
Response Body: success: TRUE | code: N/A | serverVersion: EXISTS | masterToken: EXISTS | token: EXISTS | sessionId: EXISTS | displayUserName: EXISTS | schemaName: EXISTS | warehouseName: EXISTS | roleName: EXISTS | databaseName: EXISTS
Request Cookies: N/A
Response Cookies: N/A
--------------------
POST /session/authenticate-request
Response: 200 (OK)
Response Body: success: TRUE | code: N/A | masterToken: EXISTS
Request Cookies: N/A
Response Cookies: N/A
--------------------
POST /oauth/authorization-request
Response: 200 (OK)
Response Body: success: FALSE | code: EXISTS | redirectUrl: EXISTS | nextAction: EXISTS | inFlightCtx: EXISTS
Request Cookies: N/A
Response Cookies: N/A
--------------------
GET /complete-oauth/snowflake
Response: 200 (OK)
Response Body: text/html, params var: True
Request Cookies: oauth-nonce-XXX: EXISTS
Response Cookies: user-XXX: EXISTS | oauth-nonce-XXX: EXISTS
--------------------
GET /bootstrap
Response: 200 (OK)
Response Body: BuildVersion: EXISTS | User.id: EXISTS
Request Cookies: user-XXX: EXISTS
Response Cookies: csrf-XXX: EXISTS | user-XXX: EXISTS
--------------------
POST /v0/organizations/XXX/entities/list
Response: 200 (OK)
Response Body: defaultOrgId: EXISTS
Request Cookies: user-XXX: EXISTS
Response Cookies: N/A

There is also an "Extensive Diagnostic Test" which includes Unknown Cookie Values from the request/response, as well as additional information from the response such as additional keys.

joshuataylor commented 9 months ago

Super annoying, I had to readd my security integration to get SSO to work with both this and the Snowflake Connector Python. I do not have multiple security integrations.

If you have the same issue, and can reproduce it with Snowflake Connector Python, submit a support ticket. :shrug:

Now 8.4 has been out for a week, I believe all accounts should be on the newest release and everything should hopefully work now. Double check with select concat('Snowflake Version: ', coalesce(current_version(), '-'), ' | Region: ', coalesce(current_region(), '-'));.

If you still have issues, please try this PR and post the DiagnosticTest log file, as it will help figure out what's going on.

sfc-gh-sredding commented 8 months ago

I was testing your fix branch and I did get errors when trying to use the "regionless" url format ( org_name-account_name ) as the -Account . It did look like it was able to figure out the URL when using this format but I kept getting

ConnectAppCommand threw Unable to authenticate user sredding@sfpscogs-redding_p because of Bad request; operation not supported. (390400) (SnowflakePS)

when I switched to just using the account locator URL format it worked just fine. I was also able to use SSO login to an account as well.

joshuataylor commented 8 months ago

Interesting!

Can you login normally using that format on app.snowflake.com? Is this a new login URL format?

Does it pass valid on https://app.snowflake.com/v0/validate-snowflake-url?url=XXX&isSecondaryAccount=false?

edit:

Ah https://docs.snowflake.com/en/user-guide/organizations-connect explains it!

The legacy account locator format is currently supported, but its use is discouraged.

So testing for this would be good. Looks like clicking "copy" here adds a . instead of a -, that's slightly confusing!

SCR-20240212-topn-2

IMHO, using the values from here is what you would want to do in future.

And this would be interesting to test with Snowsight

joshuataylor commented 8 months ago

This might also be related to https://stackoverflow.com/questions/60177963/how-can-you-connect-snowflake-to-an-ide-using-okta-with-mfa , could I ask what SSO provider you're testing with, so far I've been testing with Auth0 and it's been pretty flawless, but this is a pretty simple implementation and others might have more complex setups.

I tested using - for AWS & Azure, and they both seemed to work. The url and other information is gathered from the validate-url endpoint.

SFSnowsightExtensions Diagnostic Test - v2024.1.0.0 - Date: 2024-02-13
Environment: Darwin 23.3.0 Darwin Kernel Version 23.3.0: Wed Dec 20 21:31:00 PST 2023; root:xnu-10002.81.5~7/RELEASE_ARM64_T6020 - Arm64 | Dotnet Version: .NET 8.0.1 | PowerShell Version: 7.4.1 | HTTP_PROXY set: True | HTTPS_PROXY set: True
Passed Parameters: Account: True | UserName: True | Password: False | Credential: False | SSO: True | MainAppURL: default (https://app.snowflake.com)
--------------------
GET /v0/validate-snowflake-url
Response: 200 (OK) in 2705ms
Response Body: valid: VALID | account: EXISTS | appServerUrl: EXISTS | region: EXISTS | url: EXISTS
Request Cookies: N/A
Response Cookies: snowflake_deployment: EXISTS
--------------------
GET /start-oauth/snowflake
Response: 200 (OK) in 657ms
Redirected to oauth/authorize: True
Request Cookies: N/A
Response Cookies: S8_SESSION: EXISTS | oauth-nonce-XXX: EXISTS
--------------------
POST /session/authenticator-request
Response: 200 (OK) in 232ms
Response Body: success: TRUE | code: N/A | message: N/A | tokenUrl: N/A | ssoUrl: EXISTS | proofKey: EXISTS
Request Cookies: N/A
Response Cookies: N/A
--------------------
POST /session/v1/login-request
Response: 200 (OK) in 413ms
Response Body: success: TRUE | code: N/A | serverVersion: EXISTS | masterToken: EXISTS | token: EXISTS | sessionId: EXISTS | displayUserName: EXISTS | schemaName: N/A | warehouseName: EXISTS | roleName: EXISTS | databaseName: N/A
Request Cookies: N/A
Response Cookies: N/A
--------------------
POST /oauth/authorization-request
Response: 200 (OK) in 248ms
Response Body: success: FALSE | code: EXISTS | redirectUrl: EXISTS | nextAction: EXISTS | inFlightCtx: EXISTS
Request Cookies: N/A
Response Cookies: N/A
--------------------
GET /complete-oauth/snowflake
Response: 200 (OK) in 906ms
Response Body: text/html, params var: True
Request Cookies: oauth-nonce-XXX: EXISTS
Response Cookies: user-XXX: EXISTS | oauth-nonce-XXX: EXISTS
--------------------
GET /bootstrap
Response: 200 (OK) in 311ms
Response Body: BuildVersion: EXISTS | User.id: EXISTS
Request Cookies: user-XXX: EXISTS
Response Cookies: csrf-XXX: EXISTS | user-XXX: EXISTS
--------------------
POST /v0/organizations/XXX/entities/list
Response: 200 (OK) in 253ms
Response Body: defaultOrgId: EXISTS
Request Cookies: user-XXX: EXISTS
Response Cookies: N/A
sfc-gh-sredding commented 8 months ago

The SSO I used is OKTA .

sfc-gh-mybarra commented 8 months ago

Ok @joshuataylor. @sfc-gh-sredding helped me get past the issues that I was having, and will merge your pull request. Thank you for your hard work.