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

Indicator for admins about registrations pending approval #127

Open
lambdaTotoro opened this issue Nov 26, 2020 · 5 comments
Open

Indicator for admins about registrations pending approval #127

lambdaTotoro opened this issue Nov 26, 2020 · 5 comments
Labels
enhancement New feature or request
Milestone

Comments

@lambdaTotoro
Copy link
Collaborator

I've run into the following problem several times:
During the first few weeks of a semester, loads of people sign up for our JupyterHub instance and I can check the authorization page regularly to approve them easily. But as soon as this initial flood dies down, I often miss people who come in later and they have to wait very long to be approved because I don't check the authorization page often any more.

Proposed change:

It would be great to have some sort of indicator displayed to admin users if there are pending requests for authorization, maybe even how many there currently are. This information should not be visible to the normal user.

I think it should be possible to work within the existing templating mechanism (cf. #79 for necessary changes for using templates this way) to achieve this, similar to how announcements work.

@lambdaTotoro lambdaTotoro added the enhancement New feature or request label Nov 26, 2020
@matsievskiysv
Copy link

Maybe even allow configuring email notifications?

@lambdaTotoro
Copy link
Collaborator Author

I have merged the pull request in question, but displaying things only to admins seems to be not as trivial as I thought.
Maybe we even need support from the JupyterHub side of things. I have pinged @consideRatio and he will look into it when he has the time, but for now I think we shouldn't delay the 1.0 release on account of this.

@lambdaTotoro lambdaTotoro added this to the Version 1.1 milestone Oct 18, 2021
@lambdaTotoro
Copy link
Collaborator Author

@consideRatio, do you think admin-only announcements (or something similar) are something that's achievable in the near future? Otherwise, I'd bump this onto a later release.

@consideRatio
Copy link
Member

consideRatio commented Nov 2, 2021

I'm not sure. I think ideally, we would have a (3) or something next to the title Authorization area to indicate there are three pending requests.

The visual customization is trivial, but is the passing of the dynamic information about the amount of pending requests to the jinja template trivial? I'm don't think so.

We can easily customize how things look within the pages we explicitly render ourselves. This is how we would do it.

        html = await self.render_template(
            "signup.html",
            ask_email=self.authenticator.ask_email_on_signup,
            two_factor_auth=self.authenticator.allow_2fa,
            recaptcha_key=self.authenticator.recaptcha_key,
            tos=self.authenticator.tos,
        )

But, we are not the people calling .render_template() on the misc pages that makes the top meny render. So, we can't inject variables to them to my knowledge.

@consideRatio
Copy link
Member

consideRatio commented Nov 2, 2021

Discussion about this extracted from Gitter:

@minrk

Yes, the tornado setting template_vars populates the default template namespace.
Used on all pages

@consideRatio

Okay hmmm, but can we invoke logic to update that as part of a request to for example /hub/home ?

@minrk

Ah, changing over time is tricky, but technically that template_vars dict is mutable.
So wherever it is that the number changes, you could update it in template_vars
Or, probably better, put the number in the state of an object, and put that object in template_vars


My conclusion is that this is a bit tricky, and I'm not sure how to go about it. I'm not very happy if it becomes a very hacky trick that becomes hard to maintain.

But, perhaps if something like this would work, I think it would be an acceptable solution.

  1. When nativeauthenticator loads, we initialize some singleton nativeauth state object, and configure tornado settings template_vars to reference that object.
  2. We initialize the singleton nativeauth state object with the amount of pending requests etc
  3. When some state changes that we want to track, we update the singleton nativeauth state object
  4. We make our extended Page.html template render based on the template_vars object we have now referenced to include a (X) to indicate the number of pending approvals to manage in the authorization area.

@minrk

They can be callables, they don't need to be scalars. I don't know how your state is tracked, but if you have a function that can count them, that should be enough

@consideRatio

The template_vars can be callable functions?

@lambdaTotoro

We already have a function that counts them.

def get_unauthed_amount(self):

@minrk

The template_vars can be callable functions?

Yes, any Python object. You can call methods and such in jinja

The only tricky thing then is perhaps the ordering of config so that you can get an Authenticator method into the template namespace, since the Authenticator instance isn't available when a config file is evaluated.

@consideRatio

Can template_vars be set via nativeauth instance constructor?
(does it have a reference to be able to do so?)

@minrk

I'm not sure if it has a handle on the objects it needs, but maybe. This is reaching around the intended isolations of responsibilities quite a bit.

@consideRatio

Yes :(

@minrk

but

JupyterHub.instance().tornado_settings.setdefault("template_vars", {})["authenticator"] = self

probably works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants