Adjust issuer validation to handle cases where instance discovery is not performed (#591)

* Adjust issuer validation to handle cases where instance discovery is not performed

* Added more trusted host

* Removed some unnecessary conditions

Added a check for if the host for auth and issuer is same apart for region.

---------

Co-authored-by: Nilesh Choudhary <[email protected]>

Author: Avery-Dunn

apps/internal/oauth/ops/authority/authority.go

@@ -48,6 +48,8 @@ type jsonCaller interface {
 }
 
 // For backward compatibility, accept both old and new China endpoints for a transition period.
+// This list is derived from the AAD instance discovery metadata and represents all known trusted hosts
+// across different Azure clouds (Public, China, Germany, US Government, etc.)
 var aadTrustedHostList = map[string]bool{
 	"login.windows.net":                true, // Microsoft Azure Worldwide - Used in validation scenarios where host is not this list
 	"login.partner.microsoftonline.cn": true, // Microsoft Azure China (new)
@@ -56,6 +58,9 @@ var aadTrustedHostList = map[string]bool{
 	"login-us.microsoftonline.com":     true, // Microsoft Azure US Government - Legacy
 	"login.microsoftonline.us":         true, // Microsoft Azure US Government
 	"login.microsoftonline.com":        true, // Microsoft Azure Worldwide
+	"login.microsoft.com":              true,
+	"sts.windows.net":                  true,
+	"login.usgovcloudapi.net":          true,
 }
 
 // TrustedHost checks if an AAD host is trusted/valid.
@@ -104,36 +109,46 @@ func (r *TenantDiscoveryResponse) Validate() error {
 // ValidateIssuerMatchesAuthority validates that the issuer in the TenantDiscoveryResponse matches the authority.
 // This is used to identity security or configuration issues in authorities and the OIDC endpoint
 func (r *TenantDiscoveryResponse) ValidateIssuerMatchesAuthority(authorityURI string, aliases map[string]bool) error {
-
 	if authorityURI == "" {
 		return errors.New("TenantDiscoveryResponse: empty authorityURI provided for validation")
 	}
+	if r.Issuer == "" {
+		return errors.New("TenantDiscoveryResponse: empty issuer in response")
+	}
 
-	// Parse the issuer URL
 	issuerURL, err := url.Parse(r.Issuer)
 	if err != nil {
 		return fmt.Errorf("TenantDiscoveryResponse: failed to parse issuer URL: %w", err)
 	}
+	authorityURL, err := url.Parse(authorityURI)
+	if err != nil {
+		return fmt.Errorf("TenantDiscoveryResponse: failed to parse authority URL: %w", err)
+	}
+
+	// Fast path: exact scheme + host match
+	if issuerURL.Scheme == authorityURL.Scheme && issuerURL.Host == authorityURL.Host {
+		return nil
+	}
 
-	// Even if it doesn't match the authority, issuers from known and trusted hosts are valid
+	// Alias-based acceptance
 	if aliases != nil && aliases[issuerURL.Host] {
 		return nil
 	}
 
-	// Parse the authority URL for comparison
-	authorityURL, err := url.Parse(authorityURI)
-	if err != nil {
-		return fmt.Errorf("TenantDiscoveryResponse: failed to parse authority URL: %w", err)
+	issuerHost := issuerURL.Host
+	authorityHost := authorityURL.Host
+
+	// Accept if issuer host is trusted
+	if TrustedHost(issuerHost) {
+		return nil
 	}
 
-	// Check if the scheme and host match (paths can be ignored when validating the issuer)
-	if issuerURL.Scheme == authorityURL.Scheme && issuerURL.Host == authorityURL.Host {
+	// Accept if authority is a regional variant ending with ".<issuerHost>"
+	if strings.HasSuffix(authorityHost, "."+issuerHost) {
 		return nil
 	}
 
-	// If we get here, validation failed
-	return fmt.Errorf("TenantDiscoveryResponse: issuer from OIDC discovery '%s' does not match authority '%s' or a known pattern",
-		r.Issuer, authorityURI)
+	return fmt.Errorf("TenantDiscoveryResponse: issuer '%s' does not match authority '%s' or any trusted/alias rule", r.Issuer, authorityURI)
 }
 
 type InstanceDiscoveryMetadata struct {

apps/internal/oauth/ops/authority/authority_test.go

@@ -553,10 +553,10 @@ func TestTenantDiscoveryValidateIssuer(t *testing.T) {
 			expectError: false,
 		},
 		{
-			desc:        "custom authority with a non-matching Entra issuer",
+			desc:        "custom authority with a trusted Entra issuer",
 			issuer:      "https://login.microsoftonline.com/",
 			authority:   "https://contoso.com/tenant-id",
-			expectError: true,
+			expectError: false, // Trusted hosts are always valid issuers
 		},
 		{
 			desc:        "Entra authority with a non-matching custom issuer",
@@ -606,6 +606,48 @@ func TestTenantDiscoveryValidateIssuer(t *testing.T) {
 			aliases:     map[string]bool{},
 			expectError: true,
 		},
+		// Test cases for regional authority scenarios where instance discovery isn't performed
+		{
+			desc:        "regional authority with trusted issuer host (no aliases)",
+			issuer:      "https://login.microsoftonline.com/tenant-id",
+			authority:   "https://westus2.login.microsoft.com/tenant-id",
+			aliases:     nil,
+			expectError: false,
+		},
+		{
+			desc:        "regional authority with different trusted issuer host (no aliases)",
+			issuer:      "https://login.windows.net/tenant-id",
+			authority:   "https://eastus.login.microsoft.com/tenant-id",
+			aliases:     map[string]bool{},
+			expectError: false,
+		},
+		{
+			desc:        "regional authority with Azure Government trusted issuer",
+			issuer:      "https://login.microsoftonline.us/tenant-id",
+			authority:   "https://usgovvirginia.login.microsoftonline.us/tenant-id",
+			aliases:     nil,
+			expectError: false,
+		},
+		{
+			desc:        "regional authority with untrusted issuer host (no aliases)",
+			issuer:      "https://malicious.example.com/tenant-id",
+			authority:   "https://westus2.login.microsoft.com/tenant-id",
+			aliases:     nil,
+			expectError: true,
+		},
+		{
+			desc:        "regional authority subdomain with matching trusted issuer",
+			issuer:      "https://login.microsoftonline.com/tenant-id",
+			authority:   "https://region.login.microsoftonline.com/tenant-id",
+			aliases:     nil,
+			expectError: false,
+		}, {
+			desc:        "regional authority subdomain with matching trusted issuer",
+			issuer:      "https://login.dummy-uri.com/tenant-id",
+			authority:   "https://region.login.dummy-uri.com/tenant-id",
+			aliases:     nil,
+			expectError: false,
+		},
 	}
 
 	for _, test := range tests {
@@ -625,3 +667,39 @@ func TestTenantDiscoveryValidateIssuer(t *testing.T) {
 		})
 	}
 }
+
+func TestTrustedHost(t *testing.T) {
+	tests := []struct {
+		host          string
+		expectedTrust bool
+	}{
+		// Microsoft Azure Worldwide hosts
+		{"login.microsoftonline.com", true},
+		{"login.windows.net", true},
+		{"login.microsoft.com", true},
+		{"sts.windows.net", true},
+		// Microsoft Azure China hosts
+		{"login.partner.microsoftonline.cn", true},
+		{"login.chinacloudapi.cn", true},
+		// Microsoft Azure Germany hosts
+		{"login.microsoftonline.de", true},
+		// Microsoft Azure US Government hosts
+		{"login.microsoftonline.us", true},
+		{"login.usgovcloudapi.net", true},
+		{"login-us.microsoftonline.com", true},
+		// Untrusted hosts
+		{"malicious.example.com", false},
+		{"fake-login.microsoftonline.com", false},
+		{"login.example.com", false},
+		{"", false},
+	}
+
+	for _, test := range tests {
+		t.Run(test.host, func(t *testing.T) {
+			result := TrustedHost(test.host)
+			if result != test.expectedTrust {
+				t.Errorf("TrustedHost(%q) = %v, want %v", test.host, result, test.expectedTrust)
+			}
+		})
+	}
+}