Skip to content

Mfa totp demoapp #6629

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
Merged
merged 2 commits into from
Oct 5, 2022
Merged

Mfa totp demoapp #6629

merged 2 commits into from
Oct 5, 2022

Conversation

prameshj
Copy link
Contributor

Exported TOTP objects and updated the demo app to support TOTP MFA enrollment.

Modified the demo app package.json to pick up auth changes from HEAD.

@changeset-bot
Copy link

changeset-bot bot commented Sep 26, 2022 •
edited
Loading

⚠️ No Changeset found

Latest commit: d73dfbb

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

@prameshj
Copy link
Contributor Author

cc @sam-gc @bhparijat @renkelvin

@prameshj prameshj force-pushed the mfa-totp-demoapp branch 2 times, most recently from b89fb5c to cfc7493 Compare September 26, 2022 17:28
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Sep 26, 2022 •
edited
Loading

Size Report 1

Affected Products

  • @firebase/auth

    TypeBase (fe42bac)Merge (3326ad4)Diff
    browser156 kB159 kB+3.12 kB (+2.0%)
    esm5203 kB207 kB+3.87 kB (+1.9%)
    main149 kB153 kB+4.39 kB (+3.0%)
    module156 kB159 kB+3.12 kB (+2.0%)
    react-native169 kB169 kB+478 B (+0.3%)

  • @firebase/auth/internal

    TypeBase (fe42bac)Merge (3326ad4)Diff
    browser166 kB170 kB+3.12 kB (+1.9%)
    esm5217 kB221 kB+3.87 kB (+1.8%)
    main184 kB188 kB+4.35 kB (+2.4%)
    module166 kB170 kB+3.12 kB (+1.9%)

  • @firebase/auth/react-native

    TypeBase (fe42bac)Merge (3326ad4)Diff
    browser169 kB169 kB+478 B (+0.3%)
    module169 kB169 kB+478 B (+0.3%)

  • firebase

    TypeBase (fe42bac)Merge (3326ad4)Diff
    firebase-auth-react-native.js148 kB149 kB+240 B (+0.2%)
    firebase-auth.js116 kB118 kB+1.93 kB (+1.7%)

Test Logs

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Sep 26, 2022 •
edited
Loading

Size Analysis Report 1

Affected Products

  • @firebase/auth

    • TotpMultiFactorGenerator

      Size

      TypeBase (fe42bac)Merge (3326ad4)Diff
      size?33.7 kB? (?)
      size-with-ext-deps?53.5 kB? (?)

      Dependency

      TypeBase (fe42bac)Merge (3326ad4)Diff
      functions?

      52 dependencies

      _addTidIfNecessary
_assert
_castAuth
_createError
_emulatorUrl
_errorWithCustomMessage
_fail
_getBrowserName
_getClientVersion
_getCurrentScheme
_getFinalTarget
_getInstance
_getUserLanguage
_initializeAuthInstance
_isAndroid
_isBlackBerry
_isChromeIOS
_isEmptyString
_isFirefox
_isHttpOrHttps
_isIEMobile
_isOnline
_isSafari
_isWebOS
_logError
_logoutIfInvalidated
_makeTaggedError
_parseToken
_performApiRequest
_performFetchWithErrorHandling
_persistenceKeyName
_prodErrorMap
_reloadWithoutSaving
_tokenExpiresIn
assertStringOrUndefined
createErrorInternal
debugAssert
debugFail
deleteAccount
extractProviderData
finalizeEnrollTotpMfa
getAccountInfo
getIdTokenResult
getVersionForPlatform
isUserInvalidated
mergeProviderData
registerAuth
reload
requestStsToken
secondsStringToMilliseconds
startEnrollTotpMfa
utcTimestampToDateString

      		?
      classes?

      17 dependencies

      AuthImpl
AuthInterop
AuthMiddlewareQueue
Delay
FetchProvider
InMemoryPersistence
MultiFactorAssertionImpl
NetworkTimeout
PersistenceUserManager
ProactiveRefresh
StsTokenManager
Subscription
TotpMultiFactorAssertionImpl
TotpMultiFactorGenerator
TotpSecret
UserImpl
UserMetadata

      		?
      variables?

      DEFAULT_API_TIMEOUT_MS
SERVER_ERROR_MAP
_DEFAULT_AUTH_ERROR_FACTORY
inMemoryPersistence
instanceCache
logClient
name
prodErrorMap
version

      		?
      enums??

      External Dependency

      ModuleBase (fe42bac)Merge (3326ad4)Diff
      @firebase/app?

      SDK_VERSION
_registerComponent
registerVersion

      		?
      @firebase/component?

      Component

      		?
      @firebase/logger?

      LogLevel
Logger

      		?
      @firebase/util?

      ErrorFactory
FirebaseError
base64Decode
createSubscribe
getModularInstance
getUA
isBrowserExtension
isMobileCordova
isReactNative
querystring

      		?
      tslib?

      __rest

      		?

    • TotpSecret

      Size

      TypeBase (fe42bac)Merge (3326ad4)Diff
      size?32.3 kB? (?)
      size-with-ext-deps?52.1 kB? (?)

      Dependency

      TypeBase (fe42bac)Merge (3326ad4)Diff
      functions?

      49 dependencies

      _assert
