fix a few bugs and addressed pr comments

devspace-test
atefeh 2024-05-10 00:48:35 -07:00 committed by Ritik Kapoor
parent afe58fc6cd
commit d404f3f9e9
26 changed files with 137 additions and 125 deletions

View File

@ -17,11 +17,26 @@ package principal
import (
"context"
apiauth "github.com/harness/gitness/app/api/auth"
"github.com/harness/gitness/app/auth"
"github.com/harness/gitness/types"
"github.com/harness/gitness/types/enum"
)
func (c controller) Find(ctx context.Context, session *auth.Session, principalID int64) (*types.PrincipalInfo, error) {
if err := apiauth.Check(
ctx,
c.authorizer,
session,
&types.Scope{},
&types.Resource{
Type: enum.ResourceTypeUser,
},
enum.PermissionUserView,
); err != nil {
return nil, err
}
principal, err := c.principalStore.Find(ctx, principalID)
if err != nil {
return nil, err

View File

@ -17,6 +17,7 @@ package principal
import (
"context"
"github.com/harness/gitness/app/auth"
"github.com/harness/gitness/types"
)
@ -24,6 +25,6 @@ import (
// principal related information.
type Controller interface {
// List lists the principals based on the provided filter.
List(ctx context.Context, opts *types.PrincipalFilter) ([]*types.PrincipalInfo, error)
Find(ctx context.Context, principalID int64) (*types.PrincipalInfo, error)
List(ctx context.Context, session *auth.Session, opts *types.PrincipalFilter) ([]*types.PrincipalInfo, error)
Find(ctx context.Context, session *auth.Session, principalID int64) (*types.PrincipalInfo, error)
}

View File

@ -17,11 +17,27 @@ package principal
import (
"context"
apiauth "github.com/harness/gitness/app/api/auth"
"github.com/harness/gitness/app/auth"
"github.com/harness/gitness/types"
"github.com/harness/gitness/types/enum"
)
func (c controller) List(ctx context.Context, opts *types.PrincipalFilter) (
func (c controller) List(ctx context.Context, session *auth.Session, opts *types.PrincipalFilter) (
[]*types.PrincipalInfo, error) {
if err := apiauth.Check(
ctx,
c.authorizer,
session,
&types.Scope{},
&types.Resource{
Type: enum.ResourceTypeUser,
},
enum.PermissionUserView,
); err != nil {
return nil, err
}
principals, err := c.principalStore.List(ctx, opts)
if err != nil {
return nil, err

View File

@ -74,7 +74,7 @@ func (c *Controller) Update(ctx context.Context,
}
if err = apiauth.CheckRepo(ctx, c.authorizer, session, sourceRepo,
enum.PermissionRepoReview); err != nil {
enum.PermissionRepoView); err != nil {
return nil, fmt.Errorf("failed to acquire access to source repo: %w", err)
}
}

View File

@ -16,6 +16,7 @@ package repo
import (
"context"
"encoding/json"
"fmt"
"strconv"
"strings"
@ -44,8 +45,21 @@ import (
)
type RepositoryOutput struct {
types.Repository `json:",inline"`
IsPublic bool `json:"is_public" yaml:"is_public"`
types.Repository
IsPublic bool `json:"is_public" yaml:"is_public"`
}
// TODO [CODE-1363]: remove after identifier migration.
func (r RepositoryOutput) MarshalJSON() ([]byte, error) {
// alias allows us to embed the original object while avoiding an infinite loop of marshaling.
type alias RepositoryOutput
return json.Marshal(&struct {
alias
UID string `json:"uid"`
}{
alias: (alias)(r),
UID: r.Identifier,
})
}
type Controller struct {

View File

@ -62,7 +62,7 @@ type CreateInput struct {
// Create creates a new repository.
//
//nolint:gocognit
func (c *Controller) Create(ctx context.Context, session *auth.Session, in *CreateInput) (*Repository, error) {
func (c *Controller) Create(ctx context.Context, session *auth.Session, in *CreateInput) (*RepositoryOutput, error) {
if err := c.sanitizeCreateInput(in); err != nil {
return nil, fmt.Errorf("failed to sanitize input: %w", err)
}
@ -119,7 +119,8 @@ func (c *Controller) Create(ctx context.Context, session *auth.Session, in *Crea
return nil, err
}
if err = c.SetRepoPublicAccess(ctx, repo, in.IsPublic); err != nil {
err = c.publicAccess.Set(ctx, enum.PublicResourceTypeRepo, repo.Path, in.IsPublic)
if err != nil {
if dErr := c.PurgeNoAuth(ctx, session, repo); dErr != nil {
log.Ctx(ctx).Warn().Err(dErr).Msg("failed to purge repo for cleanup")
}
@ -129,7 +130,7 @@ func (c *Controller) Create(ctx context.Context, session *auth.Session, in *Crea
// backfil GitURL
repo.GitURL = c.urlProvider.GenerateGITCloneURL(repo.Path)
repoData := &Repository{
repoData := &RepositoryOutput{
Repository: *repo,
IsPublic: in.IsPublic,
}
@ -188,10 +189,6 @@ func (c *Controller) sanitizeCreateInput(in *CreateInput) error {
in.Identifier = in.UID
}
if in.IsPublic && !c.publicResourceCreationEnabled {
return errPublicRepoCreationDisabled
}
if err := c.validateParentRef(in.ParentRef); err != nil {
return err
}
@ -280,18 +277,6 @@ func (c *Controller) createGitRepository(ctx context.Context, session *auth.Sess
return resp, len(files) == 0, nil
}
func (c *Controller) SetRepoPublicAccess(
ctx context.Context,
repo *types.Repository,
isPublic bool,
) error {
if isPublic && !c.publicResourceCreationEnabled {
return errPublicRepoCreationDisabled
}
return c.publicAccess.Set(ctx, enum.PublicResourceTypeRepo, repo.Path, isPublic)
}
func createReadme(name, description string) []byte {
content := bytes.Buffer{}
content.WriteString("# " + name + "\n")

View File

@ -69,6 +69,7 @@ func (c *Controller) Import(ctx context.Context, session *auth.Session, in *Impo
in.Identifier,
in.Description,
&session.Principal,
c.publicResourceCreationEnabled,
)
// lock the space for update during repo creation to prevent racing conditions with space soft delete.
@ -82,14 +83,11 @@ func (c *Controller) Import(ctx context.Context, session *auth.Session, in *Impo
return fmt.Errorf("failed to create repository in storage: %w", err)
}
if err = c.SetRepoPublicAccess(ctx, repo, isPublic); err != nil {
return fmt.Errorf("failed to set repo public access: %w", err)
}
err = c.importer.Run(ctx,
provider,
repo,
remoteRepository.CloneURL,
isPublic,
in.Pipelines,
)
if err != nil {

View File

@ -88,8 +88,9 @@ func (c *Controller) SoftDeleteNoAuth(
return c.PurgeNoAuth(ctx, session, repo)
}
// unset repo public access regardless if it's public/private
if err := c.SetRepoPublicAccess(ctx, repo, false); err != nil {
// unset repo public access regardless if it's public/private for simplicity.
err := c.publicAccess.Set(ctx, enum.PublicResourceTypeRepo, repo.Path, false)
if err != nil {
return fmt.Errorf("failed to disable repo public access: %w", err)
}

View File

@ -29,6 +29,7 @@ import (
"github.com/harness/gitness/types"
"github.com/harness/gitness/types/check"
"github.com/harness/gitness/types/enum"
"github.com/rs/zerolog/log"
)
var (
@ -71,6 +72,14 @@ func (c *Controller) Create(
return nil, err
}
err = c.publicAccess.Set(ctx, enum.PublicResourceTypeSpace, space.Path, in.IsPublic)
if err != nil {
if dErr := c.PurgeNoAuth(ctx, session, space); dErr != nil {
log.Ctx(ctx).Warn().Err(dErr).Msg("failed to set a space public")
}
return nil, fmt.Errorf("failed to set space public access: %w", err)
}
return GetSpaceOutput(ctx, c.publicAccess, space)
}
@ -146,10 +155,6 @@ func (c *Controller) createSpaceInnerInTX(
}
}
if err := c.setSpacePublicAccess(ctx, space, in.IsPublic); err != nil {
return nil, fmt.Errorf("failed to set a space public: %w", err)
}
return space, nil
}
@ -187,14 +192,6 @@ func (c *Controller) getSpaceCheckAuthSpaceCreation(
return parentSpace, nil
}
func (c *Controller) setSpacePublicAccess(
ctx context.Context,
space *types.Space,
isPublic bool,
) error {
return c.publicAccess.Set(ctx, enum.PublicResourceTypeSpace, space.Path, isPublic)
}
func (c *Controller) sanitizeCreateInput(in *CreateInput) error {
// TODO [CODE-1363]: remove after identifier migration.
if in.Identifier == "" {

View File

@ -67,6 +67,7 @@ func (c *Controller) Import(ctx context.Context, session *auth.Session, in *Impo
}
repoIDs := make([]int64, len(remoteRepositories))
repoIsPublicVals := make([]bool, len(remoteRepositories))
cloneURLs := make([]string, len(remoteRepositories))
repos := make([]*types.Repository, 0, len(remoteRepositories))
@ -88,6 +89,7 @@ func (c *Controller) Import(ctx context.Context, session *auth.Session, in *Impo
remoteRepository.Identifier,
"",
&session.Principal,
c.publicResourceCreationEnabled,
)
err = c.repoStore.Create(ctx, repo)
@ -97,11 +99,7 @@ func (c *Controller) Import(ctx context.Context, session *auth.Session, in *Impo
repos = append(repos, repo)
repoIDs[i] = repo.ID
cloneURLs[i] = remoteRepository.CloneURL
if err := c.repoCtrl.SetRepoPublicAccess(ctx, repo, isPublic); err != nil {
return fmt.Errorf("failed to set repo public access: %w", err)
}
repoIsPublicVals[i] = isPublic
}
jobGroupID := fmt.Sprintf("space-import-%d", space.ID)
@ -109,6 +107,7 @@ func (c *Controller) Import(ctx context.Context, session *auth.Session, in *Impo
jobGroupID,
provider,
repoIDs,
repoIsPublicVals,
cloneURLs,
in.Pipelines,
)

View File

@ -93,6 +93,7 @@ func (c *Controller) ImportRepositories(
}
repoIDs := make([]int64, 0, len(remoteRepositories))
repoIsPublicVals := make([]bool, 0, len(remoteRepositories))
cloneURLs := make([]string, 0, len(remoteRepositories))
repos := make([]*types.Repository, 0, len(remoteRepositories))
duplicateRepos := make([]*types.Repository, 0, len(remoteRepositories))
@ -115,6 +116,7 @@ func (c *Controller) ImportRepositories(
remoteRepository.Identifier,
"",
&session.Principal,
c.publicResourceCreationEnabled,
)
err = c.repoStore.Create(ctx, repo)
@ -126,13 +128,10 @@ func (c *Controller) ImportRepositories(
return fmt.Errorf("failed to create repository in storage: %w", err)
}
if err := c.repoCtrl.SetRepoPublicAccess(ctx, repo, isPublic); err != nil {
return fmt.Errorf("failed to set repo public access: %w", err)
}
repos = append(repos, repo)
repoIDs = append(repoIDs, repo.ID)
cloneURLs = append(cloneURLs, remoteRepository.CloneURL)
repoIsPublicVals = append(repoIsPublicVals, isPublic)
}
if len(repoIDs) == 0 {
return nil
@ -143,6 +142,7 @@ func (c *Controller) ImportRepositories(
jobGroupID,
provider,
repoIDs,
repoIsPublicVals,
cloneURLs,
in.Pipelines,
)

View File

@ -48,7 +48,7 @@ func (c *Controller) Purge(
return fmt.Errorf("failed to authorize on space purge: %w", err)
}
return c.PurgeNoAuth(ctx, session, space.ID, deletedAt)
return c.PurgeNoAuth(ctx, session, space)
}
// PurgeNoAuth purges the space - no authorization is verified.
@ -56,8 +56,7 @@ func (c *Controller) Purge(
func (c *Controller) PurgeNoAuth(
ctx context.Context,
session *auth.Session,
spaceID int64,
deletedAt int64,
space *types.Space,
) error {
// the max time we give a purge space to succeed
const timeout = 15 * time.Minute
@ -71,11 +70,11 @@ func (c *Controller) PurgeNoAuth(
var toBeDeletedRepos []*types.Repository
var err error
err = c.tx.WithTx(ctx, func(ctx context.Context) error {
toBeDeletedRepos, err = c.purgeSpaceInnerInTx(ctx, spaceID, deletedAt)
toBeDeletedRepos, err = c.purgeSpaceInnerInTx(ctx, space.ID, *space.Deleted)
return err
})
if err != nil {
return fmt.Errorf("failed to purge space %d in a tnx: %w", spaceID, err)
return fmt.Errorf("failed to purge space %d in a tnx: %w", space.ID, err)
}
// permanently purge all repositories in the space and its subspaces after successful space purge tnx.

View File

@ -26,8 +26,9 @@ func HandleList(principalCtrl principal.Controller) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
session, _ := request.AuthSessionFrom(ctx)
principalFilter := request.ParsePrincipalFilter(r)
principalInfos, err := principalCtrl.List(ctx, principalFilter)
principalInfos, err := principalCtrl.List(ctx, session, principalFilter)
if err != nil {
render.TranslatedUserError(ctx, w, err)
return

View File

@ -31,12 +31,6 @@ func Attempt(authenticator authn.Authenticator) func(http.Handler) http.Handler
return performAuthentication(authenticator, false)
}
// Required returns an http.HandlerFunc middleware that authenticates
// the http.Request and fails the request if no auth data was available.
func Required(authenticator authn.Authenticator) func(http.Handler) http.Handler {
return performAuthentication(authenticator, true)
}
// performAuthentication returns an http.HandlerFunc middleware that authenticates
// the http.Request if authentication payload is available.
// Depending on whether it is required or not, the request will be failed.

View File

@ -114,9 +114,14 @@ func (a *MembershipAuthorizer) Check(
spacePath = scope.SpacePath
case enum.ResourceTypeUser:
// a user is allowed to view / edit themselves
// a user is allowed to edit themselves
if resource.Identifier == session.Principal.UID &&
(permission == enum.PermissionUserView || permission == enum.PermissionUserEdit) {
permission == enum.PermissionUserEdit {
return true, nil
}
// user can see all other users in the system.
if permission == enum.PermissionUserView {
return true, nil
}

View File

@ -28,25 +28,28 @@ func (a *MembershipAuthorizer) CheckPublicAccess(
resource *types.Resource,
permission enum.Permission,
) (bool, error) {
// public access only permits view.
if permission != enum.PermissionRepoView &&
permission != enum.PermissionSpaceView {
var pubResType enum.PublicResourceType
//nolint:exhaustive
switch resource.Type {
case enum.ResourceTypeSpace:
pubResType = enum.PublicResourceTypeSpace
case enum.ResourceTypeRepo:
if resource.Identifier != "" {
pubResType = enum.PublicResourceTypeRepo
} else { // for spaceScope checks
pubResType = enum.PublicResourceTypeSpace
}
case enum.ResourceTypePipeline:
pubResType = enum.PublicResourceTypeRepo
default:
return false, nil
}
pubResType, pubResPath := mapResource(scope, resource)
pubResPath := paths.Concatenate(scope.SpacePath, resource.Identifier)
return a.publicAccess.Get(ctx, pubResType, pubResPath)
}
func mapResource(scope *types.Scope, resource *types.Resource,
) (enum.PublicResourceType, string) {
pubResType := enum.PublicResourceTypeSpace
if resource.Type == enum.ResourceTypeRepo &&
resource.Identifier != "" {
pubResType = enum.PublicResourceTypeRepo
}
return pubResType, paths.Concatenate(scope.SpacePath, resource.Identifier)
}

View File

@ -205,7 +205,7 @@ func setupRoutesV1(r chi.Router,
setupSecrets(r, secretCtrl)
setupUser(r, userCtrl)
setupServiceAccounts(r, saCtrl)
setupPrincipals(r, authenticator, principalCtrl)
setupPrincipals(r, principalCtrl)
setupInternal(r, githookCtrl, git)
setupAdmin(r, userCtrl)
setupAccount(r, userCtrl, sysCtrl, config)
@ -668,9 +668,8 @@ func setupResources(r chi.Router) {
})
}
func setupPrincipals(r chi.Router, authenticator authn.Authenticator, principalCtrl principal.Controller) {
func setupPrincipals(r chi.Router, principalCtrl principal.Controller) {
r.Route("/principals", func(r chi.Router) {
r.Use(middlewareauthn.Required(authenticator))
r.Get("/", handlerprincipal.HandleList(principalCtrl))
r.Get(fmt.Sprintf("/{%s}", request.PathParamPrincipalID), handlerprincipal.HandleFind(principalCtrl))
})

View File

@ -81,6 +81,7 @@ func (r *RepositoryInfo) ToRepo(
identifier string,
description string,
principal *types.Principal,
publicResourceCreationEnabled bool,
) (*types.Repository, bool) {
now := time.Now().UnixMilli()
gitTempUID := fmt.Sprintf("importing-%s-%d", hash(fmt.Sprintf("%d:%s", spaceID, identifier)), now)
@ -96,7 +97,7 @@ func (r *RepositoryInfo) ToRepo(
ForkID: 0,
DefaultBranch: r.DefaultBranch,
Importing: true,
}, r.IsPublic
}, (r.IsPublic && publicResourceCreationEnabled)
}
func hash(s string) string {

View File

@ -16,6 +16,7 @@ package importer
import (
"github.com/harness/gitness/app/services/keywordsearch"
"github.com/harness/gitness/app/services/publicaccess"
"github.com/harness/gitness/app/sse"
"github.com/harness/gitness/app/store"
"github.com/harness/gitness/app/url"
@ -45,6 +46,7 @@ func ProvideRepoImporter(
executor *job.Executor,
sseStreamer sse.Streamer,
indexer keywordsearch.Indexer,
publicAccess publicaccess.Service,
) (*Repository, error) {
importer := &Repository{
defaultBranch: config.Git.DefaultBranch,
@ -58,6 +60,7 @@ func ProvideRepoImporter(
scheduler: scheduler,
sseStreamer: sseStreamer,
indexer: indexer,
publicAccess: publicAccess,
}
err := executor.Register(jobType, importer)

View File

@ -1,26 +1,26 @@
-- copy public repositories
ALTER TABLE repositories ADD COLUMN repo_is_public BOOLEAN;
-- update repo public access
UPDATE repositories
SET repo_is_public = TRUE
WHERE repo_id IN (
SELECT public_access_repo_id
FROM public_access
WHERE public_access_repo_id IS NOT NULL;
) SET
repo_is_public = TRUE;
WHERE public_access_repo_id IS NOT NULL
);
-- copy public spaces
ALTER TABLE spaces ADD COLUMN space_is_public BOOLEAN;
-- update public access
-- update space public access
UPDATE spaces
SET space_is_public = TRUE
WHERE space_id IN (
SELECT public_access_space_id
FROM public_access
WHERE public_access_space_id IS NOT NULL;
) SET
space_is_public = TRUE;
WHERE public_access_space_id IS NOT NULL
);
-- clear public access
DROP INDEX public_access_space_id_key;

View File

@ -1,26 +1,26 @@
-- copy public repositories
ALTER TABLE repositories ADD COLUMN repo_is_public BOOLEAN;
-- update repo public access
UPDATE repositories
SET repo_is_public = TRUE
WHERE repo_id IN (
SELECT public_access_repo_id
FROM public_access
WHERE public_access_repo_id IS NOT NULL;
) SET
repo_is_public = TRUE;
WHERE public_access_repo_id IS NOT NULL
);
-- copy public spaces
ALTER TABLE spaces ADD COLUMN space_is_public BOOLEAN;
-- update public access
-- update space public access
UPDATE spaces
SET space_is_public = TRUE
WHERE space_id IN (
SELECT public_access_space_id
FROM public_access
WHERE public_access_space_id IS NOT NULL;
) SET
space_is_public = TRUE;
WHERE public_access_space_id IS NOT NULL
);
-- clear public_access
DROP INDEX public_access_space_id_key;

View File

@ -95,17 +95,13 @@ func (p *PublicAccessStore) Create(
) error {
stmt := database.Builder.
Insert("").
Into("public_access").
Columns(
"public_access_space_id",
"public_access_repo_id",
)
Into("public_access")
switch typ {
case enum.PublicResourceTypeRepo:
stmt = stmt.Values(null.Int{}, null.IntFrom(id))
stmt = stmt.Columns("public_access_repo_id").Values(null.IntFrom(id))
case enum.PublicResourceTypeSpace:
stmt = stmt.Values(null.IntFrom(id), null.Int{})
stmt = stmt.Columns("public_access_space_id").Values(null.IntFrom(id))
default:
return fmt.Errorf("public resource type %q is not supported", typ)
}

View File

@ -176,7 +176,7 @@ func initSystem(ctx context.Context, config *types.Config) (*server.System, erro
streamer := sse.ProvideEventsStreaming(pubSub)
localIndexSearcher := keywordsearch.ProvideLocalIndexSearcher()
indexer := keywordsearch.ProvideIndexer(localIndexSearcher)
repository, err := importer.ProvideRepoImporter(config, provider, gitInterface, transactor, repoStore, pipelineStore, triggerStore, encrypter, jobScheduler, executor, streamer, indexer)
repository, err := importer.ProvideRepoImporter(config, provider, gitInterface, transactor, repoStore, pipelineStore, triggerStore, encrypter, jobScheduler, executor, streamer, indexer, publicaccessService)
if err != nil {
return nil, err
}

View File

@ -15,8 +15,6 @@
package types
import (
"encoding/json"
"github.com/harness/gitness/types/enum"
)
@ -69,19 +67,6 @@ func (r Repository) Clone() Repository {
return r
}
// TODO [CODE-1363]: remove after identifier migration.
func (r Repository) MarshalJSON() ([]byte, error) {
// alias allows us to embed the original object while avoiding an infinite loop of marshaling.
type alias Repository
return json.Marshal(&struct {
alias
UID string `json:"uid"`
}{
alias: (alias)(r),
UID: r.Identifier,
})
}
type RepositorySizeInfo struct {
ID int64 `json:"id"`
GitUID string `json:"git_uid"`