Skip to content

Broker Secret Key Rotation, Fixes AB#2956404#3048

Open
fadidurah wants to merge 24 commits into
devfrom
fadi/secret-key-rotation
Open

Broker Secret Key Rotation, Fixes AB#2956404#3048
fadidurah wants to merge 24 commits into
devfrom
fadi/secret-key-rotation

Conversation

@fadidurah
Copy link
Copy Markdown
Contributor

@fadidurah fadidurah commented Mar 23, 2026

Symmetric Key Rotation Common Module Changes

Overview

This PR merges the fadi/secret-key-rotation feature branch into dev, introducing the common module components for the Symmetric Key Rotation feature.

The Symmetric Key Rotation feature addresses SDL compliance gaps in the Android Broker's AES-256 symmetric key management. Currently, the broker uses a single, never-rotated symmetric key for encrypting local caches (tokens, accounts, credentials). This feature introduces a multi-key-version architecture that supports key rotation without requiring data migration new data is encrypted with the active key, while old data remains decryptable using any previous key version.

Design Document

PRs Included

The following PRs were merged into the feature branch and are included in this rollup:

PR Title Description
#3023 Add KeyMetadata data class Lombok @Builder data class for symmetric key metadata (versionId, createdAtMillis, algorithm, keySize, isDeprecated)
#3031 Implement KeyVersionRegistry Core registry managing versioned symmetric key lifecycle generate, activate, deprecate, prune, load/unwrap via AndroidKeyStore
#3033 KeyVersionRegistry unit tests Comprehensive Robolectric unit tests for KeyVersionRegistry + removed misplaced CommonFlight entry
#3034 CommonFlight + KeyVersionRegistry wiring Added SYMMETRIC_KEY_MAX_AGE_DAYS flight to CommonFlight enum for key expiry configuration
#3049 SpanName entries Added SymmetricKeyRotationCheck and SymmetricKeyRotation to SpanName enum for OpenTelemetry instrumentation

Companion PR

  • Broker repo: ad-accounts-for-android #125 contains the broker-side implementation (RotationOrchestrator, VersionedStorageEncryptionManager, startup integration, flight definitions, telemetry attributes, and tests)

AB#2956404

