Add KeyMetadata data class for symmetric key rotation, Fixes AB#3546146#3023
Conversation
|
✅ Work item link check complete. Description contains link AB#3546146 to an Azure Boards work item. |
Co-authored-by: fadidurah <88730756+fadidurah@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a foundational immutable metadata model in common4j to represent symmetric key versions for upcoming key-rotation work (intended to be consumed by a KeyVersionRegistry and broker encryption managers).
Changes:
- Introduces
KeyMetadata(immutable, builder-based) with JSON serialization/deserialization for persistence. - Adds unit tests validating builder behavior and JSON round-tripping.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| common4j/src/main/com/microsoft/identity/common/java/crypto/KeyMetadata.java | New immutable key-version metadata type with a Builder and org.json (de)serialization helpers. |
| common4j/src/test/com/microsoft/identity/common/java/crypto/KeyMetadataTest.java | Unit tests for builder defaults/validation and JSON round-trip behavior. |
common4j/src/main/com/microsoft/identity/common/java/crypto/KeyMetadata.java
Outdated
Show resolved
Hide resolved
common4j/src/main/com/microsoft/identity/common/java/crypto/KeyMetadata.java
Outdated
Show resolved
Hide resolved
common4j/src/test/com/microsoft/identity/common/java/crypto/KeyMetadataTest.java
Show resolved
Hide resolved
|
@copilot we can make this class much more concise using Lombok's Getter and Builder annotations |
Co-authored-by: fadidurah <88730756+fadidurah@users.noreply.github.com>
Done in 7e519e3. The class now uses |
…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>
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.javafinalclass using Lombok@Getterand@Builderannotations to eliminate boilerplateversionId,createdAtMillis,algorithm(defaultAES/CBC/PKCS5Padding),keySize(default 256),deprecatedversionIdthrowsIllegalStateException; non-positivekeySizethrowsIllegalArgumentExceptiontoJson()/fromJson(String)viaorg.jsonfor persistence by the KeyVersionRegistryfromJson()usesopt*APIs for optional fields (algorithm,keySize,deprecated) so deserialization is backward-compatible with older persisted JSONcommon4j/.../crypto/KeyMetadataTest.javaUnit 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.
Original prompt
🔒 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.