From 3ac6d7f7a287b3a4274e31e868cb240a9f5ee843 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20=C4=8Ctvrtka?= <62988319+JiriCtvrtka@users.noreply.github.com> Date: Mon, 1 Apr 2024 12:30:02 +0200 Subject: [PATCH] PMM-12686 Basic/Token auth between server and client. (#2852) * PMM-12686 Authorization between server and client. * PMM-12686 Tests. * PMM-12686 Lint. * PMM-12686 Comment fix. * PMM-12686 New required permissions for Connect endpoint. * PMM-12686 Apply suggestion. * PMM-12686 Add unit test for authenticate method. * PMM-12686 Format. * PMM-12686 Part changes after review. * PMM-12686 Unit tests for auth server. * PMM-12686 Revert unnecessary changes anymore. * PMM-12686 Dynamic names in auth test. * PMM-12686 Skip check for pmm-server agent. * PMM-12686 Better local auth check. * PMM-12686 Local auth check. * PMM-12686 Refactor. * PMM-12686 Refactor. * PMM-12686 Refactor. * PMM-12686 Fix. * Update managed/services/grafana/auth_server.go Co-authored-by: Alex Demidoff * PMM-12686 Revert of some changes. * PMM-12686 Another reverted changes. * PMM-12686 Years in licence. * PMM-12686 Missed parallel in one test case. --------- Co-authored-by: Alex Demidoff --- managed/services/grafana/auth_server.go | 46 ++++++++---- managed/services/grafana/auth_server_test.go | 73 +++++++++++++++++++- 2 files changed, 105 insertions(+), 14 deletions(-) diff --git a/managed/services/grafana/auth_server.go b/managed/services/grafana/auth_server.go index e04ea1937f..2050962b73 100644 --- a/managed/services/grafana/auth_server.go +++ b/managed/services/grafana/auth_server.go @@ -38,10 +38,13 @@ import ( "github.com/percona/pmm/managed/models" ) +const ( + connectionEndpoint = "/agent.Agent/Connect" +) + // rules maps original URL prefix to minimal required role. var rules = map[string]role{ - // TODO https://jira.percona.com/browse/PMM-4420 - "/agent.Agent/Connect": none, + connectionEndpoint: admin, "/inventory.": admin, "/management.": admin, @@ -459,6 +462,17 @@ func nextPrefix(path string) string { return path[:i+1] } +func isLocalAgentConnection(req *http.Request) bool { + ip := strings.Split(req.RemoteAddr, ":")[0] + pmmAgent := req.Header.Get("Pmm-Agent-Id") + path := req.Header.Get("X-Original-Uri") + if ip == "127.0.0.1" && pmmAgent == "pmm-server" && path == connectionEndpoint { + return true + } + + return false +} + // authenticate checks if user has access to a specific path. // It returns user information retrieved during authentication. // Paths which require no Grafana role return zero value for @@ -498,22 +512,30 @@ func (s *AuthServer) authenticate(ctx context.Context, req *http.Request, l *log return nil, nil } - // Get authenticated user from Grafana - authUser, authErr := s.getAuthUser(ctx, req, l) - if authErr != nil { - return nil, authErr + var user *authUser + if isLocalAgentConnection(req) { + user = &authUser{ + role: rules[connectionEndpoint], + userID: 0, + } + } else { + var authErr *authError + // Get authenticated user from Grafana + user, authErr = s.getAuthUser(ctx, req, l) + if authErr != nil { + return nil, authErr + } } + l = l.WithField("role", user.role.String()) - l = l.WithField("role", authUser.role.String()) - - if authUser.role == grafanaAdmin { + if user.role == grafanaAdmin { l.Debugf("Grafana admin, allowing access.") - return authUser, nil + return user, nil } - if minRole <= authUser.role { + if minRole <= user.role { l.Debugf("Minimal required role is %q, granting access.", minRole) - return authUser, nil + return user, nil } l.Warnf("Minimal required role is %q.", minRole) diff --git a/managed/services/grafana/auth_server_test.go b/managed/services/grafana/auth_server_test.go index 15822094b4..0b1bbbe14c 100644 --- a/managed/services/grafana/auth_server_test.go +++ b/managed/services/grafana/auth_server_test.go @@ -31,6 +31,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" "gopkg.in/reform.v1" "gopkg.in/reform.v1/dialects/postgresql" @@ -161,7 +162,6 @@ func TestAuthServerMustSetup(t *testing.T) { func TestAuthServerAuthenticate(t *testing.T) { t.Parallel() - // logrus.SetLevel(logrus.TraceLevel) checker := &mockAwsInstanceChecker{} checker.Test(t) @@ -198,7 +198,7 @@ func TestAuthServerAuthenticate(t *testing.T) { }) for uri, minRole := range map[string]role{ - "/agent.Agent/Connect": none, + "/agent.Agent/Connect": admin, "/inventory.Nodes/ListNodes": admin, "/management.Actions/StartMySQLShowTableStatusAction": viewer, @@ -270,6 +270,75 @@ func TestAuthServerAuthenticate(t *testing.T) { } } +func TestServerClientConnection(t *testing.T) { + t.Parallel() + + checker := &mockAwsInstanceChecker{} + checker.Test(t) + t.Cleanup(func() { checker.AssertExpectations(t) }) + + ctx := context.Background() + c := NewClient("127.0.0.1:3000") + s := NewAuthServer(c, checker, nil) + + t.Run("Basic auth - success", func(t *testing.T) { + t.Parallel() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, connectionEndpoint, nil) + require.NoError(t, err) + req.SetBasicAuth("admin", "admin") + + _, authError := s.authenticate(ctx, req, logrus.WithField("test", t.Name())) + assert.Nil(t, authError) + }) + + t.Run("Basic auth - fail", func(t *testing.T) { + t.Parallel() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, connectionEndpoint, nil) + require.NoError(t, err) + req.SetBasicAuth("admin", "wrong") + + _, authError := s.authenticate(ctx, req, logrus.WithField("test", t.Name())) + assert.Equal(t, codes.Unauthenticated, authError.code) + }) + + t.Run("Token auth - success", func(t *testing.T) { + t.Parallel() + + nodeName := fmt.Sprintf("N1-%d", time.Now().UnixNano()) + headersMD := metadata.New(map[string]string{ + "Authorization": "Basic YWRtaW46YWRtaW4=", + }) + ctx := metadata.NewIncomingContext(context.Background(), headersMD) + _, serviceToken, err := c.CreateServiceAccount(ctx, nodeName, true) + require.NoError(t, err) + defer func() { + warning, err := c.DeleteServiceAccount(ctx, nodeName, true) + require.NoError(t, err) + require.Empty(t, warning) + }() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, connectionEndpoint, nil) + require.NoError(t, err) + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", serviceToken)) + + _, authError := s.authenticate(ctx, req, logrus.WithField("test", t.Name())) + assert.Nil(t, authError) + }) + + t.Run("Token auth - fail", func(t *testing.T) { + t.Parallel() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, connectionEndpoint, nil) + require.NoError(t, err) + req.Header.Set("Authorization", "Bearer wrong") + + _, authError := s.authenticate(ctx, req, logrus.WithField("test", t.Name())) + assert.Equal(t, codes.Unauthenticated, authError.code) + }) +} + func TestAuthServerAddVMGatewayToken(t *testing.T) { ctx := logger.Set(context.Background(), t.Name()) uuid.SetRand(&tests.IDReader{})