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

[PM-8216] Add warning to people who don't have two-factor authentication turned on #1208

Merged
merged 71 commits into from
Dec 27, 2024
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
71 commits
Select commit Hold shift + click to select a range
9c124a8
WIP
KatherineInCode Dec 4, 2024
70090b7
WIP
KatherineInCode Dec 9, 2024
5610511
WIP
KatherineInCode Dec 9, 2024
d75ee94
Two-step view
KatherineInCode Dec 9, 2024
e412f51
Update image
KatherineInCode Dec 9, 2024
f341416
WIP
KatherineInCode Dec 10, 2024
5c073a2
WIP
KatherineInCode Dec 10, 2024
8c55746
WIP
KatherineInCode Dec 11, 2024
c747432
WIP
KatherineInCode Dec 11, 2024
520d56a
Rename
KatherineInCode Dec 11, 2024
44cd639
Flesh out email acces screen
KatherineInCode Dec 11, 2024
fdf53bf
Clean up set up view
KatherineInCode Dec 11, 2024
bc1d05c
Add to app settings
KatherineInCode Dec 12, 2024
67d0020
State service
KatherineInCode Dec 12, 2024
3508862
Add feature flags
KatherineInCode Dec 12, 2024
3e8fb10
Add logic
KatherineInCode Dec 12, 2024
2fc6c40
Save email attestation
KatherineInCode Dec 12, 2024
a8622e3
Implement remind me later
KatherineInCode Dec 12, 2024
9322b9b
Add URLs
KatherineInCode Dec 12, 2024
cee664c
Fix urls on set up two factor screen
KatherineInCode Dec 12, 2024
4ebc843
WIP
KatherineInCode Dec 13, 2024
d9cbd87
Move things into helper
KatherineInCode Dec 13, 2024
c95ae08
Update tests
KatherineInCode Dec 13, 2024
402665f
Merge branch 'main' into pm-8216/tfa-warning
KatherineInCode Dec 13, 2024
2800125
Environment things that were done in another PR
KatherineInCode Dec 13, 2024
8cfe3c3
Merge branch 'main' into pm-8216/tfa-warning
KatherineInCode Dec 16, 2024
a77ec02
Undo line break
KatherineInCode Dec 16, 2024
f51e482
Add text
KatherineInCode Dec 16, 2024
2c534b6
Add tests
KatherineInCode Dec 16, 2024
ca2a4dc
More tests
KatherineInCode Dec 17, 2024
f4ca68f
Rename
KatherineInCode Dec 17, 2024
e30ebe7
Backfill tests
KatherineInCode Dec 17, 2024
8b9bd5b
Update tests
KatherineInCode Dec 17, 2024
ce64c87
Merge branch 'main' into pm-8216/tfa-warning
KatherineInCode Dec 17, 2024
ff24e48
Comments and organization
KatherineInCode Dec 17, 2024
bed21b9
Test
KatherineInCode Dec 17, 2024
a7d10ae
Tests
KatherineInCode Dec 17, 2024
94af4cb
Additional test
KatherineInCode Dec 17, 2024
2bf34a2
Remove dead code
KatherineInCode Dec 17, 2024
db0409b
Don't show remind button when in permanent mode
KatherineInCode Dec 18, 2024
fa61a1a
More tests
KatherineInCode Dec 18, 2024
06a11c5
Test
KatherineInCode Dec 18, 2024
0f7908e
Remove parentheses
KatherineInCode Dec 18, 2024
c6783b2
Refactor view
KatherineInCode Dec 18, 2024
11e0d54
Merge branch 'main' into pm-8216/tfa-warning
KatherineInCode Dec 18, 2024
97be596
Track if account has two-factor enabled
KatherineInCode Dec 19, 2024
df2bb4c
Include SSO check
KatherineInCode Dec 19, 2024
e8c91d0
Remove dead code
KatherineInCode Dec 19, 2024
cb34426
Remove defaults
KatherineInCode Dec 19, 2024
26e36cf
Reorganize
KatherineInCode Dec 19, 2024
cdf4477
Merge branch 'main' into pm-8216/tfa-warning
KatherineInCode Dec 19, 2024
672d159
Remove extraneous spacer
KatherineInCode Dec 20, 2024
9fd8505
Await on Async buttons
KatherineInCode Dec 26, 2024
eb63fbc
Replace dynamicimagetextstackview with pageheaderview
KatherineInCode Dec 26, 2024
7b30cb0
Adjust padding
KatherineInCode Dec 26, 2024
77affd0
Add comments
KatherineInCode Dec 26, 2024
9385e93
Show email address
KatherineInCode Dec 26, 2024
0ed76f8
Update strings
KatherineInCode Dec 26, 2024
5c9a6de
Update comments
KatherineInCode Dec 26, 2024
69c0f20
Pull into constant
KatherineInCode Dec 26, 2024
5288393
Fix tests
KatherineInCode Dec 26, 2024
2bfec46
Update test
KatherineInCode Dec 26, 2024
05631b1
Use guard
KatherineInCode Dec 26, 2024
027a163
Add creation date to profile and remove special two-factor enabled fuโ€ฆ
KatherineInCode Dec 26, 2024
1b4e12a
Remove
KatherineInCode Dec 26, 2024
cf41842
Add condition for accounts relative to being seven days old
KatherineInCode Dec 26, 2024
1411193
Add test
KatherineInCode Dec 26, 2024
5a2f7a7
Adjust string names
KatherineInCode Dec 26, 2024
4f2e5a5
Refactor into string function
KatherineInCode Dec 26, 2024
20e4e8a
Merge branch 'main' into pm-8216/tfa-warning
KatherineInCode Dec 26, 2024
264a845
Reorder logic for better short circuiting
KatherineInCode Dec 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ enum FeatureFlag: String, CaseIterable, Codable {
/// A feature flag for the create account flow.
case nativeCreateAccountFlow = "native-create-account-flow"

/// A feature flag for the new device verification flow.
case newDeviceVerificationTemporaryDismiss = "new-device-temporary-dismiss"

/// A feature flag for the new device verification flow.
case newDeviceVerificationPermanentDismiss = "new-device-permanent-dismiss"

KatherineInCode marked this conversation as resolved.
Show resolved Hide resolved
case sshKeyVaultItem = "ssh-key-vault-item"

/// A feature flag for the refactor on the SSO details endpoint.
Expand Down Expand Up @@ -79,6 +85,8 @@ enum FeatureFlag: String, CaseIterable, Codable {
/// but if `isRemotelyConfigured` is false for the flag, then the value here will be used.
/// This is a helpful way to manage local feature flags.
static let initialValues: [FeatureFlag: AnyCodable] = [
.newDeviceVerificationTemporaryDismiss: .bool(true),
.newDeviceVerificationPermanentDismiss: .bool(true),
.testLocalInitialBoolFlag: .bool(true),
.testLocalInitialIntFlag: .int(42),
.testLocalInitialStringFlag: .string("Test String"),
Expand All @@ -97,6 +105,8 @@ enum FeatureFlag: String, CaseIterable, Codable {
.importLoginsFlow,
.nativeCarouselFlow,
.nativeCreateAccountFlow,
.newDeviceVerificationPermanentDismiss,
.newDeviceVerificationTemporaryDismiss,
.testLocalFeatureFlag,
.testLocalInitialBoolFlag,
.testLocalInitialIntFlag,
Expand Down
43 changes: 43 additions & 0 deletions BitwardenShared/Core/Platform/Services/StateService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,14 @@ protocol StateService: AnyObject {
///
func getTimeoutAction(userId: String?) async throws -> SessionTimeoutAction

/// Gets the display state of the no-two-factor notice for a user ID.
///
/// - Parameters:
/// - userId: The user ID for the account; defaults to current active user if `nil`.
/// - Returns: The display state.
///
func getTwoFactorNoticeDisplayState(userId: String?) async throws -> TwoFactorNoticeDisplayState

/// Get the two-factor token (non-nil if the user selected the "remember me" option).
///
/// - Parameter email: The user's email address.
Expand Down Expand Up @@ -642,6 +650,14 @@ protocol StateService: AnyObject {
///
func setTimeoutAction(action: SessionTimeoutAction, userId: String?) async throws

/// Sets the user's no-two-factor notice display state for a userID.
///
/// - Parameters:
/// - state: The display state to set.
/// - userId: The user ID associated with the state
///
func setTwoFactorNoticeDisplayState(_ state: TwoFactorNoticeDisplayState, userId: String?) async throws

/// Sets the user's two-factor token.
///
/// - Parameters:
Expand Down Expand Up @@ -937,6 +953,14 @@ extension StateService {
try await getTimeoutAction(userId: nil)
}

/// Gets the display state of the no-two-factor notice for the current user.
///
/// - Returns: The display state.
///
func getTwoFactorNoticeDisplayState() async throws -> TwoFactorNoticeDisplayState {
try await getTwoFactorNoticeDisplayState(userId: nil)
}

/// Sets the number of unsuccessful attempts to unlock the vault for the active account.
///
/// - Returns: The number of unsuccessful unlock attempts for the active account.
Expand Down Expand Up @@ -1155,6 +1179,15 @@ extension StateService {
try await setSyncToAuthenticator(syncToAuthenticator, userId: nil)
}

/// Sets the display state for the no-two-factor notice
///
/// - Parameters:
/// - state: The state to set.
///
func setTwoFactorNoticeDisplayState(state: TwoFactorNoticeDisplayState) async throws {
try await setTwoFactorNoticeDisplayState(state, userId: nil)
}

/// Sets the session timeout action.
///
/// - Parameter action: The action to take when the user's session times out.
Expand Down Expand Up @@ -1545,6 +1578,11 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
return timeoutAction
}

func getTwoFactorNoticeDisplayState(userId: String?) async throws -> TwoFactorNoticeDisplayState {
let userId = try userId ?? getActiveAccountUserId()
return appSettingsStore.twoFactorNoticeDisplayState(userId: userId)
}

func getTwoFactorToken(email: String) async -> String? {
appSettingsStore.twoFactorToken(email: email)
}
Expand Down Expand Up @@ -1835,6 +1873,11 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
appSettingsStore.setTimeoutAction(key: action, userId: userId)
}

func setTwoFactorNoticeDisplayState(_ state: TwoFactorNoticeDisplayState, userId: String?) async throws {
let userId = try userId ?? getActiveAccountUserId()
appSettingsStore.setTwoFactorNoticeDisplayState(state, userId: userId)
}

func setTwoFactorToken(_ token: String?, email: String) async {
appSettingsStore.setTwoFactorToken(token, email: email)
}
Expand Down
27 changes: 27 additions & 0 deletions BitwardenShared/Core/Platform/Services/StateServiceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,27 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
XCTAssertEqual(action, .logout)
}

/// `getTwoFactorNoticeDisplayState(userId:)` gets the display state of the two-factor notice for the user.
func test_getTwoFactorNoticeDisplayState() async throws {
appSettingsStore.setTwoFactorNoticeDisplayState(.canAccessEmail, userId: "[email protected]")

let value = try await subject.getTwoFactorNoticeDisplayState(userId: "[email protected]")
XCTAssertEqual(value, .canAccessEmail)
}

/// `getTwoFactorNoticeDisplayState()` gets the display state of the two-factor notice for the current user
/// and throws an error if there is no current user.
func test_getTwoFactorNoticeDisplayState_noId() async throws {
appSettingsStore.setTwoFactorNoticeDisplayState(.canAccessEmail, userId: "1")

do {
try await _ = subject.getTwoFactorNoticeDisplayState()
XCTFail("subject.getTwoFactorNoticeDisplayState() should throw an error if there is no active account")
} catch {
XCTAssertEqual(error as? StateServiceError, StateServiceError.noActiveAccount)
}
KatherineInCode marked this conversation as resolved.
Show resolved Hide resolved
}

/// `getTwoFactorToken(email:)` gets the two-factor code associated with the email.
func test_getTwoFactorToken() async {
appSettingsStore.setTwoFactorToken("yay_you_win!", email: "[email protected]")
Expand Down Expand Up @@ -1976,6 +1997,12 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
}
}

/// `setTwoFactorNoticeDisplayState(_:userId:)` sets the display state of the two-factor notice for the user.
func test_setTwoFactorNoticeDisplayState() async throws {
try await subject.setTwoFactorNoticeDisplayState(.hasNotSeen, userId: "[email protected]")
XCTAssertEqual(appSettingsStore.twoFactorNoticeDisplayState(userId: "[email protected]"), .hasNotSeen)
}

/// `setTwoFactorToken(_:email:)` sets the two-factor code for the email.
func test_setTwoFactorToken() async {
await subject.setTwoFactorToken("yay_you_win!", email: "[email protected]")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,14 @@ protocol AppSettingsStore: AnyObject {
///
func setTimeoutAction(key: SessionTimeoutAction, userId: String)

/// Sets the display state for the two-factor notice.
///
/// - Parameters:
/// - state: The display state.
/// - userId: The userID associated with the state.
///
func setTwoFactorNoticeDisplayState(_ state: TwoFactorNoticeDisplayState, userId: String)

/// Sets the two-factor token.
///
/// - Parameters:
Expand All @@ -463,7 +471,7 @@ protocol AppSettingsStore: AnyObject {
/// Sets the number of unsuccessful attempts to unlock the vault for a user ID.
///
/// - Parameters:
/// - attempts: The number of unsuccessful unlock attempts..
/// - attempts: The number of unsuccessful unlock attempts.
/// - userId: The user ID associated with the unsuccessful unlock attempts.
///
func setUnsuccessfulUnlockAttempts(_ attempts: Int, userId: String)
Expand Down Expand Up @@ -513,6 +521,14 @@ protocol AppSettingsStore: AnyObject {
///
func timeoutAction(userId: String) -> Int?

/// Get the display state of the no-two-factor notice for a user ID.
///
/// - Parameters:
/// - userId: The user ID associated with the state.
/// - Returns: The state for the user ID.
///
func twoFactorNoticeDisplayState(userId: String) -> TwoFactorNoticeDisplayState

/// Get the two-factor token associated with a user's email.
///
/// - Parameter email: The user's email.
Expand Down Expand Up @@ -713,6 +729,7 @@ extension DefaultAppSettingsStore: AppSettingsStore {
case shouldTrustDevice(userId: String)
case syncToAuthenticator(userId: String)
case state
case twoFactorNoticeDisplayState(userId: String)
case twoFactorToken(email: String)
case unsuccessfulUnlockAttempts(userId: String)
case usernameGenerationOptions(userId: String)
Expand Down Expand Up @@ -806,6 +823,8 @@ extension DefaultAppSettingsStore: AppSettingsStore {
key = "state"
case let .syncToAuthenticator(userId):
key = "shouldSyncToAuthenticator_\(userId)"
case let .twoFactorNoticeDisplayState(userId):
key = "twoFactorNoticeDisplayState_\(userId)"
case let .twoFactorToken(email):
key = "twoFactorToken_\(email)"
case let .unsuccessfulUnlockAttempts(userId):
Expand Down Expand Up @@ -1115,6 +1134,10 @@ extension DefaultAppSettingsStore: AppSettingsStore {
store(key, for: .vaultTimeoutAction(userId: userId))
}

func setTwoFactorNoticeDisplayState(_ state: TwoFactorNoticeDisplayState, userId: String) {
store(state, for: .twoFactorNoticeDisplayState(userId: userId))
}

func setTwoFactorToken(_ token: String?, email: String) {
store(token, for: .twoFactorToken(email: email))
}
Expand All @@ -1139,6 +1162,10 @@ extension DefaultAppSettingsStore: AppSettingsStore {
fetch(for: .vaultTimeoutAction(userId: userId))
}

func twoFactorNoticeDisplayState(userId: String) -> TwoFactorNoticeDisplayState {
fetch(for: .twoFactorNoticeDisplayState(userId: userId)) ?? .hasNotSeen
}

func twoFactorToken(email: String) -> String? {
fetch(for: .twoFactorToken(email: email))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,21 @@ class AppSettingsStoreTests: BitwardenTestCase { // swiftlint:disable:this type_
XCTAssertFalse(userDefaults.bool(forKey: "bwPreferencesStorage:shouldSyncToAuthenticator_2"))
}

/// `twoFactorNoticeDisplayState(userId:)` returns `.hasNotSeen` if there isn't a previously stored value.
func test_twoFactorNoticeDisplayState_isInitiallyNotSeen() {
XCTAssertEqual(subject.twoFactorNoticeDisplayState(userId: "[email protected]"), .hasNotSeen)
}

/// `twoFactorToken(email:)` can be used to get and set the persisted value in user defaults.
func test_twoFactorNoticeDisplayState_withValue() {
let date = Date()
subject.setTwoFactorNoticeDisplayState(.canAccessEmail, userId: "[email protected]")
subject.setTwoFactorNoticeDisplayState(.seen(date), userId: "[email protected]")

XCTAssertEqual(subject.twoFactorNoticeDisplayState(userId: "[email protected]"), .canAccessEmail)
XCTAssertEqual(subject.twoFactorNoticeDisplayState(userId: "[email protected]"), .seen(date))
}

/// `twoFactorToken(email:)` returns `nil` if there isn't a previously stored value.
func test_twoFactorToken_isInitiallyNil() {
XCTAssertNil(subject.twoFactorToken(email: "[email protected]"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class MockAppSettingsStore: AppSettingsStore { // swiftlint:disable:this type_bo
var shouldTrustDevice = [String: Bool?]()
var syncToAuthenticatorByUserId = [String: Bool]()
var timeoutAction = [String: Int]()
var twoFactorNoticeDisplayState = [String: TwoFactorNoticeDisplayState]()
var twoFactorTokens = [String: String]()
var usesKeyConnector = [String: Bool]()
var vaultTimeout = [String: Int]()
Expand Down Expand Up @@ -279,6 +280,14 @@ class MockAppSettingsStore: AppSettingsStore { // swiftlint:disable:this type_bo
timeoutAction[userId] = key.rawValue
}

func setTwoFactorNoticeDisplayState(_ state: TwoFactorNoticeDisplayState, userId: String) {
twoFactorNoticeDisplayState[userId] = state
}

func twoFactorNoticeDisplayState(userId: String) -> TwoFactorNoticeDisplayState {
twoFactorNoticeDisplayState[userId] ?? .hasNotSeen
}

func setTwoFactorToken(_ token: String?, email: String) {
twoFactorTokens[email] = token
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,13 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
var setAppRehydrationStateError: Error?
var setBiometricAuthenticationEnabledResult: Result<Void, Error> = .success(())
var setBiometricIntegrityStateError: Error?
var setTwoFactorNoticeDisplayStateError: Error?
var settingsBadgeSubject = CurrentValueSubject<SettingsBadgeState, Never>(.fixture())
var shouldTrustDevice = [String: Bool?]()
var syncToAuthenticatorByUserId = [String: Bool]()
var syncToAuthenticatorResult: Result<Void, Error> = .success(())
var syncToAuthenticatorSubject = CurrentValueSubject<(String?, Bool), Never>((nil, false))
var twoFactorNoticeDisplayState = [String: TwoFactorNoticeDisplayState]()
var twoFactorTokens = [String: String]()
var unsuccessfulUnlockAttempts = [String: Int]()
var updateProfileResponse: ProfileResponseModel?
Expand Down Expand Up @@ -324,6 +326,11 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
return timeoutAction[userId] ?? .lock
}

func getTwoFactorNoticeDisplayState(userId: String?) async throws -> TwoFactorNoticeDisplayState {
let userId = try unwrapUserId(userId)
return twoFactorNoticeDisplayState[userId] ?? .hasNotSeen
}

func getTwoFactorToken(email: String) async -> String? {
twoFactorTokens[email]
}
Expand Down Expand Up @@ -601,6 +608,14 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
accountTokens = Account.AccountTokens(accessToken: accessToken, refreshToken: refreshToken)
}

func setTwoFactorNoticeDisplayState(_ state: TwoFactorNoticeDisplayState, userId: String?) async throws {
if let error = setTwoFactorNoticeDisplayStateError {
throw error
}
let userId = try unwrapUserId(userId)
twoFactorNoticeDisplayState[userId] = state
}

func setTwoFactorToken(_ token: String?, email: String) async {
twoFactorTokens[email] = token
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// MARK: - Alert + TwoFactorNotice

extension Alert {
// MARK: Methods

/// An alert that asks if the user wants to change their email
/// in a browser.
///
/// - Parameters:
/// - action: The action taken if they select continue.
/// - Returns: An alert that asks if the user wants to navigate to the change email page
///
static func changeEmailAlert(action: @escaping () -> Void) -> Alert {
Alert(
title: Localizations.changeAccountEmail,
message: Localizations.changeEmailConfirmation,
KatherineInCode marked this conversation as resolved.
Show resolved Hide resolved
alertActions: [
AlertAction(title: Localizations.cancel, style: .cancel),
AlertAction(title: Localizations.continue, style: .default) { _ in
action()
},
]
)
}

/// An alert that asks if the user wants to turn two-factor login on
/// in a browser.
///
/// - Parameters:
/// - action: The action taken if they select continue.
/// - Returns: An alert that asks if the user wants to navigate to the two-factor login setup page
///
static func turnOnTwoFactorLoginAlert(action: @escaping () -> Void) -> Alert {
Alert(
title: Localizations.turnOnTwoStepLogin,
message: Localizations.turnOnTwoStepLoginConfirmation,
alertActions: [
AlertAction(title: Localizations.cancel, style: .cancel),
AlertAction(title: Localizations.continue, style: .default) { _ in
action()
},
]
)
}
}
Loading
Loading