Recommended method of refreshing tokens

[cdriehuys]

I would like to implement a method of refreshing tokens from a single page app. The refresh itself seems pretty simple, but I was wondering what the best way of accessing a token's expiration time is. I was thinking about overriding the login view to return the expiration time as well, but the manager method to create new tokens only returns the token's key. Any input would be much appreciated.

[belugame]

I'm not sure if I understand your problem correctly. As you suggested I would go with overriding the post method of the LoginView and simply add 'expiry': token.expiry to the returned dictionary at https://github.com/James1345/django-rest-knox/blob/develop/knox/views.py#L23.

[cdriehuys]

Thanks for the response. However, the custom manger used for the AuthToken model only returns the token string as seen here.

[belugame]

Ah, should have read more carefully. I can't see a reason why create shouldn't return the AuthToken instance and it should not need much changes in the codebase either. So if you'd like to make a pull request...

Otherwise I would suggest just looking up the AuthToken record by the token string, the performance impact should be minimal.

[cdriehuys]

I messed around with the codebase a little, and it appears that a change like that is not so simple. For starters, the token generated here is not equivalent to the token_key property on the model generated later on in that method here. Since the token that is currently returned from the create method is necessary for authentication, it is crucial that the LoginView is able to return that token.

If you have any more ideas on how to get the same token value generated earlier in the function from an AuthToken instance, I would be happy to make a pull request.

Thanks for your time.

Edit: On second thought, this could be accomplished by returning a tuple of the form (authtoken, token) from the create method. My only issues with this are that it seems very unconventional to return two items from the create method, and it would be a breaking change for anyone who has overridden the LoginView.

[dhbradshaw]

What if create returned an authtoken object with a token property? The token property would be ephemeral, but could be accessed for the api.

This would bring the create method return value back to a more standard form while still allowing access to the undigested token.

[belugame]

close due to inactivity

[muelli]

We are currently using

class KnoxTokenAuthentication(TokenAuthentication):
    def authenticate(self, request):
        try:
            result = super().authenticate(request)
            if result:
                user, auth_token = result
                auth_token.expires = timezone.now() + knox_settings.TOKEN_TTL
                auth_token.save(update_fields=('expires',))
            return result
        except AuthenticationFailed:
            return None

in combination with a setting:

REST_FRAMEWORK = {
    'DEFAULT_AUTHENTICATION_CLASSES': (
        'KnoxTokenAuthentication',
    }
}

We believe it works to some extent. It seems to break sqlite though :( That's particularly annoying for testing.

[belugame]

Hey @muelli also in regards to your comment at #3 (comment). Yes, that is missing and I would love see your implementation in a fully working state.
Could you already replicate the sqlite issue in a unit test?

[muelli]

Not really. But I think I believe to know what the problem is.
In that naive implementation mentioned earlier, the token is refreshed on every request. On. Every. Request. If you fire multiple requests at once, Django attempts to write as often to the database. I am under the impression that sqlite does not deal with this workload very well. Also, I dare to say that we don't need sub-second precision in the auth-token.
So I came up with the following which works reasonably well (well, so far at least):

class KnoxTokenAuthentication(TokenAuthentication):
    def authenticate(self, request):
        try:
            result = super().authenticate(request)
            if result:
                user, auth_token = result
                current_expiry = auth_token.expires
                new_expiry = timezone.now() + knox_settings.TOKEN_TTL
                auth_token.expires = new_expiry
                # This is only papering over sqlite's poor parallel write handling
                # Once we think we don't need sqlite any more, we can as well get rid
                # of this guard.
                if (new_expiry - current_expiry).total_seconds() > 1:
                    log.info("Expiry of token %r: %s  Updating to: %s", auth_token, current_expiry, new_expiry)
                    auth_token.save(update_fields=('expires',))
            return result
        except AuthenticationFailed:
            return None

[belugame]

Hmm maybe we can think of some buffer mechanism to avoid writing to the db always but rather when the last request was x seconds in the past. But I would not want to introduce any 3rd party dependency for that. Well but first of all we would need a unit test to replicate the problem, I haven't experienced such problem with sqlite before.

[muelli]

simply issue multiple requests at once which cause writes to the DB.
Just to be clear: It's not a Knox specific thing. It's sqlite falling apart with concurrent writes.

And yeah, if Knox itself had a mechanism of updating the token's expiry, it could include a precision test as exampled in my snippet.

[rphlo]

To me, it makes more sense to put the logic of the expiration extension in the authenticate_credentials method.

class KnoxTokenAuthentication(TokenAuthentication):
    def authenticate_credentials(self, token):
        result = super().authenticate_credentials(token)
        if result:
            user, auth_token = result
            current_expiry = auth_token.expires
            new_expiry = timezone.now() + knox_settings.TOKEN_TTL
            auth_token.expires = new_expiry
            # Update once per second max
            if (new_expiry - current_expiry).total_seconds() > 1:
                auth_token.save(update_fields=('expires',))
        return result

[belugame]

will be released in 3.2.0, see #105 for some more unit tests. Feedback welcome!