feat: Support cookie authentication

[khvn26]

Thanks for submitting a PR! Please check the boxes below:

• I have added information to docs/ if required so people know about the feature!
• I have filled in the "Changes" section below?
• I have filled in the "How did you test this code" section below?
• I have used a Conventional Commit title for this Pull Request

Changes

This adds a basic JWT cookie support. As long as COOKIE_AUTH_ENABLED setting is set to True:

• The HTTP-only cookie is returned from registration and login endpoints. It then can be used to authenticate requests to endpoints that expect a token.
• The classic token is not being returned from registration and login endpoints. Login endpoint produces a 204 response.
• The CORS setting defaults are modified to allow credentials and the FE domain origin. All of the CORS settings are still settable individually for special cases / local development.

The JWT lifetime is set to be 10 hours by default. No refresh endpoints have been implemented at this point; when we do implement refreshing, token lifetime should be shortened significantly.

It's possible to invalidate the cookie via a POST /api/v1/auth/logout/ request. The token is added to blacklist and the cookie is unset.

A signal handler is added to set Access-Control-Allow-Origin based on get_current_site_url utility function. The function is modified to properly use the Django sites framework cache.

How did you test this code?

Added an integration test.

BE (including SAML) + FE has been tested in staging.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

3 Skipped Deployments

Name	Status	Preview	Comments	Updated (UTC)
docs	⬜️ Ignored (Inspect)	Visit Preview		Oct 3, 2024 11:18am
flagsmith-frontend-preview	⬜️ Ignored (Inspect)	Visit Preview		Oct 3, 2024 11:18am
flagsmith-frontend-staging	⬜️ Ignored (Inspect)	Visit Preview		Oct 3, 2024 11:18am

[github-actions]

Docker builds report

Image	Build Status	Security report
ghcr.io/flagsmith/flagsmith-e2e:pr-4662	Finished ✅	Skipped
ghcr.io/flagsmith/flagsmith-api-test:pr-4662	Finished ✅	Skipped
ghcr.io/flagsmith/flagsmith-frontend:pr-4662	Finished ✅	Results ✅
ghcr.io/flagsmith/flagsmith-api:pr-4662	Finished ✅	Results ✅
ghcr.io/flagsmith/flagsmith:pr-4662	Finished ✅	Results ✅
ghcr.io/flagsmith/flagsmith-private-cloud:pr-4662	Finished ✅	Results ✅

[github-actions]

Uffizzi Preview deployment-56666 was deleted.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.20%. Comparing base (4bca509) to head (c0839dc).
Report is 13 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4662      +/-   ##
==========================================
+ Coverage   97.19%   97.20%   +0.01%     
==========================================
  Files        1166     1171       +5     
  Lines       40436    40640     +204     
==========================================
+ Hits        39301    39506     +205     
+ Misses       1135     1134       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[matthewelwell]

So, I guess what you're saying in other conversations is that TokenDestroyView does nothing, is that right? I guess I'm concerned about a couple of things here:

1. Are we removing functionality here for regular tokens? I assume not, but just checking.
2. Shouldn't we just have a Logout view class where we can internalise the conditional logic depending on the setting?

[khvn26]

TokenDestroyView, in fact, does remove the token. However, the endpoint has never been used by the frontend; to log the user out, the application simply removed the t cookie.

Now that I expect the endpoint to be actually used, I ultimately decided to remove function related to classic tokens and only implement the JWT logout.

I have renamed the view and added a docstring to hopefully be more clear.

[matthewelwell]

Maybe the FE never used this functionality, but that's not to say that it's never been used by someone integrating with the API. It's obviously very unlikely, but not totally out of the question since we only provided API access via these user tokens for a long time.

I don't see the harm in either:

1. Reverting to what you had initially and subclassing TokenDestroyView and calling super()
2. Having 2 separate views that we add into urlpatterns depending on the existence of the environment variable.

[khvn26]

but that's not to say that it's never been used by someone integrating with the API

They will still have the DELETE /api/v1/auth/token/ endpoint that serves this exact function, and arguably has better semantics as we do not delete classic tokens on logout.

I can add the notion to OpenAPI docs.

[matthewelwell]

They will still have the DELETE /api/v1/auth/token/ endpoint that serves this exact function, and arguably has better semantics as we do not delete classic tokens on logout.

But it's technically a breaking API change, right? We're removing functionality from an existing endpoint, that existed, and was documented for people to use.

[khvn26]

It seems you are correct here:

Still, I'd prefer to deprecate this functionality and direct people to DELETE /api/v1/auth/token/ (as it will result in "logout" too).

[matthewelwell]

We can't just remove it though. I don't mind adding a warning that it'll be deprecated in the next version (and let's add it to the V2 API issue).

[khvn26]

The problem is that it's being called in FE now. We need to either add conditional logic to FE or explain to users that their tokens get invalidated if they logout explicitly. The latter is fine by me as using an admin API key is the clear alternative.

[matthewelwell]

their tokens get invalidated if they login explicitly

I assume here you meant to say 'logout explicitly' ?

It doesn't seem like it'd be hard to add conditional logic to the frontend to only call the logout endpoint if the jwt cookie exists, and seems to be the right thing to do to maintain compatibility, but I can't speak for @kyle-ssg here.

Alternatively, we do what you suggested, and just show a warning when the user hits the logout button to say that any direct API integrations they have using the user token will stop functioning.

Another alternative (that doesn't involve any more FE work) is still to conditionally change the view class used by the logout route depending on the existence of environment variable, but we may then need to think about whether we can actually run both in parallel.

[khvn26]

As discussed offline, the FE conditional logic was added and token deletion function restored in the BE.

[matthewelwell]

Approving with one minor comment

[matthewelwell]

... is the semi-colon here some horrible JS syntax that I'm not aware of, or a typo? @kyle-ssg