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

Remove support for more than 1 domain per hub #460

Merged
merged 2 commits into from
Jun 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 1 addition & 3 deletions config/hubs/cloudbank.cluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -335,9 +335,7 @@ hubs:
- nfguebels@pipeline.sbcc.edu
admin_users: *sbcc_users
- name: mills
domain:
- mills.cloudbank.2i2c.cloud
- datahub.mills.edu
domain: datahub.mills.edu
template: basehub
auth0:
connection: google-oauth2
Expand Down
28 changes: 11 additions & 17 deletions config/hubs/schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ properties:
Cloud provider this cluster is running on. Used to perform
authentication against the cluster. Currently supports gcp
and raw kubeconfig files.
enum:
enum:
- gcp
- kubeconfig
kubeconfig:
type: object
description: |
Configuration to connect to a cluster purely via a kubeconfig
file.
file.
additionalProperties: false
properties:
file:
Expand Down Expand Up @@ -83,28 +83,22 @@ properties:
Name of the hub. This will be used to determine
the namespace the hub is deployed to
domain:
anyOf:
- type: string
description: |
Domain the hub should be running on. This domain should resolve
to the IP of the ingress controller on the cluster - most likely
via a wildcard DNS entry.
type: string
description: |
Domain the hub should be running on. This domain should resolve
to the IP of the ingress controller on the cluster - most likely
via a wildcard DNS entry.

For example, there's a entry for '*.pilot.2i2c.cloud' pointing to
the ingress controller of the cluster running hubs in `2i2c.cluster.yaml`.
Similar for '*.cloudbank.2i2c.cloud', etc.
- type: array
description: |
Multiple domain names that can resolve to this hub. Primarily
used as a way to redirect old hub URLs to new - should be replaced
with something nicer.
For example, there's a entry for '*.pilot.2i2c.cloud' pointing to
the ingress controller of the cluster running hubs in `2i2c.cluster.yaml`.
Similar for '*.cloudbank.2i2c.cloud', etc.
template:
type: string
description: |
Template to deploy the hub with. This refers to a directory under
`hub-templates` containing a helm chart with base values and dependencies
that determine the kind of hub deployed.
enum:
enum:
- basehub
- daskhub
auth0:
Expand Down
64 changes: 25 additions & 39 deletions deployer/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,70 +46,56 @@ def get_connections(self):
for connection in self.auth0.connections.all()
}

def create_client(self, name, domains):
callbacks = self._get_callback_url_list(domains)
logout_urls = self._get_allowed_logout_url_list(domains)

def create_client(self, name, callback_url, logout_url):
client = {
'name': name,
'app_type': 'regular_web',
'callbacks': callbacks,
'allowed_logout_urls': logout_urls
'callbacks': [callback_url],
'allowed_logout_urls': [logout_url]
}
created_client = self.auth0.clients.create(client)
return created_client

def _get_callback_url_list(self, domains):
return [f'https://{domain}/hub/oauth_callback' for domain in domains] if isinstance(domains, list) else [f'https://{domains}/hub/oauth_callback']

def _get_allowed_logout_url_list(self, domains):
# Mark the hub address as a valid URL address to return to after logout
return [f'https://{domain}' for domain in domains] if isinstance(domains, list) else [f'https://{domains}']


def _ensure_client_callback(self, client, domains):
callback_urls = self._get_callback_url_list(domains)
missing_callbacks = []

for callback_url in callback_urls:
if 'callbacks' not in client or callback_url not in client['callbacks']:
missing_callbacks.append(callback_url)

if missing_callbacks:
def _ensure_client_callback(self, client, callback_url):
"""
Ensure client has correct callback URL
"""
if client['callbacks'] != [callback_url]:
self.auth0.clients.update(
client['client_id'],
{
# Don't remove other callback URLs
'callbacks': client['callbacks'] + missing_callbacks
# Overwrite any other callback URL specified
# Only one hub should use any one auth0 application, and it
# should only have one callback url. Additional URLs can be
# a security risk, since people who control those URLs could
# potentially steal user credentials (if they have client_id and secret).
# Fully managing list of callback URLs in code keeps everything
# simpler
'callbacks': [callback_url]
}
)

