feat(auth): Add enableAnonymousUser param to CreateTenant and UpdateTenant #412

sonatard · 2020-11-09T16:29:41Z

"https://identitytoolkit.googleapis.com/v2beta1" does not support enableAnonymousUser param. use "https://identitytoolkit.googleapis.com/v2".

RELEASE NOTE: Allowed enabling of anonymous provider via tenant configuration

google-cla · 2020-11-09T16:29:46Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

sonatard · 2020-11-09T16:31:29Z

@googlebot I signed it!

sonatard · 2021-01-13T00:21:36Z

#411 (comment)

@bojeil-google @rsgowman How is the progress of this plan?

sonatard · 2021-09-17T03:21:00Z

@bojeil-google @rsgowman Is there any problem with this pull request? I'm having trouble because I need this feature.

hiranya911 · 2021-09-17T17:51:19Z

This was proposed and added to the Node.js SDK: https://firebase.google.com/docs/reference/admin/node/admin.auth.Tenant#anonymoussigninenabled

I don't think it's been approved for the Go SDK yet.

sonatard · 2021-10-03T01:59:13Z

Node.js SDK was released 8 months ago.

firebase/firebase-admin-node#802
https://github.com/firebase/firebase-admin-node/releases/tag/v9.5.0

lahirumaramba

Thank you @sonatard for your contribution. I apologies for the delay. We have completed the required internal reviews for this API. There is a minor change requested, please check the review comments. Also, please change the base branch to dev instead of master, you might also have to rebase it to dev. Once that is done, we can merge this change.

auth/tenant_mgt.go

…titytoolkit.googleapis.com/v2beta1

sonatard · 2022-03-25T02:13:09Z

@lahirumaramba Thank you for your review !! I fixed it.

auth/tenant_mgt.go

lahirumaramba · 2022-03-25T23:28:37Z

Hi @sonatard Thank you for updating the PR!
It looks like there are some lint issues. Please take a look at https://github.com/firebase/firebase-admin-go/runs/5699483758?check_suite_focus=true

I think your changes to go.sum cause the other CI failures. You should be able to fix them by running go mod download golang.org/x/lint or by just excluding the changes to go.sum from this PR.

sonatard · 2022-03-26T01:21:34Z

@lahirumaramba I executed go mod download golang.org/x/lint.

sonatard · 2022-03-29T08:10:02Z

@lahirumaramba I found CI failed. Is there anything that I need to do? Fix format?

egilmorez

A couple of comment phrasing things to look at, thanks!

auth/tenant_mgt.go

egilmorez · 2022-03-29T21:10:05Z

auth/tenant_mgt.go

@@ -240,6 +242,11 @@ func (t *TenantToCreate) EnableEmailLinkSignIn(enable bool) *TenantToCreate {
 	return t.set(enableEmailLinkSignInKey, enable)
 }

+// EnableAnonymousUsers enables or disables anonymous user.


Looks like EnableAnonymousUsers is a literal that should be backticked for code font.

It doesn't look like we back-tick literals in Go SDK. ex: TenantClient, AndroidNotification etc.

egilmorez · 2022-03-29T21:11:11Z

auth/tenant_mgt.go

@@ -240,6 +242,11 @@ func (t *TenantToCreate) EnableEmailLinkSignIn(enable bool) *TenantToCreate {
 	return t.set(enableEmailLinkSignInKey, enable)
 }

+// EnableAnonymousUsers enables or disables anonymous user.


Our auth docs use the term "anonymous authentication." Should that be used here?

I'd suggest that phrase, or, alternatively, "anonymous user access."

egilmorez · 2022-03-29T21:11:42Z

auth/tenant_mgt.go

@@ -275,6 +282,11 @@ func (t *TenantToUpdate) EnableEmailLinkSignIn(enable bool) *TenantToUpdate {
 	return t.set(enableEmailLinkSignInKey, enable)
 }

+// EnableAnonymousUsers enables or disables anonymous user.


Looks like EnableAnonymousUsers is a literal that should be backticked for code font.

egilmorez · 2022-03-29T21:11:56Z

auth/tenant_mgt.go

@@ -275,6 +282,11 @@ func (t *TenantToUpdate) EnableEmailLinkSignIn(enable bool) *TenantToUpdate {
 	return t.set(enableEmailLinkSignInKey, enable)
 }

+// EnableAnonymousUsers enables or disables anonymous user.


