From dc37c83e11d3f9dc98b83a2e7832ab8df044f1d4 Mon Sep 17 00:00:00 2001 From: He Weiwei Date: Thu, 15 Apr 2021 10:52:48 +0800 Subject: [PATCH] refactor: use singular as the tag for user APIs (#14654) Use singular as the tag for user APIs to align with other APIs. Signed-off-by: He Weiwei --- Makefile | 2 +- api/v2.0/swagger.yaml | 44 ++++++++++---------- src/server/v2.0/handler/handler.go | 4 +- src/server/v2.0/handler/model/user.go | 2 +- src/server/v2.0/handler/user.go | 60 +++++++++++++-------------- src/server/v2.0/handler/user_test.go | 2 +- tests/apitests/python/library/base.py | 4 +- tests/apitests/python/library/user.py | 2 +- 8 files changed, 60 insertions(+), 60 deletions(-) diff --git a/Makefile b/Makefile index b2326612b..7ee76c828 100644 --- a/Makefile +++ b/Makefile @@ -297,7 +297,7 @@ endif SWAGGER_IMAGENAME=goharbor/swagger SWAGGER_VERSION=v0.21.0 SWAGGER=$(DOCKERCMD) run --rm -u $(shell id -u):$(shell id -g) -v $(BUILDPATH):$(BUILDPATH) -w $(BUILDPATH) ${SWAGGER_IMAGENAME}:${SWAGGER_VERSION} -SWAGGER_GENERATE_SERVER=${SWAGGER} generate server --template-dir=$(TOOLSPATH)/swagger/templates --exclude-main --additional-initialism=CVE --additional-initialism=GC +SWAGGER_GENERATE_SERVER=${SWAGGER} generate server --template-dir=$(TOOLSPATH)/swagger/templates --exclude-main --additional-initialism=CVE --additional-initialism=GC --additional-initialism=OIDC SWAGGER_IMAGE_BUILD_CMD=${DOCKERBUILD} -f ${TOOLSPATH}/swagger/Dockerfile --build-arg SWAGGER_VERSION=${SWAGGER_VERSION} -t ${SWAGGER_IMAGENAME}:$(SWAGGER_VERSION) . SWAGGER_IMAGENAME: diff --git a/api/v2.0/swagger.yaml b/api/v2.0/swagger.yaml index f74504f54..13bcb958e 100644 --- a/api/v2.0/swagger.yaml +++ b/api/v2.0/swagger.yaml @@ -182,7 +182,7 @@ paths: '403': description: User does not have permission of admin role. '500': - description: Unexpected internal errors. + description: Unexpected internal errors. /configurations: get: summary: Get system configurations. @@ -191,7 +191,7 @@ paths: tags: - configure parameters: - - $ref: '#/parameters/requestId' + - $ref: '#/parameters/requestId' responses: '200': description: Get system configurations successfully. The response body is a map. @@ -455,7 +455,7 @@ paths: parameters: - $ref: '#/parameters/requestId' - $ref: '#/parameters/isResourceName' - - $ref: '#/parameters/projectNameOrId' + - $ref: '#/parameters/projectNameOrId' - $ref: '#/parameters/page' - $ref: '#/parameters/pageSize' - name: entityname @@ -473,12 +473,12 @@ paths: type: integer Link: description: Link refers to the previous page and next page - type: string + type: string schema: type: array items: $ref: '#/definitions/ProjectMemberEntity' - + '400': $ref: '#/responses/400' '401': @@ -498,7 +498,7 @@ paths: parameters: - $ref: '#/parameters/requestId' - $ref: '#/parameters/isResourceName' - - $ref: '#/parameters/projectNameOrId' + - $ref: '#/parameters/projectNameOrId' - name: project_member in: body schema: @@ -530,7 +530,7 @@ paths: parameters: - $ref: '#/parameters/requestId' - $ref: '#/parameters/isResourceName' - - $ref: '#/parameters/projectNameOrId' + - $ref: '#/parameters/projectNameOrId' - name: mid in: path type: integer @@ -561,7 +561,7 @@ paths: parameters: - $ref: '#/parameters/requestId' - $ref: '#/parameters/isResourceName' - - $ref: '#/parameters/projectNameOrId' + - $ref: '#/parameters/projectNameOrId' - name: mid in: path type: integer @@ -593,7 +593,7 @@ paths: parameters: - $ref: '#/parameters/requestId' - $ref: '#/parameters/isResourceName' - - $ref: '#/parameters/projectNameOrId' + - $ref: '#/parameters/projectNameOrId' - name: mid in: path type: integer @@ -610,7 +610,7 @@ paths: '403': $ref: '#/responses/403' '500': - $ref: '#/responses/500' + $ref: '#/responses/500' /repositories: get: summary: List all authorized repositories @@ -4485,7 +4485,7 @@ paths: get: summary: List users tags: - - users + - user operationId: listUsers parameters: - $ref: '#/parameters/requestId' @@ -4517,7 +4517,7 @@ paths: summary: Create a local user. description: This API can be used only when the authentication mode is for local DB. When self registration is disabled. tags: - - users + - user operationId: createUser parameters: - $ref: '#/parameters/requestId' @@ -4544,7 +4544,7 @@ paths: get: summary: Get current user info. tags: - - users + - user operationId: getCurrentUserInfo parameters: - $ref: '#/parameters/requestId' @@ -4563,7 +4563,7 @@ paths: description: | This endpoint is to search the users by username. It's open for all authenticated requests. tags: - - users + - user operationId: searchUsers parameters: - $ref: '#/parameters/requestId' @@ -4603,7 +4603,7 @@ paths: format: int required: true tags: - - users + - user operationId: getUser responses: '200': @@ -4635,7 +4635,7 @@ paths: schema: $ref: '#/definitions/UserProfile' tags: - - users + - user operationId: updateUserProfile responses: '200': @@ -4660,7 +4660,7 @@ paths: required: true description: User ID for marking as to be removed. tags: - - users + - user operationId: deleteUser responses: '200': @@ -4677,7 +4677,7 @@ paths: put: summary: Update a registered user to change to be an administrator of Harbor. tags: - - users + - user operationId: setUserSysAdmin parameters: - name: user_id @@ -4708,7 +4708,7 @@ paths: description: | This endpoint is for user to update password. Users with the admin role can change any user's password. Regular users can change only their own password. tags: - - users + - user operationId: updateUserPassword parameters: - name: user_id @@ -4737,7 +4737,7 @@ paths: get: summary: Get current user permissions. tags: - - users + - user operationId: getCurrentUserPermissions parameters: - name: scope @@ -4771,7 +4771,7 @@ paths: Once this API returns with successful status, the old secret will be invalid, as there will be only one CLI secret for a user. tags: - - users + - user operationId: setCliSecret parameters: - $ref: '#/parameters/requestId' @@ -7528,7 +7528,7 @@ definitions: description: 'The parameters of the policy, the values are dependant on the type of the policy.' Configurations: type: object - additionalProperties: + additionalProperties: type: object StringConfigItem: type: object diff --git a/src/server/v2.0/handler/handler.go b/src/server/v2.0/handler/handler.go index 7668abca4..345527cd8 100644 --- a/src/server/v2.0/handler/handler.go +++ b/src/server/v2.0/handler/handler.go @@ -55,11 +55,11 @@ func New() http.Handler { WebhookAPI: newNotificationPolicyAPI(), WebhookjobAPI: newNotificationJobAPI(), ImmutableAPI: newImmutableAPI(), - OidcAPI: newOIDCAPI(), + OIDCAPI: newOIDCAPI(), SystemCVEAllowlistAPI: newSystemCVEAllowListAPI(), ConfigureAPI: newConfigAPI(), UsergroupAPI: newUserGroupAPI(), - UsersAPI: newUsersAPI(), + UserAPI: newUsersAPI(), }) if err != nil { log.Fatal(err) diff --git a/src/server/v2.0/handler/model/user.go b/src/server/v2.0/handler/model/user.go index 81b38ab60..dd5e1b5a3 100644 --- a/src/server/v2.0/handler/model/user.go +++ b/src/server/v2.0/handler/model/user.go @@ -42,7 +42,7 @@ func (u *User) ToUserResp() *svrmodels.UserResp { UpdateTime: strfmt.DateTime(u.UpdateTime), } if u.OIDCUserMeta != nil { - res.OidcUserMeta = &svrmodels.OIDCUserInfo{ + res.OIDCUserMeta = &svrmodels.OIDCUserInfo{ ID: u.OIDCUserMeta.ID, UserID: int64(u.OIDCUserMeta.UserID), Subiss: u.OIDCUserMeta.SubIss, diff --git a/src/server/v2.0/handler/user.go b/src/server/v2.0/handler/user.go index 1db6e3f27..0b3f6dd88 100644 --- a/src/server/v2.0/handler/user.go +++ b/src/server/v2.0/handler/user.go @@ -37,8 +37,7 @@ import ( usermodels "github.com/goharbor/harbor/src/pkg/user/models" "github.com/goharbor/harbor/src/server/v2.0/handler/model" "github.com/goharbor/harbor/src/server/v2.0/models" - "github.com/goharbor/harbor/src/server/v2.0/restapi" - "github.com/goharbor/harbor/src/server/v2.0/restapi/operations/users" + operation "github.com/goharbor/harbor/src/server/v2.0/restapi/operations/user" ) var userResource = system.NewNamespace().Resource(rbac.ResourceUser) @@ -49,14 +48,14 @@ type usersAPI struct { getAuth func(ctx context.Context) (string, error) // For testing } -func newUsersAPI() restapi.UsersAPI { +func newUsersAPI() *usersAPI { return &usersAPI{ ctl: user.Ctl, getAuth: config.AuthMode, } } -func (u *usersAPI) SetCliSecret(ctx context.Context, params users.SetCliSecretParams) middleware.Responder { +func (u *usersAPI) SetCliSecret(ctx context.Context, params operation.SetCliSecretParams) middleware.Responder { uid := int(params.UserID) if err := u.requireForCLISecret(ctx, uid); err != nil { return u.SendError(ctx, err) @@ -68,10 +67,10 @@ func (u *usersAPI) SetCliSecret(ctx context.Context, params users.SetCliSecretPa log.G(ctx).Errorf("Failed to set CLI secret, error: %v", err) return u.SendError(ctx, err) } - return users.NewSetCliSecretOK() + return operation.NewSetCliSecretOK() } -func (u *usersAPI) CreateUser(ctx context.Context, params users.CreateUserParams) middleware.Responder { +func (u *usersAPI) CreateUser(ctx context.Context, params operation.CreateUserParams) middleware.Responder { if err := u.requireCreatable(ctx); err != nil { return u.SendError(ctx, err) } @@ -94,11 +93,11 @@ func (u *usersAPI) CreateUser(ctx context.Context, params users.CreateUserParams return u.SendError(ctx, err) } location := fmt.Sprintf("%s/%d", strings.TrimSuffix(params.HTTPRequest.URL.Path, "/"), uid) - return users.NewCreateUserCreated().WithLocation(location) + return operation.NewCreateUserCreated().WithLocation(location) } -func (u *usersAPI) ListUsers(ctx context.Context, params users.ListUsersParams) middleware.Responder { +func (u *usersAPI) ListUsers(ctx context.Context, params operation.ListUsersParams) middleware.Responder { if err := u.RequireSystemAccess(ctx, rbac.ActionList, userResource); err != nil { return u.SendError(ctx, err) } @@ -118,25 +117,25 @@ func (u *usersAPI) ListUsers(ctx context.Context, params users.ListUsersParams) } payload := make([]*models.UserResp, 0) if total > 0 { - ul, err := u.ctl.List(ctx, query) + users, err := u.ctl.List(ctx, query) if err != nil { return u.SendError(ctx, err) } - payload = make([]*models.UserResp, len(ul)) - for i, u := range ul { + payload = make([]*models.UserResp, len(users)) + for i, u := range users { m := &model.User{ User: u, } payload[i] = m.ToUserResp() } } - return users.NewListUsersOK(). + return operation.NewListUsersOK(). WithPayload(payload). WithLink(u.Links(ctx, params.HTTPRequest.URL, total, query.PageNumber, query.PageSize).String()). WithXTotalCount(total) } -func (u *usersAPI) GetCurrentUserPermissions(ctx context.Context, params users.GetCurrentUserPermissionsParams) middleware.Responder { +func (u *usersAPI) GetCurrentUserPermissions(ctx context.Context, params operation.GetCurrentUserPermissionsParams) middleware.Responder { if err := u.RequireAuthenticated(ctx); err != nil { u.SendError(ctx, err) } @@ -172,10 +171,10 @@ func (u *usersAPI) GetCurrentUserPermissions(ctx context.Context, params users.G Action: policy.Action.String(), }) } - return users.NewGetCurrentUserPermissionsOK().WithPayload(res) + return operation.NewGetCurrentUserPermissionsOK().WithPayload(res) } -func (u *usersAPI) DeleteUser(ctx context.Context, params users.DeleteUserParams) middleware.Responder { +func (u *usersAPI) DeleteUser(ctx context.Context, params operation.DeleteUserParams) middleware.Responder { uid := int(params.UserID) if err := u.requireDeletable(ctx, uid); err != nil { return u.SendError(ctx, err) @@ -184,10 +183,10 @@ func (u *usersAPI) DeleteUser(ctx context.Context, params users.DeleteUserParams log.G(ctx).Errorf("Failed to delete user %d, error: %v", uid, err) return u.SendError(ctx, err) } - return users.NewDeleteUserOK() + return operation.NewDeleteUserOK() } -func (u *usersAPI) GetCurrentUserInfo(ctx context.Context, params users.GetCurrentUserInfoParams) middleware.Responder { +func (u *usersAPI) GetCurrentUserInfo(ctx context.Context, params operation.GetCurrentUserInfoParams) middleware.Responder { if err := u.RequireAuthenticated(ctx); err != nil { return u.SendError(ctx, err) } @@ -201,10 +200,10 @@ func (u *usersAPI) GetCurrentUserInfo(ctx context.Context, params users.GetCurre return u.SendError(ctx, err) } - return users.NewGetCurrentUserInfoOK().WithPayload(resp) + return operation.NewGetCurrentUserInfoOK().WithPayload(resp) } -func (u *usersAPI) GetUser(ctx context.Context, params users.GetUserParams) middleware.Responder { +func (u *usersAPI) GetUser(ctx context.Context, params operation.GetUserParams) middleware.Responder { uid := int(params.UserID) if err := u.requireReadable(ctx, uid); err != nil { return u.SendError(ctx, err) @@ -214,7 +213,7 @@ func (u *usersAPI) GetUser(ctx context.Context, params users.GetUserParams) midd log.G(ctx).Errorf("Failed to get user info for ID %d, error: %v", uid, err) return u.SendError(ctx, err) } - return users.NewGetUserOK().WithPayload(resp) + return operation.NewGetUserOK().WithPayload(resp) } func (u *usersAPI) getUserByID(ctx context.Context, id int) (*models.UserResp, error) { @@ -237,7 +236,7 @@ func (u *usersAPI) getUserByID(ctx context.Context, id int) (*models.UserResp, e return m.ToUserResp(), nil } -func (u *usersAPI) UpdateUserProfile(ctx context.Context, params users.UpdateUserProfileParams) middleware.Responder { +func (u *usersAPI) UpdateUserProfile(ctx context.Context, params operation.UpdateUserProfileParams) middleware.Responder { uid := int(params.UserID) if err := u.requireModifiable(ctx, uid); err != nil { return u.SendError(ctx, err) @@ -255,10 +254,10 @@ func (u *usersAPI) UpdateUserProfile(ctx context.Context, params users.UpdateUse log.G(ctx).Errorf("Failed to update user profile, error: %v", err) return u.SendError(ctx, err) } - return users.NewUpdateUserProfileOK() + return operation.NewUpdateUserProfileOK() } -func (u *usersAPI) SearchUsers(ctx context.Context, params users.SearchUsersParams) middleware.Responder { +func (u *usersAPI) SearchUsers(ctx context.Context, params operation.SearchUsersParams) middleware.Responder { if err := u.RequireAuthenticated(ctx); err != nil { return u.SendError(ctx, err) } @@ -272,7 +271,7 @@ func (u *usersAPI) SearchUsers(ctx context.Context, params users.SearchUsersPara return u.SendError(ctx, err) } if total == 0 { - return users.NewSearchUsersOK().WithXTotalCount(0).WithPayload([]*models.UserSearchRespItem{}) + return operation.NewSearchUsersOK().WithXTotalCount(0).WithPayload([]*models.UserSearchRespItem{}) } l, err := u.ctl.List(ctx, query) if err != nil { @@ -283,13 +282,13 @@ func (u *usersAPI) SearchUsers(ctx context.Context, params users.SearchUsersPara m := &model.User{User: us} result = append(result, m.ToSearchRespItem()) } - return users.NewSearchUsersOK(). + return operation.NewSearchUsersOK(). WithXTotalCount(total). WithPayload(result). WithLink(u.Links(ctx, params.HTTPRequest.URL, total, query.PageNumber, query.PageSize).String()) } -func (u *usersAPI) UpdateUserPassword(ctx context.Context, params users.UpdateUserPasswordParams) middleware.Responder { +func (u *usersAPI) UpdateUserPassword(ctx context.Context, params operation.UpdateUserPasswordParams) middleware.Responder { uid := int(params.UserID) if err := u.requireModifiable(ctx, uid); err != nil { return u.SendError(ctx, err) @@ -322,10 +321,10 @@ func (u *usersAPI) UpdateUserPassword(ctx context.Context, params users.UpdateUs log.G(ctx).Errorf("Failed to update password, error: %v", err) return u.SendError(ctx, err) } - return users.NewUpdateUserPasswordOK() + return operation.NewUpdateUserPasswordOK() } -func (u *usersAPI) SetUserSysAdmin(ctx context.Context, params users.SetUserSysAdminParams) middleware.Responder { +func (u *usersAPI) SetUserSysAdmin(ctx context.Context, params operation.SetUserSysAdminParams) middleware.Responder { id := int(params.UserID) if err := u.RequireSystemAccess(ctx, rbac.ActionUpdate, rbac.ResourceUser); err != nil { return u.SendError(ctx, err) @@ -333,7 +332,7 @@ func (u *usersAPI) SetUserSysAdmin(ctx context.Context, params users.SetUserSysA if err := u.ctl.SetSysAdmin(ctx, id, params.SysadminFlag.SysadminFlag); err != nil { return u.SendError(ctx, err) } - return users.NewSetUserSysAdminOK() + return operation.NewSetUserSysAdminOK() } func (u *usersAPI) requireForCLISecret(ctx context.Context, id int) error { @@ -471,9 +470,10 @@ func validateUserProfile(user *usermodels.User) error { if utils.IsContainIllegalChar(user.Realname, []string{",", "~", "#", "$", "%"}) { return errors.BadRequestError(nil).WithMessage("realname contains illegal characters") } + if utils.IsIllegalLength(user.Comment, -1, 30) { return errors.BadRequestError(nil).WithMessage("comment with illegal length") } - return nil + return nil } diff --git a/src/server/v2.0/handler/user_test.go b/src/server/v2.0/handler/user_test.go index b14b29573..095d2953e 100644 --- a/src/server/v2.0/handler/user_test.go +++ b/src/server/v2.0/handler/user_test.go @@ -41,7 +41,7 @@ type UserTestSuite struct { func (uts *UserTestSuite) SetupSuite() { uts.uCtl = &usertesting.Controller{} uts.Config = &restapi.Config{ - UsersAPI: &usersAPI{ + UserAPI: &usersAPI{ ctl: uts.uCtl, getAuth: func(ctx context.Context) (string, error) { return common.DBAuth, nil diff --git a/tests/apitests/python/library/base.py b/tests/apitests/python/library/base.py index d63414269..efc662086 100644 --- a/tests/apitests/python/library/base.py +++ b/tests/apitests/python/library/base.py @@ -31,7 +31,7 @@ def _create_client(server, credential, debug, api_type="products"): cfg = None if api_type in ('projectv2', 'artifact', 'repository', 'scanner', 'scan', 'scanall', 'preheat', 'quota', 'replication', 'registry', 'robot', 'gc', 'retention', 'immutable', 'system_cve_allowlist', - 'configure', 'users', 'member'): + 'configure', 'user', 'member'): cfg = v2_swagger_client.Configuration() else: cfg = swagger_client.Configuration() @@ -72,7 +72,7 @@ def _create_client(server, credential, debug, api_type="products"): "immutable": v2_swagger_client.ImmutableApi(v2_swagger_client.ApiClient(cfg)), "system_cve_allowlist": v2_swagger_client.SystemCVEAllowlistApi(v2_swagger_client.ApiClient(cfg)), "configure": v2_swagger_client.ConfigureApi(v2_swagger_client.ApiClient(cfg)), - "users": v2_swagger_client.UsersApi(v2_swagger_client.ApiClient(cfg)), + "user": v2_swagger_client.UserApi(v2_swagger_client.ApiClient(cfg)), "member": v2_swagger_client.MemberApi(v2_swagger_client.ApiClient(cfg)), }.get(api_type,'Error: Wrong API type') diff --git a/tests/apitests/python/library/user.py b/tests/apitests/python/library/user.py index 2611f6a5f..81bcfc6ee 100644 --- a/tests/apitests/python/library/user.py +++ b/tests/apitests/python/library/user.py @@ -8,7 +8,7 @@ from v2_swagger_client.rest import ApiException class User(base.Base, object): def __init__(self): - super(User, self).__init__(api_type = "users") + super(User, self).__init__(api_type = "user") def create_user(self, name=None, email=None, user_password=None, realname=None, expect_status_code=201, **kwargs):