[chore] Refactor AP authentication, other small bits of tidying up (#1874)

internal/middleware/signaturecheck.go

@@ -19,95 +19,102 @@ package middleware
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 	"net/url"
 
-	"github.com/superseriousbusiness/gotosocial/internal/ap"
 	"github.com/superseriousbusiness/gotosocial/internal/db"
+	"github.com/superseriousbusiness/gotosocial/internal/gtscontext"
 	"github.com/superseriousbusiness/gotosocial/internal/log"
 
 	"github.com/gin-gonic/gin"
 	"github.com/go-fed/httpsig"
 )
 
-var (
-	// this mimics an untyped error returned by httpsig when no signature is present;
-	// define it here so that we can use it to decide what to log without hitting
-	// performance too hard
-	noSignatureError    = fmt.Sprintf("neither %q nor %q have signature parameters", httpsig.Signature, httpsig.Authorization)
-	signatureHeader     = string(httpsig.Signature)
-	authorizationHeader = string(httpsig.Authorization)
+const (
+	sigHeader  = string(httpsig.Signature)
+	authHeader = string(httpsig.Authorization)
+	// untyped error returned by httpsig when no signature is present
+	noSigError = "neither \"" + sigHeader + "\" nor \"" + authHeader + "\" have signature parameters"
 )
 
 // SignatureCheck returns a gin middleware for checking http signatures.
 //
-// The middleware first checks whether an incoming http request has been http-signed with a well-formed signature.
+// The middleware first checks whether an incoming http request has been
+// http-signed with a well-formed signature. If so, it will check if the
+// domain that signed the request is permitted to access the server, using
+// the provided uriBlocked function. If the domain is blocked, the middleware
+// will abort the request chain with http code 403 forbidden. If it is not
+// blocked, the handler will set the key verifier and the signature in the
+// context for use down the line.
 //
-// If so, it will check if the domain that signed the request is permitted to access the server, using the provided isURIBlocked function.
-//
-// If it is permitted, the handler will set the key verifier and the signature in the gin context for use down the line.
-//
-// If the domain is blocked, the middleware will abort the request chain instead with http code 403 forbidden.
-//
-// In case of an error, the request will be aborted with http code 500 internal server error.
-func SignatureCheck(isURIBlocked func(context.Context, *url.URL) (bool, db.Error)) func(*gin.Context) {
+// In case of an error, the request will be aborted with http code 500.
+func SignatureCheck(uriBlocked func(context.Context, *url.URL) (bool, db.Error)) func(*gin.Context) {
 	return func(c *gin.Context) {
-		// Acquire ctx from gin request.
 		ctx := c.Request.Context()
 
-		// create the verifier from the request, this will error if the request wasn't signed
+		// Create the signature verifier from the request;
+		// this will error if the request wasn't signed.
 		verifier, err := httpsig.NewVerifier(c.Request)
 		if err != nil {
-			// Something went wrong, so we need to return regardless, but only actually
-			// *abort* the request with 401 if a signature was present but malformed
-			if err.Error() != noSignatureError {
+			// Only actually *abort* the request with 401
+			// if a signature was present but malformed.
+			// Otherwise proceed with an unsigned request;
+			// it's up to other functions to reject this.
+			if err.Error() != noSigError {
 				log.Debugf(ctx, "http signature was present but invalid: %s", err)
 				c.AbortWithStatus(http.StatusUnauthorized)
 			}
+
 			return
 		}
 
-		// The request was signed!
-		// The key ID should be given in the signature so that we know where to fetch it from the remote server.
-		// This will be something like https://example.org/users/whatever_requesting_user#main-key
-		requestingPublicKeyIDString := verifier.KeyId()
-		requestingPublicKeyID, err := url.Parse(requestingPublicKeyIDString)
+		// The request was signed! The key ID should be given
+		// in the signature so that we know where to fetch it
+		// from the remote server. This will be something like:
+		// https://example.org/users/some_remote_user#main-key
+		pubKeyIDStr := verifier.KeyId()
+
+		// Key can sometimes be nil, according to url parse
+		// func: 'Trying to parse a hostname and path without
+		// a scheme is invalid but may not necessarily return
+		// an error, due to parsing ambiguities'. Catch this.
+		pubKeyID, err := url.Parse(pubKeyIDStr)
+		if err != nil || pubKeyID == nil {
+			log.Warnf(ctx, "pubkey id %s could not be parsed as a url", pubKeyIDStr)
+			c.AbortWithStatus(http.StatusUnauthorized)
+			return
+		}
+
+		// If the domain is blocked we want to bail as fast as
+		// possible without the request proceeding further.
+		blocked, err := uriBlocked(ctx, pubKeyID)
 		if err != nil {
-			log.Debugf(ctx, "http signature requesting public key id %s could not be parsed as a url: %s", requestingPublicKeyIDString, err)
-			c.AbortWithStatus(http.StatusUnauthorized)
-			return
-		} else if requestingPublicKeyID == nil {
-			// Key can sometimes be nil, according to url parse function:
-			// 'Trying to parse a hostname and path without a scheme is invalid but may not necessarily return an error, due to parsing ambiguities'
-			log.Debugf(ctx, "http signature requesting public key id %s was nil after parsing as a url", requestingPublicKeyIDString)
-			c.AbortWithStatus(http.StatusUnauthorized)
-			return
-		}
-
-		// we managed to parse the url!
-		// if the domain is blocked we want to bail as early as possible
-		if blocked, err := isURIBlocked(c.Request.Context(), requestingPublicKeyID); err != nil {
-			log.Errorf(ctx, "could not tell if domain %s was blocked or not: %s", requestingPublicKeyID.Host, err)
+			log.Errorf(ctx, "error checking block for domain %s: %s", pubKeyID.Host, err)
 			c.AbortWithStatus(http.StatusInternalServerError)
 			return
-		} else if blocked {
-			log.Infof(ctx, "domain %s is blocked", requestingPublicKeyID.Host)
+		}
+
+		if blocked {
+			log.Infof(ctx, "domain %s is blocked", pubKeyID.Host)
 			c.AbortWithStatus(http.StatusForbidden)
 			return
 		}
 
-		// assume signature was set on Signature header (most common behavior),
-		// but fall back to Authorization header if necessary
-		var signature string
-		if s := c.GetHeader(signatureHeader); s != "" {
-			signature = s
-		} else {
-			signature = c.GetHeader(authorizationHeader)
+		// Assume signature was set on Signature header,
+		// but fall back to Authorization header if necessary.
+		signature := c.GetHeader(sigHeader)
+		if signature == "" {
+			signature = c.GetHeader(authHeader)
 		}
 
-		// set the verifier and signature on the context here to save some work further down the line
-		c.Set(string(ap.ContextRequestingPublicKeyVerifier), verifier)
-		c.Set(string(ap.ContextRequestingPublicKeySignature), signature)
+		// Set relevant values on the request context
+		// to save some work further down the line.
+		ctx = gtscontext.SetHTTPSignatureVerifier(ctx, verifier)
+		ctx = gtscontext.SetHTTPSignature(ctx, signature)
+		ctx = gtscontext.SetHTTPSignaturePubKeyID(ctx, pubKeyID)
+
+		// Replace request with a shallow
+		// copy with the new context.
+		c.Request = c.Request.WithContext(ctx)
 	}
 }