From c67d8fd45a1e1abdebd6e9e41c7955ad30652c65 Mon Sep 17 00:00:00 2001 From: byhsu Date: Fri, 28 Apr 2023 15:39:20 -0700 Subject: [PATCH 1/5] Add ssl unverify mode because cert not working in some cases Signed-off-by: byhsu --- flytekit/clients/auth/authenticator.py | 34 +++++++++----------------- flytekit/clients/auth/token_client.py | 13 +++++++--- flytekit/clients/auth_helper.py | 23 +++++++++++------ 3 files changed, 36 insertions(+), 34 deletions(-) diff --git a/flytekit/clients/auth/authenticator.py b/flytekit/clients/auth/authenticator.py index bde25602e0..3824707379 100644 --- a/flytekit/clients/auth/authenticator.py +++ b/flytekit/clients/auth/authenticator.py @@ -48,17 +48,12 @@ class Authenticator(object): Base authenticator for all authentication flows """ - def __init__( - self, - endpoint: str, - header_key: str, - credentials: Credentials = None, - http_proxy_url: typing.Optional[str] = None, - ): + def __init__(self, endpoint: str, header_key: str, credentials: Credentials = None, http_proxy_url: typing.Optional[str] = None, verify: typing.Optional[typing.Union[bool, str]] = None): self._endpoint = endpoint self._creds = credentials self._header_key = header_key if header_key else "authorization" self._http_proxy_url = http_proxy_url + self._verify = verify def get_credentials(self) -> Credentials: return self._creds @@ -94,10 +89,9 @@ def __init__( """ Initialize with default creds from KeyStore using the endpoint name """ - super().__init__(endpoint, header_key, KeyringStore.retrieve(endpoint)) + super().__init__(endpoint, header_key, KeyringStore.retrieve(endpoint), verify=verify) self._cfg_store = cfg_store self._auth_client = None - self._verify = verify def _initialize_auth_client(self): if not self._auth_client: @@ -170,6 +164,7 @@ def __init__( header_key: typing.Optional[str] = None, scopes: typing.Optional[typing.List[str]] = None, http_proxy_url: typing.Optional[str] = None, + verify: typing.Optional[typing.Union[bool, str]] = None, ): if not client_id or not client_secret: raise ValueError("Client ID and Client SECRET both are required.") @@ -179,7 +174,7 @@ def __init__( self._scopes = scopes or cfg.scopes self._client_id = client_id self._client_secret = client_secret - super().__init__(endpoint, cfg.header_key or header_key, http_proxy_url=http_proxy_url) + super().__init__(endpoint, cfg.header_key or header_key, http_proxy_url=http_proxy_url, verify=verify) def refresh_credentials(self): """ @@ -195,9 +190,8 @@ def refresh_credentials(self): # Note that unlike the Pkce flow, the client ID does not come from Admin. logging.debug(f"Basic authorization flow with client id {self._client_id} scope {scopes}") authorization_header = token_client.get_basic_authorization_header(self._client_id, self._client_secret) - token, expires_in = token_client.get_token( - token_endpoint, scopes, authorization_header, http_proxy_url=self._http_proxy_url - ) + + token, expires_in = token_client.get_token(token_endpoint, scopes, authorization_header, http_proxy_url=self._http_proxy_url, verify=self._verify) logging.info("Retrieved new token, expires in {}".format(expires_in)) self._creds = Credentials(token) @@ -218,6 +212,7 @@ def __init__( header_key: typing.Optional[str] = None, audience: typing.Optional[str] = None, http_proxy_url: typing.Optional[str] = None, + verify: typing.Optional[typing.Union[bool, str]] = None, ): self._audience = audience cfg = cfg_store.get_client_config() @@ -230,16 +225,11 @@ def __init__( "Device Authentication is not available on the Flyte backend / authentication server" ) super().__init__( - endpoint=endpoint, - header_key=header_key or cfg.header_key, - credentials=KeyringStore.retrieve(endpoint), - http_proxy_url=http_proxy_url, + endpoint=endpoint, header_key=header_key or cfg.header_key, credentials=KeyringStore.retrieve(endpoint), http_proxy_url=http_proxy_url, verify=verify ) def refresh_credentials(self): - resp = token_client.get_device_code( - self._device_auth_endpoint, self._client_id, self._audience, self._scope, self._http_proxy_url - ) + resp = token_client.get_device_code(self._device_auth_endpoint, self._client_id, self._audience, self._scope, self._http_proxy_url, self._verify) print( f""" To Authenticate navigate in a browser to the following URL: {resp.verification_uri} and enter code: {resp.user_code} @@ -248,9 +238,7 @@ def refresh_credentials(self): try: # Currently the refresh token is not retreived. We may want to add support for refreshTokens so that # access tokens can be refreshed for once authenticated machines - token, expires_in = token_client.poll_token_endpoint( - resp, self._token_endpoint, client_id=self._client_id, http_proxy_url=self._http_proxy_url - ) + token, expires_in = token_client.poll_token_endpoint(resp, self._token_endpoint, client_id=self._client_id, http_proxy_url=self._http_proxy_url, verify=self._verify) self._creds = Credentials(access_token=token, expires_in=expires_in, for_endpoint=self._endpoint) KeyringStore.store(self._creds) except Exception: diff --git a/flytekit/clients/auth/token_client.py b/flytekit/clients/auth/token_client.py index 728a56b5f7..2e14fe8afc 100644 --- a/flytekit/clients/auth/token_client.py +++ b/flytekit/clients/auth/token_client.py @@ -76,6 +76,7 @@ def get_token( device_code: typing.Optional[str] = None, grant_type: GrantType = GrantType.CLIENT_CREDS, http_proxy_url: typing.Optional[str] = None, + verify: typing.Optional[typing.Union[bool, str]] = None, ) -> typing.Tuple[str, int]: """ :rtype: (Text,Int) The first element is the access token retrieved from the IDP, the second is the expiration @@ -99,7 +100,7 @@ def get_token( body["scope"] = ",".join(scopes) proxies = {"https": http_proxy_url, "http": http_proxy_url} if http_proxy_url else None - response = requests.post(token_endpoint, data=body, headers=headers, proxies=proxies) + response = requests.post(token_endpoint, data=body, headers=headers, proxies=proxies, verify=verify) if not response.ok: j = response.json() if "error" in j: @@ -119,6 +120,7 @@ def get_device_code( audience: typing.Optional[str] = None, scope: typing.Optional[typing.List[str]] = None, http_proxy_url: typing.Optional[str] = None, + verify: typing.Optional[typing.Union[bool, str]] = None, ) -> DeviceCodeResponse: """ Retrieves the device Authentication code that can be done to authenticate the request using a browser on a @@ -127,14 +129,18 @@ def get_device_code( _scope = " ".join(scope) if scope is not None else "" payload = {"client_id": client_id, "scope": _scope, "audience": audience} proxies = {"https": http_proxy_url, "http": http_proxy_url} if http_proxy_url else None - resp = requests.post(device_auth_endpoint, payload, proxies=proxies) + resp = requests.post(device_auth_endpoint, payload, proxies=proxies, verify=verify) if not resp.ok: raise AuthenticationError(f"Unable to retrieve Device Authentication Code for {payload}, Reason {resp.reason}") return DeviceCodeResponse.from_json_response(resp.json()) def poll_token_endpoint( - resp: DeviceCodeResponse, token_endpoint: str, client_id: str, http_proxy_url: typing.Optional[str] = None + resp: DeviceCodeResponse, + token_endpoint: str, + client_id: str, + http_proxy_url: typing.Optional[str] = None, + verify: typing.Optional[typing.Union[bool, str]] = None, ) -> typing.Tuple[str, int]: tick = datetime.now() interval = timedelta(seconds=resp.interval) @@ -147,6 +153,7 @@ def poll_token_endpoint( client_id=client_id, device_code=resp.device_code, http_proxy_url=http_proxy_url, + verify=verify, ) print("Authentication successful!") return access_token, expires_in diff --git a/flytekit/clients/auth_helper.py b/flytekit/clients/auth_helper.py index a068e487cf..1c79933909 100644 --- a/flytekit/clients/auth_helper.py +++ b/flytekit/clients/auth_helper.py @@ -58,12 +58,13 @@ def get_authenticator(cfg: PlatformConfig, cfg_store: ClientConfigStore) -> Auth logging.warning(f"Authentication type {cfg_auth} does not exist, defaulting to standard") cfg_auth = AuthType.STANDARD + verify = None + if cfg.insecure_skip_verify: + verify = False + elif cfg.ca_cert_file_path: + verify = cfg.ca_cert_file_path + if cfg_auth == AuthType.STANDARD or cfg_auth == AuthType.PKCE: - verify = None - if cfg.insecure_skip_verify: - verify = False - elif cfg.ca_cert_file_path: - verify = cfg.ca_cert_file_path return PKCEAuthenticator(cfg.endpoint, cfg_store, verify=verify) elif cfg_auth == AuthType.BASIC or cfg_auth == AuthType.CLIENT_CREDENTIALS or cfg_auth == AuthType.CLIENTSECRET: return ClientCredentialsAuthenticator( @@ -73,6 +74,7 @@ def get_authenticator(cfg: PlatformConfig, cfg_store: ClientConfigStore) -> Auth cfg_store=cfg_store, scopes=cfg.scopes, http_proxy_url=cfg.http_proxy_url, + verify=verify ) elif cfg_auth == AuthType.EXTERNAL_PROCESS or cfg_auth == AuthType.EXTERNALCOMMAND: client_cfg = None @@ -84,7 +86,11 @@ def get_authenticator(cfg: PlatformConfig, cfg_store: ClientConfigStore) -> Auth ) elif cfg_auth == AuthType.DEVICEFLOW: return DeviceCodeAuthenticator( - endpoint=cfg.endpoint, cfg_store=cfg_store, audience=cfg.audience, http_proxy_url=cfg.http_proxy_url + endpoint=cfg.endpoint, + cfg_store=cfg_store, + audience=cfg.audience, + http_proxy_url=cfg.http_proxy_url, + verify=verify ) else: raise ValueError( @@ -120,8 +126,9 @@ def load_cert(cert_file: str) -> crypto.X509: """ Given a cert-file loads the PEM certificate and returns """ - st_cert = open(cert_file, "rt").read() - return crypto.load_certificate(crypto.FILETYPE_PEM, st_cert) + st_cert = open(cert_file, "rb").read() + cert = crypto.load_certificate(crypto.FILETYPE_PEM, st_cert) + return crypto.dump_certificate(crypto.FILETYPE_PEM, cert) def bootstrap_creds_from_server(endpoint: str) -> grpc.ChannelCredentials: From 25b78b72176840eedf6aad2ccf46fad7899e9d7d Mon Sep 17 00:00:00 2001 From: byhsu Date: Mon, 15 May 2023 22:27:30 -0700 Subject: [PATCH 2/5] revert load_cert Signed-off-by: byhsu --- flytekit/clients/auth_helper.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/flytekit/clients/auth_helper.py b/flytekit/clients/auth_helper.py index 1c79933909..badcf460d4 100644 --- a/flytekit/clients/auth_helper.py +++ b/flytekit/clients/auth_helper.py @@ -126,9 +126,8 @@ def load_cert(cert_file: str) -> crypto.X509: """ Given a cert-file loads the PEM certificate and returns """ - st_cert = open(cert_file, "rb").read() - cert = crypto.load_certificate(crypto.FILETYPE_PEM, st_cert) - return crypto.dump_certificate(crypto.FILETYPE_PEM, cert) + st_cert = open(cert_file, "rt").read() + return crypto.load_certificate(crypto.FILETYPE_PEM, st_cert) def bootstrap_creds_from_server(endpoint: str) -> grpc.ChannelCredentials: From 05f9417f53cafb3e72d949f015da4b42b7c3520c Mon Sep 17 00:00:00 2001 From: byhsu Date: Mon, 15 May 2023 22:35:34 -0700 Subject: [PATCH 3/5] add tests Signed-off-by: byhsu --- tests/flytekit/unit/clients/auth/test_authenticator.py | 4 +++- tests/flytekit/unit/clients/auth/test_token_client.py | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/flytekit/unit/clients/auth/test_authenticator.py b/tests/flytekit/unit/clients/auth/test_authenticator.py index 495f4648ac..36bf1707a0 100644 --- a/tests/flytekit/unit/clients/auth/test_authenticator.py +++ b/tests/flytekit/unit/clients/auth/test_authenticator.py @@ -96,6 +96,7 @@ def test_device_flow_authenticator(poll_mock: MagicMock, device_mock: MagicMock, ENDPOINT, static_cfg_store, audience="x", + verify=True ) cfg_store = StaticClientConfigStore( @@ -107,7 +108,7 @@ def test_device_flow_authenticator(poll_mock: MagicMock, device_mock: MagicMock, device_authorization_endpoint="dev", ) ) - authn = DeviceCodeAuthenticator(ENDPOINT, cfg_store, audience="x", http_proxy_url="http://my-proxy:9000") + authn = DeviceCodeAuthenticator(ENDPOINT, cfg_store, audience="x", http_proxy_url="http://my-proxy:9000", verfiy=False) device_mock.return_value = DeviceCodeResponse("x", "y", "s", 1000, 0) poll_mock.return_value = ("access", 100) @@ -124,6 +125,7 @@ def test_client_creds_authenticator_with_custom_scopes(mock_requests): client_secret="secret", cfg_store=static_cfg_store, scopes=expected_scopes, + verify=True, ) response = MagicMock() response.status_code = 200 diff --git a/tests/flytekit/unit/clients/auth/test_token_client.py b/tests/flytekit/unit/clients/auth/test_token_client.py index f0c10b16d1..d0e75ec88a 100644 --- a/tests/flytekit/unit/clients/auth/test_token_client.py +++ b/tests/flytekit/unit/clients/auth/test_token_client.py @@ -29,7 +29,7 @@ def test_get_token(mock_requests): response.json.return_value = json.loads("""{"access_token": "abc", "expires_in": 60}""") mock_requests.post.return_value = response access, expiration = get_token( - "https://corp.idp.net", client_id="abc123", scopes=["my_scope"], http_proxy_url="http://proxy:3000" + "https://corp.idp.net", client_id="abc123", scopes=["my_scope"], http_proxy_url="http://proxy:3000", verify=True ) assert access == "abc" assert expiration == 60 @@ -66,13 +66,13 @@ def test_poll_token_endpoint(mock_requests): r = DeviceCodeResponse(device_code="x", user_code="y", verification_uri="v", expires_in=1, interval=1) with pytest.raises(AuthenticationError): - poll_token_endpoint(r, "test.com", "test", http_proxy_url="http://proxy:3000") + poll_token_endpoint(r, "test.com", "test", http_proxy_url="http://proxy:3000", verify=True) response = MagicMock() response.ok = True response.json.return_value = {"access_token": "abc", "expires_in": 60} mock_requests.post.return_value = response r = DeviceCodeResponse(device_code="x", user_code="y", verification_uri="v", expires_in=1, interval=0) - t, e = poll_token_endpoint(r, "test.com", "test", http_proxy_url="http://proxy:3000") + t, e = poll_token_endpoint(r, "test.com", "test", http_proxy_url="http://proxy:3000", verify=True) assert t assert e From 631513e475d6de573005f1d837244bc0cb227e41 Mon Sep 17 00:00:00 2001 From: byhsu Date: Mon, 15 May 2023 22:46:24 -0700 Subject: [PATCH 4/5] lint Signed-off-by: byhsu --- flytekit/clients/auth/authenticator.py | 31 ++++++++++++++++--- flytekit/clients/auth_helper.py | 4 +-- .../unit/clients/auth/test_authenticator.py | 11 +++---- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/flytekit/clients/auth/authenticator.py b/flytekit/clients/auth/authenticator.py index 3824707379..65bc994a27 100644 --- a/flytekit/clients/auth/authenticator.py +++ b/flytekit/clients/auth/authenticator.py @@ -48,7 +48,14 @@ class Authenticator(object): Base authenticator for all authentication flows """ - def __init__(self, endpoint: str, header_key: str, credentials: Credentials = None, http_proxy_url: typing.Optional[str] = None, verify: typing.Optional[typing.Union[bool, str]] = None): + def __init__( + self, + endpoint: str, + header_key: str, + credentials: Credentials = None, + http_proxy_url: typing.Optional[str] = None, + verify: typing.Optional[typing.Union[bool, str]] = None, + ): self._endpoint = endpoint self._creds = credentials self._header_key = header_key if header_key else "authorization" @@ -191,7 +198,9 @@ def refresh_credentials(self): logging.debug(f"Basic authorization flow with client id {self._client_id} scope {scopes}") authorization_header = token_client.get_basic_authorization_header(self._client_id, self._client_secret) - token, expires_in = token_client.get_token(token_endpoint, scopes, authorization_header, http_proxy_url=self._http_proxy_url, verify=self._verify) + token, expires_in = token_client.get_token( + token_endpoint, scopes, authorization_header, http_proxy_url=self._http_proxy_url, verify=self._verify + ) logging.info("Retrieved new token, expires in {}".format(expires_in)) self._creds = Credentials(token) @@ -225,11 +234,17 @@ def __init__( "Device Authentication is not available on the Flyte backend / authentication server" ) super().__init__( - endpoint=endpoint, header_key=header_key or cfg.header_key, credentials=KeyringStore.retrieve(endpoint), http_proxy_url=http_proxy_url, verify=verify + endpoint=endpoint, + header_key=header_key or cfg.header_key, + credentials=KeyringStore.retrieve(endpoint), + http_proxy_url=http_proxy_url, + verify=verify, ) def refresh_credentials(self): - resp = token_client.get_device_code(self._device_auth_endpoint, self._client_id, self._audience, self._scope, self._http_proxy_url, self._verify) + resp = token_client.get_device_code( + self._device_auth_endpoint, self._client_id, self._audience, self._scope, self._http_proxy_url, self._verify + ) print( f""" To Authenticate navigate in a browser to the following URL: {resp.verification_uri} and enter code: {resp.user_code} @@ -238,7 +253,13 @@ def refresh_credentials(self): try: # Currently the refresh token is not retreived. We may want to add support for refreshTokens so that # access tokens can be refreshed for once authenticated machines - token, expires_in = token_client.poll_token_endpoint(resp, self._token_endpoint, client_id=self._client_id, http_proxy_url=self._http_proxy_url, verify=self._verify) + token, expires_in = token_client.poll_token_endpoint( + resp, + self._token_endpoint, + client_id=self._client_id, + http_proxy_url=self._http_proxy_url, + verify=self._verify, + ) self._creds = Credentials(access_token=token, expires_in=expires_in, for_endpoint=self._endpoint) KeyringStore.store(self._creds) except Exception: diff --git a/flytekit/clients/auth_helper.py b/flytekit/clients/auth_helper.py index badcf460d4..eebac9611d 100644 --- a/flytekit/clients/auth_helper.py +++ b/flytekit/clients/auth_helper.py @@ -74,7 +74,7 @@ def get_authenticator(cfg: PlatformConfig, cfg_store: ClientConfigStore) -> Auth cfg_store=cfg_store, scopes=cfg.scopes, http_proxy_url=cfg.http_proxy_url, - verify=verify + verify=verify, ) elif cfg_auth == AuthType.EXTERNAL_PROCESS or cfg_auth == AuthType.EXTERNALCOMMAND: client_cfg = None @@ -90,7 +90,7 @@ def get_authenticator(cfg: PlatformConfig, cfg_store: ClientConfigStore) -> Auth cfg_store=cfg_store, audience=cfg.audience, http_proxy_url=cfg.http_proxy_url, - verify=verify + verify=verify, ) else: raise ValueError( diff --git a/tests/flytekit/unit/clients/auth/test_authenticator.py b/tests/flytekit/unit/clients/auth/test_authenticator.py index 36bf1707a0..9295b614d5 100644 --- a/tests/flytekit/unit/clients/auth/test_authenticator.py +++ b/tests/flytekit/unit/clients/auth/test_authenticator.py @@ -92,12 +92,7 @@ def test_client_creds_authenticator(mock_requests): @patch("flytekit.clients.auth.token_client.poll_token_endpoint") def test_device_flow_authenticator(poll_mock: MagicMock, device_mock: MagicMock, mock_keyring: MagicMock): with pytest.raises(AuthenticationError): - DeviceCodeAuthenticator( - ENDPOINT, - static_cfg_store, - audience="x", - verify=True - ) + DeviceCodeAuthenticator(ENDPOINT, static_cfg_store, audience="x", verify=True) cfg_store = StaticClientConfigStore( ClientConfig( @@ -108,7 +103,9 @@ def test_device_flow_authenticator(poll_mock: MagicMock, device_mock: MagicMock, device_authorization_endpoint="dev", ) ) - authn = DeviceCodeAuthenticator(ENDPOINT, cfg_store, audience="x", http_proxy_url="http://my-proxy:9000", verfiy=False) + authn = DeviceCodeAuthenticator( + ENDPOINT, cfg_store, audience="x", http_proxy_url="http://my-proxy:9000", verfiy=False + ) device_mock.return_value = DeviceCodeResponse("x", "y", "s", 1000, 0) poll_mock.return_value = ("access", 100) From 5c644a281f42db8b8edaccf702214581f69e3aac Mon Sep 17 00:00:00 2001 From: byhsu Date: Tue, 16 May 2023 09:41:50 -0700 Subject: [PATCH 5/5] typo Signed-off-by: byhsu --- tests/flytekit/unit/clients/auth/test_authenticator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/flytekit/unit/clients/auth/test_authenticator.py b/tests/flytekit/unit/clients/auth/test_authenticator.py index 9295b614d5..32709e1eaa 100644 --- a/tests/flytekit/unit/clients/auth/test_authenticator.py +++ b/tests/flytekit/unit/clients/auth/test_authenticator.py @@ -104,7 +104,7 @@ def test_device_flow_authenticator(poll_mock: MagicMock, device_mock: MagicMock, ) ) authn = DeviceCodeAuthenticator( - ENDPOINT, cfg_store, audience="x", http_proxy_url="http://my-proxy:9000", verfiy=False + ENDPOINT, cfg_store, audience="x", http_proxy_url="http://my-proxy:9000", verify=False ) device_mock.return_value = DeviceCodeResponse("x", "y", "s", 1000, 0)