-
Notifications
You must be signed in to change notification settings - Fork 481
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.
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
SNOW-1825621 OAuth code flow support #2135
Open
sfc-gh-mkeller
wants to merge
18
commits into
main
Choose a base branch
from
mkeller/SNOW-1825621/oauth-code-flow-support
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+540
−9
Open
Changes from 12 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
40033cf
adding new AuthHttpServer class
sfc-gh-mkeller 3b07b8e
unrelated simplifications
sfc-gh-mkeller 385f763
adding new oauth code flow implementation
sfc-gh-mkeller ac7cded
adding new TODO
sfc-gh-mkeller 2a4456c
adding changelog entry
sfc-gh-mkeller ee45f2d
making oauth state random
sfc-gh-mkeller f39e952
make client_secret optional
sfc-gh-mkeller 8fde1fc
make state check failure fail auth
sfc-gh-mkeller 6d025e5
switch to cryptographically-secure source
sfc-gh-mkeller 54e7833
redo connection parameters based on feedback
sfc-gh-mkeller d8edbf7
some string updates
sfc-gh-mkeller 5275879
add Authorization header to token request
sfc-gh-mkeller 4d71f0c
fix logging bug
sfc-gh-mkeller a887c29
review feedback
sfc-gh-mkeller a20dd97
rewording parameters based on discussion
sfc-gh-mkeller 4d78183
reworking the defaut oauth scope
sfc-gh-mkeller 60b461a
adding port support into oauth code flow auth
sfc-gh-mkeller ac3c287
do not log state
sfc-gh-mkeller File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,128 @@ | ||
# | ||
# Copyright (c) 2012-2023 Snowflake Computing Inc. All rights reserved. | ||
# | ||
|
||
from __future__ import annotations | ||
|
||
import logging | ||
import os | ||
import select | ||
import socket | ||
import time | ||
|
||
from ..compat import IS_WINDOWS | ||
from ..errorcode import ER_NO_HOSTNAME_FOUND | ||
from ..errors import OperationalError | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
class AuthHttpServer: | ||
"""Simple HTTP server to receive callbacks through for auth purposes.""" | ||
|
||
def __init__( | ||
self, | ||
hostname: str = "localhost", | ||
sfc-gh-eworoshow marked this conversation as resolved.
Show resolved
Hide resolved
|
||
buf_size: int = 16384, | ||
) -> None: | ||
self.buf_size = buf_size | ||
self._socket_connection = socket.socket(socket.AF_INET, socket.SOCK_STREAM) | ||
|
||
if os.getenv("SNOWFLAKE_AUTH_SOCKET_REUSE_PORT", "False").lower() == "true": | ||
if IS_WINDOWS: | ||
logger.warning( | ||
"Configuration SNOWFLAKE_AUTH_SOCKET_REUSE_PORT is not available in Windows. Ignoring." | ||
) | ||
else: | ||
self._socket_connection.setsockopt( | ||
socket.SOL_SOCKET, socket.SO_REUSEPORT, 1 | ||
) | ||
|
||
try: | ||
self._socket_connection.bind( | ||
( | ||
os.getenv("SF_AUTH_SOCKET_ADDR", hostname), | ||
int(os.getenv("SF_AUTH_SOCKET_PORT", 0)), | ||
) | ||
) | ||
except socket.gaierror as ex: | ||
if ex.args[0] == socket.EAI_NONAME and hostname == "localhost": | ||
raise OperationalError( | ||
msg="localhost is not found. Ensure /etc/hosts has " | ||
sfc-gh-eworoshow marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"localhost entry.", | ||
errno=ER_NO_HOSTNAME_FOUND, | ||
) | ||
raise | ||
try: | ||
self._socket_connection.listen(0) # no backlog | ||
self.port = self._socket_connection.getsockname()[1] | ||
except Exception: | ||
self._socket_connection.close() | ||
|
||
def receive_block( | ||
self, | ||
max_attempts: int = 15, | ||
) -> tuple[list[str], socket.socket]: | ||
"""Receive a message with a maximum attempt count, blocking.""" | ||
socket_client = None | ||
while True: | ||
try: | ||
attempts = 0 | ||
raw_data = bytearray() | ||
|
||
msg_dont_wait = ( | ||
os.getenv("SNOWFLAKE_AUTH_SOCKET_MSG_DONTWAIT", "false").lower() | ||
== "true" | ||
) | ||
if IS_WINDOWS: | ||
if msg_dont_wait: | ||
logger.warning( | ||
"Configuration SNOWFLAKE_AUTH_SOCKET_MSG_DONTWAIT is not available in Windows. Ignoring." | ||
) | ||
msg_dont_wait = False | ||
|
||
# when running in a containerized environment, socket_client.recv ocassionally returns an empty byte array | ||
# an immediate successive call to socket_client.recv gets the actual data | ||
while len(raw_data) == 0 and attempts < max_attempts: | ||
attempts += 1 | ||
read_sockets, _write_sockets, _exception_sockets = select.select( | ||
[self._socket_connection], [], [] | ||
) | ||
|
||
if read_sockets[0] is not None: | ||
# Receive the data in small chunks and retransmit it | ||
socket_client, _ = self._socket_connection.accept() | ||
|
||
try: | ||
if msg_dont_wait: | ||
# WSL containerized environment sometimes causes socket_client.recv to hang indefinetly | ||
# To avoid this, passing the socket.MSG_DONTWAIT flag which raises BlockingIOError if | ||
# operation would block | ||
logger.debug( | ||
"Calling socket_client.recv with MSG_DONTWAIT flag due to SNOWFLAKE_AUTH_SOCKET_MSG_DONTWAIT env var" | ||
) | ||
raw_data = socket_client.recv( | ||
BUF_SIZE, socket.MSG_DONTWAIT | ||
) | ||
else: | ||
raw_data = socket_client.recv(self.buf_size) | ||
|
||
except BlockingIOError: | ||
logger.debug( | ||
"BlockingIOError raised from socket.recv while attempting to retrieve callback request" | ||
) | ||
if attempts < max_attempts: | ||
sleep_time = 0.25 | ||
logger.debug( | ||
f"Waiting {sleep_time} seconds before trying again" | ||
) | ||
time.sleep(sleep_time) | ||
else: | ||
logger.debug("Exceeded retry count") | ||
|
||
assert socket_client is not None | ||
return raw_data.decode("utf-8").split("\r\n"), socket_client | ||
except Exception: | ||
if socket_client is not None: | ||
socket_client.shutdown(socket.SHUT_RDWR) | ||
socket_client.close() |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume we have this to avoid taking additional dependencies in the driver?
(As we move to introduce additional forms of CSP-specific authentication I suspect we'll want to introduce dependencies on the various CSP libraries, which is in part why I ask.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I copied this code all from externalbrowser auth. So it's tried and tested given that I have made no mistakes. I didn't feel like I needed to introduce any dependencies for this OAuth implementation and I'm not sure I'd have time to really investigate different options anymore. I'd like to leave this up to the new owners of the library if you don't mind