support authentication #666

bitnik · 2018-09-18T13:29:53Z

Clean version of #596.

From #596:

Mainly I think the separate use_oauth config is unnecessary, since auth_enabled seems to never be true without it.

It seems we don't nee to have separate auth_enabled and use_oauth flags. We can have a single auth_enabled flag that sets up HubOAuth, perhaps? Or even run with a jupyterhub_service flag that will load the full service config (URL, etc.)?

Maybe I don't understand what @minrk meant here. I still think that we need 2 flags: auth_enabled is to hold if authentication is enabled and use_oauth is to hold if an authenticator from https://github.com/jupyterhub/oauthenticator is used.

I would love to get some tests for the authenticated cases. Do you think you are up for that?

I started writing tests but I am not sure what I did is right.

There is already a problem while running auth tests locally. The problem is that HubAuth.api_token is not filled with JUPYTERHUB_API_TOKEN env variable by default (https://github.com/jupyterhub/jupyterhub/blob/master/jupyterhub/services/auth.py#L172). As a workaround I added the dynamic default value generator for api_token in jupyterhub/services/auth.py:

    @default('api_token')
    def _default_api_token(self):
        return os.getenv('JUPYTERHUB_API_TOKEN', '')

When I do this, tests work locally. Is this a bug in JupyterHub or is the way I did auth tests wrong?

minrk · 2018-09-21T13:34:41Z

binderhub/tests/conftest.py

+    cfg.BinderHub.use_oauth = False
+    cfg.BinderHub.use_named_servers = False
+
+    os.environ['JUPYTERHUB_API_TOKEN'] = cfg.BinderHub.hub_api_token


Since the class has been imported already, setting the environment variable here is too late. Instead, we can set cfg.HubAuthenticated.api_token = cfg.BinderHub.hub_api_token directly. Same, I imagine, for the other environment variables. Set config values directly instead of relying on environment variables.

Did you mean cfg.HubOAuth.api_token = cfg.BinderHub.hub_api_token instead of cfg.HubAuthenticated.api_token = cfg.BinderHub.hub_api_token?

But still I don't understand how this is going to configure HubOAuth. This configuration in tests is only used here to create a bhub instance.

I think reading https://traitlets.readthedocs.io/en/latest/config.html will help me to figure out how to do this

minrk · 2018-09-21T13:35:07Z

I still think that we need 2 flags: auth_enabled is to hold if authentication is enabled and use_oauth is to hold if an authenticator from https://github.com/jupyterhub/oauthenticator is used.

I don't think so. The switch here doesn't seem to be related to jupyterhub's own authenticator choice. use_oauth selects between the old HubAuthenticated (relies on cookies, fragile and inflexible) and the newer HubOAuthenticated (uses oauth to authorize with the Hub). I was recommending that we eliminate the use_oauth flag and always authenticate with the Hub via HubOAuthenticated.

betatim · 2018-09-23T17:31:21Z

binderhub/launcher.py

+        )
+        body = json.loads(resp.body.decode('utf-8'))
+        return body
+
    def username_from_repo(self, repo_url):


Maybe rename it to unique_name_from_repo now that it generates more than just a username.

binderhub/launcher.py

betatim · 2018-09-23T17:33:57Z

binderhub/launcher.py

+                    username, e, body,
+                )
+                raise web.HTTPError(500, "Failed to create temporary user for %s" % image)
+        elif server_name == '':


elif server_name == '': -> elif server_name: should be enough to check if a string is not empty.

binderhub/launcher.py

bitnik · 2018-09-25T10:14:09Z

@minrk @betatim thanks a lot for reviews!

@minrk sorry, you were right, I was thinking wrong about HubOAuthenticated and HubAuthenticated. I changed it to use only auth_enabled flag and authenticate with JupyterHub through HubOAuthenticated.

bitnik · 2018-09-27T07:23:57Z

hmm I don't understand why test_auth fails on travis. It gets 404 for http://10.20.0.132:30123/services/binder/. But locally I don't have this error. Do you have any idea why this is happening?

Btw I did all changes that you requested.

minrk · 2018-10-08T12:42:17Z

testing/minikube/jupyterhub-helm-auth-config.yaml

+hub:
+  services:
+    binder:
+      url: http://10.0.2.2:8585


This hardcoded ip may not be stable for any given deployment

true, this IP is only for when VirtualBox driver is used

This reverts commit 51a53df.

only when VirtualBox drive is used. minikube runs on travis without VM driver. try to get and use host IP.

This reverts commit d546980.

bitnik · 2018-10-09T11:20:33Z

@minrk tests passed after I replaced 10.0.2.2 with host IP.

minrk · 2018-10-11T13:22:16Z

Awesome, thanks!

choldgraf · 2018-10-11T14:18:11Z

boom! this one has been a long time coming! really nice @bitnik :-)

bitnik · 2018-10-12T05:28:14Z

