Merge pull request #583 from jhiemstrawisc/issue-111

Update ns key discovery to follow openid-style lookups

Author: jhiemstrawisc

director/origin_api.go

@@ -34,12 +34,13 @@ import (
 	"github.com/lestrrat-go/jwx/v2/jwa"
 	"github.com/lestrrat-go/jwx/v2/jwk"
 	"github.com/lestrrat-go/jwx/v2/jwt"
+	"github.com/pkg/errors"
+	log "github.com/sirupsen/logrus"
+
 	"github.com/pelicanplatform/pelican/config"
 	"github.com/pelicanplatform/pelican/param"
 	"github.com/pelicanplatform/pelican/token_scopes"
 	"github.com/pelicanplatform/pelican/utils"
-	"github.com/pkg/errors"
-	log "github.com/sirupsen/logrus"
 )
 
 type (
@@ -126,7 +127,7 @@ 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
 	//       the token issuer url for this namespace
-	issuerUrl, err := GetRegistryIssuerURL(namespace)
+	issuerUrl, err := GetNSIssuerURL(namespace)
 	if err != nil {
 		return "", err
 	}
@@ -170,7 +171,12 @@ func CreateAdvertiseToken(namespace string) (string, error) {
 // see if the entity is authorized to advertise an origin for the
 // namespace
 func VerifyAdvertiseToken(ctx context.Context, token, namespace string) (bool, error) {
-	issuerUrl, err := GetRegistryIssuerURL(namespace)
+	issuerUrl, err := GetNSIssuerURL(namespace)
+	if err != nil {
+		return false, err
+	}
+
+	keyLoc, err := GetJWKSURLFromIssuerURL(issuerUrl)
 	if err != nil {
 		return false, err
 	}
@@ -201,7 +207,7 @@ func VerifyAdvertiseToken(ctx context.Context, token, namespace string) (bool, e
 	if ar == nil {
 		ar = jwk.NewCache(ctx)
 		client := &http.Client{Transport: config.GetTransport()}
-		if err = ar.Register(issuerUrl, jwk.WithMinRefreshInterval(15*time.Minute), jwk.WithHTTPClient(client)); err != nil {
+		if err = ar.Register(keyLoc, jwk.WithMinRefreshInterval(15*time.Minute), jwk.WithHTTPClient(client)); err != nil {
 			return false, err
 		}
 		namespaceKeysMutex.Lock()
@@ -215,8 +221,8 @@ func VerifyAdvertiseToken(ctx context.Context, token, namespace string) (bool, e
 		}
 
 	}
-	log.Debugln("Attempting to fetch keys from ", issuerUrl)
-	keyset, err := ar.Get(ctx, issuerUrl)
+	log.Debugln("Attempting to fetch keys from ", keyLoc)
+	keyset, err := ar.Get(ctx, keyLoc)
 
 	if err != nil {
 		return false, err
@@ -323,18 +329,73 @@ func VerifyDirectorTestReportToken(strToken string) (bool, error) {
 	return false, nil
 }
 
-func GetRegistryIssuerURL(prefix string) (string, error) {
-	namespace_url_string := param.Federation_RegistryUrl.GetString()
-	if namespace_url_string == "" {
-		return "", errors.New("Namespace URL is not set")
+// For a given prefix, get the prefix's issuer URL, where we consider that the openid endpoint
+// we use to look up a key location. Note that this is NOT the same as the issuer key -- to
+// find that, follow openid-style discovery using the issuer URL as a base.
+func GetNSIssuerURL(prefix string) (string, error) {
+	if prefix == "" || !strings.HasPrefix(prefix, "/") {
+		return "", errors.New(fmt.Sprintf("the prefix \"%s\" is invalid", prefix))
+	}
+	registryUrlStr := param.Federation_RegistryUrl.GetString()
+	if registryUrlStr == "" {
+		return "", errors.New("federation registry URL is not set and was not discovered")
 	}
-	namespace_url, err := url.Parse(namespace_url_string)
+	registryUrl, err := url.Parse(registryUrlStr)
 	if err != nil {
 		return "", err
 	}
-	namespace_url.Path, err = url.JoinPath(namespace_url.Path, "api", "v1.0", "registry", prefix, ".well-known", "issuer.jwks")
+
+	registryUrl.Path, err = url.JoinPath(registryUrl.Path, "api", "v1.0", "registry", prefix)
+
 	if err != nil {
-		return "", err
+		return "", errors.Wrapf(err, "failed to construct openid-configuration lookup URL for prefix %s", prefix)
+	}
+	return registryUrl.String(), nil
+}
+
+// Given an issuer url, lookup the JWKS URL from the openid-configuration
+// For example, if the issuer URL is https://registry.com:8446/api/v1.0/registry/test-namespace,
+// this function will return the key indicated by the openid-configuration JSON hosted at
+// https://registry.com:8446/api/v1.0/registry/test-namespace/.well-known/openid-configuration.
+func GetJWKSURLFromIssuerURL(issuerUrl string) (string, error) {
+	// Get/parse the openid-configuration JSON to lookup key location
+	issOpenIDUrl, err := url.Parse(issuerUrl)
+	if err != nil {
+		return "", errors.Wrap(err, "failed to parse issuer URL")
+	}
+	issOpenIDUrl.Path, _ = url.JoinPath(issOpenIDUrl.Path, ".well-known", "openid-configuration")
+
+	client := &http.Client{Transport: config.GetTransport()}
+	openIDCfg, err := client.Get(issOpenIDUrl.String())
+	if err != nil {
+		return "", errors.Wrapf(err, "failed to lookup openid-configuration for issuer %s", issuerUrl)
+	}
+	defer openIDCfg.Body.Close()
+
+	// If we hit an old registry, it may not have the openid-configuration. In that case, we fallback to the old
+	// behavior of looking for the key directly at the issuer URL.
+	if openIDCfg.StatusCode == http.StatusNotFound {
+		oldKeyLoc, err := url.JoinPath(issuerUrl, ".well-known", "issuer.jwks")
+		if err != nil {
+			return "", errors.Wrapf(err, "failed to construct key lookup URL for issuer %s", issuerUrl)
+		}
+		return oldKeyLoc, nil
+	}
+
+	body, err := io.ReadAll(openIDCfg.Body)
+	if err != nil {
+		return "", errors.Wrapf(err, "failed to read response body from %s", issuerUrl)
+	}
+
+	var openIDCfgMap map[string]string
+	err = json.Unmarshal(body, &openIDCfgMap)
+	if err != nil {
+		return "", errors.Wrapf(err, "failed to unmarshal openid-configuration for issuer %s", issuerUrl)
+	}
+
+	if keyLoc, ok := openIDCfgMap["jwks_uri"]; ok {
+		return keyLoc, nil
+	} else {
+		return "", errors.New(fmt.Sprintf("no key found in openid-configuration for issuer %s", issuerUrl))
 	}
-	return namespace_url.String(), nil
 }

director/origin_api_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"path/filepath"
 	"testing"
 	"time"
@@ -13,14 +14,35 @@ import (
 	"github.com/lestrrat-go/jwx/v2/jwa"
 	"github.com/lestrrat-go/jwx/v2/jwk"
 	"github.com/lestrrat-go/jwx/v2/jwt"
-	"github.com/pelicanplatform/pelican/config"
-	"github.com/pelicanplatform/pelican/test_utils"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/sync/errgroup"
+
+	"github.com/pelicanplatform/pelican/config"
+	"github.com/pelicanplatform/pelican/test_utils"
 )
 
+// For these tests, we only need to lookup key locations. Create a dummy registry that only returns
+// the jwks_uri location for the given key. Once a server is instantiated, it will only return
+// locations for the provided prefix. To change prefixes, create a new registry mockup.
+func registryMockup(t *testing.T, prefix string) *httptest.Server {
+	registryUrl, _ := url.Parse("https://registry.com:8446")
+	path, err := url.JoinPath("/api/v1.0/registry", prefix, ".well-known/issuer.jwks")
+	if err != nil {
+		t.Fatalf("Failed to parse key path for prefix %s", prefix)
+	}
+	registryUrl.Path = path
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		jsonResponse := `{"jwks_uri": "` + registryUrl.String() + `"}`
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte(jsonResponse))
+	}))
+	return server
+}
+
 func TestVerifyAdvertiseToken(t *testing.T) {
 	ctx, cancel, egrp := test_utils.TestContext(context.Background(), t)
 	defer func() { require.NoError(t, egrp.Wait()) }()
@@ -160,11 +182,14 @@ func TestCreateAdvertiseToken(t *testing.T) {
 	viper.Set("Federation.RegistryUrl", "")
 	viper.Set("Federation.DirectorURL", "")
 
-	// Test without a namsepace set and check to see if it returns the expected error
+	registry := registryMockup(t, "/test-namespace")
+	defer registry.Close()
+
+	// Test without a registry URL set and check to see if it returns the expected error
 	tok, err := CreateAdvertiseToken("/test-namespace")
 	assert.Equal(t, "", tok)
-	assert.Equal(t, "Namespace URL is not set", err.Error())
-	viper.Set("Federation.RegistryUrl", "https://get-your-tokens.org")
+	assert.Equal(t, "federation registry URL is not set and was not discovered", err.Error())
+	viper.Set("Federation.RegistryUrl", registry.URL)
 
 	// Test without a DirectorURL set and check to see if it returns the expected error
 	tok, err = CreateAdvertiseToken("/test-namespace")
@@ -178,23 +203,28 @@ func TestCreateAdvertiseToken(t *testing.T) {
 	assert.NotEqual(t, "", tok)
 }
 
-func TestGetRegistryIssuerURL(t *testing.T) {
-	/*
-	* Runs unit tests on the GetRegistryIssuerURL function
-	 */
+func TestGetNSIssuerURL(t *testing.T) {
 	viper.Reset()
+	viper.Set("Federation.RegistryUrl", "https://registry.com:8446")
+	url, err := GetNSIssuerURL("/test-prefix")
+	assert.Equal(t, nil, err)
+	assert.Equal(t, "https://registry.com:8446/api/v1.0/registry/test-prefix", url)
+	viper.Reset()
+}
 
-	// No namespace url has been set, so an error is expected
-	url, err := GetRegistryIssuerURL("")
-	assert.Equal(t, "", url)
-	assert.Equal(t, "Namespace URL is not set", err.Error())
-
-	// Test to make sure the path is as expected
-	viper.Set("Federation.RegistryUrl", "test-path")
-	url, err = GetRegistryIssuerURL("test-prefix")
+func TestGetJWKSURLFromIssuerURL(t *testing.T) {
+	viper.Reset()
+	registry := registryMockup(t, "/test-prefix")
+	defer registry.Close()
+	viper.Set("Federation.RegistryUrl", registry.URL)
+	expectedIssuerUrl := registry.URL + "/api/v1.0/registry/test-prefix"
+	url, err := GetNSIssuerURL("/test-prefix")
 	assert.Equal(t, nil, err)
-	assert.Equal(t, "test-path/api/v1.0/registry/test-prefix/.well-known/issuer.jwks", url)
+	assert.Equal(t, expectedIssuerUrl, url)
 
+	keyLoc, err := GetJWKSURLFromIssuerURL(url)
+	assert.Equal(t, nil, err)
+	assert.Equal(t, "https://registry.com:8446/api/v1.0/registry/test-prefix/.well-known/issuer.jwks", keyLoc)
 }
 
 func TestNamespaceKeysCacheEviction(t *testing.T) {

director/redirect_test.go

@@ -21,12 +21,13 @@ import (
 	"github.com/lestrrat-go/jwx/v2/jwa"
 	"github.com/lestrrat-go/jwx/v2/jwk"
 	"github.com/lestrrat-go/jwx/v2/jwt"
-	"github.com/pelicanplatform/pelican/config"
-	"github.com/pelicanplatform/pelican/test_utils"
-	"github.com/pelicanplatform/pelican/token_scopes"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/pelicanplatform/pelican/config"
+	"github.com/pelicanplatform/pelican/test_utils"
+	"github.com/pelicanplatform/pelican/token_scopes"
 )
 
 type MockCache struct {

registry/client_commands.go

@@ -233,9 +233,9 @@ func NamespaceDelete(endpoint string, prefix string) error {
 	// TODO: We might consider moving widely-useful functions like `GetRegistryIssuerURL`
 	//       to a more generic `pelican/utils` package so that they're easier to find
 	//       and more likely to be used.
-	issuerURL, err := director.GetRegistryIssuerURL(prefix)
+	issuerURL, err := director.GetNSIssuerURL(prefix)
 	if err != nil {
-		return errors.Wrap(err, "Failed to determine issuer URL for creating deletion token")
+		return errors.Wrap(err, "Failed to determine prefix's issuer/pubkey URL for creating deletion token")
 	}
 
 	// TODO: Eventually we should think about a naming scheme for

registry/registry.go

@@ -105,6 +105,13 @@ type checkStatusRes struct {
 }
 
 // Various auxiliary functions used for client-server security handshakes
+type NamespaceConfig struct {
+	JwksUri string `json:"jwks_uri"`
+}
+
+/*
+Various auxiliary functions used for client-server security handshakes
+*/
 type registrationData struct {
 	ClientNonce     string `json:"client_nonce"`
 	ClientPayload   string `json:"client_payload"`
@@ -697,10 +704,44 @@ func wildcardHandler(ctx *gin.Context) {
 		}
 		ctx.JSON(http.StatusOK, jwks)
 		return
-	}
+	} else if strings.HasSuffix(path, "/.well-known/openid-configuration") {
+		// Check that the namespace exists before constructing config JSON
+		prefix := strings.TrimSuffix(path, "/.well-known/openid-configuration")
+		exists, err := namespaceExists(prefix)
+		if err != nil {
+			ctx.JSON(http.StatusInternalServerError, gin.H{"error": "Server encountered an error while checking if the prefix exists"})
+			log.Errorf("Error while checking for existence of prefix %s: %v", prefix, err)
+			return
+		}
+		if !exists {
+			ctx.JSON(http.StatusNotFound, gin.H{"error": fmt.Sprintf("The requested prefix %s does not exist in the registry's database", prefix)})
+		}
+		// Construct the openid-configuration JSON and return to the requester
+		// For a given namespace "foo", the jwks should be located at <registry url>/api/v1.0/registry/foo/.well-known/issuer.jwks
+		configUrl, err := url.Parse(param.Server_ExternalWebUrl.GetString())
+		if err != nil {
+			log.Errorf("Failed to parse configured external web URL while constructing namespace jwks location: %v", err)
+			return
+		}
 
-	// No match found, return 404
-	ctx.String(http.StatusNotFound, "404 Not Found")
+		path := strings.TrimSuffix(path, "/openid-configuration")
+		configUrl.Path, err = url.JoinPath("api", "v1.0", "registry", path, "issuer.jwks")
+		if err != nil {
+			log.Errorf("Failed to construct namespace jwks URL: %v", err)
+			return
+		}
+
+		nsCfg := NamespaceConfig{
+			JwksUri: configUrl.String(),
+		}
+
+		ctx.JSON(http.StatusOK, nsCfg)
+		return
+	} else {
+
+		ctx.String(http.StatusNotFound, "404 Page not found")
+		return
+	}
 }
 
 // Check if a namespace prefix exists and its public key matches the registry record