Copilot AI and others added 4 commits March 18, 2026 20:41
…46 (#3023)

Foundational immutable data class for tracking symmetric encryption key
versions, enabling SDL-compliant key rotation in the Android Broker.
Each key instance captures its version ID, creation timestamp,
algorithm, key size, and deprecation status (decrypt-only flag).

## Changes

### `common4j/.../crypto/KeyMetadata.java`
- Immutable `final` class using Lombok `@Getter` and `@Builder`
annotations to eliminate boilerplate
- Fields: `versionId`, `createdAtMillis`, `algorithm` (default
`AES/CBC/PKCS5Padding`), `keySize` (default 256), `deprecated`
- Fail-fast validation in constructor: blank `versionId` throws
`IllegalStateException`; non-positive `keySize` throws
`IllegalArgumentException`
- `toJson()` / `fromJson(String)` via `org.json` for persistence by the
KeyVersionRegistry
- `fromJson()` uses `opt*` APIs for optional fields (`algorithm`,
`keySize`, `deprecated`) so deserialization is backward-compatible with
older persisted JSON

```java
KeyMetadata key = KeyMetadata.builder()
        .versionId("K001")
        .createdAtMillis(System.currentTimeMillis())
        .deprecated(false)
        .build();

String json = key.toJson();
KeyMetadata restored = KeyMetadata.fromJson(json);
```

### `common4j/.../crypto/KeyMetadataTest.java`
Unit tests covering builder defaults, field validation, JSON round-trip,
deprecated flag semantics, backward-compatible deserialization of JSON
missing optional fields, and malformed JSON error handling.

<!-- START COPILOT CODING AGENT SUFFIX -->



<!-- START COPILOT ORIGINAL PROMPT -->



<details>

<summary>Original prompt</summary>

> ## [Symmetric Key Rotation] Add KeyMetadata data class
> 
> Fixes
[AB#3546146](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3546146)
> 
> ### Objective
> 
> Add a KeyMetadata data class to common4j that stores version
information for symmetric encryption keys. This is the foundational data
structure for the symmetric key rotation feature.
> 
> ### Context
> 
> The Android Broker needs to support multiple symmetric key versions to
enable SDL-compliant key rotation. Each key needs metadata including
version ID, creation timestamp, algorithm, and deprecation status. This
data class will be used by the KeyVersionRegistry (in common) and the
encryption managers (in broker).
> 
> ### Technical Requirements
> 
> - Create new file
common4j/src/main/com/microsoft/identity/common/java/crypto/KeyMetadata.java
> - Implement as an immutable data class with the following fields:
>   - versionId (String): Key identifier like K001, K002
>   - createdAtMillis (long): Unix timestamp of key creation
> - algorithm (String): Encryption algorithm, default
AES/CBC/PKCS5Padding
>   - keySize (int): Key size in bits, default 256
> - isDeprecated (boolean): If true, key can only decrypt (not encrypt)
> - Add JSON serialization/deserialization methods (toJson(),
fromJson(String)) using org.json
> - Follow existing patterns in common4j for data classes
> - Add appropriate JavaDoc comments
> - Add copyright header to the new file
> 
> ### Scope
> 
> In scope: KeyMetadata data class with all fields, JSON
serialization/deserialization, Builder pattern for construction.
> Out of scope: Persistence logic (handled by KeyVersionRegistry), Key
generation logic (handled elsewhere).
> 
> ### Files to Create
> 
> -
common4j/src/main/com/microsoft/identity/common/java/crypto/KeyMetadata.java
> 
> ### Acceptance Criteria
> 
> - KeyMetadata class exists with all required fields
> - toJson() produces valid JSON representation
> - fromJson(String) reconstructs the object correctly
> - Class is immutable (final fields, no setters)
> - JavaDoc comments document each field
> - Compile check passes: ./gradlew :common4j:compileJava
> 
> Follow .github/copilot-instructions.md strictly.
> 


</details>



<!-- START COPILOT CODING AGENT TIPS -->
---

🔒 GitHub Advanced Security automatically protects Copilot coding agent
pull requests. You can protect all pull requests by enabling Advanced
Security for your repositories. [Learn more about Advanced
Security.](https://gh.io/cca-advanced-security)

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: fadidurah <88730756+fadidurah@users.noreply.github.com>
…agement, Fixes AB#3546148 (#3031)

Introduces `KeyVersionRegistry` to manage multiple versioned AES-256
symmetric encryption keys — tracking metadata, generating new keys,
marking keys deprecated, and pruning expired ones. Builds on the
existing `KeyMetadata` data class and follows
`AndroidWrappedKeyProvider` patterns for RSA-wrapping via
AndroidKeyStore.

## New: `KeyVersionRegistry`

- **Storage**: Metadata in `SharedPreferences` (`brokerks_metadata`);
wrapped key blobs in files (`brokerks_K001`, `brokerks_K002`, …)
- **Wrapping**: RSA/ECB/PKCS1Padding via AndroidKeyStore alias
`brokerks_wrapping_key`; API-level-aware key pair generation (M+ /
legacy)
- **Version IDs**: Auto-incremented (`K001`, `K002`, …)
- **Constants**: `MAX_KEY_AGE_MILLIS` = 3 years, `GRACE_PERIOD_MILLIS` =
90 days
- **Pruning**: Removes any non-active key whose total age exceeds
`MAX_KEY_AGE_MILLIS + GRACE_PERIOD_MILLIS`, regardless of whether it has
been explicitly deprecated; the active key is never pruned
- **Telemetry**: `generateNewKey()` emits a
`KeyVersionRegistryGenerateKey` span (new entry in `SpanName`)

## Public API

```java
KeyVersionRegistry registry = new KeyVersionRegistry(context);

KeyMetadata key  = registry.generateNewKey();       // K001 — not active yet
registry.setActiveKey(key.getVersionId());          // promote to active
KeyMetadata active = registry.getActiveKey();

SecretKey secretKey = registry.loadSecretKey("K001"); // unwrap for decryption

registry.deprecateKey("K001");
List<KeyMetadata> deprecated = registry.getDeprecatedKeys();

registry.pruneExpiredKeys();                        // remove stale keys
```

All public methods are `synchronized`; thread-safety is instance-scoped
(not cross-process).

## Tests

Added `KeyVersionRegistryTest` as an instrumented test in
`common/src/androidTest`, covering all public API methods across 18 test
cases:

- **`generateNewKey`**: metadata fields, wrapped key file creation,
version ID auto-increment (`K001` → `K002`), no auto-promotion to active
- **`setActiveKey` / `getActiveKey`**: initially `null`, promotion,
throws `IllegalStateException` for unknown version
- **`getKeyByVersion`**: found and not-found cases
- **`deprecateKey` / `getDeprecatedKeys`**: marks key deprecated,
idempotent, throws for unknown version, list filtering
- **`loadSecretKey`**: returns unwrapped AES key, same key material on
repeated loads, throws for unknown version
- **`pruneExpiredKeys`**: recent keys kept, expired non-active key
pruned (including key file deletion), active key never pruned even when
old, prunes regardless of deprecation status, boundary threshold check

> [!WARNING]
>
>

<!-- START COPILOT CODING AGENT SUFFIX -->



<!-- START COPILOT ORIGINAL PROMPT -->



<details>

<summary>Original prompt</summary>

> ## [Symmetric Key Rotation][Common] Implement KeyVersionRegistry
> 
> Fixes
[AB#3546148](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3546148)
> 
> ### Objective
> 
> Implement KeyVersionRegistry in common/common to manage multiple
encryption key versions. This registry tracks key metadata, generates
new keys, marks keys as deprecated, and prunes expired keys.
> 
> ### Context
> 
> The KeyVersionRegistry is the central component for managing symmetric
key lifecycle. It stores key metadata in SharedPreferences (encrypted)
and actual wrapped keys in files. Old keys are retained for decryption
until pruned (3 years + 90-day grace period after deprecation). This
builds on the KeyMetadata data class already merged in the codebase at
common4j/src/main/com/microsoft/identity/common/java/crypto/KeyMetadata.java.
> 
> ### Technical Requirements
> 
> - Create
common/src/main/java/com/microsoft/identity/common/crypto/KeyVersionRegistry.java
> - Constructor takes Context for storage access
> - Implement key metadata storage using SharedPreferences with filename
brokerks_metadata
> - Implement the following public methods:
>   - KeyMetadata getActiveKey()
>   - KeyMetadata getKeyByVersion(String versionId)
>   - List<KeyMetadata> getDeprecatedKeys()
> - KeyMetadata generateNewKey() - Creates key, wraps with KeyStore,
stores
>   - void deprecateKey(String versionId) - Sets isDeprecated=true
> - void setActiveKey(String versionId) - Updates activeVersion pointer
> - SecretKey loadSecretKey(String versionId) - Loads/unwraps key
material
> - void pruneExpiredKeys() - Remove keys older than MAX_AGE +
GRACE_PERIOD
> - Key version IDs format: K001, K002, etc. (auto-incrementing)
> - Use AndroidKeyStore for RSA wrapping of symmetric keys (follow
existing StorageCryptoManager patterns)
> - Key file naming: brokerks_K001, brokerks_K002, etc.
> - Constants: MAX_KEY_AGE_MILLIS = 3 years, GRACE_PERIOD_MILLIS = 90
days
> - Add telemetry span for key generation operations
> 
> ### Storage Format (SharedPreferences)
> 
> {activeVersion: K002, keys: [{versionId: K001, createdAtMillis:
1673827200000, algorithm: AES/CBC/PKCS5Padding, keySize: 256,
isDeprecated: true}, {versionId: K002, createdAtMillis: 1740787200000,
algorithm: AES/CBC/PKCS5Padding, keySize: 256, isDeprecated: false}]}
> 
> ### Scope
> 
> In scope: KeyVersionRegistry class with all public APIs, key
generation using AES256SecretKeyGenerator, key wrapping/unwrapping with
AndroidKeyStore, metadata persistence in SharedPreferences, key file
management.
> Out of scope: Rotation orchestration logic (Broker), feature flag
integration (Broker), cross-app coordination (Broker).
> 
> ### Files to Create
> 
> -
common/src/main/java/com/microsoft/identity/common/crypto/KeyVersionRegistry.java
> 
> ### Acceptance Criteria
> 
> - getActiveKey() returns current active key metadata
> - generateNewKey() creates a new key with auto-incremented version ID
> - deprecateKey() marks a key as deprecated
> - loadSecretKey() correctly unwraps and returns the SecretKey
> - pruneExpiredKeys() removes keys older than 3 years + 90 days past
deprecation
> - Metadata persists across app restarts
> - Compile check passes: ./gradlew :common:compileDebugKotlin
> 
> Follow .github/copilot-instructions.md strictly.
> 


</details>



<!-- START COPILOT CODING AGENT TIPS -->
---

📍 Connect Copilot coding agent with [Jira](https://gh.io/cca-jira-docs),
[Azure Boards](https://gh.io/cca-azure-boards-docs) or
[Linear](https://gh.io/cca-linear-docs) to delegate work to Copilot in
one click without leaving your project management tool.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: fadidurah <88730756+fadidurah@users.noreply.github.com>
…ersionRegistry, Fixes AB#3546149 (#3034)

Adds ECS-controllable key lifetime thresholds for the symmetric key
rotation feature. Since `common` cannot import `BrokerFlight`, the two
age threshold flights are added to `CommonFlight` with matching string
keys — the established broker-override pattern where
`AndroidBrokerFlightsManager` injects itself at runtime, making
ECS-backed values transparently available through
`CommonFlightsManager`.

### CommonFlight (`common4j`)

Two new entries following the `ENABLE_PLAYSTORE_URL_LAUNCH` /
`ENABLE_WEBVIEW_MULTIPLE_WINDOWS` broker-override pattern:

```java
SYMMETRIC_KEY_MAX_AGE_DAYS("SymmetricKeyMaxAgeDays", 1095),  // 3yr SDL max
SYMMETRIC_KEY_RECOMMENDED_AGE_DAYS("SymmetricKeyRecommendedAgeDays", 365), // 1yr SDL recommendation
```

String keys match BrokerFlight's keys exactly so ECS overrides flow
through correctly.

### KeyVersionRegistry (`common`)

`pruneExpiredKeys()` now reads `SYMMETRIC_KEY_MAX_AGE_DAYS` dynamically
instead of using the hardcoded `MAX_KEY_AGE_MILLIS` constant:

```java
int maxAgeDays = CommonFlightsManager.INSTANCE.getFlightsProvider()
        .getIntValue(CommonFlight.SYMMETRIC_KEY_MAX_AGE_DAYS);
if (maxAgeDays <= 0) {
    // guard against invalid ECS value
    maxAgeDays = (int) CommonFlight.SYMMETRIC_KEY_MAX_AGE_DAYS.getDefaultValue();
}
final long maxAgeMillis = (long) maxAgeDays * 24 * 60 * 60 * 1000;
```

- `MAX_KEY_AGE_MILLIS` constant is retained (existing tests reference
it; its value equals the new flight default).
- Grace period (90 days) remains hardcoded.

<!-- START COPILOT CODING AGENT SUFFIX -->



<!-- START COPILOT ORIGINAL PROMPT -->



<details>

<summary>Original prompt</summary>

> ## [Symmetric Key Rotation][Common] Add key lifetime flights to
CommonFlight and wire into KeyVersionRegistry
> 
> Fixes
[AB#3546149](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3546149)
> 
> ### Objective
> 
> Add two key lifetime threshold flights to CommonFlight enum in
common4j, and update KeyVersionRegistry (on the fadi/secret-key-rotation
branch) to read these flights instead of using hardcoded constants. This
enables ECS-controlled key lifetime thresholds for the symmetric key
rotation feature.
> 
> ### Context
> 
> The symmetric key rotation feature defines 4 flights in BrokerFlight
(broker4j). Two of them the age threshold flights are also needed by
KeyVersionRegistry in common, which determines when deprecated keys are
ready for pruning. Since common cannot import BrokerFlight, these two
flights must also be added to CommonFlight with the SAME string keys
(this is the established broker-override pattern, used by
EnablePlaystoreUrlLaunch, EnableWebViewMultipleWindows, etc.).
> 
> At runtime in broker, AndroidBrokerFlightsManager injects itself into
CommonFlightsManager, so CommonFlight lookups go through the ECS-backed
provider which uses BrokerFlight's default values (overriding
CommonFlight's defaults).
> 
> ### Technical Requirements
> 
> #### 1. Add to CommonFlight enum (common4j)
> 
> File:
common/common4j/src/main/com/microsoft/identity/common/java/flighting/CommonFlight.java
> 
> Add these two entries before the semicolon at the end of the enum
(after ENABLE_WEBVIEW_MULTIPLE_WINDOWS):
> 
> `java
> /**
>  * Maximum key age in days before rotation is required.
>  * Default: 1095 (3 years per SDL maximum lifetime requirement).
> * When running in broker, BrokerFlight overrides this default via ECS.
>  */
> SYMMETRIC_KEY_MAX_AGE_DAYS(SymmetricKeyMaxAgeDays, 1095),
> 
> /**
>  * Recommended key age in days before rotation is triggered.
>  * Default: 365 (1 year per SDL recommendation).
> * When running in broker, BrokerFlight overrides this default via ECS.
>  */
> SYMMETRIC_KEY_RECOMMENDED_AGE_DAYS(SymmetricKeyRecommendedAgeDays,
365),
> `
> 
> #### 2. Update KeyVersionRegistry to use flights
> 
> File:
common/common/src/main/java/com/microsoft/identity/common/crypto/KeyVersionRegistry.kt
(on fadi/secret-key-rotation branch, merged via PR #3031)
> 
> In pruneExpiredKeys(), replace the hardcoded MAX_KEY_AGE_MILLIS
constant with a runtime flight lookup:
> 
> `kotlin
> val maxAgeDays = CommonFlightsManager.getFlightsProvider()
>     .getIntValue(CommonFlight.SYMMETRIC_KEY_MAX_AGE_DAYS)
> val maxAgeMillis = maxAgeDays.toLong() * 24 * 60 * 60 * 1000
> `
> 
> Use this maxAgeMillis + the existing grace period constant when
determining which deprecated keys to prune.
> 
> Remove or update the companion object constant MAX_KEY_AGE_MILLIS to
derive from the flight default, or remove it entirely and always read
the flight value.
> 
> ### Reference Files
> 
> - CommonFlight pattern:
common/common4j/src/main/com/microsoft/identity/common/java/flighting/CommonFlight.java
(see ENABLE_WEBVIEW_MULTIPLE_WINDOWS as example of broker-override
pattern)
> - CommonFlightsManager:
common/common4j/src/main/com/microsoft/identity/common/java/flighting/CommonFlightsManager.kt
> - IFlightsProvider.getIntValue():
common/common4j/src/main/com/microsoft/identity/common/java/flighting/IFlightsProvider.java
> - KeyVersionRegistry:
common/common/src/main/java/com/microsoft/identity/common/crypto/KeyVersionRegistry.kt
(on fadi/secret-key-rotation branch)
> 
> ### Acceptance Criteria
> 
> - Two new CommonFlight entries added with correct string keys matching
BrokerFlight
> - KeyVersionRegistry.pruneExpiredKeys() reads
SYMMETRIC_KEY_MAX_AGE_DAYS via CommonFlightsManager instead of hardcoded
constant
> - Grace period (90 days) can remain hardcoded
> - JavaDoc on all new enum entries
> - Copyright header preserved
> - Compile check passes: ./gradlew :common4j:compileJava; ./gradlew
:common:compileDebugKotlin
> 
> Follow .github/copilot-instructions.md strictly.
> 


</details>



<!-- START COPILOT CODING AGENT TIPS -->
---

💬 Send tasks to Copilot coding agent from
[Slack](https://gh.io/cca-slack-docs) and
[Teams](https://gh.io/cca-teams-docs) to turn conversations into code.
Copilot posts an update in your thread when it's finished.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: fadidurah <88730756+fadidurah@users.noreply.github.com>
…nFlight entry, Fixes AB#3546155 (#3033)

Adds comprehensive JVM-based unit tests for `KeyVersionRegistry`
(introduced in #3031) using Robolectric, covering the full symmetric key
lifecycle. Also removes the mistakenly added
`SYMMETRIC_KEY_RECOMMENDED_AGE_DAYS` flight from `CommonFlight` (it
belongs solely in `BrokerFlight`, where `RotationOrchestrator` reads
it).

## Test coverage

- **`generateNewKey`** — metadata fields (`versionId` K001/K002
sequencing, `algorithm`, `keySize`, `isDeprecated=false`), wrapped key
file creation, no auto-promotion to active
- **`getActiveKey` / `setActiveKey`** — null on empty registry,
promotion, throws on unknown version
- **`getKeyByVersion`** — hit and miss cases
- **`deprecateKey` / `getDeprecatedKeys`** — marks deprecated, does not
affect sibling keys, idempotent, throws on unknown version, filtered
list
- **`loadSecretKey`** — returns AES `SecretKey`, deterministic re-load
(same encoded bytes), throws on unknown version
- **`pruneExpiredKeys`** — age-based pruning, active key is never
pruned, key file deleted on prune, boundary condition (1 s under
threshold kept), expired non-deprecated keys are pruned
- **Multi-key lifecycle** — generate → deprecate → generate new → assert
both accessible with correct active/deprecated state

## Setup

- `@RunWith(RobolectricTestRunner::class)` + `@Config(sdk =
[Build.VERSION_CODES.P])`, consistent with existing module tests
- `overrideKeyCreationTimestamp()` helper manipulates SharedPreferences
directly to simulate aged keys without real-time delays
- `cleanUp()` runs before/after each test: clears SharedPreferences,
removes the AndroidKeyStore wrapping key pair, deletes wrapped key files
K001–K010
- `AndroidKeyStoreUtil` static methods are stubbed via `mockkStatic` to
avoid Robolectric `KeyStoreException` when generating RSA wrapping key
pairs

## CommonFlight cleanup

Removed `SYMMETRIC_KEY_RECOMMENDED_AGE_DAYS` from `CommonFlight`. This
entry was added by mistake in PR #3034 — it is only consumed by
`RotationOrchestrator` in broker (via `BrokerFlight`) and has no callers
in the common library. `SYMMETRIC_KEY_MAX_AGE_DAYS` remains as the sole
symmetric-key flight in `CommonFlight`, used by
`KeyVersionRegistry.pruneExpiredKeys()`.

<!-- START COPILOT CODING AGENT SUFFIX -->



<!-- START COPILOT ORIGINAL PROMPT -->



<details>

<summary>Original prompt</summary>

> ## [Symmetric Key Rotation][Common] Add KeyVersionRegistry unit tests
> 
> Fixes
[AB#3546155](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3546155)
> 
> ### Objective
> 
> Write comprehensive unit tests for KeyVersionRegistry (merged in PR
#3031 on the fadi/secret-key-rotation branch). Cover key generation,
deprecation, metadata persistence, pruning, and edge cases.
> 
> ### Context
> 
> KeyVersionRegistry was implemented in PR #3031 and merged into the
fadi/secret-key-rotation branch of
AzureAD/microsoft-authentication-library-common-for-android. It manages
versioned encryption keys with metadata stored in SharedPreferences.
Tests should validate the registry's core operations.
> 
> ### Technical Requirements
> 
> 1. Create test file:
common/common/src/test/java/com/microsoft/identity/common/crypto/KeyVersionRegistryTest.kt
> 2. Use JUnit 4/5 with Robolectric (existing pattern in common module
tests)
> 3. Test cases to implement:
> - generateNewKey creates key with isDeprecated=false and current
timestamp
> - generateNewKey assigns incrementing version IDs (K001, K002, etc.)
> - getActiveKey returns the most recently generated non-deprecated key
>    - getKeyByVersion returns correct metadata for known version
>    - getKeyByVersion returns null for unknown version
>    - deprecateKey marks key as deprecated (isDeprecated=true)
>    - deprecateKey does not affect other keys
>    - loadSecretKey returns AES SecretKey for valid version
>    - loadSecretKey throws for unknown version
> - pruneExpiredKeys removes deprecated keys older than retention period
> - pruneExpiredKeys does NOT remove non-deprecated (active) keys
regardless of age
> - pruneExpiredKeys does NOT remove deprecated keys within retention
period
> - Multiple key lifecycle: generate -> deprecate old -> generate new ->
verify both accessible
>    - getDeprecatedKeys returns only deprecated keys
> 4. Use @RunWith(RobolectricTestRunner.class) pattern
> 5. Use ApplicationProvider.getApplicationContext() for context
> 
> ### Reference Files
> 
> - KeyVersionRegistry on fadi/secret-key-rotation branch (PR #3031)
> - KeyMetadata data class (PR #3023)
> - Existing test patterns:
common/common/src/test/java/com/microsoft/identity/common/internal/cache/SharedPreferencesFileManagerTest.java
> - Design doc test cases: design-docs/[Android] Symmetric Key
Rotation/symmetric-key-rotation.md (Testing Strategy section)
> 
> ### Acceptance Criteria
> 
> - Test file created at specified path
> - All test cases pass: ./gradlew :common:testDebugUnitTest --tests
*.KeyVersionRegistryTest
> - Tests cover happy path and edge cases
> - Uses Robolectric for Android context
> - Copyright header included
> - KDoc comments on test class
> 
> Follow .github/copilot-instructions.md strictly.
> 


</details>



<!-- START COPILOT CODING AGENT TIPS -->
---

💬 Send tasks to Copilot coding agent from
[Slack](https://gh.io/cca-slack-docs) and
[Teams](https://gh.io/cca-teams-docs) to turn conversations into code.
Copilot posts an update in your thread when it's finished.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: fadidurah <88730756+fadidurah@users.noreply.github.com>
Co-authored-by: mohitc1 <22034758+mohitc1@users.noreply.github.com>
Co-authored-by: Mohit <mchand@microsoft.com>
Co-authored-by: pedro romero vargas <76129899+p3dr0rv@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: fadidurah <fadidurah@microsoft.com>
@fadidurah fadidurah requested review from a team as code owners March 23, 2026 05:47
Copilot AI review requested due to automatic review settings March 23, 2026 05:47
@github-actions
Copy link
Copy Markdown

❌ Work item link check failed. Description does not contain AB#{ID}.

Click here to Learn more.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces infrastructure for symmetric key versioning/rotation and expands telemetry capabilities (EU telemetry routing via TDBR claim + enhanced passkey/WebAuthn telemetry), along with a small WebView silent-flow behavior fix.

Changes:

  • Add a new KeyVersionRegistry (Android) and KeyMetadata (common4j) to support versioned symmetric keys and pruning/rotation policies (flighted via SYMMETRIC_KEY_MAX_AGE_DAYS).
  • Add EU telemetry region storage (EUClaimStorageUtil) keyed by tenant + emit tenant_id/new OTel attributes, and enhance passkey telemetry (origin + AAGUID extraction and emission).
  • Prevent silent embedded WebView flows from registering a back-press interceptor; add tests.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
common4j/src/test/com/microsoft/identity/common/java/opentelemetry/EUClaimStorageUtilTest.kt Unit tests for storing TDBR claim by tenant.
common4j/src/test/com/microsoft/identity/common/java/crypto/KeyMetadataTest.java Unit tests for KeyMetadata builder + JSON roundtrip/validation.
common4j/src/main/com/microsoft/identity/common/java/providers/microsoft/azureactivedirectory/ClientInfo.java Add xms_tdbr claim parsing + set tenant_id attribute when present.
common4j/src/main/com/microsoft/identity/common/java/opentelemetry/SpanName.java Add span name for key generation in KeyVersionRegistry.
common4j/src/main/com/microsoft/identity/common/java/opentelemetry/EUClaimStorageUtil.kt Store TDBR claim by tenant and attach tenant to current span.
common4j/src/main/com/microsoft/identity/common/java/opentelemetry/AttributeName.java Add correlation_id_v2, passkey_origin, passkey_aaguid.
common4j/src/main/com/microsoft/identity/common/java/flighting/CommonFlight.java Add SYMMETRIC_KEY_MAX_AGE_DAYS flight for key pruning/rotation.
common4j/src/main/com/microsoft/identity/common/java/crypto/KeyMetadata.java New immutable metadata model + JSON serialization for key versions.
common4j/src/main/com/microsoft/identity/common/java/constants/FidoConstants.kt Add WebAuthn registration + authenticator-data parsing constants.
common4j/src/main/com/microsoft/identity/common/java/constants/CborConstants.kt Add CBOR parsing constants used for attestation parsing.
common/src/test/java/com/microsoft/identity/common/internal/providers/oauth2/SilentWebViewAuthorizationFragmentTest.kt Robolectric test ensuring silent fragment doesn’t register back callback.
common/src/test/java/com/microsoft/identity/common/internal/fido/WebAuthnJsonUtilTest.kt Tests for extracting origin + AAGUID from registration responses/attestation.
common/src/test/java/com/microsoft/identity/common/crypto/KeyVersionRegistryTest.java Robolectric unit tests for KeyVersionRegistry behavior.
common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/SilentWebViewAuthorizationFragment.kt Skip back-press registration by not calling parent onViewCreated().
common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/PasskeyWebListener.kt Ensure CredentialManager calls are launched on main; thread model doc; pass OTel context.
common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/PasskeyReplyChannel.kt Record additional passkey telemetry (origin/AAGUID) asynchronously; propagate baggage/OTel context.
common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationActivity.java Add explicit getOtelContext() accessor.
common/src/main/java/com/microsoft/identity/common/internal/fido/WebAuthnJsonUtil.kt Add origin and AAGUID extraction logic; restructure util as object.
common/src/main/java/com/microsoft/identity/common/internal/fido/LegacyFidoActivityResultContract.kt Update import due to WebAuthnJsonUtil companion/object refactor.
common/src/main/java/com/microsoft/identity/common/crypto/KeyVersionRegistry.java New registry for generating/wrapping/pruning versioned symmetric keys with telemetry.
common/src/androidTest/java/com/microsoft/identity/common/crypto/KeyVersionRegistryTest.java Instrumented tests exercising AndroidKeyStore + file/prefs persistence.
common/build.gradle Add opentelemetry-extension-kotlin dependency for coroutine context propagation.
changelog.txt Add vNext entries for EU TDBR routing, silent WebView back behavior, and passkey telemetry.

Comment thread common/src/main/java/com/microsoft/identity/common/crypto/KeyVersionRegistry.java Outdated
Comment on lines +224 to +241
// Wrap and persist key material.
// RSA does not require an IV/AlgorithmParameters, so null is correct here.
final KeyPair keyPair = getOrCreateWrappingKeyPair();
final byte[] wrappedKey = AndroidKeyStoreUtil.wrap(secretKey, keyPair, WRAP_ALGORITHM, null);
FileUtil.writeDataToFile(wrappedKey, getKeyFile(newVersionId));

// Build and persist metadata. Explicitly set algorithm and keySize to match
// the AES-256 key generated above.
final KeyMetadata metadata = KeyMetadata.builder()
.versionId(newVersionId)
.createdAtMillis(System.currentTimeMillis())
.algorithm(KeyMetadata.DEFAULT_ALGORITHM)
.keySize(KeyMetadata.DEFAULT_KEY_SIZE)
.build();

state.keys.add(metadata);
saveState(state);

Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In generateNewKey(), the wrapped key file is written to disk before metadata is persisted. If saveState(state) throws (e.g., JSON/SharedPreferences failure), the key file can be left orphaned on disk without a corresponding metadata entry. Impact: leaked key blobs and inconsistent registry state that may accumulate over time. Recommendation: make the operation more atomic (e.g., write metadata first, or delete the newly written key file in the catch path when state persistence fails).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@fadidurah fadidurah Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot is it better that a key be orphaned on disk, or the alternative of key ketadata is persisted first, but then key file write fails, in which case our persisted data has a noisy failure?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Comment thread common/src/main/java/com/microsoft/identity/common/crypto/KeyVersionRegistry.java Outdated
…um (#3049)

Broker's `RotationOrchestrator` (PR #119) requires two span names for
symmetric key rotation telemetry that are currently hardcoded. This adds
them centrally to `SpanName` in common4j.

## Changes

- **`SpanName.java`** — appended two new entries after
`KeyVersionRegistryGenerateKey`:
- `SymmetricKeyRotationCheck` — span for key rotation eligibility check
on broker startup
  - `SymmetricKeyRotation` — span for key rotation execution

```java
/**
 * Span name for symmetric key rotation check on broker startup.
 */
SymmetricKeyRotationCheck,
/**
 * Span name for symmetric key rotation execution.
 */
SymmetricKeyRotation
```

> [!WARNING]
>
> <details>
> <summary>Firewall rules blocked me from connecting to one or more
addresses (expand for details)</summary>
>
> #### I tried to connect to the following addresses, but was blocked by
firewall rules:
>
> - `www.puppycrawl.com`
> - Triggering command:
`/opt/hostedtoolcache/CodeQL/2.24.3/x64/codeql/tools/linux64/java/bin/java
/opt/hostedtoolcache/CodeQL/2.24.3/x64/codeql/tools/linux64/java/bin/java
-jar
/opt/hostedtoolcache/CodeQL/2.24.3/x64/codeql/xml/tools/xml-extractor.jar
--fileList=/tmp/codeql-scratch-b2678d0398b00160/dbs/java/working/files-to-index778550329887067804.list
--sourceArchiveDir=/tmp/codeql-scratch-b2678d0398b00160/dbs/java/src
--outputDir=/tmp/codeql-scratch-b2678d0398b00160/dbs/java/trap/java`
(dns block)
>
> If you need me to access, download, or install something from one of
these locations, you can either:
>
> - Configure [Actions setup
steps](https://gh.io/copilot/actions-setup-steps) to set up my
environment, which run before the firewall is enabled
> - Add the appropriate URLs or hosts to the custom allowlist in this
repository's [Copilot coding agent
settings](https://github.com/AzureAD/microsoft-authentication-library-common-for-android/settings/copilot/coding_agent)
(admins only)
>
> </details>

<!-- START COPILOT CODING AGENT SUFFIX -->



<!-- START COPILOT ORIGINAL PROMPT -->



<details>

<summary>Original prompt</summary>

> ## [Symmetric Key Rotation][Common] Add key rotation span names to
SpanName enum
> 
> ### Objective
> 
> Add two span name entries to the SpanName enum in common4j for
symmetric key rotation telemetry. These are needed by
RotationOrchestrator in broker (PR #119) which currently hardcodes them.
> 
> ### Technical Requirements
> 
> File:
common/common4j/src/main/com/microsoft/identity/common/java/opentelemetry/SpanName.java
> 
> Add these two entries after KeyVersionRegistryGenerateKey (note: need
to add a comma after KeyVersionRegistryGenerateKey first):
> 
> `java
> /**
>  * Span name for symmetric key rotation check on broker startup.
>  */
> SymmetricKeyRotationCheck,
> /**
>  * Span name for symmetric key rotation execution.
>  */
> SymmetricKeyRotation
> `
> 
> ### Acceptance Criteria
> 
> - Two new SpanName enum values added with JavaDoc
> - Comma added after existing KeyVersionRegistryGenerateKey entry
> - Copyright header preserved
> - Compile check passes: ./gradlew :common4j:compileJava
> 
> Follow .github/copilot-instructions.md strictly.
> 


</details>



<!-- START COPILOT CODING AGENT TIPS -->
---

📍 Connect Copilot coding agent with [Jira](https://gh.io/cca-jira-docs),
[Azure Boards](https://gh.io/cca-azure-boards-docs) or
[Linear](https://gh.io/cca-linear-docs) to delegate work to Copilot in
one click without leaving your project management tool.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: fadidurah <88730756+fadidurah@users.noreply.github.com>
@fadidurah fadidurah changed the title Fadi/secret key rotation Broker Secret Key Rotation Mar 23, 2026
@github-actions
Copy link
Copy Markdown

✅ Work item link check complete. Description contains link AB#2956404 to an Azure Boards work item.

@github-actions github-actions Bot changed the title Broker Secret Key Rotation Broker Secret Key Rotation, Fixes AB#2956404 Mar 24, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Comment thread common4j/src/main/com/microsoft/identity/common/java/crypto/KeyMetadata.java Outdated
Comment on lines +548 to +556
private KeyPair getOrCreateWrappingKeyPair() throws ClientException {
final String methodTag = TAG + ":getOrCreateWrappingKeyPair";
KeyPair keyPair = AndroidKeyStoreUtil.readKey(WRAPPING_KEY_ALIAS);
if (keyPair == null) {
Logger.info(methodTag, "No existing wrapping key pair, generating a new one.");
keyPair = generateNewWrappingKeyPair();
}
return keyPair;
}
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: High – Wrapping key pair recovery can permanently orphan existing wrapped keys. getOrCreateWrappingKeyPair() generates a new RSA key pair whenever AndroidKeyStoreUtil.readKey(WRAPPING_KEY_ALIAS) returns null; if brokerks_metadata already contains existing key versions, those wrapped key blobs will no longer be unwrap-able, breaking decryption of existing encrypted data and leaving the registry in an inconsistent state. Recommendation: if the wrapping key pair is missing while any key versions exist, fail fast (throw a ClientException that the caller can handle by wiping caches) or explicitly wipe all wrapped key files + metadata before generating a new wrapping key pair, so the system does not silently keep undecryptable historical keys.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@fadidurah fadidurah Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot realistically, when would AndroidKeyStoreUtil.readKey(WRAPPING_KEY_ALIAS) ever return null if brokerks_metadata already contains some existing key that were wrapped by it?

Comment thread common4j/src/main/com/microsoft/identity/common/java/crypto/KeyMetadata.java Outdated
…yMetadata.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 24, 2026

@fadidurah I've opened a new pull request, #3054, to work on those changes. Once the pull request is ready, I'll request review from you.

fadidurah and others added 6 commits March 24, 2026 17:48
…yMetadata.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…_AGE_DAYS returns 0 or negative (#3054)

The `pruneExpiredKeys()` fallback path for invalid ECS values
(`maxAgeDays <= 0`) had no test coverage, meaning regressions (e.g.,
pruning all keys when the flight returns 0) would go unnoticed.

## Changes

- **Two new tests** in `KeyVersionRegistryTest`:
- `pruneExpiredKeys_usesDefaultWhenFlightReturnsZero` — asserts a
recently-created key survives when the flight returns `0`
- `pruneExpiredKeys_usesDefaultWhenFlightReturnsNegative` — same for
`-1`

- **`buildFlightsManagerWithMaxAgeDays(int)`** helper — concrete
`IFlightsManager`/`IFlightsProvider` implementation that returns the
specified value for `SYMMETRIC_KEY_MAX_AGE_DAYS` and each flight's own
default otherwise; injected via
`CommonFlightsManager.INSTANCE.initializeCommonFlightsManager(...)`

- **`tearDown()`** now calls
`CommonFlightsManager.INSTANCE.resetFlightsManager()` to prevent test
pollution

```java
@test
public void pruneExpiredKeys_usesDefaultWhenFlightReturnsZero() throws ClientException {
    registry.generateNewKey();
    CommonFlightsManager.INSTANCE.initializeCommonFlightsManager(buildFlightsManagerWithMaxAgeDays(0));
    registry.pruneExpiredKeys();
    assertNotNull("Key should not be pruned when flight returns 0; default should be used",
            registry.getKeyByVersion("K001"));
}
```

<!-- START COPILOT CODING AGENT TIPS -->
---

💡 You can make Copilot smarter by setting up custom instructions,
customizing its development environment and configuring Model Context
Protocol (MCP) servers. Learn more [Copilot coding agent
tips](https://gh.io/copilot-coding-agent-tips) in the docs.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: fadidurah <88730756+fadidurah@users.noreply.github.com>
@fadidurah
Copy link
Copy Markdown
Contributor Author

fadidurah commented Mar 24, 2026

@copilot please respond to my two recent comment replies that tagged you

Comment thread common4j/src/main/com/microsoft/identity/common/java/crypto/KeyMetadata.java Outdated
Comment thread common4j/src/main/com/microsoft/identity/common/java/crypto/KeyMetadata.java Outdated
Comment thread common/src/main/java/com/microsoft/identity/common/crypto/KeyVersionRegistry.java Outdated
Comment thread common/src/main/java/com/microsoft/identity/common/crypto/KeyVersionRegistry.java Outdated
Comment thread common/src/main/java/com/microsoft/identity/common/crypto/KeyVersionRegistry.java Outdated
Comment thread common/src/main/java/com/microsoft/identity/common/crypto/KeyVersionRegistry.java Outdated
Comment thread common/src/main/java/com/microsoft/identity/common/crypto/KeyVersionRegistry.java Outdated
Comment thread common/src/main/java/com/microsoft/identity/common/crypto/KeyVersionRegistry.java Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants