Skip to content

Commit

Permalink
[KeyVault] - Address archboard review feedback (#18319)
Browse files Browse the repository at this point in the history
## What

- return bytes from getRandomBytes
- rename KeyReleasePolicy.data to KeyReleasePolicy.encodedPolicy
- rename target to targetAttestationToken

## Why

These address recent architecture board review comments and will improve the
overall experience when using these new APIs.

Resolves #18307
Resolves #18317
  • Loading branch information
maorleger authored Oct 22, 2021
1 parent 66a7eb4 commit f26bb90
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 41 deletions.
5 changes: 5 additions & 0 deletions sdk/keyvault/keyvault-keys/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@

### Breaking Changes

- `KeyClient.getRandomBytes` will now return the generated bytes directly instead of wrapping them in a `RandomBytes` model.
- Since it's no longer used, `RandomBytes` has been removed from the public API.
- `KeyReleasePolicy.data` has been renamed to `KeyReleasePolicy.encodedPolicy`.
- `KeyClient.releaseKey`'s `target` parameter has been renamed to `targetAttestationToken`.

### Bugs Fixed

### Other Changes
Expand Down
11 changes: 3 additions & 8 deletions sdk/keyvault/keyvault-keys/review/keyvault-keys.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,13 @@ export class KeyClient {
getDeletedKey(name: string, options?: GetDeletedKeyOptions): Promise<DeletedKey>;
getKey(name: string, options?: GetKeyOptions): Promise<KeyVaultKey>;
getKeyRotationPolicy(name: string, options?: GetKeyRotationPolicyOptions): Promise<KeyRotationPolicy>;
getRandomBytes(count: number, options?: GetRandomBytesOptions): Promise<RandomBytes>;
getRandomBytes(count: number, options?: GetRandomBytesOptions): Promise<Uint8Array>;
importKey(name: string, key: JsonWebKey_2, options?: ImportKeyOptions): Promise<KeyVaultKey>;
listDeletedKeys(options?: ListDeletedKeysOptions): PagedAsyncIterableIterator<DeletedKey>;
listPropertiesOfKeys(options?: ListPropertiesOfKeysOptions): PagedAsyncIterableIterator<KeyProperties>;
listPropertiesOfKeyVersions(name: string, options?: ListPropertiesOfKeyVersionsOptions): PagedAsyncIterableIterator<KeyProperties>;
purgeDeletedKey(name: string, options?: PurgeDeletedKeyOptions): Promise<void>;
releaseKey(name: string, target: string, options?: ReleaseKeyOptions): Promise<ReleaseKeyResult>;
releaseKey(name: string, targetAttestationToken: string, options?: ReleaseKeyOptions): Promise<ReleaseKeyResult>;
restoreKeyBackup(backup: Uint8Array, options?: RestoreKeyBackupOptions): Promise<KeyVaultKey>;
rotateKey(name: string, options?: RotateKeyOptions): Promise<KeyVaultKey>;
updateKeyProperties(name: string, keyVersion: string, options?: UpdateKeyPropertiesOptions): Promise<KeyVaultKey>;
Expand Down Expand Up @@ -300,7 +300,7 @@ export interface KeyProperties {
// @public
export interface KeyReleasePolicy {
contentType?: string;
data?: Uint8Array;
encodedPolicy?: Uint8Array;
}

// @public
Expand Down Expand Up @@ -464,11 +464,6 @@ export { PollOperationState }
export interface PurgeDeletedKeyOptions extends coreHttp.OperationOptions {
}

// @public
export interface RandomBytes {
bytes: Uint8Array;
}

// @public
export interface ReleaseKeyOptions extends coreHttp.OperationOptions {
algorithm?: KeyExportEncryptionAlgorithm;
Expand Down
2 changes: 1 addition & 1 deletion sdk/keyvault/keyvault-keys/src/generated/models/index.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion sdk/keyvault/keyvault-keys/src/generated/models/mappers.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 5 additions & 7 deletions sdk/keyvault/keyvault-keys/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ import {
ReleaseKeyResult,
KeyReleasePolicy,
KeyExportEncryptionAlgorithm,
RandomBytes,
GetCryptographyClientOptions,
RotateKeyOptions,
UpdateKeyRotationPolicyOptions,
Expand Down Expand Up @@ -153,7 +152,6 @@ export {
GetDeletedKeyOptions,
GetKeyOptions,
GetRandomBytesOptions,
RandomBytes,
ImportKeyOptions,
JsonWebKey,
KeyCurveName,
Expand Down Expand Up @@ -784,10 +782,10 @@ export class KeyClient {
* @param count - The number of bytes to generate between 1 and 128 inclusive.
* @param options - The optional parameters.
*/
public getRandomBytes(count: number, options: GetRandomBytesOptions = {}): Promise<RandomBytes> {
public getRandomBytes(count: number, options: GetRandomBytesOptions = {}): Promise<Uint8Array> {
return withTrace("getRandomBytes", options, async (updatedOptions) => {
const response = await this.client.getRandomBytes(this.vaultUrl, count, updatedOptions);
return { bytes: response.value! };
return response.value!;
});
}

Expand Down Expand Up @@ -822,12 +820,12 @@ export class KeyClient {
* ```
*
* @param name - The name of the key.
* @param target - The attestation assertion for the target of the key release.
* @param targetAttestationToken - The attestation assertion for the target of the key release.
* @param options - The optional parameters.
*/
public releaseKey(
name: string,
target: string,
targetAttestationToken: string,
options: ReleaseKeyOptions = {}
): Promise<ReleaseKeyResult> {
return withTrace("releaseKey", options, async (updatedOptions) => {
Expand All @@ -836,7 +834,7 @@ export class KeyClient {
this.vaultUrl,
name,
options?.version || "",
target,
targetAttestationToken,
{
enc: algorithm,
nonce,
Expand Down
10 changes: 1 addition & 9 deletions sdk/keyvault/keyvault-keys/src/keysModels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ export interface KeyReleasePolicy {
contentType?: string;

/** Blob encoding the policy rules under which the key can be released. */
data?: Uint8Array;
encodedPolicy?: Uint8Array;
}

/**
Expand Down Expand Up @@ -596,14 +596,6 @@ export enum KnownKeyExportEncryptionAlgorithm {
export type KeyExportEncryptionAlgorithm = string;
/* eslint-enable tsdoc/syntax */

/**
* Result of the {@link KeyClient.getRandomBytes} operation.
*/
export interface RandomBytes {
/** The random bytes returned by the service. */
bytes: Uint8Array;
}

/**
* Options for {@link KeyClient.getCryptographyClient}.
*/
Expand Down
12 changes: 10 additions & 2 deletions sdk/keyvault/keyvault-keys/swagger/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ directive:
### Update swagger enum values for LifetimeActionsType to reflect what the service actually returns
There is an ongoing thread about changing the swagger or returning lowercase values for enum values.
```yaml
directive:
- from: swagger-document
Expand All @@ -48,3 +46,13 @@ directive:
$.values[0].value = "Rotate";
$.values[1].value = "Notify";
```
### Rename KeyReleasePolicy.data to KeyReleasePolicy.encodedPolicy
```yaml
directive:
- from: swagger-document
where: $.definitions.KeyReleasePolicy.properties.data
transform: >
$["x-ms-client-name"] = "encodedPolicy";
```
28 changes: 15 additions & 13 deletions sdk/keyvault/keyvault-keys/test/public/keyClient.hsm.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ onVersions({ minVer: "7.2" }).describe(
onVersions({ minVer: "7.3-preview" }).describe("getRandomBytes", () => {
it("can return the required number of bytes", async () => {
const result = await hsmClient.getRandomBytes(10);
assert.exists(result.bytes);
assert.equal(result.bytes.length, 10);
assert.exists(result);
assert.equal(result.length, 10);
});

it("returns an error when bytes is out of range", async () => {
Expand Down Expand Up @@ -106,13 +106,13 @@ onVersions({ minVer: "7.2" }).describe(
const keyName = recorder.getUniqueName("exportkey");
const createdKey = await hsmClient.createKey(keyName, "RSA", {
exportable: true,
releasePolicy: { data: encodedReleasePolicy },
releasePolicy: { encodedPolicy: encodedReleasePolicy },
keyOps: ["encrypt", "decrypt"]
});

assert.exists(createdKey.properties.releasePolicy?.data);
assert.exists(createdKey.properties.releasePolicy?.encodedPolicy);
assert.isNotEmpty(
JSON.parse(uint8ArrayToString(createdKey.properties.releasePolicy!.data!))
JSON.parse(uint8ArrayToString(createdKey.properties.releasePolicy!.encodedPolicy!))
);
assert.isTrue(createdKey.properties.exportable);
const releaseResult = await hsmClient.releaseKey(keyName, attestation);
Expand All @@ -125,12 +125,12 @@ onVersions({ minVer: "7.2" }).describe(

const importedKey = await hsmClient.importKey(keyName, createRsaKey(), {
exportable: true,
releasePolicy: { data: encodedReleasePolicy }
releasePolicy: { encodedPolicy: encodedReleasePolicy }
});

assert.exists(importedKey.properties.releasePolicy?.data);
assert.exists(importedKey.properties.releasePolicy?.encodedPolicy);
assert.isNotEmpty(
JSON.parse(uint8ArrayToString(importedKey.properties.releasePolicy!.data!))
JSON.parse(uint8ArrayToString(importedKey.properties.releasePolicy!.encodedPolicy!))
);
const releaseResult = await hsmClient.releaseKey(keyName, attestation, {
version: importedKey.properties.version,
Expand All @@ -145,7 +145,7 @@ onVersions({ minVer: "7.2" }).describe(
const keyName = recorder.getUniqueName("exportkey");
const createdKey = await hsmClient.createKey(keyName, "RSA", {
exportable: true,
releasePolicy: { data: encodedReleasePolicy },
releasePolicy: { encodedPolicy: encodedReleasePolicy },
keyOps: ["encrypt", "decrypt"]
});

Expand All @@ -165,12 +165,12 @@ onVersions({ minVer: "7.2" }).describe(
version: "1.0"
};
const updatedKey = await hsmClient.updateKeyProperties(createdKey.name, {
releasePolicy: { data: stringToUint8Array(JSON.stringify(newReleasePolicy)) }
releasePolicy: { encodedPolicy: stringToUint8Array(JSON.stringify(newReleasePolicy)) }
});

assert.exists(updatedKey.properties.releasePolicy?.data);
assert.exists(updatedKey.properties.releasePolicy?.encodedPolicy);
const decodedReleasePolicy = JSON.parse(
uint8ArrayToString(updatedKey.properties.releasePolicy!.data!)
uint8ArrayToString(updatedKey.properties.releasePolicy!.encodedPolicy!)
);

// Note: the service will parse the policy and return a different shape, for example: { "claim": "sdk-test", "equals": "false" } in this test.
Expand All @@ -188,7 +188,9 @@ onVersions({ minVer: "7.2" }).describe(
it("errors when a key has a release policy but is not exportable", async () => {
const keyName = recorder.getUniqueName("policynonexportable");
await assert.isRejected(
hsmClient.createRsaKey(keyName, { releasePolicy: { data: encodedReleasePolicy } }),
hsmClient.createRsaKey(keyName, {
releasePolicy: { encodedPolicy: encodedReleasePolicy }
}),
/exportable/i
);
});
Expand Down

0 comments on commit f26bb90

Please sign in to comment.