[go: nahoru, domu]
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

Expose enrollmentCompletionDeadline field in TotpSecret. #6870

Merged
merged 1 commit into from
Dec 27, 2022
Merged

Expose enrollmentCompletionDeadline field in TotpSecret. #6870

merged 1 commit into from
Dec 27, 2022

Conversation

prameshj
Copy link
Contributor
@prameshj prameshj commented Dec 8, 2022

Also changed the demo app to display this field as a countdown.

cc @bhparijat @sam-gc

@changeset-bot
Copy link
changeset-bot bot commented Dec 8, 2022 •
edited
Loading

⚠️ No Changeset found

Latest commit: e18344a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@google-oss-bot
Copy link
Contributor
google-oss-bot commented Dec 8, 2022 •
edited
Loading

Size Report 1

Affected Products

  • @firebase/auth

    TypeBase (0df93b4)Merge (48fb2d9)Diff
    browser160 kB160 kB+24 B (+0.0%)
    esm5210 kB210 kB+24 B (+0.0%)
    main154 kB154 kB+24 B (+0.0%)
    module160 kB160 kB+24 B (+0.0%)

  • @firebase/auth/internal

    TypeBase (0df93b4)Merge (48fb2d9)Diff
    browser171 kB171 kB+24 B (+0.0%)
    esm5223 kB223 kB+24 B (+0.0%)
    main190 kB190 kB+24 B (+0.0%)
    module171 kB171 kB+24 B (+0.0%)

  • firebase

    TypeBase (0df93b4)Merge (48fb2d9)Diff
    firebase-auth.js120 kB120 kB+8 B (+0.0%)

Test Logs

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

@google-oss-bot
Copy link
Contributor
google-oss-bot commented Dec 8, 2022 •
edited
Loading

Size Analysis Report 1

Affected Products

  • @firebase/auth

    • TotpMultiFactorGenerator

      Size

      TypeBase (0df93b4)Merge (48fb2d9)Diff
      size34.0 kB34.0 kB+8 B (+0.0%)
      size-with-ext-deps54.6 kB54.6 kB+8 B (+0.0%)

    • TotpSecret

      Size

      TypeBase (0df93b4)Merge (48fb2d9)Diff
      size32.4 kB32.4 kB+8 B (+0.0%)
      size-with-ext-deps53.0 kB53.0 kB+8 B (+0.0%)

Test Logs

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

@github-actions github-actions bot added the doc-changes PRs that affect docs label Dec 8, 2022
@prameshj prameshj marked this pull request as ready for review December 8, 2022 22:54
bhparijat
bhparijat reviewed Dec 13, 2022
View reviewed changes
Copy link
Contributor
@bhparijat bhparijat left a comment

Choose a reason for hiding this comment

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

Just 1 question

common/api-review/auth.api.md
@@ -767,6 +767,7 @@ export interface TotpMultiFactorInfo extends MultiFactorInfo {
export class TotpSecret {
readonly codeIntervalSeconds: number;
readonly codeLength: number;
readonly enrollmentCompletionDeadline: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't understand why it was changed to this instead of finalizeEnrollmentBy

Copy link
Contributor Author
@prameshj prameshj Dec 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This is still pending API review, but IMO enrollmentCompletionDeadline sounds slightly clearer. Modified the naming since users of SDK will now see this field name.

@prameshj
Expose enrollmentCompletionDeadline field in TotpSecret.
e18344a 
Also changed the demo app to display this field as a countdown.
@prameshj
Copy link
Contributor Author

Merging based on offline approval from @sam-gc. This is being merged to the feature branch, not master.

@prameshj prameshj merged commit e132fd2 into mfa-totp Dec 27, 2022
@prameshj prameshj deleted the prameshj-mfa-totp branch December 27, 2022 22:51
prameshj added a commit that referenced this pull request Jan 27, 2023
@prameshj
Expose enrollmentCompletionDeadline field in TotpSecret. (#6870)
9c77f8f 
Also changed the demo app to display this field as a countdown.
@firebase firebase locked and limited conversation to collaborators Jan 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bhparijat bhparijat bhparijat left review comments

@avolkovi avolkovi Awaiting requested review from avolkovi avolkovi is a code owner

@lisajian lisajian Awaiting requested review from lisajian lisajian is a code owner

@sam-gc sam-gc Awaiting requested review from sam-gc sam-gc is a code owner

@yuchenshi yuchenshi Awaiting requested review from yuchenshi yuchenshi is a code owner

@dwyfrequency dwyfrequency Awaiting requested review from dwyfrequency

@hsubox76 hsubox76 Awaiting requested review from hsubox76 hsubox76 is a code owner

Assignees

@egilmorez egilmorez

Labels
doc-changes PRs that affect docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@prameshj @google-oss-bot @bhparijat @egilmorez