Skip to content

Conversation

Copy link

Copilot AI commented Dec 15, 2025

The implementation had AuthorityTypeDefault and AuthorityTypeMultiTenant reversed - single-tenant apps were getting the /common endpoint instead of their tenant-specific endpoint.

// Before: Single-tenant incorrectly used /common
authority := identity.AuthorityConfiguration{
    AuthorityType: identity.AuthorityTypeDefault,
    TenantID: "my-tenant-id",
}
// Produced: https://login.microsoftonline.com/common ❌

// After: Single-tenant correctly uses tenant ID
authority := identity.AuthorityConfiguration{
    AuthorityType: identity.AuthorityTypeDefault,
    TenantID: "my-tenant-id",
}
// Produces: https://login.microsoftonline.com/my-tenant-id ✅

Changes

  • identity/authority_configuration.go: Swapped case logic

    • AuthorityTypeDefault: now requires TenantID, returns /{tenantID} endpoint
    • AuthorityTypeMultiTenant: now returns /common endpoint, TenantID optional/ignored
    • Updated comments to clarify single vs multi-tenant usage
  • Test updates: Corrected expectations in authority_configuration_test.go and updated test fixtures across 7 files to use AuthorityTypeMultiTenant for generic test scenarios

  • Examples: Fixed clientsecret and clientcert examples to use AuthorityTypeDefault

  • Documentation: Clarified in README when TenantID is required vs optional

Original prompt

This section details on the original issue you should resolve

<issue_title>[Bug]: Authority configuration for single/multi tenant applications doesn't match documentation</issue_title>
<issue_description>### Describe the bug

In the readme, authority configuration is documented as:

// Multi-tenant application
authority := identity.AuthorityConfiguration{
    AuthorityType: identity.AuthorityTypeMultiTenant,
    TenantID: "common",
}

// Single-tenant application
authority := identity.AuthorityConfiguration{
    AuthorityType: identity.AuthorityTypeDefault,
    TenantID: os.Getenv("AZURE_TENANT_ID"),
}

With AuthorityTypeDefault being used for single tenant applications with a specified tenant id. However the implementation produces an authority url with the common endpoint:

        case AuthorityTypeDefault:
		return "https://login.microsoftonline.com/common", nil
	case AuthorityTypeMultiTenant:
		if a.TenantID == "" {
			return "", fmt.Errorf("tenant ID is required when using multi-tenant authority type")
		}
		return fmt.Sprintf("https://login.microsoftonline.com/%s", a.TenantID), nil

It looks like these cases are the wrong way round.

To Reproduce

  1. Create a credentials provider with
	entraid.NewConfidentialCredentialsProvider(
		entraid.ConfidentialCredentialsProviderOptions{
			ConfidentialIdentityProviderOptions: identity.ConfidentialIdentityProviderOptions{
				ClientID:        clientID,
				ClientSecret:    clientSecret,
				CredentialsType: identity.ClientSecretCredentialType,
				Authority: identity.AuthorityConfiguration{
					AuthorityType: identity.AuthorityTypeDefault,
					TenantID:      tenantID,
				},
				Scopes: []string{identity.RedisScopeDefault},
			},
		},
	)
  1. Connect to redis

  2. Actual behaviour - token is requested with common endpoint https://login.microsoftonline.com/common

Expected behavior

A token is requested using the tenant-specific authority https://login.microsoftonline.com/<tenantID>.

Code Example

Go Version

1.24.0

Package version

v1.0.6

go-redis Version

v9.16.0

Redis Server Version

Azure cache for redis v 6.0

Additional environment details

No response

Additional context

No response</issue_description>

Comments on the Issue (you are @copilot in this section)

@ndyakov Hello @benlings, thank you for reporting this, we will take a look shortly and update the doc.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 4 commits December 15, 2025 09:19
…rityTypeMultiTenant logic

Co-authored-by: ndyakov <1547186+ndyakov@users.noreply.github.com>
Co-authored-by: ndyakov <1547186+ndyakov@users.noreply.github.com>
Co-authored-by: ndyakov <1547186+ndyakov@users.noreply.github.com>
Co-authored-by: ndyakov <1547186+ndyakov@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix authority configuration for single and multi-tenant applications Fix swapped AuthorityTypeDefault and AuthorityTypeMultiTenant implementations Dec 15, 2025
Copilot AI requested a review from ndyakov December 15, 2025 09:29
@ndyakov ndyakov requested a review from ofekshenawa December 15, 2025 13:38
Copy link

@ofekshenawa ofekshenawa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just fix the failing test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Authority configuration for single/multi tenant applications doesn't match documentation

3 participants