Skip to content

Commit

Permalink
Merge branch 'main' into brant/authenticator-sync-integration-test
Browse files Browse the repository at this point in the history
  • Loading branch information
brant-livefront committed Nov 1, 2024
2 parents 95b83db + 70b9eb6 commit 3df7e02
Show file tree
Hide file tree
Showing 104 changed files with 1,228 additions and 134 deletions.
4 changes: 2 additions & 2 deletions .github/ISSUE_TEMPLATE/bug.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ body:
attributes:
label: Environment Details
placeholder: |
- Device: [e.g. iPhone 15 Pro, iPad Air (5th Generation)]
- OS Version: [e.g. 17.4.1]
- Device: [e.g. iPhone 16 Pro, iPad Air (5th Generation)]
- OS Version: [e.g. 18.0.1]
- type: checkboxes
id: issue-tracking-info
attributes:
Expand Down
8 changes: 6 additions & 2 deletions .github/workflows/CI-main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ on:
description: "Distribute to TestFlight"
type: boolean
env:
XCODE_VERSION: '15.4'
DISTRIBUTE_TO_TESTFLIGHT: ${{ github.event_name == 'push' || inputs.distribute }}
INTERNAL_BETA_PATCH_NUMBER: 999

Expand All @@ -30,7 +29,7 @@ jobs:
outputs:
version_name: ${{ steps.version_info.outputs.version_name }}
version_number: ${{ steps.version_info.outputs.version_number }}
xcode_version: ${{ env.XCODE_VERSION }}
xcode_version: ${{ steps.xcode_version.outputs.xcode_version }}
distribute_to_testflight: ${{ env.DISTRIBUTE_TO_TESTFLIGHT }}
internal_beta_version_name: ${{ steps.internal_versions.outputs.internal_beta_version_name}}
steps:
Expand All @@ -43,6 +42,11 @@ jobs:
echo '```' >> $GITHUB_STEP_SUMMARY
echo "</details>" >> $GITHUB_STEP_SUMMARY
- name: Read default Xcode version
id: xcode_version
run: |
echo "xcode_version=$(cat .xcode-version | tr -d '\n')" >> "$GITHUB_OUTPUT"
- name: Calculate version
if: ${{ inputs.build-number == '' || inputs.build-version == '' }}
uses: bitwarden/ios/.github/actions/dispatch-and-download@main
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ on:
base_version_number:
description: "Base Version Number - Will be added to the calculated version number"
type: number
default: 2000
default: 1500
patch_version:
description: "Patch Version Override - e.g. '999'"
type: string
Expand Down Expand Up @@ -55,7 +55,7 @@ on:
base_version_number:
description: "Base Version Number - Will be added to the calculated version number"
type: number
default: 2000
default: 1500
patch_version:
description: "Patch Version Override - e.g. '999'"
type: string
Expand All @@ -72,7 +72,7 @@ env:
jobs:
build:
name: Build
runs-on: macos-14
runs-on: macos-15
env:
MINT_PATH: .mint/lib
MINT_LINK_PATH: .mint/bin
Expand Down Expand Up @@ -347,7 +347,7 @@ jobs:
- name: Upload IPA & dSYM files
uses: actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 # v4.4.3
with:
name: Bitwarden iOS ${{ steps.version_info.outputs.version_name }} (${{ steps.version_info.outputs.version_number }}) ${{ env.BUILD_VARIANT }} ${{ env.XCODE_VERSION }}
name: Bitwarden iOS ${{ steps.version_info.outputs.version_name }} (${{ steps.version_info.outputs.version_number }}) ${{ env.BUILD_VARIANT }} ${{ env.XCODE_VERSION || env.DEFAULT_XCODE_VERSION }}
path: export
if-no-files-found: error

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ jobs:

