From eb93d512baf435a8ddffb3c864a4e4cdc5de20f6 Mon Sep 17 00:00:00 2001 From: guglielmoc Date: Wed, 22 Apr 2026 17:31:21 +0000 Subject: [PATCH 1/7] feat: add automatic application_type inference and retry logic for dynamic client registration --- auth/authorization_code.go | 44 ++++++- auth/authorization_code_test.go | 217 ++++++++++++++++++++++++++++++++ oauthex/dcr.go | 4 + 3 files changed, 264 insertions(+), 1 deletion(-) diff --git a/auth/authorization_code.go b/auth/authorization_code.go index 846feb7b..8b086ae8 100644 --- a/auth/authorization_code.go +++ b/auth/authorization_code.go @@ -150,6 +150,9 @@ func NewAuthorizationCodeHandler(config *AuthorizationCodeHandlerConfig) (*Autho } else if !slices.Contains(dCfg.Metadata.RedirectURIs, config.RedirectURL) { return nil, fmt.Errorf("RedirectURL %q is not in the list of allowed redirect URIs for dynamic client registration", config.RedirectURL) } + if dCfg.Metadata.ApplicationType == "" { + dCfg.Metadata.ApplicationType = inferApplicationType(dCfg.Metadata.RedirectURIs) + } } if config.RedirectURL == "" { // If the RedirectURL was supposed to be set by the dynamic client registration, @@ -170,6 +173,31 @@ func isNonRootHTTPSURL(u string) bool { return pu.Scheme == "https" && pu.Path != "" } +// inferApplicationType returns "native" or "web" based on the redirect URIs. +// If any redirect URI uses a loopback host or a non-http(s) scheme (custom +// scheme), the application is classified as "native". Otherwise, it is "web". +func inferApplicationType(redirectURIs []string) string { + for _, uri := range redirectURIs { + u, err := url.Parse(uri) + if err != nil { + continue + } + switch u.Scheme { + case "http", "https": + if isLoopback(u.Hostname()) { + return "native" + } + default: + return "native" + } + } + return "web" +} + +func isLoopback(host string) bool { + return host == "localhost" || host == "127.0.0.1" || host == "::1" +} + // Authorize performs the authorization flow. // It is designed to perform the whole Authorization Code Grant flow. // On success, [AuthorizationCodeHandler.TokenSource] will return a token source with the fetched token. @@ -432,7 +460,21 @@ func (h *AuthorizationCodeHandler) handleRegistration(ctx context.Context, asm * if dcrCfg != nil && asm.RegistrationEndpoint != "" { regResp, err := oauthex.RegisterClient(ctx, asm.RegistrationEndpoint, dcrCfg.Metadata, h.config.Client) if err != nil { - return nil, fmt.Errorf("failed to register client: %w", err) + // If registration failed due to redirect URI constraints, retry with + // the opposite application_type per the MCP specification. + var regErr *oauthex.ClientRegistrationError + if errors.As(err, ®Err) && regErr.ErrorCode == "invalid_redirect_uri" { + retryType := "native" + if dcrCfg.Metadata.ApplicationType == "native" { + retryType = "web" + } + retryCfg := *dcrCfg.Metadata + retryCfg.ApplicationType = retryType + regResp, err = oauthex.RegisterClient(ctx, asm.RegistrationEndpoint, &retryCfg, h.config.Client) + } + if err != nil { + return nil, fmt.Errorf("failed to register client: %w", err) + } } cfg := &resolvedClientConfig{ registrationType: registrationTypeDynamic, diff --git a/auth/authorization_code_test.go b/auth/authorization_code_test.go index cd4741a5..8d25acfa 100644 --- a/auth/authorization_code_test.go +++ b/auth/authorization_code_test.go @@ -6,7 +6,9 @@ package auth import ( "context" + "encoding/json" "fmt" + "io" "net/http" "net/http/httptest" "net/http/httputil" @@ -608,6 +610,221 @@ func TestDynamicRegistration(t *testing.T) { } } +func TestDynamicRegistrationRetryApplicationType(t *testing.T) { + tests := []struct { + name string + redirectURIs []string + wantRetryType string + wantRegistered bool + rejectOnRetry bool + }{ + { + name: "inferred native retries with web", + redirectURIs: []string{"http://localhost:8085/callback"}, + wantRetryType: "web", + wantRegistered: true, + }, + { + name: "inferred web retries with native", + redirectURIs: []string{"https://example.com/callback"}, + wantRetryType: "native", + wantRegistered: true, + }, + { + name: "retry also fails", + redirectURIs: []string{"http://localhost:8085/callback"}, + rejectOnRetry: true, + wantRegistered: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var attempt int + regHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + attempt++ + body, _ := io.ReadAll(r.Body) + var meta oauthex.ClientRegistrationMetadata + json.Unmarshal(body, &meta) + + if attempt == 1 { + // Always reject the first attempt with invalid_redirect_uri. + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + w.Write([]byte(`{"error":"invalid_redirect_uri","error_description":"Redirect URI not allowed for this application type"}`)) + return + } + if tt.rejectOnRetry { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + w.Write([]byte(`{"error":"invalid_redirect_uri","error_description":"Still not allowed"}`)) + return + } + // Verify the retry used the adjusted application_type. + if meta.ApplicationType != tt.wantRetryType { + t.Errorf("retry application_type = %q, want %q", meta.ApplicationType, tt.wantRetryType) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + json.NewEncoder(w).Encode(&oauthex.ClientRegistrationResponse{ + ClientID: "test-client", + ClientSecret: "test-secret", + ClientRegistrationMetadata: meta, + }) + }) + + server := httptest.NewServer(regHandler) + t.Cleanup(server.Close) + + handler, err := NewAuthorizationCodeHandler(&AuthorizationCodeHandlerConfig{ + DynamicClientRegistrationConfig: &DynamicClientRegistrationConfig{ + Metadata: &oauthex.ClientRegistrationMetadata{ + RedirectURIs: tt.redirectURIs, + }, + }, + RedirectURL: tt.redirectURIs[0], + AuthorizationCodeFetcher: func(ctx context.Context, args *AuthorizationArgs) (*AuthorizationResult, error) { + return nil, nil + }, + }) + if err != nil { + t.Fatalf("NewAuthorizationCodeHandler() error = %v", err) + } + + asm := &oauthex.AuthServerMeta{ + RegistrationEndpoint: server.URL, + } + + got, err := handler.handleRegistration(t.Context(), asm) + if !tt.wantRegistered { + if err == nil { + t.Fatal("handleRegistration() = nil error, want error") + } + return + } + if err != nil { + t.Fatalf("handleRegistration() error = %v", err) + } + if got.clientID != "test-client" { + t.Errorf("clientID = %q, want %q", got.clientID, "test-client") + } + if attempt != 2 { + t.Errorf("registration attempts = %d, want 2", attempt) + } + }) + } +} + +func TestInferApplicationType(t *testing.T) { + tests := []struct { + name string + redirectURIs []string + want string + }{ + { + name: "localhost", + redirectURIs: []string{"http://localhost:8085/callback"}, + want: "native", + }, + { + name: "127.0.0.1", + redirectURIs: []string{"http://127.0.0.1:8085/callback"}, + want: "native", + }, + { + name: "IPv6 loopback", + redirectURIs: []string{"http://[::1]:8085/callback"}, + want: "native", + }, + { + name: "custom scheme", + redirectURIs: []string{"myapp://callback"}, + want: "native", + }, + { + name: "HTTPS remote", + redirectURIs: []string{"https://myapp.example.com/callback"}, + want: "web", + }, + { + name: "mixed with localhost", + redirectURIs: []string{"https://myapp.example.com/callback", "http://localhost:8085/callback"}, + want: "native", + }, + { + name: "multiple remote", + redirectURIs: []string{"https://app1.example.com/cb", "https://app2.example.com/cb"}, + want: "web", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := inferApplicationType(tt.redirectURIs) + if got != tt.want { + t.Errorf("inferApplicationType() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestApplicationTypeInference(t *testing.T) { + fetcher := func(ctx context.Context, args *AuthorizationArgs) (*AuthorizationResult, error) { + return nil, nil + } + + t.Run("inferred as native for localhost", func(t *testing.T) { + cfg := &AuthorizationCodeHandlerConfig{ + DynamicClientRegistrationConfig: &DynamicClientRegistrationConfig{ + Metadata: &oauthex.ClientRegistrationMetadata{ + RedirectURIs: []string{"http://localhost:8085/callback"}, + }, + }, + AuthorizationCodeFetcher: fetcher, + } + if _, err := NewAuthorizationCodeHandler(cfg); err != nil { + t.Fatalf("NewAuthorizationCodeHandler() error = %v", err) + } + if cfg.DynamicClientRegistrationConfig.Metadata.ApplicationType != "native" { + t.Errorf("ApplicationType = %q, want %q", cfg.DynamicClientRegistrationConfig.Metadata.ApplicationType, "native") + } + }) + + t.Run("inferred as web for remote", func(t *testing.T) { + cfg := &AuthorizationCodeHandlerConfig{ + DynamicClientRegistrationConfig: &DynamicClientRegistrationConfig{ + Metadata: &oauthex.ClientRegistrationMetadata{ + RedirectURIs: []string{"https://example.com/callback"}, + }, + }, + AuthorizationCodeFetcher: fetcher, + } + if _, err := NewAuthorizationCodeHandler(cfg); err != nil { + t.Fatalf("NewAuthorizationCodeHandler() error = %v", err) + } + if cfg.DynamicClientRegistrationConfig.Metadata.ApplicationType != "web" { + t.Errorf("ApplicationType = %q, want %q", cfg.DynamicClientRegistrationConfig.Metadata.ApplicationType, "web") + } + }) + + t.Run("explicit value preserved", func(t *testing.T) { + cfg := &AuthorizationCodeHandlerConfig{ + DynamicClientRegistrationConfig: &DynamicClientRegistrationConfig{ + Metadata: &oauthex.ClientRegistrationMetadata{ + RedirectURIs: []string{"http://localhost:8085/callback"}, + ApplicationType: "web", + }, + }, + AuthorizationCodeFetcher: fetcher, + } + if _, err := NewAuthorizationCodeHandler(cfg); err != nil { + t.Fatalf("NewAuthorizationCodeHandler() error = %v", err) + } + if cfg.DynamicClientRegistrationConfig.Metadata.ApplicationType != "web" { + t.Errorf("ApplicationType = %q, want %q (should not be overridden)", cfg.DynamicClientRegistrationConfig.Metadata.ApplicationType, "web") + } + }) +} + // validConfig for test to create an AuthorizationCodeHandler using its constructor. // Values that are relevant to the test should be set explicitly. func validConfig() *AuthorizationCodeHandlerConfig { diff --git a/oauthex/dcr.go b/oauthex/dcr.go index ce21467e..b3013dd8 100644 --- a/oauthex/dcr.go +++ b/oauthex/dcr.go @@ -89,6 +89,10 @@ type ClientRegistrationMetadata struct { // SoftwareStatement is an OPTIONAL JWT that asserts client metadata values. // Values in the software statement take precedence over other metadata values. SoftwareStatement string `json:"software_statement,omitempty"` + + // ApplicationType indicates the type of application, valid values are "native" and "web". + // If omitted, OIDC-compliant authorization servers default to "web". + ApplicationType string `json:"application_type,omitempty"` } // ClientRegistrationResponse represents the fields returned by the Authorization Server From f8b9373e3c75234156801dfecc5a5237b8aa6c1b Mon Sep 17 00:00:00 2001 From: guglielmoc Date: Thu, 23 Apr 2026 12:58:11 +0000 Subject: [PATCH 2/7] refactor: replace application type retry logic with explicit validation and strict inference from redirect URIs --- auth/authorization_code.go | 53 +++++++------ auth/authorization_code_test.go | 131 +++++--------------------------- oauthex/dcr.go | 3 +- 3 files changed, 49 insertions(+), 138 deletions(-) diff --git a/auth/authorization_code.go b/auth/authorization_code.go index 8b086ae8..35bdccf1 100644 --- a/auth/authorization_code.go +++ b/auth/authorization_code.go @@ -15,6 +15,7 @@ import ( "slices" "strings" + "github.com/modelcontextprotocol/go-sdk/internal/util" "github.com/modelcontextprotocol/go-sdk/oauthex" "golang.org/x/oauth2" ) @@ -151,7 +152,11 @@ func NewAuthorizationCodeHandler(config *AuthorizationCodeHandlerConfig) (*Autho return nil, fmt.Errorf("RedirectURL %q is not in the list of allowed redirect URIs for dynamic client registration", config.RedirectURL) } if dCfg.Metadata.ApplicationType == "" { - dCfg.Metadata.ApplicationType = inferApplicationType(dCfg.Metadata.RedirectURIs) + var err error + dCfg.Metadata.ApplicationType, err = inferApplicationType(dCfg.Metadata.RedirectURIs) + if err != nil { + return nil, fmt.Errorf("failed to infer application type from redirect URIs: %w", err) + } } } if config.RedirectURL == "" { @@ -174,30 +179,38 @@ func isNonRootHTTPSURL(u string) bool { } // inferApplicationType returns "native" or "web" based on the redirect URIs. -// If any redirect URI uses a loopback host or a non-http(s) scheme (custom -// scheme), the application is classified as "native". Otherwise, it is "web". -func inferApplicationType(redirectURIs []string) string { +// If all redirect URIs use a loopback host or a non-http(s) scheme, +// the application is classified as "native". Otherwise, it is "web". +func inferApplicationType(redirectURIs []string) (string, error) { + hasNative := false + hasWeb := false for _, uri := range redirectURIs { u, err := url.Parse(uri) if err != nil { - continue + return "", fmt.Errorf("invalid redirect URI %q: %w", uri, err) } switch u.Scheme { case "http", "https": - if isLoopback(u.Hostname()) { - return "native" + if util.IsLoopback(u.Hostname()) { + hasNative = true + } else { + hasWeb = true } default: - return "native" + hasNative = true } } - return "web" -} -func isLoopback(host string) bool { - return host == "localhost" || host == "127.0.0.1" || host == "::1" + if hasNative && hasWeb { + return "", errors.New("mixed redirect URI types: found both native and web URIs") + } + if hasNative { + return "native", nil + } + return "web", nil } + // Authorize performs the authorization flow. // It is designed to perform the whole Authorization Code Grant flow. // On success, [AuthorizationCodeHandler.TokenSource] will return a token source with the fetched token. @@ -460,21 +473,7 @@ func (h *AuthorizationCodeHandler) handleRegistration(ctx context.Context, asm * if dcrCfg != nil && asm.RegistrationEndpoint != "" { regResp, err := oauthex.RegisterClient(ctx, asm.RegistrationEndpoint, dcrCfg.Metadata, h.config.Client) if err != nil { - // If registration failed due to redirect URI constraints, retry with - // the opposite application_type per the MCP specification. - var regErr *oauthex.ClientRegistrationError - if errors.As(err, ®Err) && regErr.ErrorCode == "invalid_redirect_uri" { - retryType := "native" - if dcrCfg.Metadata.ApplicationType == "native" { - retryType = "web" - } - retryCfg := *dcrCfg.Metadata - retryCfg.ApplicationType = retryType - regResp, err = oauthex.RegisterClient(ctx, asm.RegistrationEndpoint, &retryCfg, h.config.Client) - } - if err != nil { - return nil, fmt.Errorf("failed to register client: %w", err) - } + return nil, fmt.Errorf("failed to register client: %w", err) } cfg := &resolvedClientConfig{ registrationType: registrationTypeDynamic, diff --git a/auth/authorization_code_test.go b/auth/authorization_code_test.go index 8d25acfa..1479ba79 100644 --- a/auth/authorization_code_test.go +++ b/auth/authorization_code_test.go @@ -6,9 +6,7 @@ package auth import ( "context" - "encoding/json" "fmt" - "io" "net/http" "net/http/httptest" "net/http/httputil" @@ -610,116 +608,12 @@ func TestDynamicRegistration(t *testing.T) { } } -func TestDynamicRegistrationRetryApplicationType(t *testing.T) { - tests := []struct { - name string - redirectURIs []string - wantRetryType string - wantRegistered bool - rejectOnRetry bool - }{ - { - name: "inferred native retries with web", - redirectURIs: []string{"http://localhost:8085/callback"}, - wantRetryType: "web", - wantRegistered: true, - }, - { - name: "inferred web retries with native", - redirectURIs: []string{"https://example.com/callback"}, - wantRetryType: "native", - wantRegistered: true, - }, - { - name: "retry also fails", - redirectURIs: []string{"http://localhost:8085/callback"}, - rejectOnRetry: true, - wantRegistered: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - var attempt int - regHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - attempt++ - body, _ := io.ReadAll(r.Body) - var meta oauthex.ClientRegistrationMetadata - json.Unmarshal(body, &meta) - - if attempt == 1 { - // Always reject the first attempt with invalid_redirect_uri. - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(`{"error":"invalid_redirect_uri","error_description":"Redirect URI not allowed for this application type"}`)) - return - } - if tt.rejectOnRetry { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(`{"error":"invalid_redirect_uri","error_description":"Still not allowed"}`)) - return - } - // Verify the retry used the adjusted application_type. - if meta.ApplicationType != tt.wantRetryType { - t.Errorf("retry application_type = %q, want %q", meta.ApplicationType, tt.wantRetryType) - } - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(&oauthex.ClientRegistrationResponse{ - ClientID: "test-client", - ClientSecret: "test-secret", - ClientRegistrationMetadata: meta, - }) - }) - - server := httptest.NewServer(regHandler) - t.Cleanup(server.Close) - - handler, err := NewAuthorizationCodeHandler(&AuthorizationCodeHandlerConfig{ - DynamicClientRegistrationConfig: &DynamicClientRegistrationConfig{ - Metadata: &oauthex.ClientRegistrationMetadata{ - RedirectURIs: tt.redirectURIs, - }, - }, - RedirectURL: tt.redirectURIs[0], - AuthorizationCodeFetcher: func(ctx context.Context, args *AuthorizationArgs) (*AuthorizationResult, error) { - return nil, nil - }, - }) - if err != nil { - t.Fatalf("NewAuthorizationCodeHandler() error = %v", err) - } - - asm := &oauthex.AuthServerMeta{ - RegistrationEndpoint: server.URL, - } - - got, err := handler.handleRegistration(t.Context(), asm) - if !tt.wantRegistered { - if err == nil { - t.Fatal("handleRegistration() = nil error, want error") - } - return - } - if err != nil { - t.Fatalf("handleRegistration() error = %v", err) - } - if got.clientID != "test-client" { - t.Errorf("clientID = %q, want %q", got.clientID, "test-client") - } - if attempt != 2 { - t.Errorf("registration attempts = %d, want 2", attempt) - } - }) - } -} - func TestInferApplicationType(t *testing.T) { tests := []struct { name string redirectURIs []string want string + wantErr bool }{ { name: "localhost", @@ -747,9 +641,9 @@ func TestInferApplicationType(t *testing.T) { want: "web", }, { - name: "mixed with localhost", + name: "mixed native and web", redirectURIs: []string{"https://myapp.example.com/callback", "http://localhost:8085/callback"}, - want: "native", + wantErr: true, }, { name: "multiple remote", @@ -759,7 +653,10 @@ func TestInferApplicationType(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := inferApplicationType(tt.redirectURIs) + got, err := inferApplicationType(tt.redirectURIs) + if (err != nil) != tt.wantErr { + t.Fatalf("inferApplicationType() error = %v, wantErr %v", err, tt.wantErr) + } if got != tt.want { t.Errorf("inferApplicationType() = %q, want %q", got, tt.want) } @@ -806,6 +703,20 @@ func TestApplicationTypeInference(t *testing.T) { } }) + t.Run("mixed native and web URIs returns error", func(t *testing.T) { + cfg := &AuthorizationCodeHandlerConfig{ + DynamicClientRegistrationConfig: &DynamicClientRegistrationConfig{ + Metadata: &oauthex.ClientRegistrationMetadata{ + RedirectURIs: []string{"https://example.com/callback", "http://localhost:8085/callback"}, + }, + }, + AuthorizationCodeFetcher: fetcher, + } + if _, err := NewAuthorizationCodeHandler(cfg); err == nil { + t.Fatal("NewAuthorizationCodeHandler() = nil error, want error for mixed redirect URI types") + } + }) + t.Run("explicit value preserved", func(t *testing.T) { cfg := &AuthorizationCodeHandlerConfig{ DynamicClientRegistrationConfig: &DynamicClientRegistrationConfig{ diff --git a/oauthex/dcr.go b/oauthex/dcr.go index b3013dd8..f46d0e8e 100644 --- a/oauthex/dcr.go +++ b/oauthex/dcr.go @@ -90,7 +90,8 @@ type ClientRegistrationMetadata struct { // Values in the software statement take precedence over other metadata values. SoftwareStatement string `json:"software_statement,omitempty"` - // ApplicationType indicates the type of application, valid values are "native" and "web". + // ApplicationType is an OPTIONAL string that indicates the type of application. + // Valid values are "native" and "web". // If omitted, OIDC-compliant authorization servers default to "web". ApplicationType string `json:"application_type,omitempty"` } From 85bb7ad64d06382ac11e003d0aefe116bd19a15c Mon Sep 17 00:00:00 2001 From: guglielmoc Date: Thu, 23 Apr 2026 13:02:51 +0000 Subject: [PATCH 3/7] refactor: extract application type inference to a local variable before assignment in dynamic client registration --- auth/authorization_code.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/auth/authorization_code.go b/auth/authorization_code.go index 35bdccf1..6bcc809c 100644 --- a/auth/authorization_code.go +++ b/auth/authorization_code.go @@ -152,11 +152,11 @@ func NewAuthorizationCodeHandler(config *AuthorizationCodeHandlerConfig) (*Autho return nil, fmt.Errorf("RedirectURL %q is not in the list of allowed redirect URIs for dynamic client registration", config.RedirectURL) } if dCfg.Metadata.ApplicationType == "" { - var err error - dCfg.Metadata.ApplicationType, err = inferApplicationType(dCfg.Metadata.RedirectURIs) + applicationType, err := inferApplicationType(dCfg.Metadata.RedirectURIs) if err != nil { return nil, fmt.Errorf("failed to infer application type from redirect URIs: %w", err) } + dCfg.Metadata.ApplicationType = applicationType } } if config.RedirectURL == "" { From fbffb6ee3898b1fa2405ba95f8f65dd354ca66e6 Mon Sep 17 00:00:00 2001 From: guglielmoc Date: Thu, 23 Apr 2026 13:06:36 +0000 Subject: [PATCH 4/7] docs: remove obsolete comment from inferApplicationType and clean up whitespace --- auth/authorization_code.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/auth/authorization_code.go b/auth/authorization_code.go index 6bcc809c..4602afdd 100644 --- a/auth/authorization_code.go +++ b/auth/authorization_code.go @@ -179,8 +179,6 @@ func isNonRootHTTPSURL(u string) bool { } // inferApplicationType returns "native" or "web" based on the redirect URIs. -// If all redirect URIs use a loopback host or a non-http(s) scheme, -// the application is classified as "native". Otherwise, it is "web". func inferApplicationType(redirectURIs []string) (string, error) { hasNative := false hasWeb := false @@ -210,7 +208,6 @@ func inferApplicationType(redirectURIs []string) (string, error) { return "web", nil } - // Authorize performs the authorization flow. // It is designed to perform the whole Authorization Code Grant flow. // On success, [AuthorizationCodeHandler.TokenSource] will return a token source with the fetched token. From 42e78df6441df8066be522c028e51446abf32604 Mon Sep 17 00:00:00 2001 From: guglielmoc Date: Thu, 23 Apr 2026 14:54:14 +0000 Subject: [PATCH 5/7] refactor: make inferApplicationType non-failing by returning empty string on invalid inputs or conflicts --- auth/authorization_code.go | 21 +++--- auth/authorization_code_test.go | 123 ++++++++++++++------------------ 2 files changed, 65 insertions(+), 79 deletions(-) diff --git a/auth/authorization_code.go b/auth/authorization_code.go index 4602afdd..07360e7c 100644 --- a/auth/authorization_code.go +++ b/auth/authorization_code.go @@ -35,6 +35,11 @@ type ClientIDMetadataDocumentConfig struct { type DynamicClientRegistrationConfig struct { // Metadata to be used in dynamic client registration request as per // https://datatracker.ietf.org/doc/html/rfc7591#section-2. + // + // If Metadata.ApplicationType is empty, it will be inferred from + // Metadata.RedirectURIs. If all valid URIs are compatible (e.g., all loopback + // or custom schemes for "native", or all non-loopback HTTP/HTTPS for "web"), + // the inferred type will be set. Otherwise, it is left empty. Metadata *oauthex.ClientRegistrationMetadata } @@ -152,11 +157,7 @@ func NewAuthorizationCodeHandler(config *AuthorizationCodeHandlerConfig) (*Autho return nil, fmt.Errorf("RedirectURL %q is not in the list of allowed redirect URIs for dynamic client registration", config.RedirectURL) } if dCfg.Metadata.ApplicationType == "" { - applicationType, err := inferApplicationType(dCfg.Metadata.RedirectURIs) - if err != nil { - return nil, fmt.Errorf("failed to infer application type from redirect URIs: %w", err) - } - dCfg.Metadata.ApplicationType = applicationType + dCfg.Metadata.ApplicationType = inferApplicationType(dCfg.Metadata.RedirectURIs) } } if config.RedirectURL == "" { @@ -179,13 +180,13 @@ func isNonRootHTTPSURL(u string) bool { } // inferApplicationType returns "native" or "web" based on the redirect URIs. -func inferApplicationType(redirectURIs []string) (string, error) { +func inferApplicationType(redirectURIs []string) string { hasNative := false hasWeb := false for _, uri := range redirectURIs { u, err := url.Parse(uri) if err != nil { - return "", fmt.Errorf("invalid redirect URI %q: %w", uri, err) + return "" } switch u.Scheme { case "http", "https": @@ -200,12 +201,12 @@ func inferApplicationType(redirectURIs []string) (string, error) { } if hasNative && hasWeb { - return "", errors.New("mixed redirect URI types: found both native and web URIs") + return "" } if hasNative { - return "native", nil + return "native" } - return "web", nil + return "web" } // Authorize performs the authorization flow. diff --git a/auth/authorization_code_test.go b/auth/authorization_code_test.go index 1479ba79..c7456d19 100644 --- a/auth/authorization_code_test.go +++ b/auth/authorization_code_test.go @@ -613,7 +613,6 @@ func TestInferApplicationType(t *testing.T) { name string redirectURIs []string want string - wantErr bool }{ { name: "localhost", @@ -643,7 +642,7 @@ func TestInferApplicationType(t *testing.T) { { name: "mixed native and web", redirectURIs: []string{"https://myapp.example.com/callback", "http://localhost:8085/callback"}, - wantErr: true, + want: "", }, { name: "multiple remote", @@ -653,10 +652,7 @@ func TestInferApplicationType(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := inferApplicationType(tt.redirectURIs) - if (err != nil) != tt.wantErr { - t.Fatalf("inferApplicationType() error = %v, wantErr %v", err, tt.wantErr) - } + got := inferApplicationType(tt.redirectURIs) if got != tt.want { t.Errorf("inferApplicationType() = %q, want %q", got, tt.want) } @@ -669,71 +665,60 @@ func TestApplicationTypeInference(t *testing.T) { return nil, nil } - t.Run("inferred as native for localhost", func(t *testing.T) { - cfg := &AuthorizationCodeHandlerConfig{ - DynamicClientRegistrationConfig: &DynamicClientRegistrationConfig{ - Metadata: &oauthex.ClientRegistrationMetadata{ - RedirectURIs: []string{"http://localhost:8085/callback"}, - }, - }, - AuthorizationCodeFetcher: fetcher, - } - if _, err := NewAuthorizationCodeHandler(cfg); err != nil { - t.Fatalf("NewAuthorizationCodeHandler() error = %v", err) - } - if cfg.DynamicClientRegistrationConfig.Metadata.ApplicationType != "native" { - t.Errorf("ApplicationType = %q, want %q", cfg.DynamicClientRegistrationConfig.Metadata.ApplicationType, "native") - } - }) - - t.Run("inferred as web for remote", func(t *testing.T) { - cfg := &AuthorizationCodeHandlerConfig{ - DynamicClientRegistrationConfig: &DynamicClientRegistrationConfig{ - Metadata: &oauthex.ClientRegistrationMetadata{ - RedirectURIs: []string{"https://example.com/callback"}, - }, - }, - AuthorizationCodeFetcher: fetcher, - } - if _, err := NewAuthorizationCodeHandler(cfg); err != nil { - t.Fatalf("NewAuthorizationCodeHandler() error = %v", err) - } - if cfg.DynamicClientRegistrationConfig.Metadata.ApplicationType != "web" { - t.Errorf("ApplicationType = %q, want %q", cfg.DynamicClientRegistrationConfig.Metadata.ApplicationType, "web") - } - }) - - t.Run("mixed native and web URIs returns error", func(t *testing.T) { - cfg := &AuthorizationCodeHandlerConfig{ - DynamicClientRegistrationConfig: &DynamicClientRegistrationConfig{ - Metadata: &oauthex.ClientRegistrationMetadata{ - RedirectURIs: []string{"https://example.com/callback", "http://localhost:8085/callback"}, - }, - }, - AuthorizationCodeFetcher: fetcher, - } - if _, err := NewAuthorizationCodeHandler(cfg); err == nil { - t.Fatal("NewAuthorizationCodeHandler() = nil error, want error for mixed redirect URI types") - } - }) + tests := []struct { + name string + redirectURIs []string + initialAppType string + wantAppType string + }{ + { + name: "inferred as native for localhost", + redirectURIs: []string{"http://localhost:8085/callback"}, + wantAppType: "native", + }, + { + name: "inferred as web for remote", + redirectURIs: []string{"https://example.com/callback"}, + wantAppType: "web", + }, + { + name: "mixed native and web URIs sets empty application type", + redirectURIs: []string{"https://example.com/callback", "http://localhost:8085/callback"}, + wantAppType: "", + }, + { + name: "explicit value preserved", + redirectURIs: []string{"http://localhost:8085/callback"}, + initialAppType: "web", + wantAppType: "web", + }, + { + name: "invalid URI returns empty application type", + redirectURIs: []string{"http://%/"}, + wantAppType: "", + }, + } - t.Run("explicit value preserved", func(t *testing.T) { - cfg := &AuthorizationCodeHandlerConfig{ - DynamicClientRegistrationConfig: &DynamicClientRegistrationConfig{ - Metadata: &oauthex.ClientRegistrationMetadata{ - RedirectURIs: []string{"http://localhost:8085/callback"}, - ApplicationType: "web", + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := &AuthorizationCodeHandlerConfig{ + DynamicClientRegistrationConfig: &DynamicClientRegistrationConfig{ + Metadata: &oauthex.ClientRegistrationMetadata{ + RedirectURIs: tt.redirectURIs, + ApplicationType: tt.initialAppType, + }, }, - }, - AuthorizationCodeFetcher: fetcher, - } - if _, err := NewAuthorizationCodeHandler(cfg); err != nil { - t.Fatalf("NewAuthorizationCodeHandler() error = %v", err) - } - if cfg.DynamicClientRegistrationConfig.Metadata.ApplicationType != "web" { - t.Errorf("ApplicationType = %q, want %q (should not be overridden)", cfg.DynamicClientRegistrationConfig.Metadata.ApplicationType, "web") - } - }) + AuthorizationCodeFetcher: fetcher, + } + if _, err := NewAuthorizationCodeHandler(cfg); err != nil { + t.Fatalf("NewAuthorizationCodeHandler() error = %v", err) + } + got := cfg.DynamicClientRegistrationConfig.Metadata.ApplicationType + if got != tt.wantAppType { + t.Errorf("ApplicationType = %q, want %q", got, tt.wantAppType) + } + }) + } } // validConfig for test to create an AuthorizationCodeHandler using its constructor. From de653926968af5f366359f1c48340523fe9599ff Mon Sep 17 00:00:00 2001 From: guglielmoc Date: Thu, 23 Apr 2026 15:35:16 +0000 Subject: [PATCH 6/7] feat: validate configured ApplicationType against inferred type in dynamic client registration --- auth/authorization_code.go | 12 +++++++----- auth/authorization_code_test.go | 25 +++++++++++++++++++++---- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/auth/authorization_code.go b/auth/authorization_code.go index 07360e7c..e7d4d78f 100644 --- a/auth/authorization_code.go +++ b/auth/authorization_code.go @@ -37,9 +37,8 @@ type DynamicClientRegistrationConfig struct { // https://datatracker.ietf.org/doc/html/rfc7591#section-2. // // If Metadata.ApplicationType is empty, it will be inferred from - // Metadata.RedirectURIs. If all valid URIs are compatible (e.g., all loopback - // or custom schemes for "native", or all non-loopback HTTP/HTTPS for "web"), - // the inferred type will be set. Otherwise, it is left empty. + // Metadata.RedirectURIs. When set will be validated against the inferred type + // and an error will be returned if they conflict. Metadata *oauthex.ClientRegistrationMetadata } @@ -156,8 +155,11 @@ func NewAuthorizationCodeHandler(config *AuthorizationCodeHandlerConfig) (*Autho } else if !slices.Contains(dCfg.Metadata.RedirectURIs, config.RedirectURL) { return nil, fmt.Errorf("RedirectURL %q is not in the list of allowed redirect URIs for dynamic client registration", config.RedirectURL) } + applicationType := inferApplicationType(dCfg.Metadata.RedirectURIs) if dCfg.Metadata.ApplicationType == "" { - dCfg.Metadata.ApplicationType = inferApplicationType(dCfg.Metadata.RedirectURIs) + dCfg.Metadata.ApplicationType = applicationType + } else if dCfg.Metadata.ApplicationType != applicationType { + return nil, fmt.Errorf("application type %q conflicts with the application type inferred from redirect URIs", dCfg.Metadata.ApplicationType) } } if config.RedirectURL == "" { @@ -179,7 +181,7 @@ func isNonRootHTTPSURL(u string) bool { return pu.Scheme == "https" && pu.Path != "" } -// inferApplicationType returns "native" or "web" based on the redirect URIs. +// inferApplicationType returns an application type based on the redirect URIs. func inferApplicationType(redirectURIs []string) string { hasNative := false hasWeb := false diff --git a/auth/authorization_code_test.go b/auth/authorization_code_test.go index c7456d19..92b6faf8 100644 --- a/auth/authorization_code_test.go +++ b/auth/authorization_code_test.go @@ -670,6 +670,7 @@ func TestApplicationTypeInference(t *testing.T) { redirectURIs []string initialAppType string wantAppType string + wantErr bool }{ { name: "inferred as native for localhost", @@ -687,10 +688,22 @@ func TestApplicationTypeInference(t *testing.T) { wantAppType: "", }, { - name: "explicit value preserved", + name: "explicit value matching inference is preserved", redirectURIs: []string{"http://localhost:8085/callback"}, + initialAppType: "native", + wantAppType: "native", + }, + { + name: "explicit value conflicts with inference returns error", + redirectURIs: []string{"http://localhost:8085/callback"}, + initialAppType: "web", + wantErr: true, + }, + { + name: "explicit value when inference is ambiguous returns error", + redirectURIs: []string{"https://example.com/callback", "http://localhost:8085/callback"}, initialAppType: "web", - wantAppType: "web", + wantErr: true, }, { name: "invalid URI returns empty application type", @@ -710,8 +723,12 @@ func TestApplicationTypeInference(t *testing.T) { }, AuthorizationCodeFetcher: fetcher, } - if _, err := NewAuthorizationCodeHandler(cfg); err != nil { - t.Fatalf("NewAuthorizationCodeHandler() error = %v", err) + _, err := NewAuthorizationCodeHandler(cfg) + if (err != nil) != tt.wantErr { + t.Fatalf("NewAuthorizationCodeHandler() error = %v, wantErr %v", err, tt.wantErr) + } + if tt.wantErr { + return } got := cfg.DynamicClientRegistrationConfig.Metadata.ApplicationType if got != tt.wantAppType { From 94286056855b53001fd978e421553eca12edeb5f Mon Sep 17 00:00:00 2001 From: Guglielmo Colombo Date: Fri, 24 Apr 2026 17:16:29 +0200 Subject: [PATCH 7/7] Update auth/authorization_code.go Co-authored-by: Maciej Kisiel --- auth/authorization_code.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auth/authorization_code.go b/auth/authorization_code.go index e7d4d78f..758b88e3 100644 --- a/auth/authorization_code.go +++ b/auth/authorization_code.go @@ -37,7 +37,7 @@ type DynamicClientRegistrationConfig struct { // https://datatracker.ietf.org/doc/html/rfc7591#section-2. // // If Metadata.ApplicationType is empty, it will be inferred from - // Metadata.RedirectURIs. When set will be validated against the inferred type + // Metadata.RedirectURIs. When set, it will be validated against the inferred type // and an error will be returned if they conflict. Metadata *oauthex.ClientRegistrationMetadata }