Skip to content

Commit

Permalink
Merge branch 'main' into PM-14497/otp-autofill-quicktype-bar
Browse files Browse the repository at this point in the history
  • Loading branch information
fedemkr committed Nov 13, 2024
2 parents 4d69ffc + f8f8977 commit b4c10de
Show file tree
Hide file tree
Showing 44 changed files with 372 additions and 83 deletions.
2 changes: 1 addition & 1 deletion .github/actions/dispatch-and-download/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ runs:
echo "</details>" >> $GITHUB_STEP_SUMMARY
- name: Dispatch an action and get the run ID and URL
uses: codex-/return-dispatch@06d9405c91c1696f12c0bf915ab9c5161c8f06fc # v2.0.1
uses: codex-/return-dispatch@2410062d00e50fbdc50dd9065a4e5f673e2455d3 # v2.0.3
id: return_dispatch
with:
token: ${{ inputs.token }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/_version.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:

- name: Check out repository
if: ${{ !inputs.skip_checkout || false }}
uses: actions/checkout@eef61447b9ff4aafe5dcd4e0bbf5d482be7e7871 # v4.2.1
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
fetch-depth: 0

Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ jobs:
echo "</details>" >> $GITHUB_STEP_SUMMARY
- name: Check out repo
uses: actions/checkout@eef61447b9ff4aafe5dcd4e0bbf5d482be7e7871 # v4.2.1
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
fetch-depth: 0
filter: tree:0
Expand Down Expand Up @@ -144,7 +144,7 @@ jobs:

- name: Cache Mint packages
id: mint-cache
uses: actions/cache@3624ceb22c1c5a301c8db4169662070a689d9ea8 # v4.1.1
uses: actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a # v4.1.2
with:
path: .mint
key: ${{ runner.os }}-mint-${{ hashFiles('**/Mintfile') }}
Expand Down Expand Up @@ -306,7 +306,7 @@ jobs:
plutil -replace aps-environment -string production Bitwarden/Application/Support/Bitwarden.entitlements
- name: Configure Ruby
uses: ruby/setup-ruby@f26937343756480a8cb3ae1f623b9c8d89ed6984 # v1.196.0
uses: ruby/setup-ruby@a2bbe5b1b236842c1cb7dd11e8e3b51e0a616acc # v1.202.0
with:
bundler-cache: true

Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/crowdin-pull.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
_CROWDIN_PROJECT_ID: "269690"
steps:
- name: Checkout repo
uses: actions/checkout@eef61447b9ff4aafe5dcd4e0bbf5d482be7e7871 # v4.2.1
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2

- name: Login to Azure - CI Subscription
uses: Azure/login@e15b166166a8746d1a47596803bd8c1b595455cf # v1.6.0
Expand All @@ -29,7 +29,7 @@ jobs:
secrets: "crowdin-api-token, github-gpg-private-key, github-gpg-private-key-passphrase"

- name: Download translations
uses: crowdin/github-action@95d6e895e871c3c7acf0cfb962f296baa41e63c6 # v2.2.0
uses: crowdin/github-action@2d540f18b0a416b1fbf2ee5be35841bd380fc1da # v2.3.0
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
CROWDIN_API_TOKEN: ${{ steps.retrieve-secrets.outputs.crowdin-api-token }}
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/crowdin-push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
_CROWDIN_PROJECT_ID: "269690"
steps:
- name: Check out repo
uses: actions/checkout@eef61447b9ff4aafe5dcd4e0bbf5d482be7e7871 # v4.2.1
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2

- name: Log in to Azure
uses: Azure/login@cb79c773a3cfa27f31f25eb3f677781210c9ce3d # v1.6.1
Expand All @@ -29,7 +29,7 @@ jobs:
secrets: "crowdin-api-token"

- name: Upload sources
uses: crowdin/github-action@95d6e895e871c3c7acf0cfb962f296baa41e63c6 # v2.2.0
uses: crowdin/github-action@2d540f18b0a416b1fbf2ee5be35841bd380fc1da # v2.3.0
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
CROWDIN_API_TOKEN: ${{ steps.retrieve-secrets.outputs.crowdin-api-token }}
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/scan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ jobs:

steps:
- name: Check out repo
uses: actions/checkout@eef61447b9ff4aafe5dcd4e0bbf5d482be7e7871 # v4.2.1
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
ref: ${{ github.event.pull_request.head.sha }}

- name: Scan with Checkmarx
uses: checkmarx/ast-github-action@f0869bd1a37fddc06499a096101e6c900e815d81 # 2.0.36
uses: checkmarx/ast-github-action@03a90e7253dadd7e2fff55f5dfbce647b39040a1 # 2.0.37
env:
INCREMENTAL: "${{ contains(github.event_name, 'pull_request') && '--sast-incremental' || '' }}"
with:
Expand All @@ -46,7 +46,7 @@ jobs:
--output-path . ${{ env.INCREMENTAL }}
- name: Upload Checkmarx results to GitHub
uses: github/codeql-action/upload-sarif@f779452ac5af1c261dce0346a8f964149f49322b # v3.26.13
uses: github/codeql-action/upload-sarif@9278e421667d5d90a2839487a482448c4ec7df4d # v3.27.2
with:
sarif_file: cx_result.sarif

Expand All @@ -60,7 +60,7 @@ jobs:

steps:
- name: Check out repo
uses: actions/checkout@eef61447b9ff4aafe5dcd4e0bbf5d482be7e7871 # v4.2.1
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
fetch-depth: 0
ref: ${{ github.event.pull_request.head.sha }}
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ jobs:
echo "</details>" >> $GITHUB_STEP_SUMMARY
- name: Check out repo
uses: actions/checkout@eef61447b9ff4aafe5dcd4e0bbf5d482be7e7871 # v4.2.1
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
ref: ${{ github.event.pull_request.head.sha }}

Expand All @@ -81,20 +81,20 @@ jobs:
xcode-version: ${{ env.XCODE_VERSION || env.DEFAULT_XCODE_VERSION }}

- name: Configure Ruby
uses: ruby/setup-ruby@f26937343756480a8cb3ae1f623b9c8d89ed6984 # v1.196.0
uses: ruby/setup-ruby@a2bbe5b1b236842c1cb7dd11e8e3b51e0a616acc # v1.202.0
with:
bundler-cache: true

- name: Cache Mint packages
uses: actions/cache@3624ceb22c1c5a301c8db4169662070a689d9ea8 # v4.1.1
uses: actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a # v4.1.2
with:
path: .mint
key: ${{ runner.os }}-mint-${{ hashFiles('**/Mintfile') }}
restore-keys: |
${{ runner.os }}-mint-
- name: Cache SPM packages
uses: actions/cache@3624ceb22c1c5a301c8db4169662070a689d9ea8 # v4.1.1
uses: actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a # v4.1.2
with:
path: build/DerivedData/SourcePackages
key: ${{ runner.os }}-spm-${{ hashFiles('**/Package.resolved') }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ final class AuthenticatorBridgeItemServiceTests: AuthenticatorBridgeKitTestCase
XCTAssertEqual(results[0], expectedItems)
}

/// Verify that the shared items publisher publishes new lists when items are deleted..
/// Verify that the shared items publisher publishes new lists when items are deleted.
///
func test_sharedItemsPublisher_withDeletes() async throws {
let initialItems = AuthenticatorBridgeItemDataView.fixtures().sorted { $0.id < $1.id }
Expand Down
22 changes: 19 additions & 3 deletions BitwardenShared/Core/Auth/Services/KeychainRepository.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ enum KeychainItem: Equatable {
/// The keychain item for a user's refresh token.
case refreshToken(userId: String)

/// The `SecAccessControlCreateFlags` protection level for this keychain item.
/// The `SecAccessControlCreateFlags` level for this keychain item.
/// If `nil`, no extra protection is applied.
///
var protection: SecAccessControlCreateFlags? {
var accessControlFlags: SecAccessControlCreateFlags? {
switch self {
case .accessToken,
.authenticatorVaultKey,
Expand All @@ -42,6 +42,21 @@ enum KeychainItem: Equatable {
}
}

/// The protection level for this keychain item.
var protection: CFTypeRef {
switch self {
case .biometrics,
.deviceKey,
.neverLock,
.pendingAdminLoginRequest:
kSecAttrAccessibleWhenUnlockedThisDeviceOnly
case .accessToken,
.authenticatorVaultKey,
.refreshToken:
kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly
}
}

/// The storage key for this keychain item.
///
var unformattedKey: String {
Expand Down Expand Up @@ -317,7 +332,8 @@ class DefaultKeychainRepository: KeychainRepository {
///
func setValue(_ value: String, for item: KeychainItem) async throws {
let accessControl = try keychainService.accessControl(
for: item.protection ?? []
protection: item.protection,
for: item.accessControlFlags ?? []
)
let query = await keychainQueryValues(
for: item,
Expand Down
20 changes: 15 additions & 5 deletions BitwardenShared/Core/Auth/Services/KeychainRepositoryTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,8 @@ final class KeychainRepositoryTests: BitwardenTestCase { // swiftlint:disable:th
String(data: XCTUnwrap(attributes[kSecValueData] as? Data), encoding: .utf8),
"ACCESS_TOKEN"
)
let protection = try XCTUnwrap(keychainService.accessControlProtection as? String)
XCTAssertEqual(protection, String(kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly))
}

/// `setAccessToken(userId:)` throws an error if one occurs.
Expand All @@ -355,7 +357,7 @@ final class KeychainRepositoryTests: BitwardenTestCase { // swiftlint:disable:th
keychainService.accessControlResult = .success(
SecAccessControlCreateWithFlags(
nil,
kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly,
[],
nil
)!
Expand All @@ -368,6 +370,8 @@ final class KeychainRepositoryTests: BitwardenTestCase { // swiftlint:disable:th
String(data: XCTUnwrap(attributes[kSecValueData] as? Data), encoding: .utf8),
"AUTHENTICATOR_VAULT_KEY"
)
let protection = try XCTUnwrap(keychainService.accessControlProtection as? String)
XCTAssertEqual(protection, String(kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly))
}

/// `setAuthenticatorVaultKey(userId:)` throws an error if one occurs.
Expand All @@ -384,7 +388,7 @@ final class KeychainRepositoryTests: BitwardenTestCase { // swiftlint:disable:th
keychainService.accessControlResult = .success(
SecAccessControlCreateWithFlags(
nil,
kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly,
[],
nil
)!
Expand All @@ -397,6 +401,8 @@ final class KeychainRepositoryTests: BitwardenTestCase { // swiftlint:disable:th
String(data: XCTUnwrap(attributes[kSecValueData] as? Data), encoding: .utf8),
"REFRESH_TOKEN"
)
let protection = try XCTUnwrap(keychainService.accessControlProtection as? String)
XCTAssertEqual(protection, String(kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly))
}

/// `setRefreshToken(userId:)` throws an error if one occurs.
Expand Down Expand Up @@ -431,7 +437,7 @@ final class KeychainRepositoryTests: BitwardenTestCase { // swiftlint:disable:th
SecAccessControlCreateWithFlags(
nil,
kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
item.protection ?? [],
item.accessControlFlags ?? [],
nil
)!
)
Expand All @@ -450,13 +456,15 @@ final class KeychainRepositoryTests: BitwardenTestCase { // swiftlint:disable:th
SecAccessControlCreateWithFlags(
nil,
kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
item.protection ?? [],
item.accessControlFlags ?? [],
nil
)!
)
keychainService.addResult = .success(())
try await subject.setUserAuthKey(for: item, value: newKey)
XCTAssertEqual(keychainService.accessControlFlags, .biometryCurrentSet)
let protection = try XCTUnwrap(keychainService.accessControlProtection as? String)
XCTAssertEqual(protection, String(kSecAttrAccessibleWhenUnlockedThisDeviceOnly))
}

/// `setUserAuthKey(_:)` succeeds quietly.
Expand All @@ -468,12 +476,14 @@ final class KeychainRepositoryTests: BitwardenTestCase { // swiftlint:disable:th
SecAccessControlCreateWithFlags(
nil,
kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
item.protection ?? [],
item.accessControlFlags ?? [],
nil
)!
)
keychainService.addResult = .success(())
try await subject.setUserAuthKey(for: item, value: newKey)
XCTAssertEqual(keychainService.accessControlFlags, [])
let protection = try XCTUnwrap(keychainService.accessControlProtection as? String)
XCTAssertEqual(protection, String(kSecAttrAccessibleWhenUnlockedThisDeviceOnly))
}
}
9 changes: 7 additions & 2 deletions BitwardenShared/Core/Auth/Services/KeychainService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@ import Foundation
protocol KeychainService: AnyObject {
/// Creates an access control for a given set of flags.
///
/// - Parameter flags: The `SecAccessControlCreateFlags` for the access control.
/// - Parameters:
/// - protection: Protection class to be used for the item. Use one of the values that go with the
/// `kSecAttrAccessible` attribute key.
/// - flags: The `SecAccessControlCreateFlags` for the access control.
/// - Returns: The SecAccessControl.
///
func accessControl(
protection: CFTypeRef,
for flags: SecAccessControlCreateFlags
) throws -> SecAccessControl

Expand Down Expand Up @@ -75,12 +79,13 @@ class DefaultKeychainService: KeychainService {
// MARK: Methods

func accessControl(
protection: CFTypeRef,
for flags: SecAccessControlCreateFlags
) throws -> SecAccessControl {
var error: Unmanaged<CFError>?
let accessControl = SecAccessControlCreateWithFlags(
nil,
kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
protection,
flags,
&error
)
Expand Down
54 changes: 54 additions & 0 deletions BitwardenShared/Core/Auth/Services/KeychainServiceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,60 @@ import XCTest
class KeychainServiceErrorTests: BitwardenTestCase {
// MARK: Tests

/// Creating an access control with no specific protection or flags results in the correct default values.
func test_accessControl_default() throws {
let subject = DefaultKeychainService()

let accessControl = try subject.accessControl(
protection: kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
for: []
)
var error: Unmanaged<CFError>?
let expected = SecAccessControlCreateWithFlags(
nil,
kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
[],
&error
)
XCTAssertEqual(accessControl, expected)
}

/// Specifying `.biometryCurrentSet` access control flag is reflected in the access control.
func test_accessControl_withBiometrics() throws {
let subject = DefaultKeychainService()

let accessControl = try subject.accessControl(
protection: kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
for: .biometryCurrentSet
)
var error: Unmanaged<CFError>?
let expected = SecAccessControlCreateWithFlags(
nil,
kSecAttrAccessibleWhenUnlockedThisDeviceOnly,
.biometryCurrentSet,
&error
)
XCTAssertEqual(accessControl, expected)
}

/// Specifying a custom protection level is reflected in the access control.
func test_accessControl_withProtection() throws {
let subject = DefaultKeychainService()

let accessControl = try subject.accessControl(
protection: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly,
for: []
)
var error: Unmanaged<CFError>?
let expected = SecAccessControlCreateWithFlags(
nil,
kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly,
[],
&error
)
XCTAssertEqual(accessControl, expected)
}

/// `getter:errorUserInfo` gets the appropriate user info based on the error case.
func test_errorUserInfo() {
let errorAccessControlFailed = KeychainServiceError.accessControlFailed(nil)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class MockKeychainRepository: KeychainRepository {

func setUserAuthKey(for item: KeychainItem, value: String) async throws {
let formattedKey = formattedKey(for: item)
securityType = item.protection
securityType = item.accessControlFlags
try setResult.get()
mockStorage[formattedKey] = value
}
Expand Down
Loading

0 comments on commit b4c10de

Please sign in to comment.