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.

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

Recaptcha public preview #7193

Merged
merged 20 commits into from
Apr 13, 2023
Merged

Recaptcha public preview #7193

merged 20 commits into from
Apr 13, 2023

Conversation

renkelvin
Copy link

@renkelvin renkelvin commented Apr 3, 2023

This PR add the support of reCAPTCHA enterprise integration to GCIP / Firebase Auth. For more details, see go/gcip-recaptcha, go/gcip-recaptcha-client-sdk-api (internal linlks).

@changeset-bot
Copy link

changeset-bot bot commented Apr 3, 2023

🦋 Changeset detected

Latest commit: f4cff13

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@firebase/auth Minor
firebase Minor
@firebase/auth-compat Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Apr 3, 2023

Size Report 1

Affected Products

  • @firebase/app-check

    TypeBase (b04f040)Merge (d9d6887)Diff
    browser25.2 kB25.7 kB+435 B (+1.7%)
    esm530.3 kB30.7 kB+447 B (+1.5%)
    main31.3 kB31.8 kB+447 B (+1.4%)
    module25.2 kB25.7 kB+435 B (+1.7%)
  • @firebase/auth

    TypeBase (b04f040)Merge (d9d6887)Diff
    browser162 kB171 kB+9.08 kB (+5.6%)
    cordova186 kB198 kB+12.1 kB (+6.5%)
    esm5212 kB223 kB+11.7 kB (+5.5%)
    main155 kB168 kB+12.3 kB (+8.0%)
    module162 kB171 kB+9.08 kB (+5.6%)
    react-native171 kB183 kB+12.0 kB (+7.0%)
  • @firebase/auth/cordova

    TypeBase (b04f040)Merge (d9d6887)Diff
    browser186 kB198 kB+12.1 kB (+6.5%)
    module186 kB198 kB+12.1 kB (+6.5%)
  • @firebase/auth/internal

    TypeBase (b04f040)Merge (d9d6887)Diff
    browser172 kB181 kB+9.08 kB (+5.3%)
    esm5225 kB237 kB+11.7 kB (+5.2%)
    main191 kB204 kB+12.4 kB (+6.5%)
    module172 kB181 kB+9.08 kB (+5.3%)
  • @firebase/auth/react-native

    TypeBase (b04f040)Merge (d9d6887)Diff
    browser171 kB183 kB+12.0 kB (+7.0%)
    module171 kB183 kB+12.0 kB (+7.0%)
  • bundle

    TypeBase (b04f040)Merge (d9d6887)Diff
    app-check (ReCaptchaEnterpriseProvider)38.5 kB38.8 kB+226 B (+0.6%)
    app-check (ReCaptchaV3Provider)38.5 kB38.7 kB+226 B (+0.6%)
    auth (Anonymous)68.1 kB71.1 kB+3.05 kB (+4.5%)
    auth (EmailAndPassword)72.2 kB76.1 kB+3.92 kB (+5.4%)
    auth (GoogleFBTwitterGitHubPopup)94.7 kB97.3 kB+2.67 kB (+2.8%)
    auth (GooglePopup)91.9 kB94.6 kB+2.67 kB (+2.9%)
    auth (GoogleRedirect)92.2 kB94.8 kB+2.67 kB (+2.9%)
    auth (Phone)78.2 kB81.0 kB+2.76 kB (+3.5%)
  • firebase

    TypeBase (b04f040)Merge (d9d6887)Diff
    firebase-app-check-compat.js22.7 kB23.1 kB+320 B (+1.4%)
    firebase-app-check.js21.5 kB21.8 kB+328 B (+1.5%)
    firebase-auth-compat.js127 kB132 kB+5.62 kB (+4.4%)
    firebase-auth-cordova.js139 kB147 kB+8.42 kB (+6.1%)
    firebase-auth-react-native.js151 kB160 kB+8.39 kB (+5.6%)
    firebase-auth.js120 kB127 kB+6.22 kB (+5.2%)
    firebase-compat.js751 kB757 kB+5.93 kB (+0.8%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/vEfECFnncZ.html

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Apr 3, 2023

Size Analysis Report 1

This report is too large (408,523 characters) to be displayed here in a GitHub comment. Please use the below link to see the full report on Google Cloud Storage.

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/6BZ8sH5JkW.html

@renkelvin
Copy link
Author

Hey @kevinthecheung, this pr is to merge the recaptcha enterprise integration from the feature branch to main. The code changes have been reviewed separately, could you help review the docs part? Thanks!

CI failure seems not related to this PR. Still investigating.

@renkelvin
Copy link
Author

Hey @hsubox76 , any ideas about the CI failure of "Update API reports"? Seems the error persists on a fresh commit #7195

packages/auth/src/core/index.ts Outdated Show resolved Hide resolved
packages/auth/src/model/public_types.ts Outdated Show resolved Hide resolved
renkelvin and others added 4 commits April 7, 2023 15:53
packages/auth/src/core/index.ts Outdated Show resolved Hide resolved
packages/auth/src/model/public_types.ts Outdated Show resolved Hide resolved
renkelvin and others added 4 commits April 10, 2023 09:48
Co-authored-by: Kevin Cheung <kevinthecheung@users.noreply.github.com>
Co-authored-by: Kevin Cheung <kevinthecheung@users.noreply.github.com>
@renkelvin renkelvin changed the title [Do not merge] Recaptcha public preview Recaptcha public preview Apr 10, 2023
docs-devsite/auth.auth.md Outdated Show resolved Hide resolved
packages/auth/demo/public/index.html Show resolved Hide resolved
packages/auth/src/api/errors.ts Outdated Show resolved Hide resolved
packages/auth/src/api/index.ts Show resolved Hide resolved
packages/auth/src/core/auth/auth_impl.ts Show resolved Hide resolved
packages/auth/src/core/index.ts Show resolved Hide resolved
signUpResponse = signUp(authInternal, requestWithRecaptcha);
} else {
signUpResponse = signUp(authInternal, request).catch(async error => {
if (error.code === `auth/${AuthErrorCode.MISSING_RECAPTCHA_TOKEN}`) {
Copy link
Contributor

Choose a reason for hiding this comment

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

not blocking this release - we should consider unifying this logic in a single util method somewhere, to which we can pass the action type. That way we implement it once and make enhancements in one place only.

@hsubox76
Copy link
Contributor

Hey @hsubox76 , any ideas about the CI failure of "Update API reports"? Seems the error persists on a fresh commit #7195

Sorry about this, should be fixed now.

Copy link
Contributor

@hsubox76 hsubox76 left a comment

Choose a reason for hiding this comment

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

  • Approved for the api.md.
  • Make sure to add a changeset (@firebase/auth minor, firebase minor).
  • This is the PR that will be traced back to from the changeset and release notes so can you add a description, even if it just points to other PRs that went into this one that have a more detailed description? Maybe a link to the API proposal/design doc (you can say "(internal link)" after it).

@renkelvin
Copy link
Author

Hey @hsubox76 , any ideas about the CI failure of "Update API reports"? Seems the error persists on a fresh commit #7195

Sorry about this, should be fixed now.

No worries, thanks!

@renkelvin
Copy link
Author

  • Approved for the api.md.
  • Make sure to add a changeset (@firebase/auth minor, firebase minor).
  • This is the PR that will be traced back to from the changeset and release notes so can you add a description, even if it just points to other PRs that went into this one that have a more detailed description? Maybe a link to the API proposal/design doc (you can say "(internal link)" after it).

Done.

@renkelvin
Copy link
Author

renkelvin commented Apr 11, 2023

@hsubox76 Seems I don't have the permission to merge this RP.
It says "Merging is blocked" because "Merging can be performed automatically with 1 approving review." even there 2 approvals already.
Could you help merge this PR? Thanks!

@hsubox76
Copy link
Contributor

@hsubox76 Seems I don't have the permission to merge this RP. It says "Merging is blocked" because "Merging can be performed automatically with 1 approving review." even there 2 approvals already. Could you help merge this PR? Thanks!

I think it's due to approval not being granted by @kevinthecheung after leaving comments.

@hsubox76 hsubox76 merged commit 6b8e0c1 into master Apr 13, 2023
@hsubox76 hsubox76 deleted the recaptcha-public-preview branch April 13, 2023 16:23
@google-oss-bot google-oss-bot mentioned this pull request Apr 13, 2023
@firebase firebase locked and limited conversation to collaborators May 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants