From c138441b4e885e8de263b9bfad896d95e1896886 Mon Sep 17 00:00:00 2001 From: Mary Gao Date: Wed, 30 Aug 2023 17:45:34 +0800 Subject: [PATCH] Fix credential issue in hlc (#1956) * Fix credential issue * Update the default value if addcredential is true * Update the UTs * Update the unit test * update the test cases --- .../src/generators/clientFileGenerator.ts | 32 ++++-- .../src/utils/schemaHelpers.ts | 26 +++-- .../test/commands/test-swagger-gen.ts | 2 +- .../test/integration/corecampat.spec.ts | 21 +++- .../generated/corecompattest/package.json | 1 + .../generated/corecompattest/src/petStore.ts | 17 ++- .../files/case-hlcReadme-without-subid.md | 108 ++++++++++++++++++ .../static/readmeFileGenerator.spec.ts | 62 ++++++++++ 8 files changed, 239 insertions(+), 30 deletions(-) create mode 100644 packages/autorest.typescript/test/unit/generators/static/files/case-hlcReadme-without-subid.md diff --git a/packages/autorest.typescript/src/generators/clientFileGenerator.ts b/packages/autorest.typescript/src/generators/clientFileGenerator.ts index 42986305ff..3cce12bc59 100644 --- a/packages/autorest.typescript/src/generators/clientFileGenerator.ts +++ b/packages/autorest.typescript/src/generators/clientFileGenerator.ts @@ -220,7 +220,12 @@ export function generateClient(clientDetails: ClientDetails, project: Project) { apiVersionParams && apiVersionParams.length === 1 ? apiVersionParams[0] : undefined; - writeClassProperties(clientClass, clientParams, importedModels, clientDetails.hasTenantLevelOperation); + writeClassProperties( + clientClass, + clientParams, + importedModels, + clientDetails.hasTenantLevelOperation + ); writeConstructor(clientDetails, clientClass, importedModels, apiVersionParam); if (useCoreV2 && apiVersionParam) { writeCustomApiVersion(clientClass); @@ -349,7 +354,7 @@ function writeConstructor( type: optionsParameterType } ] - } + }; let clientConstructor; if (clientDetails.hasTenantLevelOperation) { @@ -376,7 +381,7 @@ function writeConstructor( } ] }); - clientConstructor.addOverload(clientConstructorInitial) + clientConstructor.addOverload(clientConstructorInitial); clientConstructor.addOverload({ parameters: [ ...requiredParams @@ -396,7 +401,9 @@ function writeConstructor( ] }); } else { - clientConstructor = classDeclaration.addConstructor(clientConstructorInitial); + clientConstructor = classDeclaration.addConstructor( + clientConstructorInitial + ); } const { useCoreV2 } = getAutorestOptions(); @@ -647,11 +654,8 @@ function getTrack2DefaultContent( packageDetails: PackageDetails, clientDetails: ClientDetails ) { - const { - azureArm, - allowInsecureConnection, - addCredentials - } = getAutorestOptions(); + const { azureArm, allowInsecureConnection } = getAutorestOptions(); + const { addCredentials } = getSecurityInfoFromModel(clientDetails.security); const overloadDefaults = ` let subscriptionId: string | undefined; @@ -663,7 +667,9 @@ function getTrack2DefaultContent( } `; - const defaultContent = `${clientDetails.hasTenantLevelOperation ? overloadDefaults: ""} + const defaultContent = `${ + clientDetails.hasTenantLevelOperation ? overloadDefaults : "" + } // Initializing default values for options if (!options) { options = {}; @@ -731,8 +737,10 @@ function writeDefaultOptions( hasLro: boolean, clientDetails: ClientDetails ) { - const { useCoreV2, packageDetails, addCredentials } = getAutorestOptions(); - const { credentialScopes } = getSecurityInfoFromModel(clientDetails.security); + const { useCoreV2, packageDetails } = getAutorestOptions(); + const { credentialScopes, addCredentials } = getSecurityInfoFromModel( + clientDetails.security + ); const credentialScopesValues = getCredentialScopesValue( credentialScopes, diff --git a/packages/autorest.typescript/src/utils/schemaHelpers.ts b/packages/autorest.typescript/src/utils/schemaHelpers.ts index d7ca5712e8..491fc60f5b 100644 --- a/packages/autorest.typescript/src/utils/schemaHelpers.ts +++ b/packages/autorest.typescript/src/utils/schemaHelpers.ts @@ -154,7 +154,7 @@ export function getTypeForSchema( } break; case SchemaType.AnyObject: - kind = PropertyKind.Dictionary + kind = PropertyKind.Dictionary; typeName = `Record`; break; case SchemaType.Number: @@ -236,8 +236,8 @@ export function getSecurityInfoFromModel(security: Security) { for (const securitySchema of security.schemes) { if (securitySchema.type === "OAuth2") { (securitySchema as OAuth2SecurityScheme).scopes.forEach(scope => { - const scopes = scope.split(','); - for(const scope of scopes) { + const scopes = scope.split(","); + for (const scope of scopes) { credentialScopes.add(scope); } }); @@ -258,13 +258,21 @@ export function getSecurityInfoFromModel(security: Security) { ); } } - const scopes: string[] = []; - credentialScopes.forEach(item => { - scopes.push(item); - }); + const scopes: string[] = [...credentialScopes]; + let refinedAddCredentials = + addCredentials === false ? false : security.authenticationRequired; + // Add default info if explictly set the addCredentials as true + if ( + addCredentials === true && + !security.authenticationRequired && + scopes.length === 0 && + credentialKeyHeaderName === "" + ) { + refinedAddCredentials = true; + scopes.push("https://management.azure.com/.default"); + } return { - addCredentials: - addCredentials === false ? false : security.authenticationRequired, + addCredentials: refinedAddCredentials, credentialScopes: scopes, credentialKeyHeaderName: credentialKeyHeaderName }; diff --git a/packages/autorest.typescript/test/commands/test-swagger-gen.ts b/packages/autorest.typescript/test/commands/test-swagger-gen.ts index 604c29430c..501c2bacf3 100644 --- a/packages/autorest.typescript/test/commands/test-swagger-gen.ts +++ b/packages/autorest.typescript/test/commands/test-swagger-gen.ts @@ -862,7 +862,7 @@ let testSwaggers: { [name: string]: SwaggerConfig } = { packageName: "petstore", useCoreV2: true, allowInsecureConnection: true, - addCredentials: false, + addCredentials: true, isTestPackage: true, coreHttpCompatMode: true }, diff --git a/packages/autorest.typescript/test/integration/corecampat.spec.ts b/packages/autorest.typescript/test/integration/corecampat.spec.ts index 8d8bfc4a4f..917ac6f5c9 100644 --- a/packages/autorest.typescript/test/integration/corecampat.spec.ts +++ b/packages/autorest.typescript/test/integration/corecampat.spec.ts @@ -1,3 +1,4 @@ +import { now } from "lodash"; import { PetStore as PetStoreClient } from "./generated/corecompattest/src"; import { assert } from "chai"; @@ -5,14 +6,22 @@ describe("Integration tests for Core Comapability", () => { let client: PetStoreClient; it("should create a client successfully", async () => { - client = new PetStoreClient({ - keepAliveOptions: { - enable: true + client = new PetStoreClient( + { + getToken: async () => { + assert.equal(1, 1); + return { token: "FakeToken", expiresOnTimestamp: now() }; + } }, - redirectOptions: { - handleRedirects: true + { + keepAliveOptions: { + enable: true + }, + redirectOptions: { + handleRedirects: true + } } - }); + ); assert.notEqual(client, null); }); }); diff --git a/packages/autorest.typescript/test/integration/generated/corecompattest/package.json b/packages/autorest.typescript/test/integration/generated/corecompattest/package.json index 932bd30ec5..3c302bbd70 100644 --- a/packages/autorest.typescript/test/integration/generated/corecompattest/package.json +++ b/packages/autorest.typescript/test/integration/generated/corecompattest/package.json @@ -7,6 +7,7 @@ "engines": { "node": ">=14.0.0" }, "dependencies": { "@azure/core-client": "^1.7.0", + "@azure/core-auth": "^1.3.0", "@azure/core-http-compat": "^1.2.0", "@azure/core-rest-pipeline": "^1.12.0", "tslib": "^2.2.0" diff --git a/packages/autorest.typescript/test/integration/generated/corecompattest/src/petStore.ts b/packages/autorest.typescript/test/integration/generated/corecompattest/src/petStore.ts index f93036a6a0..38fb01486e 100644 --- a/packages/autorest.typescript/test/integration/generated/corecompattest/src/petStore.ts +++ b/packages/autorest.typescript/test/integration/generated/corecompattest/src/petStore.ts @@ -1,5 +1,6 @@ import * as coreClient from "@azure/core-client"; import * as coreHttpCompat from "@azure/core-http-compat"; +import * as coreAuth from "@azure/core-auth"; import * as Parameters from "./models/parameters"; import * as Mappers from "./models/mappers"; import { @@ -43,15 +44,24 @@ export class PetStore extends coreHttpCompat.ExtendedServiceClient { /** * Initializes a new instance of the PetStore class. + * @param credentials Subscription credentials which uniquely identify client subscription. * @param options The parameter options */ - constructor(options?: PetStoreOptionalParams) { + constructor( + credentials: coreAuth.TokenCredential, + options?: PetStoreOptionalParams + ) { + if (credentials === undefined) { + throw new Error("'credentials' cannot be null"); + } + // Initializing default values for options if (!options) { options = {}; } const defaults: PetStoreOptionalParams = { - requestContentType: "application/json; charset=utf-8" + requestContentType: "application/json; charset=utf-8", + credential: credentials }; const packageDetails = `azsdk-js-petstore/1.0.0-preview1`; @@ -60,6 +70,9 @@ export class PetStore extends coreHttpCompat.ExtendedServiceClient { ? `${options.userAgentOptions.userAgentPrefix} ${packageDetails}` : `${packageDetails}`; + if (!options.credentialScopes) { + options.credentialScopes = ["https://management.azure.com/.default"]; + } const optionsWithDefaults = { ...defaults, ...options, diff --git a/packages/autorest.typescript/test/unit/generators/static/files/case-hlcReadme-without-subid.md b/packages/autorest.typescript/test/unit/generators/static/files/case-hlcReadme-without-subid.md new file mode 100644 index 0000000000..66592d74b7 --- /dev/null +++ b/packages/autorest.typescript/test/unit/generators/static/files/case-hlcReadme-without-subid.md @@ -0,0 +1,108 @@ +# Azure KustoManagement client library for JavaScript + +This package contains an isomorphic SDK (runs both in Node.js and in browsers) for Azure KustoManagement client. + +The Azure Kusto management API provides a RESTful set of web services that interact with Azure Kusto services to manage your clusters and databases. The API enables you to create, update, and delete clusters and databases. + +[Source code](https://github.com/Azure/azure-sdk-for-js/tree/main/sdk/kusto/arm-kusto) | +[Package (NPM)](https://www.npmjs.com/package/@azure/arm-kusto) | +[API reference documentation](https://docs.microsoft.com/javascript/api/@azure/arm-kusto?view=azure-node-preview) | +[Samples](https://github.com/Azure-Samples/azure-samples-js-management) + +## Getting started + +### Currently supported environments + +- [LTS versions of Node.js](https://github.com/nodejs/release#release-schedule) +- Latest versions of Safari, Chrome, Edge and Firefox. + +See our [support policy](https://github.com/Azure/azure-sdk-for-js/blob/main/SUPPORT.md) for more details. + +### Prerequisites + +- An [Azure subscription][azure_sub]. + +### Install the `@azure/arm-kusto` package + +Install the Azure KustoManagement client library for JavaScript with `npm`: + +```bash +npm install @azure/arm-kusto +``` + +### Create and authenticate a `KustoManagementClient` + +To create a client object to access the Azure KustoManagement API, you will need the `endpoint` of your Azure KustoManagement resource and a `credential`. The Azure KustoManagement client can use Azure Active Directory credentials to authenticate. +You can find the endpoint for your Azure KustoManagement resource in the [Azure Portal][azure_portal]. + +You can authenticate with Azure Active Directory using a credential from the [@azure/identity][azure_identity] library or [an existing AAD Token](https://github.com/Azure/azure-sdk-for-js/blob/master/sdk/identity/identity/samples/AzureIdentityExamples.md#authenticating-with-a-pre-fetched-access-token). + +To use the [DefaultAzureCredential][defaultazurecredential] provider shown below, or other credential providers provided with the Azure SDK, please install the `@azure/identity` package: + +```bash +npm install @azure/identity +``` + +You will also need to **register a new AAD application and grant access to Azure KustoManagement** by assigning the suitable role to your service principal (note: roles such as `"Owner"` will not grant the necessary permissions). +Set the values of the client ID, tenant ID, and client secret of the AAD application as environment variables: `AZURE_CLIENT_ID`, `AZURE_TENANT_ID`, `AZURE_CLIENT_SECRET`. + +For more information about how to create an Azure AD Application check out [this guide](https://docs.microsoft.com/azure/active-directory/develop/howto-create-service-principal-portal). + +```javascript +const { KustoManagementClient } = require("@azure/arm-kusto"); +const { DefaultAzureCredential } = require("@azure/identity"); +// For client-side applications running in the browser, use InteractiveBrowserCredential instead of DefaultAzureCredential. See https://aka.ms/azsdk/js/identity/examples for more details. + +const client = new KustoManagementClient(new DefaultAzureCredential()); + +// For client-side applications running in the browser, use this code instead: +// const credential = new InteractiveBrowserCredential({ +// tenantId: "", +// clientId: "" +// }); +// const client = new KustoManagementClient(credential); +``` + + +### JavaScript Bundle +To use this client library in the browser, first you need to use a bundler. For details on how to do this, please refer to our [bundling documentation](https://aka.ms/AzureSDKBundling). + +## Key concepts + +### KustoManagementClient + +`KustoManagementClient` is the primary interface for developers using the Azure KustoManagement client library. Explore the methods on this client object to understand the different features of the Azure KustoManagement service that you can access. + +## Troubleshooting + +### Logging + +Enabling logging may help uncover useful information about failures. In order to see a log of HTTP requests and responses, set the `AZURE_LOG_LEVEL` environment variable to `info`. Alternatively, logging can be enabled at runtime by calling `setLogLevel` in the `@azure/logger`: + +```javascript +const { setLogLevel } = require("@azure/logger"); +setLogLevel("info"); +``` + +For more detailed instructions on how to enable logs, you can look at the [@azure/logger package docs](https://github.com/Azure/azure-sdk-for-js/tree/main/sdk/core/logger). + +## Next steps + +Please take a look at the [samples](https://github.com/Azure-Samples/azure-samples-js-management) directory for detailed examples on how to use this library. + +## Contributing + +If you'd like to contribute to this library, please read the [contributing guide](https://github.com/Azure/azure-sdk-for-js/blob/main/CONTRIBUTING.md) to learn more about how to build and test the code. + +## Related projects + +- [Microsoft Azure SDK for JavaScript](https://github.com/Azure/azure-sdk-for-js) + +![Impressions](https://azure-sdk-impressions.azurewebsites.net/api/impressions/azure-sdk-for-js%2Fsdk%2Fkusto%2Farm-kusto%2FREADME.png) + +[azure_cli]: https://docs.microsoft.com/cli/azure +[azure_sub]: https://azure.microsoft.com/free/ +[azure_sub]: https://azure.microsoft.com/free/ +[azure_portal]: https://portal.azure.com +[azure_identity]: https://github.com/Azure/azure-sdk-for-js/tree/main/sdk/identity/identity +[defaultazurecredential]: https://github.com/Azure/azure-sdk-for-js/tree/main/sdk/identity/identity#defaultazurecredential diff --git a/packages/autorest.typescript/test/unit/generators/static/readmeFileGenerator.spec.ts b/packages/autorest.typescript/test/unit/generators/static/readmeFileGenerator.spec.ts index ce802a8dfa..bb7382252f 100644 --- a/packages/autorest.typescript/test/unit/generators/static/readmeFileGenerator.spec.ts +++ b/packages/autorest.typescript/test/unit/generators/static/readmeFileGenerator.spec.ts @@ -6,6 +6,8 @@ import { generateReadmeFile } from "../../../../src/generators/static/readmeFile import { Project, IndentationText } from "ts-morph"; import { readFileSync, writeFileSync } from "fs"; import * as path from "path"; +import { SampleGroup } from "../../../../src/models/sampleDetails"; +import { ClientDetails } from "../../../../src/models/clientDetails"; describe("readmeFileGenerator", () => { describe("generateReadmeFile", () => { @@ -85,6 +87,66 @@ describe("readmeFileGenerator", () => { ); assert.strictEqual(actualContents, expectedContends); }); + + it("should generate readme if no subscriptionid param in samples", async () => { + const autorestOption = { + srcPath: ".", + azureOutputDirectory: "sdk/kusto/arm-kusto", + packageDetails: { + name: "@azure/arm-kusto", + nameWithoutScope: "arm-kusto", + scopeName: "azure", + version: "1.0.0-beta.1" + }, + licenseHeader: false, + hideClients: true, + azureArm: true, + addCredentials: true, + generateMetadata: true, + isTestPackage: false, + ignoreNullableOnOptional: false, + useCoreV2: true, + allowInsecureConnection: true + } as autorestSession.AutorestOptions; + const codeModel = new CodeModel("testCodeModel"); + // set client details info + codeModel.language = { + default: { + name: "KustoManagementClient", + description: "" + } + }; + codeModel.info.title = "KustoManagementClient"; + codeModel.info.description = + "The Azure Kusto management API provides a RESTful set of web services that interact with Azure Kusto services to manage your clusters and databases. The API enables you to create, update, and delete clusters and databases."; + sinon.replace( + autorestSession, + "getAutorestOptions", + () => autorestOption + ); + codeModel.security.authenticationRequired = true; + const project = getEmptyProject(); + const sample = { + samples: [ + { + clientParameterNames: "credential" + } + ] + } as SampleGroup; + generateReadmeFile(codeModel, project, { + samples: [sample] + } as ClientDetails); + + const expectedContends = readFileSync( + path.join(__dirname, "files/case-hlcReadme-without-subid.md"), + "utf-8" + ).replace(/(\r\n|\n|\r)/gm, " "); + const actualContents = getFirstFileContent(project).replace( + /(\r\n|\n|\r)/gm, + " " + ); + assert.strictEqual(actualContents, expectedContends); + }); }); });