Skip to content

Conversation

@jspri
Copy link

@jspri jspri commented Mar 1, 2018

Fixes #46

Adds admin.auth().sendEmailVerification(uid). Tested it locally and it works great.

This could be made much nicer if the /getOobConfirmationCode endpoint was changed to optionally accept a localId (uid) instead of an idToken. This is how the other endpoints work. This would make the process use one less REST call and not require the use of createCustomToken which isn't supported by all environments.

Let me know if this is going in the right direction and I'll clean it up a bit and add tests (or you can).

@bojeil-google
Copy link
Contributor

bojeil-google commented Mar 1, 2018

Thank you for your interest in this feature. We really appreciate your contribution. For new APIs, you need to file a new feature requests.
We have an extensive internal API review process. This has to go through that review first.
https://firebase.google.com/support/contact/bugs-features/

That said, the API proposed is inefficient and combines admin and client endpoints. You are making multiple round trips for this feature. This is exactly what developers are already doing to support this via Admin SDK.

Thanks again for your effort. We plan to support this API with a dedicated endpoint. It is on our roadmap.

@bojeil-google
Copy link
Contributor

bojeil-google commented Mar 1, 2018

Just to clarify, no admin SDK endpoint should rely on an ID token to make an authorized call. That belongs on the client SDK. For Admin SDK a service account credential is enough to send the authorized call to send an email verification (without the same client side limitations). Client side calls are subject to throttling and different checks than admin endpoints. That is why we can't rely on internal implementations in the admin SDK with such restrictions.

So for example, a developer using this implementation to send multiple email verifications could end up getting rate limited as from the perspective of the Auth backend, this is coming from a client API.

@jspri
Copy link
Author

jspri commented Mar 1, 2018

Thanks for the response,

I agree that the implementation is a bit messy but do you think this could be handy as an interim solution because:

  1. Feature Request: Cloud Function should support user.sendEmailVerification() like client sdk #46 has been open for a long time
  2. The suggested solution involves including the client-sdk (fairly large) and has a number of steps. This strays from the simplicity most firebase services offer.
  3. When the endpoint is eventually implemented the transition will be seamless to the developer.

Perhaps you could mark the method as experimental? Just putting this out there because it took me a little while to get my head around all the steps and the difference between a custom token and an id token etc.

So for example, a developer using this implementation to send multiple email verifications could end up getting rate limited as from the perspective of the Auth backend, this is coming from a client API.

Would this only prevent you from sending a large number of verify emails to the same user? I expect that would be desired behaviour for most use cases.

@bojeil-google
Copy link
Contributor

You are free to use this solution on your own. In fact, right now, your options are limited and this is a good workaround.

However, we can't accept a hack in our official SDKs for various reasons.

As I explained earlier, we have a rigorous review process for new APIs. Even if I wanted to roll out a new API, I can't just simply do it.

@hiranya911
Copy link
Contributor

I think Auth backend servers will see this as initializing many login sessions from the same IP/device, which is a problem. I kind of like the idea of having this as an interim solution though (clearly marked as experimental in the documentation). But I'll side with @bojeil-google and the auth team's decision on this since they understand the implications of this far better than I do.

@bojeil-google
Copy link
Contributor

I can use this as a way to push harder for this feature. I totally understand the importance of having an easy way to send verifications via the admin SDK. We have several features in the pipeline that developers have been asking for for some time. You will be seeing them soon. Once we can get those out of the door, we can focus on this feature.

Once again, I think you can use this solution though don't be surprised if you get throttled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants