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 CanvasOAuthenticator #406

Closed
wants to merge 2 commits into from
Closed

Conversation

yuvipanda
Copy link
Collaborator

Canvas is a very popular educational tool used to manage
classes in universities across the world. It has a very
rich API (https://canvas.instructure.com/doc/api/), and
deep integration with it makes a lot of things possible.

This Authenticator builds on top of GenericOAuthenticator,
but adds a few new features:

  • Strip domain names specific user emails to get userids. Works
    when all your Canvas users have an email for your educational
    institution.
  • Fetch list of courses the user is enrolled in, and what kind
    (student, TA, instructor, etc). This allows for finer
    grained access control, and modifying the user environment
    based on the courses they are enrolled in.
  • Ability to restrict user logins based on what courses they
    are enrolled in.

I've added some unit tests, and things seem to be working out
ok.

Canvas is a very popular educational tool used to manage
classes in universities across the world. It has a very
rich API (https://canvas.instructure.com/doc/api/), and
deep integration with it makes a lot of things possible.

This Authenticator builds on top of GenericOAuthenticator,
but adds a few new features:

- Strip domain names specific user emails to get userids. Works
  when all your Canvas users have an email for your educational
  institution.
- Fetch list of courses the user is enrolled in, and what kind
  (student, TA, instructor, etc). This allows for finer
  grained access control, and modifying the user environment
  based on the courses they are enrolled in.
- Ability to restrict user logins based on what courses they
  are enrolled in.

I've added some unit tests, and things seem to be working out
ok.
@manics
Copy link
Member

manics commented Jan 28, 2021

We've been telling other potential contributors to put new authenticators in their own repository, and have oauthenticator as a dependency. See for example:

How would you feel about moving this to it's own repo under the jupyterhub org, and linking to it from the oauthenticator README and docs as a prime example of how to derive and package your own authenticator?

@yuvipanda
Copy link
Collaborator Author

Ah, that makes sense, @manics. I started that route, but I couldn't figure out how to re-use the testing infrastructure here. Particularly around mocking. I couldn't import oauthenticator.tests. Any idea how I can get beyond that?

GET methods should generally not have a body.
Tornado's client doesn't allow it either.
@manics
Copy link
Member

manics commented Jan 28, 2021

I don't, but I've long wished for an example that we could point people to. Maybe we can use this as a driver to sort out the test infrastructure?

Jupyter Notebook packages some of their test infrastructure inside the python package, e.g. for testing ContentManagers:
https://jupyter-notebook.readthedocs.io/en/stable/extending/contents.html#testing

Could we do something similar?

@manics
Copy link
Member

manics commented Jan 28, 2021

This should let you do from oauthenticator import tests: #407

@manics
Copy link
Member

manics commented Apr 13, 2021

Can we close this, or are there further barriers to developing external authenticators?

@consideRatio
Copy link
Member

consideRatio commented May 14, 2021

@yuvipanda do you have a status update on this? Is it published externally somewhere for example?

An action point can be to document this external location like in #427

@yuvipanda
Copy link
Collaborator Author

i don't think i've put this up anywhere, I'll do that sometime in the next few weeks

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.

3 participants