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

Open
wants to merge 52 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 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
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
3 changes: 3 additions & 0 deletions BitwardenShared/Core/Platform/Models/Domain/Account.swift
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ extension Account {
/// The account's security stamp.
var stamp: String?

/// Whether the account has two-factor enabled.
var twoFactorEnabled: Bool?

/// User decryption options for the account.
var userDecryptionOptions: UserDecryptionOptions?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ extension Account.AccountProfile {
name: String? = nil,
orgIdentifier: String? = nil,
stamp: String? = "stamp",
twoFactorEnabled: Bool? = nil,
userDecryptionOptions: UserDecryptionOptions? = nil,
userId: String = "1"
) -> Account.AccountProfile {
Expand All @@ -107,6 +108,7 @@ extension Account.AccountProfile {
name: name,
orgIdentifier: orgIdentifier,
stamp: stamp,
twoFactorEnabled: twoFactorEnabled,
userDecryptionOptions: userDecryptionOptions,
userId: userId
)
Expand Down
8 changes: 8 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"

case sshKeyVaultItem = "ssh-key-vault-item"

/// A feature flag for the refactor on the SSO details endpoint.
Expand Down Expand Up @@ -97,6 +103,8 @@ enum FeatureFlag: String, CaseIterable, Codable {
.importLoginsFlow,
.nativeCarouselFlow,
.nativeCreateAccountFlow,
.newDeviceVerificationPermanentDismiss,
.newDeviceVerificationTemporaryDismiss,
.testLocalFeatureFlag,
.testLocalInitialBoolFlag,
.testLocalInitialIntFlag,
Expand Down
55 changes: 55 additions & 0 deletions BitwardenShared/Core/Platform/Services/StateService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ protocol StateService: AnyObject {
///
func doesActiveAccountHavePremium() async throws -> Bool

/// Returns whether the active user account has two-factor authentication turned on.
///
/// - Returns: Whether the active account has two-factor authentication turned on.
///
func doesActiveAccountHaveTwoFactor() async throws -> Bool

/// Gets the account for an id.
///
/// - Parameter userId: The id for an account. If nil, the active account will be returned.
Expand Down Expand Up @@ -307,6 +313,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 +656,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 +959,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 +1185,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 @@ -1353,6 +1392,11 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
return !organizations.isEmpty
}

func doesActiveAccountHaveTwoFactor() async throws -> Bool {
let account = try await getActiveAccount()
return account.profile.twoFactorEnabled ?? false
}

func getAccount(userId: String?) throws -> Account {
guard let accounts = appSettingsStore.state?.accounts else {
throw StateServiceError.noAccounts
Expand Down Expand Up @@ -1545,6 +1589,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 +1884,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 Expand Up @@ -1881,6 +1935,7 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
profile.emailVerified = response.emailVerified
profile.name = response.name
profile.stamp = response.securityStamp
profile.twoFactorEnabled = response.twoFactorEnabled

state.accounts[userId]?.profile = profile
}
Expand Down
40 changes: 39 additions & 1 deletion BitwardenShared/Core/Platform/Services/StateServiceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,14 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
XCTAssertFalse(hasPremium)
}

/// `doesActiveAccountHaveTwoFactor()` returns whether the active account
/// has two-factor enabled
func test_doesActiveAccountHaveTwoFactor() async throws {
await subject.addAccount(.fixture(profile: .fixture(twoFactorEnabled: true)))
let hasTwoFactor = try await subject.doesActiveAccountHaveTwoFactor()
XCTAssertTrue(hasTwoFactor)
}

/// `getAccountEncryptionKeys(_:)` returns the encryption keys for the user account.
func test_getAccountEncryptionKeys() async throws {
appSettingsStore.encryptedPrivateKeys["1"] = "1:PRIVATE_KEY"
Expand Down Expand Up @@ -897,6 +905,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)
}
}

/// `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 +2005,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 Expand Up @@ -2148,6 +2183,7 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
hasPremiumPersonally: false,
name: "User",
stamp: "stamp",
twoFactorEnabled: false,
userId: "1"
)
)
Expand All @@ -2160,7 +2196,8 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
emailVerified: true,
name: "Other",
premium: true,
securityStamp: "new stamp"
securityStamp: "new stamp",
twoFactorEnabled: true
),
userId: "1"
)
Expand All @@ -2176,6 +2213,7 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
hasPremiumPersonally: true,
name: "Other",
stamp: "new stamp",
twoFactorEnabled: true,
userId: "1"
)
)
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
Loading
Loading