finally :) thanks to all of you!

meeseeksmachine · 2019-12-11T09:47:01Z

This pull request has been mentioned on Jupyter Community Forum. There might be relevant details there:

https://discourse.jupyter.org/t/a-persistent-binderhub-deployment/2865/1

bitnik mentioned this pull request Sep 18, 2018

[WIP] support authentication #596

Closed

bitnik added 2 commits September 19, 2018 08:18

support authentication

f101fad

tests for auth

a6235c0

bitnik force-pushed the authnew branch from 02dbc4f to a6235c0 Compare September 19, 2018 06:19

minrk reviewed Sep 21, 2018

View reviewed changes

betatim reviewed Sep 23, 2018

View reviewed changes

binderhub/launcher.py Show resolved Hide resolved

betatim reviewed Sep 23, 2018

View reviewed changes

binderhub/launcher.py Show resolved Hide resolved

bitnik added 2 commits September 25, 2018 09:08

minor changes

3d228a8

use always HubOAuthenticated instead of HubAuthenticated

90c5d54

bitnik force-pushed the authnew branch 2 times, most recently from 03c321f to 7004ada Compare September 26, 2018 14:27

bitnik added 2 commits September 26, 2018 16:37

set config values of HubOAuth instead of relying on env variables

b37da83

fix position of rbac conf in values.yaml

b95ec01

bitnik force-pushed the authnew branch from 7004ada to b95ec01 Compare September 26, 2018 14:37

bitnik added 2 commits September 27, 2018 08:03

update test auth

316f51e

use_session fixture

12ed55a

bitnik force-pushed the authnew branch from b316387 to f207477 Compare October 8, 2018 10:08

wait more before auth tests and increase test timeout

d546980

bitnik force-pushed the authnew branch from 0e8c377 to d546980 Compare October 8, 2018 11:22

bitnik and others added 2 commits October 8, 2018 13:26

Merge branch 'master' into authnew

53e6531

update auth tests

c6a6a02

minrk reviewed Oct 8, 2018

View reviewed changes

bitnik added 3 commits October 8, 2018 15:21

try without builder

51a53df

Revert "try without builder"

c908833

This reverts commit 51a53df.

10.0.2.2 can be used to connect back to the host

dbb0783

only when VirtualBox drive is used. minikube runs on travis without VM driver. try to get and use host IP.

bitnik force-pushed the authnew branch from a63106a to dbb0783 Compare October 9, 2018 09:36

bitnik added 3 commits October 9, 2018 11:45

update test-auth

8a2825a

Revert "wait more before auth tests and increase test timeout"

19646d1

This reverts commit d546980.

sleep 30 seconds before auth tests

3b86e36

Merge branch 'master' into authnew

7d008a3

minrk merged commit b56f85b into jupyterhub:master Oct 11, 2018

This was referenced Oct 11, 2018

add authentication to binder.pangeo.io pangeo-data/pangeo-binder#6

Open

Support Authentication for BinderHub #323

Closed

bitnik deleted the authnew branch October 12, 2018 05:41

choldgraf mentioned this pull request Oct 12, 2018

Create documentation for setting up authentication #691

Closed

bitnik mentioned this pull request Oct 30, 2018

Re-add jupyterhub into default installs jupyterhub/repo2docker#458

Closed

arnim mentioned this pull request Nov 26, 2018

Persistent storage #377

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support authentication #666

support authentication #666

bitnik commented Sep 18, 2018

minrk Sep 21, 2018

bitnik Sep 25, 2018

bitnik Sep 25, 2018

minrk commented Sep 21, 2018

betatim Sep 23, 2018

betatim Sep 23, 2018

bitnik commented Sep 25, 2018

bitnik commented Sep 27, 2018

minrk Oct 8, 2018

bitnik Oct 9, 2018

bitnik commented Oct 9, 2018

minrk commented Oct 11, 2018

choldgraf commented Oct 11, 2018

bitnik commented Oct 12, 2018

meeseeksmachine commented Dec 11, 2019

support authentication #666

support authentication #666

Conversation

bitnik commented Sep 18, 2018

minrk Sep 21, 2018

Choose a reason for hiding this comment

bitnik Sep 25, 2018

Choose a reason for hiding this comment

bitnik Sep 25, 2018

Choose a reason for hiding this comment

minrk commented Sep 21, 2018

betatim Sep 23, 2018

Choose a reason for hiding this comment

betatim Sep 23, 2018

Choose a reason for hiding this comment

bitnik commented Sep 25, 2018

bitnik commented Sep 27, 2018

minrk Oct 8, 2018

Choose a reason for hiding this comment

bitnik Oct 9, 2018

Choose a reason for hiding this comment

bitnik commented Oct 9, 2018

minrk commented Oct 11, 2018

choldgraf commented Oct 11, 2018

bitnik commented Oct 12, 2018

meeseeksmachine commented Dec 11, 2019