Add validation for issuer returned by OIDC endpoint (#576)

* Add issuer validation and related tests

* Refactor to use aliases from AAD instance discovery

* Add missing metadata call

* Add missing test case

Author: Avery-Dunn

apps/confidential/confidential_test.go

@@ -1599,7 +1599,7 @@ func TestWithInstanceDiscovery(t *testing.T) {
 					idToken = mock.GetIDToken(tenant, authority)
 					refreshToken = "refresh-token"
 				}
-				mockClient.AppendResponse(mock.WithBody(mock.GetTenantDiscoveryBody(stackurl, tenant)))
+				mockClient.AppendResponse(mock.WithBody(mock.GetTenantDiscoveryBody(host, tenant)))
 				mockClient.AppendResponse(
 					mock.WithBody(mock.GetAccessTokenBody(accessToken, idToken, refreshToken, "", 3600, 0)),
 				)

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

@@ -100,6 +100,41 @@ func (r *TenantDiscoveryResponse) Validate() error {
 	return nil
 }
 
+// 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")
+	}
+
+	// Parse the issuer URL
+	issuerURL, err := url.Parse(r.Issuer)
+	if err != nil {
+		return fmt.Errorf("TenantDiscoveryResponse: failed to parse issuer URL: %w", err)
+	}
+
+	// Even if it doesn't match the authority, issuers from known and trusted hosts are valid
+	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)
+	}
+
+	// Check if the scheme and host match (paths can be ignored when validating the issuer)
+	if issuerURL.Scheme == authorityURL.Scheme && issuerURL.Host == authorityURL.Host {
+		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)
+}
+
 type InstanceDiscoveryMetadata struct {
 	PreferredNetwork string   `json:"preferred_network"`
 	PreferredCache   string   `json:"preferred_cache"`
@@ -356,6 +391,8 @@ type Info struct {
 	Tenant                    string
 	Region                    string
 	InstanceDiscoveryDisabled bool
+	// InstanceDiscoveryMetadata stores the metadata from AAD instance discovery
+	InstanceDiscoveryMetadata []InstanceDiscoveryMetadata
 }
 
 // NewInfoFromAuthorityURI creates an AuthorityInfo instance from the authority URL provided.

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

@@ -507,3 +507,121 @@ func TestMergeCapabilitiesAndClaims(t *testing.T) {
 		})
 	}
 }
+
+func TestTenantDiscoveryValidateIssuer(t *testing.T) {
+	tests := []struct {
+		desc        string
+		issuer      string
+		authority   string
+		aliases     map[string]bool
+		expectError bool
+	}{
+		{
+			desc:        "issuer exactly matches authority",
+			issuer:      "https://login.microsoftonline.com/tenant-id",
+			authority:   "https://login.microsoftonline.com/tenant-id",
+			expectError: false,
+		},
+		{
+			desc:        "issuer matches authority with trailing slash in authority",
+			issuer:      "https://login.microsoftonline.com/tenant-id",
+			authority:   "https://login.microsoftonline.com/tenant-id/",
+			expectError: false,
+		},
+		{
+			desc:        "issuer matches authority with trailing slash in issuer",
+			issuer:      "https://login.microsoftonline.com/tenant-id/",
+			authority:   "https://login.microsoftonline.com/tenant-id",
+			expectError: false,
+		},
+		{
+			desc:        "issuer is shorter than authority but is a prefix",
+			issuer:      "https://login.microsoftonline.com",
+			authority:   "https://login.microsoftonline.com/tenant-id",
+			expectError: false,
+		},
+		{
+			desc:        "authority is shorter than issuer but is a prefix",
+			issuer:      "https://login.microsoftonline.com/tenant-id/additional-path",
+			authority:   "https://login.microsoftonline.com/tenant-id",
+			expectError: false,
+		},
+		{
+			desc:        "issuer and authority have different paths",
+			issuer:      "https://login.microsoftonline.com/other-tenant",
+			authority:   "https://login.microsoftonline.com/tenant-id",
+			expectError: false,
+		},
+		{
+			desc:        "custom authority with a non-matching Entra issuer",
+			issuer:      "https://login.microsoftonline.com/",
+			authority:   "https://contoso.com/tenant-id",
+			expectError: true,
+		},
+		{
+			desc:        "Entra authority with a non-matching custom issuer",
+			issuer:      "https://contoso.com/",
+			authority:   "https://login.microsoftonline.com/tenant-id",
+			expectError: true,
+		},
+		{
+			desc:        "empty issuer",
+			issuer:      "",
+			authority:   "https://login.microsoftonline.com/tenant-id",
+			expectError: true,
+		},
+		{
+			desc:        "empty issuer and authority",
+			issuer:      "",
+			authority:   "",
+			aliases:     map[string]bool{"alias1.example.com": true, "alias2.example.com": true},
+			expectError: true,
+		},
+		// New test cases for alias validation
+		{
+			desc:        "issuer matches an alias",
+			issuer:      "https://alias1.example.com/tenant-id",
+			authority:   "https://contoso.com/tenant-id",
+			aliases:     map[string]bool{"alias1.example.com": true, "alias2.example.com": true},
+			expectError: false,
+		},
+		{
+			desc:        "issuer matches a different alias",
+			issuer:      "https://alias2.example.com/tenant-id",
+			authority:   "https://contoso.com/tenant-id",
+			aliases:     map[string]bool{"alias1.example.com": true, "alias2.example.com": true},
+			expectError: false,
+		},
+		{
+			desc:        "issuer doesn't match any alias",
+			issuer:      "https://unknown.example.com/tenant-id",
+			authority:   "https://contoso.com/tenant-id",
+			aliases:     map[string]bool{"alias1.example.com": true, "alias2.example.com": true},
+			expectError: true,
+		},
+		{
+			desc:        "empty aliases map",
+			issuer:      "https://unknown.example.com/tenant-id",
+			authority:   "https://contoso.com/tenant-id",
+			aliases:     map[string]bool{},
+			expectError: true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			response := &TenantDiscoveryResponse{
+				AuthorizationEndpoint: "https://login.microsoftonline.com/tenant-id/oauth2/v2.0/authorize",
+				TokenEndpoint:         "https://login.microsoftonline.com/tenant-id/oauth2/v2.0/token",
+				Issuer:                test.issuer,
+			}
+
+			err := response.ValidateIssuerMatchesAuthority(test.authority, test.aliases)
+			if test.expectError && err == nil {
+				t.Errorf("expected error but got none")
+			} else if !test.expectError && err != nil {
+				t.Errorf("unexpected error: %v", err)
+			}
+		})
+	}
+}

