From 3ba0f75c8d53187a1a613cb3acbe1cf10b91616d Mon Sep 17 00:00:00 2001 From: Johannes Batzill Date: Sun, 6 Nov 2022 23:14:47 -0800 Subject: [PATCH] Introduce UIDs for Space / Repo / Tokens, Add Custom Harness Validation, ... (#57) This change adds the following: - Space UID + Custom harness validation (accountId for top level space, harness identifier for child spaces) - Repo UID + Custom harness validation (harness identifier) - Store Unique casing of space / repo path and add Path.ValueUnique (with Unique index) to allow for application layer controlling the case sensitivity (case insensitive standalone vs partially case sensitive harness) - Token UID (unique index over ownertype + ownerID + tokenUID) - Add DisplayName for principals (replaces Name to avoid confustion) - Store Unique casing of principal UID and add Principal.ValueUnique (with unique index) to allow for application layer, per principal type control of case sensitivity (required in embedded mode) - Generate serviceAccount UID (+Email) Randomly (sa-{space|repo}-{ID}-{random}) - Allows to have a unique UID across all principals while reducing likelyhood of overlaps with users + avoid overlap across spaces / repos. - Sync casing of space names (accountId orgId projectId) when creating spaces on the fly (to ensure case sensitivity of - harness code) or use the existing space to update casing. - Update serviceaccount client to match updated NG Manager API - in embedded mode create spaces for harness resources owning the service account --- .harness.env | 5 +- .local.env | 2 +- Makefile | 2 +- README.md | 2 +- cli/operations/user/create_pat.go | 15 +- cli/operations/user/self.go | 2 +- cli/server/harness.wire.go | 8 +- cli/server/harness.wire_gen.go | 38 +- cli/server/standalone.wire.go | 6 +- cli/server/standalone.wire_gen.go | 25 +- client/client.go | 31 +- gitrpc/internal/service/repo.go | 8 +- gitrpc/repo.go | 7 +- internal/api/auth/auth.go | 2 +- internal/api/auth/repo.go | 2 +- internal/api/auth/space.go | 2 +- internal/api/controller/repo/controller.go | 4 + internal/api/controller/repo/create.go | 22 +- internal/api/controller/repo/create_path.go | 3 +- internal/api/controller/repo/move.go | 43 +- internal/api/controller/repo/update.go | 7 +- internal/api/controller/repo/wire.go | 8 +- internal/api/controller/service/controller.go | 6 +- internal/api/controller/service/create.go | 21 +- internal/api/controller/service/update.go | 13 +- internal/api/controller/service/wire.go | 6 +- .../controller/serviceaccount/controller.go | 28 +- .../api/controller/serviceaccount/create.go | 85 +++- .../controller/serviceaccount/create_token.go | 9 +- .../controller/serviceaccount/delete_token.go | 6 +- .../api/controller/serviceaccount/find.go | 10 +- .../api/controller/serviceaccount/wire.go | 8 +- internal/api/controller/space/controller.go | 5 +- internal/api/controller/space/create.go | 15 +- internal/api/controller/space/create_path.go | 3 +- internal/api/controller/space/helper.go | 2 +- internal/api/controller/space/move.go | 25 +- internal/api/controller/space/update.go | 7 +- internal/api/controller/space/wire.go | 5 +- internal/api/controller/user/controller.go | 5 +- internal/api/controller/user/create.go | 26 +- .../controller/user/create_access_token.go | 9 +- internal/api/controller/user/delete_token.go | 6 +- internal/api/controller/user/login.go | 19 +- internal/api/controller/user/update.go | 13 +- internal/api/controller/user/wire.go | 5 +- internal/api/handler/account/register.go | 8 +- .../handler/serviceaccount/delete_token.go | 4 +- internal/api/handler/user/delete_token.go | 4 +- internal/api/openapi/space.go | 10 +- internal/api/openapi/user.go | 3 +- internal/api/openapi/users.go | 11 +- internal/api/request/repo.go | 6 +- internal/api/request/space.go | 4 +- internal/api/request/token.go | 6 +- internal/api/usererror/usererror.go | 2 +- internal/bootstrap/bootstrap.go | 10 +- internal/paths/paths.go | 21 +- internal/router/api.go | 6 +- internal/router/git.go | 4 +- .../postgres/0001_create_table_paths.up.sql | 3 +- .../0001_create_table_principals.up.sql | 9 +- .../0001_create_table_repositories.up.sql | 5 +- .../postgres/0001_create_table_spaces.up.sql | 3 +- .../postgres/0001_create_table_tokens.up.sql | 3 +- ..._create_index_repositories_parentId.up.sql | 2 + ...2_create_index_repositories_spaceId.up.sql | 2 - .../sqlite/0001_create_table_paths.up.sql | 5 +- .../0001_create_table_principals.up.sql | 13 +- .../0001_create_table_repositories.up.sql | 5 +- .../sqlite/0001_create_table_spaces.up.sql | 3 +- .../sqlite/0001_create_table_tokens.up.sql | 3 +- ...02_create_index_repositories_parent.up.sql | 2 + ...2_create_index_repositories_spaceId.up.sql | 2 - internal/store/database/path.go | 237 ++++++++--- internal/store/database/repo.go | 101 ++--- internal/store/database/repo_sync.go | 12 +- internal/store/database/service.go | 115 ++++-- internal/store/database/service_account.go | 106 ++++- internal/store/database/space.go | 114 +++--- internal/store/database/space_sync.go | 12 +- internal/store/database/testdata/repos.json | 10 +- internal/store/database/testdata/spaces.json | 6 +- internal/store/database/token.go | 25 +- internal/store/database/token_sync.go | 7 + internal/store/database/user.go | 210 ++++++---- internal/store/database/user_test.go | 2 +- internal/store/database/wire.go | 31 +- internal/store/store.go | 15 +- internal/store/transformation.go | 27 ++ internal/store/wire.go | 23 ++ internal/token/token.go | 16 +- mocks/mock_client.go | 16 + types/check/common.go | 85 ++-- types/check/grant.go | 26 ++ types/check/path.go | 31 +- types/check/repo.go | 28 +- types/check/service.go | 18 +- types/check/service_account.go | 31 +- types/check/space.go | 28 +- types/check/user.go | 31 +- types/check/user_test.go | 14 +- types/check/wire.go | 38 ++ types/config.go | 6 +- types/enum/repo.go | 15 +- types/enum/shared.go | 2 +- types/enum/space.go | 15 +- types/principal.go | 68 ++-- types/repo.go | 5 +- types/service.go | 17 +- types/service_account.go | 21 +- types/space.go | 5 +- types/token.go | 7 +- types/user.go | 18 +- web/package.json | 2 +- web/restful-react.config.js | 8 +- .../NewRepoModalButton/NewRepoModalButton.tsx | 23 +- web/src/framework/strings/stringTypes.ts | 7 +- web/src/i18n/strings.en.yaml | 7 +- .../RepositoriesListing.tsx | 4 +- .../RepositoryHeader/RepositoryHeader.tsx | 4 +- .../RepositoryBranchessHeader.tsx | 2 +- .../RepositoryCommitsHeader.tsx | 2 +- web/src/services/scm/index.tsx | 237 +++++++---- web/src/services/scm/swagger.yaml | 374 +++++++++++++----- web/src/utils/Utils.ts | 2 +- 126 files changed, 1936 insertions(+), 1070 deletions(-) create mode 100644 internal/store/database/migrate/postgres/0002_create_index_repositories_parentId.up.sql delete mode 100644 internal/store/database/migrate/postgres/0002_create_index_repositories_spaceId.up.sql create mode 100644 internal/store/database/migrate/sqlite/0002_create_index_repositories_parent.up.sql delete mode 100644 internal/store/database/migrate/sqlite/0002_create_index_repositories_spaceId.up.sql create mode 100644 internal/store/transformation.go create mode 100644 internal/store/wire.go create mode 100644 types/check/grant.go create mode 100644 types/check/wire.go diff --git a/.harness.env b/.harness.env index d57f1ee41..bc57d3fba 100644 --- a/.harness.env +++ b/.harness.env @@ -1,11 +1,8 @@ # Gitness values GITNESS_TRACE=true -GITNESS_ADMIN_NAME=Administrator -GITNESS_ADMIN_EMAIL=gitness@harness.io -GITNESS_ADMIN_PASSWORD=changeit # Harness specifc values -HARNESS_JWT_IDENTITY="gitness" +HARNESS_JWT_IDENTITY="code" HARNESS_JWT_SECRET="IC04LYMBf1lDP5oeY4hupxd4HJhLmN6azUku3xEbeE3SUx5G3ZYzhbiwVtK4i7AmqyU9OZkwB4v8E9qM" HARNESS_JWT_VALIDINMIN=1440 HARNESS_JWT_BEARER_SECRET="dOkdsVqdRPPRJG31XU0qY4MPqmBBMk0PTAGIKM6O7TGqhjyxScIdJe80mwh5Yb5zF3KxYBHw6B3Lfzlq" diff --git a/.local.env b/.local.env index b1a2e180d..9ef054ecc 100644 --- a/.local.env +++ b/.local.env @@ -1,5 +1,5 @@ GITNESS_TRACE=true GITNESS_ADMIN_UID=admin -GITNESS_ADMIN_NAME=Administrator +GITNESS_ADMIN_DISPLAYNAME=Administrator GITNESS_ADMIN_EMAIL=admin@gitness.io GITNESS_ADMIN_PASSWORD=changeit \ No newline at end of file diff --git a/Makefile b/Makefile index bdde1e616..7789e14f2 100644 --- a/Makefile +++ b/Makefile @@ -124,7 +124,7 @@ lint: tools generate # lint the golang code # the source file has changed. ########################################### cli/server/harness.wire_gen.go: cli/server/harness.wire.go ## Update the wire dependency injection if harness.wire.go has changed. - + @sh ./scripts/wire/harness.sh cli/server/standalone.wire_gen.go: cli/server/standalone.wire.go ## Update the wire dependency injection if standalone.wire.go has changed. @sh ./scripts/wire/standalone.sh diff --git a/README.md b/README.md index 91351acd5..e1b9726ab 100644 --- a/README.md +++ b/README.md @@ -92,7 +92,7 @@ $ ./gitness user self Generate a personal access token: ```bash -$ ./gitness user pat $NAME $LIFETIME_IN_S +$ ./gitness user pat $UID $LIFETIME_IN_S ``` Debug and output http responses from the server: diff --git a/cli/operations/user/create_pat.go b/cli/operations/user/create_pat.go index 578cf348e..f8d8c4cbf 100644 --- a/cli/operations/user/create_pat.go +++ b/cli/operations/user/create_pat.go @@ -21,14 +21,15 @@ import ( ) const tokenTmpl = ` -name: {{ .Token.Name }} -expiresAt: {{ .Token.ExpiresAt }} -token: {{ .AccessToken }} +principalID: {{ .Token.PrincipalID }} +uid: {{ .Token.UID }} +expiresAt: {{ .Token.ExpiresAt }} +token: {{ .AccessToken }} ` //#nosec G101 type createPATCommand struct { client client.Client - name string + uid string lifetimeInS int64 json bool @@ -40,7 +41,7 @@ func (c *createPATCommand) run(*kingpin.ParseContext) error { defer cancel() in := user.CreateTokenInput{ - Name: c.name, + UID: c.uid, Lifetime: time.Duration(int64(time.Second) * c.lifetimeInS), Grants: enum.AccessGrantAll, } @@ -70,8 +71,8 @@ func registerCreatePAT(app *kingpin.CmdClause, client client.Client) { cmd := app.Command("pat", "create personal access token"). Action(c.run) - cmd.Arg("name", "the name of the token"). - Required().StringVar(&c.name) + cmd.Arg("uid", "the uid of the token"). + Required().StringVar(&c.uid) cmd.Arg("lifetime", "the lifetime of the token in seconds"). Required().Int64Var(&c.lifetimeInS) diff --git a/cli/operations/user/self.go b/cli/operations/user/self.go index 48ce4d9d4..34ace5adc 100644 --- a/cli/operations/user/self.go +++ b/cli/operations/user/self.go @@ -19,7 +19,7 @@ import ( const userTmpl = ` uid: {{ .UID }} -name: {{ .Name }} +name: {{ .DisplayName }} email: {{ .Email }} admin: {{ .Admin }} ` diff --git a/cli/server/harness.wire.go b/cli/server/harness.wire.go index e9ef12b14..33978bcfc 100644 --- a/cli/server/harness.wire.go +++ b/cli/server/harness.wire.go @@ -11,17 +11,18 @@ import ( "context" "github.com/harness/gitness/gitrpc" - gitrpcserver "github.com/harness/gitness/gitrpc/server" - "github.com/harness/gitness/harness/auth/authn" "github.com/harness/gitness/harness/auth/authz" "github.com/harness/gitness/harness/bootstrap" "github.com/harness/gitness/harness/client" "github.com/harness/gitness/harness/router" + "github.com/harness/gitness/harness/store" "github.com/harness/gitness/harness/types" + "github.com/harness/gitness/harness/types/check" "github.com/harness/gitness/internal/api/controller/repo" "github.com/harness/gitness/internal/api/controller/service" + "github.com/harness/gitness/internal/api/controller/serviceaccount" "github.com/harness/gitness/internal/api/controller/space" "github.com/harness/gitness/internal/api/controller/user" "github.com/harness/gitness/internal/cron" @@ -46,6 +47,7 @@ func initSystem(ctx context.Context, config *gitnessTypes.Config) (*system, erro space.WireSet, user.WireSet, service.WireSet, + serviceaccount.WireSet, gitrpcserver.WireSet, gitrpc.WireSet, types.LoadConfig, @@ -53,6 +55,8 @@ func initSystem(ctx context.Context, config *gitnessTypes.Config) (*system, erro authn.WireSet, authz.WireSet, client.WireSet, + store.WireSet, + check.WireSet, ) return &system{}, nil } diff --git a/cli/server/harness.wire_gen.go b/cli/server/harness.wire_gen.go index b24c68dc9..41e2ae477 100644 --- a/cli/server/harness.wire_gen.go +++ b/cli/server/harness.wire_gen.go @@ -7,7 +7,6 @@ package server import ( "context" - "github.com/harness/gitness/gitrpc" server2 "github.com/harness/gitness/gitrpc/server" "github.com/harness/gitness/harness/auth/authn" @@ -15,9 +14,12 @@ import ( "github.com/harness/gitness/harness/bootstrap" "github.com/harness/gitness/harness/client" "github.com/harness/gitness/harness/router" + "github.com/harness/gitness/harness/store" types2 "github.com/harness/gitness/harness/types" + "github.com/harness/gitness/harness/types/check" "github.com/harness/gitness/internal/api/controller/repo" "github.com/harness/gitness/internal/api/controller/service" + "github.com/harness/gitness/internal/api/controller/serviceaccount" "github.com/harness/gitness/internal/api/controller/space" "github.com/harness/gitness/internal/api/controller/user" "github.com/harness/gitness/internal/cron" @@ -31,6 +33,7 @@ import ( // Injectors from harness.wire.go: func initSystem(ctx context.Context, config *types.Config) (*system, error) { + checkUser := check.ProvideUserCheck() typesConfig, err := types2.LoadConfig() if err != nil { return nil, err @@ -48,11 +51,13 @@ func initSystem(ctx context.Context, config *types.Config) (*system, error) { if err != nil { return nil, err } - userStore := database.ProvideUserStore(db) + principalUIDTransformation := store.ProvidePrincipalUIDTransformation() + userStore := database.ProvideUserStore(db, principalUIDTransformation) tokenStore := database.ProvideTokenStore(db) - controller := user.NewController(authorizer, userStore, tokenStore) - serviceStore := database.ProvideServiceStore(db) - serviceController := service.NewController(authorizer, serviceStore) + controller := user.NewController(checkUser, authorizer, userStore, tokenStore) + checkService := check.ProvideServiceCheck() + serviceStore := database.ProvideServiceStore(db, principalUIDTransformation) + serviceController := service.NewController(checkService, authorizer, serviceStore) bootstrapBootstrap := bootstrap.ProvideBootstrap(config, controller, serviceController) systemStore := memory.New(config) tokenClient, err := client.ProvideTokenClient(serviceJWTProvider, typesConfig) @@ -67,24 +72,29 @@ func initSystem(ctx context.Context, config *types.Config) (*system, error) { if err != nil { return nil, err } - serviceAccountStore := database.ProvideServiceAccountStore(db) - authenticator, err := authn.ProvideAuthenticator(userStore, tokenClient, userClient, typesConfig, serviceAccountClient, serviceAccountStore, serviceStore) - if err != nil { - return nil, err - } + serviceAccount := check.ProvideServiceAccountCheck() + serviceAccountStore := database.ProvideServiceAccountStore(db, principalUIDTransformation) + pathTransformation := store.ProvidePathTransformation() + spaceStore := database.ProvideSpaceStore(db, pathTransformation) + repoStore := database.ProvideRepoStore(db, pathTransformation) + serviceaccountController := serviceaccount.NewController(serviceAccount, authorizer, serviceAccountStore, spaceStore, repoStore, tokenStore) + checkSpace := check.ProvideSpaceCheck() + spaceController := space.NewController(checkSpace, authorizer, spaceStore, repoStore, serviceAccountStore) accountClient, err := client.ProvideAccountClient(serviceJWTProvider, typesConfig) if err != nil { return nil, err } - spaceStore := database.ProvideSpaceStore(db) - repoStore := database.ProvideRepoStore(db) - spaceController := space.NewController(authorizer, spaceStore, repoStore, serviceAccountStore) + authenticator, err := authn.ProvideAuthenticator(controller, tokenClient, userClient, typesConfig, serviceAccountClient, serviceaccountController, serviceController, spaceController, accountClient) + if err != nil { + return nil, err + } + checkRepo := check.ProvideRepoCheck() gitrpcConfig := ProvideGitRPCClientConfig(config) gitrpcInterface, err := gitrpc.ProvideClient(gitrpcConfig) if err != nil { return nil, err } - repoController := repo.ProvideController(config, authorizer, spaceStore, repoStore, serviceAccountStore, gitrpcInterface) + repoController := repo.ProvideController(config, checkRepo, authorizer, spaceStore, repoStore, serviceAccountStore, gitrpcInterface) apiHandler := router.ProvideAPIHandler(systemStore, authenticator, accountClient, spaceController, repoController) gitHandler := router2.ProvideGitHandler(repoStore, authenticator, gitrpcInterface) webHandler := router2.ProvideWebHandler(systemStore) diff --git a/cli/server/standalone.wire.go b/cli/server/standalone.wire.go index 688ebe329..baf794044 100644 --- a/cli/server/standalone.wire.go +++ b/cli/server/standalone.wire.go @@ -11,9 +11,7 @@ import ( "context" "github.com/harness/gitness/gitrpc" - gitrpcserver "github.com/harness/gitness/gitrpc/server" - "github.com/harness/gitness/internal/api/controller/repo" "github.com/harness/gitness/internal/api/controller/serviceaccount" "github.com/harness/gitness/internal/api/controller/space" @@ -24,9 +22,11 @@ import ( "github.com/harness/gitness/internal/cron" "github.com/harness/gitness/internal/router" "github.com/harness/gitness/internal/server" + "github.com/harness/gitness/internal/store" "github.com/harness/gitness/internal/store/database" "github.com/harness/gitness/internal/store/memory" "github.com/harness/gitness/types" + "github.com/harness/gitness/types/check" "github.com/google/wire" ) @@ -49,6 +49,8 @@ func initSystem(ctx context.Context, config *types.Config) (*system, error) { authz.WireSet, gitrpcserver.WireSet, gitrpc.WireSet, + store.WireSet, + check.WireSet, ) return &system{}, nil } diff --git a/cli/server/standalone.wire_gen.go b/cli/server/standalone.wire_gen.go index a096f1fc1..c774dec93 100644 --- a/cli/server/standalone.wire_gen.go +++ b/cli/server/standalone.wire_gen.go @@ -7,7 +7,6 @@ package server import ( "context" - "github.com/harness/gitness/gitrpc" server2 "github.com/harness/gitness/gitrpc/server" "github.com/harness/gitness/internal/api/controller/repo" @@ -20,36 +19,44 @@ import ( "github.com/harness/gitness/internal/cron" "github.com/harness/gitness/internal/router" "github.com/harness/gitness/internal/server" + "github.com/harness/gitness/internal/store" "github.com/harness/gitness/internal/store/database" "github.com/harness/gitness/internal/store/memory" "github.com/harness/gitness/types" + "github.com/harness/gitness/types/check" ) // Injectors from standalone.wire.go: func initSystem(ctx context.Context, config *types.Config) (*system, error) { + checkUser := check.ProvideUserCheck() authorizer := authz.ProvideAuthorizer() db, err := database.ProvideDatabase(ctx, config) if err != nil { return nil, err } - userStore := database.ProvideUserStore(db) + principalUIDTransformation := store.ProvidePrincipalUIDTransformation() + userStore := database.ProvideUserStore(db, principalUIDTransformation) tokenStore := database.ProvideTokenStore(db) - controller := user.NewController(authorizer, userStore, tokenStore) + controller := user.NewController(checkUser, authorizer, userStore, tokenStore) bootstrapBootstrap := bootstrap.ProvideBootstrap(config, controller) systemStore := memory.New(config) - serviceAccountStore := database.ProvideServiceAccountStore(db) + serviceAccountStore := database.ProvideServiceAccountStore(db, principalUIDTransformation) authenticator := authn.ProvideAuthenticator(userStore, serviceAccountStore, tokenStore) - spaceStore := database.ProvideSpaceStore(db) - repoStore := database.ProvideRepoStore(db) + checkRepo := check.ProvideRepoCheck() + pathTransformation := store.ProvidePathTransformation() + spaceStore := database.ProvideSpaceStore(db, pathTransformation) + repoStore := database.ProvideRepoStore(db, pathTransformation) gitrpcConfig := ProvideGitRPCClientConfig(config) gitrpcInterface, err := gitrpc.ProvideClient(gitrpcConfig) if err != nil { return nil, err } - repoController := repo.ProvideController(config, authorizer, spaceStore, repoStore, serviceAccountStore, gitrpcInterface) - spaceController := space.NewController(authorizer, spaceStore, repoStore, serviceAccountStore) - serviceaccountController := serviceaccount.NewController(authorizer, serviceAccountStore, spaceStore, repoStore, tokenStore) + repoController := repo.ProvideController(config, checkRepo, authorizer, spaceStore, repoStore, serviceAccountStore, gitrpcInterface) + checkSpace := check.ProvideSpaceCheck() + spaceController := space.NewController(checkSpace, authorizer, spaceStore, repoStore, serviceAccountStore) + serviceAccount := check.ProvideServiceAccountCheck() + serviceaccountController := serviceaccount.NewController(serviceAccount, authorizer, serviceAccountStore, spaceStore, repoStore, tokenStore) apiHandler := router.ProvideAPIHandler(systemStore, authenticator, repoController, spaceController, serviceaccountController, controller) gitHandler := router.ProvideGitHandler(repoStore, authenticator, gitrpcInterface) webHandler := router.ProvideWebHandler(systemStore) diff --git a/client/client.go b/client/client.go index a16b0cba0..aaf454795 100644 --- a/client/client.go +++ b/client/client.go @@ -65,21 +65,21 @@ func (c *HTTPClient) Login(ctx context.Context, username, password string) (*typ form.Add("password", password) out := new(types.TokenResponse) uri := fmt.Sprintf("%s/api/v1/login", c.base) - err := c.post(ctx, uri, form, out) + err := c.post(ctx, uri, true, form, out) return out, err } // Register registers a new user and returns a JWT token. func (c *HTTPClient) Register(ctx context.Context, - username, name, email, password string) (*types.TokenResponse, error) { + username, displayName, email, password string) (*types.TokenResponse, error) { form := &url.Values{} form.Add("username", username) - form.Add("name", name) + form.Add("displayname", displayName) form.Add("email", email) form.Add("password", password) out := new(types.TokenResponse) uri := fmt.Sprintf("%s/api/v1/register", c.base) - err := c.post(ctx, uri, form, out) + err := c.post(ctx, uri, true, form, out) return out, err } @@ -99,7 +99,7 @@ func (c *HTTPClient) Self(ctx context.Context) (*types.User, error) { func (c *HTTPClient) UserCreatePAT(ctx context.Context, in user.CreateTokenInput) (*types.TokenResponse, error) { out := new(types.TokenResponse) uri := fmt.Sprintf("%s/api/v1/user/tokens", c.base) - err := c.post(ctx, uri, in, out) + err := c.post(ctx, uri, false, in, out) return out, err } @@ -123,7 +123,7 @@ func (c *HTTPClient) UserList(ctx context.Context, params types.Params) ([]types func (c *HTTPClient) UserCreate(ctx context.Context, user *types.User) (*types.User, error) { out := new(types.User) uri := fmt.Sprintf("%s/api/v1/users", c.base) - err := c.post(ctx, uri, user, out) + err := c.post(ctx, uri, false, user, out) return out, err } @@ -148,29 +148,29 @@ func (c *HTTPClient) UserDelete(ctx context.Context, key string) error { // helper function for making an http GET request. func (c *HTTPClient) get(ctx context.Context, rawurl string, out interface{}) error { - return c.do(ctx, rawurl, "GET", nil, out) + return c.do(ctx, rawurl, "GET", false, nil, out) } // helper function for making an http POST request. -func (c *HTTPClient) post(ctx context.Context, rawurl string, in, out interface{}) error { - return c.do(ctx, rawurl, "POST", in, out) +func (c *HTTPClient) post(ctx context.Context, rawurl string, noToken bool, in, out interface{}) error { + return c.do(ctx, rawurl, "POST", noToken, in, out) } // helper function for making an http PATCH request. func (c *HTTPClient) patch(ctx context.Context, rawurl string, in, out interface{}) error { - return c.do(ctx, rawurl, "PATCH", in, out) + return c.do(ctx, rawurl, "PATCH", false, in, out) } // helper function for making an http DELETE request. func (c *HTTPClient) delete(ctx context.Context, rawurl string) error { - return c.do(ctx, rawurl, "DELETE", nil, nil) + return c.do(ctx, rawurl, "DELETE", false, nil, nil) } // helper function to make an http request. -func (c *HTTPClient) do(ctx context.Context, rawurl, method string, in, out interface{}) error { +func (c *HTTPClient) do(ctx context.Context, rawurl, method string, noToken bool, in, out interface{}) error { // executes the http request and returns the body as // and io.ReadCloser - body, err := c.stream(ctx, rawurl, method, in, out) + body, err := c.stream(ctx, rawurl, method, noToken, in, out) if body != nil { defer func(body io.ReadCloser) { _ = body.Close() @@ -189,7 +189,8 @@ func (c *HTTPClient) do(ctx context.Context, rawurl, method string, in, out inte } // helper function to stream a http request. -func (c *HTTPClient) stream(ctx context.Context, rawurl, method string, in, _ interface{}) (io.ReadCloser, error) { +func (c *HTTPClient) stream(ctx context.Context, rawurl, method string, noToken bool, + in, _ interface{}) (io.ReadCloser, error) { uri, err := url.Parse(rawurl) if err != nil { return nil, err @@ -218,7 +219,7 @@ func (c *HTTPClient) stream(ctx context.Context, rawurl, method string, in, _ in if in != nil { req.Header.Set("Content-Type", "application/json") } - if c.token != "" { + if !noToken && c.token != "" { req.Header.Set("Authorization", "Bearer "+c.token) } if _, ok := in.(*url.Values); ok { diff --git a/gitrpc/internal/service/repo.go b/gitrpc/internal/service/repo.go index a5fc48bd7..68bc739de 100644 --- a/gitrpc/internal/service/repo.go +++ b/gitrpc/internal/service/repo.go @@ -64,7 +64,13 @@ func NewRepositoryService(adapter GitAdapter, store Storage, gitRoot string) (*R } func (s RepositoryService) getFullPathForRepo(uid string) string { - return filepath.Join(s.reposRoot, fmt.Sprintf("%s.%s", uid, gitRepoSuffix)) + // split repos into subfolders using their prefix to distribute repos accross a set of folders. + return filepath.Join( + s.reposRoot, // root folder + uid[0:2], // first subfolder + uid[2:4], // second subfolder + fmt.Sprintf("%s.%s", uid[4:], gitRepoSuffix), // remainder with .git + ) } //nolint:gocognit // need to refactor this code diff --git a/gitrpc/repo.go b/gitrpc/repo.go index 4b7dbca55..50e606b22 100644 --- a/gitrpc/repo.go +++ b/gitrpc/repo.go @@ -14,6 +14,11 @@ import ( "github.com/rs/zerolog/log" ) +const ( + // repoGitUIDLength is the length of the generated repo uid. + repoGitUIDLength = 21 +) + type CreateRepositoryParams struct { DefaultBranch string Files []File @@ -83,5 +88,5 @@ func (c *Client) CreateRepository(ctx context.Context, } func newRepositoryUID() (string, error) { - return gonanoid.New() + return gonanoid.New(repoGitUIDLength) } diff --git a/internal/api/auth/auth.go b/internal/api/auth/auth.go index fabf63649..2c06af22c 100644 --- a/internal/api/auth/auth.go +++ b/internal/api/auth/auth.go @@ -93,7 +93,7 @@ func getScopeForParent(ctx context.Context, spaceStore store.SpaceStore, repoSto return nil, fmt.Errorf("parent repo not found: %w", err) } - spacePath, repoName, err := paths.Disect(repo.Path) + spacePath, repoName, err := paths.DisectLeaf(repo.Path) if err != nil { return nil, errors.Wrapf(err, "Failed to disect path '%s'", repo.Path) } diff --git a/internal/api/auth/repo.go b/internal/api/auth/repo.go index dd36d0439..af3194cc9 100644 --- a/internal/api/auth/repo.go +++ b/internal/api/auth/repo.go @@ -27,7 +27,7 @@ func CheckRepo(ctx context.Context, authorizer authz.Authorizer, session *auth.S return nil } - parentSpace, name, err := paths.Disect(repo.Path) + parentSpace, name, err := paths.DisectLeaf(repo.Path) if err != nil { return errors.Wrapf(err, "Failed to disect path '%s'", repo.Path) } diff --git a/internal/api/auth/space.go b/internal/api/auth/space.go index 755bdfd60..916599db3 100644 --- a/internal/api/auth/space.go +++ b/internal/api/auth/space.go @@ -27,7 +27,7 @@ func CheckSpace(ctx context.Context, authorizer authz.Authorizer, session *auth. return nil } - parentSpace, name, err := paths.Disect(space.Path) + parentSpace, name, err := paths.DisectLeaf(space.Path) if err != nil { return errors.Wrapf(err, "Failed to disect path '%s'", space.Path) } diff --git a/internal/api/controller/repo/controller.go b/internal/api/controller/repo/controller.go index d6f99adbf..5aae97a67 100644 --- a/internal/api/controller/repo/controller.go +++ b/internal/api/controller/repo/controller.go @@ -8,10 +8,12 @@ import ( "github.com/harness/gitness/gitrpc" "github.com/harness/gitness/internal/auth/authz" "github.com/harness/gitness/internal/store" + "github.com/harness/gitness/types/check" ) type Controller struct { defaultBranch string + repoCheck check.Repo authorizer authz.Authorizer spaceStore store.SpaceStore repoStore store.RepoStore @@ -21,6 +23,7 @@ type Controller struct { func NewController( defaultBranch string, + repoCheck check.Repo, authorizer authz.Authorizer, spaceStore store.SpaceStore, repoStore store.RepoStore, @@ -29,6 +32,7 @@ func NewController( ) *Controller { return &Controller{ defaultBranch: defaultBranch, + repoCheck: repoCheck, authorizer: authorizer, spaceStore: spaceStore, repoStore: repoStore, diff --git a/internal/api/controller/repo/create.go b/internal/api/controller/repo/create.go index f585b3d29..b7a896200 100644 --- a/internal/api/controller/repo/create.go +++ b/internal/api/controller/repo/create.go @@ -8,7 +8,6 @@ import ( "bytes" "context" "fmt" - "strings" "time" "github.com/harness/gitness/gitrpc" @@ -19,15 +18,13 @@ import ( "github.com/harness/gitness/internal/api/usererror" "github.com/harness/gitness/internal/auth" "github.com/harness/gitness/types" - "github.com/harness/gitness/types/check" "github.com/harness/gitness/types/enum" zerolog "github.com/rs/zerolog/log" ) type CreateInput struct { - PathName string `json:"pathName"` - SpaceID int64 `json:"spaceId"` - Name string `json:"name"` + ParentID int64 `json:"parentID"` + UID string `json:"uid"` DefaultBranch string `json:"defaultBranch"` Description string `json:"description"` IsPublic bool `json:"isPublic"` @@ -43,13 +40,13 @@ type CreateInput struct { func (c *Controller) Create(ctx context.Context, session *auth.Session, in *CreateInput) (*types.Repository, error) { log := zerolog.Ctx(ctx) // ensure we reference a space - if in.SpaceID <= 0 { + if in.ParentID <= 0 { return nil, usererror.BadRequest("A repository can't exist by itself.") } - parentSpace, err := c.spaceStore.Find(ctx, in.SpaceID) + parentSpace, err := c.spaceStore.Find(ctx, in.ParentID) if err != nil { - log.Err(err).Msgf("Failed to get space with id '%d'.", in.SpaceID) + log.Err(err).Msgf("Failed to get space with id '%d'.", in.ParentID) return nil, usererror.BadRequest("Parent not found'") } /* @@ -74,9 +71,8 @@ func (c *Controller) Create(ctx context.Context, session *auth.Session, in *Crea // create new repo object repo := &types.Repository{ - PathName: strings.ToLower(in.PathName), - SpaceID: in.SpaceID, - Name: in.Name, + ParentID: in.ParentID, + UID: in.UID, Description: in.Description, IsPublic: in.IsPublic, CreatedBy: session.Principal.ID, @@ -87,13 +83,13 @@ func (c *Controller) Create(ctx context.Context, session *auth.Session, in *Crea } // validate repo - if err = check.Repo(repo); err != nil { + if err = c.repoCheck(repo); err != nil { return nil, err } var content []byte files := make([]gitrpc.File, 0, 3) // readme, gitignore, licence if in.Readme { - content = createReadme(in.Name, in.Description) + content = createReadme(in.UID, in.Description) files = append(files, gitrpc.File{ Path: "README.md", Content: content, diff --git a/internal/api/controller/repo/create_path.go b/internal/api/controller/repo/create_path.go index 595f03423..5c9eb9dac 100644 --- a/internal/api/controller/repo/create_path.go +++ b/internal/api/controller/repo/create_path.go @@ -6,7 +6,6 @@ package repo import ( "context" - "strings" "time" apiauth "github.com/harness/gitness/internal/api/auth" @@ -36,7 +35,7 @@ func (c *Controller) CreatePath(ctx context.Context, session *auth.Session, } params := &types.PathParams{ - Path: strings.ToLower(in.Path), + Path: in.Path, CreatedBy: session.Principal.ID, Created: time.Now().UnixMilli(), Updated: time.Now().UnixMilli(), diff --git a/internal/api/controller/repo/move.go b/internal/api/controller/repo/move.go index 582915d0f..d8b37c469 100644 --- a/internal/api/controller/repo/move.go +++ b/internal/api/controller/repo/move.go @@ -6,9 +6,9 @@ package repo import ( "context" - "strings" apiauth "github.com/harness/gitness/internal/api/auth" + "github.com/harness/gitness/internal/api/usererror" "github.com/harness/gitness/internal/auth" "github.com/harness/gitness/types" "github.com/harness/gitness/types/check" @@ -18,13 +18,13 @@ import ( // MoveInput is used for moving a repo. type MoveInput struct { - PathName *string `json:"pathName"` - SpaceID *int64 `json:"spaceId"` + UID *string `json:"uid"` + ParentID *int64 `json:"parentId"` KeepAsAlias bool `json:"keepAsAlias"` } /* -* Move moves a repository to a new space and/or name. +* Move moves a repository to a new space and/or uid. */ func (c *Controller) Move(ctx context.Context, session *auth.Session, repoRef string, in *MoveInput) (*types.Repository, error) { @@ -38,37 +38,34 @@ func (c *Controller) Move(ctx context.Context, session *auth.Session, } // backfill data - if in.PathName == nil { - in.PathName = &repo.PathName + if in.UID == nil { + in.UID = &repo.UID } - if in.SpaceID == nil { - in.SpaceID = &repo.SpaceID + if in.ParentID == nil { + in.ParentID = &repo.ParentID } - // convert name to lower case for easy of api use - *in.PathName = strings.ToLower(*in.PathName) - // verify input - if err = check.PathName(*in.PathName); err != nil { - return nil, err - } - - // ensure it's not a no-op - if *in.SpaceID == repo.SpaceID && *in.PathName == repo.PathName { + if err = check.UID(*in.UID); err != nil { return nil, err } // ensure we move to another space - if *in.SpaceID <= 0 { - return nil, err + if *in.ParentID <= 0 { + return nil, usererror.ErrBadRequest + } + + // ensure it's not a no-op + if *in.ParentID == repo.ParentID && *in.UID == repo.UID { + return nil, usererror.ErrNoChange } // Ensure we have access to the target space (if it's a space move) - if *in.SpaceID != repo.SpaceID { + if *in.ParentID != repo.ParentID { var newSpace *types.Space - newSpace, err = c.spaceStore.Find(ctx, *in.SpaceID) + newSpace, err = c.spaceStore.Find(ctx, *in.ParentID) if err != nil { - log.Err(err).Msgf("Failed to get target space with id %d for the move.", *in.SpaceID) + log.Err(err).Msgf("Failed to get target space with id %d for the move.", *in.ParentID) return nil, err } @@ -84,5 +81,5 @@ func (c *Controller) Move(ctx context.Context, session *auth.Session, } } - return c.repoStore.Move(ctx, session.Principal.ID, repo.ID, *in.SpaceID, *in.PathName, in.KeepAsAlias) + return c.repoStore.Move(ctx, session.Principal.ID, repo.ID, *in.ParentID, *in.UID, in.KeepAsAlias) } diff --git a/internal/api/controller/repo/update.go b/internal/api/controller/repo/update.go index 506c5b143..bc9328d12 100644 --- a/internal/api/controller/repo/update.go +++ b/internal/api/controller/repo/update.go @@ -11,13 +11,11 @@ import ( apiauth "github.com/harness/gitness/internal/api/auth" "github.com/harness/gitness/internal/auth" "github.com/harness/gitness/types" - "github.com/harness/gitness/types/check" "github.com/harness/gitness/types/enum" ) // UpdateInput is used for updating a repo. type UpdateInput struct { - Name *string `json:"name"` Description *string `json:"description"` IsPublic *bool `json:"isPublic"` } @@ -37,9 +35,6 @@ func (c *Controller) Update(ctx context.Context, session *auth.Session, } // update values only if provided - if in.Name != nil { - repo.Name = *in.Name - } if in.Description != nil { repo.Description = *in.Description } @@ -51,7 +46,7 @@ func (c *Controller) Update(ctx context.Context, session *auth.Session, repo.Updated = time.Now().UnixMilli() // ensure provided values are valid - if err = check.Repo(repo); err != nil { + if err = c.repoCheck(repo); err != nil { return nil, err } diff --git a/internal/api/controller/repo/wire.go b/internal/api/controller/repo/wire.go index 2fbb4de18..52c8e98b1 100644 --- a/internal/api/controller/repo/wire.go +++ b/internal/api/controller/repo/wire.go @@ -10,6 +10,7 @@ import ( "github.com/harness/gitness/internal/auth/authz" "github.com/harness/gitness/internal/store" "github.com/harness/gitness/types" + "github.com/harness/gitness/types/check" ) // WireSet provides a wire set for this package. @@ -17,7 +18,8 @@ var WireSet = wire.NewSet( ProvideController, ) -func ProvideController(config *types.Config, authorizer authz.Authorizer, spaceStore store.SpaceStore, - repoStore store.RepoStore, saStore store.ServiceAccountStore, rpcClient gitrpc.Interface) *Controller { - return NewController(config.Git.DefaultBranch, authorizer, spaceStore, repoStore, saStore, rpcClient) +func ProvideController(config *types.Config, repoCheck check.Repo, authorizer authz.Authorizer, + spaceStore store.SpaceStore, repoStore store.RepoStore, saStore store.ServiceAccountStore, + rpcClient gitrpc.Interface) *Controller { + return NewController(config.Git.DefaultBranch, repoCheck, authorizer, spaceStore, repoStore, saStore, rpcClient) } diff --git a/internal/api/controller/service/controller.go b/internal/api/controller/service/controller.go index c20f031e6..e7bf7ee90 100644 --- a/internal/api/controller/service/controller.go +++ b/internal/api/controller/service/controller.go @@ -7,15 +7,19 @@ package service import ( "github.com/harness/gitness/internal/auth/authz" "github.com/harness/gitness/internal/store" + "github.com/harness/gitness/types/check" ) type Controller struct { + serviceCheck check.Service authorizer authz.Authorizer serviceStore store.ServiceStore } -func NewController(authorizer authz.Authorizer, serviceStore store.ServiceStore) *Controller { +func NewController(serviceCheck check.Service, authorizer authz.Authorizer, + serviceStore store.ServiceStore) *Controller { return &Controller{ + serviceCheck: serviceCheck, authorizer: authorizer, serviceStore: serviceStore, } diff --git a/internal/api/controller/service/create.go b/internal/api/controller/service/create.go index 5abc363a1..b2d13b32c 100644 --- a/internal/api/controller/service/create.go +++ b/internal/api/controller/service/create.go @@ -12,14 +12,14 @@ import ( apiauth "github.com/harness/gitness/internal/api/auth" "github.com/harness/gitness/internal/auth" "github.com/harness/gitness/types" - "github.com/harness/gitness/types/check" "github.com/harness/gitness/types/enum" ) // CreateInput is the input used for create operations. type CreateInput struct { - UID string - Name string + UID string `json:"uid"` + Email string `json:"email"` + DisplayName string `json:"displayName"` } /* @@ -46,16 +46,17 @@ func (c *Controller) Create(ctx context.Context, session *auth.Session, in *Crea */ func (c *Controller) CreateNoAuth(ctx context.Context, in *CreateInput, admin bool) (*types.Service, error) { svc := &types.Service{ - UID: in.UID, - Name: in.Name, - Admin: admin, - Salt: uniuri.NewLen(uniuri.UUIDLen), - Created: time.Now().UnixMilli(), - Updated: time.Now().UnixMilli(), + UID: in.UID, + Email: in.Email, + DisplayName: in.DisplayName, + Admin: admin, + Salt: uniuri.NewLen(uniuri.UUIDLen), + Created: time.Now().UnixMilli(), + Updated: time.Now().UnixMilli(), } // validate service - if err := check.Service(svc); err != nil { + if err := c.serviceCheck(svc); err != nil { return nil, err } diff --git a/internal/api/controller/service/update.go b/internal/api/controller/service/update.go index d7633c86e..456514f41 100644 --- a/internal/api/controller/service/update.go +++ b/internal/api/controller/service/update.go @@ -12,13 +12,13 @@ import ( apiauth "github.com/harness/gitness/internal/api/auth" "github.com/harness/gitness/internal/auth" "github.com/harness/gitness/types" - "github.com/harness/gitness/types/check" "github.com/harness/gitness/types/enum" ) // UpdateInput store infos to update an existing service. type UpdateInput struct { - Name *string `json:"name"` + Email *string `json:"email"` + DisplayName *string `json:"displayName"` } /* @@ -36,13 +36,16 @@ func (c *Controller) Update(ctx context.Context, session *auth.Session, return nil, err } - if in.Name != nil { - svc.Name = ptr.ToString(in.Name) + if in.Email != nil { + svc.DisplayName = ptr.ToString(in.Email) + } + if in.DisplayName != nil { + svc.DisplayName = ptr.ToString(in.DisplayName) } svc.Updated = time.Now().UnixMilli() // validate service - if err = check.Service(svc); err != nil { + if err = c.serviceCheck(svc); err != nil { return nil, err } diff --git a/internal/api/controller/service/wire.go b/internal/api/controller/service/wire.go index b1094eb37..1defcc9fd 100644 --- a/internal/api/controller/service/wire.go +++ b/internal/api/controller/service/wire.go @@ -8,6 +8,7 @@ import ( "github.com/google/wire" "github.com/harness/gitness/internal/auth/authz" "github.com/harness/gitness/internal/store" + "github.com/harness/gitness/types/check" ) // WireSet provides a wire set for this package. @@ -15,6 +16,7 @@ var WireSet = wire.NewSet( NewController, ) -func ProvideController(authorizer authz.Authorizer, serviceStore store.ServiceStore) *Controller { - return NewController(authorizer, serviceStore) +func ProvideController(serviceCheck check.Service, authorizer authz.Authorizer, + serviceStore store.ServiceStore) *Controller { + return NewController(serviceCheck, authorizer, serviceStore) } diff --git a/internal/api/controller/serviceaccount/controller.go b/internal/api/controller/serviceaccount/controller.go index 9137844f3..db5a28f22 100644 --- a/internal/api/controller/serviceaccount/controller.go +++ b/internal/api/controller/serviceaccount/controller.go @@ -7,23 +7,27 @@ package serviceaccount import ( "github.com/harness/gitness/internal/auth/authz" "github.com/harness/gitness/internal/store" + "github.com/harness/gitness/types/check" ) type Controller struct { - authorizer authz.Authorizer - saStore store.ServiceAccountStore - spaceStore store.SpaceStore - repoStore store.RepoStore - tokenStore store.TokenStore + serviceAccountCheck check.ServiceAccount + authorizer authz.Authorizer + saStore store.ServiceAccountStore + spaceStore store.SpaceStore + repoStore store.RepoStore + tokenStore store.TokenStore } -func NewController(authorizer authz.Authorizer, saStore store.ServiceAccountStore, - spaceStore store.SpaceStore, repoStore store.RepoStore, tokenStore store.TokenStore) *Controller { +func NewController(serviceAccountCheck check.ServiceAccount, authorizer authz.Authorizer, + saStore store.ServiceAccountStore, spaceStore store.SpaceStore, repoStore store.RepoStore, + tokenStore store.TokenStore) *Controller { return &Controller{ - authorizer: authorizer, - saStore: saStore, - spaceStore: spaceStore, - repoStore: repoStore, - tokenStore: tokenStore, + serviceAccountCheck: serviceAccountCheck, + authorizer: authorizer, + saStore: saStore, + spaceStore: spaceStore, + repoStore: repoStore, + tokenStore: tokenStore, } } diff --git a/internal/api/controller/serviceaccount/create.go b/internal/api/controller/serviceaccount/create.go index a3b997526..5fe9394ba 100644 --- a/internal/api/controller/serviceaccount/create.go +++ b/internal/api/controller/serviceaccount/create.go @@ -6,21 +6,27 @@ package serviceaccount import ( "context" + "fmt" "time" "github.com/dchest/uniuri" apiauth "github.com/harness/gitness/internal/api/auth" "github.com/harness/gitness/internal/auth" "github.com/harness/gitness/types" - "github.com/harness/gitness/types/check" "github.com/harness/gitness/types/enum" + gonanoid "github.com/matoous/go-nanoid/v2" +) + +var ( + serviceAccountUIDAlphabet = "abcdefghijklmnopqrstuvwxyz0123456789" + serviceAccountUIDLength = 16 ) type CreateInput struct { - UID string `json:"uid"` - Name string `json:"name"` - ParentType enum.ParentResourceType `json:"parentType"` - ParentID int64 `json:"parentId"` + Email string `json:"email"` + DisplayName string `json:"displayName"` + ParentType enum.ParentResourceType `json:"parentType"` + ParentID int64 `json:"parentId"` } /* @@ -28,25 +34,43 @@ type CreateInput struct { */ func (c *Controller) Create(ctx context.Context, session *auth.Session, in *CreateInput) (*types.ServiceAccount, error) { - sa := &types.ServiceAccount{ - UID: in.UID, - Name: in.Name, - Salt: uniuri.NewLen(uniuri.UUIDLen), - Created: time.Now().UnixMilli(), - Updated: time.Now().UnixMilli(), - ParentType: in.ParentType, - ParentID: in.ParentID, - } - - // validate service account - if err := check.ServiceAccount(sa); err != nil { - return nil, err - } - // Ensure principal has required permissions on parent (ensures that parent exists) // since it's a create, we use don't pass a resource name. if err := apiauth.CheckServiceAccount(ctx, c.authorizer, session, c.spaceStore, c.repoStore, - sa.ParentType, sa.ParentID, "", enum.PermissionServiceAccountCreate); err != nil { + in.ParentType, in.ParentID, "", enum.PermissionServiceAccountCreate); err != nil { + return nil, err + } + + uid, err := generateServiceAccountUID(in.ParentType, in.ParentID) + if err != nil { + return nil, fmt.Errorf("failed to generate service account UID: %w", err) + } + + // TODO: There's a chance of duplicate error - we should retry? + return c.CreateNoAuth(ctx, in, uid) +} + +/* + * CreateNoAuth creates a new service account without auth checks. + * WARNING: Never call as part of user flow. + * + * Note: take uid separately to allow internally created non-random uids. + */ +func (c *Controller) CreateNoAuth(ctx context.Context, + in *CreateInput, uid string) (*types.ServiceAccount, error) { + sa := &types.ServiceAccount{ + UID: uid, + Email: in.Email, + DisplayName: in.DisplayName, + Salt: uniuri.NewLen(uniuri.UUIDLen), + Created: time.Now().UnixMilli(), + Updated: time.Now().UnixMilli(), + ParentType: in.ParentType, + ParentID: in.ParentID, + } + + // validate service account + if err := c.serviceAccountCheck(sa); err != nil { return nil, err } @@ -58,3 +82,22 @@ func (c *Controller) Create(ctx context.Context, session *auth.Session, return sa, nil } + +// generateServiceAccountUID generates a new unique UID for a service account +// NOTE: +// This method generates 36^10 = ~8*10^24 unique UIDs per parent. +// This should be enough for a very low chance of duplications. +// +// NOTE: +// We generate it automatically to ensure unique UIDs on principals. +// The downside is that they don't have very userfriendly handlers - though that should be okay for service accounts. +// The other option would be take it as an input, but a globally unique uid of a service account +// which itself is scoped to a space / repo might be weird. +func generateServiceAccountUID(parentType enum.ParentResourceType, parentID int64) (string, error) { + nid, err := gonanoid.Generate(serviceAccountUIDAlphabet, serviceAccountUIDLength) + if err != nil { + return "", err + } + + return fmt.Sprintf("sa-%s-%d-%s", string(parentType), parentID, nid), nil +} diff --git a/internal/api/controller/serviceaccount/create_token.go b/internal/api/controller/serviceaccount/create_token.go index 22b0505f9..c6201bc37 100644 --- a/internal/api/controller/serviceaccount/create_token.go +++ b/internal/api/controller/serviceaccount/create_token.go @@ -17,7 +17,7 @@ import ( ) type CreateTokenInput struct { - Name string `json:"name"` + UID string `json:"uid"` Lifetime time.Duration `json:"lifetime"` Grants enum.AccessGrant `json:"grants"` } @@ -32,12 +32,15 @@ func (c *Controller) CreateToken(ctx context.Context, session *auth.Session, return nil, err } - if err = check.Name(in.Name); err != nil { + if err = check.UID(in.UID); err != nil { return nil, err } if err = check.TokenLifetime(in.Lifetime); err != nil { return nil, err } + if err = check.AccessGrant(in.Grants, false); err != nil { + return nil, err + } // Ensure principal has required permissions on parent (ensures that parent exists) if err = apiauth.CheckServiceAccount(ctx, c.authorizer, session, c.spaceStore, c.repoStore, @@ -45,7 +48,7 @@ func (c *Controller) CreateToken(ctx context.Context, session *auth.Session, return nil, err } token, jwtToken, err := token.CreateSAT(ctx, c.tokenStore, &session.Principal, - sa, in.Name, in.Lifetime, in.Grants) + sa, in.UID, in.Lifetime, in.Grants) if err != nil { return nil, err } diff --git a/internal/api/controller/serviceaccount/delete_token.go b/internal/api/controller/serviceaccount/delete_token.go index 2da748f30..a08c20860 100644 --- a/internal/api/controller/serviceaccount/delete_token.go +++ b/internal/api/controller/serviceaccount/delete_token.go @@ -18,7 +18,7 @@ import ( * DeleteToken deletes a token of a sevice account. */ func (c *Controller) DeleteToken(ctx context.Context, session *auth.Session, - saUID string, tokenID int64) error { + saUID string, tokenUID string) error { sa, err := findServiceAccountFromUID(ctx, c.saStore, saUID) if err != nil { return err @@ -30,7 +30,7 @@ func (c *Controller) DeleteToken(ctx context.Context, session *auth.Session, return err } - token, err := c.tokenStore.Find(ctx, tokenID) + token, err := c.tokenStore.FindByUID(ctx, sa.ID, tokenUID) if err != nil { return err } @@ -43,5 +43,5 @@ func (c *Controller) DeleteToken(ctx context.Context, session *auth.Session, return usererror.ErrNotFound } - return c.tokenStore.Delete(ctx, tokenID) + return c.tokenStore.Delete(ctx, token.ID) } diff --git a/internal/api/controller/serviceaccount/find.go b/internal/api/controller/serviceaccount/find.go index 4f18acc11..85b66575b 100644 --- a/internal/api/controller/serviceaccount/find.go +++ b/internal/api/controller/serviceaccount/find.go @@ -18,7 +18,7 @@ import ( */ func (c *Controller) Find(ctx context.Context, session *auth.Session, saUID string) (*types.ServiceAccount, error) { - sa, err := findServiceAccountFromUID(ctx, c.saStore, saUID) + sa, err := c.FindNoAuth(ctx, saUID) if err != nil { return nil, err } @@ -31,3 +31,11 @@ func (c *Controller) Find(ctx context.Context, session *auth.Session, return sa, nil } + +/* + * FindNoAuth finds a service account without auth checks. + * WARNING: Never call as part of user flow. + */ +func (c *Controller) FindNoAuth(ctx context.Context, saUID string) (*types.ServiceAccount, error) { + return findServiceAccountFromUID(ctx, c.saStore, saUID) +} diff --git a/internal/api/controller/serviceaccount/wire.go b/internal/api/controller/serviceaccount/wire.go index 550157041..25764808e 100644 --- a/internal/api/controller/serviceaccount/wire.go +++ b/internal/api/controller/serviceaccount/wire.go @@ -8,6 +8,7 @@ import ( "github.com/google/wire" "github.com/harness/gitness/internal/auth/authz" "github.com/harness/gitness/internal/store" + "github.com/harness/gitness/types/check" ) // WireSet provides a wire set for this package. @@ -15,7 +16,8 @@ var WireSet = wire.NewSet( NewController, ) -func ProvideController(authorizer authz.Authorizer, saStore store.ServiceAccountStore, - spaceStore store.SpaceStore, repoStore store.RepoStore, tokenStore store.TokenStore) *Controller { - return NewController(authorizer, saStore, spaceStore, repoStore, tokenStore) +func ProvideController(serviceAccountCheck check.ServiceAccount, authorizer authz.Authorizer, + saStore store.ServiceAccountStore, spaceStore store.SpaceStore, repoStore store.RepoStore, + tokenStore store.TokenStore) *Controller { + return NewController(serviceAccountCheck, authorizer, saStore, spaceStore, repoStore, tokenStore) } diff --git a/internal/api/controller/space/controller.go b/internal/api/controller/space/controller.go index 1c9c389dd..b68047e84 100644 --- a/internal/api/controller/space/controller.go +++ b/internal/api/controller/space/controller.go @@ -7,18 +7,21 @@ package space import ( "github.com/harness/gitness/internal/auth/authz" "github.com/harness/gitness/internal/store" + "github.com/harness/gitness/types/check" ) type Controller struct { + spaceCheck check.Space authorizer authz.Authorizer spaceStore store.SpaceStore repoStore store.RepoStore saStore store.ServiceAccountStore } -func NewController(authorizer authz.Authorizer, spaceStore store.SpaceStore, +func NewController(spaceCheck check.Space, authorizer authz.Authorizer, spaceStore store.SpaceStore, repoStore store.RepoStore, saStore store.ServiceAccountStore) *Controller { return &Controller{ + spaceCheck: spaceCheck, authorizer: authorizer, spaceStore: spaceStore, repoStore: repoStore, diff --git a/internal/api/controller/space/create.go b/internal/api/controller/space/create.go index e39e8e11c..9c253035d 100644 --- a/internal/api/controller/space/create.go +++ b/internal/api/controller/space/create.go @@ -7,7 +7,6 @@ package space import ( "context" "fmt" - "strings" "time" apiauth "github.com/harness/gitness/internal/api/auth" @@ -20,9 +19,8 @@ import ( ) type CreateInput struct { - PathName string `json:"pathName"` ParentID int64 `json:"parentId"` - Name string `json:"name"` + UID string `json:"uid"` Description string `json:"description"` IsPublic bool `json:"isPublic"` } @@ -65,9 +63,8 @@ func (c *Controller) Create(ctx context.Context, session *auth.Session, in *Crea // create new space object space := &types.Space{ - PathName: strings.ToLower(in.PathName), ParentID: in.ParentID, - Name: in.Name, + UID: in.UID, Description: in.Description, IsPublic: in.IsPublic, CreatedBy: session.Principal.ID, @@ -76,14 +73,14 @@ func (c *Controller) Create(ctx context.Context, session *auth.Session, in *Crea } // validate space - if err := check.Space(space); err != nil { + if err := c.spaceCheck(space); err != nil { return nil, err } - // Validate path length (Due to racing conditions we can't be 100% sure on the path here only best effort + // Validate path depth (Due to racing conditions we can't be 100% sure on the path here only best effort // to have a quick failure) - path := paths.Concatinate(parentPath, space.PathName) - if err := check.Path(path, true); err != nil { + path := paths.Concatinate(parentPath, space.UID) + if err := check.PathDepth(path, true); err != nil { return nil, err } diff --git a/internal/api/controller/space/create_path.go b/internal/api/controller/space/create_path.go index da3419588..f3598a13f 100644 --- a/internal/api/controller/space/create_path.go +++ b/internal/api/controller/space/create_path.go @@ -6,7 +6,6 @@ package space import ( "context" - "strings" "time" apiauth "github.com/harness/gitness/internal/api/auth" @@ -36,7 +35,7 @@ func (c *Controller) CreatePath(ctx context.Context, session *auth.Session, } params := &types.PathParams{ - Path: strings.ToLower(in.Path), + Path: in.Path, CreatedBy: session.Principal.ID, Created: time.Now().UnixMilli(), Updated: time.Now().UnixMilli(), diff --git a/internal/api/controller/space/helper.go b/internal/api/controller/space/helper.go index 88a13365a..89d292353 100644 --- a/internal/api/controller/space/helper.go +++ b/internal/api/controller/space/helper.go @@ -13,7 +13,7 @@ import ( ) func findSpaceFromRef(ctx context.Context, spaceStore store.SpaceStore, spaceRef string) (*types.Space, error) { - // check if ref is spaceId - ASSUMPTION: digit only is no valid space name + // check if ref is space ID - ASSUMPTION: digit only is no valid space name id, err := strconv.ParseInt(spaceRef, 10, 64) if err == nil { return spaceStore.Find(ctx, id) diff --git a/internal/api/controller/space/move.go b/internal/api/controller/space/move.go index d42488cde..7565d4197 100644 --- a/internal/api/controller/space/move.go +++ b/internal/api/controller/space/move.go @@ -7,9 +7,9 @@ package space import ( "context" "fmt" - "strings" apiauth "github.com/harness/gitness/internal/api/auth" + "github.com/harness/gitness/internal/api/usererror" "github.com/harness/gitness/internal/auth" "github.com/harness/gitness/internal/paths" "github.com/harness/gitness/types" @@ -19,7 +19,7 @@ import ( // MoveInput is used for moving a space. type MoveInput struct { - PathName *string `json:"pathName"` + UID *string `json:"uid"` ParentID *int64 `json:"parentId"` KeepAsAlias bool `json:"keepAsAlias"` } @@ -39,24 +39,21 @@ func (c *Controller) Move(ctx context.Context, session *auth.Session, } // backfill data - if in.PathName == nil { - in.PathName = &space.PathName + if in.UID == nil { + in.UID = &space.UID } if in.ParentID == nil { in.ParentID = &space.ParentID } - // convert name to lower case for easy of api use - *in.PathName = strings.ToLower(*in.PathName) - // verify input - if err = check.PathName(*in.PathName); err != nil { + if err = check.UID(*in.UID); err != nil { return nil, err } // ensure it's not a no-op - if *in.ParentID == space.ParentID && *in.PathName == space.PathName { - return nil, err + if *in.ParentID == space.ParentID && *in.UID == space.UID { + return nil, usererror.ErrNoChange } // Ensure we can create spaces within the target space (using parent space as scope, similar to create) @@ -78,15 +75,15 @@ func (c *Controller) Move(ctx context.Context, session *auth.Session, } /* - * Validate path length (Due to racing conditions we can't be 100% sure on the path here only best + * Validate path depth (Due to racing conditions we can't be 100% sure on the path here only best * effort to avoid big transaction failure) * Only needed if we actually change the parent (and can skip top level, as we already validate the name) */ - path := paths.Concatinate(newParent.Path, *in.PathName) - if err = check.Path(path, true); err != nil { + path := paths.Concatinate(newParent.Path, *in.UID) + if err = check.PathDepth(path, true); err != nil { return nil, err } } - return c.spaceStore.Move(ctx, session.Principal.ID, space.ID, *in.ParentID, *in.PathName, in.KeepAsAlias) + return c.spaceStore.Move(ctx, session.Principal.ID, space.ID, *in.ParentID, *in.UID, in.KeepAsAlias) } diff --git a/internal/api/controller/space/update.go b/internal/api/controller/space/update.go index 5b44aba0b..5ffe5c1b6 100644 --- a/internal/api/controller/space/update.go +++ b/internal/api/controller/space/update.go @@ -11,13 +11,11 @@ import ( apiauth "github.com/harness/gitness/internal/api/auth" "github.com/harness/gitness/internal/auth" "github.com/harness/gitness/types" - "github.com/harness/gitness/types/check" "github.com/harness/gitness/types/enum" ) // UpdateInput is used for updating a space. type UpdateInput struct { - Name *string `json:"name"` Description *string `json:"description"` IsPublic *bool `json:"isPublic"` } @@ -37,9 +35,6 @@ func (c *Controller) Update(ctx context.Context, session *auth.Session, } // update values only if provided - if in.Name != nil { - space.Name = *in.Name - } if in.Description != nil { space.Description = *in.Description } @@ -51,7 +46,7 @@ func (c *Controller) Update(ctx context.Context, session *auth.Session, space.Updated = time.Now().UnixMilli() // ensure provided values are valid - if err = check.Space(space); err != nil { + if err = c.spaceCheck(space); err != nil { return nil, err } diff --git a/internal/api/controller/space/wire.go b/internal/api/controller/space/wire.go index 9a5b9eef5..52c195f32 100644 --- a/internal/api/controller/space/wire.go +++ b/internal/api/controller/space/wire.go @@ -8,6 +8,7 @@ import ( "github.com/google/wire" "github.com/harness/gitness/internal/auth/authz" "github.com/harness/gitness/internal/store" + "github.com/harness/gitness/types/check" ) // WireSet provides a wire set for this package. @@ -15,7 +16,7 @@ var WireSet = wire.NewSet( NewController, ) -func ProvideController(authorizer authz.Authorizer, spaceStore store.SpaceStore, +func ProvideController(spaceCheck check.Space, authorizer authz.Authorizer, spaceStore store.SpaceStore, repoStore store.RepoStore, saStore store.ServiceAccountStore) *Controller { - return NewController(authorizer, spaceStore, repoStore, saStore) + return NewController(spaceCheck, authorizer, spaceStore, repoStore, saStore) } diff --git a/internal/api/controller/user/controller.go b/internal/api/controller/user/controller.go index f68117b5b..5ecc88a9c 100644 --- a/internal/api/controller/user/controller.go +++ b/internal/api/controller/user/controller.go @@ -7,17 +7,20 @@ package user import ( "github.com/harness/gitness/internal/auth/authz" "github.com/harness/gitness/internal/store" + "github.com/harness/gitness/types/check" ) type Controller struct { + userCheck check.User authorizer authz.Authorizer userStore store.UserStore tokenStore store.TokenStore } -func NewController(authorizer authz.Authorizer, userStore store.UserStore, +func NewController(userCheck check.User, authorizer authz.Authorizer, userStore store.UserStore, tokenStore store.TokenStore) *Controller { return &Controller{ + userCheck: userCheck, authorizer: authorizer, userStore: userStore, tokenStore: tokenStore, diff --git a/internal/api/controller/user/create.go b/internal/api/controller/user/create.go index e13cfa372..0a0657379 100644 --- a/internal/api/controller/user/create.go +++ b/internal/api/controller/user/create.go @@ -21,10 +21,10 @@ import ( // CreateInput is the input used for create operations. // On purpose don't expose admin, has to be enabled explicitly. type CreateInput struct { - UID string `json:"uid"` - Name string `json:"name"` - Email string `json:"email"` - Password string `json:"password"` + UID string `json:"uid"` + Email string `json:"email"` + DisplayName string `json:"displayName"` + Password string `json:"password"` } /* @@ -61,18 +61,18 @@ func (c *Controller) CreateNoAuth(ctx context.Context, in *CreateInput, admin bo } user := &types.User{ - UID: in.UID, - Name: in.Name, - Email: in.Email, - Password: string(hash), - Salt: uniuri.NewLen(uniuri.UUIDLen), - Created: time.Now().UnixMilli(), - Updated: time.Now().UnixMilli(), - Admin: admin, + UID: in.UID, + DisplayName: in.DisplayName, + Email: in.Email, + Password: string(hash), + Salt: uniuri.NewLen(uniuri.UUIDLen), + Created: time.Now().UnixMilli(), + Updated: time.Now().UnixMilli(), + Admin: admin, } // validate user - if err = check.User(user); err != nil { + if err = c.userCheck(user); err != nil { return nil, err } diff --git a/internal/api/controller/user/create_access_token.go b/internal/api/controller/user/create_access_token.go index 4c81a3d77..62bd5ca7c 100644 --- a/internal/api/controller/user/create_access_token.go +++ b/internal/api/controller/user/create_access_token.go @@ -17,7 +17,7 @@ import ( ) type CreateTokenInput struct { - Name string `json:"name"` + UID string `json:"uid"` Lifetime time.Duration `json:"lifetime"` Grants enum.AccessGrant `json:"grants"` } @@ -37,15 +37,18 @@ func (c *Controller) CreateAccessToken(ctx context.Context, session *auth.Sessio return nil, err } - if err = check.Name(in.Name); err != nil { + if err = check.UID(in.UID); err != nil { return nil, err } if err = check.TokenLifetime(in.Lifetime); err != nil { return nil, err } + if err = check.AccessGrant(in.Grants, false); err != nil { + return nil, err + } token, jwtToken, err := token.CreatePAT(ctx, c.tokenStore, &session.Principal, - user, in.Name, in.Lifetime, in.Grants) + user, in.UID, in.Lifetime, in.Grants) if err != nil { return nil, err } diff --git a/internal/api/controller/user/delete_token.go b/internal/api/controller/user/delete_token.go index 36dd1bf17..e881417ee 100644 --- a/internal/api/controller/user/delete_token.go +++ b/internal/api/controller/user/delete_token.go @@ -18,7 +18,7 @@ import ( * DeleteToken deletes a token of a user. */ func (c *Controller) DeleteToken(ctx context.Context, session *auth.Session, - userUID string, tokenType enum.TokenType, tokenID int64) error { + userUID string, tokenType enum.TokenType, tokenUID string) error { user, err := findUserFromUID(ctx, c.userStore, userUID) if err != nil { return err @@ -29,7 +29,7 @@ func (c *Controller) DeleteToken(ctx context.Context, session *auth.Session, return err } - token, err := c.tokenStore.Find(ctx, tokenID) + token, err := c.tokenStore.FindByUID(ctx, user.ID, tokenUID) if err != nil { return err } @@ -48,5 +48,5 @@ func (c *Controller) DeleteToken(ctx context.Context, session *auth.Session, return usererror.ErrNotFound } - return c.tokenStore.Delete(ctx, tokenID) + return c.tokenStore.Delete(ctx, token.ID) } diff --git a/internal/api/controller/user/login.go b/internal/api/controller/user/login.go index a80f2388d..d1ee0b359 100644 --- a/internal/api/controller/user/login.go +++ b/internal/api/controller/user/login.go @@ -6,7 +6,11 @@ package user import ( "context" + "crypto/rand" "errors" + "fmt" + "math/big" + "time" "github.com/harness/gitness/internal/api/usererror" "github.com/harness/gitness/internal/auth" @@ -49,11 +53,22 @@ func (c *Controller) Login(ctx context.Context, session *auth.Session, return nil, usererror.ErrNotFound } - // TODO: how should we name session tokens? - token, jwtToken, err := token.CreateUserSession(ctx, c.tokenStore, user, "login") + tokenUID, err := generateSessionTokenUID() + if err != nil { + return nil, err + } + token, jwtToken, err := token.CreateUserSession(ctx, c.tokenStore, user, tokenUID) if err != nil { return nil, err } return &types.TokenResponse{Token: *token, AccessToken: jwtToken}, nil } + +func generateSessionTokenUID() (string, error) { + r, err := rand.Int(rand.Reader, big.NewInt(10000)) + if err != nil { + return "", fmt.Errorf("failed to generate random number: %w", err) + } + return fmt.Sprintf("login-%d-%04d", time.Now().Unix(), r.Int64()), nil +} diff --git a/internal/api/controller/user/update.go b/internal/api/controller/user/update.go index 04b1f1b57..48606d3a0 100644 --- a/internal/api/controller/user/update.go +++ b/internal/api/controller/user/update.go @@ -13,16 +13,15 @@ import ( apiauth "github.com/harness/gitness/internal/api/auth" "github.com/harness/gitness/internal/auth" "github.com/harness/gitness/types" - "github.com/harness/gitness/types/check" "github.com/harness/gitness/types/enum" "golang.org/x/crypto/bcrypt" ) // UpdateInput store infos to update an existing user. type UpdateInput struct { - Email *string `json:"email"` - Password *string `json:"password"` - Name *string `json:"name"` + Email *string `json:"email"` + Password *string `json:"password"` + DisplayName *string `json:"displayName"` } /* @@ -40,8 +39,8 @@ func (c *Controller) Update(ctx context.Context, session *auth.Session, return nil, err } - if in.Name != nil { - user.Name = ptr.ToString(in.Name) + if in.DisplayName != nil { + user.DisplayName = ptr.ToString(in.DisplayName) } if in.Email != nil { user.Email = ptr.ToString(in.Email) @@ -57,7 +56,7 @@ func (c *Controller) Update(ctx context.Context, session *auth.Session, user.Updated = time.Now().UnixMilli() // validate user - if err = check.User(user); err != nil { + if err = c.userCheck(user); err != nil { return nil, err } diff --git a/internal/api/controller/user/wire.go b/internal/api/controller/user/wire.go index ef5b5f37c..7293d721b 100644 --- a/internal/api/controller/user/wire.go +++ b/internal/api/controller/user/wire.go @@ -8,6 +8,7 @@ import ( "github.com/google/wire" "github.com/harness/gitness/internal/auth/authz" "github.com/harness/gitness/internal/store" + "github.com/harness/gitness/types/check" ) // WireSet provides a wire set for this package. @@ -15,7 +16,7 @@ var WireSet = wire.NewSet( NewController, ) -func ProvideController(authorizer authz.Authorizer, userStore store.UserStore, +func ProvideController(userCheck check.User, authorizer authz.Authorizer, userStore store.UserStore, tokenStore store.TokenStore) *Controller { - return NewController(authorizer, userStore, tokenStore) + return NewController(userCheck, authorizer, userStore, tokenStore) } diff --git a/internal/api/handler/account/register.go b/internal/api/handler/account/register.go index 5ff5bdd64..07508d277 100644 --- a/internal/api/handler/account/register.go +++ b/internal/api/handler/account/register.go @@ -18,10 +18,10 @@ func HandleRegister(userCtrl *user.Controller) http.HandlerFunc { ctx := r.Context() in := &user.CreateInput{ - UID: r.FormValue("username"), - Name: r.FormValue("name"), - Email: r.FormValue("email"), - Password: r.FormValue("password"), + UID: r.FormValue("username"), + DisplayName: r.FormValue("displayname"), + Email: r.FormValue("email"), + Password: r.FormValue("password"), } tokenResponse, err := userCtrl.Register(ctx, in) diff --git a/internal/api/handler/serviceaccount/delete_token.go b/internal/api/handler/serviceaccount/delete_token.go index 96bd05aee..2b35e0395 100644 --- a/internal/api/handler/serviceaccount/delete_token.go +++ b/internal/api/handler/serviceaccount/delete_token.go @@ -23,13 +23,13 @@ func HandleDeleteToken(saCrl *serviceaccount.Controller) http.HandlerFunc { render.TranslatedUserError(w, err) return } - tokenID, err := request.GetTokenIDFromPath(r) + tokenUID, err := request.GetTokenUIDFromPath(r) if err != nil { render.BadRequest(w) return } - err = saCrl.DeleteToken(ctx, session, saUID, tokenID) + err = saCrl.DeleteToken(ctx, session, saUID, tokenUID) if err != nil { render.TranslatedUserError(w, err) return diff --git a/internal/api/handler/user/delete_token.go b/internal/api/handler/user/delete_token.go index 11634bc49..b13923bdf 100644 --- a/internal/api/handler/user/delete_token.go +++ b/internal/api/handler/user/delete_token.go @@ -21,13 +21,13 @@ func HandleDeleteToken(userCtrl *user.Controller, tokenType enum.TokenType) http session, _ := request.AuthSessionFrom(ctx) userUID := session.Principal.UID - tokenID, err := request.GetTokenIDFromPath(r) + tokenUID, err := request.GetTokenUIDFromPath(r) if err != nil { render.BadRequest(w) return } - err = userCtrl.DeleteToken(ctx, session, userUID, tokenType, tokenID) + err = userCtrl.DeleteToken(ctx, session, userUID, tokenType, tokenUID) if err != nil { render.TranslatedUserError(w, err) return diff --git a/internal/api/openapi/space.go b/internal/api/openapi/space.go index 6ae05e581..e9d614e7f 100644 --- a/internal/api/openapi/space.go +++ b/internal/api/openapi/space.go @@ -53,11 +53,10 @@ var queryParameterSortRepo = openapi3.ParameterOrRef{ Schema: &openapi3.SchemaOrRef{ Schema: &openapi3.Schema{ Type: ptrSchemaType(openapi3.SchemaTypeString), - Default: ptrptr(enum.RepoAttrName.String()), + Default: ptrptr(enum.RepoAttrUID.String()), Enum: []interface{}{ - ptr.String(enum.RepoAttrName.String()), + ptr.String(enum.RepoAttrUID.String()), ptr.String(enum.RepoAttrPath.String()), - ptr.String(enum.RepoAttrPathName.String()), ptr.String(enum.RepoAttrCreated.String()), ptr.String(enum.RepoAttrUpdated.String()), }, @@ -89,11 +88,10 @@ var queryParameterSortSpace = openapi3.ParameterOrRef{ Schema: &openapi3.SchemaOrRef{ Schema: &openapi3.Schema{ Type: ptrSchemaType(openapi3.SchemaTypeString), - Default: ptrptr(enum.SpaceAttrName.String()), + Default: ptrptr(enum.SpaceAttrUID.String()), Enum: []interface{}{ - ptr.String(enum.SpaceAttrName.String()), + ptr.String(enum.SpaceAttrUID.String()), ptr.String(enum.SpaceAttrPath.String()), - ptr.String(enum.SpaceAttrPathName.String()), ptr.String(enum.SpaceAttrCreated.String()), ptr.String(enum.SpaceAttrUpdated.String()), }, diff --git a/internal/api/openapi/user.go b/internal/api/openapi/user.go index ffa5ced29..d1e082f7d 100644 --- a/internal/api/openapi/user.go +++ b/internal/api/openapi/user.go @@ -7,6 +7,7 @@ package openapi import ( "net/http" + "github.com/harness/gitness/internal/api/controller/user" "github.com/harness/gitness/internal/api/usererror" "github.com/harness/gitness/types" @@ -32,7 +33,7 @@ func buildUser(reflector *openapi3.Reflector) { opUpdate := openapi3.Operation{} opUpdate.WithTags("user") opUpdate.WithMapOfAnything(map[string]interface{}{"operationId": "updateUser"}) - _ = reflector.SetRequest(&opUpdate, new(types.UserInput), http.MethodPatch) + _ = reflector.SetRequest(&opUpdate, new(user.UpdateInput), http.MethodPatch) _ = reflector.SetJSONResponse(&opUpdate, new(types.User), http.StatusOK) _ = reflector.SetJSONResponse(&opUpdate, new(usererror.Error), http.StatusInternalServerError) _ = reflector.Spec.AddOperation(http.MethodPatch, "/user", opUpdate) diff --git a/internal/api/openapi/users.go b/internal/api/openapi/users.go index 96df85547..e72c54dd9 100644 --- a/internal/api/openapi/users.go +++ b/internal/api/openapi/users.go @@ -7,6 +7,7 @@ package openapi import ( "net/http" + "github.com/harness/gitness/internal/api/controller/user" "github.com/harness/gitness/internal/api/usererror" "github.com/harness/gitness/types" @@ -27,14 +28,6 @@ type ( // include pagination request paginationRequest } - - // request for updating a user. - userUpdateRequest struct { - Param string `path:"email"` - - // include request body input. - types.UserInput - } ) // helper function that constructs the openapi specification @@ -73,7 +66,7 @@ func buildUsers(reflector *openapi3.Reflector) { opUpdate := openapi3.Operation{} opUpdate.WithTags("users") opUpdate.WithMapOfAnything(map[string]interface{}{"operationId": "updateUsers"}) - _ = reflector.SetRequest(&opUpdate, new(userUpdateRequest), http.MethodPatch) + _ = reflector.SetRequest(&opUpdate, new(user.UpdateInput), http.MethodPatch) _ = reflector.SetJSONResponse(&opUpdate, new(types.User), http.StatusOK) _ = reflector.SetJSONResponse(&opUpdate, new(usererror.Error), http.StatusBadRequest) _ = reflector.SetJSONResponse(&opUpdate, new(usererror.Error), http.StatusInternalServerError) diff --git a/internal/api/request/repo.go b/internal/api/request/repo.go index de10e78e8..9dc1dabd5 100644 --- a/internal/api/request/repo.go +++ b/internal/api/request/repo.go @@ -7,7 +7,6 @@ package request import ( "net/http" "net/url" - "strings" ) const ( @@ -20,7 +19,6 @@ func GetRepoRefFromPath(r *http.Request) (string, error) { return "", err } - // paths are unescaped and lower - ref, err := url.PathUnescape(rawRef) - return strings.ToLower(ref), err + // paths are unescaped + return url.PathUnescape(rawRef) } diff --git a/internal/api/request/space.go b/internal/api/request/space.go index ffe8ccf60..662991aad 100644 --- a/internal/api/request/space.go +++ b/internal/api/request/space.go @@ -7,7 +7,6 @@ package request import ( "net/http" "net/url" - "strings" ) const ( @@ -21,6 +20,5 @@ func GetSpaceRefFromPath(r *http.Request) (string, error) { } // paths are unescaped and lower - ref, err := url.PathUnescape(rawRef) - return strings.ToLower(ref), err + return url.PathUnescape(rawRef) } diff --git a/internal/api/request/token.go b/internal/api/request/token.go index 00ac98411..c9e15815e 100644 --- a/internal/api/request/token.go +++ b/internal/api/request/token.go @@ -9,9 +9,9 @@ import ( ) const ( - PathParamTokenID = "tokenID" + PathParamTokenUID = "tokenUID" ) -func GetTokenIDFromPath(r *http.Request) (int64, error) { - return PathParamAsInt64(r, PathParamTokenID) +func GetTokenUIDFromPath(r *http.Request) (string, error) { + return PathParamOrError(r, PathParamTokenUID) } diff --git a/internal/api/usererror/usererror.go b/internal/api/usererror/usererror.go index cb6f6f0a7..325637a45 100644 --- a/internal/api/usererror/usererror.go +++ b/internal/api/usererror/usererror.go @@ -14,7 +14,7 @@ var ( ErrInvalidToken = New(http.StatusUnauthorized, "Invalid or missing token") // ErrBadRequest is returned when there was an issue with the input. - ErrBadRequest = New(http.StatusBadGateway, "Bad Request") + ErrBadRequest = New(http.StatusBadRequest, "Bad Request") // ErrUnauthorized is returned when the acting principal is not authenticated. ErrUnauthorized = New(http.StatusUnauthorized, "Unauthorized") diff --git a/internal/bootstrap/bootstrap.go b/internal/bootstrap/bootstrap.go index 32b63d69f..94abf658e 100644 --- a/internal/bootstrap/bootstrap.go +++ b/internal/bootstrap/bootstrap.go @@ -34,7 +34,7 @@ func System(config *types.Config, userCtrl *user.Controller) func(context.Contex // but then the duplicte might be due to email or name, not uid. // Futhermore, it would create unnecesary error logs. func Admin(ctx context.Context, config *types.Config, userCtrl *user.Controller) error { - if config.Admin.Name == "" { + if config.Admin.DisplayName == "" { return nil } @@ -44,10 +44,10 @@ func Admin(ctx context.Context, config *types.Config, userCtrl *user.Controller) } in := &user.CreateInput{ - UID: adminUID, - Name: config.Admin.Name, - Email: config.Admin.Email, - Password: config.Admin.Password, + UID: adminUID, + DisplayName: config.Admin.DisplayName, + Email: config.Admin.Email, + Password: config.Admin.Password, } // create user as admin diff --git a/internal/paths/paths.go b/internal/paths/paths.go index bb3560b40..2e3f35ddd 100644 --- a/internal/paths/paths.go +++ b/internal/paths/paths.go @@ -15,9 +15,9 @@ var ( ErrPathEmpty = errors.New("path is empty") ) -// Disect splits a path into its parent path and the leaf name -// e.g. /space1/space2/space3 -> (/space1/space2, space3, nil). -func Disect(path string) (string, string, error) { +// DisectLeaf splits a path into its parent path and the leaf name +// e.g. space1/space2/space3 -> (space1/space2, space3, nil). +func DisectLeaf(path string) (string, string, error) { if path == "" { return "", "", ErrPathEmpty } @@ -30,6 +30,21 @@ func Disect(path string) (string, string, error) { return path[:i], path[i+1:], nil } +// DisectRoot splits a path into its root space and sub-path +// e.g. space1/space2/space3 -> (space1, space2/space3, nil). +func DisectRoot(path string) (string, string, error) { + if path == "" { + return "", "", ErrPathEmpty + } + + i := strings.Index(path, types.PathSeparator) + if i == -1 { + return path, "", nil + } + + return path[:i], path[i+1:], nil +} + /* * Concatinate two paths together (takes care of leading / trailing '/') * e.g. (space1/, /space2/) -> space1/space2 diff --git a/internal/router/api.go b/internal/router/api.go index 87f5cdbc4..60e18b82c 100644 --- a/internal/router/api.go +++ b/internal/router/api.go @@ -203,7 +203,7 @@ func setupUsers(r chi.Router, userCtrl *user.Controller) { r.Post("/", handleruser.HandleCreateAccessToken(userCtrl)) // per token operations - r.Route(fmt.Sprintf("/{%s}", request.PathParamTokenID), func(r chi.Router) { + r.Route(fmt.Sprintf("/{%s}", request.PathParamTokenUID), func(r chi.Router) { r.Delete("/", handleruser.HandleDeleteToken(userCtrl, enum.TokenTypePAT)) }) }) @@ -213,7 +213,7 @@ func setupUsers(r chi.Router, userCtrl *user.Controller) { r.Get("/", handleruser.HandleListTokens(userCtrl, enum.TokenTypeSession)) // per token operations - r.Route(fmt.Sprintf("/{%s}", request.PathParamTokenID), func(r chi.Router) { + r.Route(fmt.Sprintf("/{%s}", request.PathParamTokenUID), func(r chi.Router) { r.Delete("/", handleruser.HandleDeleteToken(userCtrl, enum.TokenTypeSession)) }) }) @@ -235,7 +235,7 @@ func setupServiceAccounts(r chi.Router, saCtrl *serviceaccount.Controller) { r.Post("/", handlerserviceaccount.HandleCreateToken(saCtrl)) // per token operations - r.Route(fmt.Sprintf("/{%s}", request.PathParamTokenID), func(r chi.Router) { + r.Route(fmt.Sprintf("/{%s}", request.PathParamTokenUID), func(r chi.Router) { r.Delete("/", handlerserviceaccount.HandleDeleteToken(saCtrl)) }) }) diff --git a/internal/router/git.go b/internal/router/git.go index 95138f6aa..a5c92af14 100644 --- a/internal/router/git.go +++ b/internal/router/git.go @@ -114,7 +114,7 @@ func BasicAuth(realm string, auth authn.Authenticator, repoStore store.RepoStore } if !repository.IsPublic { - log.Debug().Msgf("BasicAuth middleware: repo %v is private", repository.Name) + log.Debug().Msgf("BasicAuth middleware: repo %v is private", repository.UID) _, err = auth.Authenticate(r) if err != nil { basicAuthFailed(w, realm) @@ -151,7 +151,7 @@ func stubGitHandler(repoStore store.RepoStore) http.HandlerFunc { " Method: '%s'\n"+ " Path: '%s'\n"+ " Query: '%s'", - repo.Name, repo.Path, + repo.UID, repo.Path, r.Method, r.URL.Path, r.URL.RawQuery, diff --git a/internal/store/database/migrate/postgres/0001_create_table_paths.up.sql b/internal/store/database/migrate/postgres/0001_create_table_paths.up.sql index ac02a3a27..da0ccfead 100644 --- a/internal/store/database/migrate/postgres/0001_create_table_paths.up.sql +++ b/internal/store/database/migrate/postgres/0001_create_table_paths.up.sql @@ -1,11 +1,12 @@ CREATE TABLE IF NOT EXISTS paths ( path_id SERIAL PRIMARY KEY ,path_value TEXT +,path_valueUnique TEXT ,path_isAlias BOOLEAN ,path_targetType TEXT ,path_targetId INTEGER ,path_createdBy INTEGER ,path_created BIGINT ,path_updated BIGINT -,UNIQUE(path_value) +,UNIQUE(path_valueUnique) ); \ No newline at end of file diff --git a/internal/store/database/migrate/postgres/0001_create_table_principals.up.sql b/internal/store/database/migrate/postgres/0001_create_table_principals.up.sql index 378f38eb3..1da4f0029 100644 --- a/internal/store/database/migrate/postgres/0001_create_table_principals.up.sql +++ b/internal/store/database/migrate/postgres/0001_create_table_principals.up.sql @@ -1,21 +1,22 @@ CREATE TABLE IF NOT EXISTS principals ( principal_id SERIAL PRIMARY KEY ,principal_uid TEXT +,principal_uidUnique TEXT +,principal_email TEXT ,principal_type TEXT -,principal_name TEXT +,principal_displayName TEXT ,principal_admin BOOLEAN ,principal_blocked BOOLEAN ,principal_salt TEXT ,principal_created BIGINT ,principal_updated BIGINT -,principal_user_email CITEXT ,principal_user_password TEXT ,principal_sa_parentType TEXT ,principal_sa_parentId INTEGER -,UNIQUE(principal_uid) +,UNIQUE(principal_uidUnique) +,UNIQUE(principal_email) ,UNIQUE(principal_salt) -,UNIQUE(principal_user_email) ); diff --git a/internal/store/database/migrate/postgres/0001_create_table_repositories.up.sql b/internal/store/database/migrate/postgres/0001_create_table_repositories.up.sql index ff51b4b6b..09720a348 100644 --- a/internal/store/database/migrate/postgres/0001_create_table_repositories.up.sql +++ b/internal/store/database/migrate/postgres/0001_create_table_repositories.up.sql @@ -1,8 +1,7 @@ CREATE TABLE IF NOT EXISTS repositories ( repo_id SERIAL PRIMARY KEY -,repo_pathName TEXT -,repo_spaceId INTEGER -,repo_name TEXT +,repo_parentId INTEGER +,repo_uid TEXT ,repo_description TEXT ,repo_isPublic BOOLEAN ,repo_createdBy INTEGER diff --git a/internal/store/database/migrate/postgres/0001_create_table_spaces.up.sql b/internal/store/database/migrate/postgres/0001_create_table_spaces.up.sql index a688a083c..b6cbdbafd 100644 --- a/internal/store/database/migrate/postgres/0001_create_table_spaces.up.sql +++ b/internal/store/database/migrate/postgres/0001_create_table_spaces.up.sql @@ -1,8 +1,7 @@ CREATE TABLE IF NOT EXISTS spaces ( space_id SERIAL PRIMARY KEY -,space_pathName TEXT ,space_parentId INTEGER -,space_name TEXT +,space_uid TEXT ,space_description TEXT ,space_isPublic BOOLEAN ,space_createdBy INTEGER diff --git a/internal/store/database/migrate/postgres/0001_create_table_tokens.up.sql b/internal/store/database/migrate/postgres/0001_create_table_tokens.up.sql index 5b841ab91..a3ef021d7 100644 --- a/internal/store/database/migrate/postgres/0001_create_table_tokens.up.sql +++ b/internal/store/database/migrate/postgres/0001_create_table_tokens.up.sql @@ -1,10 +1,11 @@ CREATE TABLE IF NOT EXISTS tokens ( token_id SERIAL PRIMARY KEY ,token_type TEXT -,token_name TEXT +,token_uid TEXT ,token_principalId INTEGER ,token_expiresAt BIGINT ,token_grants BIGINT ,token_issuedAt BIGINT ,token_createdBy INTEGER +,UNIQUE(token_principalId, token_uid) ); diff --git a/internal/store/database/migrate/postgres/0002_create_index_repositories_parentId.up.sql b/internal/store/database/migrate/postgres/0002_create_index_repositories_parentId.up.sql new file mode 100644 index 000000000..20e8fb516 --- /dev/null +++ b/internal/store/database/migrate/postgres/0002_create_index_repositories_parentId.up.sql @@ -0,0 +1,2 @@ +CREATE INDEX IF NOT EXISTS index_repositories_parentId +ON repositories(repo_parentId); diff --git a/internal/store/database/migrate/postgres/0002_create_index_repositories_spaceId.up.sql b/internal/store/database/migrate/postgres/0002_create_index_repositories_spaceId.up.sql deleted file mode 100644 index d085246cd..000000000 --- a/internal/store/database/migrate/postgres/0002_create_index_repositories_spaceId.up.sql +++ /dev/null @@ -1,2 +0,0 @@ -CREATE INDEX IF NOT EXISTS index_repositories_spaceId -ON repositories(repo_spaceId); diff --git a/internal/store/database/migrate/sqlite/0001_create_table_paths.up.sql b/internal/store/database/migrate/sqlite/0001_create_table_paths.up.sql index c7a098f5d..4d51f1ab7 100644 --- a/internal/store/database/migrate/sqlite/0001_create_table_paths.up.sql +++ b/internal/store/database/migrate/sqlite/0001_create_table_paths.up.sql @@ -1,11 +1,12 @@ CREATE TABLE IF NOT EXISTS paths ( path_id INTEGER PRIMARY KEY AUTOINCREMENT -,path_value TEXT COLLATE NOCASE +,path_value TEXT +,path_valueUnique TEXT ,path_isAlias BOOLEAN ,path_targetType TEXT ,path_targetId INTEGER ,path_createdBy INTEGER ,path_created BIGINT ,path_updated BIGINT -,UNIQUE(path_value COLLATE NOCASE) +,UNIQUE(path_valueUnique) ); \ No newline at end of file diff --git a/internal/store/database/migrate/sqlite/0001_create_table_principals.up.sql b/internal/store/database/migrate/sqlite/0001_create_table_principals.up.sql index d6c71406a..ae31c496d 100644 --- a/internal/store/database/migrate/sqlite/0001_create_table_principals.up.sql +++ b/internal/store/database/migrate/sqlite/0001_create_table_principals.up.sql @@ -1,21 +1,22 @@ CREATE TABLE IF NOT EXISTS principals ( principal_id INTEGER PRIMARY KEY AUTOINCREMENT ,principal_uid TEXT -,principal_type TEXT -,principal_name TEXT +,principal_uidUnique TEXT +,principal_email TEXT COLLATE NOCASE +,principal_type TEXT COLLATE NOCASE +,principal_displayName TEXT ,principal_admin BOOLEAN ,principal_blocked BOOLEAN ,principal_salt TEXT ,principal_created BIGINT ,principal_updated BIGINT -,principal_user_email TEXT ,principal_user_password TEXT -,principal_sa_parentType TEXT +,principal_sa_parentType TEXT COLLATE NOCASE ,principal_sa_parentId INTEGER -,UNIQUE(principal_uid) +,UNIQUE(principal_uidUnique) +,UNIQUE(principal_email COLLATE NOCASE) ,UNIQUE(principal_salt) -,UNIQUE(principal_user_email COLLATE NOCASE) ); diff --git a/internal/store/database/migrate/sqlite/0001_create_table_repositories.up.sql b/internal/store/database/migrate/sqlite/0001_create_table_repositories.up.sql index c0c3f154e..8423b1813 100644 --- a/internal/store/database/migrate/sqlite/0001_create_table_repositories.up.sql +++ b/internal/store/database/migrate/sqlite/0001_create_table_repositories.up.sql @@ -1,8 +1,7 @@ CREATE TABLE IF NOT EXISTS repositories ( repo_id INTEGER PRIMARY KEY AUTOINCREMENT -,repo_pathName TEXT COLLATE NOCASE -,repo_spaceId INTEGER -,repo_name TEXT +,repo_parentId INTEGER +,repo_uid TEXT ,repo_description TEXT ,repo_isPublic BOOLEAN ,repo_createdBy INTEGER diff --git a/internal/store/database/migrate/sqlite/0001_create_table_spaces.up.sql b/internal/store/database/migrate/sqlite/0001_create_table_spaces.up.sql index bc4e71f89..2aa7547d0 100644 --- a/internal/store/database/migrate/sqlite/0001_create_table_spaces.up.sql +++ b/internal/store/database/migrate/sqlite/0001_create_table_spaces.up.sql @@ -1,8 +1,7 @@ CREATE TABLE IF NOT EXISTS spaces ( space_id INTEGER PRIMARY KEY AUTOINCREMENT -,space_pathName TEXT COLLATE NOCASE ,space_parentId INTEGER -,space_name TEXT +,space_uid TEXT ,space_description TEXT ,space_isPublic BOOLEAN ,space_createdBy INTEGER diff --git a/internal/store/database/migrate/sqlite/0001_create_table_tokens.up.sql b/internal/store/database/migrate/sqlite/0001_create_table_tokens.up.sql index f0bb42165..4fe35fca1 100644 --- a/internal/store/database/migrate/sqlite/0001_create_table_tokens.up.sql +++ b/internal/store/database/migrate/sqlite/0001_create_table_tokens.up.sql @@ -1,10 +1,11 @@ CREATE TABLE IF NOT EXISTS tokens ( token_id INTEGER PRIMARY KEY AUTOINCREMENT ,token_type TEXT COLLATE NOCASE -,token_name TEXT +,token_uid TEXT COLLATE NOCASE ,token_principalId INTEGER ,token_expiresAt BIGINT ,token_grants BIGINT ,token_issuedAt BIGINT ,token_createdBy INTEGER +,UNIQUE(token_principalId, token_uid COLLATE NOCASE) ); diff --git a/internal/store/database/migrate/sqlite/0002_create_index_repositories_parent.up.sql b/internal/store/database/migrate/sqlite/0002_create_index_repositories_parent.up.sql new file mode 100644 index 000000000..20e8fb516 --- /dev/null +++ b/internal/store/database/migrate/sqlite/0002_create_index_repositories_parent.up.sql @@ -0,0 +1,2 @@ +CREATE INDEX IF NOT EXISTS index_repositories_parentId +ON repositories(repo_parentId); diff --git a/internal/store/database/migrate/sqlite/0002_create_index_repositories_spaceId.up.sql b/internal/store/database/migrate/sqlite/0002_create_index_repositories_spaceId.up.sql deleted file mode 100644 index d085246cd..000000000 --- a/internal/store/database/migrate/sqlite/0002_create_index_repositories_spaceId.up.sql +++ /dev/null @@ -1,2 +0,0 @@ -CREATE INDEX IF NOT EXISTS index_repositories_spaceId -ON repositories(repo_spaceId); diff --git a/internal/store/database/path.go b/internal/store/database/path.go index b8eb964ad..9d07476ae 100644 --- a/internal/store/database/path.go +++ b/internal/store/database/path.go @@ -7,6 +7,7 @@ package database import ( "context" "fmt" + "strings" "github.com/harness/gitness/internal/paths" "github.com/harness/gitness/internal/store" @@ -18,19 +19,33 @@ import ( "github.com/rs/zerolog/log" ) +// path is a DB representation of a Path. +// It is required to allow storing transformed paths used for uniquness constraints and searching. +type path struct { + types.Path + ValueUnique string `db:"path_valueUnique"` +} + // CreateAliasPath a new alias path (Don't call this for new path creation!) -func CreateAliasPath(ctx context.Context, db *sqlx.DB, path *types.Path) error { +func CreateAliasPath(ctx context.Context, db *sqlx.DB, path *types.Path, + transformation store.PathTransformation) error { if !path.IsAlias { return store.ErrAliasPathRequired } // ensure path length is okay - if check.PathTooLong(path.Value, path.TargetType == enum.PathTargetTypeSpace) { + if check.IsPathTooDeep(path.Value, path.TargetType == enum.PathTargetTypeSpace) { log.Warn().Msgf("Path '%s' is too long.", path.Value) return store.ErrPathTooLong } - query, arg, err := db.BindNamed(pathInsert, path) + // map to db path to ensure we store valueUnique. + dbPath, err := mapToDBPath(path, transformation) + if err != nil { + return fmt.Errorf("failed to map db path: %w", err) + } + + query, arg, err := db.BindNamed(pathInsert, dbPath) if err != nil { return processSQLErrorf(err, "Failed to bind path object") } @@ -43,9 +58,10 @@ func CreateAliasPath(ctx context.Context, db *sqlx.DB, path *types.Path) error { } // CreatePathTx creates a new path as part of a transaction. -func CreatePathTx(ctx context.Context, db *sqlx.DB, tx *sqlx.Tx, path *types.Path) error { +func CreatePathTx(ctx context.Context, db *sqlx.DB, tx *sqlx.Tx, path *types.Path, + transformation store.PathTransformation) error { // ensure path length is okay - if check.PathTooLong(path.Value, path.TargetType == enum.PathTargetTypeSpace) { + if check.IsPathTooDeep(path.Value, path.TargetType == enum.PathTargetTypeSpace) { log.Warn().Msgf("Path '%s' is too long.", path.Value) return store.ErrPathTooLong } @@ -59,7 +75,13 @@ func CreatePathTx(ctx context.Context, db *sqlx.DB, tx *sqlx.Tx, path *types.Pat } } - query, arg, err := db.BindNamed(pathInsert, path) + // map to db path to ensure we store valueUnique. + dbPath, err := mapToDBPath(path, transformation) + if err != nil { + return fmt.Errorf("failed to map db path: %w", err) + } + + query, arg, err := db.BindNamed(pathInsert, dbPath) if err != nil { return processSQLErrorf(err, "Failed to bind path object") } @@ -71,19 +93,38 @@ func CreatePathTx(ctx context.Context, db *sqlx.DB, tx *sqlx.Tx, path *types.Pat return nil } -func CountPrimaryChildPathsTx(ctx context.Context, tx *sqlx.Tx, prefix string) (int64, error) { +func CountPrimaryChildPathsTx(ctx context.Context, tx *sqlx.Tx, prefix string, + transformation store.PathTransformation) (int64, error) { + // map the Value to unique Value before searching! + prefixUnique, err := transformation(prefix) + if err != nil { + // in case we fail to transform, return a not found (as it can't exist in the first place) + log.Ctx(ctx).Debug().Msgf("failed to transform path prefix '%s': %s", prefix, err.Error()) + return 0, store.ErrResourceNotFound + } + var count int64 - err := tx.QueryRowContext(ctx, pathCountPrimaryForPrefix, paths.Concatinate(prefix, "%")).Scan(&count) + err = tx.QueryRowContext(ctx, pathCountPrimaryForPrefixUnique, paths.Concatinate(prefixUnique, "%")).Scan(&count) if err != nil { return 0, processSQLErrorf(err, "Count query failed") } return count, nil } -func ListPrimaryChildPathsTx(ctx context.Context, tx *sqlx.Tx, prefix string) ([]*types.Path, error) { - childs := []*types.Path{} +func listPrimaryChildPathsTx(ctx context.Context, tx *sqlx.Tx, prefix string, + transformation store.PathTransformation) ([]*path, error) { + // map the Value to unique Value before searching! + prefixUnique, err := transformation(prefix) + if err != nil { + // in case we fail to transform, return a not found (as it can't exist in the first place) + log.Ctx(ctx).Debug().Msgf("failed to transform path prefix '%s': %s", prefix, err.Error()) + return nil, store.ErrResourceNotFound + } - if err := tx.SelectContext(ctx, &childs, pathSelectPrimaryForPrefix, paths.Concatinate(prefix, "%")); err != nil { + childs := []*path{} + + if err = tx.SelectContext(ctx, &childs, pathSelectPrimaryForPrefixUnique, + paths.Concatinate(prefixUnique, "%")); err != nil { return nil, processSQLErrorf(err, "Select query failed") } @@ -91,60 +132,87 @@ func ListPrimaryChildPathsTx(ctx context.Context, tx *sqlx.Tx, prefix string) ([ } // ReplacePathTx replaces the path for a target as part of a transaction - keeps the existing as alias if requested. -func ReplacePathTx(ctx context.Context, db *sqlx.DB, tx *sqlx.Tx, path *types.Path, keepAsAlias bool) error { - if path.IsAlias { +func ReplacePathTx(ctx context.Context, db *sqlx.DB, tx *sqlx.Tx, newPath *types.Path, keepAsAlias bool, + transformation store.PathTransformation) error { + if newPath.IsAlias { return store.ErrPrimaryPathRequired } // ensure new path length is okay - if check.PathTooLong(path.Value, path.TargetType == enum.PathTargetTypeSpace) { - log.Warn().Msgf("Path '%s' is too long.", path.Value) + if check.IsPathTooDeep(newPath.Value, newPath.TargetType == enum.PathTargetTypeSpace) { + log.Warn().Msgf("Path '%s' is too long.", newPath.Value) return store.ErrPathTooLong } - // existing is always non-alias (as query filters for IsAlias=0) - existing := new(types.Path) - err := tx.GetContext(ctx, existing, pathSelectPrimaryForTarget, string(path.TargetType), fmt.Sprint(path.TargetID)) + // dbExisting is always non-alias (as query filters for IsAlias=0) + dbExisting := new(path) + err := tx.GetContext(ctx, dbExisting, pathSelectPrimaryForTarget, + string(newPath.TargetType), fmt.Sprint(newPath.TargetID)) if err != nil { return processSQLErrorf(err, "Failed to get the existing primary path") } + // map to db path to ensure we store valueUnique. + dbNew, err := mapToDBPath(newPath, transformation) + if err != nil { + return fmt.Errorf("failed to map db path: %w", err) + } + + // ValueUnique is the same => routing is the same, ensure we don't keep the old as alias (duplicate error) + if dbNew.ValueUnique == dbExisting.ValueUnique { + keepAsAlias = false + } + + // Space specific checks. + if newPath.TargetType == enum.PathTargetTypeSpace { + /* + * IMPORTANT + * To avoid cycles in the primary graph, we have to ensure that the old path isn't a parent of the new path. + * We have to look at the unique path here, as that is used for routing and duplicate detection. + */ + if strings.HasPrefix(dbNew.ValueUnique, dbExisting.ValueUnique+types.PathSeparator) { + return store.ErrIllegalMoveCyclicHierarchy + } + } + // Only look for children if the type can have children - if path.TargetType == enum.PathTargetTypeSpace { - err = replaceChildrenPathsTx(ctx, db, tx, existing, path, keepAsAlias) + if newPath.TargetType == enum.PathTargetTypeSpace { + err = replaceChildrenPathsTx(ctx, db, tx, &dbExisting.Path, newPath, keepAsAlias, transformation) if err != nil { return err } } + // make existing an alias (or delete) + // IMPORTANT: delete before insert as a casing only change in the path is a valid input. + // It's part of a db transaction so it should be okay. + query := pathDeleteID + if keepAsAlias { + query = pathMakeAliasID + } + if _, err = tx.ExecContext(ctx, query, dbExisting.ID); err != nil { + return processSQLErrorf(err, "Failed to mark existing path '%s' as alias (or delete)", dbExisting.Value) + } + // insert the new Path - query, arg, err := db.BindNamed(pathInsert, path) + query, arg, err := db.BindNamed(pathInsert, dbNew) if err != nil { return processSQLErrorf(err, "Failed to bind path object") } _, err = tx.ExecContext(ctx, query, arg...) if err != nil { - return processSQLErrorf(err, "Failed to create new primary path '%s'", path.Value) - } - - // make existing an alias - query = pathDeleteID - if keepAsAlias { - query = pathMakeAliasID - } - if _, err = tx.ExecContext(ctx, query, existing.ID); err != nil { - return processSQLErrorf(err, "Failed to mark existing path '%s' as alias", existing.Value) + return processSQLErrorf(err, "Failed to create new primary path '%s'", newPath.Value) } return nil } func replaceChildrenPathsTx(ctx context.Context, db *sqlx.DB, tx *sqlx.Tx, - existing *types.Path, path *types.Path, keepAsAlias bool) error { - var childPaths []*types.Path + existing *types.Path, updated *types.Path, keepAsAlias bool, transformation store.PathTransformation) error { + var childPaths []*path // get all primary paths that start with the current path before updating (or we can run into recursion) - childPaths, err := ListPrimaryChildPathsTx(ctx, tx, existing.Value) + childPaths, err := listPrimaryChildPathsTx(ctx, tx, existing.Value, transformation) if err != nil { return errors.Wrapf(err, "Failed to get primary child paths for '%s'", existing.Value) } @@ -152,16 +220,16 @@ func replaceChildrenPathsTx(ctx context.Context, db *sqlx.DB, tx *sqlx.Tx, for _, child := range childPaths { // create path with updated path (child already is primary) updatedChild := new(types.Path) - *updatedChild = *child + *updatedChild = child.Path updatedChild.ID = 0 // will be regenerated - updatedChild.Created = path.Created - updatedChild.Updated = path.Updated - updatedChild.CreatedBy = path.CreatedBy - updatedChild.Value = path.Value + updatedChild.Value[len(existing.Value):] + updatedChild.Created = updated.Created + updatedChild.Updated = updated.Updated + updatedChild.CreatedBy = updated.CreatedBy + updatedChild.Value = updated.Value + updatedChild.Value[len(existing.Value):] // ensure new child path length is okay - if check.PathTooLong(updatedChild.Value, path.TargetType == enum.PathTargetTypeSpace) { - log.Warn().Msgf("Path '%s' is too long.", path.Value) + if check.IsPathTooDeep(updatedChild.Value, updated.TargetType == enum.PathTargetTypeSpace) { + log.Warn().Msgf("Path '%s' is too long.", updated.Value) return store.ErrPathTooLong } @@ -170,7 +238,26 @@ func replaceChildrenPathsTx(ctx context.Context, db *sqlx.DB, tx *sqlx.Tx, args []interface{} ) - query, args, err = db.BindNamed(pathInsert, updatedChild) + // make existing child path an alias (or delete) + // IMPORTANT: delete before insert as a casing only change in the original path is a valid input. + // It's part of a db transaction so it should be okay. + query = pathDeleteID + if keepAsAlias { + query = pathMakeAliasID + } + if _, err = tx.ExecContext(ctx, query, child.ID); err != nil { + return processSQLErrorf(err, "Failed to mark existing child path '%s' as alias (or delete)", + updatedChild.Value) + } + + // map to db path to ensure we store valueUnique. + var dbUpdatedChild *path + dbUpdatedChild, err = mapToDBPath(updatedChild, transformation) + if err != nil { + return fmt.Errorf("failed to map db path: %w", err) + } + + query, args, err = db.BindNamed(pathInsert, dbUpdatedChild) if err != nil { return processSQLErrorf(err, "Failed to bind path object") } @@ -178,16 +265,6 @@ func replaceChildrenPathsTx(ctx context.Context, db *sqlx.DB, tx *sqlx.Tx, if _, err = tx.ExecContext(ctx, query, args...); err != nil { return processSQLErrorf(err, "Failed to create new primary child path '%s'", updatedChild.Value) } - - // make current child an alias or delete it - query = pathDeleteID - if keepAsAlias { - query = pathMakeAliasID - } - if _, err = tx.ExecContext(ctx, query, child.ID); err != nil { - return processSQLErrorf(err, "Failed to mark existing child path '%s' as alias", - updatedChild.Value) - } } return nil @@ -195,13 +272,13 @@ func replaceChildrenPathsTx(ctx context.Context, db *sqlx.DB, tx *sqlx.Tx, // FindPathTx finds the primary path for a target. func FindPathTx(ctx context.Context, tx *sqlx.Tx, targetType enum.PathTargetType, targetID int64) (*types.Path, error) { - dst := new(types.Path) + dst := new(path) err := tx.GetContext(ctx, dst, pathSelectPrimaryForTarget, string(targetType), fmt.Sprint(targetID)) if err != nil { return nil, processSQLErrorf(err, "Select query failed") } - return dst, nil + return mapDBPath(dst), nil } // DeletePath deletes a specific path alias (primary can't be deleted, only with delete all). @@ -215,10 +292,11 @@ func DeletePath(ctx context.Context, db *sqlx.DB, id int64) error { }(tx) // ensure path is an alias - dst := new(types.Path) + dst := new(path) if err = tx.GetContext(ctx, dst, pathSelectID, id); err != nil { return processSQLErrorf(err, "Failed to find path with id %d", id) - } else if !dst.IsAlias { + } + if !dst.IsAlias { return store.ErrPrimaryPathCantBeDeleted } @@ -257,7 +335,7 @@ func CountPaths(ctx context.Context, db *sqlx.DB, targetType enum.PathTargetType // ListPaths lists all paths for a target. func ListPaths(ctx context.Context, db *sqlx.DB, targetType enum.PathTargetType, targetID int64, opts *types.PathFilter) ([]*types.Path, error) { - dst := []*types.Path{} + dst := []*path{} // else we construct the sql statement. stmt := builder. Select("*"). @@ -271,7 +349,7 @@ func ListPaths(ctx context.Context, db *sqlx.DB, targetType enum.PathTargetType, // NOTE: string concatenation is safe because the // order attribute is an enum and is not user-defined, // and is therefore not subject to injection attacks. - stmt = stmt.OrderBy("path_value COLLATE NOCASE " + opts.Order.String()) + stmt = stmt.OrderBy("path_value " + opts.Order.String()) case enum.PathAttrCreated: stmt = stmt.OrderBy("path_created " + opts.Order.String()) case enum.PathAttrUpdated: @@ -287,7 +365,7 @@ func ListPaths(ctx context.Context, db *sqlx.DB, targetType enum.PathTargetType, return nil, processSQLErrorf(err, "Customer select query failed") } - return dst, nil + return mapDBPaths(dst), nil } // CountPathsTx counts paths for a target as part of a transaction. @@ -300,10 +378,41 @@ func CountPathsTx(ctx context.Context, tx *sqlx.Tx, targetType enum.PathTargetTy return count, nil } +func mapDBPath(dbPath *path) *types.Path { + return &dbPath.Path +} + +func mapDBPaths(dbPaths []*path) []*types.Path { + res := make([]*types.Path, len(dbPaths)) + for i := range dbPaths { + res[i] = mapDBPath(dbPaths[i]) + } + return res +} + +func mapToDBPath(p *types.Path, transformation store.PathTransformation) (*path, error) { + // path comes from outside. + if p == nil { + return nil, fmt.Errorf("path is nil") + } + + valueUnique, err := transformation(p.Value) + if err != nil { + return nil, fmt.Errorf("failed to transform path: %w", err) + } + dbPath := &path{ + Path: *p, + ValueUnique: valueUnique, + } + + return dbPath, nil +} + const pathBase = ` SELECT path_id ,path_value +,path_valueUnique ,path_isAlias ,path_targetType ,path_targetId @@ -318,8 +427,8 @@ const pathSelectPrimaryForTarget = pathBase + ` WHERE path_targetType = $1 AND path_targetId = $2 AND path_isAlias = 0 ` -const pathSelectPrimaryForPrefix = pathBase + ` -WHERE path_value LIKE $1 AND path_isAlias = 0 +const pathSelectPrimaryForPrefixUnique = pathBase + ` +WHERE path_valueUnique LIKE $1 AND path_isAlias = 0 ` const pathCount = ` @@ -328,15 +437,16 @@ FROM paths WHERE path_targetType = $1 AND path_targetId = $2 ` -const pathCountPrimaryForPrefix = ` +const pathCountPrimaryForPrefixUnique = ` SELECT count(*) FROM paths -WHERE path_value LIKE $1 AND path_isAlias = 0 +WHERE path_valueUnique LIKE $1 AND path_isAlias = 0 ` const pathInsert = ` INSERT INTO paths ( path_value + ,path_valueUnique ,path_isAlias ,path_targetType ,path_targetId @@ -345,6 +455,7 @@ INSERT INTO paths ( ,path_updated ) values ( :path_value + ,:path_valueUnique ,:path_isAlias ,:path_targetType ,:path_targetId diff --git a/internal/store/database/repo.go b/internal/store/database/repo.go index 51b958c70..3367e1746 100644 --- a/internal/store/database/repo.go +++ b/internal/store/database/repo.go @@ -21,13 +21,17 @@ import ( var _ store.RepoStore = (*RepoStore)(nil) // Returns a new RepoStore. -func NewRepoStore(db *sqlx.DB) *RepoStore { - return &RepoStore{db} +func NewRepoStore(db *sqlx.DB, pathTransformation store.PathTransformation) *RepoStore { + return &RepoStore{ + db: db, + pathTransformation: pathTransformation, + } } // Implements a RepoStore backed by a relational database. type RepoStore struct { - db *sqlx.DB + db *sqlx.DB + pathTransformation store.PathTransformation } // Finds the repo by id. @@ -41,8 +45,14 @@ func (s *RepoStore) Find(ctx context.Context, id int64) (*types.Repository, erro // Finds the repo by path. func (s *RepoStore) FindByPath(ctx context.Context, path string) (*types.Repository, error) { + // ensure we transform path before searching (otherwise casing might be wrong) + pathUnique, err := s.pathTransformation(path) + if err != nil { + return nil, fmt.Errorf("failed to transform path '%s': %w", path, err) + } + dst := new(types.Repository) - if err := s.db.GetContext(ctx, dst, repoSelectByPath, path); err != nil { + if err = s.db.GetContext(ctx, dst, repoSelectByPathUnique, pathUnique); err != nil { return nil, processSQLErrorf(err, "Select query failed") } return dst, nil @@ -69,13 +79,13 @@ func (s *RepoStore) Create(ctx context.Context, repo *types.Repository) error { } // Get parent path (repo always has a parent) - parentPath, err := FindPathTx(ctx, tx, enum.PathTargetTypeSpace, repo.SpaceID) + parentPath, err := FindPathTx(ctx, tx, enum.PathTargetTypeSpace, repo.ParentID) if err != nil { return errors.Wrap(err, "Failed to find path of parent space") } - // all existing paths are valid, repo name is assumed to be valid. - path := paths.Concatinate(parentPath.Value, repo.PathName) + // All existing paths are valid, repo uid is assumed to be valid => new path is valid + path := paths.Concatinate(parentPath.Value, repo.UID) // create path only once we know the id of the repo p := &types.Path{ @@ -88,7 +98,7 @@ func (s *RepoStore) Create(ctx context.Context, repo *types.Repository) error { Updated: repo.Updated, } - if err = CreatePathTx(ctx, s.db, tx, p); err != nil { + if err = CreatePathTx(ctx, s.db, tx, p, s.pathTransformation); err != nil { return errors.Wrap(err, "Failed to create primary path of repo") } @@ -104,7 +114,7 @@ func (s *RepoStore) Create(ctx context.Context, repo *types.Repository) error { } // Move moves an existing space. -func (s *RepoStore) Move(ctx context.Context, principalID int64, repoID int64, newSpaceID int64, newName string, +func (s *RepoStore) Move(ctx context.Context, principalID int64, repoID int64, newParentID int64, newName string, keepAsAlias bool) (*types.Repository, error) { tx, err := s.db.BeginTxx(ctx, nil) if err != nil { @@ -121,12 +131,14 @@ func (s *RepoStore) Move(ctx context.Context, principalID int64, repoID int64, n } // get path of new parent space - spacePath, err := FindPathTx(ctx, tx, enum.PathTargetTypeSpace, newSpaceID) + spacePath, err := FindPathTx(ctx, tx, enum.PathTargetTypeSpace, newParentID) if err != nil { return nil, errors.Wrap(err, "Failed to find the primary path of the new space") } + newPath := paths.Concatinate(spacePath.Value, newName) + // path is exactly the same => nothing to do if newPath == currentPath.Value { return nil, store.ErrNoChangeInRequestedMove } @@ -142,12 +154,12 @@ func (s *RepoStore) Move(ctx context.Context, principalID int64, repoID int64, n } // replace the primary path (also updates all child primary paths) - if err = ReplacePathTx(ctx, s.db, tx, p, keepAsAlias); err != nil { + if err = ReplacePathTx(ctx, s.db, tx, p, keepAsAlias, s.pathTransformation); err != nil { return nil, errors.Wrap(err, "Failed to update the primary path of the repo") } // Rename the repo itself - if _, err = tx.ExecContext(ctx, repoUpdateNameAndSpaceID, newName, newSpaceID, repoID); err != nil { + if _, err = tx.ExecContext(ctx, repoUpdateUIDAndParentID, newName, newParentID, repoID); err != nil { return nil, processSQLErrorf(err, "Query for renaming and updating the space id failed") } @@ -183,39 +195,38 @@ func (s *RepoStore) Update(ctx context.Context, repo *types.Repository) error { func (s *RepoStore) Delete(ctx context.Context, id int64) error { tx, err := s.db.BeginTxx(ctx, nil) if err != nil { - return processSQLErrorf(err, "Failed to start a new transaction") + return processSQLErrorf(err, "failed to start a new transaction") } defer func(tx *sqlx.Tx) { _ = tx.Rollback() }(tx) // delete all paths - err = DeleteAllPaths(ctx, tx, enum.PathTargetTypeRepo, id) - if err != nil { - return errors.Wrap(err, "Failed to delete all paths of the repo") + if err = DeleteAllPaths(ctx, tx, enum.PathTargetTypeRepo, id); err != nil { + return fmt.Errorf("failed to delete all paths of the repo: %w", err) } // delete the repo if _, err = tx.ExecContext(ctx, repoDelete, id); err != nil { - return processSQLErrorf(err, "The delete query failed") + return processSQLErrorf(err, "the delete query failed") } if err = tx.Commit(); err != nil { - return processSQLErrorf(err, "Failed to commit transaction") + return processSQLErrorf(err, "failed to commit transaction") } return nil } // Count of repos in a space. -func (s *RepoStore) Count(ctx context.Context, spaceID int64, opts *types.RepoFilter) (int64, error) { +func (s *RepoStore) Count(ctx context.Context, parentID int64, opts *types.RepoFilter) (int64, error) { stmt := builder. Select("count(*)"). From("repositories"). - Where("repo_spaceId = ?", spaceID) + Where("repo_parentId = ?", parentID) if opts.Query != "" { - stmt = stmt.Where("repo_pathName LIKE ?", fmt.Sprintf("%%%s%%", opts.Query)) + stmt = stmt.Where("repo_uid LIKE ?", fmt.Sprintf("%%%s%%", opts.Query)) } sql, args, err := stmt.ToSql() @@ -232,36 +243,34 @@ func (s *RepoStore) Count(ctx context.Context, spaceID int64, opts *types.RepoFi } // List returns a list of repos in a space. -func (s *RepoStore) List(ctx context.Context, spaceID int64, opts *types.RepoFilter) ([]*types.Repository, error) { +func (s *RepoStore) List(ctx context.Context, parentID int64, opts *types.RepoFilter) ([]*types.Repository, error) { dst := []*types.Repository{} // construct the sql statement. stmt := builder. - Select("repositories.*,path_value AS repo_path"). + Select("repositories.*,paths.path_value AS repo_path"). From("repositories"). InnerJoin("paths ON repositories.repo_id=paths.path_targetId AND paths.path_targetType='repo' "+ "AND paths.path_isAlias=0"). - Where("repo_spaceId = ?", fmt.Sprint(spaceID)) + Where("repo_parentId = ?", fmt.Sprint(parentID)) if opts.Query != "" { - stmt = stmt.Where("repo_pathName LIKE ?", fmt.Sprintf("%%%s%%", opts.Query)) + stmt = stmt.Where("repo_uid LIKE ?", fmt.Sprintf("%%%s%%", opts.Query)) } stmt = stmt.Limit(uint64(limit(opts.Size))) stmt = stmt.Offset(uint64(offset(opts.Page, opts.Size))) switch opts.Sort { - case enum.RepoAttrName, enum.RepoAttrNone: + case enum.RepoAttrUID, enum.RepoAttrNone: // NOTE: string concatenation is safe because the // order attribute is an enum and is not user-defined, // and is therefore not subject to injection attacks. - stmt = stmt.OrderBy("repo_name COLLATE NOCASE " + opts.Order.String()) + stmt = stmt.OrderBy("repo_uid COLLATE NOCASE " + opts.Order.String()) case enum.RepoAttrCreated: stmt = stmt.OrderBy("repo_created " + opts.Order.String()) case enum.RepoAttrUpdated: stmt = stmt.OrderBy("repo_updated " + opts.Order.String()) - case enum.RepoAttrPathName: - stmt = stmt.OrderBy("repo_pathName COLLATE NOCASE " + opts.Order.String()) case enum.RepoAttrPath: stmt = stmt.OrderBy("repo_path COLLATE NOCASE " + opts.Order.String()) } @@ -295,14 +304,14 @@ func (s *RepoStore) CreatePath(ctx context.Context, repoID int64, params *types. TargetID: repoID, IsAlias: true, - // get remaining infor from params + // get remaining info from params Value: params.Path, CreatedBy: params.CreatedBy, Created: params.Created, Updated: params.Updated, } - return p, CreateAliasPath(ctx, s.db, p) + return p, CreateAliasPath(ctx, s.db, p, s.pathTransformation) } // DeletePath an alias of a repository. @@ -313,10 +322,9 @@ func (s *RepoStore) DeletePath(ctx context.Context, repoID int64, pathID int64) const repoSelectBase = ` SELECT repo_id -,repo_pathName -,repo_spaceId +,repo_parentId +,repo_uid ,paths.path_value AS repo_path -,repo_name ,repo_description ,repo_isPublic ,repo_createdBy @@ -341,10 +349,10 @@ const repoSelectByID = repoSelectBaseWithJoin + ` WHERE repo_id = $1 ` -const repoSelectByPath = repoSelectBase + ` +const repoSelectByPathUnique = repoSelectBase + ` FROM paths paths1 INNER JOIN repositories ON repositories.repo_id=paths1.path_targetId AND paths1.path_targetType='repo' -AND paths1.path_value = $1 + AND paths1.path_valueUnique = $1 INNER JOIN paths ON repositories.repo_id=paths.path_targetId AND paths.path_targetType='repo' AND paths.path_isAlias=0 ` @@ -356,9 +364,8 @@ WHERE repo_id = $1 // TODO: do we have to worry about SQL injection for description? const repoInsert = ` INSERT INTO repositories ( - repo_pathName - ,repo_spaceId - ,repo_name + repo_parentId + ,repo_uid ,repo_description ,repo_isPublic ,repo_createdBy @@ -372,9 +379,8 @@ INSERT INTO repositories ( ,repo_numClosedPulls ,repo_numOpenPulls ) values ( - :repo_pathName - ,:repo_spaceId - ,:repo_name + :repo_parentId + ,:repo_uid ,:repo_description ,:repo_isPublic ,:repo_createdBy @@ -393,8 +399,7 @@ INSERT INTO repositories ( const repoUpdate = ` UPDATE repositories SET -repo_name = :repo_name -,repo_description = :repo_description +repo_description = :repo_description ,repo_isPublic = :repo_isPublic ,repo_updated = :repo_updated ,repo_numForks = :repo_numForks @@ -404,10 +409,10 @@ repo_name = :repo_name WHERE repo_id = :repo_id ` -const repoUpdateNameAndSpaceID = ` +const repoUpdateUIDAndParentID = ` UPDATE repositories SET -repo_pathName = $1 -,repo_spaceId = $2 -WHERE repo_id = $3 +repo_uid = $1 +,repo_parentId = $2 +WHERE repo_id = $3 ` diff --git a/internal/store/database/repo_sync.go b/internal/store/database/repo_sync.go index f86fd3afc..c1e103662 100644 --- a/internal/store/database/repo_sync.go +++ b/internal/store/database/repo_sync.go @@ -48,11 +48,11 @@ func (s *RepoStoreSync) Create(ctx context.Context, repo *types.Repository) erro } // Move an existing repo. -func (s *RepoStoreSync) Move(ctx context.Context, principalID int64, repoID int64, newSpaceID int64, +func (s *RepoStoreSync) Move(ctx context.Context, principalID int64, id int64, newParentID int64, newName string, keepAsAlias bool) (*types.Repository, error) { mutex.RLock() defer mutex.RUnlock() - return s.base.Move(ctx, principalID, repoID, newSpaceID, newName, keepAsAlias) + return s.base.Move(ctx, principalID, id, newParentID, newName, keepAsAlias) } // Update the repo details. @@ -70,17 +70,17 @@ func (s *RepoStoreSync) Delete(ctx context.Context, id int64) error { } // Count of repos in a space. -func (s *RepoStoreSync) Count(ctx context.Context, spaceID int64, opts *types.RepoFilter) (int64, error) { +func (s *RepoStoreSync) Count(ctx context.Context, parentID int64, opts *types.RepoFilter) (int64, error) { mutex.RLock() defer mutex.RUnlock() - return s.base.Count(ctx, spaceID, opts) + return s.base.Count(ctx, parentID, opts) } // List returns a list of repos in a space. -func (s *RepoStoreSync) List(ctx context.Context, spaceID int64, opts *types.RepoFilter) ([]*types.Repository, error) { +func (s *RepoStoreSync) List(ctx context.Context, parentID int64, opts *types.RepoFilter) ([]*types.Repository, error) { mutex.RLock() defer mutex.RUnlock() - return s.base.List(ctx, spaceID, opts) + return s.base.List(ctx, parentID, opts) } // CountPaths returns a count of all paths of a repo. diff --git a/internal/store/database/service.go b/internal/store/database/service.go index 1577d2ab3..52416a0c3 100644 --- a/internal/store/database/service.go +++ b/internal/store/database/service.go @@ -7,52 +7,79 @@ package database import ( "context" "database/sql" + "fmt" "github.com/harness/gitness/internal/store" "github.com/harness/gitness/types" + "github.com/harness/gitness/types/enum" + "github.com/rs/zerolog/log" "github.com/jmoiron/sqlx" ) var _ store.ServiceStore = (*ServiceStore)(nil) +// service is a DB representation of a service principal. +// It is required to allow storing transformed UIDs used for uniquness constraints and searching. +type service struct { + types.Service + UIDUnique string `db:"principal_uidUnique"` +} + // NewServiceStore returns a new ServiceStore. -func NewServiceStore(db *sqlx.DB) *ServiceStore { - return &ServiceStore{db} +func NewServiceStore(db *sqlx.DB, uidTransformation store.PrincipalUIDTransformation) *ServiceStore { + return &ServiceStore{ + db: db, + uidTransformation: uidTransformation, + } } // ServiceStore implements a ServiceStore backed by a relational // database. type ServiceStore struct { - db *sqlx.DB + db *sqlx.DB + uidTransformation store.PrincipalUIDTransformation } // Find finds the service by id. func (s *ServiceStore) Find(ctx context.Context, id int64) (*types.Service, error) { - dst := new(types.Service) + dst := new(service) if err := s.db.GetContext(ctx, dst, serviceSelectID, id); err != nil { return nil, processSQLErrorf(err, "Select by id query failed") } - return dst, nil + return s.mapDBService(dst), nil } // FindUID finds the service by uid. func (s *ServiceStore) FindUID(ctx context.Context, uid string) (*types.Service, error) { - dst := new(types.Service) - if err := s.db.GetContext(ctx, dst, serviceSelectUID, uid); err != nil { + // map the UID to unique UID before searching! + uidUnique, err := s.uidTransformation(enum.PrincipalTypeService, uid) + if err != nil { + // in case we fail to transform, return a not found (as it can't exist in the first place) + log.Ctx(ctx).Debug().Msgf("failed to transform uid '%s': %s", uid, err.Error()) + return nil, store.ErrResourceNotFound + } + + dst := new(service) + if err = s.db.GetContext(ctx, dst, serviceSelectUIDUnique, uidUnique); err != nil { return nil, processSQLErrorf(err, "Select by uid query failed") } - return dst, nil + return s.mapDBService(dst), nil } // Create saves the service. -func (s *ServiceStore) Create(ctx context.Context, sa *types.Service) error { - query, arg, err := s.db.BindNamed(serviceInsert, sa) +func (s *ServiceStore) Create(ctx context.Context, svc *types.Service) error { + dbSVC, err := s.mapToDBservice(svc) + if err != nil { + return fmt.Errorf("failed to map db service: %w", err) + } + + query, arg, err := s.db.BindNamed(serviceInsert, dbSVC) if err != nil { return processSQLErrorf(err, "Failed to bind service object") } - if err = s.db.QueryRowContext(ctx, query, arg...).Scan(&sa.ID); err != nil { + if err = s.db.QueryRowContext(ctx, query, arg...).Scan(&svc.ID); err != nil { return processSQLErrorf(err, "Insert query failed") } @@ -60,8 +87,13 @@ func (s *ServiceStore) Create(ctx context.Context, sa *types.Service) error { } // Update updates the service. -func (s *ServiceStore) Update(ctx context.Context, sa *types.Service) error { - query, arg, err := s.db.BindNamed(serviceUpdate, sa) +func (s *ServiceStore) Update(ctx context.Context, svc *types.Service) error { + dbSVC, err := s.mapToDBservice(svc) + if err != nil { + return fmt.Errorf("failed to map db service: %w", err) + } + + query, arg, err := s.db.BindNamed(serviceUpdate, dbSVC) if err != nil { return processSQLErrorf(err, "Failed to bind service object") } @@ -91,13 +123,13 @@ func (s *ServiceStore) Delete(ctx context.Context, id int64) error { // List returns a list of service for a specific parent. func (s *ServiceStore) List(ctx context.Context) ([]*types.Service, error) { - dst := []*types.Service{} + dst := []*service{} err := s.db.SelectContext(ctx, &dst, serviceSelect) if err != nil { return nil, processSQLErrorf(err, "Failed executing default list query") } - return dst, nil + return s.mapDBServices(dst), nil } // Count returns a count of service for a specific parent. @@ -110,6 +142,36 @@ func (s *ServiceStore) Count(ctx context.Context) (int64, error) { return count, nil } +func (s *ServiceStore) mapDBService(dbSvc *service) *types.Service { + return &dbSvc.Service +} + +func (s *ServiceStore) mapDBServices(dbSVCs []*service) []*types.Service { + res := make([]*types.Service, len(dbSVCs)) + for i := range dbSVCs { + res[i] = s.mapDBService(dbSVCs[i]) + } + return res +} + +func (s *ServiceStore) mapToDBservice(svc *types.Service) (*service, error) { + // service comes from outside. + if svc == nil { + return nil, fmt.Errorf("service is nil") + } + + uidUnique, err := s.uidTransformation(enum.PrincipalTypeService, svc.UID) + if err != nil { + return nil, fmt.Errorf("failed to transform service UID: %w", err) + } + dbService := &service{ + Service: *svc, + UIDUnique: uidUnique, + } + + return dbService, nil +} + const serviceCount = ` SELECT count(*) FROM principals @@ -120,7 +182,9 @@ const serviceBase = ` SELECT principal_id ,principal_uid -,principal_name +,principal_uidUnique +,principal_email +,principal_displayName ,principal_blocked ,principal_salt ,principal_created @@ -130,15 +194,15 @@ FROM principals const serviceSelect = serviceBase + ` WHERE principal_type = "service" -ORDER BY principal_name ASC +ORDER BY principal_uid ASC ` const serviceSelectID = serviceBase + ` WHERE principal_type = "service" AND principal_id = $1 ` -const serviceSelectUID = serviceBase + ` -WHERE principal_type = "service" AND principal_uid = $1 +const serviceSelectUIDUnique = serviceBase + ` +WHERE principal_type = "service" AND principal_uidUnique = $1 ` const serviceDelete = ` @@ -150,7 +214,9 @@ const serviceInsert = ` INSERT INTO principals ( principal_type ,principal_uid -,principal_name +,principal_uidUnique +,principal_email +,principal_displayName ,principal_admin ,principal_blocked ,principal_salt @@ -159,7 +225,9 @@ principal_type ) values ( "service" ,:principal_uid -,:principal_name +,:principal_uidUnique +,:principal_email +,:principal_displayName ,:principal_admin ,:principal_blocked ,:principal_salt @@ -171,8 +239,9 @@ principal_type const serviceUpdate = ` UPDATE principals SET - principal_name = :principal_name -,principal_admin = :principal_admin +principal_email = :principal_email +,principal_displayName = :principal_displayName +,principal_admin = :principal_admin ,principal_blocked = :principal_blocked ,principal_updated = :principal_updated WHERE principal_type = "service" AND principal_id = :principal_id diff --git a/internal/store/database/service_account.go b/internal/store/database/service_account.go index caf3c2874..91e8b5591 100644 --- a/internal/store/database/service_account.go +++ b/internal/store/database/service_account.go @@ -7,48 +7,74 @@ package database import ( "context" "database/sql" + "fmt" "github.com/harness/gitness/internal/store" "github.com/harness/gitness/types" "github.com/harness/gitness/types/enum" + "github.com/rs/zerolog/log" "github.com/jmoiron/sqlx" ) var _ store.ServiceAccountStore = (*ServiceAccountStore)(nil) +// serviceAccount is a DB representation of a service account principal. +// It is required to allow storing transformed UIDs used for uniquness constraints and searching. +type serviceAccount struct { + types.ServiceAccount + UIDUnique string `db:"principal_uidUnique"` +} + // NewServiceAccountStore returns a new ServiceAccountStore. -func NewServiceAccountStore(db *sqlx.DB) *ServiceAccountStore { - return &ServiceAccountStore{db} +func NewServiceAccountStore(db *sqlx.DB, uidTransformation store.PrincipalUIDTransformation) *ServiceAccountStore { + return &ServiceAccountStore{ + db: db, + uidTransformation: uidTransformation, + } } // ServiceAccountStore implements a ServiceAccountStore backed by a relational // database. type ServiceAccountStore struct { - db *sqlx.DB + db *sqlx.DB + uidTransformation store.PrincipalUIDTransformation } // Find finds the service account by id. func (s *ServiceAccountStore) Find(ctx context.Context, id int64) (*types.ServiceAccount, error) { - dst := new(types.ServiceAccount) + dst := new(serviceAccount) if err := s.db.GetContext(ctx, dst, serviceAccountSelectID, id); err != nil { return nil, processSQLErrorf(err, "Select by id query failed") } - return dst, nil + return s.mapDBServiceAccount(dst), nil } // FindUID finds the service account by uid. func (s *ServiceAccountStore) FindUID(ctx context.Context, uid string) (*types.ServiceAccount, error) { - dst := new(types.ServiceAccount) - if err := s.db.GetContext(ctx, dst, serviceAccountSelectUID, uid); err != nil { + // map the UID to unique UID before searching! + uidUnique, err := s.uidTransformation(enum.PrincipalTypeServiceAccount, uid) + if err != nil { + // in case we fail to transform, return a not found (as it can't exist in the first place) + log.Ctx(ctx).Debug().Msgf("failed to transform uid '%s': %s", uid, err.Error()) + return nil, store.ErrResourceNotFound + } + + dst := new(serviceAccount) + if err = s.db.GetContext(ctx, dst, serviceAccountSelectUIDUnique, uidUnique); err != nil { return nil, processSQLErrorf(err, "Select by uid query failed") } - return dst, nil + return s.mapDBServiceAccount(dst), nil } // Create saves the service account. func (s *ServiceAccountStore) Create(ctx context.Context, sa *types.ServiceAccount) error { - query, arg, err := s.db.BindNamed(serviceAccountInsert, sa) + dbSA, err := s.mapToDBserviceAccount(sa) + if err != nil { + return fmt.Errorf("failed to map db service account: %w", err) + } + + query, arg, err := s.db.BindNamed(serviceAccountInsert, dbSA) if err != nil { return processSQLErrorf(err, "Failed to bind service account object") } @@ -62,7 +88,12 @@ func (s *ServiceAccountStore) Create(ctx context.Context, sa *types.ServiceAccou // Update updates the service account details. func (s *ServiceAccountStore) Update(ctx context.Context, sa *types.ServiceAccount) error { - query, arg, err := s.db.BindNamed(serviceAccountUpdate, sa) + dbSA, err := s.mapToDBserviceAccount(sa) + if err != nil { + return fmt.Errorf("failed to map db service account: %w", err) + } + + query, arg, err := s.db.BindNamed(serviceAccountUpdate, dbSA) if err != nil { return processSQLErrorf(err, "Failed to bind service account object") } @@ -93,13 +124,13 @@ func (s *ServiceAccountStore) Delete(ctx context.Context, id int64) error { // List returns a list of service accounts for a specific parent. func (s *ServiceAccountStore) List(ctx context.Context, parentType enum.ParentResourceType, parentID int64) ([]*types.ServiceAccount, error) { - dst := []*types.ServiceAccount{} + dst := []*serviceAccount{} err := s.db.SelectContext(ctx, &dst, serviceAccountSelectByParentTypeAndID, parentType, parentID) if err != nil { return nil, processSQLErrorf(err, "Failed executing default list query") } - return dst, nil + return s.mapDBServiceAccounts(dst), nil } // Count returns a count of service accounts for a specific parent. @@ -113,6 +144,36 @@ func (s *ServiceAccountStore) Count(ctx context.Context, return count, nil } +func (s *ServiceAccountStore) mapDBServiceAccount(dbSA *serviceAccount) *types.ServiceAccount { + return &dbSA.ServiceAccount +} + +func (s *ServiceAccountStore) mapDBServiceAccounts(dbSAs []*serviceAccount) []*types.ServiceAccount { + res := make([]*types.ServiceAccount, len(dbSAs)) + for i := range dbSAs { + res[i] = s.mapDBServiceAccount(dbSAs[i]) + } + return res +} + +func (s *ServiceAccountStore) mapToDBserviceAccount(sa *types.ServiceAccount) (*serviceAccount, error) { + // service account comes from outside. + if sa == nil { + return nil, fmt.Errorf("service account is nil") + } + + uidUnique, err := s.uidTransformation(enum.PrincipalTypeServiceAccount, sa.UID) + if err != nil { + return nil, fmt.Errorf("failed to transform service account UID: %w", err) + } + dbSA := &serviceAccount{ + ServiceAccount: *sa, + UIDUnique: uidUnique, + } + + return dbSA, nil +} + const serviceAccountCountByParentTypeAndID = ` SELECT count(*) FROM principals @@ -123,7 +184,9 @@ const serviceAccountBase = ` SELECT principal_id ,principal_uid -,principal_name +,principal_uidUnique +,principal_email +,principal_displayName ,principal_blocked ,principal_salt ,principal_created @@ -135,15 +198,15 @@ FROM principals const serviceAccountSelectByParentTypeAndID = serviceAccountBase + ` WHERE principal_type = "serviceaccount" AND principal_sa_parentType = $1 AND principal_sa_parentId = $2 -ORDER BY principal_name ASC +ORDER BY principal_uid ASC ` const serviceAccountSelectID = serviceAccountBase + ` WHERE principal_type = "serviceaccount" AND principal_id = $1 ` -const serviceAccountSelectUID = serviceAccountBase + ` -WHERE principal_type = "serviceaccount" AND principal_uid = $1 +const serviceAccountSelectUIDUnique = serviceAccountBase + ` +WHERE principal_type = "serviceaccount" AND principal_uidUnique = $1 ` const serviceAccountDelete = ` @@ -155,7 +218,9 @@ const serviceAccountInsert = ` INSERT INTO principals ( principal_type ,principal_uid -,principal_name +,principal_uidUnique +,principal_email +,principal_displayName ,principal_admin ,principal_blocked ,principal_salt @@ -166,7 +231,9 @@ principal_type ) values ( "serviceaccount" ,:principal_uid -,:principal_name +,:principal_uidUnique +,:principal_email +,:principal_displayName ,false ,:principal_blocked ,:principal_salt @@ -180,7 +247,8 @@ principal_type const serviceAccountUpdate = ` UPDATE principals SET - principal_name = :principal_name +principal_email = :principal_email +,principal_displayName = :principal_displayName ,principal_blocked = :principal_blocked ,principal_salt = :principal_salt ,principal_updated = :principal_updated diff --git a/internal/store/database/space.go b/internal/store/database/space.go index c8cc90b57..04a426e7e 100644 --- a/internal/store/database/space.go +++ b/internal/store/database/space.go @@ -7,7 +7,6 @@ package database import ( "context" "fmt" - "strings" "time" "github.com/harness/gitness/internal/paths" @@ -22,13 +21,17 @@ import ( var _ store.SpaceStore = (*SpaceStore)(nil) // NewSpaceStore returns a new SpaceStore. -func NewSpaceStore(db *sqlx.DB) *SpaceStore { - return &SpaceStore{db} +func NewSpaceStore(db *sqlx.DB, pathTransformation store.PathTransformation) *SpaceStore { + return &SpaceStore{ + db: db, + pathTransformation: pathTransformation, + } } // SpaceStore implements a SpaceStore backed by a relational database. type SpaceStore struct { - db *sqlx.DB + db *sqlx.DB + pathTransformation store.PathTransformation } // Find the space by id. @@ -42,8 +45,14 @@ func (s *SpaceStore) Find(ctx context.Context, id int64) (*types.Space, error) { // FindByPath finds the space by path. func (s *SpaceStore) FindByPath(ctx context.Context, path string) (*types.Space, error) { + // ensure we transform path before searching (otherwise casing might be wrong) + pathUnique, err := s.pathTransformation(path) + if err != nil { + return nil, fmt.Errorf("failed to transform path '%s': %w", path, err) + } + dst := new(types.Space) - if err := s.db.GetContext(ctx, dst, spaceSelectByPath, path); err != nil { + if err = s.db.GetContext(ctx, dst, spaceSelectByPathUnique, pathUnique); err != nil { return nil, processSQLErrorf(err, "Select query failed") } return dst, nil @@ -70,7 +79,7 @@ func (s *SpaceStore) Create(ctx context.Context, space *types.Space) error { } // Get path (get parent if needed) - path := space.PathName + path := space.UID if space.ParentID > 0 { var parentPath *types.Path parentPath, err = FindPathTx(ctx, tx, enum.PathTargetTypeSpace, space.ParentID) @@ -78,8 +87,8 @@ func (s *SpaceStore) Create(ctx context.Context, space *types.Space) error { return errors.Wrap(err, "Failed to find path of parent space") } - // all existing paths are valid, space name is assumed to be valid. - path = paths.Concatinate(parentPath.Value, space.PathName) + // all existing paths are valid, space uid is assumed to be valid. + path = paths.Concatinate(parentPath.Value, space.UID) } // create path only once we know the id of the space @@ -92,7 +101,7 @@ func (s *SpaceStore) Create(ctx context.Context, space *types.Space) error { Created: space.Created, Updated: space.Updated, } - err = CreatePathTx(ctx, s.db, tx, p) + err = CreatePathTx(ctx, s.db, tx, p, s.pathTransformation) if err != nil { return errors.Wrap(err, "Failed to create primary path of space") } @@ -109,7 +118,7 @@ func (s *SpaceStore) Create(ctx context.Context, space *types.Space) error { } // Move moves an existing space. -func (s *SpaceStore) Move(ctx context.Context, principalID int64, spaceID int64, newParentID int64, newName string, +func (s *SpaceStore) Move(ctx context.Context, principalID int64, id int64, newParentID int64, newName string, keepAsAlias bool) (*types.Space, error) { tx, err := s.db.BeginTxx(ctx, nil) if err != nil { @@ -120,13 +129,13 @@ func (s *SpaceStore) Move(ctx context.Context, principalID int64, spaceID int64, }(tx) // always get currentpath (either it didn't change or we need to for validation) - currentPath, err := FindPathTx(ctx, tx, enum.PathTargetTypeSpace, spaceID) + currentPath, err := FindPathTx(ctx, tx, enum.PathTargetTypeSpace, id) if err != nil { return nil, errors.Wrap(err, "Failed to find the primary path of the space") } // get path of new parent if needed - newPath := newName + newPathValue := newName if newParentID > 0 { // get path of new parent space var spacePath *types.Path @@ -135,42 +144,37 @@ func (s *SpaceStore) Move(ctx context.Context, principalID int64, spaceID int64, return nil, errors.Wrap(err, "Failed to find the primary path of the new parent space") } - newPath = paths.Concatinate(spacePath.Value, newName) + newPathValue = paths.Concatinate(spacePath.Value, newName) } - /* - * IMPORTANT - * To avoid cycles in the primary graph, we have to ensure that the old path isn't a parent of the new path. - */ - if newPath == currentPath.Value { + // path is exactly the same => nothing to do + if newPathValue == currentPath.Value { return nil, store.ErrNoChangeInRequestedMove - } else if strings.HasPrefix(newPath, currentPath.Value+types.PathSeparator) { - return nil, store.ErrIllegalMoveCyclicHierarchy } p := &types.Path{ TargetType: enum.PathTargetTypeSpace, - TargetID: spaceID, + TargetID: id, IsAlias: false, - Value: newPath, + Value: newPathValue, CreatedBy: principalID, Created: time.Now().UnixMilli(), Updated: time.Now().UnixMilli(), } // replace the primary path (also updates all child primary paths) - if err = ReplacePathTx(ctx, s.db, tx, p, keepAsAlias); err != nil { + if err = ReplacePathTx(ctx, s.db, tx, p, keepAsAlias, s.pathTransformation); err != nil { return nil, errors.Wrap(err, "Failed to update the primary path of the space") } // Update the space itself - if _, err = tx.ExecContext(ctx, spaceUpdateNameAndParentID, newName, newParentID, spaceID); err != nil { + if _, err = tx.ExecContext(ctx, spaceUpdateUIDAndParentID, newName, newParentID, id); err != nil { return nil, processSQLErrorf(err, "Query for renaming and updating the parent id failed") } // TODO: return space as part of rename operation dst := new(types.Space) - if err = tx.GetContext(ctx, dst, spaceSelectByID, spaceID); err != nil { + if err = tx.GetContext(ctx, dst, spaceSelectByID, id); err != nil { return nil, processSQLErrorf(err, "Select query to get the space's latest state failed") } @@ -213,7 +217,7 @@ func (s *SpaceStore) Delete(ctx context.Context, id int64) error { } // Get child count and ensure there are none - count, err := CountPrimaryChildPathsTx(ctx, tx, path.Value) + count, err := CountPrimaryChildPathsTx(ctx, tx, path.Value, s.pathTransformation) if err != nil { return fmt.Errorf("child count error: %w", err) } @@ -223,8 +227,7 @@ func (s *SpaceStore) Delete(ctx context.Context, id int64) error { } // delete all paths - err = DeleteAllPaths(ctx, tx, enum.PathTargetTypeSpace, id) - if err != nil { + if err = DeleteAllPaths(ctx, tx, enum.PathTargetTypeSpace, id); err != nil { return errors.Wrap(err, "Failed to delete all paths of the space") } @@ -248,7 +251,7 @@ func (s *SpaceStore) Count(ctx context.Context, id int64, opts *types.SpaceFilte Where("space_parentId = ?", id) if opts.Query != "" { - stmt = stmt.Where("space_pathName LIKE ?", fmt.Sprintf("%%%s%%", opts.Query)) + stmt = stmt.Where("space_uid LIKE ?", fmt.Sprintf("%%%s%%", opts.Query)) } sql, args, err := stmt.ToSql() @@ -277,21 +280,19 @@ func (s *SpaceStore) List(ctx context.Context, id int64, opts *types.SpaceFilter stmt = stmt.Offset(uint64(offset(opts.Page, opts.Size))) if opts.Query != "" { - stmt = stmt.Where("space_pathName LIKE ?", fmt.Sprintf("%%%s%%", opts.Query)) + stmt = stmt.Where("space_uid LIKE ?", fmt.Sprintf("%%%s%%", opts.Query)) } switch opts.Sort { - case enum.SpaceAttrName, enum.SpaceAttrNone: + case enum.SpaceAttrUID, enum.SpaceAttrNone: // NOTE: string concatenation is safe because the // order attribute is an enum and is not user-defined, // and is therefore not subject to injection attacks. - stmt = stmt.OrderBy("space_name COLLATE NOCASE " + opts.Order.String()) + stmt = stmt.OrderBy("space_uid COLLATE NOCASE " + opts.Order.String()) case enum.SpaceAttrCreated: stmt = stmt.OrderBy("space_created " + opts.Order.String()) case enum.SpaceAttrUpdated: stmt = stmt.OrderBy("space_updated " + opts.Order.String()) - case enum.SpaceAttrPathName: - stmt = stmt.OrderBy("space_pathName COLLATE NOCASE " + opts.Order.String()) case enum.SpaceAttrPath: stmt = stmt.OrderBy("space_path COLLATE NOCASE " + opts.Order.String()) } @@ -319,34 +320,33 @@ func (s *SpaceStore) ListPaths(ctx context.Context, id int64, opts *types.PathFi } // CreatePath creates an alias for a space. -func (s *SpaceStore) CreatePath(ctx context.Context, spaceID int64, params *types.PathParams) (*types.Path, error) { +func (s *SpaceStore) CreatePath(ctx context.Context, id int64, params *types.PathParams) (*types.Path, error) { p := &types.Path{ TargetType: enum.PathTargetTypeSpace, - TargetID: spaceID, + TargetID: id, IsAlias: true, - // get remaining infor from params + // get remaining info from params Value: params.Path, CreatedBy: params.CreatedBy, Created: params.Created, Updated: params.Updated, } - return p, CreateAliasPath(ctx, s.db, p) + return p, CreateAliasPath(ctx, s.db, p, s.pathTransformation) } // DeletePath an alias of a space. -func (s *SpaceStore) DeletePath(ctx context.Context, spaceID int64, pathID int64) error { +func (s *SpaceStore) DeletePath(ctx context.Context, id int64, pathID int64) error { return DeletePath(ctx, s.db, pathID) } const spaceSelectBase = ` SELECT space_id -,space_pathName -,paths.path_value AS space_path ,space_parentId -,space_name +,space_uid +,paths.path_value AS space_path ,space_description ,space_isPublic ,space_createdBy @@ -364,9 +364,10 @@ const spaceSelectByID = spaceSelectBaseWithJoin + ` WHERE space_id = $1 ` -const spaceSelectByPath = spaceSelectBase + ` +const spaceSelectByPathUnique = spaceSelectBase + ` FROM paths paths1 -INNER JOIN spaces ON spaces.space_id=paths1.path_targetId AND paths1.path_targetType='space' AND paths1.path_value = $1 +INNER JOIN spaces ON spaces.space_id=paths1.path_targetId AND paths1.path_targetType='space' + AND paths1.path_valueUnique = $1 INNER JOIN paths ON spaces.space_id=paths.path_targetId AND paths.path_targetType='space' AND paths.path_isAlias=0 ` @@ -378,18 +379,16 @@ WHERE space_id = $1 // TODO: do we have to worry about SQL injection for description? const spaceInsert = ` INSERT INTO spaces ( - space_pathName - ,space_parentId - ,space_name + space_parentId + ,space_uid ,space_description ,space_isPublic ,space_createdBy ,space_created ,space_updated ) values ( - :space_pathName - ,:space_parentId - ,:space_name + :space_parentId + ,:space_uid ,:space_description ,:space_isPublic ,:space_createdBy @@ -401,17 +400,16 @@ INSERT INTO spaces ( const spaceUpdate = ` UPDATE spaces SET -space_name = :space_name -,space_description = :space_description -,space_isPublic = :space_isPublic -,space_updated = :space_updated -WHERE space_id = :space_id +space_description = :space_description +,space_isPublic = :space_isPublic +,space_updated = :space_updated +WHERE space_id = :space_id ` -const spaceUpdateNameAndParentID = ` +const spaceUpdateUIDAndParentID = ` UPDATE spaces SET -space_pathName = $1 +space_uid = $1 ,space_parentId = $2 -WHERE space_id = $3 +WHERE space_id = $3 ` diff --git a/internal/store/database/space_sync.go b/internal/store/database/space_sync.go index 603210826..325c77c58 100644 --- a/internal/store/database/space_sync.go +++ b/internal/store/database/space_sync.go @@ -48,11 +48,11 @@ func (s *SpaceStoreSync) Create(ctx context.Context, space *types.Space) error { } // Move moves an existing space. -func (s *SpaceStoreSync) Move(ctx context.Context, principalID int64, spaceID int64, newParentID int64, newName string, +func (s *SpaceStoreSync) Move(ctx context.Context, principalID int64, id int64, newParentID int64, newName string, keepAsAlias bool) (*types.Space, error) { mutex.RLock() defer mutex.RUnlock() - return s.base.Move(ctx, principalID, spaceID, newParentID, newName, keepAsAlias) + return s.base.Move(ctx, principalID, id, newParentID, newName, keepAsAlias) } // Update the space details. @@ -94,15 +94,15 @@ func (s *SpaceStoreSync) ListPaths(ctx context.Context, id int64, opts *types.Pa } // CreatePath a path for a space. -func (s *SpaceStoreSync) CreatePath(ctx context.Context, spaceID int64, params *types.PathParams) (*types.Path, error) { +func (s *SpaceStoreSync) CreatePath(ctx context.Context, id int64, params *types.PathParams) (*types.Path, error) { mutex.RLock() defer mutex.RUnlock() - return s.base.CreatePath(ctx, spaceID, params) + return s.base.CreatePath(ctx, id, params) } // DeletePath a path of a space. -func (s *SpaceStoreSync) DeletePath(ctx context.Context, spaceID int64, pathID int64) error { +func (s *SpaceStoreSync) DeletePath(ctx context.Context, id int64, pathID int64) error { mutex.RLock() defer mutex.RUnlock() - return s.base.DeletePath(ctx, spaceID, pathID) + return s.base.DeletePath(ctx, id, pathID) } diff --git a/internal/store/database/testdata/repos.json b/internal/store/database/testdata/repos.json index ec0a69f22..3f818b811 100644 --- a/internal/store/database/testdata/repos.json +++ b/internal/store/database/testdata/repos.json @@ -1,9 +1,8 @@ [ { "id": 1, - "pathName": "repo1", - "spaceId": 1, - "name": "Repository 1", + "uid": "repo1", + "parentId": 1, "description": "Some repository.", "isPublic": true, "createdBy": 1, @@ -17,9 +16,8 @@ }, { "id": 2, - "pathName": "repo2", - "spaceId": 2, - "name": "Repository 2", + "uid": "repo2", + "parentId": 2, "description": "Some other repository.", "isPublic": true, "createdBy": 1, diff --git a/internal/store/database/testdata/spaces.json b/internal/store/database/testdata/spaces.json index 9741444b0..a246ac1d4 100644 --- a/internal/store/database/testdata/spaces.json +++ b/internal/store/database/testdata/spaces.json @@ -1,9 +1,8 @@ [ { "id": 1, - "pathName": "space1", + "uid": "space1", "parentId": 0, - "name": "Space 1", "description": "Some space.", "isPublic": true, "createdBy": 1, @@ -12,9 +11,8 @@ }, { "id": 2, - "pathName": "space2", + "uid": "space2", "parentId": 1, - "name": "Space 2", "description": "Some subspace.", "isPublic": true, "createdBy": 1, diff --git a/internal/store/database/token.go b/internal/store/database/token.go index 37f03355e..3a7b458e0 100644 --- a/internal/store/database/token.go +++ b/internal/store/database/token.go @@ -35,6 +35,15 @@ func (s *TokenStore) Find(ctx context.Context, id int64) (*types.Token, error) { return dst, nil } +// Find finds the token by principalId and tokenUID. +func (s *TokenStore) FindByUID(ctx context.Context, principalID int64, tokenUID string) (*types.Token, error) { + dst := new(types.Token) + if err := s.db.GetContext(ctx, dst, TokenSelectByPrincipalIDAndUID, principalID, tokenUID); err != nil { + return nil, processSQLErrorf(err, "Select query failed") + } + return dst, nil +} + // Create saves the token details. func (s *TokenStore) Create(ctx context.Context, token *types.Token) error { query, arg, err := s.db.BindNamed(tokenInsert, token) @@ -96,7 +105,7 @@ const tokenSelectBase = ` SELECT token_id ,token_type -,token_name +,token_uid ,token_principalId ,token_expiresAt ,token_grants @@ -106,18 +115,22 @@ FROM tokens ` //#nosec G101 const tokenSelectForPrincipalIDOfType = tokenSelectBase + ` -WHERE token_principalId = $1 and token_type = $2 +WHERE token_principalId = $1 AND token_type = $2 ORDER BY token_issuedAt DESC ` //#nosec G101 const tokenCountForPrincipalIDOfType = ` SELECT count(*) FROM tokens -WHERE token_principalId = $1 and token_type = $2 +WHERE token_principalId = $1 AND token_type = $2 ` //#nosec G101 const TokenSelectByID = tokenSelectBase + ` -WHERE token_id = $2 +WHERE token_id = $1 +` + +const TokenSelectByPrincipalIDAndUID = tokenSelectBase + ` +WHERE token_principalId = $1 AND token_uid = $2 ` const tokenDelete = ` @@ -133,7 +146,7 @@ WHERE token_principalId = $1 const tokenInsert = ` INSERT INTO tokens ( token_type - ,token_name + ,token_uid ,token_principalId ,token_expiresAt ,token_grants @@ -141,7 +154,7 @@ INSERT INTO tokens ( ,token_createdBy ) values ( :token_type - ,:token_name + ,:token_uid ,:token_principalId ,:token_expiresAt ,:token_grants diff --git a/internal/store/database/token_sync.go b/internal/store/database/token_sync.go index 8f766f42f..3676f43c7 100644 --- a/internal/store/database/token_sync.go +++ b/internal/store/database/token_sync.go @@ -32,6 +32,13 @@ func (s *TokenStoreSync) Find(ctx context.Context, id int64) (*types.Token, erro return s.base.Find(ctx, id) } +// Find finds the token by principalId and tokenUID. +func (s *TokenStoreSync) FindByUID(ctx context.Context, principalID int64, tokenUID string) (*types.Token, error) { + mutex.RLock() + defer mutex.RUnlock() + return s.base.FindByUID(ctx, principalID, tokenUID) +} + // Create saves the token details. func (s *TokenStoreSync) Create(ctx context.Context, token *types.Token) error { mutex.Lock() diff --git a/internal/store/database/user.go b/internal/store/database/user.go index ba318e269..987ca7829 100644 --- a/internal/store/database/user.go +++ b/internal/store/database/user.go @@ -7,107 +7,84 @@ package database import ( "context" "database/sql" + "fmt" "github.com/harness/gitness/internal/store" "github.com/harness/gitness/types" "github.com/harness/gitness/types/enum" "github.com/pkg/errors" + "github.com/rs/zerolog/log" "github.com/jmoiron/sqlx" ) var _ store.UserStore = (*UserStore)(nil) +// user is a DB representation of a user principal. +// It is required to allow storing transformed UIDs used for uniquness constraints and searching. +type user struct { + types.User + UIDUnique string `db:"principal_uidUnique"` +} + // NewUserStore returns a new UserStore. -func NewUserStore(db *sqlx.DB) *UserStore { - return &UserStore{db} +func NewUserStore(db *sqlx.DB, uidTransformation store.PrincipalUIDTransformation) *UserStore { + return &UserStore{ + db: db, + uidTransformation: uidTransformation, + } } // UserStore implements a UserStore backed by a relational // database. type UserStore struct { - db *sqlx.DB + db *sqlx.DB + uidTransformation store.PrincipalUIDTransformation } // Find finds the user by id. func (s *UserStore) Find(ctx context.Context, id int64) (*types.User, error) { - dst := new(types.User) + dst := new(user) if err := s.db.GetContext(ctx, dst, userSelectID, id); err != nil { return nil, processSQLErrorf(err, "Select by id query failed") } - return dst, nil + return s.mapDBUser(dst), nil } // FindUID finds the user by uid. func (s *UserStore) FindUID(ctx context.Context, uid string) (*types.User, error) { - dst := new(types.User) - if err := s.db.GetContext(ctx, dst, userSelectUID, uid); err != nil { + // map the UID to unique UID before searching! + uidUnique, err := s.uidTransformation(enum.PrincipalTypeUser, uid) + if err != nil { + // in case we fail to transform, return a not found (as it can't exist in the first place) + log.Ctx(ctx).Debug().Msgf("failed to transform uid '%s': %s", uid, err.Error()) + return nil, store.ErrResourceNotFound + } + + dst := new(user) + if err = s.db.GetContext(ctx, dst, userSelectUIDUnique, uidUnique); err != nil { return nil, processSQLErrorf(err, "Select by uid query failed") } - return dst, nil + return s.mapDBUser(dst), nil } // FindEmail finds the user by email. func (s *UserStore) FindEmail(ctx context.Context, email string) (*types.User, error) { - dst := new(types.User) + dst := new(user) if err := s.db.GetContext(ctx, dst, userSelectEmail, email); err != nil { return nil, processSQLErrorf(err, "Select by email query failed") } - return dst, nil -} - -// List returns a list of users. -func (s *UserStore) List(ctx context.Context, opts *types.UserFilter) ([]*types.User, error) { - dst := []*types.User{} - - // if the user does not provide any customer filter - // or sorting we use the default select statement. - if opts.Sort == enum.UserAttrNone { - err := s.db.SelectContext(ctx, &dst, userSelect, limit(opts.Size), offset(opts.Page, opts.Size)) - if err != nil { - return nil, processSQLErrorf(err, "Failed executing default list query") - } - return dst, nil - } - - // else we construct the sql statement. - stmt := builder.Select("*").From("users") - stmt = stmt.Limit(uint64(limit(opts.Size))) - stmt = stmt.Offset(uint64(offset(opts.Page, opts.Size))) - - switch opts.Sort { - case enum.UserAttrName, enum.UserAttrNone: - // NOTE: string concatenation is safe because the - // order attribute is an enum and is not user-defined, - // and is therefore not subject to injection attacks. - stmt = stmt.OrderBy("principal_name " + opts.Order.String()) - case enum.UserAttrCreated: - stmt = stmt.OrderBy("principal_created " + opts.Order.String()) - case enum.UserAttrUpdated: - stmt = stmt.OrderBy("principal_updated " + opts.Order.String()) - case enum.UserAttrEmail: - stmt = stmt.OrderBy("principal_user_email " + opts.Order.String()) - case enum.UserAttrUID: - stmt = stmt.OrderBy("principal_uid " + opts.Order.String()) - case enum.UserAttrAdmin: - stmt = stmt.OrderBy("principal_admin " + opts.Order.String()) - } - - sql, _, err := stmt.ToSql() - if err != nil { - return nil, errors.Wrap(err, "Failed to convert query to sql") - } - - if err = s.db.SelectContext(ctx, &dst, sql); err != nil { - return nil, processSQLErrorf(err, "Failed executing custom list query") - } - - return dst, nil + return s.mapDBUser(dst), nil } // Create saves the user details. func (s *UserStore) Create(ctx context.Context, user *types.User) error { - query, arg, err := s.db.BindNamed(userInsert, user) + dbUser, err := s.mapToDBUser(user) + if err != nil { + return fmt.Errorf("failed to map db user: %w", err) + } + + query, arg, err := s.db.BindNamed(userInsert, dbUser) if err != nil { return processSQLErrorf(err, "Failed to bind user object") } @@ -121,7 +98,12 @@ func (s *UserStore) Create(ctx context.Context, user *types.User) error { // Update updates the user details. func (s *UserStore) Update(ctx context.Context, user *types.User) error { - query, arg, err := s.db.BindNamed(userUpdate, user) + dbUser, err := s.mapToDBUser(user) + if err != nil { + return fmt.Errorf("failed to map db user: %w", err) + } + + query, arg, err := s.db.BindNamed(userUpdate, dbUser) if err != nil { return processSQLErrorf(err, "Failed to bind user object") } @@ -149,6 +131,55 @@ func (s *UserStore) Delete(ctx context.Context, id int64) error { return tx.Commit() } +// List returns a list of users. +func (s *UserStore) List(ctx context.Context, opts *types.UserFilter) ([]*types.User, error) { + dst := []*user{} + + // if the user does not provide any customer filter + // or sorting we use the default select statement. + if opts.Sort == enum.UserAttrNone { + err := s.db.SelectContext(ctx, &dst, userSelect, limit(opts.Size), offset(opts.Page, opts.Size)) + if err != nil { + return nil, processSQLErrorf(err, "Failed executing default list query") + } + return s.mapDBUsers(dst), nil + } + + // else we construct the sql statement. + stmt := builder.Select("*").From("users") + stmt = stmt.Limit(uint64(limit(opts.Size))) + stmt = stmt.Offset(uint64(offset(opts.Page, opts.Size))) + + switch opts.Sort { + case enum.UserAttrName, enum.UserAttrNone: + // NOTE: string concatenation is safe because the + // order attribute is an enum and is not user-defined, + // and is therefore not subject to injection attacks. + stmt = stmt.OrderBy("principal_displayName " + opts.Order.String()) + case enum.UserAttrCreated: + stmt = stmt.OrderBy("principal_created " + opts.Order.String()) + case enum.UserAttrUpdated: + stmt = stmt.OrderBy("principal_updated " + opts.Order.String()) + case enum.UserAttrEmail: + stmt = stmt.OrderBy("principal_email " + opts.Order.String()) + case enum.UserAttrUID: + stmt = stmt.OrderBy("principal_uid " + opts.Order.String()) + case enum.UserAttrAdmin: + stmt = stmt.OrderBy("principal_admin " + opts.Order.String()) + } + + sql, _, err := stmt.ToSql() + if err != nil { + return nil, errors.Wrap(err, "Failed to convert query to sql") + } + + if err = s.db.SelectContext(ctx, &dst, sql); err != nil { + return nil, processSQLErrorf(err, "Failed executing custom list query") + } + + return s.mapDBUsers(dst), nil +} + // Count returns a count of users. func (s *UserStore) Count(ctx context.Context) (int64, error) { var count int64 @@ -159,6 +190,36 @@ func (s *UserStore) Count(ctx context.Context) (int64, error) { return count, nil } +func (s *UserStore) mapDBUser(dbUser *user) *types.User { + return &dbUser.User +} + +func (s *UserStore) mapDBUsers(dbUsers []*user) []*types.User { + res := make([]*types.User, len(dbUsers)) + for i := range dbUsers { + res[i] = s.mapDBUser(dbUsers[i]) + } + return res +} + +func (s *UserStore) mapToDBUser(usr *types.User) (*user, error) { + // user comes from outside. + if usr == nil { + return nil, fmt.Errorf("user is nil") + } + + uidUnique, err := s.uidTransformation(enum.PrincipalTypeUser, usr.UID) + if err != nil { + return nil, fmt.Errorf("failed to transform user UID: %w", err) + } + dbUser := &user{ + User: *usr, + UIDUnique: uidUnique, + } + + return dbUser, nil +} + const userCount = ` SELECT count(*) FROM principals @@ -169,20 +230,21 @@ const userBase = ` SELECT principal_id ,principal_uid -,principal_name +,principal_uidUnique +,principal_email +,principal_displayName ,principal_admin ,principal_blocked ,principal_salt ,principal_created ,principal_updated -,principal_user_email ,principal_user_password FROM principals ` const userSelect = userBase + ` WHERE principal_type = "user" -ORDER BY principal_name ASC +ORDER BY principal_uid ASC LIMIT $1 OFFSET $2 ` @@ -190,12 +252,12 @@ const userSelectID = userBase + ` WHERE principal_type = "user" AND principal_id = $1 ` -const userSelectUID = userBase + ` -WHERE principal_type = "user" AND principal_uid = $1 +const userSelectUIDUnique = userBase + ` +WHERE principal_type = "user" AND principal_uidUnique = $1 ` const userSelectEmail = userBase + ` -WHERE principal_type = "user" AND principal_user_email = $1 +WHERE principal_type = "user" AND principal_email = $1 ` const userDelete = ` @@ -207,24 +269,26 @@ const userInsert = ` INSERT INTO principals ( principal_type ,principal_uid -,principal_name +,principal_uidUnique +,principal_email +,principal_displayName ,principal_admin ,principal_blocked ,principal_salt ,principal_created ,principal_updated -,principal_user_email ,principal_user_password ) values ( "user" ,:principal_uid -,:principal_name +,:principal_uidUnique +,:principal_email +,:principal_displayName ,:principal_admin ,:principal_blocked ,:principal_salt ,:principal_created ,:principal_updated -,:principal_user_email ,:principal_user_password ) RETURNING principal_id ` @@ -232,12 +296,12 @@ principal_type const userUpdate = ` UPDATE principals SET - principal_name = :principal_name +principal_email = :principal_email +,principal_displayName = :principal_displayName ,principal_admin = :principal_admin ,principal_blocked = :principal_blocked ,principal_salt = :principal_salt ,principal_updated = :principal_updated -,principal_user_email = :principal_user_email ,principal_user_password = :principal_user_password WHERE principal_type = "user" AND principal_id = :principal_id ` diff --git a/internal/store/database/user_test.go b/internal/store/database/user_test.go index 02ca67f9a..fb562ec23 100644 --- a/internal/store/database/user_test.go +++ b/internal/store/database/user_test.go @@ -39,7 +39,7 @@ func TestUser(t *testing.T) { return } - userStoreSync := NewUserStoreSync(NewUserStore(db)) + userStoreSync := NewUserStoreSync(NewUserStore(db, store.ToLowerPrincipalUIDTransformation)) t.Run("create", testUserCreate(userStoreSync)) t.Run("duplicate", testUserDuplicate(userStoreSync)) t.Run("count", testUserCount(userStoreSync)) diff --git a/internal/store/database/wire.go b/internal/store/database/wire.go index 37aec0020..e01694f78 100644 --- a/internal/store/database/wire.go +++ b/internal/store/database/wire.go @@ -39,61 +39,62 @@ func ProvideDatabase(ctx context.Context, config *types.Config) (*sqlx.DB, error } // ProvideUserStore provides a user store. -func ProvideUserStore(db *sqlx.DB) store.UserStore { +func ProvideUserStore(db *sqlx.DB, uidTransformation store.PrincipalUIDTransformation) store.UserStore { switch db.DriverName() { case postgres: - return NewUserStore(db) + return NewUserStore(db, uidTransformation) default: return NewUserStoreSync( - NewUserStore(db), + NewUserStore(db, uidTransformation), ) } } // ProvideServiceAccountStore provides a service account store. -func ProvideServiceAccountStore(db *sqlx.DB) store.ServiceAccountStore { +func ProvideServiceAccountStore(db *sqlx.DB, + uidTransformation store.PrincipalUIDTransformation) store.ServiceAccountStore { switch db.DriverName() { case postgres: - return NewServiceAccountStore(db) + return NewServiceAccountStore(db, uidTransformation) default: return NewServiceAccountStoreSync( - NewServiceAccountStore(db), + NewServiceAccountStore(db, uidTransformation), ) } } // ProvideServiceStore provides a service store. -func ProvideServiceStore(db *sqlx.DB) store.ServiceStore { +func ProvideServiceStore(db *sqlx.DB, uidTransformation store.PrincipalUIDTransformation) store.ServiceStore { switch db.DriverName() { case postgres: - return NewServiceStore(db) + return NewServiceStore(db, uidTransformation) default: return NewServiceStoreSync( - NewServiceStore(db), + NewServiceStore(db, uidTransformation), ) } } // ProvideSpaceStore provides a space store. -func ProvideSpaceStore(db *sqlx.DB) store.SpaceStore { +func ProvideSpaceStore(db *sqlx.DB, pathTransformation store.PathTransformation) store.SpaceStore { switch db.DriverName() { case postgres: - return NewSpaceStore(db) + return NewSpaceStore(db, pathTransformation) default: return NewSpaceStoreSync( - NewSpaceStore(db), + NewSpaceStore(db, pathTransformation), ) } } // ProvideRepoStore provides a repo store. -func ProvideRepoStore(db *sqlx.DB) store.RepoStore { +func ProvideRepoStore(db *sqlx.DB, pathTransformation store.PathTransformation) store.RepoStore { switch db.DriverName() { case postgres: - return NewRepoStore(db) + return NewRepoStore(db, pathTransformation) default: return NewRepoStoreSync( - NewRepoStore(db), + NewRepoStore(db, pathTransformation), ) } } diff --git a/internal/store/store.go b/internal/store/store.go index 7453ad6a4..2f42dd8c9 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -100,7 +100,7 @@ type ( Create(ctx context.Context, space *types.Space) error // Move moves an existing space. - Move(ctx context.Context, principalID int64, spaceID int64, newParentID int64, newName string, + Move(ctx context.Context, principalID int64, id int64, newParentID int64, newName string, keepAsAlias bool) (*types.Space, error) // Update updates the space details. @@ -122,10 +122,10 @@ type ( ListPaths(ctx context.Context, id int64, opts *types.PathFilter) ([]*types.Path, error) // CreatePath create an alias for a space - CreatePath(ctx context.Context, spaceID int64, params *types.PathParams) (*types.Path, error) + CreatePath(ctx context.Context, id int64, params *types.PathParams) (*types.Path, error) // DeletePath delete an alias of a space - DeletePath(ctx context.Context, spaceID int64, pathID int64) error + DeletePath(ctx context.Context, id int64, pathID int64) error } // RepoStore defines the repository data storage. @@ -140,7 +140,7 @@ type ( Create(ctx context.Context, repo *types.Repository) error // Move moves an existing repo. - Move(ctx context.Context, principalID int64, repoID int64, newSpaceID int64, newName string, + Move(ctx context.Context, principalID int64, repoID int64, newParentID int64, newName string, keepAsAlias bool) (*types.Repository, error) // Update the repo details. @@ -150,10 +150,10 @@ type ( Delete(ctx context.Context, id int64) error // Count of repos in a space. - Count(ctx context.Context, spaceID int64, opts *types.RepoFilter) (int64, error) + Count(ctx context.Context, parentID int64, opts *types.RepoFilter) (int64, error) // List returns a list of repos in a space. - List(ctx context.Context, spaceID int64, opts *types.RepoFilter) ([]*types.Repository, error) + List(ctx context.Context, parentID int64, opts *types.RepoFilter) ([]*types.Repository, error) // CountPaths returns a count of all paths of a repo. CountPaths(ctx context.Context, id int64, opts *types.PathFilter) (int64, error) @@ -173,6 +173,9 @@ type ( // Find finds the token by id Find(ctx context.Context, id int64) (*types.Token, error) + // Find finds the token by principalId and tokenUID + FindByUID(ctx context.Context, principalID int64, tokenUID string) (*types.Token, error) + // Create saves the token details. Create(ctx context.Context, token *types.Token) error diff --git a/internal/store/transformation.go b/internal/store/transformation.go new file mode 100644 index 000000000..eb19708b7 --- /dev/null +++ b/internal/store/transformation.go @@ -0,0 +1,27 @@ +// Copyright 2022 Harness Inc. All rights reserved. +// Use of this source code is governed by the Polyform Free Trial License +// that can be found in the LICENSE.md file for this repository. + +package store + +import ( + "strings" + + "github.com/harness/gitness/types/enum" +) + +// PrincipalUIDTransformation transforms a principalUID to a value that should be duplicate free. +// This allows us to simply switch between principalUIDs being case sensitive, insensitive or anything inbetween. +type PrincipalUIDTransformation func(principalType enum.PrincipalType, uid string) (string, error) + +func ToLowerPrincipalUIDTransformation(principalType enum.PrincipalType, uid string) (string, error) { + return strings.ToLower(uid), nil +} + +// PathTransformation transforms a path to a value that should be duplicate free. +// This allows us to simply switch between paths being case sensitive, insensitive or anything inbetween. +type PathTransformation func(string) (string, error) + +func ToLowerPathTransformation(original string) (string, error) { + return strings.ToLower(original), nil +} diff --git a/internal/store/wire.go b/internal/store/wire.go new file mode 100644 index 000000000..6ea7caa16 --- /dev/null +++ b/internal/store/wire.go @@ -0,0 +1,23 @@ +// Copyright 2022 Harness Inc. All rights reserved. +// Use of this source code is governed by the Polyform Free Trial License +// that can be found in the LICENSE.md file for this repository. + +package store + +import ( + "github.com/google/wire" +) + +// WireSet provides a wire set for this package. +var WireSet = wire.NewSet( + ProvidePathTransformation, + ProvidePrincipalUIDTransformation, +) + +func ProvidePathTransformation() PathTransformation { + return ToLowerPathTransformation +} + +func ProvidePrincipalUIDTransformation() PrincipalUIDTransformation { + return ToLowerPrincipalUIDTransformation +} diff --git a/internal/token/token.go b/internal/token/token.go index 0f5c0d465..02e2ff88b 100644 --- a/internal/token/token.go +++ b/internal/token/token.go @@ -20,7 +20,7 @@ const ( ) func CreateUserSession(ctx context.Context, tokenStore store.TokenStore, - user *types.User, name string) (*types.Token, string, error) { + user *types.User, uid string) (*types.Token, string, error) { principal := types.PrincipalFromUser(user) return Create( ctx, @@ -28,7 +28,7 @@ func CreateUserSession(ctx context.Context, tokenStore store.TokenStore, enum.TokenTypeSession, principal, principal, - name, + uid, userTokenLifeTime, enum.AccessGrantAll, ) @@ -36,14 +36,14 @@ func CreateUserSession(ctx context.Context, tokenStore store.TokenStore, func CreatePAT(ctx context.Context, tokenStore store.TokenStore, createdBy *types.Principal, createdFor *types.User, - name string, lifetime time.Duration, grants enum.AccessGrant) (*types.Token, string, error) { + uid string, lifetime time.Duration, grants enum.AccessGrant) (*types.Token, string, error) { return Create( ctx, tokenStore, enum.TokenTypePAT, createdBy, types.PrincipalFromUser(createdFor), - name, + uid, lifetime, grants, ) @@ -51,14 +51,14 @@ func CreatePAT(ctx context.Context, tokenStore store.TokenStore, func CreateSAT(ctx context.Context, tokenStore store.TokenStore, createdBy *types.Principal, createdFor *types.ServiceAccount, - name string, lifetime time.Duration, grants enum.AccessGrant) (*types.Token, string, error) { + uid string, lifetime time.Duration, grants enum.AccessGrant) (*types.Token, string, error) { return Create( ctx, tokenStore, enum.TokenTypeSAT, createdBy, types.PrincipalFromServiceAccount(createdFor), - name, + uid, lifetime, grants, ) @@ -81,14 +81,14 @@ func CreateOAuth(ctx context.Context, tokenStore store.TokenStore, func Create(ctx context.Context, tokenStore store.TokenStore, tokenType enum.TokenType, createdBy *types.Principal, createdFor *types.Principal, - name string, lifetime time.Duration, grants enum.AccessGrant) (*types.Token, string, error) { + uid string, lifetime time.Duration, grants enum.AccessGrant) (*types.Token, string, error) { issuedAt := time.Now() expiresAt := issuedAt.Add(lifetime) // create db entry first so we get the id. token := types.Token{ Type: tokenType, - Name: name, + UID: uid, PrincipalID: createdFor.ID, IssuedAt: issuedAt.UnixMilli(), ExpiresAt: expiresAt.UnixMilli(), diff --git a/mocks/mock_client.go b/mocks/mock_client.go index dca3bd1f7..dfbf6dd19 100644 --- a/mocks/mock_client.go +++ b/mocks/mock_client.go @@ -9,6 +9,7 @@ import ( reflect "reflect" gomock "github.com/golang/mock/gomock" + user "github.com/harness/gitness/internal/api/controller/user" types "github.com/harness/gitness/types" ) @@ -110,6 +111,21 @@ func (mr *MockClientMockRecorder) UserCreate(arg0, arg1 interface{}) *gomock.Cal return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UserCreate", reflect.TypeOf((*MockClient)(nil).UserCreate), arg0, arg1) } +// UserCreatePAT mocks base method. +func (m *MockClient) UserCreatePAT(arg0 context.Context, arg1 user.CreateTokenInput) (*types.TokenResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UserCreatePAT", arg0, arg1) + ret0, _ := ret[0].(*types.TokenResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UserCreatePAT indicates an expected call of UserCreatePAT. +func (mr *MockClientMockRecorder) UserCreatePAT(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UserCreatePAT", reflect.TypeOf((*MockClient)(nil).UserCreatePAT), arg0, arg1) +} + // UserDelete mocks base method. func (m *MockClient) UserDelete(arg0 context.Context, arg1 string) error { m.ctrl.T.Helper() diff --git a/types/check/common.go b/types/check/common.go index 95a7992f2..197b5508d 100644 --- a/types/check/common.go +++ b/types/check/common.go @@ -7,74 +7,63 @@ package check import ( "fmt" "regexp" + "strings" ) const ( - minPathNameLength = 1 - maxPathNameLength = 64 - pathNameRegex = "^[a-z][a-z0-9\\-\\_]*$" - - minNameLength = 1 - maxNameLength = 256 - nameRegex = "^[a-zA-Z][a-zA-Z0-9\\-\\_ ]*$" + minDisplayNameLength = 1 + maxDisplayNameLength = 256 minUIDLength = 2 maxUIDLength = 64 - uidRegex = "^[a-z][a-z0-9\\-\\_]*$" + uidRegex = "^[a-zA-Z_][a-zA-Z0-9-_]*$" + + minEmailLength = 1 + maxEmailLength = 250 ) var ( - ErrPathNameLength = &ValidationError{ - fmt.Sprintf("Path name has to be between %d and %d in length.", minPathNameLength, maxPathNameLength), - } - ErrPathNameRegex = &ValidationError{"Path name has to start with a letter and only contain the following [a-z0-9-_]."} - - ErrNameLength = &ValidationError{ - fmt.Sprintf("Name has to be between %d and %d in length.", - minNameLength, maxNameLength), - } - ErrNameRegex = &ValidationError{ - "Name has to start with a letter and only contain the following [a-zA-Z0-9-_ ].", + ErrDisplayNameLength = &ValidationError{ + fmt.Sprintf("DisplayName has to be between %d and %d in length.", minDisplayNameLength, maxDisplayNameLength), } + ErrDisplayNameContainsInvalidASCII = &ValidationError{"DisplayName has to consist of valid ASCII characters."} ErrUIDLength = &ValidationError{ fmt.Sprintf("UID has to be between %d and %d in length.", minUIDLength, maxUIDLength), } ErrUIDRegex = &ValidationError{ - "UID has to start with a letter and only contain the following [a-z0-9-_].", + "UID has to start with a letter (or _) and only contain the following characters [a-zA-Z0-9-_].", + } + + ErrEmailLen = &ValidationError{ + fmt.Sprintf("Email address has to be within %d and %d characters", minEmailLength, maxEmailLength), } ) -// PathName checks the provided name and returns an error in it isn't valid. -func PathName(pathName string) error { - l := len(pathName) - if l < minPathNameLength || l > maxPathNameLength { - return ErrPathNameLength +// DisplayName checks the provided display name and returns an error if it isn't valid. +func DisplayName(displayName string) error { + l := len(displayName) + if l < minDisplayNameLength || l > maxDisplayNameLength { + return ErrDisplayNameLength } - if ok, _ := regexp.Match(pathNameRegex, []byte(pathName)); !ok { - return ErrPathNameRegex + // created sanitized string restricted to ASCII characters (without control characters). + sanitizedString := strings.Map(func(r rune) rune { + if r < 32 || r == 127 || r > 255 { + return -1 + } + return r + }, displayName) + + if len(sanitizedString) != len(displayName) { + return ErrDisplayNameContainsInvalidASCII } return nil } -// Name checks the provided name and returns an error in it isn't valid. -func Name(name string) error { - l := len(name) - if l < minNameLength || l > maxNameLength { - return ErrNameLength - } - - if ok, _ := regexp.Match(nameRegex, []byte(name)); !ok { - return ErrNameRegex - } - - return nil -} - -// UID checks the provided uid and returns an error in it isn't valid. +// UID checks the provided uid and returns an error if it isn't valid. func UID(uid string) error { l := len(uid) if l < minUIDLength || l > maxUIDLength { @@ -87,3 +76,15 @@ func UID(uid string) error { return nil } + +// Email checks the provided email and returns an error if it isn't valid. +func Email(email string) error { + l := len(email) + if l < minEmailLength || l > maxEmailLength { + return ErrEmailLen + } + + // TODO: add better email validation. + + return nil +} diff --git a/types/check/grant.go b/types/check/grant.go new file mode 100644 index 000000000..8d13cb660 --- /dev/null +++ b/types/check/grant.go @@ -0,0 +1,26 @@ +// Copyright 2022 Harness Inc. All rights reserved. +// Use of this source code is governed by the Polyform Free Trial License +// that can be found in the LICENSE.md file for this repository. + +package check + +import ( + "github.com/harness/gitness/types/enum" +) + +var ( + ErrTokenGrantEmpty = &ValidationError{ + "The token requires at least one grant.", + } +) + +// AccessGrant returns true if the access grant is valid. +func AccessGrant(grant enum.AccessGrant, allowNone bool) error { + if !allowNone && grant == enum.AccessGrantNone { + return ErrTokenGrantEmpty + } + + // TODO: Ensure grant contains valid values? + + return nil +} diff --git a/types/check/path.go b/types/check/path.go index 5c16cc0f2..8d7cb4343 100644 --- a/types/check/path.go +++ b/types/check/path.go @@ -13,7 +13,6 @@ import ( ) const ( - minPathSegments = 1 maxPathSegmentsForSpace = 9 maxPathSegments = 10 ) @@ -22,9 +21,9 @@ var ( ErrPathEmpty = &ValidationError{ "Path can't be empty.", } - ErrPathInvalidSize = &ValidationError{ - fmt.Sprintf("A path has to be between %d and %d segments long (%d for spaces).", - minPathSegments, maxPathSegments, maxPathSegmentsForSpace), + ErrPathInvalidDepth = &ValidationError{ + fmt.Sprintf("A path can have at most %d segments (%d for spaces).", + maxPathSegments, maxPathSegmentsForSpace), } ErrEmptyPathSegment = &ValidationError{ "Empty segments are not allowed.", @@ -53,17 +52,16 @@ func Path(path string, isSpace bool) error { } // ensure path is not too deep - segments := strings.Split(path, types.PathSeparator) - l := len(segments) - if l < minPathSegments || (!isSpace && l > maxPathSegments) || (isSpace && l > maxPathSegmentsForSpace) { - return ErrPathInvalidSize + if err := PathDepth(path, isSpace); err != nil { + return err } - // ensure all segments of the path are valid + // ensure all segments of the path are valid uids + segments := strings.Split(path, types.PathSeparator) for _, s := range segments { if s == "" { return ErrEmptyPathSegment - } else if err := PathName(s); err != nil { + } else if err := UID(s); err != nil { return errors.Wrapf(err, "Invalid segment '%s'", s) } } @@ -101,9 +99,18 @@ func PathParams(path *types.PathParams, currentPath string, isSpace bool) error return nil } -// PathTooLong Checks if the provided path is too long. +// PathDepth Checks the depth of the provided path. +func PathDepth(path string, isSpace bool) error { + if IsPathTooDeep(path, isSpace) { + return ErrPathInvalidDepth + } + + return nil +} + +// IsPathTooDeep Checks if the provided path is too long. // NOTE: A repository path can be one deeper than a space path (as otherwise the space would be useless). -func PathTooLong(path string, isSpace bool) bool { +func IsPathTooDeep(path string, isSpace bool) bool { l := strings.Count(path, types.PathSeparator) + 1 return (!isSpace && l > maxPathSegments) || (isSpace && l > maxPathSegmentsForSpace) } diff --git a/types/check/repo.go b/types/check/repo.go index dfda854a8..c6d96e131 100644 --- a/types/check/repo.go +++ b/types/check/repo.go @@ -9,26 +9,30 @@ import ( ) var ( - ErrRepositoryRequiresSpaceID = &ValidationError{ - "SpaceID required - Repositories don't exist outside of a space.", + ErrRepositoryRequiresParentID = &ValidationError{ + "ParentID required - Standalone repositories are not supported.", } ) -// Repo checks the provided repository and returns an error in it isn't valid. -func Repo(repo *types.Repository) error { - // validate name - if err := PathName(repo.PathName); err != nil { +// Repo returns true if the Repo is valid. +type Repo func(*types.Repository) error + +// RepoDefault is the default Repo validation. +func RepoDefault(repo *types.Repository) error { + // validate UID + if err := UID(repo.UID); err != nil { return err } - // validate display name - if err := Name(repo.Name); err != nil { - return err - } + // validate the rest + return RepoNoUID(repo) +} +// RepoNoUID validates the repo and ignores the UID field. +func RepoNoUID(repo *types.Repository) error { // validate repo within a space - if repo.SpaceID <= 0 { - return ErrRepositoryRequiresSpaceID + if repo.ParentID <= 0 { + return ErrRepositoryRequiresParentID } // TODO: validate defaultBranch, ... diff --git a/types/check/service.go b/types/check/service.go index 0f8c829f2..d97f6d8e4 100644 --- a/types/check/service.go +++ b/types/check/service.go @@ -8,15 +8,23 @@ import ( "github.com/harness/gitness/types" ) -// Service returns true if the Service if valid. -func Service(sa *types.Service) error { +// Service returns true if the Service is valid. +type Service func(*types.Service) error + +// ServiceDefault is the default Service validation. +func ServiceDefault(svc *types.Service) error { // validate UID - if err := UID(sa.UID); err != nil { + if err := UID(svc.UID); err != nil { return err } - // validate name - if err := Name(sa.Name); err != nil { + // Validate Email + if err := Email(svc.Email); err != nil { + return err + } + + // validate DisplayName + if err := DisplayName(svc.DisplayName); err != nil { return err } diff --git a/types/check/service_account.go b/types/check/service_account.go index 636f6689a..afb15d8d9 100644 --- a/types/check/service_account.go +++ b/types/check/service_account.go @@ -13,24 +13,47 @@ var ( ErrServiceAccountParentTypeIsInvalid = &ValidationError{ "Provided parent type is invalid.", } + ErrServiceAccountParentIDInvalid = &ValidationError{ + "ParentID required - Global service accounts are not supported.", + } ) -// ServiceAccount returns true if the ServiceAccount if valid. -func ServiceAccount(sa *types.ServiceAccount) error { +// ServiceAccount returns true if the ServiceAccount is valid. +type ServiceAccount func(*types.ServiceAccount) error + +// ServiceAccountDefault is the default ServiceAccount validation. +func ServiceAccountDefault(sa *types.ServiceAccount) error { // validate UID if err := UID(sa.UID); err != nil { return err } - // validate name - if err := Name(sa.Name); err != nil { + // Validate Email + if err := Email(sa.Email); err != nil { return err } + // validate DisplayName + if err := DisplayName(sa.DisplayName); err != nil { + return err + } + + // validate remaining + return ServiceAccountNoPrincipal(sa) +} + +// ServiceAccountNoPrincipal verifies the remaining fields of a service account +// that aren't inhereted from principal. +func ServiceAccountNoPrincipal(sa *types.ServiceAccount) error { // validate parentType if sa.ParentType != enum.ParentResourceTypeRepo && sa.ParentType != enum.ParentResourceTypeSpace { return ErrServiceAccountParentTypeIsInvalid } + // validate service account belongs to a space + if sa.ParentID <= 0 { + return ErrServiceAccountParentIDInvalid + } + return nil } diff --git a/types/check/space.go b/types/check/space.go index fee7b39f6..172026701 100644 --- a/types/check/space.go +++ b/types/check/space.go @@ -17,31 +17,35 @@ var ( ErrRootSpaceNameNotAllowed = &ValidationError{ fmt.Sprintf("The following names are not allowed for a root space: %v", illegalRootSpaceNames), } - ErrInvalidParentSpaceID = &ValidationError{ - "Parent space ID has to be either zero for a root space or greater than zero for a child space.", + ErrInvalidParentID = &ValidationError{ + "Parent ID has to be either zero for a root space or greater than zero for a child space.", } ) -// Space checks the provided space and returns an error in it isn't valid. -func Space(space *types.Space) error { - // validate name - if err := PathName(space.PathName); err != nil { +// Space returns true if the Space is valid. +type Space func(*types.Space) error + +// SpaceDefault is the default space validation. +func SpaceDefault(space *types.Space) error { + // validate UID + if err := UID(space.UID); err != nil { return err } - // validate display name - if err := Name(space.Name); err != nil { - return err - } + // validate the rest + return SpaceNoUID(space) +} +// SpaceNoUID validates the space and ignores the UID field. +func SpaceNoUID(space *types.Space) error { if space.ParentID < 0 { - return ErrInvalidParentSpaceID + return ErrInvalidParentID } // root space specific validations if space.ParentID == 0 { for _, p := range illegalRootSpaceNames { - if strings.HasPrefix(space.PathName, p) { + if strings.HasPrefix(space.UID, p) { return ErrRootSpaceNameNotAllowed } } diff --git a/types/check/user.go b/types/check/user.go index 21e27ee96..e79fb877b 100644 --- a/types/check/user.go +++ b/types/check/user.go @@ -5,40 +5,27 @@ package check import ( - "fmt" - "github.com/harness/gitness/types" ) -const ( - minEmailLength = 1 - maxEmailLength = 250 -) - -var ( - // ErrEmailLen is returned when the email address - // exceeds the range of allowed number of characters. - ErrEmailLen = &ValidationError{ - fmt.Sprintf("Email address has to be within %d and %d characters", minEmailLength, maxEmailLength), - } -) - // User returns true if the User is valid. -func User(user *types.User) error { +type User func(*types.User) error + +// UserDefault is the default User validation. +func UserDefault(user *types.User) error { // validate UID if err := UID(user.UID); err != nil { return err } - // validate name - if err := Name(user.Name); err != nil { + // Validate Email + if err := Email(user.Email); err != nil { return err } - // validate email - l := len(user.Email) - if l < minEmailLength || l > maxEmailLength { - return ErrEmailLen + // validate DisplayName + if err := DisplayName(user.DisplayName); err != nil { + return err } return nil diff --git a/types/check/user_test.go b/types/check/user_test.go index 21141901b..8d523ca4c 100644 --- a/types/check/user_test.go +++ b/types/check/user_test.go @@ -13,16 +13,20 @@ import ( func TestUser(t *testing.T) { tests := []struct { - email string - error error + uid string + email string + displayName string + error error }{ { - email: "jane@gmail.com", + uid: "jane", + email: "jane@gmail.com", + displayName: "Jane Smith", }, } for _, test := range tests { - user := &types.User{Email: test.email} - err := User(user) + user := &types.User{UID: test.uid, Email: test.email, DisplayName: test.displayName} + err := UserDefault(user) if got, want := err, test.error; !errors.Is(got, want) { t.Errorf("Want user %s error %v, got %v", test.email, want, got) } diff --git a/types/check/wire.go b/types/check/wire.go new file mode 100644 index 000000000..7a3c39940 --- /dev/null +++ b/types/check/wire.go @@ -0,0 +1,38 @@ +// Copyright 2022 Harness Inc. All rights reserved. +// Use of this source code is governed by the Polyform Free Trial License +// that can be found in the LICENSE.md file for this repository. + +package check + +import ( + "github.com/google/wire" +) + +// WireSet provides a wire set for this package. +var WireSet = wire.NewSet( + ProvideRepoCheck, + ProvideSpaceCheck, + ProvideUserCheck, + ProvideServiceAccountCheck, + ProvideServiceCheck, +) + +func ProvideRepoCheck() Repo { + return RepoDefault +} + +func ProvideSpaceCheck() Space { + return SpaceDefault +} + +func ProvideUserCheck() User { + return UserDefault +} + +func ProvideServiceAccountCheck() ServiceAccount { + return ServiceAccountDefault +} + +func ProvideServiceCheck() Service { + return ServiceDefault +} diff --git a/types/config.go b/types/config.go index 7f4ef860a..b3a07ee3a 100644 --- a/types/config.go +++ b/types/config.go @@ -81,8 +81,8 @@ type Config struct { // Admin defines admin user params (no admin setup if either is empty) Admin struct { - Name string `envconfig:"GITNESS_ADMIN_NAME"` - Email string `envconfig:"GITNESS_ADMIN_EMAIL"` - Password string `envconfig:"GITNESS_ADMIN_PASSWORD"` + DisplayName string `envconfig:"GITNESS_ADMIN_DISPLAYNAME"` + Email string `envconfig:"GITNESS_ADMIN_EMAIL"` + Password string `envconfig:"GITNESS_ADMIN_PASSWORD"` } } diff --git a/types/enum/repo.go b/types/enum/repo.go index 92f7ba51b..77477b687 100644 --- a/types/enum/repo.go +++ b/types/enum/repo.go @@ -14,9 +14,8 @@ type RepoAttr int // Order enumeration. const ( RepoAttrNone RepoAttr = iota - RepoAttrPathName RepoAttrPath - RepoAttrName + RepoAttrUID RepoAttrCreated RepoAttrUpdated ) @@ -25,12 +24,10 @@ const ( // and returns the equivalent enumeration. func ParseRepoAtrr(s string) RepoAttr { switch strings.ToLower(s) { - case name: - return RepoAttrName + case uid: + return RepoAttrUID case path: return RepoAttrPath - case pathName: - return RepoAttrPathName case created, createdAt: return RepoAttrCreated case updated, updatedAt: @@ -43,12 +40,10 @@ func ParseRepoAtrr(s string) RepoAttr { // String returns the string representation of the attribute. func (a RepoAttr) String() string { switch a { - case RepoAttrPathName: - return pathName case RepoAttrPath: return path - case RepoAttrName: - return name + case RepoAttrUID: + return uid case RepoAttrCreated: return created case RepoAttrUpdated: diff --git a/types/enum/shared.go b/types/enum/shared.go index 0caea211b..8f0c0e2d2 100644 --- a/types/enum/shared.go +++ b/types/enum/shared.go @@ -6,9 +6,9 @@ package enum const ( id = "id" + uid = "uid" path = "path" name = "name" - pathName = "path_name" created = "created" createdAt = "created_at" updated = "updated" diff --git a/types/enum/space.go b/types/enum/space.go index e32109f25..7256a5688 100644 --- a/types/enum/space.go +++ b/types/enum/space.go @@ -12,9 +12,8 @@ type SpaceAttr int // Order enumeration. const ( SpaceAttrNone SpaceAttr = iota - SpaceAttrPathName SpaceAttrPath - SpaceAttrName + SpaceAttrUID SpaceAttrCreated SpaceAttrUpdated ) @@ -23,12 +22,10 @@ const ( // and returns the equivalent enumeration. func ParseSpaceAttr(s string) SpaceAttr { switch strings.ToLower(s) { - case name: - return SpaceAttrName + case uid: + return SpaceAttrUID case path: return SpaceAttrPath - case pathName: - return SpaceAttrPathName case created, createdAt: return SpaceAttrCreated case updated, updatedAt: @@ -41,12 +38,10 @@ func ParseSpaceAttr(s string) SpaceAttr { // String returns the string representation of the attribute. func (a SpaceAttr) String() string { switch a { - case SpaceAttrPathName: - return pathName case SpaceAttrPath: return path - case SpaceAttrName: - return name + case SpaceAttrUID: + return uid case SpaceAttrCreated: return created case SpaceAttrUpdated: diff --git a/types/principal.go b/types/principal.go index f3bc2c3f9..cee446152 100644 --- a/types/principal.go +++ b/types/principal.go @@ -11,11 +11,12 @@ type ( // Represents the identity of an acting entity (User, ServiceAccount, Service). Principal struct { // TODO: int64 ID doesn't match DB - ID int64 `db:"principal_id" json:"-"` - UID string `db:"principal_uid" json:"uid"` - Type enum.PrincipalType `db:"principal_type" json:"type"` - Name string `db:"principal_name" json:"name"` - Admin bool `db:"principal_admin" json:"admin"` + ID int64 `db:"principal_id" json:"-"` + UID string `db:"principal_uid" json:"uid"` + Email string `db:"principal_email" json:"email"` + Type enum.PrincipalType `db:"principal_type" json:"type"` + DisplayName string `db:"principal_displayName" json:"displayName"` + Admin bool `db:"principal_admin" json:"admin"` // Should be part of principal or not? Blocked bool `db:"principal_blocked" json:"blocked"` @@ -29,42 +30,45 @@ type ( func PrincipalFromUser(user *User) *Principal { return &Principal{ - ID: user.ID, - UID: user.UID, - Type: enum.PrincipalTypeUser, - Name: user.Name, - Admin: user.Admin, - Blocked: user.Blocked, - Salt: user.Salt, - Created: user.Created, - Updated: user.Updated, + ID: user.ID, + UID: user.UID, + Email: user.Email, + Type: enum.PrincipalTypeUser, + DisplayName: user.DisplayName, + Admin: user.Admin, + Blocked: user.Blocked, + Salt: user.Salt, + Created: user.Created, + Updated: user.Updated, } } func PrincipalFromServiceAccount(sa *ServiceAccount) *Principal { return &Principal{ - ID: sa.ID, - UID: sa.UID, - Type: enum.PrincipalTypeServiceAccount, - Name: sa.Name, - Admin: false, - Blocked: sa.Blocked, - Salt: sa.Salt, - Created: sa.Created, - Updated: sa.Updated, + ID: sa.ID, + UID: sa.UID, + Email: sa.Email, + Type: enum.PrincipalTypeServiceAccount, + DisplayName: sa.DisplayName, + Admin: false, + Blocked: sa.Blocked, + Salt: sa.Salt, + Created: sa.Created, + Updated: sa.Updated, } } func PrincipalFromService(s *Service) *Principal { return &Principal{ - ID: s.ID, - UID: s.UID, - Type: enum.PrincipalTypeService, - Name: s.Name, - Admin: s.Admin, - Blocked: s.Blocked, - Salt: s.Salt, - Created: s.Created, - Updated: s.Updated, + ID: s.ID, + UID: s.UID, + Email: s.Email, + Type: enum.PrincipalTypeService, + DisplayName: s.DisplayName, + Admin: s.Admin, + Blocked: s.Blocked, + Salt: s.Salt, + Created: s.Created, + Updated: s.Updated, } } diff --git a/types/repo.go b/types/repo.go index 839ac1a02..a71dd30d8 100644 --- a/types/repo.go +++ b/types/repo.go @@ -12,10 +12,9 @@ import ( type Repository struct { // TODO: int64 ID doesn't match DB ID int64 `db:"repo_id" json:"id"` - SpaceID int64 `db:"repo_spaceId" json:"spaceId"` - PathName string `db:"repo_pathName" json:"pathName"` + ParentID int64 `db:"repo_parentId" json:"parentId"` + UID string `db:"repo_uid" json:"uid"` Path string `db:"repo_path" json:"path"` - Name string `db:"repo_name" json:"name"` Description string `db:"repo_description" json:"description"` IsPublic bool `db:"repo_isPublic" json:"isPublic"` CreatedBy int64 `db:"repo_createdBy" json:"createdBy"` diff --git a/types/service.go b/types/service.go index fc1d91273..b8a38ae46 100644 --- a/types/service.go +++ b/types/service.go @@ -9,13 +9,14 @@ type ( // Service is a principal representing a different internal service that runs alongside gitness. Service struct { // Fields from Principal - ID int64 `db:"principal_id" json:"-"` - UID string `db:"principal_uid" json:"uid"` - Name string `db:"principal_name" json:"name"` - Admin bool `db:"principal_admin" json:"admin"` - Blocked bool `db:"principal_blocked" json:"blocked"` - Salt string `db:"principal_salt" json:"-"` - Created int64 `db:"principal_created" json:"created"` - Updated int64 `db:"principal_updated" json:"updated"` + ID int64 `db:"principal_id" json:"-"` + UID string `db:"principal_uid" json:"uid"` + Email string `db:"principal_email" json:"email"` + DisplayName string `db:"principal_displayName" json:"displayName"` + Admin bool `db:"principal_admin" json:"admin"` + Blocked bool `db:"principal_blocked" json:"blocked"` + Salt string `db:"principal_salt" json:"-"` + Created int64 `db:"principal_created" json:"created"` + Updated int64 `db:"principal_updated" json:"updated"` } ) diff --git a/types/service_account.go b/types/service_account.go index c7cab7d32..b386aafad 100644 --- a/types/service_account.go +++ b/types/service_account.go @@ -11,13 +11,14 @@ type ( // ServiceAccount is a principal representing a service account. ServiceAccount struct { // Fields from Principal (without admin, as it's never an admin) - ID int64 `db:"principal_id" json:"-"` - UID string `db:"principal_uid" json:"uid"` - Name string `db:"principal_name" json:"name"` - Blocked bool `db:"principal_blocked" json:"blocked"` - Salt string `db:"principal_salt" json:"-"` - Created int64 `db:"principal_created" json:"created"` - Updated int64 `db:"principal_updated" json:"updated"` + ID int64 `db:"principal_id" json:"-"` + UID string `db:"principal_uid" json:"uid"` + Email string `db:"principal_email" json:"email"` + DisplayName string `db:"principal_displayName" json:"displayName"` + Blocked bool `db:"principal_blocked" json:"blocked"` + Salt string `db:"principal_salt" json:"-"` + Created int64 `db:"principal_created" json:"created"` + Updated int64 `db:"principal_updated" json:"updated"` // ServiceAccount specific fields ParentType enum.ParentResourceType `db:"principal_sa_parentType" json:"parentType"` @@ -27,8 +28,8 @@ type ( // ServiceAccountInput store details used to // create or update a service account. ServiceAccountInput struct { - Name *string `json:"name"` - ParentType *enum.ParentResourceType `json:"parentType"` - ParentID *int64 `json:"parentId"` + DisplayName *string `json:"displayName"` + ParentType *enum.ParentResourceType `json:"parentType"` + ParentID *int64 `json:"parentId"` } ) diff --git a/types/space.go b/types/space.go index 6c16f99ac..611f8ff10 100644 --- a/types/space.go +++ b/types/space.go @@ -14,7 +14,7 @@ There isn't a one-solves-all hierarchical data structure for DBs, so for now we are using a mix of materialized paths and adjacency list. Every space stores its parent, and a space's path is stored in a separate table. PRO: Quick lookup of childs, quick lookup based on fqdn (apis) -CON: Changing a space name requires changing all its ancestors' Paths. +CON: Changing a space uid requires changing all its ancestors' Paths. Interesting reads: https://stackoverflow.com/questions/4048151/what-are-the-options-for-storing-hierarchical-data-in-a-relational-database @@ -24,9 +24,8 @@ type Space struct { // TODO: int64 ID doesn't match DB ID int64 `db:"space_id" json:"id"` ParentID int64 `db:"space_parentId" json:"parentId"` - PathName string `db:"space_pathName" json:"pathName"` Path string `db:"space_path" json:"path"` - Name string `db:"space_name" json:"name"` + UID string `db:"space_uid" json:"uid"` Description string `db:"space_description" json:"description"` IsPublic bool `db:"space_isPublic" json:"isPublic"` CreatedBy int64 `db:"space_createdBy" json:"createdBy"` diff --git a/types/token.go b/types/token.go index 1cd3f9a5a..73bdeaf02 100644 --- a/types/token.go +++ b/types/token.go @@ -11,11 +11,10 @@ import ( // Represents server side infos stored for tokens we distribute. type Token struct { // TODO: int64 ID doesn't match DB - ID int64 `db:"token_id" json:"id"` - Type enum.TokenType `db:"token_type" json:"type"` - Name string `db:"token_name" json:"name"` - // TODO: change name to OwnerID? + ID int64 `db:"token_id" json:"-"` PrincipalID int64 `db:"token_principalId" json:"principalId"` + Type enum.TokenType `db:"token_type" json:"type"` + UID string `db:"token_uid" json:"uid"` ExpiresAt int64 `db:"token_expiresAt" json:"expiresAt"` IssuedAt int64 `db:"token_issuedAt" json:"issuedAt"` Grants enum.AccessGrant `db:"token_grants" json:"grants"` diff --git a/types/user.go b/types/user.go index 312551e56..816273d32 100644 --- a/types/user.go +++ b/types/user.go @@ -13,17 +13,17 @@ type ( // User is a principal representing an end user. User struct { // Fields from Principal - ID int64 `db:"principal_id" json:"-"` - UID string `db:"principal_uid" json:"uid"` - Name string `db:"principal_name" json:"name"` - Admin bool `db:"principal_admin" json:"admin"` - Blocked bool `db:"principal_blocked" json:"blocked"` - Salt string `db:"principal_salt" json:"-"` - Created int64 `db:"principal_created" json:"created"` - Updated int64 `db:"principal_updated" json:"updated"` + ID int64 `db:"principal_id" json:"-"` + UID string `db:"principal_uid" json:"uid"` + Email string `db:"principal_email" json:"email"` + DisplayName string `db:"principal_displayName" json:"displayName"` + Admin bool `db:"principal_admin" json:"admin"` + Blocked bool `db:"principal_blocked" json:"blocked"` + Salt string `db:"principal_salt" json:"-"` + Created int64 `db:"principal_created" json:"created"` + Updated int64 `db:"principal_updated" json:"updated"` // User specific fields - Email string `db:"principal_user_email" json:"email"` Password string `db:"principal_user_password" json:"-"` } diff --git a/web/package.json b/web/package.json index 88dc3aac1..b73250a9a 100644 --- a/web/package.json +++ b/web/package.json @@ -22,7 +22,7 @@ "typecheck": "tsc", "clean": "rm -rf dist && rm -rf node_modules/.cache", "services": "npm-run-all services:*", - "services:pm": "restful-react import --config restful-react.config.js pm", + "services:scm": "restful-react import --config restful-react.config.js scm", "postservices": "prettier --write src/services/**/*.tsx", "build": "rm -rf dist && webpack --config config/webpack.prod.js", "coverage": "npm test --coverage", diff --git a/web/restful-react.config.js b/web/restful-react.config.js index adbf41630..2be472386 100644 --- a/web/restful-react.config.js +++ b/web/restful-react.config.js @@ -5,12 +5,12 @@ const customGenerator = require('./scripts/swagger-custom-generator.js') module.exports = { - pm: { - output: 'src/services/policy-mgmt/index.tsx', - file: '../design/gen/http/openapi3.json', + scm: { + output: 'src/services/scm/index.tsx', + file: 'src/services/scm/swagger.yaml', customImport: `import { getConfigNew } from "../config";`, customProps: { - base: `{getConfigNew("pm")}` + base: `{getConfigNew("scm")}` } } } diff --git a/web/src/components/NewRepoModalButton/NewRepoModalButton.tsx b/web/src/components/NewRepoModalButton/NewRepoModalButton.tsx index f563aed59..bc2d0de5b 100644 --- a/web/src/components/NewRepoModalButton/NewRepoModalButton.tsx +++ b/web/src/components/NewRepoModalButton/NewRepoModalButton.tsx @@ -33,7 +33,7 @@ import { useStrings } from 'framework/strings' import { DEFAULT_BRANCH_NAME, getErrorMessage, - REGEX_VALID_REPO_NAME, + REGEX_VALID_IDENTIFIER, SUGGESTED_BRANCH_NAMES, Unknown } from 'utils/Utils' @@ -48,7 +48,7 @@ enum RepoVisibility { } interface RepoFormData { - name: string + identifier: string description: string license: string defaultBranch: string @@ -58,7 +58,7 @@ interface RepoFormData { } const formInitialValues: RepoFormData = { - name: '', + identifier: '', description: '', license: '', defaultBranch: 'main', @@ -122,10 +122,9 @@ export const NewRepoModalButton: React.FC = ({ gitIgnore: get(formData, 'gitignore', 'none'), isPublic: get(formData, 'isPublic') === RepoVisibility.PUBLIC, license: get(formData, 'license', 'none'), - name: get(formData, 'name', '').trim(), - pathName: get(formData, 'name', '').trim(), + uid: get(formData, 'identifier', '').trim(), readme: get(formData, 'addReadme', false), - spaceId: standalone ? space : 0 + parentId: standalone ? space : 0 } as OpenapiCreateRepositoryRequest) .then(response => { hideModal() @@ -160,22 +159,22 @@ export const NewRepoModalButton: React.FC = ({ formName="editVariations" enableReinitialize={true} validationSchema={yup.object().shape({ - name: yup + identifier: yup .string() .trim() .required() - .matches(REGEX_VALID_REPO_NAME, getString('validation.namePatternIsNotValid')) + .matches(REGEX_VALID_IDENTIFIER, getString('validation.identifierPatternIsNotValid')) })} validateOnChange validateOnBlur onSubmit={handleSubmit}> diff --git a/web/src/framework/strings/stringTypes.ts b/web/src/framework/strings/stringTypes.ts index 4ae82736d..4008ed41f 100644 --- a/web/src/framework/strings/stringTypes.ts +++ b/web/src/framework/strings/stringTypes.ts @@ -29,11 +29,12 @@ export interface StringsMap { description: string email: string enterDescription: string - enterRepoName: string + enterRepoIdentifier: string existingAccount: string failedToCreateRepo: string files: string history: string + identifier: string inactiveBranches: string loading: string name: string @@ -51,8 +52,8 @@ export interface StringsMap { 'repos.activities': string 'repos.data': string 'repos.enterBranchName': string + 'repos.identifier': string 'repos.lastChange': string - 'repos.name': string 'repos.noDataMessage': string 'repos.updated': string repositories: string @@ -64,7 +65,7 @@ export interface StringsMap { status: string updated: string 'validation.gitBranchNameInvalid': string - 'validation.namePatternIsNotValid': string + 'validation.identifierPatternIsNotValid': string viewCommitDetails: string yourBranches: string } diff --git a/web/src/i18n/strings.en.yaml b/web/src/i18n/strings.en.yaml index 73733342b..952808d1a 100644 --- a/web/src/i18n/strings.en.yaml +++ b/web/src/i18n/strings.en.yaml @@ -9,6 +9,7 @@ public: Public private: Private cancel: Cancel name: Name +identifier: Identifier search: Search description: Description repositories: Repositories @@ -22,7 +23,7 @@ content: Content history: History newRepo: New Repository createRepo: Create Repository -enterRepoName: Enter Repository Name +enterRepoIdentifier: Enter Repository Identifier enterDescription: Enter a description (optional) addLicense: Add License none: None @@ -50,7 +51,7 @@ searchBranches: Search branches updated: Updated cloneHTTPS: Clone with HTTPS repos: - name: Repo Name + identifier: Repo Identifier data: Repo Data activities: Monthly Activities updated: Updated Date @@ -63,5 +64,5 @@ createRepoModal: publicLabel: Anyone on the internet can see this repository. privateLabel: You choose who can see and commit to this repository. validation: - namePatternIsNotValid: 'Name can only contain alphanumerics, . _ and -' + identifierPatternIsNotValid: 'Identifier can only contain alphanumerics, _ and $' gitBranchNameInvalid: Branch name is invalid. diff --git a/web/src/pages/RepositoriesListing/RepositoriesListing.tsx b/web/src/pages/RepositoriesListing/RepositoriesListing.tsx index 4e472361a..3f0e1862b 100644 --- a/web/src/pages/RepositoriesListing/RepositoriesListing.tsx +++ b/web/src/pages/RepositoriesListing/RepositoriesListing.tsx @@ -56,7 +56,7 @@ export default function RepositoriesListing() { const columns: Column[] = useMemo( () => [ { - Header: getString('repos.name'), + Header: getString('repos.identifier'), width: 'calc(100% - 180px)', Cell: ({ row }: CellProps) => { const record = row.original @@ -65,7 +65,7 @@ export default function RepositoriesListing() { - {record.name} + {record.uid} {record.description && ( diff --git a/web/src/pages/Repository/RepositoryHeader/RepositoryHeader.tsx b/web/src/pages/Repository/RepositoryHeader/RepositoryHeader.tsx index 337e58806..d4cea697b 100644 --- a/web/src/pages/Repository/RepositoryHeader/RepositoryHeader.tsx +++ b/web/src/pages/Repository/RepositoryHeader/RepositoryHeader.tsx @@ -25,7 +25,7 @@ export function RepositoryHeader({ repoMetadata }: RepositoryHeaderProps) { {getString('repositories')} - {repoMetadata.name} + {repoMetadata.uid} - {repoMetadata.name} + {repoMetadata.uid} diff --git a/web/src/pages/RepositoryBranches/RepositoryBranchesHeader/RepositoryBranchessHeader.tsx b/web/src/pages/RepositoryBranches/RepositoryBranchesHeader/RepositoryBranchessHeader.tsx index 3f3f61609..e2da17086 100644 --- a/web/src/pages/RepositoryBranches/RepositoryBranchesHeader/RepositoryBranchessHeader.tsx +++ b/web/src/pages/RepositoryBranches/RepositoryBranchesHeader/RepositoryBranchessHeader.tsx @@ -22,7 +22,7 @@ export function RepositoryBranchesHeader({ repoMetadata }: RepositoryCommitsHead {getString('repositories')} - {repoMetadata.name} + {repoMetadata.uid} {getString('branches')} diff --git a/web/src/pages/RepositoryCommits/RepositoryCommitsHeader/RepositoryCommitsHeader.tsx b/web/src/pages/RepositoryCommits/RepositoryCommitsHeader/RepositoryCommitsHeader.tsx index c24b6e731..8f339f3b5 100644 --- a/web/src/pages/RepositoryCommits/RepositoryCommitsHeader/RepositoryCommitsHeader.tsx +++ b/web/src/pages/RepositoryCommits/RepositoryCommitsHeader/RepositoryCommitsHeader.tsx @@ -22,7 +22,7 @@ export function RepositoryCommitsHeader({ repoMetadata }: RepositoryCommitsHeade {getString('repositories')} - {repoMetadata.name} + {repoMetadata.uid} {getString('commits')} diff --git a/web/src/services/scm/index.tsx b/web/src/services/scm/index.tsx index 7f9572500..8c24f7e57 100644 --- a/web/src/services/scm/index.tsx +++ b/web/src/services/scm/index.tsx @@ -52,18 +52,16 @@ export interface OpenapiCreateRepositoryRequest { gitIgnore?: string isPublic?: boolean license?: string - name?: string - pathName?: string + parentID?: number readme?: boolean - spaceId?: number + uid?: string } export interface OpenapiCreateSpaceRequest { description?: string isPublic?: boolean - name?: string parentId?: number - pathName?: string + uid?: string } export interface OpenapiCurrentUserResponse { @@ -86,43 +84,35 @@ export interface OpenapiGetContentOutput { export interface OpenapiMoveRepoRequest { keepAsAlias?: boolean - pathName?: string | null + parentId?: number | null ref?: string - spaceId?: number | null + uid?: string | null } export interface OpenapiMoveSpaceRequest { keepAsAlias?: boolean parentId?: number | null - pathName?: string | null ref?: string + uid?: string | null } export interface OpenapiUpdateRepoRequest { description?: string | null isPublic?: boolean | null - name?: string | null ref?: string } export interface OpenapiUpdateSpaceRequest { description?: string | null isPublic?: boolean | null - name?: string | null ref?: string } -export interface OpenapiUserUpdateRequest { - admin?: boolean | null - email?: string | null - name?: string | null - password?: string | null -} - export interface RepoBranch { commit?: RepoCommit sha?: string name?: string + sha?: string } export interface RepoCommit { @@ -133,6 +123,16 @@ export interface RepoCommit { title?: string } +export interface RepoCommitTag { + commit?: RepoCommit + isAnnotated?: boolean + message?: string + name?: string + sha?: string + tagger?: RepoSignature + title?: string +} + // tslint:disable-next-line:no-empty-interface export interface RepoContent {} @@ -193,21 +193,21 @@ export interface TypesRepository { forkId?: number id?: number isPublic?: boolean - name?: string numClosedPulls?: number numForks?: number numOpenPulls?: number numPulls?: number + parentId?: number path?: string - pathName?: string - spaceId?: number + uid?: string updated?: number } export interface TypesServiceAccount { blocked?: boolean created?: number - name?: string + displayName?: string + email?: string parentId?: number parentType?: EnumParentResourceType uid?: string @@ -220,10 +220,9 @@ export interface TypesSpace { description?: string id?: number isPublic?: boolean - name?: string parentId?: number path?: string - pathName?: string + uid?: string updated?: number } @@ -231,11 +230,10 @@ export interface TypesToken { createdBy?: number expiresAt?: number grants?: EnumAccessGrant - id?: number issuedAt?: number - name?: string principalId?: number type?: EnumTokenType + uid?: string } export interface TypesTokenResponse { @@ -247,8 +245,8 @@ export interface TypesUser { admin?: boolean blocked?: boolean created?: number + displayName?: string email?: string - name?: string uid?: string updated?: number } @@ -260,6 +258,12 @@ export interface TypesUserInput { password?: string | null } +export interface UserUpdateInput { + displayName?: string | null + email?: string | null + password?: string | null +} + export interface UsererrorError { message?: string } @@ -438,14 +442,22 @@ export const useUpdateRepository = ({ repoRef, ...props }: UseUpdateRepositoryPr ) export interface ListBranchesQueryParams { - /** - * The git reference (branch / tag / commitID) that will be used to retrieve the data. If no value is provided the default branch of the repository is used. - */ - git_ref?: string /** * Indicates whether optional commit information should be included in the response. */ include_commit?: boolean + /** + * The substring by which the branches are filtered. + */ + query?: string + /** + * The order of the output. + */ + direction?: 'asc' | 'desc' + /** + * The data by which the branches are sorted. + */ + sort?: 'name' | 'date' /** * The page to return. */ @@ -605,18 +617,29 @@ export const useMoveRepository = ({ repoRef, ...props }: UseMoveRepositoryProps) { base: getConfigNew('scm'), pathParams: { repoRef }, ...props } ) +export interface ListRepositoryPathsQueryParams { + /** + * The page to return. + */ + page?: number + /** + * The number of entries returned per page. + */ + per_page?: number +} + export interface ListRepositoryPathsPathParams { repoRef: string } export type ListRepositoryPathsProps = Omit< - GetProps, + GetProps, 'path' > & ListRepositoryPathsPathParams export const ListRepositoryPaths = ({ repoRef, ...props }: ListRepositoryPathsProps) => ( - + path={`/repos/${repoRef}/paths`} base={getConfigNew('scm')} {...props} @@ -624,13 +647,13 @@ export const ListRepositoryPaths = ({ repoRef, ...props }: ListRepositoryPathsPr ) export type UseListRepositoryPathsProps = Omit< - UseGetProps, + UseGetProps, 'path' > & ListRepositoryPathsPathParams export const useListRepositoryPaths = ({ repoRef, ...props }: UseListRepositoryPathsProps) => - useGet( + useGet( (paramsInPath: ListRepositoryPathsPathParams) => `/repos/${paramsInPath.repoRef}/paths`, { base: getConfigNew('scm'), pathParams: { repoRef }, ...props } ) @@ -729,6 +752,63 @@ export const useListRepositoryServiceAccounts = ({ repoRef, ...props }: UseListR { base: getConfigNew('scm'), pathParams: { repoRef }, ...props } ) +export interface ListTagsQueryParams { + /** + * Indicates whether optional commit information should be included in the response. + */ + include_commit?: boolean + /** + * The substring by which the tags are filtered. + */ + query?: string + /** + * The order of the output. + */ + direction?: 'asc' | 'desc' + /** + * The data by which the tags are sorted. + */ + sort?: 'name' | 'date' + /** + * The page to return. + */ + page?: number + /** + * The number of entries returned per page. + */ + per_page?: number +} + +export interface ListTagsPathParams { + repoRef: string +} + +export type ListTagsProps = Omit< + GetProps, + 'path' +> & + ListTagsPathParams + +export const ListTags = ({ repoRef, ...props }: ListTagsProps) => ( + + path={`/repos/${repoRef}/tags`} + base={getConfigNew('scm')} + {...props} + /> +) + +export type UseListTagsProps = Omit< + UseGetProps, + 'path' +> & + ListTagsPathParams + +export const useListTags = ({ repoRef, ...props }: UseListTagsProps) => + useGet( + (paramsInPath: ListTagsPathParams) => `/repos/${paramsInPath.repoRef}/tags`, + { base: getConfigNew('scm'), pathParams: { repoRef }, ...props } + ) + export type ListGitignoreProps = Omit, 'path'> export const ListGitignore = (props: ListGitignoreProps) => ( @@ -893,26 +973,43 @@ export const useMoveSpace = ({ spaceRef, ...props }: UseMoveSpaceProps) => { base: getConfigNew('scm'), pathParams: { spaceRef }, ...props } ) +export interface ListPathsQueryParams { + /** + * The page to return. + */ + page?: number + /** + * The number of entries returned per page. + */ + per_page?: number +} + export interface ListPathsPathParams { spaceRef: string } -export type ListPathsProps = Omit, 'path'> & +export type ListPathsProps = Omit< + GetProps, + 'path' +> & ListPathsPathParams export const ListPaths = ({ spaceRef, ...props }: ListPathsProps) => ( - + path={`/spaces/${spaceRef}/paths`} base={getConfigNew('scm')} {...props} /> ) -export type UseListPathsProps = Omit, 'path'> & +export type UseListPathsProps = Omit< + UseGetProps, + 'path' +> & ListPathsPathParams export const useListPaths = ({ spaceRef, ...props }: UseListPathsProps) => - useGet( + useGet( (paramsInPath: ListPathsPathParams) => `/spaces/${paramsInPath.spaceRef}/paths`, { base: getConfigNew('scm'), pathParams: { spaceRef }, ...props } ) @@ -982,6 +1079,18 @@ export const useDeletePath = ({ spaceRef, ...props }: UseDeletePathProps) => ) export interface ListReposQueryParams { + /** + * The substring which is used to filter the repositories by their path name. + */ + query?: string + /** + * The data by which the repositories are sorted. + */ + sort?: 'uid' | 'path' | 'created' | 'updated' + /** + * The order of the output. + */ + direction?: 'asc' | 'desc' /** * The page to return. */ @@ -1053,6 +1162,18 @@ export const useListServiceAccounts = ({ spaceRef, ...props }: UseListServiceAcc ) export interface ListSpacesQueryParams { + /** + * The substring which is used to filter the spaces by their path name. + */ + query?: string + /** + * The data by which the spaces are sorted. + */ + sort?: 'uid' | 'path' | 'created' | 'updated' + /** + * The order of the output. + */ + direction?: 'asc' | 'desc' /** * The page to return. */ @@ -1104,10 +1225,10 @@ export type UseGetUserProps = Omit useGet(`/user`, { base: getConfigNew('scm'), ...props }) -export type UpdateUserProps = Omit, 'path' | 'verb'> +export type UpdateUserProps = Omit, 'path' | 'verb'> export const UpdateUser = (props: UpdateUserProps) => ( - + verb="PATCH" path={`/user`} base={getConfigNew('scm')} @@ -1116,12 +1237,12 @@ export const UpdateUser = (props: UpdateUserProps) => ( ) export type UseUpdateUserProps = Omit< - UseMutateProps, + UseMutateProps, 'path' | 'verb' > export const useUpdateUser = (props: UseUpdateUserProps) => - useMutate('PATCH', `/user`, { + useMutate('PATCH', `/user`, { base: getConfigNew('scm'), ...props }) @@ -1230,35 +1351,3 @@ export const useGetUserEmail = ({ email, ...props }: UseGetUserEmailProps) => (paramsInPath: GetUserEmailPathParams) => `/users/${paramsInPath.email}`, { base: getConfigNew('scm'), pathParams: { email }, ...props } ) - -export interface UpdateUsersPathParams { - email: string -} - -export type UpdateUsersProps = Omit< - MutateProps, - 'path' | 'verb' -> & - UpdateUsersPathParams - -export const UpdateUsers = ({ email, ...props }: UpdateUsersProps) => ( - - verb="PATCH" - path={`/users/${email}`} - base={getConfigNew('scm')} - {...props} - /> -) - -export type UseUpdateUsersProps = Omit< - UseMutateProps, - 'path' | 'verb' -> & - UpdateUsersPathParams - -export const useUpdateUsers = ({ email, ...props }: UseUpdateUsersProps) => - useMutate( - 'PATCH', - (paramsInPath: UpdateUsersPathParams) => `/users/${paramsInPath.email}`, - { base: getConfigNew('scm'), pathParams: { email }, ...props } - ) diff --git a/web/src/services/scm/swagger.yaml b/web/src/services/scm/swagger.yaml index 2959da162..ac7195f1b 100644 --- a/web/src/services/scm/swagger.yaml +++ b/web/src/services/scm/swagger.yaml @@ -273,15 +273,6 @@ paths: get: operationId: listBranches parameters: - - description: The git reference (branch / tag / commitID) that will be used - to retrieve the data. If no value is provided the default branch of the - repository is used. - in: query - name: git_ref - required: false - schema: - default: '{Repository Default Branch}' - type: string - description: Indicates whether optional commit information should be included in the response. in: query @@ -290,6 +281,32 @@ paths: schema: default: false type: boolean + - description: The substring by which the branches are filtered. + in: query + name: query + required: false + schema: + type: string + - description: The order of the output. + in: query + name: direction + required: false + schema: + default: asc + enum: + - asc + - desc + type: string + - description: The data by which the branches are sorted. + in: query + name: sort + required: false + schema: + default: name + enum: + - name + - date + type: string - description: The page to return. in: query name: page @@ -532,6 +549,23 @@ paths: get: operationId: listRepositoryPaths parameters: + - description: The page to return. + in: query + name: page + required: false + schema: + default: 1 + minimum: 1 + type: integer + - description: The number of entries returned per page. + in: query + name: per_page + required: false + schema: + default: 50 + maximum: 100 + minimum: 1 + type: integer - in: path name: repoRef required: true @@ -705,6 +739,101 @@ paths: description: Internal Server Error tags: - repository + /repos/{repoRef}/tags: + get: + operationId: listTags + parameters: + - description: Indicates whether optional commit information should be included + in the response. + in: query + name: include_commit + required: false + schema: + default: false + type: boolean + - description: The substring by which the tags are filtered. + in: query + name: query + required: false + schema: + type: string + - description: The order of the output. + in: query + name: direction + required: false + schema: + default: asc + enum: + - asc + - desc + type: string + - description: The data by which the tags are sorted. + in: query + name: sort + required: false + schema: + default: name + enum: + - name + - date + type: string + - description: The page to return. + in: query + name: page + required: false + schema: + default: 1 + minimum: 1 + type: integer + - description: The number of entries returned per page. + in: query + name: per_page + required: false + schema: + default: 50 + maximum: 100 + minimum: 1 + type: integer + - in: path + name: repoRef + required: true + schema: + type: string + responses: + "200": + content: + application/json: + schema: + items: + $ref: '#/components/schemas/RepoCommitTag' + type: array + description: OK + "401": + content: + application/json: + schema: + $ref: '#/components/schemas/UsererrorError' + description: Unauthorized + "403": + content: + application/json: + schema: + $ref: '#/components/schemas/UsererrorError' + description: Forbidden + "404": + content: + application/json: + schema: + $ref: '#/components/schemas/UsererrorError' + description: Not Found + "500": + content: + application/json: + schema: + $ref: '#/components/schemas/UsererrorError' + description: Internal Server Error + tags: + - repository /resources/gitignore: get: operationId: listGitignore @@ -997,6 +1126,23 @@ paths: get: operationId: listPaths parameters: + - description: The page to return. + in: query + name: page + required: false + schema: + default: 1 + minimum: 1 + type: integer + - description: The number of entries returned per page. + in: query + name: per_page + required: false + schema: + default: 50 + maximum: 100 + minimum: 1 + type: integer - in: path name: spaceRef required: true @@ -1130,6 +1276,35 @@ paths: get: operationId: listRepos parameters: + - description: The substring which is used to filter the repositories by their + path name. + in: query + name: query + required: false + schema: + type: string + - description: The data by which the repositories are sorted. + in: query + name: sort + required: false + schema: + default: uid + enum: + - uid + - path + - created + - updated + type: string + - description: The order of the output. + in: query + name: direction + required: false + schema: + default: asc + enum: + - asc + - desc + type: string - description: The page to return. in: query name: page @@ -1235,6 +1410,35 @@ paths: get: operationId: listSpaces parameters: + - description: The substring which is used to filter the spaces by their path + name. + in: query + name: query + required: false + schema: + type: string + - description: The data by which the spaces are sorted. + in: query + name: sort + required: false + schema: + default: uid + enum: + - uid + - path + - created + - updated + type: string + - description: The order of the output. + in: query + name: direction + required: false + schema: + default: asc + enum: + - asc + - desc + type: string - description: The page to return. in: query name: page @@ -1316,7 +1520,7 @@ paths: content: application/json: schema: - $ref: '#/components/schemas/TypesUserInput' + $ref: '#/components/schemas/UserUpdateInput' responses: "200": content: @@ -1509,46 +1713,6 @@ paths: description: Internal Server Error tags: - users - patch: - operationId: updateUsers - parameters: - - in: path - name: email - required: true - schema: - type: string - requestBody: - content: - application/json: - schema: - $ref: '#/components/schemas/OpenapiUserUpdateRequest' - responses: - "200": - content: - application/json: - schema: - $ref: '#/components/schemas/TypesUser' - description: OK - "400": - content: - application/json: - schema: - $ref: '#/components/schemas/UsererrorError' - description: Bad Request - "404": - content: - application/json: - schema: - $ref: '#/components/schemas/UsererrorError' - description: Not Found - "500": - content: - application/json: - schema: - $ref: '#/components/schemas/UsererrorError' - description: Internal Server Error - tags: - - users components: schemas: EnumAccessGrant: @@ -1628,14 +1792,12 @@ components: type: boolean license: type: string - name: - type: string - pathName: - type: string + parentID: + type: integer readme: type: boolean - spaceId: - type: integer + uid: + type: string type: object OpenapiCreateSpaceRequest: properties: @@ -1643,11 +1805,9 @@ components: type: string isPublic: type: boolean - name: - type: string parentId: type: integer - pathName: + uid: type: string type: object OpenapiCurrentUserResponse: @@ -1688,14 +1848,14 @@ components: properties: keepAsAlias: type: boolean - pathName: - nullable: true - type: string - ref: - type: string - spaceId: + parentId: nullable: true type: integer + ref: + type: string + uid: + nullable: true + type: string type: object OpenapiMoveSpaceRequest: properties: @@ -1704,11 +1864,11 @@ components: parentId: nullable: true type: integer - pathName: - nullable: true - type: string ref: type: string + uid: + nullable: true + type: string type: object OpenapiUpdateRepoRequest: properties: @@ -1718,9 +1878,6 @@ components: isPublic: nullable: true type: boolean - name: - nullable: true - type: string ref: type: string type: object @@ -1732,33 +1889,17 @@ components: isPublic: nullable: true type: boolean - name: - nullable: true - type: string ref: type: string type: object - OpenapiUserUpdateRequest: - properties: - admin: - nullable: true - type: boolean - email: - nullable: true - type: string - name: - nullable: true - type: string - password: - nullable: true - type: string - type: object RepoBranch: properties: commit: $ref: '#/components/schemas/RepoCommit' name: type: string + sha: + type: string type: object RepoCommit: properties: @@ -1773,6 +1914,23 @@ components: title: type: string type: object + RepoCommitTag: + properties: + commit: + $ref: '#/components/schemas/RepoCommit' + isAnnotated: + type: boolean + message: + type: string + name: + type: string + sha: + type: string + tagger: + $ref: '#/components/schemas/RepoSignature' + title: + type: string + type: object RepoContent: {} RepoContentInfo: properties: @@ -1864,8 +2022,6 @@ components: type: integer isPublic: type: boolean - name: - type: string numClosedPulls: type: integer numForks: @@ -1874,12 +2030,12 @@ components: type: integer numPulls: type: integer + parentId: + type: integer path: type: string - pathName: + uid: type: string - spaceId: - type: integer updated: type: integer type: object @@ -1889,7 +2045,9 @@ components: type: boolean created: type: integer - name: + displayName: + type: string + email: type: string parentId: type: integer @@ -1912,13 +2070,11 @@ components: type: integer isPublic: type: boolean - name: - type: string parentId: type: integer path: type: string - pathName: + uid: type: string updated: type: integer @@ -1931,16 +2087,14 @@ components: type: integer grants: $ref: '#/components/schemas/EnumAccessGrant' - id: - type: integer issuedAt: type: integer - name: - type: string principalId: type: integer type: $ref: '#/components/schemas/EnumTokenType' + uid: + type: string type: object TypesTokenResponse: properties: @@ -1957,9 +2111,9 @@ components: type: boolean created: type: integer - email: + displayName: type: string - name: + email: type: string uid: type: string @@ -1981,6 +2135,18 @@ components: nullable: true type: string type: object + UserUpdateInput: + properties: + displayName: + nullable: true + type: string + email: + nullable: true + type: string + password: + nullable: true + type: string + type: object UsererrorError: properties: message: diff --git a/web/src/utils/Utils.ts b/web/src/utils/Utils.ts index 14228d53a..01f6905e5 100644 --- a/web/src/utils/Utils.ts +++ b/web/src/utils/Utils.ts @@ -12,7 +12,7 @@ export const X_TOTAL_PAGES = 'x-total-pages' export const X_PER_PAGE = 'x-per-page' export type Unknown = any // eslint-disable-line @typescript-eslint/no-explicit-any export const DEFAULT_BRANCH_NAME = 'main' -export const REGEX_VALID_REPO_NAME = /^[A-Za-z0-9_.-][A-Za-z0-9 _.-]*$/ +export const REGEX_VALID_IDENTIFIER = /^[a-zA-Z_][0-9a-zA-Z_$]{0,63}$/ export const SUGGESTED_BRANCH_NAMES = [DEFAULT_BRANCH_NAME, 'master'] /** This utility shows a toaster without being bound to any component.