Skip to content

Conversation

@aero31aero
Copy link
Member

Continuing from #16455 with @showell.

Testing Plan:

GIFs or Screenshots:

@aero31aero aero31aero force-pushed the 5-oauth branch 2 times, most recently from ff29d9b to 325ccbd Compare October 14, 2020 00:27
showell and others added 3 commits October 20, 2020 17:08
Right now we manually install django-oauth-toolkit.

This includes:

    - help Django find the relevant templates
      for the application view

    - add in relevant views from the toolkit

    - validate access tokens for Zulip views

TODO:

    We should lock down the views and only allow admin
    users to create applications, probably.

    And we should automatically install the toolkit.

    We should refine read/write scopes.  (For now we
    assume users just authorize with write scope.)

    We can clean up the template-related code (to
    de-duplicate the logic we use for the two-factor
    templates).

    And still lots else to do, like writing tests, etc.
@zulipbot
Copy link
Member

Heads up @aero31aero, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

@showell
Copy link
Contributor

showell commented Nov 29, 2020

Just to give a status of this:

  • this PR is a collaboration between me and Rohitt
  • we have tested it fairly extensively in practice
  • we would need to subsume some of the canned forms from the toolkit into oauth
  • we would need automated tests

But, overall, if anybody ever needs to take this over, they have a pretty solid foundation.

It's likely to have rebase conflicts, but they will generally just be around the pip/requirements stuff.

@edith007
Copy link
Contributor

@showell can i take this PR forward?

@showell
Copy link
Contributor

showell commented Jan 16, 2021

@edith007 Sure, give it a shot.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants