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.

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

OAuth2 support for flyte-cli and SDK engine #23

Merged
merged 46 commits into from
Dec 6, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
d0715bb
WIP pkce auth flow
katrogan Oct 11, 2019
f576129
Add discovery
katrogan Oct 23, 2019
c7eba16
pass authentication metadata in gRPC calls
katrogan Oct 23, 2019
1bda5da
add credential type, misc clean-up
katrogan Oct 23, 2019
cc2517c
nits
katrogan Nov 12, 2019
2a350c7
removing a redirect follow, comments, adding a sample default discove…
Nov 13, 2019
1cb1620
checkpoint
katrogan Nov 13, 2019
2b9d178
proper sequence of events
katrogan Nov 13, 2019
1cc9999
adding config file read, sample config file, and a test that will be …
Nov 13, 2019
89dcb1e
pulling and resolving conflicts
Nov 13, 2019
1cbcf3a
let ur browser handle redirects
katrogan Nov 13, 2019
6ccb327
pull out urlencode separately, doesn't work for python3
Nov 14, 2019
58f97e9
fixing python3 import, changing keyring var names
Nov 14, 2019
986d49b
some super pythonic code for handling token refresh
katrogan Nov 15, 2019
ff842e2
use retry library
katrogan Nov 16, 2019
2928417
customizable rpc error callback fn
katrogan Nov 22, 2019
e50df71
configurable refresh handlers
katrogan Nov 25, 2019
41c6c2b
expose set_access_token
katrogan Nov 25, 2019
fd1083e
remove refresh_metadata()
katrogan Nov 26, 2019
33e1586
pyflyte component of auth (#52)
wild-endeavor Nov 26, 2019
e380b7e
update the character range for code verifier to include - _ . ~
Nov 26, 2019
1ecf29f
adding test for set token
Nov 26, 2019
aeb6529
unit test for basic auth handler
Nov 26, 2019
b27ae35
add preliminary auth and credentials tests
katrogan Nov 26, 2019
96416fd
Merge branch 'pkce-auth' of github.com:lyft/flytekit into pkce-auth
katrogan Nov 26, 2019
d2ddf4a
add discovery client tests
katrogan Nov 26, 2019
18180e9
fix test for updated regex
katrogan Nov 26, 2019
83c91e1
change str to decode because python2
Nov 26, 2019
77ce838
no auth mode, rm comment
katrogan Nov 27, 2019
bd7324e
nevermind
katrogan Nov 27, 2019
7004c59
one more revert
katrogan Nov 27, 2019
244d032
change handling around default home directory config file loading so …
Nov 27, 2019
48f74fe
Merge branch 'pkce-auth' of github.com:lyft/flytekit into pkce-auth
Nov 27, 2019
067a604
address review comments
katrogan Dec 4, 2019
23a14c8
nits
katrogan Dec 4, 2019
a52ba07
Env var (from pkce-auth pr) option 2 (#64)
wild-endeavor Dec 5, 2019
7f54611
remove no longer necessary location backup for credentials secret
Dec 5, 2019
dcd7d34
merge master and resolve conflict
Dec 5, 2019
053e03e
use the real auth exception class
Dec 5, 2019
0dc3186
get_discovery_endpoint changes
Dec 6, 2019
946a058
cleanup keyring code
katrogan Dec 6, 2019
a940a0c
Merge branch 'pkce-auth' of github.com:lyft/flytekit into pkce-auth
katrogan Dec 6, 2019
d200b55
add expiration handling
katrogan Dec 6, 2019
feb403e
bump flytekit version
katrogan Dec 6, 2019
fc56ce6
no need to force twice, now that it's been added to the base client
Dec 6, 2019
bd9bbb6
use non-beta release
katrogan Dec 6, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion flytekit/clis/auth/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
from http import client as _StatusCodes
except ImportError: # Python 2
import httplib as _StatusCodes
import BaseHTTPServer as _BaseHTTPServer
try: # Python 3
import http.server as _BaseHTTPServer
except ImportError: # Python 2
import BaseHTTPServer as _BaseHTTPServer

try: # Python 3
import urllib.parse as _urlparse
Expand Down
8 changes: 0 additions & 8 deletions flytekit/clis/auth/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,6 @@ def get_authorization_endpoints(self):
resp = _requests.get(
url=self._discovery_url,
)
# Follow at most one redirect.
if resp.status_code == _StatusCodes.FOUND:
redirect_location = resp.headers['Location']
if redirect_location is None:
raise ValueError('Received a 302 but no follow up location was provided in headers')
resp = _requests.get(
url=redirect_location,
)

response_body = resp.json()
if response_body[_authorization_endpoint_key] is None:
Expand Down
15 changes: 9 additions & 6 deletions flytekit/configuration/creds.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

from flytekit.configuration import common as _config_common

DISCOVERY_ENDPOINT = _config_common.FlyteStringConfigurationEntry('credentials', 'discovery_endpoint', default=None)
DISCOVERY_ENDPOINT = _config_common.FlyteStringConfigurationEntry('credentials', 'discovery_endpoint', default='https://company.idp.com/.well-known/oauth-authorization-server')
"""
This endpoint fetches authorization server metadata as describe in this proposal:
https://tools.ietf.org/id/draft-ietf-oauth-discovery-08.html.
This endpoint fetches authorization server metadata as described in:
https://tools.ietf.org/html/rfc8414
The endpoint path can be relative or absolute.
"""

Expand All @@ -15,13 +15,16 @@
More details here: https://www.oauth.com/oauth2-servers/client-registration/client-id-secret/.
"""

REDIRECT_URI = _config_common.FlyteStringConfigurationEntry('credentials', 'redirect_uri', default=None)
REDIRECT_URI = _config_common.FlyteStringConfigurationEntry('credentials', 'redirect_uri', default="http://localhost:53593/callback")
"""
This is the redirect uri registered with the app which handles authorization for a Flyte deployment.
This is the callback uri registered with the app which handles authorization for a Flyte deployment.
Please note the hardcoded port number. Ideally we would not do this, but some IDPs do not allow wildcards for
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haytham suggested that we implement logic to either accept a wildcard port or a list of ports (i.e. if we register a bunch of hard-coded ports with the IDP.) That way we have some flexibility with choosing an available system port. I'm happy to save that for the next PR though and leave this comment in for now

the URL, which means we have to use the same port every time. This is the only reason this is a configuration option,
otherwise, we'd just hardcode the callback path as a constant.
FYI, to see if a given port is already in use, run `sudo lsof -i :<port>` if on a Linux system.
More details here: https://www.oauth.com/oauth2-servers/redirect-uris/.
"""


AUTHORIZATION_METADATA_KEY = _config_common.FlyteStringConfigurationEntry('credentials', 'authorization_metadata_key',
default="authorization")
"""
Expand Down