fix: Apply default values to custom AuthConfiguration in CloudAdapter

[Namielusi]

Description

This PR fixes an issue where the authority field is not properly populated when a custom AuthConfiguration is provided to CloudAdapter, despite this field being marked as optional in the TypeScript interface.

Problem

When creating a CloudAdapter with a custom authentication configuration, the optional authority field was not receiving its default value. This caused runtime errors in MsalTokenProvider methods that expect authority to be defined, even though TypeScript types suggest it's optional.

Error Example

// This code would fail at runtime
const customConfig: AuthConfiguration = {
  clientId: 'my-client-id',
  clientSecret: 'my-secret',
  tenantId: 'my-tenant-id'
  // authority is optional in TypeScript, but missing it causes runtime error
};

const adapter = new CloudAdapter(customConfig);

When adapter tries to acquire token, it fails with:

ClientConfigurationError: url_parse_error: URL could not be parsed into appropriate segments.
     at createClientConfigurationError (/demo-app/node_modules/@azure/msal-common/dist/error/ClientConfigurationError.mjs:146:12)
     at UrlString.validateAsUri (/demo-app/node_modules/@azure/msal-common/dist/url/UrlString.mjs:64:19)
     at Authority.set canonicalAuthority (/demo-app/node_modules/@azure/msal-common/dist/authority/Authority.mjs:97:34)
     at new Authority (/demo-app/node_modules/@azure/msal-common/dist/authority/Authority.mjs:33:32)
     at AuthorityFactory.createDiscoveredInstance (/demo-app/node_modules/@azure/msal-common/dist/authority/AuthorityFactory.mjs:28:35)
     at ConfidentialClientApplication.createAuthority (/demo-app/node_modules/@azure/src/client/ClientApplication.ts:667:16)
     at ConfidentialClientApplication.acquireTokenByClientCredential (/demo-app/node_modules/@azure/src/client/ConfidentialClientApplication.ts:224:52)
     at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
     at async MsalTokenProvider.acquireAccessTokenViaSecret (/demo-app/node_modules/@microsoft/agents-hosting/src/auth/msalTokenProvider.ts:172:16)
     at async MsalTokenProvider.getAccessToken (/demo-app/node_modules/@microsoft/agents-hosting/src/auth/msalTokenProvider.ts:36:15)

Solution

Implemented a new helper function applyAuthConfigDefaults() that automatically applies default values to any AuthConfiguration object, ensuring consistency between custom configs and those loaded from environment variables.

The CloudAdapter constructor now uses this helper to apply defaults when a custom authConfig is provided, ensuring that authority and other optional fields receive their appropriate default values. This maintains backward compatibility while preventing runtime errors.

[Namielusi]

@microsoft-github-policy-service agree

[rido-min]

Hi @Namielusi, thank you very much for looking at this.

When using a custom authConfig you must configure the issuers, otherwise you will get a type error

const authConfig: AuthConfiguration = {
  clientId: '',
  clientSecret: '',
  tenantId: '',
}

Property 'issuers' is missing in type '{ clientId: string; clientSecret: string; tenantId: string | undefined; }' but required in type 'AuthConfiguration'.

As the issuers are built on top of the authorityEndpoint, when using customConfig you dont need to set it, hence why authority is optional.

Keep in mind that loadAuthConfigFromEnv already applies the default authority

Agents-for-js/packages/agents-hosting/src/auth/authConfiguration.ts

Line 88 in ecd5464

const authority = process.env.authorityEndpoint ?? 'https://login.microsoftonline.com'

or maybe I'm missing something from you use case...

[Namielusi]

Hi @rido-min!

In my use case, I use custom names for environment variables, hence I can't make use of loadAuthConfigFromEnv or loadPrevAuthConfigFromEnv, which apply default values to the authConfig object.

In my code, the initialisation of CloudAdapter currently looks like this:

const adapter = new CloudAdapter({
  clientId: config.microsoftClientId,
  clientSecret: config.microsoftClientSecret,
  // Reference: https://github.com/microsoft/Agents-for-js/blob/ecd5464d28e3e20e2cd5664c5dc55d16003fa9bb/packages/agents-hosting/src/auth/authConfiguration.ts#L88C57-L88C90
  authority: 'https://login.microsoftonline.com',
  // Reference: https://github.com/microsoft/Agents-for-js/blob/ecd5464d28e3e20e2cd5664c5dc55d16003fa9bb/packages/agents-hosting/src/auth/authConfiguration.ts#L101-L105
  issuers: [
    'https://api.botframework.com',
    `https://sts.windows.net/${config.microsoftTenantId}/`,
    `https://login.microsoftonline.com/${config.microsoftTenantId}/v2.0`,
  ],
});

Prior to creating this PR I've spent some time debugging the url_parse_error error I mentioned above that comes from here (the authority resolves to undefined/botframework.com):

Agents-for-js/packages/agents-hosting/src/auth/msalTokenProvider.ts

Lines 162 to 176 in ecd5464

	private async acquireAccessTokenViaSecret (authConfig: AuthConfiguration, scope: string) {
	const cca = new ConfidentialClientApplication({
	auth: {
	clientId: authConfig.clientId as string,
	authority: `${authConfig.authority}/${authConfig.tenantId || 'botframework.com'}`,
	clientSecret: authConfig.clientSecret
	},
	system: this.sysOptions
	})
	const token = await cca.acquireTokenByClientCredential({
	scopes: [`${scope}/.default`],
	correlationId: v4()
	})
	return token?.accessToken as string
	}

What I've found is that you have to explicitly provide value for authority (even though it's optional in TypeScript interface).

With this PR, my code can be simplified to:

const adapter = new CloudAdapter({
  clientId: config.microsoftClientId,
  clientSecret: config.microsoftClientSecret,
});

Alternative approach would be to fix the interface (authority?: string -> authority: string), but that would still require users to explicitly provide values for authority and issuers, even if they don't want to customize them.

[Namielusi]

I think this bug was introduced in #467, previously, it was working fine without providing authority.

In any case, the main priority for me is to fix the authority bug so others don't spend time debugging it. If you think tweaking issuers is out of scope of this, let's at least either make authority not optional in the interface or provide a default value for it.

What do you think?

[rido-min]

oh... I see now.

Yes, you are right, we should rethink how authority and issuers can be customized when not using load** to avoid the url_parsing_error. I recommend filing a new issue so we can track this fix in the backlog.

In the meantime, I want to make sure you are not blocked and can use your custom envvars with the workaround you show above.

nit, now that ABS removed MultiTenant support, I think tenantId should be required.

/c @ceciliaavila

[ceciliaavila]

Hi @Namielusi, thanks for your contribution! We used your PR as a base for the solution that was integrated into PR #573 (commit).

Since the fix is now included there, feel free to close this PR if you agree.

Thanks!

/c @MattB-msft