From ed3e18aece4a8ed670ee8e6a06166e83e75a1317 Mon Sep 17 00:00:00 2001 From: James Cotter Date: Mon, 21 Jul 2025 11:17:05 +0100 Subject: [PATCH 1/3] Disallow SAT in clientcredentials middleware --- auth/clientcredentials/connectrpc.go | 4 ++++ auth/clientcredentials/connectrpc_test.go | 10 ++++++++++ auth/clientcredentials/http.go | 8 ++++++++ auth/clientcredentials/http_test.go | 10 ++++++++++ 4 files changed, 32 insertions(+) diff --git a/auth/clientcredentials/connectrpc.go b/auth/clientcredentials/connectrpc.go index 7d559a8..4a4ede7 100644 --- a/auth/clientcredentials/connectrpc.go +++ b/auth/clientcredentials/connectrpc.go @@ -129,6 +129,10 @@ func (i *Interceptor) requireScope(ctx context.Context, headers http.Header, req if err != nil { return nil, connectInternalError(ctx, i.logger, err, "unable to validate token") } + if result.UserID != "" { + return nil, connect.NewError(connect.CodeUnauthenticated, errors.New("SAT token not allowed")) + } + span.SetAttributes( attribute.String("client_id", result.ClientID), attribute.String("token_expires_at", result.ExpiresAt.String()), diff --git a/auth/clientcredentials/connectrpc_test.go b/auth/clientcredentials/connectrpc_test.go index 5348963..6e923ec 100644 --- a/auth/clientcredentials/connectrpc_test.go +++ b/auth/clientcredentials/connectrpc_test.go @@ -88,6 +88,16 @@ func TestInterceptor(t *testing.T) { // client credentials token wantError: autogold.Expect("permission_denied: permission denied"), wantLogs: autogold.Expect([]string{}), + }, { + name: "SAT token with userID rejected", + token: &sams.IntrospectTokenResponse{ + Active: true, + ClientID: "test-client", + UserID: "test-user", + Scopes: scopes.Scopes{"profile"}, + }, + wantError: autogold.Expect("unauthenticated: SAT token not allowed"), + wantLogs: autogold.Expect([]string{}), }} { t.Run(tc.name, func(t *testing.T) { logger, exportLogs := logtest.Captured(t) diff --git a/auth/clientcredentials/http.go b/auth/clientcredentials/http.go index d88ebf9..0c60672 100644 --- a/auth/clientcredentials/http.go +++ b/auth/clientcredentials/http.go @@ -68,6 +68,14 @@ func (a *HTTPAuthenticator) RequireScopes(requiredScopes scopes.Scopes, next htt return } + if result.UserID != "" { + logger.Warn("attempt to authenticate using SAMS token with user ID", + log.String("client", result.ClientID), + log.String("userID", result.UserID)) + http.Error(w, "Forbidden: User tokens not allowed", http.StatusForbidden) + return + } + // Check for our required scope. for _, required := range requiredScopes { if !result.Scopes.Match(required) { diff --git a/auth/clientcredentials/http_test.go b/auth/clientcredentials/http_test.go index b0f0e87..553d21b 100644 --- a/auth/clientcredentials/http_test.go +++ b/auth/clientcredentials/http_test.go @@ -52,6 +52,16 @@ func TestHTTPAuthenticator(t *testing.T) { }, wantError: autogold.Expect("Forbidden: Missing required scope\n"), wantLogs: autogold.Expect([]string{"attempt to authenticate using SAMS token without required scope"}), + }, { + name: "SAT token with userID rejected", + token: &sams.IntrospectTokenResponse{ + Active: true, + ClientID: "test-client", + UserID: "test-user", + Scopes: scopes.Scopes{"profile"}, + }, + wantError: autogold.Expect("Forbidden: User tokens not allowed\n"), + wantLogs: autogold.Expect([]string{"attempt to authenticate using SAMS token with user ID"}), }} { t.Run(tc.name, func(t *testing.T) { logger, exportLogs := logtest.Captured(t) From 8d8ba99664abaf1e627a817b9d9aafeb87f98928 Mon Sep 17 00:00:00 2001 From: James Cotter <35706755+jac@users.noreply.github.com> Date: Mon, 21 Jul 2025 15:04:52 +0100 Subject: [PATCH 2/3] Update auth/clientcredentials/connectrpc.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: ᴊᴏᴇ ᴄʜᴇɴ --- auth/clientcredentials/connectrpc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auth/clientcredentials/connectrpc.go b/auth/clientcredentials/connectrpc.go index 4a4ede7..e2b63f4 100644 --- a/auth/clientcredentials/connectrpc.go +++ b/auth/clientcredentials/connectrpc.go @@ -130,7 +130,7 @@ func (i *Interceptor) requireScope(ctx context.Context, headers http.Header, req return nil, connectInternalError(ctx, i.logger, err, "unable to validate token") } if result.UserID != "" { - return nil, connect.NewError(connect.CodeUnauthenticated, errors.New("SAT token not allowed")) + return nil, connect.NewError(connect.CodeUnauthenticated, errors.New("a user-scoped token is not allowed")) } span.SetAttributes( From fc7a61c04d6a1972f05b18b554a43e57e7b30101 Mon Sep 17 00:00:00 2001 From: James Cotter Date: Mon, 21 Jul 2025 15:58:25 +0100 Subject: [PATCH 3/3] fix test --- auth/clientcredentials/connectrpc_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auth/clientcredentials/connectrpc_test.go b/auth/clientcredentials/connectrpc_test.go index 6e923ec..612be2f 100644 --- a/auth/clientcredentials/connectrpc_test.go +++ b/auth/clientcredentials/connectrpc_test.go @@ -96,7 +96,7 @@ func TestInterceptor(t *testing.T) { UserID: "test-user", Scopes: scopes.Scopes{"profile"}, }, - wantError: autogold.Expect("unauthenticated: SAT token not allowed"), + wantError: autogold.Expect("unauthenticated: a user-scoped token is not allowed"), wantLogs: autogold.Expect([]string{}), }} { t.Run(tc.name, func(t *testing.T) {