Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(auth): Add returnOobLink to the ActionCodeSettings #502

Open
wants to merge 33 commits into
base: dev
Choose a base branch
from
Open

feat(auth): Add returnOobLink to the ActionCodeSettings #502

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
54b8114
Merge dev into master
google-oss-bot May 21, 2020
cef91ac
Merge dev into master
google-oss-bot Jun 16, 2020
77177c7
Merge dev into master
google-oss-bot Oct 22, 2020
a957589
Merge dev into master
google-oss-bot Jan 28, 2021
eb0d2a0
Merge dev into master
google-oss-bot Mar 24, 2021
05378ef
Merge dev into master
google-oss-bot Mar 29, 2021
4121c50
Merge dev into master
google-oss-bot Apr 14, 2021
928b104
Merge dev into master
google-oss-bot Jun 2, 2021
02cde4f
Merge dev into master
google-oss-bot Nov 4, 2021
6b40682
Merge dev into master
google-oss-bot Dec 15, 2021
e60757f
Merge dev into master
google-oss-bot Jan 20, 2022
bb055ed
Merge dev into master
google-oss-bot Apr 6, 2022
e8dfc32
Add returnOobLink to the ActionCodeSettings
Dal-Papa Jun 27, 2022
23a1f17
Merge dev into master
google-oss-bot Oct 6, 2022
1d24577
Merge dev into master
google-oss-bot Nov 10, 2022
f9dc53b
Merge remote-tracking branch 'upstream/dev'
Dal-Papa Nov 14, 2022
12a4788
Fix all tests with both returnOobLink cases
Dal-Papa Nov 15, 2022
4cf413a
Change `ReturnOobLink` into `SendEmailLink`
Dal-Papa Nov 23, 2022
54af579
Avoid name conflict with url package
Dal-Papa Nov 23, 2022
981442d
Add an unset test case and a comment
Dal-Papa Mar 8, 2023
61c6c04
Merge dev into master
google-oss-bot Apr 6, 2023
32af2b8
[chore] Release 4.12.0 (#561)
lahirumaramba Jun 22, 2023
02300a8
Revert "[chore] Release 4.12.0 (#561)" (#565)
lahirumaramba Jul 11, 2023
74c9bd5
Merge dev into master
google-oss-bot Jul 12, 2023
37c7936
Merge dev into master
google-oss-bot Sep 25, 2023
bef15cf
Merge remote-tracking branch 'fruitz-fork/master'
Dal-Papa Oct 13, 2023
b28ccfb
Merge remote-tracking branch 'upstream/dev'
Dal-Papa Oct 13, 2023
b04387e
Merge dev into master
google-oss-bot Nov 23, 2023
c348341
Merge remote-tracking branch 'upstream/master'
Dal-Papa Mar 4, 2024
da18d13
Fix comment
Dal-Papa Mar 4, 2024
77f51b8
Merge branch 'dev'
Dal-Papa Mar 6, 2024
147a277
Merge branch 'dev'
Dal-Papa Mar 29, 2024
e5a5134
Resolve conflicts on tests
Dal-Papa Mar 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions auth/email_action_links.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,16 @@ type ActionCodeSettings struct {
AndroidMinimumVersion string `json:"androidMinimumVersion,omitempty"`
AndroidInstallApp bool `json:"androidInstallApp,omitempty"`
DynamicLinkDomain string `json:"dynamicLinkDomain,omitempty"`
SendEmailLink bool `json:"-"`
}

func (settings *ActionCodeSettings) toMap() (map[string]interface{}, error) {
if settings.URL == "" {
return nil, errors.New("URL must not be empty")
}

url, err := url.Parse(settings.URL)
if err != nil || url.Scheme == "" || url.Host == "" {
parsedURL, err := url.Parse(settings.URL)
if err != nil || parsedURL.Scheme == "" || parsedURL.Host == "" {
return nil, fmt.Errorf("malformed url string: %q", settings.URL)
}

Expand All @@ -58,6 +59,7 @@ func (settings *ActionCodeSettings) toMap() (map[string]interface{}, error) {
if err := json.Unmarshal(b, &result); err != nil {
return nil, err
}
result["returnOobLink"] = !settings.SendEmailLink
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a comment here like : "Since SendEmailLink defaults to true, we will always set "returnOobLink" to false by default, this means no email is sent out in the defautl case".

And we can remove the "returnOobLink" : true setting in line 121?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this after several weeks - can you remind me how SendEmailLink defaults to true? I think the comment should be:

"Since SendEmailLink defaults to false, we will always set "returnOobLink" to true by default, this means no email is sent out in the default case".

Apologies for the mistake in my earlier comment.

return result, nil
}

Expand Down
140 changes: 88 additions & 52 deletions auth/email_action_links_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,24 +111,30 @@ func TestEmailVerificationLinkWithSettings(t *testing.T) {
s := echoServer(testActionLinkResponse, t)
defer s.Close()

link, err := s.Client.EmailVerificationLinkWithSettings(context.Background(), testEmail, testActionCodeSettings)
if err != nil {
t.Fatal(err)
}
if link != testActionLink {
t.Errorf("EmailVerificationLinkWithSettings() = %q; want = %q", link, testActionLink)
}
cases := []bool{true, false}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we can make this a list of testActionCodeSettings and testActionCodeSettingsMap instead, so we also test the "unset" case. Added some sample here, to illustrate what i mean.. (I did not verify that this compiles)

sendEmailLinkValues := []string{"true", "false", "unset"}
testActionCodeSettingsCases := []ActionCodeSettings{}
testActionCodeSettingsMapCases := []map[string]interface{}{}
for _, val := range sendEmailLinkValues {
  settings, settingsMap:= getCopiesOfTestSettings(testActionCodeSettings,
		testActionCodeSettingsMap)
  switch(val) {
  case "true":
    settings.SendEmailLink = true;
    settingsMap["returnOobLink"] = false;
  case "false":
     settings.SendEmailLink = false;
     settingsMap["returnOobLink"] = true;
  case "unset":
  }
  testActionCodeSettingsCases = append(testActionCodeSettingsCases, settings)
 testActionCodeSettingsMapCases = append(testActionCodeSettingsMapCases, settingsMap)
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

testActionCodeSettingsCustom, testActionCodeSettingsMapCustom := getCopiesOfTestSettings(testActionCodeSettings,
testActionCodeSettingsMap)
for _, returnOobLink := range cases {
testActionCodeSettingsCustom.SendEmailLink = !returnOobLink
testActionCodeSettingsMapCustom["returnOobLink"] = returnOobLink
link, err := s.Client.EmailVerificationLinkWithSettings(context.Background(), testEmail, testActionCodeSettingsCustom)
if err != nil {
t.Fatal(err)
}
if link != testActionLink {
t.Errorf("EmailVerificationLinkWithSettings() = %q; want = %q", link, testActionLink)
}

want := map[string]interface{}{
"requestType": "VERIFY_EMAIL",
"email": testEmail,
"returnOobLink": true,
}
for k, v := range testActionCodeSettingsMap {
want[k] = v
}
if err := checkActionLinkRequest(want, s); err != nil {
t.Fatalf("EmailVerificationLinkWithSettings() %v", err)
want := map[string]interface{}{
"requestType": "VERIFY_EMAIL",
"email": testEmail,
}
for k, v := range testActionCodeSettingsMapCustom {
want[k] = v
}
if err := checkActionLinkRequest(want, s); err != nil {
t.Fatalf("EmailVerificationLinkWithSettings() %v", err)
}
}
}

Expand Down Expand Up @@ -158,24 +164,30 @@ func TestPasswordResetLinkWithSettings(t *testing.T) {
s := echoServer(testActionLinkResponse, t)
defer s.Close()

link, err := s.Client.PasswordResetLinkWithSettings(context.Background(), testEmail, testActionCodeSettings)
if err != nil {
t.Fatal(err)
}
if link != testActionLink {
t.Errorf("PasswordResetLinkWithSettings() = %q; want = %q", link, testActionLink)
}
cases := []bool{true, false}
testActionCodeSettingsCustom, testActionCodeSettingsMapCustom := getCopiesOfTestSettings(testActionCodeSettings,
testActionCodeSettingsMap)
for _, returnOobLink := range cases {
testActionCodeSettingsCustom.SendEmailLink = !returnOobLink
testActionCodeSettingsMapCustom["returnOobLink"] = returnOobLink
link, err := s.Client.PasswordResetLinkWithSettings(context.Background(), testEmail, testActionCodeSettingsCustom)
if err != nil {
t.Fatal(err)
}
if link != testActionLink {
t.Errorf("PasswordResetLinkWithSettings() = %q; want = %q", link, testActionLink)
}

want := map[string]interface{}{
"requestType": "PASSWORD_RESET",
"email": testEmail,
"returnOobLink": true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make sure we keep the current SDK behaviour intact. When sendEmailLink is not set (or false) we should set returnOobLink to true to ensure we don't break the existing users.

}
for k, v := range testActionCodeSettingsMap {
want[k] = v
}
if err := checkActionLinkRequest(want, s); err != nil {
t.Fatalf("PasswordResetLinkWithSettings() %v", err)
want := map[string]interface{}{
"requestType": "PASSWORD_RESET",
"email": testEmail,
}
for k, v := range testActionCodeSettingsMapCustom {
want[k] = v
}
if err := checkActionLinkRequest(want, s); err != nil {
t.Fatalf("PasswordResetLinkWithSettings() %v", err)
}
}
}

Expand Down Expand Up @@ -204,24 +216,29 @@ func TestEmailSignInLink(t *testing.T) {
s := echoServer(testActionLinkResponse, t)
defer s.Close()

link, err := s.Client.EmailSignInLink(context.Background(), testEmail, testActionCodeSettings)
if err != nil {
t.Fatal(err)
}
if link != testActionLink {
t.Errorf("EmailSignInLink() = %q; want = %q", link, testActionLink)
}

want := map[string]interface{}{
"requestType": "EMAIL_SIGNIN",
"email": testEmail,
"returnOobLink": true,
}
for k, v := range testActionCodeSettingsMap {
want[k] = v
}
if err := checkActionLinkRequest(want, s); err != nil {
t.Fatalf("EmailSignInLink() %v", err)
cases := []bool{true, false}
testActionCodeSettingsCustom, testActionCodeSettingsMapCustom := getCopiesOfTestSettings(testActionCodeSettings,
testActionCodeSettingsMap)
for _, returnOobLink := range cases {
testActionCodeSettingsCustom.SendEmailLink = !returnOobLink
testActionCodeSettingsMapCustom["returnOobLink"] = returnOobLink
link, err := s.Client.EmailSignInLink(context.Background(), testEmail, testActionCodeSettingsCustom)
if err != nil {
t.Fatal(err)
}
if link != testActionLink {
t.Errorf("EmailSignInLink() = %q; want = %q", link, testActionLink)
}
want := map[string]interface{}{
"requestType": "EMAIL_SIGNIN",
"email": testEmail,
}
for k, v := range testActionCodeSettingsMapCustom {
want[k] = v
}
if err := checkActionLinkRequest(want, s); err != nil {
t.Fatalf("EmailSignInLink() %v", err)
}
}
}

Expand Down Expand Up @@ -333,3 +350,22 @@ func checkActionLinkRequestWithURL(want map[string]interface{}, wantURL string,
}
return nil
}

func getCopiesOfTestSettings(testSettings *ActionCodeSettings,
testSettingsMap map[string]interface{}) (*ActionCodeSettings, map[string]interface{}) {
testActionCodeSettingsCustom := &ActionCodeSettings{
URL: testSettings.URL,
HandleCodeInApp: testSettings.HandleCodeInApp,
IOSBundleID: testSettings.IOSBundleID,
AndroidPackageName: testSettings.AndroidPackageName,
AndroidMinimumVersion: testSettings.AndroidMinimumVersion,
AndroidInstallApp: testSettings.AndroidInstallApp,
DynamicLinkDomain: testSettings.DynamicLinkDomain,
SendEmailLink: testSettings.SendEmailLink,
}
testActionCodeSettingsMapCustom := map[string]interface{}{}
for k, v := range testSettingsMap {
testActionCodeSettingsMapCustom[k] = v
}
return testActionCodeSettingsCustom, testActionCodeSettingsMapCustom
}
40 changes: 23 additions & 17 deletions auth/tenant_mgt_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,24 +548,30 @@ func TestTenantEmailSignInLink(t *testing.T) {
t.Fatalf("AuthForTenant() = %v", err)
}

link, err := client.EmailSignInLink(context.Background(), testEmail, testActionCodeSettings)
if err != nil {
t.Fatal(err)
}
if link != testActionLink {
t.Errorf("EmailSignInLink() = %q; want = %q", link, testActionLink)
}
cases := []bool{true, false}
testActionCodeSettingsCustom, testActionCodeSettingsMapCustom := getCopiesOfTestSettings(testActionCodeSettings,
testActionCodeSettingsMap)
for _, returnOobLink := range cases {
testActionCodeSettingsCustom.SendEmailLink = !returnOobLink
testActionCodeSettingsMapCustom["returnOobLink"] = returnOobLink
link, err := client.EmailSignInLink(context.Background(), testEmail, testActionCodeSettingsCustom)
if err != nil {
t.Fatal(err)
}
if link != testActionLink {
t.Errorf("EmailSignInLink() = %q; want = %q", link, testActionLink)
}

want := map[string]interface{}{
"requestType": "EMAIL_SIGNIN",
"email": testEmail,
"returnOobLink": true,
}
for k, v := range testActionCodeSettingsMap {
want[k] = v
}
if err := checkActionLinkRequestWithURL(want, wantEmailActionURL, s); err != nil {
t.Fatalf("EmailSignInLink() %v", err)
want := map[string]interface{}{
"requestType": "EMAIL_SIGNIN",
"email": testEmail,
}
for k, v := range testActionCodeSettingsMapCustom {
want[k] = v
}
if err := checkActionLinkRequestWithURL(want, wantEmailActionURL, s); err != nil {
t.Fatalf("EmailSignInLink() %v", err)
}
}
}

Expand Down