_castAuth
_createError
_emulatorUrl
_errorWithCustomMessage
_fail
_getBrowserName
_getClientVersion
_getCurrentScheme
_getFinalTarget
_getInstance
_getUserLanguage
_initializeAuthInstance
_isAndroid
_isBlackBerry
_isChromeIOS
_isEmptyString
_isFirefox
_isHttpOrHttps
_isIEMobile
_isOnline
_isSafari
_isWebOS
_logError
_logoutIfInvalidated
_makeTaggedError
_parseToken
_performApiRequest
_performFetchWithErrorHandling
_persistenceKeyName
_prodErrorMap
_reloadWithoutSaving
_tokenExpiresIn
assertStringOrUndefined
createErrorInternal
debugAssert
debugFail
deleteAccount
extractProviderData
getAccountInfo
getIdTokenResult
getVersionForPlatform
isUserInvalidated
mergeProviderData
registerAuth
reload
requestStsToken
secondsStringToMilliseconds
utcTimestampToDateString

      		?
      classes?

      14 dependencies

      AuthImpl
AuthInterop
AuthMiddlewareQueue
Delay
FetchProvider
InMemoryPersistence
NetworkTimeout
PersistenceUserManager
ProactiveRefresh
StsTokenManager
Subscription
TotpSecret
UserImpl
UserMetadata

      		?
      variables?

      DEFAULT_API_TIMEOUT_MS
SERVER_ERROR_MAP
_DEFAULT_AUTH_ERROR_FACTORY
inMemoryPersistence
instanceCache
logClient
name
prodErrorMap
version

      		?
      enums??

      External Dependency

      ModuleBase (fe42bac)Merge (3326ad4)Diff
      @firebase/app?

      SDK_VERSION
_registerComponent
registerVersion

      		?
      @firebase/component?

      Component

      		?
      @firebase/logger?

      LogLevel
Logger

      		?
      @firebase/util?

      ErrorFactory
FirebaseError
base64Decode
createSubscribe
getModularInstance
getUA
isBrowserExtension
isMobileCordova
isReactNative
querystring

      		?
      tslib?

      __rest

      		?

Test Logs

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

@github-actions github-actions bot added the doc-changes PRs that affect docs label Sep 26, 2022
@prameshj prameshj mentioned this pull request Oct 5, 2022
Merged
sam-gc
sam-gc reviewed Oct 5, 2022
View reviewed changes
common/api-review/auth.api.md
static assertionForEnrollment(secret: TotpSecret, oneTimePassword: string): TotpMultiFactorAssertion;
static assertionForSignIn(enrollmentId: string, oneTimePassword: string): TotpMultiFactorAssertion;
// Warning: (ae-forgotten-export) The symbol "FactorId" needs to be exported by the entry point index.d.ts
static FACTOR_ID: FactorId_2;
Copy link
Contributor

Choose a reason for hiding this comment

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

FactorId_2 is fishy, the public type might be using the wrong enum

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This uses FactorId from public_types - https://github.com/firebase/firebase-js-sdk/blob/mfa-totp/packages/auth/src/mfa/assertions/totp.ts#L21, which is using the enum type -

firebase-js-sdk/packages/auth/src/model/public_types.ts

Line 546 in fe42bac

export const enum FactorId {

This looks right to me.. maybe we are running into something like - microsoft/rushstack#3581

The other option is to define FACTOR_ID as string in

firebase-js-sdk/packages/auth/src/mfa/assertions/totp.ts

Line 107 in fe42bac

static FACTOR_ID = FactorId.TOTP;
, similar to what phone mfa does - https://github.com/firebase/firebase-js-sdk/blob/mfa-totp/packages/auth/src/platform_browser/mfa/assertions/phone.ts#L98

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Synced offline that we will leave this as-is for now and switch to String if we see the need for it.

@prameshj prameshj merged commit 8cf98ba into mfa-totp Oct 5, 2022
@prameshj prameshj deleted the mfa-totp-demoapp branch October 5, 2022 20:38
bhparijat pushed a commit that referenced this pull request Oct 11, 2022
@prameshj @bhparijat
Mfa totp demoapp (#6629)
3f94477 
* Export TOTP symbols to be picked up by demo app.

* Update the demo app to support TOTP enrollment, use local firebase auth version.

The QR code image is generated using the qrserver api at https://goqr.me/api/doc/
bhparijat pushed a commit that referenced this pull request Oct 13, 2022
@prameshj @bhparijat
Mfa totp demoapp (#6629)
500429a 
* Export TOTP symbols to be picked up by demo app.

* Update the demo app to support TOTP enrollment, use local firebase auth version.

The QR code image is generated using the qrserver api at https://goqr.me/api/doc/
@firebase firebase locked and limited conversation to collaborators Nov 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sam-gc sam-gc sam-gc approved these changes

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

@lisajian lisajian Awaiting requested review from lisajian lisajian 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.
4 participants
@prameshj @google-oss-bot @sam-gc @egilmorez