Our auth docs use the term "anonymous authentication." Should that be used here?

I'd suggest that phrase, or, alternatively, "anonymous user access."

This makes sense, but in Go SDK we normally mention the function name followed by a description. Ex: AllowPasswordSignUp enables or disables email sign-in provider. or EnableEmailLinkSignIn enables or disables email link sign-in.

Hm, OK, no backticks I could live with. But just "anonymous user" doesn't sound to me like a thing that can be enabled or disabled. Does it to you Lahiru?

I'd say that "email link sign-in" does sound like it can be disabled/enabled. FWIW at this point, "email sign-in provider" does not; "email sign-in provider support" or "email sign-in provider functionality" might.

This sounds like I'm splitting hairs, but one of the few things I bring to the table is an ear for written English. :) Let's get Kevin's input as well!

Agreed.

"EnableAnonymousUsers enables or disables anonymous authentication."

(And FWIW, "AllowPasswordSignUp enables or disables the email sign-in provider.")

+1 Yup, "the" solves that latter one!

Oh I misread your previous comment, sorry! I understand what we are referring to now.
I am good with // EnableAnonymousUsers enables or disables anonymous user authentication.
Thank you!

lahirumaramba · 2022-03-30T17:33:51Z

Hi, @sonatard Thank you for addressing the PR feedback!
Could you fix the formatting issues, please? You should be able to see them locally if you run gofmt -l -s .. Thank you!

sonatard · 2022-03-31T02:10:00Z

@lahirumaramba I fixed format 👍

lahirumaramba

LGTM! Thank you!

lahirumaramba

Hi @sonatard Thank you again for being patient with all the PR changes. Just one more change to reference docs and we can merge this PR to include in the upcoming release.

Please change the comment // EnableAnonymousUsers enables or disables user. to // EnableAnonymousUsers enables or disables anonymous authentication.

Thank you!

auth/tenant_mgt.go

Co-authored-by: Lahiru Maramba <llahiru@gmail.com>

sonatard · 2022-04-01T04:03:57Z

@lahirumaramba Thank you for your kind communication. I have had a very pleasant experience since the review began.
I’m so excited to release it!

egilmorez

If Kevin's OK, I'm OK!

Thanks for sending!

- Bumped version to 4.8.0 - Includes #412 #485 #492

lahirumaramba · 2022-04-06T18:16:55Z

Thank you @sonatard for your contribution! This feature is now released in v4.8.0

sonatard · 2022-04-07T00:56:33Z

Thanks!!

sonatard marked this pull request as ready for review November 9, 2020 16:33

lahirumaramba self-assigned this Feb 7, 2022

lahirumaramba self-requested a review March 24, 2022 19:54

lahirumaramba requested changes Mar 24, 2022

View reviewed changes

auth/tenant_mgt.go Outdated Show resolved Hide resolved

auth/tenant_mgt.go Outdated Show resolved Hide resolved

auth/tenant_mgt.go Outdated Show resolved Hide resolved

sonatard added 3 commits March 25, 2022 10:49

Add enableAnonymousUser param to CreateTenant and UpdateTenant

ba6e78b

Use https://identitytoolkit.googleapis.com/v2 instead of https://iden…

0f7899f

…titytoolkit.googleapis.com/v2beta1

Rename EnableAnonymousUser to EnableAnonymousUsers

a3dfd67

sonatard changed the base branch from master to dev March 25, 2022 01:52

sonatard requested a review from lahirumaramba March 25, 2022 02:38

lahirumaramba reviewed Mar 25, 2022

View reviewed changes

auth/tenant_mgt.go Outdated Show resolved Hide resolved

lahirumaramba requested a review from egilmorez March 25, 2022 23:25

lahirumaramba assigned sonatard and egilmorez and unassigned lahirumaramba Mar 25, 2022

go mod download golang.org/x/lint

2600d00

egilmorez reviewed Mar 29, 2022

View reviewed changes

egilmorez requested a review from kevinthecheung March 29, 2022 21:12

go fmt ./...

41db8d3

lahirumaramba requested a review from egilmorez March 31, 2022 15:50

lahirumaramba assigned kevinthecheung Mar 31, 2022

lahirumaramba approved these changes Mar 31, 2022

View reviewed changes

lahirumaramba unassigned egilmorez and kevinthecheung Mar 31, 2022

lahirumaramba requested changes Mar 31, 2022

View reviewed changes

auth/tenant_mgt.go Outdated Show resolved Hide resolved

auth/tenant_mgt.go Outdated Show resolved Hide resolved

sonatard and others added 2 commits April 1, 2022 12:59

Update auth/tenant_mgt.go

9d6bfe9

Co-authored-by: Lahiru Maramba <llahiru@gmail.com>

Update auth/tenant_mgt.go

6e3fc83

Co-authored-by: Lahiru Maramba <llahiru@gmail.com>

lahirumaramba approved these changes Apr 1, 2022

View reviewed changes

kevinthecheung approved these changes Apr 1, 2022

View reviewed changes

egilmorez approved these changes Apr 5, 2022

View reviewed changes

lahirumaramba added the release-note label Apr 6, 2022

lahirumaramba merged commit 6f9551f into firebase:dev Apr 6, 2022

lahirumaramba changed the title ~~Add enableAnonymousUser param to CreateTenant and UpdateTenant~~ Apr 6, 2022

sonatard deleted the enable-anonymous-user branch April 6, 2022 14:00

sonatard mentioned this pull request Apr 6, 2022

FR: Fix endpoint from v2beta1 to v2 #372

Closed

lahirumaramba added a commit that referenced this pull request Apr 6, 2022

[chore] Release 4.8.0

d1666d4

- Bumped version to 4.8.0 - Includes #412 #485 #492

lahirumaramba mentioned this pull request Apr 6, 2022

[chore] Release 4.8.0 #493

Merged

lahirumaramba added a commit that referenced this pull request Apr 6, 2022

[chore] Release 4.8.0 (#493)

e423056

- Bumped version to 4.8.0 - Includes #412 #485 #492

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(auth): Add enableAnonymousUser param to CreateTenant and UpdateTenant #412

feat(auth): Add enableAnonymousUser param to CreateTenant and UpdateTenant #412

sonatard commented Nov 9, 2020 •

edited by lahirumaramba

Loading

google-cla bot commented Nov 9, 2020

sonatard commented Nov 9, 2020

sonatard commented Jan 13, 2021

sonatard commented Sep 17, 2021

hiranya911 commented Sep 17, 2021

sonatard commented Oct 3, 2021

lahirumaramba left a comment

sonatard commented Mar 25, 2022

lahirumaramba commented Mar 25, 2022

sonatard commented Mar 26, 2022

sonatard commented Mar 29, 2022

egilmorez left a comment

egilmorez Mar 29, 2022

lahirumaramba Mar 30, 2022

egilmorez Mar 29, 2022

egilmorez Mar 29, 2022

egilmorez Mar 29, 2022

lahirumaramba Mar 30, 2022

egilmorez Mar 31, 2022

kevinthecheung Mar 31, 2022

egilmorez Mar 31, 2022

lahirumaramba Mar 31, 2022

lahirumaramba commented Mar 30, 2022

sonatard commented Mar 31, 2022

lahirumaramba left a comment

lahirumaramba left a comment

sonatard commented Apr 1, 2022

egilmorez left a comment

lahirumaramba commented Apr 6, 2022

sonatard commented Apr 7, 2022

feat(auth): Add enableAnonymousUser param to CreateTenant and UpdateTenant #412

feat(auth): Add enableAnonymousUser param to CreateTenant and UpdateTenant #412

Conversation

sonatard commented Nov 9, 2020 • edited by lahirumaramba Loading

google-cla bot commented Nov 9, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

sonatard commented Nov 9, 2020

sonatard commented Jan 13, 2021

sonatard commented Sep 17, 2021

hiranya911 commented Sep 17, 2021

sonatard commented Oct 3, 2021

lahirumaramba left a comment

Choose a reason for hiding this comment

sonatard commented Mar 25, 2022

lahirumaramba commented Mar 25, 2022

sonatard commented Mar 26, 2022

sonatard commented Mar 29, 2022

egilmorez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lahirumaramba commented Mar 30, 2022

sonatard commented Mar 31, 2022

lahirumaramba left a comment

Choose a reason for hiding this comment

lahirumaramba left a comment

Choose a reason for hiding this comment

sonatard commented Apr 1, 2022

egilmorez left a comment

Choose a reason for hiding this comment

lahirumaramba commented Apr 6, 2022

sonatard commented Apr 7, 2022

sonatard commented Nov 9, 2020 •

edited by lahirumaramba

Loading