Hook up authz tuple writing to Minder server (#2179)

This hooks up the initial creation of tuples for authorization in the
server.

Tuples define the relationships between users and projects in minder via
a role.

The intent is for these to happen first, and roll them back if database
operations fail.

cmd/server/app/serve.go

@@ -29,6 +29,7 @@ import (
 	"golang.org/x/sync/errgroup"
 
 	"github.com/stacklok/minder/internal/auth"
+	"github.com/stacklok/minder/internal/authz/mock"
 	serverconfig "github.com/stacklok/minder/internal/config/server"
 	"github.com/stacklok/minder/internal/controlplane"
 	"github.com/stacklok/minder/internal/db"
@@ -104,15 +105,22 @@ var serveCmd = &cobra.Command{
 			return fmt.Errorf("failed to fetch and cache identity provider JWKS: %w\n", err)
 		}
 
-		err = controlplane.SubscribeToIdentityEvents(ctx, store, cfg)
+		// TODO: Change this for an actual authz client
+		authzc := &mock.NoopClient{Authorized: true}
+
+		err = controlplane.SubscribeToIdentityEvents(ctx, store, authzc, cfg)
 		if err != nil {
 			return fmt.Errorf("unable to subscribe to identity server events: %w", err)
 		}
 
 		serverMetrics := controlplane.NewMetrics()
 		providerMetrics := provtelemetry.NewProviderMetrics()
 
-		s, err := controlplane.NewServer(store, evt, serverMetrics, cfg, vldtr, controlplane.WithProviderMetrics(providerMetrics))
+		s, err := controlplane.NewServer(
+			store, evt, serverMetrics, cfg, vldtr,
+			controlplane.WithProviderMetrics(providerMetrics),
+			controlplane.WithAuthzClient(authzc),
+		)
 		if err != nil {
 			return fmt.Errorf("unable to create server: %w", err)
 		}

database/query/projects.sql

@@ -7,6 +7,16 @@ INSERT INTO projects (
     $1, $2, sqlc.arg(metadata)::jsonb
 ) RETURNING *;
 
+-- name: CreateProjectWithID :one
+INSERT INTO projects (
+    id,
+    name,
+    parent_id,
+    metadata
+) VALUES (
+    $1, $2, $3, sqlc.arg(metadata)::jsonb
+) RETURNING *;
+
 -- name: GetRootProjects :many
 SELECT * FROM projects
 WHERE parent_id IS NULL;

internal/controlplane/default_project.go

@@ -21,10 +21,12 @@ import (
 	"fmt"
 
 	"github.com/google/uuid"
+	"github.com/rs/zerolog/log"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/types/known/timestamppb"
 
+	"github.com/stacklok/minder/internal/authz"
 	"github.com/stacklok/minder/internal/db"
 	github "github.com/stacklok/minder/internal/providers/github"
 	pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
@@ -41,8 +43,8 @@ type ProjectMeta struct {
 }
 
 // CreateDefaultRecordsForOrg creates the default records, such as projects, roles and provider for the organization
-func CreateDefaultRecordsForOrg(ctx context.Context, qtx db.Querier,
-	org db.Project, projectName string) (*pb.Project, []int32, error) {
+func (s *Server) CreateDefaultRecordsForOrg(ctx context.Context, qtx db.Querier,
+	org db.Project, projectName string, userSub string) (outproj *pb.Project, outroles []int32, projerr error) {
 	projectmeta := &ProjectMeta{
 		Description: fmt.Sprintf("Default admin project for %s", org.Name),
 	}
@@ -52,12 +54,30 @@ func CreateDefaultRecordsForOrg(ctx context.Context, qtx db.Querier,
 		return nil, nil, status.Errorf(codes.Internal, "failed to marshal meta: %v", err)
 	}
 
+	projectID := uuid.New()
+
+	// Create authorization tuple
+	// NOTE: This is only creating a tuple for the project, not the organization
+	//       We currently have no use for the organization and it might be
+	//       removed in the future.
+	if err := s.authzClient.Write(ctx, userSub, authz.AuthzRoleAdmin, projectID); err != nil {
+		return nil, nil, status.Errorf(codes.Internal, "failed to create authorization tuple: %v", err)
+	}
+	defer func() {
+		if outproj == nil && projerr != nil {
+			if err := s.authzClient.Delete(ctx, userSub, authz.AuthzRoleAdmin, projectID); err != nil {
+				log.Ctx(ctx).Error().Err(err).Msg("failed to delete authorization tuple")
+			}
+		}
+	}()
+
 	// we need to create the default records for the organization
-	project, err := qtx.CreateProject(ctx, db.CreateProjectParams{
+	project, err := qtx.CreateProjectWithID(ctx, db.CreateProjectWithIDParams{
 		ParentID: uuid.NullUUID{
 			UUID:  org.ID,
 			Valid: true,
 		},
+		ID:       projectID,
 		Name:     projectName,
 		Metadata: jsonmeta,
 	})
@@ -93,6 +113,9 @@ func CreateDefaultRecordsForOrg(ctx context.Context, qtx db.Querier,
 	})
 
 	if err != nil {
+		if err := s.authzClient.Delete(ctx, userSub, authz.AuthzRoleAdmin, projectID); err != nil {
+			log.Ctx(ctx).Error().Err(err).Msg("failed to delete authorization tuple")
+		}
 		return nil, nil, status.Errorf(codes.Internal, "failed to create default project role: %v", err)
 	}
 

internal/controlplane/handlers_user.go

@@ -88,7 +88,7 @@ func (s *Server) CreateUser(ctx context.Context,
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to create organization: %s", err)
 	}
-	orgProject, userRoles, err := CreateDefaultRecordsForOrg(ctx, qtx, organization, baseName)
+	orgProject, userRoles, err := s.CreateDefaultRecordsForOrg(ctx, qtx, organization, baseName, subject)
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to create default organization records: %s", err)
 	}
@@ -187,7 +187,7 @@ func (s *Server) DeleteUser(ctx context.Context,
 		return nil, status.Errorf(codes.Internal, "unexpected status code when deleting account: %d", resp.StatusCode)
 	}
 
-	err = DeleteUser(ctx, s.store, subject)
+	err = DeleteUser(ctx, s.store, s.authzClient, subject)
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to delete user from database: %v", err)
 	}

internal/controlplane/handlers_user_test.go

@@ -35,6 +35,7 @@ import (
 	mockdb "github.com/stacklok/minder/database/mock"
 	"github.com/stacklok/minder/internal/auth"
 	mockjwt "github.com/stacklok/minder/internal/auth/mock"
+	"github.com/stacklok/minder/internal/authz/mock"
 	serverconfig "github.com/stacklok/minder/internal/config/server"
 	"github.com/stacklok/minder/internal/crypto"
 	"github.com/stacklok/minder/internal/db"
@@ -78,7 +79,7 @@ func TestCreateUserDBMock(t *testing.T) {
 					CreateOrganization(gomock.Any(), gomock.Any()).
 					Return(db.Project{ID: orgID}, nil)
 				store.EXPECT().
-					CreateProject(gomock.Any(), gomock.Any()).
+					CreateProjectWithID(gomock.Any(), gomock.Any()).
 					Return(db.Project{
 						ID: projectID,
 						ParentID: uuid.NullUUID{
@@ -151,6 +152,7 @@ func TestCreateUserDBMock(t *testing.T) {
 				cfg:          &serverconfig.Config{},
 				cryptoEngine: crypeng,
 				vldtr:        mockJwtValidator,
+				authzClient:  &mock.NoopClient{Authorized: true},
 			}
 
 			resp, err := server.CreateUser(ctx, tc.req)
@@ -183,7 +185,7 @@ func TestCreateUser_gRPC(t *testing.T) {
 					CreateOrganization(gomock.Any(), gomock.Any()).
 					Return(db.Project{ID: orgID}, nil)
 				store.EXPECT().
-					CreateProject(gomock.Any(), gomock.Any()).
+					CreateProjectWithID(gomock.Any(), gomock.Any()).
 					Return(db.Project{
 						ID: projectID,
 						ParentID: uuid.NullUUID{
@@ -339,6 +341,8 @@ func TestDeleteUserDBMock(t *testing.T) {
 	mockStore.EXPECT().
 		DeleteOrganization(gomock.Any(), orgID).
 		Return(nil)
+	mockStore.EXPECT().GetUserProjects(gomock.Any(), gomock.Any()).
+		Return([]db.GetUserProjectsRow{}, nil)
 	mockStore.EXPECT().Commit(gomock.Any())
 	mockStore.EXPECT().Rollback(gomock.Any())
 
@@ -357,6 +361,7 @@ func TestDeleteUserDBMock(t *testing.T) {
 		},
 		vldtr:        mockJwtValidator,
 		cryptoEngine: crypeng,
+		authzClient:  &mock.NoopClient{Authorized: true},
 	}
 
 	response, err := server.DeleteUser(ctx, request)
@@ -393,6 +398,8 @@ func TestDeleteUser_gRPC(t *testing.T) {
 					Return(db.User{
 						OrganizationID: orgID,
 					}, nil)
+				store.EXPECT().GetUserProjects(gomock.Any(), gomock.Any()).
+					Return([]db.GetUserProjectsRow{}, nil)
 				store.EXPECT().
 					DeleteOrganization(gomock.Any(), orgID).
 					Return(nil)

internal/controlplane/identity_events.go

@@ -30,6 +30,7 @@ import (
 	"golang.org/x/oauth2"
 	"golang.org/x/oauth2/clientcredentials"
 
+	"github.com/stacklok/minder/internal/authz"
 	serverconfig "github.com/stacklok/minder/internal/config/server"
 	"github.com/stacklok/minder/internal/db"
 )
@@ -49,10 +50,15 @@ type AccountEvent struct {
 }
 
 // SubscribeToIdentityEvents starts a cron job that periodically fetches events from the identity provider
-func SubscribeToIdentityEvents(ctx context.Context, store db.Store, cfg *serverconfig.Config) error {
+func SubscribeToIdentityEvents(
+	ctx context.Context,
+	store db.Store,
+	authzClient authz.Client,
+	cfg *serverconfig.Config,
+) error {
 	c := cron.New()
 	_, err := c.AddFunc(eventFetchInterval, func() {
-		HandleEvents(ctx, store, cfg)
+		HandleEvents(ctx, store, authzClient, cfg)
 	})
 	if err != nil {
 		return err
@@ -62,7 +68,12 @@ func SubscribeToIdentityEvents(ctx context.Context, store db.Store, cfg *serverc
 }
 
 // HandleEvents fetches events from the identity provider and performs any related changes to the minder database
-func HandleEvents(ctx context.Context, store db.Store, cfg *serverconfig.Config) {
+func HandleEvents(
+	ctx context.Context,
+	store db.Store,
+	authzClient authz.Client,
+	cfg *serverconfig.Config,
+) {
 	d := time.Now().Add(time.Duration(10) * time.Minute)
 	ctx, cancel := context.WithDeadline(ctx, d)
 	defer cancel()
@@ -122,14 +133,14 @@ func HandleEvents(ctx context.Context, store db.Store, cfg *serverconfig.Config)
 	}
 	for _, event := range events {
 		if event.Type == deleteAccountEventType {
-			err := DeleteUser(ctx, store, event.UserId)
+			err := DeleteUser(ctx, store, authzClient, event.UserId)
 			zerolog.Ctx(ctx).Error().Msgf("events chron: error deleting user account: %v", err)
 		}
 	}
 }
 
 // DeleteUser deletes a user and all their associated data from the minder database
-func DeleteUser(ctx context.Context, store db.Store, userId string) error {
+func DeleteUser(ctx context.Context, store db.Store, authzClient authz.Client, userId string) error {
 	tx, err := store.BeginTransaction()
 	if err != nil {
 		return err
@@ -145,6 +156,18 @@ func DeleteUser(ctx context.Context, store db.Store, userId string) error {
 		}
 		return fmt.Errorf("error retrieving user %v", err)
 	}
+
+	projs, err := store.GetUserProjects(ctx, user.ID)
+	if err != nil {
+		return fmt.Errorf("error retrieving user projects %v", err)
+	}
+
+	for _, proj := range projs {
+		if err := authzClient.Delete(ctx, userId, authz.AuthzRoleAdmin, proj.ID); err != nil {
+			return fmt.Errorf("error deleting authorization tuple %v", err)
+		}
+	}
+
 	err = qtx.DeleteOrganization(ctx, user.OrganizationID)
 	if err != nil {
 		return fmt.Errorf("error deleting organization %w", err)

internal/controlplane/identity_events_test.go

@@ -27,6 +27,7 @@ import (
 	"golang.org/x/oauth2"
 
 	mockdb "github.com/stacklok/minder/database/mock"
+	"github.com/stacklok/minder/internal/authz/mock"
 	serverconfig "github.com/stacklok/minder/internal/config/server"
 	"github.com/stacklok/minder/internal/db"
 )
@@ -72,6 +73,8 @@ func TestHandleEvents(t *testing.T) {
 	mockStore.EXPECT().
 		GetUserBySubject(gomock.Any(), "alreadyDeletedUserId").
 		Return(db.User{}, sql.ErrNoRows)
+	mockStore.EXPECT().GetUserProjects(gomock.Any(), gomock.Any()).
+		Return([]db.GetUserProjectsRow{}, nil)
 	mockStore.EXPECT().Rollback(gomock.Any())
 
 	c := serverconfig.Config{
@@ -83,7 +86,7 @@ func TestHandleEvents(t *testing.T) {
 			},
 		},
 	}
-	HandleEvents(context.Background(), mockStore, &c)
+	HandleEvents(context.Background(), mockStore, &mock.NoopClient{Authorized: true}, &c)
 }
 
 func tokenHandler(t *testing.T, w http.ResponseWriter) {

internal/controlplane/server.go

@@ -124,16 +124,14 @@ func NewServer(
 		vldtr:        vldtr,
 		mt:           cpm,
 		provMt:       provtelemetry.NewNoopMetrics(),
+		// TODO: this currently always returns authorized as a transitionary measure.
+		// When OpenFGA is fully rolled out, we may want to make this a hard error or set to false.
+		authzClient: &mock.NoopClient{Authorized: true},
 	}
 
 	for _, opt := range opts {
 		opt(s)
 	}
-	if s.authzClient == nil {
-		// TODO: this currently always returns authorized as a transitionary measure.
-		// When OpenFGA is fully rolled out, we may want to make this a hard error or set to false.
-		s.authzClient = &mock.NoopClient{Authorized: true}
-	}
 
 	return s, nil
 }