Errors when creating or editing groups due to expired auth cookie tickets #8914

seanh · 2024-08-30T22:18:24Z

A problem with the implementation of h's new API auth cookie (#8861) causes creating and editing groups to start failing after 7 days, requiring the user to log out and in again to fix it.

TLDR: Each auth cookie has an associated auth ticket in the DB. Both the cookie in the request and the ticket in the DB need to be present and valid for a request to be authenticated. The tickets in the DB are expiring, leaving the cookies unusable.

Steps to reproduce

First, you need to revert 87b8133. That commit reverts the API auth cookie feature, which fixes the issue. So you'll have to revert the revert before you can reproduce the issue:
```
git revert --no-commit 87b8133cef514abc10d2482de2665639b4f59203
```

Second, reduce the auth ticket TTL and refresh interval times, so that you don't have to wait around for weeks to reproduce the issue:

diff --git a/h/services/auth_ticket.py b/h/services/auth_ticket.py
index 59a8772d9..84351bbe1 100644
--- a/h/services/auth_ticket.py
+++ b/h/services/auth_ticket.py
@@ -6,12 +6,12 @@ from h.models import AuthTicket, User


 class AuthTicketService:
-    TICKET_TTL = timedelta(days=7)
+    TICKET_TTL = timedelta(minutes=1)

     # We only want to update the `expires` column when the tickets `expires` is
     # at least one minute smaller than the potential new value. This prevents
     # that we update the `expires` column on every single request.
-    TICKET_REFRESH_INTERVAL = timedelta(minutes=1)
+    TICKET_REFRESH_INTERVAL = timedelta(seconds=1)

     def __init__(self, session, user_service):
         self._session = session

Now, to reproduce the issue:
1. Log in and load http://localhost:5000/groups/new/
2. Wait 30 seconds, then reload the page. This will refresh the auth ticket for your HTML auth cookie but not your API auth cookie.
3. Wait another 30 seconds. The auth ticket for your API auth cookie will expire, but not the one for your HTML auth cookie (since you refreshed it, so it still has another 30s to live)
4. Try to create a group, you'll get an error.
5. Reload the page and try again, you'll keeping getting errors
6. Log out and log in again, you should now be able to create groups without errors (for the next 60s)

Explanation

