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

Improve docs for LoginRedirectIfUnauthenticated DRF permission #364

Open
robrap opened this issue Jul 25, 2023 · 0 comments
Open

Improve docs for LoginRedirectIfUnauthenticated DRF permission #364

robrap opened this issue Jul 25, 2023 · 0 comments

Comments

@robrap
Copy link
Contributor

robrap commented Jul 25, 2023

The LoginRedirectIfUnauthenticated DRF permission class was introduced while working on the Payment MFE transition. It requires a service to include the middleware JwtRedirectToLoginIfUnauthenticatedMiddleware to work its magic.

The class's current docstring:

class LoginRedirectIfUnauthenticated(IsAuthenticated):
    """
    A DRF permission class that will login redirect unauthorized users.

    It can be used to convert a plain Django view that was using @login_required
    into a DRF APIView, which is useful to enable our DRF JwtAuthentication class.

    Requires JwtRedirectToLoginIfUnauthenticatedMiddleware to work.

    """

The best description of why we introduced this complexity is detailed in this ecommerce ADR on the Payment MFE:

Additionally, the BasketAddItemsView was converted to a DRF API endpoint using the new
LoginRedirectIfUnauthenticated permission class. This enables /basket/add calls, which are
made before reaching the Payment MFE, to use JWT Authentication and avoid initiating the
old ecommerce SSO flow which is slow and unnecessary for the Payment MFE which uses
JWT Authentication.

Note: This endpoint has no associated UI, and simply redirects the user as appropriate. Therefore, the typical behavior of a DRF API, that simply returns a 4XX error, would not suffice.

Currently, this class is used in ecommerce, commerce-coordinator, and a private edx-org repo. The use case still exists, and I don't know a better solution at this point.

At this point, I think the best move is to improve the docstring(s) with notes from this ticket. Rather than just doing so, I've captured this ticket to remind myself of the purpose, and to convince myself that it is still needed and that I still can't think of a better alternative. This ticket can be linked to from the commit message of any doc improvement.

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

No branches or pull requests

1 participant