GodotNuts / GodotFirebase

Implementations of Firebase for Godot using GDScript
MIT License
554 stars 79 forks source link

[FEATURE REQUEST] Support Custom Tokens for Authentication #191

Closed WolfgangSenff closed 3 years ago

WolfgangSenff commented 3 years ago

Is your feature request related to a problem? Please describe. We do not currently support custom tokens when using Firebase Authentication, and it would be nice to do so.

Describe the solution you'd like We should be able to support them and without too much difficulty.

Describe alternatives you've considered None really - just a missing feature of the Firebase plugin itself.

Additional context See discussion of feature here: https://github.com/GodotNuts/GodotFirebase/discussions/189

mars3142 commented 3 years ago

You can find a sample for REST API using the custom token as login: https://zemke.io/firebase-custom-token-rest-auth

PS: The custom token can't be created within the client. You need a server for that, because you need the Firebase admin SDK.

fenix-hub commented 3 years ago

I'll consider this closed after #192 and no other feebdack received. Reopen this if needed.

mars3142 commented 3 years ago

I commented within the pull request (#192), because I just did a testing for this features.

mars3142 commented 3 years ago

@fenix-hub I need to reopen this issue, because the signal is not emitted. As you can see, it jumps into the else branch and did not send any signal. Should I create a PR or will you change it?

godot_custom_token_response

But I don't know if this should be the correct signal, because in "res" are following properties: kind, idToken, refreshToken, expiresIn and isNewUser. As far, as I understand you cleanup the res data and put it into the auth object. But want is in auth with a email login? I believe the generated uuid (which is in my case the chosen username), so we didn't need it within the auth object.

PS: If you need a test project, which created you a custom token, contact me. I can give you access to my API for testing purpose.

fenix-hub commented 3 years ago

@mars3142 , A test project would be very useful, but actually I think I might already know what needs to be fixed. Basically to write the function my only way was to follow the documentation, but the documentation itself doesn't provide the structure of the response, so I thought that the response didn't have the kind field. Now that I know thanks to you, we should just treat the new function's logic as any other function that has the same key in the response. 1) Remove lines 269-270 and eventually 54 since will not be used 2) Add a new res.kind value as a new const variable

(line 26) 
const RESPONSE_CUSTOM_TOKEN : String = "identitytoolkit#VerifyCustomTokenResponse"

3) Add the new value in the match structure in order to let it handle automatically everything

(line 278)
RESONSE_SIGNIN, RESPONSE_ASSERTION, RESPONSE_CUSTOM_TOKEN

That should do the trick. Might need something more, but without testing it and with a quick view everything seems like it should work like that out of the box. of course this logic applies if you want to use the login_succeeded signal as the signal emitted with a successful username/password login, as all other login methods currently do.

mars3142 commented 3 years ago

Would you like to change that, so I can test it or should I try it first local on my machine?

We also need to check if the refresh token logic will work after that, but as you wrote, it should because it's nearly the same structure.

fenix-hub commented 3 years ago

yes for sure, will be a dirty way since I can't open the project on this machine. I'll leave you the tests :P

fenix-hub commented 3 years ago

@mars3142 you can check out the new branch

mars3142 commented 3 years ago

I checked the new branch. My findings are:

The only weird part, but this is depending on the Google API: the auth object is different for login and after refresh.

login:

auth_after_login

after refresh:

auth_after_refresh

I also tested get_user_data() which didn't work, because of a different data structure:

get_user_data

PS: localId is my username; lastLoginAt and createdAt are int like 1621952116019; customAuth is a boolean = true and lastRefreshAt is a timestamp like 2021-05-25T14:15:16.019Z.

fenix-hub commented 3 years ago

@mars3142 About the payload being different between the first request and the refresh, as you alread know we can't do much about that, but that's not a big issue in our plugin. About the get_user_data() after the refresh, as far as the signal is emitted correctly and the payload is not an error (regarding permissions), the thing seems to work as intended. Do you have any other specific issue or something that is not working for you? Do you want to share with me a test project so I can further investigate?

mars3142 commented 3 years ago

You unterstand the issue wrong. I can call get_user_data() after login, but it will crash, because the dictionary has other values as you expect. So the signal userdata_received will never emit. I don't know every auth method, so I just checked login/refresh and getuserdata.

I'll think, I create a fresh github project for you and add you as a contributor, so you can use this as a starting point for tests and fixing issues with custom tokens.

I'll come back to you soon.

fenix-hub commented 3 years ago

Ok thank you very much, I'll wait to be added to your project so I can check the different payloads with those signals.

mars3142 commented 3 years ago

Done. I've added you and included the latest addon commit (with custom token) in the test project.

fenix-hub commented 3 years ago

Thanks! I'll check it out later today and will let you know :)