Skip to content
This repository has been archived by the owner on Sep 23, 2024. It is now read-only.

Add support for browser SSO authentication #427

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dlouseiro
Copy link

Problem

The current implementation of the target only allows for Snowflake basic authentication, which is probably the most common way for Snowflake system users/service accounts to authenticate in a prod-like scenario.

Although, for development purposes, it is common for the authentication of personal Snowflake users to be done via the company's SSO (e.g. Google), using an external browser to follow the regular SSO auth flow.

It would be ideal for the target to support authentication using an external browser.

Proposed changes

In this PR I'm adding another authentication method, using SSO browser authentication, by:

  • Adding a new config property (use_browser_authentication) that, when provided and set to True triggers a browser based authentication instead of basic authentication flow, achieved by passing authenticator=externalbrowser as documented here.

While there, I needed to:

  • Add secure-local-storage extra to snowflake-connector-python to ensure SSO tokens can be cached (documented here)
  • Updated snowflake-connector-python to latest version
  • Make password conditionally optional (only mandatory if use_browser_authentication is not True)
  • Add tests for config validation
  • Update documentation accordingly
  • Bump version (non breaking change - new feature)
  • Updated changelog

Types of changes

What types of changes does your code introduce to PipelineWise?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • [ x ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ x ] Documentation Update (if none of the other choices apply)

Checklist

  • [ x ] Description above provides context of the change
  • [ x ] I have added tests that prove my fix is effective or that my feature works
  • [ x ] Unit tests for changes (not needed for documentation changes)
  • CI checks pass with my changes
  • [ x ] Bumping version in setup.py is an individual PR and not mixed with feature or bugfix PRs
  • Commit message/PR title starts with [AP-NNNN] (if applicable. AP-NNNN = JIRA ID)
  • Branch name starts with AP-NNN (if applicable. AP-NNN = JIRA ID)
  • Commits follow "How to write a good git commit message"
  • [ x ] Relevant documentation is updated including usage instructions

@dlouseiro
Copy link
Author

PR to solve this issue

@dlouseiro
Copy link
Author

FYI this is my first PR in this repo, so I may be missing something. Would appreciate some guidance!

Replace usage of invalid `password` authenticator by proper `snowflake`.
@dlouseiro
Copy link
Author

@Samira-El @koszti @louis-pie tagging you as this is my first PR in one of your repos and you were listed as contributors here. The docs here mention the guidelines for adding new taps and targets, but not updating current ones, so I'm not sure who can better help me in understanding what is the best process to be followed for PRs to get some traction! Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant