feat(auth): Add returnOobLink to the ActionCodeSettings

[Dal-Papa]

Discussion

https://github.com/firebase/firebase-admin-go/issues/501

Testing

• Make sure all existing tests in the repository pass after your change. ✅
• If you fixed a bug or added a feature, add a new test to cover your code. ✅

API Changes

• At this time we cannot accept changes that affect the public API. If you'd like to help us make Firebase APIs better, please propose your change in an issue so that we can discuss it together.

[Dal-Papa]

@lahirumaramba @hiranya911 👋

[lahirumaramba]

Hi @Dal-Papa Thank you for your patience on this. The Admin SDKs currently do not support sending email action links. Setting returnOobLink does not send the email, in-fact we already set it to true in the code below. https://github.com/firebase/firebase-admin-go/blob/23a1f17959238d989add41d562602b8bf04e71d6/auth/email_action_links.go#L119

If you are looking to generate email action links the SDK already supports that and you can use your own email API to send out the emails.

[Dal-Papa]

@lahirumaramba : Either you read the code wrong or you used it wrong but I was able to make Firebase send me an email with my version of the code. https://github.com/firebase/firebase-admin-go/blob/23a1f17959238d989add41d562602b8bf04e71d6/auth/email_action_links.go#L119 This is overwritten by the settings object so therefore it becomes false and hence it sends an email. Please consider this PR, we are using the fork in the meantime.

[prameshj]

Lahiru Maramba : Either you read the code wrong or you used it wrong but I was able to make Firebase send me an email with my version of the code.

https://github.com/firebase/firebase-admin-go/blob/23a1f17959238d989add41d562602b8bf04e71d6/auth/email_action_links.go#L119

This is overwritten by the settings object so therefore it becomes false and hence it sends an email. Please consider this PR, we are using the fork in the meantime.

Hi @Dal-Papa , just to clarify, you are exposing ReturnOObLink in ActionCodeSettings, so you can override to false, in case you want the email to be sent automatically, correct? That makes sense, however, I am curious about the use-case.. typically the email verification/password reset etc are triggered by the client, so maybe more suitable in the client SDKs? Can you explain how it is useful in admin SDKs? Happy to move forward on this, but would like to better understand the scenario. Thanks!

[Dal-Papa]

Lahiru Maramba : Either you read the code wrong or you used it wrong but I was able to make Firebase send me an email with my version of the code. https://github.com/firebase/firebase-admin-go/blob/23a1f17959238d989add41d562602b8bf04e71d6/auth/email_action_links.go#L119

This is overwritten by the settings object so therefore it becomes false and hence it sends an email. Please consider this PR, we are using the fork in the meantime.

Hi @Dal-Papa , just to clarify, you are exposing ReturnOObLink in ActionCodeSettings, so you can override to false, in case you want the email to be sent automatically, correct? That makes sense, however, I am curious about the use-case.. typically the email verification/password reset etc are triggered by the client, so maybe more suitable in the client SDKs? Can you explain how it is useful in admin SDKs? Happy to move forward on this, but would like to better understand the scenario. Thanks!

That's correct ! We do this so we can make a few checks on the account in the backend before sending such link (i.e. if the account isn't banned or if their email has been verified in the past).

[prameshj]

Lahiru Maramba : Either you read the code wrong or you used it wrong but I was able to make Firebase send me an email with my version of the code. https://github.com/firebase/firebase-admin-go/blob/23a1f17959238d989add41d562602b8bf04e71d6/auth/email_action_links.go#L119

This is overwritten by the settings object so therefore it becomes false and hence it sends an email. Please consider this PR, we are using the fork in the meantime.

Hi Clement DAL PALU , just to clarify, you are exposing ReturnOObLink in ActionCodeSettings, so you can override to false, in case you want the email to be sent automatically, correct? That makes sense, however, I am curious about the use-case.. typically the email verification/password reset etc are triggered by the client, so maybe more suitable in the client SDKs? Can you explain how it is useful in admin SDKs? Happy to move forward on this, but would like to better understand the scenario. Thanks!

That's correct ! We do this so we can make a few checks on the account in the backend before sending such link (i.e. if the account isn't banned or if their email has been verified in the past).

Thanks! Left a couple of comments in the unit test, LGTM otherwise.

[Dal-Papa]

Lahiru Maramba : Either you read the code wrong or you used it wrong but I was able to make Firebase send me an email with my version of the code. https://github.com/firebase/firebase-admin-go/blob/23a1f17959238d989add41d562602b8bf04e71d6/auth/email_action_links.go#L119

This is overwritten by the settings object so therefore it becomes false and hence it sends an email. Please consider this PR, we are using the fork in the meantime.

Hi Clement DAL PALU , just to clarify, you are exposing ReturnOObLink in ActionCodeSettings, so you can override to false, in case you want the email to be sent automatically, correct? That makes sense, however, I am curious about the use-case.. typically the email verification/password reset etc are triggered by the client, so maybe more suitable in the client SDKs? Can you explain how it is useful in admin SDKs? Happy to move forward on this, but would like to better understand the scenario. Thanks!

That's correct ! We do this so we can make a few checks on the account in the backend before sending such link (i.e. if the account isn't banned or if their email has been verified in the past).

Thanks! Left a couple of comments in the unit test, LGTM otherwise.

I went ahead and fixed all the tests that were using the settings, let me know if that's not what you had in mind.

[lahirumaramba]

@Dal-Papa Thank you for your contribution! I definitely did misunderstood the API in my initial comment, my apologies!

@prameshj I think we need an internal API proposal for the public API change before we can merge this. Thanks!

[Dal-Papa]

Hey @prameshj & @lahirumaramba ! I've fixed your comments and I've recently merged the latest changes, would you mind taking a look so we can merge this old PR ? 🙂

[drmaples]

any updates on when this will be merged?

[Dal-Papa]

@prameshj : I've updated to the latest upstream branch and fixed the comment. Please take a look.

[Dal-Papa]

@lahirumaramba : It looks like @prameshj is AWOL, can you please take over ?

[Dal-Papa]

@jonathanedey : You seem like someone who's still active here, can you please take a look ?