feat(sdk): make enforcement of DPoP optional (#617)

* add `auth.enforceDPoP`. enabling this setting tells the auth
middleware to accept tokens that do not have a `cnf` claim as valid.
since having DPoP or not is a property this config goes along with each
issuer
* change the return of `checkToken` to return a context that has the
right stuff in it so that we don't have to validate that something that
came back without an error is non-nil
* make it possible to disable auth on rewrap. currently we can't because
we require there be a dpop JWK

addresses #566

---------

Co-authored-by: Dave Mihalcik <[email protected]>

Author: mkleene

docs/configuration.md

@@ -41,6 +41,7 @@ The server configuration is used to define how the application runs its server.
 | `tls.key` | The path to the tls key. | |
 | `auth.audience` | The audience for the IDP. | |
 | `auth.issuer` | The issuer for the IDP. | |
+| `auth.enforceDPoP` | If false, we allow access tokens that do not have DPoP bindings. | `true` |
 
 Example:
 

service/internal/auth/authn.go

@@ -58,6 +58,7 @@ const refreshInterval = 15 * time.Minute
 
 // Authentication holds a jwks cache and information about the openid configuration
 type Authentication struct {
+	enforceDPoP bool
 	// cache holds the jwks cache
 	cache *jwk.Cache
 	// openidConfigurations holds the openid configuration for each issuer
@@ -70,7 +71,9 @@ type Authentication struct {
 
 // Creates new authN which is used to verify tokens for a set of given issuers
 func NewAuthenticator(ctx context.Context, cfg Config, d *db.Client) (*Authentication, error) {
-	a := &Authentication{}
+	a := &Authentication{
+		enforceDPoP: cfg.EnforceDPoP,
+	}
 	a.oidcConfigurations = make(map[string]AuthNConfig)
 
 	// validate the configuration
@@ -146,7 +149,7 @@ func (a Authentication) MuxHandler(handler http.Handler) http.Handler {
 			http.Error(w, "missing authorization header", http.StatusUnauthorized)
 			return
 		}
-		tok, dpopKey, err := a.checkToken(r.Context(), header, dpopInfo{
+		tok, newCtx, err := a.checkToken(r.Context(), header, dpopInfo{
 			headers: r.Header["Dpop"],
 			path:    r.URL.Path,
 			method:  r.Method,
@@ -184,7 +187,7 @@ func (a Authentication) MuxHandler(handler http.Handler) http.Handler {
 			return
 		}
 
-		handler.ServeHTTP(w, r.WithContext(ContextWithJWK(r.Context(), dpopKey)))
+		handler.ServeHTTP(w, r.WithContext(newCtx))
 	})
 }
 
@@ -225,7 +228,7 @@ func (a Authentication) UnaryServerInterceptor(ctx context.Context, req any, inf
 		action = "other"
 	}
 
-	token, dpopJWK, err := a.checkToken(
+	token, newCtx, err := a.checkToken(
 		ctx,
 		header,
 		dpopInfo{
@@ -251,26 +254,20 @@ func (a Authentication) UnaryServerInterceptor(ctx context.Context, req any, inf
 		return nil, status.Errorf(codes.PermissionDenied, "permission denied")
 	}
 
-	// add dpop key to context
-	ctx = ContextWithJWK(ctx, dpopJWK)
-
-	return handler(ctx, req)
+	return handler(newCtx, req)
 }
 
 // checkToken is a helper function to verify the token.
-func (a Authentication) checkToken(ctx context.Context, authHeader []string, dpopInfo dpopInfo) (jwt.Token, jwk.Key, error) {
+func (a Authentication) checkToken(ctx context.Context, authHeader []string, dpopInfo dpopInfo) (jwt.Token, context.Context, error) {
 	var (
-		tokenRaw  string
-		tokenType string
+		tokenRaw string
 	)
 
 	// If we don't get a DPoP/Bearer token type, we can't proceed
 	switch {
 	case strings.HasPrefix(authHeader[0], "DPoP "):
-		tokenType = "DPoP"
 		tokenRaw = strings.TrimPrefix(authHeader[0], "DPoP ")
 	case strings.HasPrefix(authHeader[0], "Bearer "):
-		tokenType = "Bearer"
 		tokenRaw = strings.TrimPrefix(authHeader[0], "Bearer ")
 	default:
 		return nil, nil, fmt.Errorf("not of type bearer or dpop")
@@ -314,16 +311,17 @@ func (a Authentication) checkToken(ctx context.Context, authHeader []string, dpo
 		return nil, nil, err
 	}
 
-	if tokenType == "Bearer" {
-		slog.Warn("Presented bearer token. validating as DPoP")
+	_, tokenHasCNF := accessToken.Get("cnf")
+	if !tokenHasCNF && a.enforceDPoP {
+		// this condition is not quite tight because it's possible that the `cnf` claim may
+		// come from token introspection
+		return accessToken, ctx, nil
 	}
-
 	key, err := validateDPoP(accessToken, tokenRaw, dpopInfo)
 	if err != nil {
 		return nil, nil, err
 	}
-
-	return accessToken, *key, nil
+	return accessToken, ContextWithJWK(ctx, key), nil
 }
 
 func ContextWithJWK(ctx context.Context, key jwk.Key) context.Context {
@@ -339,10 +337,10 @@ func GetJWKFromContext(ctx context.Context) jwk.Key {
 		return jwk
 	}
 
-	return nil
+	panic("got something that is not a jwk.Key from the JWK context")
 }
 
-func validateDPoP(accessToken jwt.Token, acessTokenRaw string, dpopInfo dpopInfo) (*jwk.Key, error) {
+func validateDPoP(accessToken jwt.Token, acessTokenRaw string, dpopInfo dpopInfo) (jwk.Key, error) {
 	if len(dpopInfo.headers) != 1 {
 		return nil, fmt.Errorf("got %d dpop headers, should have 1", len(dpopInfo.headers))
 	}
@@ -457,7 +455,7 @@ func validateDPoP(accessToken jwt.Token, acessTokenRaw string, dpopInfo dpopInfo
 	if ath != base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(h.Sum(nil)) {
 		return nil, fmt.Errorf("incorrect `ath` claim in DPoP JWT")
 	}
-	return &dpopKey, nil
+	return dpopKey, nil
 }
 
 func (a Authentication) isPublicRoute(path string) func(string) bool {

service/internal/auth/authn_test.go

@@ -144,8 +144,9 @@ func (s *AuthSuite) SetupTest() {
 		context.Background(),
 		Config{
 			AuthNConfig: AuthNConfig{
-				Issuer:   s.server.URL,
-				Audience: "test",
+				EnforceDPoP: false,
+				Issuer:      s.server.URL,
+				Audience:    "test",
 			},
 			PublicRoutes: []string{"/public", "/public2/*", "/public3/static", "/static/*", "/static/*/*"},
 		},
@@ -545,6 +546,33 @@ func makeDPoPToken(t *testing.T, tc dpopTestCase) string {
 	return string(signedToken)
 }
 
+func (s *AuthSuite) Test_Allowing_Auth_With_No_DPoP() {
+	authnConfig := AuthNConfig{
+		EnforceDPoP: true,
+		Issuer:      s.server.URL,
+		Audience:    "test",
+	}
+	config := Config{}
+	config.AuthNConfig = authnConfig
+	auth, err := NewAuthenticator(context.Background(), config, nil)
+
+	s.Require().NoError(err)
+
+	tok := jwt.New()
+	s.Require().NoError(tok.Set(jwt.ExpirationKey, time.Now().Add(time.Hour)))
+	s.Require().NoError(tok.Set("iss", s.server.URL))
+	s.Require().NoError(tok.Set("aud", "test"))
+	s.Require().NoError(tok.Set("client_id", "client1"))
+	signedTok, err := jwt.Sign(tok, jwt.WithKey(jwa.RS256, s.key))
+
+	s.NotNil(signedTok)
+	s.Require().NoError(err)
+
+	_, ctx, err := auth.checkToken(context.Background(), []string{fmt.Sprintf("Bearer %s", string(signedTok))}, dpopInfo{})
+	s.Require().NoError(err)
+	s.Require().Nil(GetJWKFromContext(ctx))
+}
+
 func (s *AuthSuite) Test_PublicPath_Matches() {
 	// Passing routes
 	s.Require().True(slices.ContainsFunc(s.auth.publicRoutes, s.auth.isPublicRoute("/public")))

service/internal/auth/config.go

@@ -11,6 +11,7 @@ type Config struct {
 
 // AuthNConfig is the configuration need for the platform to validate tokens
 type AuthNConfig struct {
+	EnforceDPoP       bool   `yaml:"enforceDPoP" json:"enforceDPoP" default:"true"`
 	Issuer            string `yaml:"issuer" json:"issuer"`
 	Audience          string `yaml:"audience" json:"audience"`
 	OIDCConfiguration `yaml:"-" json:"-"`

service/internal/server/server.go

@@ -90,7 +90,6 @@ func NewOpenTDFServer(config Config, d *db.Client) (*OpenTDFServer, error) {
 	// Add authN interceptor
 	// TODO Remove this conditional once we move to the hardening phase (https://github.com/opentdf/platform/issues/381)
 	if config.Auth.Enabled {
-		slog.Info("authentication enabled")
 		authN, err = auth.NewAuthenticator(
 			context.Background(),
 			config.Auth,
@@ -99,6 +98,9 @@ func NewOpenTDFServer(config Config, d *db.Client) (*OpenTDFServer, error) {
 		if err != nil {
 			return nil, fmt.Errorf("failed to create authentication interceptor: %w", err)
 		}
+		slog.Error("disabling authentication. this is deprecated and will be removed. if you are using an IdP without DPoP set `enforceDPoP = false`")
+	} else {
+		slog.Info("authentication enabled")
 	}
 
 	// Try an register oidc issuer to wellknown service but don't return an error if it fails
@@ -162,6 +164,8 @@ func newHttpServer(c Config, h http.Handler, a *auth.Authentication, g *grpc.Ser
 	// TODO check if this is needed or if it is handled by gRPC
 	if c.Auth.Enabled {
 		h = a.MuxHandler(h)
+	} else {
+		slog.Error("disabling authentication. this is deprecated and will be removed. if you are using an IdP without DPoP set `enforceDPoP = false`")
 	}
 
 	// Add CORS // TODO We need to make cors configurable (https://github.com/opentdf/platform/issues/305)
@@ -218,9 +222,10 @@ func newGrpcServer(c Config, a *auth.Authentication) (*grpc.Server, error) {
 		slog.Warn("failed to create proto validator", slog.String("error", err.Error()))
 	}
 
-	// Add authN interceptor
 	if c.Auth.Enabled {
 		i = append(i, a.UnaryServerInterceptor)
+	} else {
+		slog.Error("disabling authentication. this is deprecated and will be removed. if you are using an IdP without DPoP you can set `enforceDpop = false`")
 	}
 
 	// Add tls creds if tls is not nil

service/kas/access/rewrap.go

@@ -87,23 +87,31 @@ func verifySignedRequestToken(ctx context.Context, in *kaspb.RewrapRequest) (*Re
 	// get dpop public key from context
 	dpopJWK := auth.GetJWKFromContext(ctx)
 
-	// if we don't have a dpop public key then we can't verify the request
+	var token jwt.Token
+	var err error
 	if dpopJWK == nil {
-		slog.ErrorContext(ctx, "missing dpop public key")
-		return nil, err401("dpop public key missing")
-	}
-
-	// verify and validate the request token
-	token, err := jwt.Parse([]byte(in.GetSignedRequestToken()),
-		jwt.WithKey(dpopJWK.Algorithm(), dpopJWK),
-		jwt.WithValidate(true),
-	)
-	// we have failed to verify the signed request token
-	if err != nil {
-		slog.WarnContext(ctx, "unable to verify request token", "err", err)
-		return nil, err401("unable to verify request token")
+		slog.InfoContext(ctx, "no DPoP key provided")
+		// if we have no DPoP key it's for one of two reasons:
+		// 1. auth is disabled so we can't get a DPoP JWK
+		// 2. auth is enabled _but_ we aren't requiring DPoP
+		// in either case letting the request through makes sense
+		token, err = jwt.Parse([]byte(in.GetSignedRequestToken()), jwt.WithValidate(false))
+		if err != nil {
+			slog.WarnContext(ctx, "unable to verify parse token", "err", err)
+			return nil, err401("could not parse token")
+		}
+	} else {
+		// verify and validate the request token
+		token, err = jwt.Parse([]byte(in.GetSignedRequestToken()),
+			jwt.WithKey(dpopJWK.Algorithm(), dpopJWK),
+			jwt.WithValidate(true),
+		)
+		// we have failed to verify the signed request token
+		if err != nil {
+			slog.WarnContext(ctx, "unable to verify request token", "err", err)
+			return nil, err401("unable to verify request token")
+		}
 	}
-
 	rb, exists := token.Get("requestBody")
 	if !exists {
 		slog.WarnContext(ctx, "missing request body")

service/pkg/server/start_test.go

@@ -4,8 +4,8 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"net"
 	"net/http"
+	"net/http/httptest"
 	"os"
 	"testing"
 	"time"
@@ -29,7 +29,7 @@ type TestServiceService interface{}
 type TestService struct{}
 
 func (t TestService) TestHandler(w http.ResponseWriter, r *http.Request, pathParams map[string]string) {
-	_, err := w.Write([]byte("hello " + pathParams["name"] + " from test service!"))
+	_, err := w.Write([]byte("hello from test service!"))
 	if err != nil {
 		panic(err)
 	}
@@ -48,37 +48,52 @@ func ServiceRegistrationTest() serviceregistry.Registration {
 				if !ok {
 					return fmt.Errorf("Surprise! Not a TestService")
 				}
-				return mux.HandlePath(http.MethodGet, "/testpath/{name}", t.TestHandler)
+				return mux.HandlePath(http.MethodGet, "/healthz", t.TestHandler)
 			}
 		},
 	}
 }
 
 func Test_Start_When_Extra_Service_Registered_Expect_Response(t *testing.T) {
-	// Start wiremock
-	wiremock, err := startWireMock()
-	require.NoError(t, err)
-
-	defer func() {
-		err := wiremock.Terminate(context.Background())
-		require.NoError(t, err)
-	}()
+	discoveryURL := "not set yet"
+
+	discoveryEndpoint := httptest.NewServer(
+		http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+			var resp string
+			switch req.URL.Path {
+			case "/.well-known/openid-configuration":
+				resp = `{
+					"issuer":	"https://example.com",
+					"authorization_endpoint":	"https://example.com/oauth2/v1/authorize",
+					"token_endpoint":	"https://example.com/oauth2/v1/token",
+					"userinfo_endpoint": "https://example.com/oauth2/v1/userinfo",
+					"registration_endpoint": "https://example.com/oauth2/v1/clients",
+					"jwks_uri": "` + discoveryURL + `/oauth2/v1/keys"
+				}`
+			case "/oauth2/v1/keys":
+				resp = `{
+					"keys":[{"kty":"RSA","alg":"RS256","kid":"saqvCEEc1QX1kjGRh3sf0o4bdPMiiQBVj9xYz95M-X0","use":"sig","e":"AQAB","n":"yXgJvKqNfKoOoc1KiTg8QYfAO2AA47PjHtqZFsPSh93FI3tobD52t1I9cbD7ZotIYfYmZ6KwDvtrAIMVAPKvqvVUji3xSsNQ_Vv4XRmoWwP1vgJNJxoHOyj7pfDdhjplZZaQEcEEpm_J9rXN6V2lLyL6zYLJr_SlI5JeMc8i0tigFW_yLTUpSQ_85r5fAvkr0VDeUHfonaueaFhF5r-fne-F9EZzAVZvG3P8IG8_K6NEoM6muzsplPWJ-95hheRa3Zh58vYTVHcX8DXd8rpS3laUlLuEmIVs-FlqYrIBKpP2spQYGRvf-P1wpNftMH7OTB4j6ULQjwlNRmiQ34TOhw"}]
+				}`
+			default:
+				w.WriteHeader(http.StatusNotFound)
+				return
+			}
+			_, _ = w.Write([]byte(resp))
+		}),
+	)
 
-	port, err := wiremock.MappedPort(context.Background(), "8184/tcp")
-	require.NoError(t, err)
+	discoveryURL = discoveryEndpoint.URL
 
-	host := net.JoinHostPort("localhost", port.Port())
 	// Create new opentdf server
 	d, _ := db.NewClient(db.Config{})
 	s, err := server.NewOpenTDFServer(server.Config{
 		WellKnownConfigRegister: func(namespace string, config any) error {
 			return nil
 		},
 		Auth: auth.Config{
-			Enabled: true,
 			AuthNConfig: auth.AuthNConfig{
-				Issuer:   fmt.Sprintf("http://%s/auth", host),
-				Audience: "opentdf",
+				Issuer:   discoveryEndpoint.URL,
+				Audience: "test",
 			},
 			PublicRoutes: []string{"/testpath/*"},
 		},
@@ -107,7 +122,7 @@ func Test_Start_When_Extra_Service_Registered_Expect_Response(t *testing.T) {
 	var resp *http.Response
 	// Make request to test service and ensure it registered
 	for i := 3; i > 0; i-- {
-		resp, err = http.Get("http://localhost:43481/testpath/world")
+		resp, err = http.Get("http://localhost:43481/healthz")
 		if err == nil {
 			break
 		}
@@ -122,7 +137,7 @@ func Test_Start_When_Extra_Service_Registered_Expect_Response(t *testing.T) {
 	respBody, err := io.ReadAll(resp.Body)
 
 	require.NoError(t, err)
-	assert.Equal(t, "hello world from test service!", string(respBody))
+	assert.Equal(t, "hello from test service!", string(respBody))
 }
 
 func startWireMock() (tc.Container, error) {