Move allowedDomains to construction time for consistency

Previously allowedDomains was passed at call time to UserInfo(), while
allowedOrgs was configured at construction. This inconsistency made the
interface harder to understand and didn't follow the principle that
access control should be configured when the provider is created.

Changes:
- Provider.UserInfo() now takes only context and token
- All providers store allowedDomains internally
- Factory accepts allowedDomains parameter
- All tests updated to reflect new interface

Author: claude

internal/idp/azure.go

@@ -4,7 +4,7 @@ import "fmt"
 
 // NewAzureProvider creates an Azure AD provider using OIDC discovery.
 // Azure AD is OIDC-compliant, so we use the generic OIDC provider with Azure's tenant-specific discovery URL.
-func NewAzureProvider(tenantID, clientID, clientSecret, redirectURI string) (*OIDCProvider, error) {
+func NewAzureProvider(tenantID, clientID, clientSecret, redirectURI string, allowedDomains []string) (*OIDCProvider, error) {
 	if tenantID == "" {
 		return nil, fmt.Errorf("tenantId is required for Azure AD")
 	}
@@ -15,11 +15,12 @@ func NewAzureProvider(tenantID, clientID, clientSecret, redirectURI string) (*OI
 	)
 
 	return NewOIDCProvider(OIDCConfig{
-		ProviderType: "azure",
-		DiscoveryURL: discoveryURL,
-		ClientID:     clientID,
-		ClientSecret: clientSecret,
-		RedirectURI:  redirectURI,
-		Scopes:       []string{"openid", "email", "profile"},
+		ProviderType:   "azure",
+		DiscoveryURL:   discoveryURL,
+		ClientID:       clientID,
+		ClientSecret:   clientSecret,
+		RedirectURI:    redirectURI,
+		Scopes:         []string{"openid", "email", "profile"},
+		AllowedDomains: allowedDomains,
 	})
 }

internal/idp/azure_test.go

@@ -8,7 +8,7 @@ import (
 )
 
 func TestNewAzureProvider_MissingTenantID(t *testing.T) {
-	_, err := NewAzureProvider("", "client-id", "client-secret", "https://example.com/callback")
+	_, err := NewAzureProvider("", "client-id", "client-secret", "https://example.com/callback", nil)
 
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "tenantId is required")

internal/idp/factory.go

@@ -7,13 +7,15 @@ import (
 )
 
 // NewProvider creates a Provider based on the IDPConfig.
-func NewProvider(cfg config.IDPConfig) (Provider, error) {
+// allowedDomains configures domain-based access control for all provider types.
+func NewProvider(cfg config.IDPConfig, allowedDomains []string) (Provider, error) {
 	switch cfg.Provider {
 	case "google":
 		return NewGoogleProvider(
 			cfg.ClientID,
 			string(cfg.ClientSecret),
 			cfg.RedirectURI,
+			allowedDomains,
 		), nil
 
 	case "azure":
@@ -22,13 +24,15 @@ func NewProvider(cfg config.IDPConfig) (Provider, error) {
 			cfg.ClientID,
 			string(cfg.ClientSecret),
 			cfg.RedirectURI,
+			allowedDomains,
 		)
 
 	case "github":
 		return NewGitHubProvider(
 			cfg.ClientID,
 			string(cfg.ClientSecret),
 			cfg.RedirectURI,
+			allowedDomains,
 			cfg.AllowedOrgs,
 		), nil
 
@@ -43,6 +47,7 @@ func NewProvider(cfg config.IDPConfig) (Provider, error) {
 			ClientSecret:     string(cfg.ClientSecret),
 			RedirectURI:      cfg.RedirectURI,
 			Scopes:           cfg.Scopes,
+			AllowedDomains:   allowedDomains,
 		})
 
 	default:

internal/idp/factory_test.go

@@ -87,7 +87,7 @@ func TestNewProvider(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			provider, err := NewProvider(tt.cfg)
+			provider, err := NewProvider(tt.cfg, nil)
 
 			if tt.wantErr {
 				require.Error(t, err)

internal/idp/github.go

@@ -14,9 +14,10 @@ import (
 // GitHubProvider implements the Provider interface for GitHub OAuth.
 // GitHub uses OAuth 2.0 (not OIDC) and has its own API for user info and org membership.
 type GitHubProvider struct {
-	config      oauth2.Config
-	apiBaseURL  string   // defaults to https://api.github.com, can be overridden for testing
-	allowedOrgs []string // organizations users must be members of (empty = no restriction)
+	config         oauth2.Config
+	apiBaseURL     string   // defaults to https://api.github.com, can be overridden for testing
+	allowedDomains []string // email domains users must belong to (empty = no restriction)
+	allowedOrgs    []string // organizations users must be members of (empty = no restriction)
 }
 
 // githubUserResponse represents GitHub's user API response.
@@ -41,8 +42,7 @@ type githubOrgResponse struct {
 }
 
 // NewGitHubProvider creates a new GitHub OAuth provider.
-// allowedOrgs specifies organizations users must be members of (empty = no restriction).
-func NewGitHubProvider(clientID, clientSecret, redirectURI string, allowedOrgs []string) *GitHubProvider {
+func NewGitHubProvider(clientID, clientSecret, redirectURI string, allowedDomains, allowedOrgs []string) *GitHubProvider {
 	return &GitHubProvider{
 		config: oauth2.Config{
 			ClientID:     clientID,
@@ -51,8 +51,9 @@ func NewGitHubProvider(clientID, clientSecret, redirectURI string, allowedOrgs [
 			Scopes:       []string{"user:email", "read:org"},
 			Endpoint:     github.Endpoint,
 		},
-		apiBaseURL:  "https://api.github.com",
-		allowedOrgs: allowedOrgs,
+		apiBaseURL:     "https://api.github.com",
+		allowedDomains: allowedDomains,
+		allowedOrgs:    allowedOrgs,
 	}
 }
 
@@ -72,9 +73,9 @@ func (p *GitHubProvider) ExchangeCode(ctx context.Context, code string) (*oauth2
 }
 
 // UserInfo fetches user information from GitHub's API.
-// Validates organization membership if allowedOrgs was configured at construction.
+// Validates domain and organization membership based on construction-time config.
 // TODO: Consider caching org membership to reduce API calls.
-func (p *GitHubProvider) UserInfo(ctx context.Context, token *oauth2.Token, allowedDomains []string) (*UserInfo, error) {
+func (p *GitHubProvider) UserInfo(ctx context.Context, token *oauth2.Token) (*UserInfo, error) {
 	client := p.config.Client(ctx, token)
 
 	// Fetch user profile
@@ -99,7 +100,7 @@ func (p *GitHubProvider) UserInfo(ctx context.Context, token *oauth2.Token, allo
 	domain := emailutil.ExtractDomain(email)
 
 	// Validate domain if configured
-	if err := ValidateDomain(domain, allowedDomains); err != nil {
+	if err := ValidateDomain(domain, p.allowedDomains); err != nil {
 		return nil, err
 	}
 

internal/idp/github_test.go

@@ -13,12 +13,12 @@ import (
 )
 
 func TestGitHubProvider_Type(t *testing.T) {
-	provider := NewGitHubProvider("client-id", "client-secret", "https://example.com/callback", nil)
+	provider := NewGitHubProvider("client-id", "client-secret", "https://example.com/callback", nil, nil)
 	assert.Equal(t, "github", provider.Type())
 }
 
 func TestGitHubProvider_AuthURL(t *testing.T) {
-	provider := NewGitHubProvider("client-id", "client-secret", "https://example.com/callback", nil)
+	provider := NewGitHubProvider("client-id", "client-secret", "https://example.com/callback", nil, nil)
 
 	authURL := provider.AuthURL("test-state")
 
@@ -184,12 +184,13 @@ func TestGitHubProvider_UserInfo(t *testing.T) {
 						TokenURL: server.URL + "/token",
 					},
 				},
-				apiBaseURL:  server.URL,
-				allowedOrgs: tt.allowedOrgs,
+				apiBaseURL:     server.URL,
+				allowedDomains: tt.allowedDomains,
+				allowedOrgs:    tt.allowedOrgs,
 			}
 
 			token := &oauth2.Token{AccessToken: "test-token"}
-			userInfo, err := provider.UserInfo(context.Background(), token, tt.allowedDomains)
+			userInfo, err := provider.UserInfo(context.Background(), token)
 
 			if tt.wantErr {
 				require.Error(t, err)
@@ -244,7 +245,7 @@ func TestGitHubProvider_UserInfo_APIErrors(t *testing.T) {
 			}
 
 			token := &oauth2.Token{AccessToken: "test-token"}
-			_, err := provider.UserInfo(context.Background(), token, nil)
+			_, err := provider.UserInfo(context.Background(), token)
 
 			require.Error(t, err)
 			assert.Contains(t, err.Error(), tt.errContains)
@@ -277,7 +278,7 @@ func TestGitHubProvider_UserInfo_NoVerifiedEmail(t *testing.T) {
 	}
 
 	token := &oauth2.Token{AccessToken: "test-token"}
-	_, err := provider.UserInfo(context.Background(), token, nil)
+	_, err := provider.UserInfo(context.Background(), token)
 
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "no verified email")

internal/idp/google.go

@@ -14,8 +14,9 @@ import (
 // GoogleProvider implements the Provider interface for Google OAuth.
 // Google has specific quirks like `hd` for hosted domain and `verified_email` field.
 type GoogleProvider struct {
-	config      oauth2.Config
-	userInfoURL string
+	config         oauth2.Config
+	userInfoURL    string
+	allowedDomains []string
 }
 
 // googleUserInfoResponse represents Google's userinfo response.
@@ -30,7 +31,7 @@ type googleUserInfoResponse struct {
 }
 
 // NewGoogleProvider creates a new Google OAuth provider.
-func NewGoogleProvider(clientID, clientSecret, redirectURI string) *GoogleProvider {
+func NewGoogleProvider(clientID, clientSecret, redirectURI string, allowedDomains []string) *GoogleProvider {
 	return &GoogleProvider{
 		config: oauth2.Config{
 			ClientID:     clientID,
@@ -39,7 +40,8 @@ func NewGoogleProvider(clientID, clientSecret, redirectURI string) *GoogleProvid
 			Scopes:       []string{"openid", "profile", "email"},
 			Endpoint:     google.Endpoint,
 		},
-		userInfoURL: "https://www.googleapis.com/oauth2/v2/userinfo",
+		userInfoURL:    "https://www.googleapis.com/oauth2/v2/userinfo",
+		allowedDomains: allowedDomains,
 	}
 }
 
@@ -62,7 +64,7 @@ func (p *GoogleProvider) ExchangeCode(ctx context.Context, code string) (*oauth2
 }
 
 // UserInfo fetches user information from Google's userinfo endpoint.
-func (p *GoogleProvider) UserInfo(ctx context.Context, token *oauth2.Token, allowedDomains []string) (*UserInfo, error) {
+func (p *GoogleProvider) UserInfo(ctx context.Context, token *oauth2.Token) (*UserInfo, error) {
 	client := p.config.Client(ctx, token)
 
 	resp, err := client.Get(p.userInfoURL)
@@ -87,7 +89,7 @@ func (p *GoogleProvider) UserInfo(ctx context.Context, token *oauth2.Token, allo
 	}
 
 	// Validate domain if configured
-	if err := ValidateDomain(domain, allowedDomains); err != nil {
+	if err := ValidateDomain(domain, p.allowedDomains); err != nil {
 		return nil, err
 	}
 

internal/idp/google_test.go

@@ -13,12 +13,12 @@ import (
 )
 
 func TestGoogleProvider_Type(t *testing.T) {
-	provider := NewGoogleProvider("client-id", "client-secret", "https://example.com/callback")
+	provider := NewGoogleProvider("client-id", "client-secret", "https://example.com/callback", nil)
 	assert.Equal(t, "google", provider.Type())
 }
 
 func TestGoogleProvider_AuthURL(t *testing.T) {
-	provider := NewGoogleProvider("client-id", "client-secret", "https://example.com/callback")
+	provider := NewGoogleProvider("client-id", "client-secret", "https://example.com/callback", nil)
 
 	authURL := provider.AuthURL("test-state")
 
@@ -102,11 +102,12 @@ func TestGoogleProvider_UserInfo(t *testing.T) {
 						TokenURL: server.URL + "/token",
 					},
 				},
-				userInfoURL: server.URL,
+				userInfoURL:    server.URL,
+				allowedDomains: tt.allowedDomains,
 			}
 			token := &oauth2.Token{AccessToken: "test-token"}
 
-			userInfo, err := provider.UserInfo(context.Background(), token, tt.allowedDomains)
+			userInfo, err := provider.UserInfo(context.Background(), token)
 
 			if tt.wantErr {
 				require.Error(t, err)
@@ -142,7 +143,7 @@ func TestGoogleProvider_UserInfo_ServerError(t *testing.T) {
 	}
 	token := &oauth2.Token{AccessToken: "test-token"}
 
-	_, err := provider.UserInfo(context.Background(), token, nil)
+	_, err := provider.UserInfo(context.Background(), token)
 
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "status 500")

internal/idp/oidc.go

@@ -29,13 +29,17 @@ type OIDCConfig struct {
 	ClientSecret string
 	RedirectURI  string
 	Scopes       []string
+
+	// Access control.
+	AllowedDomains []string
 }
 
 // OIDCProvider implements the Provider interface for OIDC-compliant identity providers.
 type OIDCProvider struct {
-	providerType string
-	config       oauth2.Config
-	userInfoURL  string
+	providerType   string
+	config         oauth2.Config
+	userInfoURL    string
+	allowedDomains []string
 }
 
 // oidcDiscoveryDocument represents the OIDC discovery document.
@@ -99,7 +103,8 @@ func NewOIDCProvider(cfg OIDCConfig) (*OIDCProvider, error) {
 				TokenURL: tokenURL,
 			},
 		},
-		userInfoURL: userInfoURL,
+		userInfoURL:    userInfoURL,
+		allowedDomains: cfg.AllowedDomains,
 	}, nil
 }
 
@@ -147,7 +152,7 @@ func (p *OIDCProvider) ExchangeCode(ctx context.Context, code string) (*oauth2.T
 
 // UserInfo fetches user information from the OIDC userinfo endpoint.
 // TODO: Add ID token validation as optimization (avoids network call).
-func (p *OIDCProvider) UserInfo(ctx context.Context, token *oauth2.Token, allowedDomains []string) (*UserInfo, error) {
+func (p *OIDCProvider) UserInfo(ctx context.Context, token *oauth2.Token) (*UserInfo, error) {
 	client := p.config.Client(ctx, token)
 	resp, err := client.Get(p.userInfoURL)
 	if err != nil {
@@ -167,7 +172,7 @@ func (p *OIDCProvider) UserInfo(ctx context.Context, token *oauth2.Token, allowe
 	domain := emailutil.ExtractDomain(userInfoResp.Email)
 
 	// Validate domain if configured
-	if err := ValidateDomain(domain, allowedDomains); err != nil {
+	if err := ValidateDomain(domain, p.allowedDomains); err != nil {
 		return nil, err
 	}
 

internal/idp/oidc_test.go

@@ -145,7 +145,7 @@ func TestOIDCProvider_UserInfo(t *testing.T) {
 	require.NoError(t, err)
 
 	token := &oauth2.Token{AccessToken: "test-token"}
-	userInfo, err := provider.UserInfo(context.Background(), token, nil)
+	userInfo, err := provider.UserInfo(context.Background(), token)
 
 	require.NoError(t, err)
 	require.NotNil(t, userInfo)
@@ -175,11 +175,12 @@ func TestOIDCProvider_UserInfo_DomainValidation(t *testing.T) {
 		ClientID:         "client-id",
 		ClientSecret:     "client-secret",
 		RedirectURI:      "https://example.com/callback",
+		AllowedDomains:   []string{"example.com"},
 	})
 	require.NoError(t, err)
 
 	token := &oauth2.Token{AccessToken: "test-token"}
-	_, err = provider.UserInfo(context.Background(), token, []string{"example.com"})
+	_, err = provider.UserInfo(context.Background(), token)
 
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "domain 'other.com' is not allowed")