test:
name: Test
runs-on: macos-14-xlarge
runs-on: macos-15-xlarge
needs: check-run
permissions:
contents: read
Expand Down
2 changes: 1 addition & 1 deletion .test-simulator-device-name
Original file line number Diff line number Diff line change
@@ -1 +1 @@
iPhone 15 Pro
iPhone 16 Pro
2 changes: 1 addition & 1 deletion .test-simulator-ios-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
18.0
18.1
2 changes: 1 addition & 1 deletion .xcode-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
15.4
16.1
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ public class DefaultAuthenticatorBridgeItemService: AuthenticatorBridgeItemServi
context: dataStore.persistentContainer.viewContext,
request: fetchRequest
)
.tryMap { dataItems in
.map { dataItems in
dataItems.compactMap(\.model)
}
.asyncTryMap { itemModel in
Expand Down
2 changes: 2 additions & 0 deletions AuthenticatorBridgeKit/SharedCryptographyService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ public class DefaultAuthenticatorCryptographyService: SharedCryptographyService
public func decryptAuthenticatorItems(
_ items: [AuthenticatorBridgeItemDataModel]
) async throws -> [AuthenticatorBridgeItemDataView] {
guard !items.isEmpty else { return [] }

let key = try await sharedKeychainRepository.getAuthenticatorKey()
let symmetricKey = SymmetricKey(data: key)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,17 @@ final class SharedCryptographyServiceTests: AuthenticatorBridgeKitTestCase {

// MARK: Tests

/// Verify that `SharedCryptographyService.decryptAuthenticatorItems()' returns and empty
/// array when the input is an empty array even when the authenticator key is missing.
///
func test_decryptAuthenticatorItems_returnsEmpty() async throws {
try sharedKeychainRepository.deleteAuthenticatorKey()
await assertAsyncDoesNotThrow {
let result = try await subject.decryptAuthenticatorItems([])
XCTAssertEqual(result, [])
}
}

/// Verify that `SharedCryptographyService.decryptAuthenticatorItems(:)` correctly
/// decrypts an encrypted array of `AuthenticatorBridgeItemDataModel`.
///
Expand All @@ -44,11 +55,12 @@ final class SharedCryptographyServiceTests: AuthenticatorBridgeKitTestCase {
/// when the `SharedKeyRepository` authenticator key is missing.
///
func test_decryptAuthenticatorItems_throwsKeyMissingError() async throws {
let encryptedItems = try await subject.encryptAuthenticatorItems(items)
let error = AuthenticatorKeychainServiceError.keyNotFound(SharedKeychainItem.authenticatorKey)

try sharedKeychainRepository.deleteAuthenticatorKey()
await assertAsyncThrows(error: error) {
_ = try await subject.decryptAuthenticatorItems([])
_ = try await subject.decryptAuthenticatorItems(encryptedItems)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,14 @@ import Foundation
class MockSharedCryptographyService: SharedCryptographyService {
var decryptCalled = false
var encryptCalled = false
var errorToThrow: Error?

func decryptAuthenticatorItems(
_ items: [AuthenticatorBridgeItemDataModel]
) async throws -> [AuthenticatorBridgeItemDataView] {
if let errorToThrow {
throw errorToThrow
}
decryptCalled = true
return items.map { model in
AuthenticatorBridgeItemDataView(
Expand All @@ -27,6 +31,9 @@ class MockSharedCryptographyService: SharedCryptographyService {
func encryptAuthenticatorItems(
_ items: [AuthenticatorBridgeItemDataView]
) async throws -> [AuthenticatorBridgeItemDataModel] {
if let errorToThrow {
throw errorToThrow
}
encryptCalled = true
return items.map { view in
AuthenticatorBridgeItemDataModel(
Expand Down
3 changes: 3 additions & 0 deletions BitwardenShared/Core/Platform/Models/Enum/FeatureFlag.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ enum FeatureFlag: String, CaseIterable, Codable {
/// A feature flag for the create account flow.
case nativeCreateAccountFlow = "native-create-account-flow"

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

// MARK: Test Flags

/// A test feature flag that isn't remotely configured and has no initial value.
Expand Down Expand Up @@ -82,6 +84,7 @@ enum FeatureFlag: String, CaseIterable, Codable {
.importLoginsFlow,
.nativeCarouselFlow,
.nativeCreateAccountFlow,
.sshKeyVaultItem,
.testLocalFeatureFlag,
.testLocalInitialBoolFlag,
.testLocalInitialIntFlag,
Expand Down
19 changes: 19 additions & 0 deletions BitwardenShared/Core/Platform/Models/Enum/FeatureFlagTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,25 @@ final class FeatureFlagTests: BitwardenTestCase {
XCTAssertEqual(filtered, [])
}

/// `getter:isRemotelyConfigured` returns the correct value for each flag.
func test_isRemotelyConfigured() {
XCTAssertTrue(FeatureFlag.emailVerification.isRemotelyConfigured)
XCTAssertTrue(FeatureFlag.testRemoteInitialBoolFlag.isRemotelyConfigured)
XCTAssertTrue(FeatureFlag.testRemoteInitialIntFlag.isRemotelyConfigured)
XCTAssertTrue(FeatureFlag.testRemoteInitialStringFlag.isRemotelyConfigured)

XCTAssertFalse(FeatureFlag.enableAuthenticatorSync.isRemotelyConfigured)
XCTAssertFalse(FeatureFlag.enableCipherKeyEncryption.isRemotelyConfigured)
XCTAssertFalse(FeatureFlag.importLoginsFlow.isRemotelyConfigured)
XCTAssertFalse(FeatureFlag.nativeCarouselFlow.isRemotelyConfigured)
XCTAssertFalse(FeatureFlag.nativeCreateAccountFlow.isRemotelyConfigured)
XCTAssertFalse(FeatureFlag.sshKeyVaultItem.isRemotelyConfigured)
XCTAssertFalse(FeatureFlag.testLocalFeatureFlag.isRemotelyConfigured)
XCTAssertFalse(FeatureFlag.testLocalInitialBoolFlag.isRemotelyConfigured)
XCTAssertFalse(FeatureFlag.testLocalInitialIntFlag.isRemotelyConfigured)
XCTAssertFalse(FeatureFlag.testLocalInitialStringFlag.isRemotelyConfigured)
}

/// `name` formats the raw value of a feature flag
func test_name() {
XCTAssertEqual(FeatureFlag.testLocalFeatureFlag.name, "Test Local Feature Flag")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ actor DefaultAuthenticatorSyncService: NSObject, AuthenticatorSyncService {
/// The service used by the application to manage account state.
private let stateService: StateService

/// A Task to hold the subscription that waits for sync to be turned on/off.
private var syncSubscriber: Task<Void, Never>?

/// A Task to hold the subscription that waits for the vault to be locked/unlocked..
private var vaultSubscriber: Task<Void, Never>?

/// The service used by the application to manage vault access.
private let vaultTimeoutService: VaultTimeoutService

Expand Down Expand Up @@ -107,6 +113,11 @@ actor DefaultAuthenticatorSyncService: NSObject, AuthenticatorSyncService {
super.init()
}

deinit {
syncSubscriber?.cancel()
vaultSubscriber?.cancel()
}

// MARK: Public Methods

public func getTemporaryTotpItem() async -> AuthenticatorBridgeItemDataView? {
Expand All @@ -130,7 +141,7 @@ actor DefaultAuthenticatorSyncService: NSObject, AuthenticatorSyncService {
return
}

Task {
syncSubscriber = Task {
for await (userId, _) in await self.stateService.syncToAuthenticatorPublisher().values {
guard let userId else { continue }

Expand All @@ -141,7 +152,7 @@ actor DefaultAuthenticatorSyncService: NSObject, AuthenticatorSyncService {
}
}
}
Task {
vaultSubscriber = Task {
for await vaultStatus in await self.vaultTimeoutService.vaultLockStatusPublisher().values {
guard let vaultStatus else { continue }

Expand Down Expand Up @@ -205,7 +216,7 @@ actor DefaultAuthenticatorSyncService: NSObject, AuthenticatorSyncService {

return decryptedCiphers.map { cipher in
AuthenticatorBridgeItemDataView(
accountDomain: account.settings.environmentUrls?.webVaultHost,
accountDomain: account.settings.environmentUrls?.webVaultHost ?? Constants.defaultWebVaultHost,
accountEmail: account.profile.email,
favorite: false,
id: cipher.id ?? UUID().uuidString,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,33 @@ protocol NotificationCenterService: AnyObject {
/// A default implementation of the `NotificationCenterService` which accesses the app's notification center.
///
class DefaultNotificationCenterService: NotificationCenterService {
// MARK: Properties

/// The NotificationCenter to use in subscribing to notifications.
let notificationCenter: NotificationCenter

// MARK: Initialization

/// Initialize a `DefaultNotificationCenterService`.
///
/// - Parameter notificationCenter: The NotificationCenter to use in subscribing to notifications.
///
init(notificationCenter: NotificationCenter = NotificationCenter.default) {
self.notificationCenter = notificationCenter
}

// MARK: Methods

func didEnterBackgroundPublisher() -> AsyncPublisher<AnyPublisher<Void, Never>> {
NotificationCenter.default
notificationCenter
.publisher(for: UIApplication.didEnterBackgroundNotification)
.map { _ in }
.eraseToAnyPublisher()
.values
}

func willEnterForegroundPublisher() -> AsyncPublisher<AnyPublisher<Void, Never>> {
NotificationCenter.default
notificationCenter
.publisher(for: UIApplication.willEnterForegroundNotification)
.map { _ in }
.eraseToAnyPublisher()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import XCTest

@testable import BitwardenShared

final class NotificationCenterServiceTests: BitwardenTestCase {
// MARK: Properties

var notificationCenter: NotificationCenter!
var subject: DefaultNotificationCenterService!

// MARK: Setup & Teardown

override func setUp() {
notificationCenter = NotificationCenter()
subject = DefaultNotificationCenterService(notificationCenter: notificationCenter)
}

override func tearDown() {
notificationCenter = nil
subject = nil
}

// MARK: Tests

/// `didEnterBackgroundPublisher` publishes a notification when the app enters the background.
func testDidEnterBackgroundPublisher() async throws {
var iterator = subject.didEnterBackgroundPublisher().makeAsyncIterator()
Task {
notificationCenter.post(
name: UIApplication.didEnterBackgroundNotification,
object: nil
)
}
let result: Void? = await iterator.next()

XCTAssertNotNil(result)
}

/// `willEnterForegroundPublisher` publishes a notification when the app will enter the foreground.
func testWillEnterForegroundPublisher() async throws {
var iterator = subject.willEnterForegroundPublisher().makeAsyncIterator()
Task {
notificationCenter.post(
name: UIApplication.willEnterForegroundNotification,
object: nil
)
}
let result: Void? = await iterator.next()

XCTAssertNotNil(result)
}
}
14 changes: 11 additions & 3 deletions BitwardenShared/Core/Platform/Services/StateService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1494,11 +1494,19 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le

func getVaultTimeout(userId: String?) async throws -> SessionTimeoutValue {
let userId = try getAccount(userId: userId).profile.userId
let userAuthKey = try? await keychainRepository.getUserAuthKeyValue(for: .neverLock(userId: userId))
guard let rawValue = appSettingsStore.vaultTimeout(userId: userId) else {
let userAuthKey = try? await keychainRepository.getUserAuthKeyValue(for: .neverLock(userId: userId))
return userAuthKey == nil ? .fifteenMinutes : .never
// If there isn't a stored value, it may be because MAUI stored `nil` for never timeout.
// So if the never lock key exists, set the timeout to never, otherwise to default.
return userAuthKey != nil ? .never : .fifteenMinutes
}
return SessionTimeoutValue(rawValue: rawValue)

let timeoutValue = SessionTimeoutValue(rawValue: rawValue)
if timeoutValue == .never, userAuthKey == nil {
// If never lock but no key (possibly due to logging out), return the default timeout.
return .fifteenMinutes
}
return timeoutValue
}

func isAuthenticated(userId: String?) async throws -> Bool {
Expand Down
11 changes: 11 additions & 0 deletions BitwardenShared/Core/Platform/Services/StateServiceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,17 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
XCTAssertEqual(vaultTimeout, .never)
}

/// `getVaultTimeout(userId:)` returns the default timeout if the user has a never lock value
/// stored but the never lock key doesn't exist.
func test_getVaultTimeout_neverLock_missingKey() async throws {
appSettingsStore.vaultTimeout["1"] = -2

await subject.addAccount(.fixture(profile: .fixture(userId: "1")))

let vaultTimeout = try await subject.getVaultTimeout()
XCTAssertEqual(vaultTimeout, .fifteenMinutes)
}

/// `lastSyncTimePublisher()` returns a publisher for the user's last sync time.
func test_lastSyncTimePublisher() async throws {
await subject.addAccount(.fixture(profile: .fixture(userId: "1")))
Expand Down
Loading

0 comments on commit 3df7e02

Please sign in to comment.