When a user logs in we set two cookies: html_authcookie (used to authenticate HTML page loads) and api_authcookie (used to authenticate the frontend code's API requests to the create-group and edit-group APIs):

h/h/security/policy/_cookie.py

Lines 79 to 82 in 87b8133

    
           return [ 
        
               *self.helper.remember(self.html_authcookie, request, userid), 
        
               *self.helper.remember(self.api_authcookie, request, userid), 
        
           ]

We also create two AuthTicket's in the DB, one for each cookie:

h/h/security/policy/helpers.py

Lines 29 to 32 in 87b8133

    
           def remember(self, cookie: SignedCookieProfile, request: Request, userid: str): 
        
               ticket_id = AuthTicket.generate_ticket_id() 
        
               request.find_service(AuthTicketService).add_ticket(userid, ticket_id) 
        
               return cookie.get_headers([userid, ticket_id])

When a cookie-authenticated request is made we require both the cookie in the request and the cookie's corresponding auth ticket in the DB to be valid. Both CookiePolicy.identity() (for HTML page requests) and APICookiePolicy.identity() (for cookie-authenticated API requests) call AuthTicketCookieHelper.identity() which in turn calls AuthTicketService.verify_ticket() to verify the auth ticket in the DB.

Both cookies and tickets have limited lifetimes. HTML auth cookies live for 30 days and we never refresh them, so users get logged out of h after 30 days even if they have been continuously using the site. The corresponding auth ticket lives for only 7 days but every time a user makes an html_authcookie-authenticated request and AuthTicketService successfully verifies the cookie's ticket in the DB it resets the ticket's TTL to 7 days again. So actually if you don't visit h you will be logged out after 7 days. And even if you do visit h every 7 days, you'll still be logged out after 30 days.

API auth cookies live for 60 days but they also have corresponding auth tickets in the DB that live for only 7 days.

The problem is that loading any HTML page will refresh the html_authcookie's ticket, but it won't refresh the api_authcookie's ticket. Only making a cookie-authenticated API request will refresh the api_authcookie's ticket, and currently only creating or editing a group does that.

So after 7 days, if the user has been visiting h pages so their HTML cookie's ticket has been getting refreshed, but the user hasn't been creating or editing groups so their API cookie's ticket hasn't been refreshed, the user ends up with a valid HTML auth cookie but an invalid API auth cookie.

This means the user gets errors whenever they try to take an action that makes an api_authcookie-authenticated request (currently: creating or editing groups) and the user will be stuck in this situation until they log out and log in again (which will create a brand new API auth cookie with a fresh auth ticket).

The text was updated successfully, but these errors were encountered:

seanh · 2024-08-30T22:41:15Z

I think the fix may be to make each (html_authcookie, api_authcookie) pair share a single auth ticket in the DB. Then when an HTML request refreshes html_authcookie's ticket that's also refreshing api_authcookie's ticket since they're the same ticket. API requests refresh api_authcookie's ticket, so those would refresh the shared ticket too.

Alternatively stick with two separate tickets but make CookiePolicy (which is the security policy that authenticates html_authcookie's for HTML page requests) refresh both the html_authcookie and the api_authcookie's tickets. Both cookies are included in HTML page requests and CookiePolicy does have access to both cookies. This call to AuthTicketCookieHelper.identity(self.html_authcookie, ...) is what triggers the ticket refresh (because identifying the user from a cookie also refreshes the cookie's ticket as a side-effect, two things that should probably be separate method calls). If we refreshed both self.html_authcookie and self.api_authcookie in CookiePolicy.identity() I think that would fix the issue. But this would prevent us from restricting api_authcookie to API requests only, which is something we wanted to do, but that's probably fine: I don't think we really need to do that. Restricting api_authcookie to API requests only is currently also prevented by this code.

Another alternative: make auth tickets corresponding to API auth cookies have TTLs longer than the lifetimes of the cookies themselves, then the tickets wouldn't need to be refreshed (but we could still invalidate people's cookies by deleting the tickets from the DB, so the tickets wouldn't be pointless).

Yet another alternative: make API auth cookies not require auth tickets in the DB.

Make each HTML and API auth cookie pair share a single auth ticket so that it's not possible to get into a situation where the HTML auth cookie's ticket is still valid but the API auth cookie's ticket has expired. Fixes #8914. This commit also renames the API auth cookie so that existing API auth cookies in the wild, which have their own auth tickets that're separate from the auth tickets of the HTML auth cookies and that will have expired, will no longer be used and new API auth cookies with the new cookie name will be issued to these browsers.

Fixes #8914. This reverts commit 87b8133 and adds a change that should avoid ever getting into a situation where a browser has an HTML auth cookie with a valid auth ticket in the DB but has an API auth cookie whose auth ticket has expired: Each API auth cookie and its paired HTML auth cookie now share the same auth ticket in the DB. This is a second attempt at getting h to use two separate auth cookies: one for HTML page requests and another for JSON API requests. This enables the API auth cookie to be more secure: it can use `SameSite=Strict` rather than `Lax`, which gives better protection against CSRF attacks but would not be desirable for the HTML auth cookie (because whenever a user followed a link from another site to Hypothesis they would find themselves not authenticated). History: * An earlier attempt to introduce separate auth cookies was made in: #8861 * There was an issue that the API auth cookie and it's paired HTML auth cookie each had their own corresponding auth _ticket_ in the DB. The HTML auth cookie's ticket's expiry time would be reset each time an HTML page request was made, but the API auth cookie's ticket's expiry time would only be reset when a JSON API request was made (which is much rarer). As a result the API auth cookies would end up unusable because their auth tickets had expired, whilst the HTML auth cookies were still usable. This created a situation where the browser had a usable HTML auth cookie so it could load HTML pages and the user would appear to be logged in, but the browser did not have a usable API auth cookie so any API requests made by those HTML pages would fail and the user would see errors. See: #8914 * As a result of this issue the API auth cookie was "reverted" in #8913 (note: not an actual revert, just the minimal change necessary to make h once again use a single cookie for both HTML and API requests). * This commit now reintroduces the reverted change, plus a fix for the issue.

seanh self-assigned this Sep 6, 2024

seanh linked a pull request Sep 11, 2024 that will close this issue

Use a separate auth cookie for API requests (fixed version) #8946

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Errors when creating or editing groups due to expired auth cookie tickets #8914

Errors when creating or editing groups due to expired auth cookie tickets #8914

seanh commented Aug 30, 2024 •

edited

Loading

seanh commented Aug 30, 2024 •

edited

Loading

Errors when creating or editing groups due to expired auth cookie tickets #8914

Errors when creating or editing groups due to expired auth cookie tickets #8914

Comments

seanh commented Aug 30, 2024 • edited Loading

Steps to reproduce

Explanation

seanh commented Aug 30, 2024 • edited Loading

seanh commented Aug 30, 2024 •

edited

Loading

seanh commented Aug 30, 2024 •

edited

Loading