Merge pull request PelicanPlatform#685 from haoming29/reject-random-s…

…erver

Reject unapproved origin/caches based on config and more robust director error response

client/handle_http_test.go

@@ -474,6 +474,8 @@ func TestFullUpload(t *testing.T) {
 	viper.Set("Server.EnableUI", false)
 	viper.Set("Registry.DbLocation", filepath.Join(t.TempDir(), "ns-registry.sqlite"))
 	viper.Set("Xrootd.RunLocation", tmpPath)
+	viper.Set("Registry.RequireOriginApproval", false)
+	viper.Set("Registry.RequireCacheApproval", false)
 
 	err = config.InitServer(ctx, modules)
 	require.NoError(t, err)

cmd/fed_serve_test.go

@@ -83,6 +83,8 @@ func TestFedServePosixOrigin(t *testing.T) {
 	viper.Set("TLSSkipVerify", true)
 	viper.Set("Server.EnableUI", false)
 	viper.Set("Registry.DbLocation", filepath.Join(t.TempDir(), "ns-registry.sqlite"))
+	viper.Set("Registry.RequireOriginApproval", false)
+	viper.Set("Registry.RequireCacheApproval", false)
 
 	err = config.InitServer(ctx, modules)
 	require.NoError(t, err)

cmd/object_get_put_test.go

@@ -99,6 +99,9 @@ func TestGetAndPut(t *testing.T) {
 	err = config.InitServer(ctx, modules)
 	require.NoError(t, err)
 
+	viper.Set("Registry.RequireOriginApproval", false)
+	viper.Set("Registry.RequireCacheApproval", false)
+
 	fedCancel, err := launchers.LaunchModules(ctx, modules)
 	if err != nil {
 		t.Fatalf("Failure in fedServeInternal: %v", err)

config/resources/defaults.yaml

@@ -38,6 +38,8 @@ Origin:
   SelfTest: true
 Registry:
   InstitutionsUrlReloadMinutes: 15m
+  CacheApprovedOnly: false
+  OriginApprovedOnly: false
 Monitoring:
   PortLower: 9930
   PortHigher: 9999

config/resources/osdf.yaml

@@ -22,3 +22,6 @@ Federation:
   DiscoveryUrl: osg-htc.org
   TopologyNamespaceURL: https://topology.opensciencegrid.org/osdf/namespaces
   TopologyReloadInterval: 10
+Registry:
+  CacheApprovedOnly: true
+  OriginApprovedOnly: true

director/origin_api.go

@@ -19,8 +19,11 @@
 package director
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
+	"fmt"
+	"io"
 	"net/http"
 	"net/url"
 	"strings"
@@ -48,6 +51,14 @@ type (
 		EnableWrite        bool          `json:"enablewrite"`
 		EnableFallbackRead bool          `json:"enable-fallback-read"` // True if the origin will allow direct client reads when no caches are available
 	}
+
+	checkStatusReq struct {
+		Prefix string `json:"prefix"`
+	}
+
+	checkStatusRes struct {
+		Approved bool `json:"approved"`
+	}
 )
 
 // Create interface
@@ -64,6 +75,53 @@ var (
 	adminApprovalErr error
 )
 
+func checkNamespaceStatus(prefix string, registryWebUrlStr string) (bool, error) {
+	registryUrl, err := url.Parse(registryWebUrlStr)
+	if err != nil {
+		return false, err
+	}
+	reqUrl := registryUrl.JoinPath("/api/v1.0/registry/checkNamespaceStatus")
+
+	reqBody := checkStatusReq{Prefix: prefix}
+	reqByte, err := json.Marshal(reqBody)
+	if err != nil {
+		return false, err
+	}
+	client := http.Client{Transport: config.GetTransport()}
+	req, err := http.NewRequest("POST", reqUrl.String(), bytes.NewBuffer(reqByte))
+	req.Header.Add("Content-Type", "application/json")
+	if err != nil {
+		return false, err
+	}
+
+	res, err := client.Do(req)
+	if err != nil {
+		return false, err
+	}
+
+	if res.StatusCode != 200 {
+		if res.StatusCode == 404 {
+			// This is when we hit a legacy OSDF registry (or Pelican registry <= 7.4.0) which doesn't have such endpoint
+			log.Warningf("Request %q hit 404, either it's an OSDF registry or Pelican registry <= 7.4.0. Fallback to return true for approval status check", reqUrl.String())
+			return true, nil
+		} else {
+			return false, errors.New(fmt.Sprintf("Server error with status code %d", res.StatusCode))
+		}
+	}
+
+	resBody := checkStatusRes{}
+	bodyByte, err := io.ReadAll(res.Body)
+	if err != nil {
+		return false, err
+	}
+
+	if err := json.Unmarshal(bodyByte, &resBody); err != nil {
+		return false, err
+	}
+
+	return resBody.Approved, nil
+}
+
 func CreateAdvertiseToken(namespace string) (string, error) {
 	// TODO: Need to come back and carefully consider a few naming practices.
 	//       Here, issuerUrl is actually the registry database url, and not
@@ -131,6 +189,15 @@ func VerifyAdvertiseToken(ctx context.Context, token, namespace string) (bool, e
 			}
 		}
 	}()
+	regUrlStr := param.Federation_RegistryUrl.GetString()
+	approved, err := checkNamespaceStatus(namespace, regUrlStr)
+	if err != nil {
+		return false, errors.Wrap(err, "Failed to check namespace approval status")
+	}
+	if !approved {
+		adminApprovalErr = errors.New(namespace + " has not been approved by an administrator.")
+		return false, adminApprovalErr
+	}
 	if ar == nil {
 		ar = jwk.NewCache(ctx)
 		client := &http.Client{Transport: config.GetTransport()}
@@ -151,21 +218,6 @@ func VerifyAdvertiseToken(ctx context.Context, token, namespace string) (bool, e
 	log.Debugln("Attempting to fetch keys from ", issuerUrl)
 	keyset, err := ar.Get(ctx, issuerUrl)
 
-	if log.IsLevelEnabled(log.DebugLevel) {
-		// Let's check that we can convert to JSON and get the right thing...
-		jsonbuf, err := json.Marshal(keyset)
-		if err != nil {
-			return false, errors.Wrap(err, "failed to marshal the public keyset into JWKS JSON")
-		}
-		log.Debugln("Constructed JWKS from fetching jwks:", string(jsonbuf))
-		// This seems never get reached, as registry returns 403 for pending approval namespace
-		// and there will be HTTP error in getting jwks; thus it will always be error
-		if jsonbuf == nil {
-			adminApprovalErr = errors.New(namespace + " has not been approved by an administrator.")
-			return false, adminApprovalErr
-		}
-	}
-
 	if err != nil {
 		return false, err
 	}

director/origin_api_test.go

@@ -2,6 +2,9 @@ package director
 
 import (
 	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
 	"path/filepath"
 	"testing"
 	"time"
@@ -31,18 +34,39 @@ func TestVerifyAdvertiseToken(t *testing.T) {
 	//Setup a private key and a token
 	viper.Set("IssuerKey", kfile)
 
-	viper.Set("Federation.RegistryUrl", "https://get-your-tokens.org")
 	viper.Set("Federation.DirectorURL", "https://director-url.org")
 
 	config.InitConfig()
 	err := config.InitServer(ctx, config.DirectorType)
 	require.NoError(t, err)
+	// Mock registry server
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		if req.Method == "POST" && req.URL.Path == "/api/v1.0/registry/checkNamespaceStatus" {
+			res := checkStatusRes{Approved: true}
+			resByte, err := json.Marshal(res)
+			if err != nil {
+				w.WriteHeader(http.StatusInternalServerError)
+				return
+			}
+			_, err = w.Write(resByte)
+			if err != nil {
+				w.WriteHeader(http.StatusInternalServerError)
+				return
+			}
+			w.WriteHeader(http.StatusOK)
+		} else {
+			w.WriteHeader(http.StatusNotFound)
+		}
+	}))
+	defer ts.Close()
+
+	viper.Set("Federation.RegistryUrl", ts.URL)
 
 	kSet, err := config.GetIssuerPublicJWKS()
 	ar := MockCache{
 		GetFn: func(key string, keyset *jwk.Set) (jwk.Set, error) {
-			if key != "https://get-your-tokens.org/api/v1.0/registry/test-namespace/.well-known/issuer.jwks" {
-				t.Errorf("expecting: https://get-your-tokens.org/api/v1.0/registry/test-namespace/.well-known/issuer.jwks, got %q", key)
+			if key != ts.URL+"/api/v1.0/registry/test-namespace/.well-known/issuer.jwks" {
+				t.Errorf("expecting: %s/api/v1.0/registry/test-namespace/.well-known/issuer.jwks, got %q", ts.URL, key)
 			}
 			return *keyset, nil
 		},

director/redirect.go

@@ -428,14 +428,20 @@ func registerServeAd(engineCtx context.Context, ctx *gin.Context, sType ServerTy
 			token := strings.TrimPrefix(tokens[0], "Bearer ")
 			ok, err := VerifyAdvertiseToken(engineCtx, token, namespace.Path)
 			if err != nil {
-				log.Warningln("Failed to verify token:", err)
-				ctx.JSON(http.StatusForbidden, gin.H{"error": "Authorization token verification failed"})
-				return
+				if err == adminApprovalErr {
+					log.Warningf("Failed to verify advertise token. Namespace %q requires administrator approval", namespace.Path)
+					ctx.JSON(http.StatusForbidden, gin.H{"approval_error": true, "error": fmt.Sprintf("The namespace %q was not approved by an administrator", namespace.Path)})
+					return
+				} else {
+					log.Warningln("Failed to verify token:", err)
+					ctx.JSON(http.StatusForbidden, gin.H{"error": "Authorization token verification failed"})
+					return
+				}
 			}
 			if !ok {
-				log.Warningf("%s %v advertised to namespace %v without valid registration\n",
+				log.Warningf("%s %v advertised to namespace %v without valid token scope\n",
 					sType, ad.Name, namespace.Path)
-				ctx.JSON(http.StatusForbidden, gin.H{"error": sType + " not authorized to advertise to this namespace"})
+				ctx.JSON(http.StatusForbidden, gin.H{"error": "Authorization token verification failed. Token missing required scope"})
 				return
 			}
 		}
@@ -445,18 +451,18 @@ func registerServeAd(engineCtx context.Context, ctx *gin.Context, sType ServerTy
 		ok, err := VerifyAdvertiseToken(engineCtx, token, prefix)
 		if err != nil {
 			if err == adminApprovalErr {
-				log.Warningln("Failed to verify token. Cache was not approved:", err)
-				ctx.JSON(http.StatusForbidden, gin.H{"error": "Cache is not admin approved"})
+				log.Warningf("Failed to verify token. Cache %q was not approved", ad.Name)
+				ctx.JSON(http.StatusForbidden, gin.H{"approval_error": true, "error": fmt.Sprintf("Cache %q was not approved by an administrator", ad.Name)})
+				return
 			} else {
 				log.Warningln("Failed to verify token:", err)
-				ctx.JSON(http.StatusForbidden, gin.H{"error": "Authorization token verification failed"})
+				ctx.JSON(http.StatusForbidden, gin.H{"error": "Authorization token verification failed."})
+				return
 			}
-			return
 		}
 		if !ok {
-			log.Warningf("%s %v advertised to namespace %v without valid registration\n",
-				sType, ad.Name, prefix)
-			ctx.JSON(http.StatusForbidden, gin.H{"error": sType + " not authorized to advertise to this namespace"})
+			log.Warningf("%s %v advertised without valid token scope\n", sType, ad.Name)
+			ctx.JSON(http.StatusForbidden, gin.H{"error": "Authorization token verification failed. Token missing required scope"})
 			return
 		}
 	}
@@ -470,8 +476,8 @@ func registerServeAd(engineCtx context.Context, ctx *gin.Context, sType ServerTy
 
 	adWebUrl, err := url.Parse(ad.WebURL)
 	if err != nil && ad.WebURL != "" { // We allow empty WebURL string for backward compatibility
-		log.Warningf("Failed to parse origin Web URL %v: %v\n", ad.WebURL, err)
-		ctx.JSON(http.StatusBadRequest, gin.H{"error": "Invalid origin Web URL"})
+		log.Warningf("Failed to parse server Web URL %v: %v\n", ad.WebURL, err)
+		ctx.JSON(http.StatusBadRequest, gin.H{"error": "Invalid server Web URL"})
 		return
 	}