refactor(plugins): enhance user authorization handling and streamline form data processing

Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan 2026-02-13 15:47:51 -05:00
parent b1a51f9bbe
commit 425fe862ba
4 changed files with 62 additions and 63 deletions

View File

@ -86,11 +86,8 @@ func (s *subsonicAPIServiceImpl) executeRequest(ctx context.Context, uri string,
RawQuery: query.Encode(),
}
// Create HTTP request with a fresh context to avoid Chi RouteContext pollution.
// Using http.NewRequest (instead of http.NewRequestWithContext) ensures the internal
// SubsonicAPI call doesn't inherit routing information from the parent handler,
// which would cause Chi to invoke the wrong handler. Authentication context is
// explicitly added in the next step via request.WithInternalAuth.
// Use http.NewRequest (not WithContext) to avoid inheriting Chi RouteContext;
// auth context is set explicitly below via request.WithInternalAuth.
httpReq, err := http.NewRequest("GET", finalURL.String(), nil)
if err != nil {
return nil, fmt.Errorf("failed to create HTTP request: %w", err)

View File

@ -104,7 +104,8 @@ func (h *endpointHandler) serveWithNativeAuth(w http.ResponseWriter, r *http.Req
func (h *endpointHandler) dispatch(w http.ResponseWriter, r *http.Request, p *plugin) {
ctx := r.Context()
// Check user authorization (skip for auth:"none")
// Check user authorization and extract user info (skip for auth:"none")
var httpUser *capabilities.HTTPUser
if p.manifest.Permissions.Endpoints.Auth != EndpointsPermissionAuthNone {
user, ok := request.UserFrom(ctx)
if !ok {
@ -116,6 +117,12 @@ func (h *endpointHandler) dispatch(w http.ResponseWriter, r *http.Request, p *pl
http.Error(w, "Forbidden", http.StatusForbidden)
return
}
httpUser = &capabilities.HTTPUser{
ID: user.ID,
Username: user.UserName,
Name: user.Name,
IsAdmin: user.IsAdmin,
}
}
// Read request body with size limit
@ -128,21 +135,10 @@ func (h *endpointHandler) dispatch(w http.ResponseWriter, r *http.Request, p *pl
// Build the plugin request
// Normalize path: both /ext/plugin and /ext/plugin/ map to ""
relPath := "/" + chi.URLParam(r, "*")
if relPath == "/" || relPath == "" {
relPath = ""
}
var httpUser *capabilities.HTTPUser
if p.manifest.Permissions.Endpoints.Auth != EndpointsPermissionAuthNone {
if user, ok := request.UserFrom(ctx); ok {
httpUser = &capabilities.HTTPUser{
ID: user.ID,
Username: user.UserName,
Name: user.Name,
IsAdmin: user.IsAdmin,
}
}
rawPath := chi.URLParam(r, "*")
relPath := ""
if rawPath != "" {
relPath = "/" + rawPath
}
pluginReq := capabilities.HTTPHandleRequest{

View File

@ -61,39 +61,44 @@ func callPluginFunction[I any, O any](ctx context.Context, plugin *plugin, funcN
startCall := time.Now()
exit, output, err := p.CallWithContext(ctx, funcName, inputBytes)
elapsed := time.Since(startCall)
success := false
skipMetrics := false
defer func() {
if !skipMetrics {
plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, success, elapsed.Milliseconds())
}
}()
if err != nil {
// If context was cancelled, return that error instead of the plugin error
if ctx.Err() != nil {
skipMetrics = true
log.Debug(ctx, "Plugin call cancelled", "plugin", plugin.name, "function", funcName, "pluginDuration", elapsed)
return result, ctx.Err()
}
plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, false, elapsed.Milliseconds())
log.Trace(ctx, "Plugin call failed", "plugin", plugin.name, "function", funcName, "pluginDuration", elapsed, "navidromeDuration", startCall.Sub(start), err)
return result, fmt.Errorf("plugin call failed: %w", err)
}
if exit != 0 {
if exit == notImplementedCode {
skipMetrics = true
log.Trace(ctx, "Plugin function not implemented", "plugin", plugin.name, "function", funcName, "pluginDuration", elapsed, "navidromeDuration", startCall.Sub(start))
// TODO Should we record metrics for not implemented calls?
//plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, true, elapsed.Milliseconds())
return result, fmt.Errorf("%w: %s", errNotImplemented, funcName)
}
plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, false, elapsed.Milliseconds())
return result, fmt.Errorf("plugin call exited with code %d", exit)
}
if len(output) > 0 {
err = json.Unmarshal(output, &result)
if err != nil {
if err = json.Unmarshal(output, &result); err != nil {
log.Trace(ctx, "Plugin call failed", "plugin", plugin.name, "function", funcName, "pluginDuration", elapsed, "navidromeDuration", startCall.Sub(start), err)
return result, err
}
}
// Record metrics for successful calls (or JSON unmarshal failures)
plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, err == nil, elapsed.Milliseconds())
success = true
log.Trace(ctx, "Plugin call succeeded", "plugin", plugin.name, "function", funcName, "pluginDuration", time.Since(startCall), "navidromeDuration", startCall.Sub(start))
return result, err
return result, nil
}
// callPluginFunctionRaw calls a plugin function using binary framing for []byte fields.
@ -133,64 +138,64 @@ func callPluginFunctionRaw[I any, O any](
startCall := time.Now()
exit, output, err := p.CallWithContext(ctx, funcName, frame)
elapsed := time.Since(startCall)
success := false
skipMetrics := false
defer func() {
if !skipMetrics {
plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, success, elapsed.Milliseconds())
}
}()
if err != nil {
if ctx.Err() != nil {
skipMetrics = true
log.Debug(ctx, "Plugin call cancelled", "plugin", plugin.name, "function", funcName, "pluginDuration", elapsed)
return result, ctx.Err()
}
plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, false, elapsed.Milliseconds())
log.Trace(ctx, "Plugin call failed", "plugin", plugin.name, "function", funcName, "pluginDuration", elapsed, "navidromeDuration", startCall.Sub(start), err)
return result, fmt.Errorf("plugin call failed: %w", err)
}
if exit != 0 {
if exit == notImplementedCode {
skipMetrics = true
log.Trace(ctx, "Plugin function not implemented", "plugin", plugin.name, "function", funcName, "pluginDuration", elapsed, "navidromeDuration", startCall.Sub(start))
return result, fmt.Errorf("%w: %s", errNotImplemented, funcName)
}
plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, false, elapsed.Milliseconds())
return result, fmt.Errorf("plugin call exited with code %d", exit)
}
// Parse output frame
if len(output) < 1 {
plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, false, elapsed.Milliseconds())
return result, fmt.Errorf("empty response from plugin")
}
statusByte := output[0]
if statusByte == 0x01 {
// Error frame: [0x01][UTF-8 error message]
plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, false, elapsed.Milliseconds())
errMsg := string(output[1:])
return result, fmt.Errorf("plugin error: %s", errMsg)
return result, fmt.Errorf("plugin error: %s", string(output[1:]))
}
if statusByte != 0x00 {
plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, false, elapsed.Milliseconds())
return result, fmt.Errorf("unknown response status byte: 0x%02x", statusByte)
}
// Success frame: [0x00][json_len:4B][JSON][raw bytes]
if len(output) < 5 {
plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, false, elapsed.Milliseconds())
return result, fmt.Errorf("malformed success response from plugin")
}
jsonLen := binary.BigEndian.Uint32(output[1:5])
if uint32(len(output)-5) < jsonLen {
plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, false, elapsed.Milliseconds())
return result, fmt.Errorf("invalid json length in response frame: %d exceeds available %d bytes", jsonLen, len(output)-5)
}
jsonData := output[5 : 5+jsonLen]
rawData := output[5+jsonLen:]
if err := json.Unmarshal(jsonData, &result); err != nil {
plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, false, elapsed.Milliseconds())
return result, fmt.Errorf("failed to unmarshal response: %w", err)
}
setRawOutput(&result, rawData)
plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, true, elapsed.Milliseconds())
success = true
log.Trace(ctx, "Plugin call succeeded", "plugin", plugin.name, "function", funcName, "pluginDuration", time.Since(startCall), "navidromeDuration", startCall.Sub(start))
return result, nil
}

