classify user errors in http handlers

Signed-off-by: Kristoffer Dalby <[email protected]>

Author: kradalby

hscontrol/auth.go

@@ -72,7 +72,7 @@ func (h *Headscale) handleExistingNode(
 	machineKey key.MachinePublic,
 ) (*tailcfg.RegisterResponse, error) {
 	if node.MachineKey != machineKey {
-		return nil, errors.New("node already exists with different machine key")
+		return nil, NewHTTPError(http.StatusUnauthorized, "node exist with different machine key", nil)
 	}
 
 	expired := node.IsExpired()
@@ -81,7 +81,7 @@ func (h *Headscale) handleExistingNode(
 
 		// The client is trying to extend their key, this is not allowed.
 		if requestExpiry.After(time.Now()) {
-			return nil, errors.New("extending key is not allowed")
+			return nil, NewHTTPError(http.StatusBadRequest, "extending key is not allowed", nil)
 		}
 
 		// If the request expiry is in the past, we consider it a logout.
@@ -159,6 +159,8 @@ func (h *Headscale) handleRegisterWithAuthKey(
 	regReq tailcfg.RegisterRequest,
 	machineKey key.MachinePublic,
 ) (*tailcfg.RegisterResponse, error) {
+	// TODO(kradalby) Refactor and get the validate away from the database
+	// so we can return nice http errors.
 	pak, err := h.db.ValidatePreAuthKey(regReq.Auth.AuthKey)
 	if err != nil {
 		return nil, fmt.Errorf("invalid pre auth key: %w", err)

hscontrol/handlers.go

@@ -70,12 +70,12 @@ func parseCabailityVersion(req *http.Request) (tailcfg.CapabilityVersion, error)
 	clientCapabilityStr := req.URL.Query().Get("v")
 
 	if clientCapabilityStr == "" {
-		return 0, ErrNoCapabilityVersion
+		return 0, NewHTTPError(http.StatusBadRequest, "capability version must be set", nil)
 	}
 
 	clientCapabilityVersion, err := strconv.Atoi(clientCapabilityStr)
 	if err != nil {
-		return 0, fmt.Errorf("failed to parse capability version: %w", err)
+		return 0, NewHTTPError(http.StatusBadRequest, "invalid capability version", fmt.Errorf("failed to parse capability version: %w", err))
 	}
 
 	return tailcfg.CapabilityVersion(clientCapabilityVersion), nil
@@ -108,13 +108,13 @@ func (h *Headscale) VerifyHandler(
 	req *http.Request,
 ) {
 	if req.Method != http.MethodPost {
-		httpError(writer, nil, "Wrong method", http.StatusMethodNotAllowed)
+		httpError(writer, errMethodNotAllowed)
 		return
 	}
 
 	allow, err := h.derpRequestIsAllowed(req)
 	if err != nil {
-		httpError(writer, err, "Internal error", http.StatusInternalServerError)
+		httpError(writer, err)
 		return
 	}
 
@@ -135,7 +135,7 @@ func (h *Headscale) KeyHandler(
 	// New Tailscale clients send a 'v' parameter to indicate the CurrentCapabilityVersion
 	capVer, err := parseCabailityVersion(req)
 	if err != nil {
-		httpError(writer, err, "Internal error", http.StatusInternalServerError)
+		httpError(writer, err)
 		return
 	}
 
@@ -222,7 +222,7 @@ func (a *AuthProviderWeb) RegisterHandler(
 	// the template and log an error.
 	registrationId, err := types.RegistrationIDFromString(registrationIdStr)
 	if err != nil {
-		httpError(writer, err, "invalid registration ID", http.StatusBadRequest)
+		httpError(writer, NewHTTPError(http.StatusBadRequest, "invalid registration id", err))
 		return
 	}
 

hscontrol/noise.go

@@ -3,6 +3,7 @@ package hscontrol
 import (
 	"encoding/binary"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -12,6 +13,7 @@ import (
 	"github.com/juanfont/headscale/hscontrol/types"
 	"github.com/rs/zerolog/log"
 	"golang.org/x/net/http2"
+	"gorm.io/gorm"
 	"tailscale.com/control/controlbase"
 	"tailscale.com/control/controlhttp/controlhttpserver"
 	"tailscale.com/tailcfg"
@@ -81,7 +83,7 @@ func (h *Headscale) NoiseUpgradeHandler(
 		noiseServer.earlyNoise,
 	)
 	if err != nil {
-		httpError(writer, err, "noise upgrade failed", http.StatusInternalServerError)
+		httpError(writer, fmt.Errorf("noise upgrade failed: %w", err))
 		return
 	}
 
@@ -198,7 +200,7 @@ func (ns *noiseServer) NoisePollNetMapHandler(
 
 	var mapRequest tailcfg.MapRequest
 	if err := json.Unmarshal(body, &mapRequest); err != nil {
-		httpError(writer, err, "Internal error", http.StatusInternalServerError)
+		httpError(writer, err)
 		return
 	}
 
@@ -211,7 +213,11 @@ func (ns *noiseServer) NoisePollNetMapHandler(
 
 	node, err := ns.headscale.db.GetNodeByNodeKey(mapRequest.NodeKey)
 	if err != nil {
-		httpError(writer, err, "Internal error", http.StatusInternalServerError)
+		if errors.Is(err, gorm.ErrRecordNotFound) {
+			httpError(writer, NewHTTPError(http.StatusNotFound, "node not found", nil))
+			return
+		}
+		httpError(writer, err)
 		return
 	}
 
@@ -230,7 +236,7 @@ func (ns *noiseServer) NoiseRegistrationHandler(
 	req *http.Request,
 ) {
 	if req.Method != http.MethodPost {
-		httpError(writer, nil, "Wrong method", http.StatusMethodNotAllowed)
+		httpError(writer, errMethodNotAllowed)
 
 		return
 	}

hscontrol/oidc.go

@@ -141,21 +141,21 @@ func (a *AuthProviderOIDC) RegisterHandler(
 	// the template and log an error.
 	registrationId, err := types.RegistrationIDFromString(registrationIdStr)
 	if err != nil {
-		httpError(writer, err, "invalid registration ID", http.StatusBadRequest)
+		httpError(writer, NewHTTPError(http.StatusBadRequest, "invalid registration id", err))
 		return
 	}
 
 	// Set the state and nonce cookies to protect against CSRF attacks
 	state, err := setCSRFCookie(writer, req, "state")
 	if err != nil {
-		httpError(writer, err, "Internal server error", http.StatusInternalServerError)
+		httpError(writer, err)
 		return
 	}
 
 	// Set the state and nonce cookies to protect against CSRF attacks
 	nonce, err := setCSRFCookie(writer, req, "nonce")
 	if err != nil {
-		httpError(writer, err, "Internal server error", http.StatusInternalServerError)
+		httpError(writer, err)
 		return
 	}
 
@@ -219,64 +219,63 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler(
 ) {
 	code, state, err := extractCodeAndStateParamFromRequest(req)
 	if err != nil {
-		httpError(writer, err, err.Error(), http.StatusBadRequest)
+		httpError(writer, err)
 		return
 	}
 
 	cookieState, err := req.Cookie("state")
 	if err != nil {
-		httpError(writer, err, "state not found", http.StatusBadRequest)
+		httpError(writer, NewHTTPError(http.StatusBadRequest, "state not found", err))
 		return
 	}
 
 	if state != cookieState.Value {
-		httpError(writer, err, "state did not match", http.StatusBadRequest)
+		httpError(writer, NewHTTPError(http.StatusForbidden, "state did not match", nil))
 		return
 	}
 
 	idToken, err := a.extractIDToken(req.Context(), code, state)
 	if err != nil {
-		httpError(writer, err, err.Error(), http.StatusBadRequest)
+		httpError(writer, err)
 		return
 	}
 
 	nonce, err := req.Cookie("nonce")
 	if err != nil {
-		httpError(writer, err, "nonce not found", http.StatusBadRequest)
+		httpError(writer, NewHTTPError(http.StatusBadRequest, "nonce not found", err))
 		return
 	}
 	if idToken.Nonce != nonce.Value {
-		httpError(writer, err, "nonce did not match", http.StatusBadRequest)
+		httpError(writer, NewHTTPError(http.StatusForbidden, "nonce did not match", nil))
 		return
 	}
 
 	nodeExpiry := a.determineNodeExpiry(idToken.Expiry)
 
 	var claims types.OIDCClaims
 	if err := idToken.Claims(&claims); err != nil {
-		err = fmt.Errorf("decoding ID token claims: %w", err)
-		httpError(writer, err, err.Error(), http.StatusInternalServerError)
+		httpError(writer, fmt.Errorf("decoding ID token claims: %w", err))
 		return
 	}
 
 	if err := validateOIDCAllowedDomains(a.cfg.AllowedDomains, &claims); err != nil {
-		httpError(writer, err, err.Error(), http.StatusUnauthorized)
+		httpError(writer, err)
 		return
 	}
 
 	if err := validateOIDCAllowedGroups(a.cfg.AllowedGroups, &claims); err != nil {
-		httpError(writer, err, err.Error(), http.StatusUnauthorized)
+		httpError(writer, err)
 		return
 	}
 
 	if err := validateOIDCAllowedUsers(a.cfg.AllowedUsers, &claims); err != nil {
-		httpError(writer, err, err.Error(), http.StatusUnauthorized)
+		httpError(writer, err)
 		return
 	}
 
 	user, err := a.createOrUpdateUserFromClaim(&claims)
 	if err != nil {
-		httpError(writer, err, err.Error(), http.StatusInternalServerError)
+		httpError(writer, err)
 		return
 	}
 
@@ -289,9 +288,9 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler(
 	// Register the node if it does not exist.
 	if registrationId != nil {
 		verb := "Reauthenticated"
-		newNode, err := a.handleRegistrationID(user, *registrationId, nodeExpiry)
+		newNode, err := a.handleRegistration(user, *registrationId, nodeExpiry)
 		if err != nil {
-			httpError(writer, err, err.Error(), http.StatusInternalServerError)
+			httpError(writer, err)
 			return
 		}
 
@@ -302,7 +301,7 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler(
 		// TODO(kradalby): replace with go-elem
 		content, err := renderOIDCCallbackTemplate(user, verb)
 		if err != nil {
-			httpError(writer, err, err.Error(), http.StatusInternalServerError)
+			httpError(writer, err)
 			return
 		}
 
@@ -317,7 +316,7 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler(
 
 	// Neither node nor machine key was found in the state cache meaning
 	// that we could not reauth nor register the node.
-	httpError(writer, nil, "login session expired, try again", http.StatusInternalServerError)
+	httpError(writer, NewHTTPError(http.StatusGone, "login session expired, try again", nil))
 	return
 }
 
@@ -328,7 +327,7 @@ func extractCodeAndStateParamFromRequest(
 	state := req.URL.Query().Get("state")
 
 	if code == "" || state == "" {
-		return "", "", errEmptyOIDCCallbackParams
+		return "", "", NewHTTPError(http.StatusBadRequest, "missing code or state parameter", errEmptyOIDCCallbackParams)
 	}
 
 	return code, state, nil
@@ -346,7 +345,7 @@ func (a *AuthProviderOIDC) extractIDToken(
 	if a.cfg.PKCE.Enabled {
 		regInfo, ok := a.registrationCache.Get(state)
 		if !ok {
-			return nil, errNoOIDCRegistrationInfo
+			return nil, NewHTTPError(http.StatusNotFound, "registration not found", errNoOIDCRegistrationInfo)
 		}
 		if regInfo.Verifier != nil {
 			exchangeOpts = []oauth2.AuthCodeOption{oauth2.VerifierOption(*regInfo.Verifier)}
@@ -355,18 +354,18 @@ func (a *AuthProviderOIDC) extractIDToken(
 
 	oauth2Token, err := a.oauth2Config.Exchange(ctx, code, exchangeOpts...)
 	if err != nil {
-		return nil, fmt.Errorf("could not exchange code for token: %w", err)
+		return nil, NewHTTPError(http.StatusForbidden, "invalid code", fmt.Errorf("could not exchange code for token: %w", err))
 	}
 
 	rawIDToken, ok := oauth2Token.Extra("id_token").(string)
 	if !ok {
-		return nil, errNoOIDCIDToken
+		return nil, NewHTTPError(http.StatusBadRequest, "no id_token", errNoOIDCIDToken)
 	}
 
 	verifier := a.oidcProvider.Verifier(&oidc.Config{ClientID: a.cfg.ClientID})
 	idToken, err := verifier.Verify(ctx, rawIDToken)
 	if err != nil {
-		return nil, fmt.Errorf("failed to verify ID token: %w", err)
+		return nil, NewHTTPError(http.StatusForbidden, "failed to verify id_token", fmt.Errorf("failed to verify ID token: %w", err))
 	}
 
 	return idToken, nil
@@ -381,7 +380,7 @@ func validateOIDCAllowedDomains(
 	if len(allowedDomains) > 0 {
 		if at := strings.LastIndex(claims.Email, "@"); at < 0 ||
 			!slices.Contains(allowedDomains, claims.Email[at+1:]) {
-			return errOIDCAllowedDomains
+			return NewHTTPError(http.StatusUnauthorized, "unauthorised domain", errOIDCAllowedDomains)
 		}
 	}
 
@@ -403,7 +402,7 @@ func validateOIDCAllowedGroups(
 			}
 		}
 
-		return errOIDCAllowedGroups
+		return NewHTTPError(http.StatusUnauthorized, "unauthorised group", errOIDCAllowedGroups)
 	}
 
 	return nil
@@ -417,7 +416,7 @@ func validateOIDCAllowedUsers(
 ) error {
 	if len(allowedUsers) > 0 &&
 		!slices.Contains(allowedUsers, claims.Email) {
-		return errOIDCAllowedUsers
+		return NewHTTPError(http.StatusUnauthorized, "unauthorised user", errOIDCAllowedUsers)
 	}
 
 	return nil
@@ -488,7 +487,7 @@ func (a *AuthProviderOIDC) createOrUpdateUserFromClaim(
 	return user, nil
 }
 
-func (a *AuthProviderOIDC) handleRegistrationID(
+func (a *AuthProviderOIDC) handleRegistration(
 	user *types.User,
 	registrationID types.RegistrationID,
 	expiry time.Time,

hscontrol/platform_config.go

@@ -39,19 +39,19 @@ func (h *Headscale) ApplePlatformConfig(
 	vars := mux.Vars(req)
 	platform, ok := vars["platform"]
 	if !ok {
-		httpError(writer, nil, "No platform specified", http.StatusBadRequest)
+		httpError(writer, NewHTTPError(http.StatusBadRequest, "no platform specified", nil))
 		return
 	}
 
 	id, err := uuid.NewV4()
 	if err != nil {
-		httpError(writer, nil, "Failed to create UUID", http.StatusInternalServerError)
+		httpError(writer, err)
 		return
 	}
 
 	contentID, err := uuid.NewV4()
 	if err != nil {
-		httpError(writer, nil, "Failed to create UUID", http.StatusInternalServerError)
+		httpError(writer, err)
 		return
 	}
 
@@ -65,21 +65,21 @@ func (h *Headscale) ApplePlatformConfig(
 	switch platform {
 	case "macos-standalone":
 		if err := macosStandaloneTemplate.Execute(&payload, platformConfig); err != nil {
-			httpError(writer, err, "Could not render Apple macOS template", http.StatusInternalServerError)
+			httpError(writer, err)
 			return
 		}
 	case "macos-app-store":
 		if err := macosAppStoreTemplate.Execute(&payload, platformConfig); err != nil {
-			httpError(writer, err, "Could not render Apple macOS template", http.StatusInternalServerError)
+			httpError(writer, err)
 			return
 		}
 	case "ios":
 		if err := iosTemplate.Execute(&payload, platformConfig); err != nil {
-			httpError(writer, err, "Could not render Apple iOS template", http.StatusInternalServerError)
+			httpError(writer, err)
 			return
 		}
 	default:
-		httpError(writer, err, "Invalid platform. Only ios, macos-app-store and macos-standalone are supported", http.StatusInternalServerError)
+		httpError(writer, NewHTTPError(http.StatusBadRequest, "platform must be ios, macos-app-store or macos-standalone", nil))
 		return
 	}
 
@@ -91,7 +91,7 @@ func (h *Headscale) ApplePlatformConfig(
 
 	var content bytes.Buffer
 	if err := commonTemplate.Execute(&content, config); err != nil {
-		httpError(writer, err, "Could not render platform iOS template", http.StatusInternalServerError)
+		httpError(writer, err)
 		return
 	}