Changes based on latest feedback

Signed-off-by: Andre Wanlin <awanlin@spotify.com>

.changeset/unlucky-clouds-build.md

@@ -1,5 +1,7 @@
 ---
-'@backstage/plugin-azure-devops-backend': patch
+'@backstage/plugin-azure-devops-backend': minor
 ---
 
-Added support for using `AzureDevOpsCredentialsProvider` and deprecated `azureDevOps.token` configuration value
+**BREAKING** New `fromConfig` static method must be used now when created an instance of the `AzureDevOpsApi`
+
+Added support for using the `AzureDevOpsCredentialsProvider` and deprecated the entire `azureDevOps` configuration section

plugins/azure-devops-backend/config.d.ts

@@ -15,10 +15,15 @@
  */
 
 export interface Config {
-  /** Configuration options for the azure-devops-backend plugin */
+  /** Configuration options for the azure-devops-backend plugin
+   * @deprecated Use `integrations.azure` instead.
+   * @see https://backstage.io/docs/integrations/azure/locations
+   */
   azureDevOps: {
     /**
      * The hostname of the given Azure instance
+     * @deprecated Use `integrations.azure` instead.
+     * @see https://backstage.io/docs/integrations/azure/locations
      */
     host: string;
     /**
@@ -30,6 +35,8 @@ export interface Config {
     token: string;
     /**
      * The organization of the given Azure instance
+     * @deprecated Use `integrations.azure` instead.
+     * @see https://backstage.io/docs/integrations/azure/locations
      */
     organization: string;
   };

plugins/azure-devops-backend/src/api/AzureDevOpsApi.ts

@@ -51,7 +51,7 @@ import { Logger } from 'winston';
 import { PolicyEvaluationRecord } from 'azure-devops-node-api/interfaces/PolicyInterfaces';
 import {
   WebApi,
-  getBearerHandler,
+  getHandlerFromToken,
   getPersonalAccessTokenHandler,
 } from 'azure-devops-node-api';
 import {
@@ -61,6 +61,7 @@ import {
 import { UrlReader } from '@backstage/backend-common';
 import { Config } from '@backstage/config';
 import {
+  AzureDevOpsCredentialsProvider,
   DefaultAzureDevOpsCredentialsProvider,
   ScmIntegrations,
 } from '@backstage/integration';
@@ -70,42 +71,57 @@ export class AzureDevOpsApi {
   private readonly logger: Logger;
   private readonly urlReader: UrlReader;
   private readonly config: Config;
-  private readonly credentialsProvider: DefaultAzureDevOpsCredentialsProvider;
+  private readonly credentialsProvider: AzureDevOpsCredentialsProvider;
 
-  private constructor(logger: Logger, urlReader: UrlReader, config: Config) {
+  private constructor(
+    logger: Logger,
+    urlReader: UrlReader,
+    config: Config,
+    credentialsProvider: AzureDevOpsCredentialsProvider,
+  ) {
     this.logger = logger;
     this.urlReader = urlReader;
     this.config = config;
-
-    const scmIntegrations = ScmIntegrations.fromConfig(this.config);
-    this.credentialsProvider =
-      DefaultAzureDevOpsCredentialsProvider.fromIntegrations(scmIntegrations);
+    this.credentialsProvider = credentialsProvider;
   }
 
   static fromConfig(
     config: Config,
     options: { logger: Logger; urlReader: UrlReader },
   ) {
-    return new AzureDevOpsApi(options.logger, options.urlReader, config);
+    const scmIntegrations = ScmIntegrations.fromConfig(config);
+    const credentialsProvider =
+      DefaultAzureDevOpsCredentialsProvider.fromIntegrations(scmIntegrations);
+    return new AzureDevOpsApi(
+      options.logger,
+      options.urlReader,
+      config,
+      credentialsProvider,
+    );
   }
 
   private async getWebApi(host?: string, org?: string): Promise<WebApi> {
+    // If no host or org is provided we fall back to the values from the `azureDevOps` config section
+    // these may have been setup in the `integrations.azure` config section
+    // which is why use them here and not just falling back on them entirely
     const validHost = host ?? this.config.getString('azureDevOps.host');
     const validOrg = org ?? this.config.getString('azureDevOps.organization');
-    const url = `https://${validHost}/${validOrg}`;
+    const url = `https://${validHost}/${encodeURI(validOrg)}`;
 
     const credentials = await this.credentialsProvider.getCredentials({
       url,
     });
 
-    // TODO:(awanlin) use `getHandlerFromToken` once we no longer
-    // need to support the 'azureDevOps.token' fallback config value
-    const authHandler =
-      credentials?.type === 'pat'
-        ? getPersonalAccessTokenHandler(
-            credentials!.token ?? this.config.getString('azureDevOps.token'),
-          )
-        : getBearerHandler(credentials!.token);
+    let authHandler;
+    if (!credentials) {
+      // No credentials found for the provided host and org in the `integrations.azure` config section
+      // use the fall back personal access token from `azureDevOps.token`
+      const token = this.config.getString('azureDevOps.token');
+      authHandler = getPersonalAccessTokenHandler(token);
+    } else {
+      authHandler = getHandlerFromToken(credentials.token);
+    }
+
     const webApi = new WebApi(url, authHandler);
     return webApi;
   }