Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hook up authz tuple writing to Minder server #2179

Merged
merged 1 commit into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions cmd/server/app/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down
15 changes: 15 additions & 0 deletions database/mock/store.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions database/query/projects.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
29 changes: 26 additions & 3 deletions internal/controlplane/default_project.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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),
}
Expand All @@ -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,
})
Expand Down Expand Up @@ -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)
}

Expand Down
4 changes: 2 additions & 2 deletions internal/controlplane/handlers_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
11 changes: 9 additions & 2 deletions internal/controlplane/handlers_user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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())

Expand All @@ -357,6 +361,7 @@ func TestDeleteUserDBMock(t *testing.T) {
},
vldtr: mockJwtValidator,
cryptoEngine: crypeng,
authzClient: &mock.NoopClient{Authorized: true},
}

response, err := server.DeleteUser(ctx, request)
Expand Down Expand Up @@ -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)
Expand Down
33 changes: 28 additions & 5 deletions internal/controlplane/identity_events.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion internal/controlplane/identity_events_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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{
Expand All @@ -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) {
Expand Down
8 changes: 3 additions & 5 deletions internal/controlplane/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
38 changes: 38 additions & 0 deletions internal/db/projects.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions internal/db/querier.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading