Merge pull request #992 from haoming29/verbose-err-msg-dir-reg

More verbose error message at director registration failure
PelicanPlatform · Apr 1, 2024 · 5635a1e · 5635a1e
2 parents 5dd6572 + 1757fbd
commit 5635a1e
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 12 deletions.
diff --git a/director/director.go b/director/director.go
@@ -553,7 +553,7 @@ func registerServeAd(engineCtx context.Context, ctx *gin.Context, sType server_s
 					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": fmt.Sprintf("Authorization token verification failed %v", err)})
 					return
 				}
 			}
@@ -575,7 +575,7 @@ func registerServeAd(engineCtx context.Context, ctx *gin.Context, sType server_s
 				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": fmt.Sprintf("Authorization token verification failed %v", err)})
 				return
 			}
 		}

diff --git a/director/director_test.go b/director/director_test.go
@@ -389,7 +389,7 @@ func TestDirectorRegistration(t *testing.T) {
 
 		assert.Equal(t, http.StatusForbidden, w.Result().StatusCode, "Expected failing status code of 403")
 		body, _ := io.ReadAll(w.Result().Body)
-		assert.Equal(t, `{"error":"Authorization token verification failed"}`, string(body), "Failure wasn't because token verification failed")
+		assert.Contains(t, string(body), "Authorization token verification failed", "Failure wasn't because token verification failed")
 
 		namaspaceADs := listNamespacesFromOrigins()
 		assert.False(t, NamespaceAdContainsPath(namaspaceADs, "/foo/bar"), "Found namespace in the director cache even if the token validation failed.")
@@ -424,7 +424,7 @@ func TestDirectorRegistration(t *testing.T) {
 
 		assert.Equal(t, http.StatusForbidden, w.Result().StatusCode, "Expected failing status code of 403")
 		body, _ := io.ReadAll(w.Result().Body)
-		assert.Equal(t, `{"error":"Authorization token verification failed"}`, string(body), "Failure wasn't because token verification failed")
+		assert.Contains(t, string(body), "Authorization token verification failed", "Failure wasn't because token verification failed")
 
 		namaspaceADs := listNamespacesFromOrigins()
 		assert.False(t, NamespaceAdContainsPath(namaspaceADs, "/foo/bar"), "Found namespace in the director cache even if the token validation failed.")

diff --git a/director/origin_api.go b/director/origin_api.go
@@ -110,12 +110,12 @@ func checkNamespaceStatus(prefix string, registryWebUrlStr string) (bool, error)
 func verifyAdvertiseToken(ctx context.Context, token, namespace string) (bool, error) {
 	issuerUrl, err := server_utils.GetNSIssuerURL(namespace)
 	if err != nil {
-		return false, err
+		return false, errors.Wrap(err, "failed to get issuer for namespace "+namespace)
 	}
 
 	keyLoc, err := server_utils.GetJWKSURLFromIssuerURL(issuerUrl)
 	if err != nil {
-		return false, err
+		return false, errors.Wrap(err, "failed to get JWKS URL from the issuer URL at "+issuerUrl)
 	}
 
 	var ar NamespaceCache
@@ -135,17 +135,17 @@ 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")
+		return false, errors.Wrap(err, "failed to check namespace approval status")
 	}
 	if !approved {
-		adminApprovalErr = errors.New(namespace + " has not been approved by an administrator.")
+		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()}
 		if err = ar.Register(keyLoc, jwk.WithMinRefreshInterval(15*time.Minute), jwk.WithHTTPClient(client)); err != nil {
-			return false, err
+			return false, errors.Wrap(err, fmt.Sprintf("failed to register JWKS URL %s at the JWKS cache", keyLoc))
 		}
 		namespaceKeysMutex.Lock()
 		defer namespaceKeysMutex.Unlock()
@@ -162,7 +162,7 @@ func verifyAdvertiseToken(ctx context.Context, token, namespace string) (bool, e
 	keyset, err := ar.Get(ctx, keyLoc)
 
 	if err != nil {
-		return false, err
+		return false, errors.Wrap(err, "failed to fetch JWKS from JWKS cache for "+keyLoc)
 	}
 
 	tok, err := jwt.Parse([]byte(token), jwt.WithKeySet(keyset), jwt.WithValidate(true))
@@ -172,7 +172,7 @@ func verifyAdvertiseToken(ctx context.Context, token, namespace string) (bool, e
 
 	scope_any, present := tok.Get("scope")
 	if !present {
-		return false, errors.New("No scope is present; required to advertise to director")
+		return false, errors.New("no scope is present; required to advertise to director")
 	}
 	scope, ok := scope_any.(string)
 	if !ok {

diff --git a/director/origin_api_test.go b/director/origin_api_test.go
@@ -137,7 +137,7 @@ func TestVerifyAdvertiseToken(t *testing.T) {
 
 	ok, err = verifyAdvertiseToken(ctx, tok, "/test-namespace")
 	assert.Equal(t, false, ok)
-	assert.Equal(t, "No scope is present; required to advertise to director", err.Error())
+	assert.Equal(t, "no scope is present; required to advertise to director", err.Error())
 
 	// Create a token with a bad scope - should return an error upon validation
 	wrongScopeTokenCfg := token.NewWLCGToken()