View File

@ -28,20 +28,29 @@ import (
"github.com/navidrome/navidrome/utils/req"
)
// mergeFormIntoQuery parses form data (both URL query params and POST body)
// and writes all values back into r.URL.RawQuery. This is needed because
// some Subsonic clients send parameters as form fields instead of query params.
// This support the OpenSubsonic `formPost` extension
func mergeFormIntoQuery(r *http.Request) error {
if err := r.ParseForm(); err != nil {
return err
}
var parts []string
for key, values := range r.Form {
for _, v := range values {
parts = append(parts, url.QueryEscape(key)+"="+url.QueryEscape(v))
}
}
r.URL.RawQuery = strings.Join(parts, "&")
return nil
}
func postFormToQueryParams(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
err := r.ParseForm()
if err != nil {
if err := mergeFormIntoQuery(r); err != nil {
sendError(w, r, newError(responses.ErrorGeneric, err.Error()))
}
var parts []string
for key, values := range r.Form {
for _, v := range values {
parts = append(parts, url.QueryEscape(key)+"="+url.QueryEscape(v))
}
}
r.URL.RawQuery = strings.Join(parts, "&")
next.ServeHTTP(w, r)
})
}
@ -169,17 +178,9 @@ func authenticate(ds model.DataStore) func(next http.Handler) http.Handler {
func ValidateAuth(ds model.DataStore, r *http.Request) (*model.User, error) {
// Parse form data into query params (same as postFormToQueryParams middleware,
// which is not in the call chain when ValidateAuth is used directly)
if err := r.ParseForm(); err != nil {
if err := mergeFormIntoQuery(r); err != nil {
return nil, fmt.Errorf("parsing form: %w", err)
}
var parts []string
for key, values := range r.Form {
for _, v := range values {
parts = append(parts, url.QueryEscape(key)+"="+url.QueryEscape(v))
}
}
r.URL.RawQuery = strings.Join(parts, "&")
return authenticateRequest(ds, r)
}