failed to create admin account

[Ledenel]

Describe the bug:

I'd like to create a hub on a remote server. mlhub is running, but I failed to create any account, got this error

Expected behaviour:

I'd like to sign up a user called admin. No error in the server log.

Steps to reproduce the issue:

1. create empty folder /data/ml-hub-data
2. run docker image via docker run \ -p 8849:8080 \ -v /var/run/docker.sock:/var/run/docker.sock \ -v /data/ml-hub-data:/data \ mltooling/ml-hub:0.1.10
3. access http://host_ip:8849 to get login page.
4. click sign up, then type in admin for username and adminadmin for password.
5. when click the 'Sign up' button, I got server logs below:

   [I 2019-12-20 16:02:09.767 JupyterHub log:174] 200 GET /hub/signup (@::ffff:127.0.0.1) 51.35ms
   ERROR:asyncio:Future exception was never retrieved
   future: <Future finished exception=TypeError('authenticate() takes 3 positional arguments but 4 were given',)>
   Traceback (most recent call last):
   File "/usr/local/lib/python3.6/dist-packages/tornado/gen.py", line 307, in wrapper
   result = func(*args, **kwargs)
   File "/usr/lib/python3.6/types.py", line 248, in wrapped
   coro = func(*args, **kwargs)
   TypeError: authenticate() takes 3 positional arguments but 4 were given

6. Failed to login with user admin.
7. Tried:
   • create linux user admin inside and outside the container, and set proper password
   • set PAMAuthentatior inside ml-hub container, re-tried linux user creation

Technical details:

• Hub version ml-hub:0.1.10
• Docker version :

  
  Client: Docker Engine - Community
  Version:           19.03.5
  API version:       1.40
  Go version:        go1.12.12
  Git commit:        633a0ea
  Built:             Wed Nov 13 07:25:41 2019
  OS/Arch:           linux/amd64
  Experimental:      false

Server: Docker Engine - Community Engine: Version: 19.03.5 API version: 1.40 (minimum version 1.12) Go version: go1.12.12 Git commit: 633a0ea Built: Wed Nov 13 07:24:18 2019 OS/Arch: linux/amd64 Experimental: false containerd: Version: 1.2.10 GitCommit: b34a5c8af56e510852c35414db4c1f4fa6172339 runc: Version: 1.0.0-rc8+dev GitCommit: 3e425f80a8c931f88e6d94a8c831b9d5aa481657 docker-init: Version: 0.18.0 GitCommit: fec3683

- Host Machine OS (Windows/Linux/Mac): Linux 3.10.0 CentOS 7 (
Linux version 3.10.0-1062.4.1.el7.x86_64 (mockbuild@kbuilder.bsys.centos.org) (gcc version 4.8.5 20150623 (Red Hat 4.8.5-39) (GCC) ))
- Command used to start the hub :```docker run \
    -p 8849:8080 \
    -v /var/run/docker.sock:/var/run/docker.sock \
    -v /data/ml-hub-data:/data \
    mltooling/ml-hub:0.1.10```
- Browser (Chrome/Firefox/Safari): Chrome

**Possible Fix:**

```TypeError: authenticate() takes 3 positional arguments but 4 were given```
may indicate that there's some inconsistence with tornado version and authenticator version.

**Additional context:**

Here is the  tornado version on host machine via `conda list | grep tornado`:
`tornado 4.5.3 pypi_0 pypi`

But this shouldn't trouble within a docker container (as I guess).

[cgddrd]

@Ledenel, please can you confirm that you're not using the nativeauthenticator fork provided out-of-the-box with the mlhub Docker image? I've been able to replicate the error message you report from a fresh Docker pull/run, using all the default settings/components (MacOS Catalina, Docker Desktop 2.1.0.5, Tornado 5.1.1).

However, using the standard out-of-the-box nativeauthenticator provided by 'mlhub' (https://github.com/ml-tooling/nativeauthenticator), I am able to create a new user with the username admin and a password of my choosing (despite the exception being thrown). Instructions relating to this are provided with the README.md (https://github.com/ml-tooling/ml-hub#default-login).

Further investigation of this issue has found that the call to authenticate() actually passes the username and password as separate positional arguments (https://github.com/ml-tooling/nativeauthenticator/blob/master/nativeauthenticator/handlers.py#L86), despite the method signature actually accepting these together as a dictionary (i.e. as a single positional parameter) (https://github.com/ml-tooling/nativeauthenticator/blob/master/nativeauthenticator/nativeauthenticator.py#L127).

Changing the function signature from def authenticate(self, handler, data): to def authenticate(self, handler, username, password): seems to address the issue, but I've noted this function hasn't been updated in the original repo from jupyterhub (https://github.com/jupyterhub/nativeauthenticator/blob/master/nativeauthenticator/nativeauthenticator.py#L127), and there are no issues I can find reporting this elsewhere, which I find somewhat surprising?

@raethlein - any thoughts? I can raise a PR with the changes I've described above if that would be useful, but I'm not very familiar with Tornado/Jupyterhub auth so wanted to sanity check this with someone first?

[cgddrd]

Update to my previous comments above:

• The offending line making the call to authenticate() (https://github.com/ml-tooling/nativeauthenticator/blob/master/nativeauthenticator/handlers.py#L86) doesn't exist in the original jupyterhub/nativeauthenticator (https://github.com/jupyterhub/nativeauthenticator/blob/master/nativeauthenticator/handlers.py#L67), so this explains why I can't replicate this in the standard Jupyterhub Docker image.

This must be needed for something specific to MLHub, but I'm unsure at this stage what this is).

I'll propose a modification and raise as a PR in ml-tooling/nativeauthenticator (https://github.com/ml-tooling/nativeauthenticator)

[raethlein]

Thanks for reporting the issue @Ledenel and thanks for looking into it @cgddrd!

In fact, the reported issue with authenticate() - while, of course, not nice - should actually be ignorable and should not cause the error that the admin user cannot login. @Ledenel have you added some configurations for the authenticator in your jupyterhub config file, e.g. c.Authenticator.admin_users?

@cgddrd The mentioned line was added in commit a5fcce293d4051112c84cf06a5d6fe0fbaff5f48 when I cherry-picked the changes from this pull request of the original repo. Though, after taking over the work-in-progress changes, I did not follow up on it.

[Ledenel]

@Ledenel, please can you confirm that you're not using the nativeauthenticator fork provided out-of-the-box with the mlhub Docker image? I've been able to replicate the error message you report from a fresh Docker pull/run, using all the default settings/components (MacOS Catalina, Docker Desktop 2.1.0.5, Tornado 5.1.1).

However, using the standard out-of-the-box nativeauthenticator provided by 'mlhub' (https://github.com/ml-tooling/nativeauthenticator), I am able to create a new user with the username admin and a password of my choosing (despite the exception being thrown). Instructions relating to this are provided with the README.md (https://github.com/ml-tooling/ml-hub#default-login).

Further investigation of this issue has found that the call to authenticate() actually passes the username and password as separate positional arguments (https://github.com/ml-tooling/nativeauthenticator/blob/master/nativeauthenticator/handlers.py#L86), despite the method signature actually accepting these together as a dictionary (i.e. as a single positional parameter) (https://github.com/ml-tooling/nativeauthenticator/blob/master/nativeauthenticator/nativeauthenticator.py#L127).

Changing the function signature from def authenticate(self, handler, data): to def authenticate(self, handler, username, password): seems to address the issue, but I've noted this function hasn't been updated in the original repo from jupyterhub (https://github.com/jupyterhub/nativeauthenticator/blob/master/nativeauthenticator/nativeauthenticator.py#L127), and there are no issues I can find reporting this elsewhere, which I find somewhat surprising?

@raethlein - any thoughts? I can raise a PR with the changes I've described above if that would be useful, but I'm not very familiar with Tornado/Jupyterhub auth so wanted to sanity check this with someone first?

Thanks for the reply. Actually I did almost nothing, just pull out the docker image and find it failed to create account admin. Extra try is not the point. So to my knowledge the only thing infect the NativeAuthenticator version is in the DockerFile.

Now I found the relavant part is: https://github.com/ml-tooling/ml-hub/blob/b96c316d67a8a183035496ccb05d92972e61f66a/Dockerfile#L96 commit hash matches with https://github.com/ml-tooling/nativeauthenticator/master.

[Ledenel]

Sorry my fault. Actually extra try is the key point, which leaves some old jupyterhub files I forgot to clean up. In another word, /data/ml-hub-data is actuall not empty, which caused that issue. Clean up the data folder solved the issue.

[raethlein]

Oh, yes then the admin account probably already existed with a password. Thanks for the update! I am going to close this issue then.