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.

Sign up for GitHub

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

Jump to bottom

SNOW-1526628 implemented passcode MFA authentication #739

Merged
merged 12 commits into from
Sep 12, 2024
Merged

SNOW-1526628 implemented passcode MFA authentication #739

merged 12 commits into from
Sep 12, 2024

Conversation

sfc-gh-ext-simba-jy
Copy link
Collaborator

@sfc-gh-ext-simba-jy sfc-gh-ext-simba-jy commented Aug 20, 2024 •
edited
Loading

  • Implemented the authentication with MFA (DUO application)
  • With passcode and passcodeInPassword, user can connect to the snowflake server with the passcode instead of getting "push" notification.

@sfc-gh-ext-simba-hx
Copy link
Collaborator

@sfc-gh-ext-simba-jy
Have this passed test on your local?
For the test failure of test_error_handlings please check my temporary change here:
https://github.com/snowflakedb/libsnowflakeclient/pull/738/files#diff-dc418a059566c5b2f962176f3f3c8efa5c150d2daa12a128d60111836b1e448d
to run test before we have server change to enable MFA authentication for libsfc, please check my temporary change here:

I don't think we really need the whole Authenticator thing if you check jdbc/odbc implementation they simply add EXT_AUTHN_DUO_METHOD and PASSCODE in login request when passcode or passcodeInPassword specified in connection parameters, regardless what authenticator type specified.

sfc-gh-ext-simba-hx
sfc-gh-ext-simba-hx approved these changes Aug 27, 2024
View reviewed changes
Copy link
Collaborator

@sfc-gh-ext-simba-hx sfc-gh-ext-simba-hx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Interesting, we have connection parameters of SF_CON_PASSCODE and SF_CON_PASSCODE_IN_PASSWORD since 7 years ago and never been implemented till now.

@sfc-gh-ext-simba-jy sfc-gh-ext-simba-jy marked this pull request as ready for review August 27, 2024 00:55
@sfc-gh-ext-simba-jy sfc-gh-ext-simba-jy requested a review from a team as a code owner August 27, 2024 00:55
sfc-gh-dstempniak
sfc-gh-dstempniak approved these changes Sep 12, 2024
View reviewed changes
Copy link
Contributor

@sfc-gh-dstempniak sfc-gh-dstempniak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Please add description to the PR

@sfc-gh-ext-simba-jy sfc-gh-ext-simba-jy merged commit f907edb into master Sep 12, 2024
43 checks passed
@sfc-gh-ext-simba-jy sfc-gh-ext-simba-jy deleted the SNOW-1526628 branch September 12, 2024 16:19
@github-actions github-actions bot locked and limited conversation to collaborators Sep 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sfc-gh-ext-simba-hx sfc-gh-ext-simba-hx sfc-gh-ext-simba-hx approved these changes

@sfc-gh-dstempniak sfc-gh-dstempniak sfc-gh-dstempniak approved these changes

@sfc-gh-pbulawa sfc-gh-pbulawa Awaiting requested review from sfc-gh-pbulawa

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sfc-gh-ext-simba-jy @sfc-gh-ext-simba-hx @sfc-gh-dstempniak