apps/internal/oauth/resolvers.go

@@ -21,10 +21,12 @@ import (
 type cacheEntry struct {
 	Endpoints             authority.Endpoints
 	ValidForDomainsInList map[string]bool
+	// Aliases stores host aliases from instance discovery for quick lookup
+	Aliases map[string]bool
 }
 
 func createcacheEntry(endpoints authority.Endpoints) cacheEntry {
-	return cacheEntry{endpoints, map[string]bool{}}
+	return cacheEntry{endpoints, map[string]bool{}, map[string]bool{}}
 }
 
 // AuthorityEndpoint retrieves endpoints from an authority for auth and token acquisition.
@@ -71,10 +73,15 @@ func (m *authorityEndpoint) ResolveEndpoints(ctx context.Context, authorityInfo
 
 	m.addCachedEndpoints(authorityInfo, userPrincipalName, endpoints)
 
+	if err := resp.ValidateIssuerMatchesAuthority(authorityInfo.CanonicalAuthorityURI,
+		m.cache[authorityInfo.CanonicalAuthorityURI].Aliases); err != nil {
+		return authority.Endpoints{}, fmt.Errorf("ResolveEndpoints(): %w", err)
+	}
+
 	return endpoints, nil
 }
 
-// cachedEndpoints returns a the cached endpoints if they exists. If not, we return false.
+// cachedEndpoints returns the cached endpoints if they exist. If not, we return false.
 func (m *authorityEndpoint) cachedEndpoints(authorityInfo authority.Info, userPrincipalName string) (authority.Endpoints, bool) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
@@ -113,6 +120,13 @@ func (m *authorityEndpoint) addCachedEndpoints(authorityInfo authority.Info, use
 		}
 	}
 
+	// Extract aliases from instance discovery metadata and add to cache
+	for _, metadata := range authorityInfo.InstanceDiscoveryMetadata {
+		for _, alias := range metadata.Aliases {
+			updatedCacheEntry.Aliases[alias] = true
+		}
+	}
+
 	m.cache[authorityInfo.CanonicalAuthorityURI] = updatedCacheEntry
 }
 
@@ -127,12 +141,14 @@ func (m *authorityEndpoint) openIDConfigurationEndpoint(ctx context.Context, aut
 		if err != nil {
 			return "", err
 		}
+		authorityInfo.InstanceDiscoveryMetadata = resp.Metadata
 		return resp.TenantDiscoveryEndpoint, nil
 	} else if authorityInfo.Region != "" {
 		resp, err := m.rest.Authority().AADInstanceDiscovery(ctx, authorityInfo)
 		if err != nil {
 			return "", err
 		}
+		authorityInfo.InstanceDiscoveryMetadata = resp.Metadata
 		return resp.TenantDiscoveryEndpoint, nil
 	}
 

apps/public/public_test.go

@@ -210,7 +210,7 @@ func TestAcquireTokenByDeviceCode(t *testing.T) {
 		VerificationURL: "https://device.code.local",
 	}
 	mockClient := mock.NewClient()
-	mockClient.AppendResponse(mock.WithBody(mock.GetTenantDiscoveryBody("http://localhost", "tenant")))
+	mockClient.AppendResponse(mock.WithBody(mock.GetTenantDiscoveryBody("login.microsoftonline.com", "tenant")))
 	mockClient.AppendResponse(mock.WithBody([]byte(
 		fmt.Sprintf(
 			`{"device_code":%q,"expires_in":60,"interval":%d,"message":%q,"user_code":%q,"verification_uri":%q}`,