feat: disabled 2fa feature #631
Conversation
gorjan5sk
commented
Sep 7, 2021
gorjan5sk
commented
- use new snjs mfa method to check if 2fa feature is available
| @@ -18,7 +18,8 @@ export const PreferencesViewWrapper: FunctionComponent<PreferencesViewWrapperPro | |||
| <PreferencesView | |||
| closePreferences={() => appState.preferences.closePreferences()} | |||
| application={application} | |||
| mfaGateway={application} | |||
| mfaProvider={application} | |||
| userProvider={application} | |||
There was a problem hiding this comment.
We use 2 separate types for mfaProvider and userProvider but pass the same application object to them. Probably it's better to either use one prop for application with appropriate name or pass application's respective fields to mfaProvider and userProvider (e.g., pass only application.getUser to userProvider)
There was a problem hiding this comment.
Separation of concerns. We can test components that only depend on x:getUser(), that only depend on MfaProvider stuff, without mocking the whole WebApplication along with the kitchen sink :D
There was a problem hiding this comment.
It looks very strange tbh :D But I think we'll have better (practical) understanding of pros and cons of this approach only after writing some tests :)
There was a problem hiding this comment.
They're coming! :D
Can I get an approval now? :)
| @@ -11,7 +11,7 @@ export const is2FADisabled = (s: TwoFactorStatus): s is 'two-factor-disabled' => | |||
| s === 'two-factor-disabled'; | |||
|
|
|||
| export const is2FAActivation = (s: TwoFactorStatus): s is TwoFactorActivation => | |||
There was a problem hiding this comment.
Let's use more meaningful name for s :)
app/assets/javascripts/preferences/panes/two-factor-auth/TwoFactorAuthView.tsx
Outdated
Show resolved
Hide resolved
…ctorAuthView.tsx Co-authored-by: Vardan Hakobyan <[email protected]>