def _ensure_client_logout_urls(self, client, domains):
logout_urls = self._get_allowed_logout_url_list(domains)
missing_logout_urls = []

for logout_url in logout_urls:
if 'allowed_logout_urls' not in client or logout_url not in client['allowed_logout_urls']:
missing_logout_urls.append(logout_url)

if missing_logout_urls:
def _ensure_client_logout_url(self, client, logout_url):
if client['allowed_logout_urls'] != [logout_url]:
self.auth0.clients.update(
client['client_id'],
{
# Don't remove other logout URLs
'allowed_logout_urls': client.get('allowed_logout_urls', []) + missing_logout_urls
# Overwrite any other logout URL - users should only land on
# the hub home page after logging out.
'allowed_logout_urls': [logout_url]
}
)

def ensure_client(self, name, domains, connection_name, connection_config):
def ensure_client(self, name, callback_url, logout_url, connection_name, connection_config):
current_clients = self.get_clients()
if name not in current_clients:
# Create the client, all good
client = self.create_client(name, domains)
client = self.create_client(name, callback_url, logout_url)
else:
client = current_clients[name]
self._ensure_client_callback(client, domains)
self._ensure_client_logout_urls(client, domains)
self._ensure_client_callback(client, callback_url)
self._ensure_client_logout_url(client, logout_url)

current_connections = self.get_connections()

Expand Down
22 changes: 11 additions & 11 deletions deployer/hub.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,15 +147,15 @@ def get_generated_config(self, auth_provider: KeyProvider, secret_key):
'jupyterhub': {
'proxy': {
'https': {
'hosts': self.spec['domain'] if isinstance(self.spec['domain'], list) else [self.spec['domain']]
'hosts': [self.spec['domain']]
}
},
'ingress': {
'hosts': self.spec['domain'] if isinstance(self.spec['domain'], list) else [self.spec['domain']],
'hosts': [self.spec['domain']],
'tls': [
{
'secretName': 'https-auto-tls',
'hosts': self.spec['domain'] if isinstance(self.spec['domain'], list) else [self.spec['domain']]
'hosts': [self.spec['domain']]
}
]

Expand Down Expand Up @@ -246,13 +246,18 @@ def get_generated_config(self, auth_provider: KeyProvider, secret_key):
#
# Allow explicilty ignoring auth0 setup
if self.spec['auth0'].get('enabled', True):
# Auth0 sends users back to this URL after they authenticate
callback_url = f"https://{self.spec['domain']}/hub/oauth_callback"
# Users are redirected to this URL after they log out
logout_url = f"https://{self.spec['domain']}"
client = auth_provider.ensure_client(
name=self.spec['auth0'].get('application_name', f"{self.cluster.spec['name']}-{self.spec['name']}"),
domains=self.spec['domain'],
callback_url=callback_url,
logout_url=logout_url,
connection_name=self.spec['auth0']['connection'],
connection_config=self.spec['auth0'].get(self.spec['auth0']['connection'], {}),
)
# FIXME: We're hardcoding GenericOAuthenticator here
# FIXME: We're hardcoding Auth0OAuthenticator here
# We should *not*. We need dictionary merging in code, so
# these can all exist fine.
generated_config['jupyterhub']['hub']['config']['Auth0OAuthenticator'] = auth_provider.get_client_creds(client, self.spec['auth0']['connection'])
Expand Down Expand Up @@ -359,12 +364,7 @@ def deploy(self, auth_provider, secret_key, skip_hub_health_test=False):
else:
service_api_token = generated_values["jupyterhub"]["hub"]["services"]["hub-health"]["apiToken"]

domain = self.spec["domain"]
if type(domain) == str:
hub_url = f'https://{domain}'

else:
hub_url = f'https://{random.choice(domain)}'
hub_url = f'https://{self.spec["domain"]}'

exit_code = pytest.main([
"-v", "deployer/tests",
Expand Down