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

Allow users to reset their own passwords with First Use Authenticator #2

Closed
yuvipanda opened this Issue Sep 18, 2018 · 10 comments

Comments

Projects
None yet
3 participants
@yuvipanda
Copy link
Member

yuvipanda commented Sep 18, 2018

FirstUseAuthenticator is a JupyterHub authenticator where the first password a user uses is considered their password of record. This works great for simple use cases, but is problematic when users need to reset their own password.

For this task, make a GitHub Pull Request to the FirstUseAuthenticator github repo with a feature that lets users change their own passwords:

  1. Providing a web page at a custom URL - /auth/change-password. Users need to manually go to this URL to change their passwords.
  2. A text box for users to enter new password. Another for them to type it again so it can be confirmed.
  3. A 'confirm change' button that when pressed, sets it as the new user password

Helpful links:

  1. Tornado tutorial: http://www.tornadoweb.org/en/stable/
  2. https://jupyterhub.readthedocs.io/en/stable/reference/authenticators.html#how-to-write-a-custom-authenticator has information on how authenticators work
  3. https://github.com/yuvipanda/jupyterhub-simplespawner is a useful spawner class to test authenticator implementations
  4. https://github.com/jupyterhub/oauthenticator/blob/master/oauthenticator/globus.py#L223 example of adding custom web pages to authenticators

Getting Help:

Comment on this issue, or ping us on Gitter for help!

@leportella

This comment has been minimized.

Copy link

leportella commented Sep 21, 2018

Hi! I am an outreachy applicant and I would like to work on this :)

@minrk

This comment has been minimized.

Copy link
Member

minrk commented Sep 24, 2018

@leportella great! Feel free to ask guidance questions here or on gitter.

@leportella

This comment has been minimized.

Copy link

leportella commented Sep 25, 2018

Hi guys! I started the implementation:

  • I created a ResetPasswordHandler that inherits from BaseHandler and it returns a template called reset.html
  • I added this handler and the url /auth/change-password in the method get_handlers as shown by the example
  • I created (on the FirstUseAuthenticator) a function called reset_password that changes the passoword on the db based on the input

I'm having some doubts about:

  • I created the template on a fork of the Jupyterhub project, on the folder shared/jupyterhub/templates and installed the project locally. I'm pretty sure that this is not the best approach, but it was the easiest one I could think about. Where this template should be stored?

  • I understand how I could gather the handler and the authenticator, but I'm not quite sure how can I pass information from the input that comes from the handler form to the authenticator.

  • Since I don't know how to pass information from the form to the authenticator, I'm not sure if the reset_password method is best to stay in the authenticator or if it can remain on the handler.

The code I did is on this repo/branch and more specifically on this commit

Thank you for your help!

@minrk

This comment has been minimized.

Copy link
Member

minrk commented Sep 25, 2018

That sounds like great progress!

You are right that modifying jupyterhub/jupyterhub probably isn't the right approach, but the principles ought to be the same, it's just a bit of a question about where the code lands. In this case, it should all live in firstuseauthenticator.

Where this template should be stored?

I would put it in firstuseauthenticator/templates/reset.html (templates would be a sibling of firstuseauthenticator.py)

Jinja has the ability to load templates from multiple sources. One way to do that is to register your path with the jinja environment. We can then add this by creating a ChoiceLoader, which takes one or more Loaders and iterates through them until it finds a match:

import os
from jinja2 import ChoiceLoader, FileSystemLoader

# a 'templates' directory next to the current file
TEMPLATE_DIR = os.path.join(os.path.dirname(__file__), "templates")


class ResetPasswordHandler(BaseHandler):
    """Render the reset password page."""

    # flag to indicate whether the template path has been loaded
    _loaded = False

    def _register_template_path(self):
        if ResetPasswordHandler._loaded:
            # already loaded, do nothing
            return
        # create a FileSystemLoader that will find our template
        self.log.debug("Adding %s to template path", TEMPLATE_DIR)
        my_loader = FileSystemLoader([TEMPLATE_DIR])

        # register our loader with the jinja environment
        # *in addition to* the existing loaders
        env = self.settings['jinja2_env']
        previous_loader = env.loader
        env.loader = ChoiceLoader([previous_loader, my_loader])

        # signal that this modification should only occur once
        ResetPasswordHandler._loaded = True


    @web.authenticated
    async def get(self):
        # make sure our template is registered
        self._register_template_path()
        ...

I'm not quite sure how can I pass information from the input that comes from the handler form to the authenticator.

This would generally be in the form of a POST request coming from a <form>. There's an example in JupyterHub on the login page, where the form is defined here and the resulting POST request is handled by adding a def post method here. The URL for the post is defined by the action attribute here. If you don't specify action at all, then it will default to the current page, which is probably what you want (define get and post methods both on the same Handler class).

I'm not sure if the reset_password method is best to stay in the authenticator or if it can remain on the handler.

In practice, it usually doesn't matter too much, but as a general guideline in JupyterHub and web applications in general, I like to get as much logic implemented in portable classes (Authenticator, Spawner, etc.) so that they can be used and tested without a web application. Then the only job of the Handler is to expose the APIs on those objects via the web.

@leportella

This comment has been minimized.

Copy link

leportella commented Sep 25, 2018

@minrk thank you for your comments. They were extremely helpful!

I created a Pull Request but @yuvipanda comments puts a link for testing the Authenticator. Since no tests were available, I created a PR without it and came here to ask for help on this subject. Should this be done with tests? Should I created tests for everything?

Again, thank you both for your help :)

@minrk

This comment has been minimized.

Copy link
Member

minrk commented Sep 26, 2018

@leportella If you'd like to take a stab at adding tests, that would be awesome, but since we haven't set up the initial testing on that repo, I don't think we can reasonably ask you to do that right now. I've started the basics in jupyterhub/firstuseauthenticator#9, which should make it doable to add a test for the .reset_password method. Testing the handler might be more tricky, since you'll need to do either extensive mocking or instantiate JupyterHub in the tests.

@yuvipanda

This comment has been minimized.

Copy link
Member

yuvipanda commented Oct 1, 2018

@leportella I'd like you to add some docs to firstuseauthenticator about this, but as @minrk said I think you can skip adding tests for now.

Congratulations on getting the PR merged!

@leportella

This comment has been minimized.

Copy link

leportella commented Oct 2, 2018

I will add it to the docs as well :) I also saw an issue about name sanitizing that I could do, as well, but I would like your thoughts on that :)

@leportella

This comment has been minimized.

Copy link

leportella commented Oct 2, 2018

@yuvipanda I added a question on FAQ to serve as docs. If you prefer to do it in some other way, please let me know.

@yuvipanda

This comment has been minimized.

Copy link
Member

yuvipanda commented Oct 2, 2018

Awesome, @leportella! I'll call this task as done!

Congratulations :)

@yuvipanda yuvipanda closed this Oct 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment