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

Add terms conditions to JupyterHub login page and update to latest JupyterHub version #104

Merged
merged 4 commits into from
Nov 18, 2020

Conversation

tlvu
Copy link
Collaborator

@tlvu tlvu commented Nov 18, 2020

User have to check the checkbox agreeing to the terms and conditions in order to login (fixes Ouranosinc/pavics-sdi#188).

User will have to accept the terms and conditions (the checkbox) each time he needs to login. However, if user do not logout or wipe his browser cookies, the next time he navigate to the login page, he'll just log right in, no password is asked so no terms and conditions to accept either.

This behavior is optional and only enabled if JUPYTER_LOGIN_TERMS_URL in env.local is set.

Had to patch the login.html template from jupyterhub docker image for this feature (PR jupyterhub/jupyterhub#3264).

Also update jupyterhub docker image to latest version.

Deployed to my test server https://lvupavics.ouranos.ca/jupyter/hub/login (pointing to a bogus terms and conditions link for now).

Tested on Firefox and Google Chrome.

Tested that upgrade from jupyterhub 1.0.0 to 1.2.1 is completely transparent to already logged in jupyter users.

[D 2020-11-18 19:53:52.517 JupyterHub app:2055] Verifying that lvu is running at http://172.18.0.3:8888/jupyter/user/lvu/                             
[D 2020-11-18 19:53:52.523 JupyterHub utils:220] Server at http://172.18.0.3:8888/jupyter/user/lvu/ responded with 302                                
[D 2020-11-18 19:53:52.523 JupyterHub _version:76] jupyterhub and jupyterhub-singleuser both on version 1.2.1                                         
[I 2020-11-18 19:53:52.524 JupyterHub app:2069] lvu still running    

recorded

Edit:

  • Add corresponding jupyterhub PR Add optional user agreement to login screen jupyterhub/jupyterhub#3264
  • Add "Tested on Firefox and Google Chrome."
  • Add "User will have to accept the terms and conditions (the checkbox) each time he needs to login. However, if user do not logout or wipe his browser cookies, the next time he navigate to the login page, he'll just log right in, no password is asked so no terms and conditions to accept either."
  • Add "Tested that upgrade from jupyterhub 1.0.0 to 1.2.1 is completely transparent to already logged in jupyter users."

User have to accept the terms and conditions in order to login.
No existing config deleted.

Config rename:

* `Authenticator.whitelist` renamed to `allowed_users`.

* `Authenticator.blacklist` renamed to `blocked_users`.

So no config needs updated.

The block user still work since we already knew the rename was coming so
we already use both the old and new name for forward-compat.

```
[W 2020-11-17 23:36:56.966 JupyterHub auth:487] User 'authtest' blocked. Stop authentication
[W 2020-11-17 23:36:56.967 JupyterHub base:752] Failed login for authtest
```
Copy link
Collaborator

@huard huard left a comment

Choose a reason for hiding this comment

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

Is this a one-time thing ? Or users will need to accept every time they login ?

@tlvu
Copy link
Collaborator Author

tlvu commented Nov 18, 2020

Is this a one-time thing ? Or users will need to accept every time they login ?

We do not keep which user accepted so every time they login.

However, if they do not signout or do not clear their cookies, next time they arrive to the page, they won't need to login.

Copy link
Collaborator

@dbyrns dbyrns left a comment

Choose a reason for hiding this comment

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

We will be able soon to add terms and conditions that must be accepted for joining groups too. Do you think it might be a good idea to prepare a directory of "terms and conditions" hosted by nginx to serve them? Or do you plan to host it elsewhere?

@tlvu
Copy link
Collaborator Author

tlvu commented Nov 18, 2020

We will be able soon to add terms and conditions that must be accepted for joining groups too.

Joining what group? Magpie group?

Do you think it might be a good idea to prepare a directory of "terms and conditions" hosted by nginx to serve them? Or do you plan to host it elsewhere?

So far we are just thinking a direct link to some .md file somewhere on github. So user can easily diff the changes as the terms and conditions evolve.

The go-live will have this option disabled since the .md file is not ready yet.

Fix minor bug the login-page.patch by testing this feature.
@tlvu tlvu merged commit 8be8eea into master Nov 18, 2020
@tlvu tlvu deleted the add-term-conditions-to-jupyterhub-login branch November 18, 2020 20:00
@tlvu
Copy link
Collaborator Author

tlvu commented Nov 25, 2020

@tlogan2000 @huard I got feedback from the corresponding PR I submitted on jupyterhub side (jupyterhub/jupyterhub#3264 (comment))

They suggested an alternate way to implement this. Basically instead of toggling disabling/enabling the submit button using javascript, we make the checkbox "required" so submit button is not disabled but will do nothing and recall user to check the checkbox.

recorded

I think it's better because:

  • no javascript required at all
  • user get a pop-up notifying them they need the checkbox

You guys okay with this change?

@tlogan2000
Copy link
Collaborator

Ok for me.

@dbyrns
Copy link
Collaborator

dbyrns commented Nov 25, 2020

Ok too.

tlvu added a commit that referenced this pull request Dec 14, 2020
…atch

jupyterhub: update to version 1.3.0 to include login terms patch

This version of jupyterhub includes the login terms patch originally
introduced in commit 8be8eea (PR #104).

This official login terms feature has a few enhancements (see jupyterhub/jupyterhub#3264 (comment)):

* no javascript dependency
* pop-up reminder for user to check the checkbox

Behavior change is the "Sign in" button is not longer disabled if
unchecked.  It simply does not work and reminds the user to check the
checkbox if unchecked.

Before:

![recorded](https://user-images.githubusercontent.com/11966697/99327962-1aa9ee80-2849-11eb-9ce8-3be6a1484e94.gif)

After:
![recorded](https://user-images.githubusercontent.com/11966697/100246404-18115e00-2f07-11eb-9061-d35434ace3aa.gif)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Method for accepting terms and conditions
5 participants