garyburgmann / drf-firebase-auth

Firebase backend to receive a user idToken and authenticate via Django REST Framework 'authentication.BaseAuthentication'. Optionally, a new local user can be created in the process.
MIT License
128 stars 62 forks source link

Calls to firebase_auth.get_user() can add 100-200ms to request time #28

Open keeth opened 3 years ago

keeth commented 3 years ago

Hi,

Just reviewing my web API performance and found high latency whenever firebase_auth.get_user() is called, since it sends a blocking HTTP request to the Firebase Auth API.

I've read a few different Python Firebase Auth tutorials and they all just use the decoded JWT to supply user data, they do not make a call out to the Firebase web API. This makes sense since a synchronous HTTP call is bad for performance.

If FIREBASE_CHECK_JWT_REVOKED is enabled, the get_user() call is made twice per request cycle. One of these calls is surely redundant.

Apart from revocation is there a reason to fetch the user rather than using the properties that are already embedded in the JWT?

Thanks!

garyburgmann commented 3 years ago

Hi @keeth ,

I will have to get my head into the context to be able to answer the why properly (and do be honest, remember what I did). I have a couple of weeks off over Easter, will look into it then. Watch this space ;)

Cheers

Longwelwind commented 3 years ago

I've looked a bit into this question, here are the result of my investigation, so that if someone is interested in solving this, s.he can have more info on this. At the moment, when a request is received by Django, FirebaseAuthentication:

The step in bold includes a request to Firebase.

What it truly should be doing is:

That way, a trip to Firebase is only needed during local user creation. In case the local user creation doesn't require data from the firebase user, a request to Firebase is not even needed.

I'll try to create a PR if I get the time to do this.

keeth commented 3 years ago

My solution was to always just use the token. For my purposes it has enough info even for account creation..

    def authenticate_token(self, decoded_token):
        return UserRecord(
            dict(
                localId=decoded_token["uid"],
                display_name=decoded_token.get("name"),
                email=decoded_token.get("email"),
                photo_url=decoded_token.get("picture"),
            )
        )
Longwelwind commented 3 years ago

The best would to let it the user choose whether they need to fetch the firebase user. firebase_auth.get_user() could never be called and api_settings.FIREBASE_USERNAME_MAPPING_FUNC(firebase_user) could be changed to only give the decoded_token as parameter. That way, the developer could choose themselves to call firebase_auth.get_user() if they need to.

garyburgmann commented 3 years ago

Hey guys this is still on my radar, just need to find time to get my head into it. Thanks for the feedback so far!

If there is some sort of consensus re: best way forward and I haven't gotten to it yet, feel free to make a pull req.

Have opened a ticket in Trello to track this, feel free to add code snippets/screenshots/more info

https://trello.com/c/RlMcBCmi