From 7b616468b0a5662e2d293b2d9ea9ae2b9565d7b3 Mon Sep 17 00:00:00 2001 From: Rodney Osodo <28790446+rodneyosodo@users.noreply.github.com> Date: Wed, 13 Dec 2023 15:39:30 +0300 Subject: [PATCH] Add property based testing to users API using schemathesis Signed-off-by: Rodney Osodo <28790446+rodneyosodo@users.noreply.github.com> --- .github/workflows/api-tests.yml | 54 ++++++ .gitignore | 3 + Makefile | 12 +- api/openapi/users.yml | 287 ++++++++++++++++++++++++++++- auth/postgres/domains.go | 20 +- auth/service.go | 31 +--- internal/api/common.go | 26 ++- internal/api/common_test.go | 4 +- internal/apiutil/errors.go | 6 +- internal/groups/api/responses.go | 4 +- internal/groups/postgres/groups.go | 16 +- internal/groups/service.go | 21 +-- internal/groups/service_test.go | 5 +- internal/postgres/common.go | 33 ---- invitations/api/endpoint_test.go | 2 +- invitations/api/requests_test.go | 11 +- invitations/invitations.go | 11 +- invitations/invitations_test.go | 11 +- invitations/service_test.go | 4 +- pkg/clients/postgres/clients.go | 12 +- pkg/clients/status.go | 4 - pkg/errors/repository/types.go | 3 + pkg/errors/service/types.go | 18 ++ pkg/errors/types.go | 6 + pkg/groups/errors.go | 3 - pkg/sdk/go/channels_test.go | 8 +- pkg/sdk/go/groups_test.go | 8 +- pkg/sdk/go/things_test.go | 8 +- pkg/sdk/go/tokens_test.go | 2 +- pkg/sdk/go/users_test.go | 13 +- things/service.go | 2 +- things/service_test.go | 8 +- users/api/clients.go | 32 ++-- users/api/endpoint_test.go | 63 +------ users/api/responses.go | 4 +- users/postgres/clients.go | 4 +- users/service.go | 26 +-- users/service_test.go | 10 +- 38 files changed, 526 insertions(+), 269 deletions(-) create mode 100644 .github/workflows/api-tests.yml diff --git a/.github/workflows/api-tests.yml b/.github/workflows/api-tests.yml new file mode 100644 index 000000000..98a313d6c --- /dev/null +++ b/.github/workflows/api-tests.yml @@ -0,0 +1,54 @@ +# Copyright (c) Abstract Machines +# SPDX-License-Identifier: Apache-2.0 + +name: Property Based Tests + +on: + push: + branches: + - main + pull_request: + branches: + - main + +env: + TOKEN_URL: http://localhost:9002/users/tokens/issue + USER_IDENTITY: admin@example.com + USER_SECRET: 12345678 + +jobs: + api-test: + runs-on: ubuntu-latest + steps: + - name: Checkout Code + uses: actions/checkout@v4 + + - name: Install Go + uses: actions/setup-go@v4 + with: + go-version: 1.21.x + cache-dependency-path: "go.sum" + + - name: Build images + run: make all -j $(nproc) && make dockers_dev -j $(nproc) + + - name: Start containers + run: make run up args="-d" && sleep 10 + + - name: Set access token + run: | + export USER_TOKEN=$(curl -sSX POST $TOKEN_URL -H "Content-Type: application/json" -d "{\"identity\": \"$USER_IDENTITY\",\"secret\": \"$USER_SECRET\"}" | jq -r .access_token) + echo "USER_TOKEN=$USER_TOKEN" >> $GITHUB_ENV + + - name: Run Users API tests + uses: schemathesis/action@v1 + with: + schema: api/openapi/users.yml + base-url: http://localhost:9002 + checks: all + report: false + args: '--header "Authorization: Bearer ${{ env.USER_TOKEN }}" --contrib-unique-data --contrib-openapi-formats-uuid --hypothesis-suppress-health-check=filter_too_much --stateful=links' + + - name: Stop containers + if: always() + run: make run down args="-v" diff --git a/.gitignore b/.gitignore index 8d9ae5778..60286b531 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,6 @@ tools/provision/mgconn.toml # coverage files coverage + +# Schemathesis +.hypothesis diff --git a/Makefile b/Makefile index d1de47292..dc6a953da 100644 --- a/Makefile +++ b/Makefile @@ -97,7 +97,7 @@ FILTERED_SERVICES = $(filter-out $(RUN_ADDON_ARGS), $(SERVICES)) all: $(SERVICES) -.PHONY: all $(SERVICES) dockers dockers_dev latest release run run_addons grpc_mtls_certs check_mtls check_certs +.PHONY: all $(SERVICES) dockers dockers_dev latest release run run_addons grpc_mtls_certs check_mtls check_certs test_api clean: rm -rf ${BUILD_DIR} @@ -129,6 +129,16 @@ test: mocks done go test -v --race -count 1 -tags test -coverprofile=coverage/coverage.out $$(go list ./... | grep -v 'consumers\|readers\|postgres\|internal\|opcua\|cmd') +test_api: + @which st > /dev/null || (echo "schemathesis not found, please install it from https://github.com/schemathesis/schemathesis#getting-started" && exit 1) + st run api/openapi/users.yml \ + --checks all \ + --base-url http://localhost:9002 \ + --header "Authorization: Bearer $(USER_TOKEN)" \ + --contrib-unique-data --contrib-openapi-formats-uuid \ + --hypothesis-suppress-health-check=filter_too_much \ + --stateful=links + proto: protoc -I. --go_out=. --go_opt=paths=source_relative pkg/messaging/*.proto protoc -I. --go_out=. --go_opt=paths=source_relative --go-grpc_out=. --go-grpc_opt=paths=source_relative ./*.proto diff --git a/api/openapi/users.yml b/api/openapi/users.yml index e4dd97b3b..3c2bfb5dc 100644 --- a/api/openapi/users.yml +++ b/api/openapi/users.yml @@ -34,6 +34,7 @@ tags: paths: /users: post: + operationId: createUser tags: - Users summary: Registers user account @@ -49,14 +50,19 @@ paths: description: Failed due to malformed JSON. "401": description: Missing or invalid access token provided. + "403": + description: Failed to perform authorization over the entity. "409": description: Failed due to using an existing identity. "415": description: Missing or invalid content type. + "422": + description: Database can't process request. "500": $ref: "#/components/responses/ServiceError" get: + operationId: listUsers tags: - Users summary: List users @@ -93,6 +99,7 @@ paths: /users/profile: get: + operationId: getProfile summary: Gets info on currently logged in user. description: | Gets info on currently logged in user. Info is obtained using @@ -113,6 +120,7 @@ paths: /users/{userID}: get: + operationId: getUser summary: Retrieves a user description: | Retrieves a specific user that is identifier by the user ID. @@ -129,6 +137,8 @@ paths: description: Failed due to malformed query parameters. "401": description: Missing or invalid access token provided. + "403": + description: Failed to perform authorization over the entity. "404": description: A non-existent entity request. "422": @@ -137,6 +147,7 @@ paths: $ref: "#/components/responses/ServiceError" patch: + operationId: updateUser summary: Updates name and metadata of the user. description: | Updates name and metadata of the user with provided ID. Name and metadata @@ -154,15 +165,24 @@ paths: $ref: "#/components/responses/UserRes" "400": description: Failed due to malformed JSON. + "403": + description: Failed to perform authorization over the entity. "404": description: Failed due to non existing user. "401": description: Missing or invalid access token provided. + "409": + description: Failed due to using an existing identity. + "415": + description: Missing or invalid content type. + "422": + description: Database can't process request. "500": $ref: "#/components/responses/ServiceError" /users/{userID}/tags: patch: + operationId: updateUserTags summary: Updates tags the user. description: | Updates tags of the user with provided ID. Tags is updated using @@ -180,15 +200,22 @@ paths: $ref: "#/components/responses/UserRes" "400": description: Failed due to malformed JSON. + "403": + description: Failed to perform authorization over the entity. "404": description: Failed due to non existing user. "401": description: Missing or invalid access token provided. + "415": + description: Missing or invalid content type. + "422": + description: Database can't process request. "500": $ref: "#/components/responses/ServiceError" /users/{userID}/identity: patch: + operationId: updateUserIdentity summary: Updates Identity of the user. description: | Updates identity of the user with provided ID. Identity is @@ -206,15 +233,24 @@ paths: $ref: "#/components/responses/UserRes" "400": description: Failed due to malformed JSON. + "403": + description: Failed to perform authorization over the entity. "404": description: Failed due to non existing user. "401": description: Missing or invalid access token provided. + "409": + description: Failed due to using an existing identity. + "415": + description: Missing or invalid content type. + "422": + description: Database can't process request. "500": $ref: "#/components/responses/ServiceError" /users/{userID}/role: patch: + operationId: updateUserRole summary: Updates the user role. description: | Updates role for the user with provided ID. @@ -231,15 +267,22 @@ paths: $ref: "#/components/responses/UserRes" "400": description: Failed due to malformed JSON. + "403": + description: Failed to perform authorization over the entity. "404": description: Failed due to non existing user. "401": description: Missing or invalid access token provided. + "415": + description: Missing or invalid content type. + "422": + description: Database can't process request. "500": $ref: "#/components/responses/ServiceError" /users/{userID}/disable: post: + operationId: disableUser summary: Disables a user description: | Disables a specific user that is identifier by the user ID. @@ -256,8 +299,14 @@ paths: description: Failed due to malformed query parameters. "401": description: Missing or invalid access token provided. + "403": + description: Failed to perform authorization over the entity. "404": description: A non-existent entity request. + "409": + description: Failed due to already disabled user. + "415": + description: Missing or invalid content type. "422": description: Database can't process request. "500": @@ -265,6 +314,7 @@ paths: /users/{userID}/enable: post: + operationId: enableUser summary: Enables a user description: | Enables a specific user that is identifier by the user ID. @@ -281,8 +331,14 @@ paths: description: Failed due to malformed query parameters. "401": description: Missing or invalid access token provided. + "403": + description: Failed to perform authorization over the entity. "404": description: A non-existent entity request. + "409": + description: Failed due to already enabled user. + "415": + description: Missing or invalid content type. "422": description: Database can't process request. "500": @@ -290,6 +346,7 @@ paths: /users/secret: patch: + operationId: updateUserSecret summary: Updates Secret of currently logged in user. description: | Updates secret of currently logged in user. Secret is updated using @@ -305,15 +362,20 @@ paths: $ref: "#/components/responses/UserRes" "400": description: Failed due to malformed JSON. - "404": - description: Failed due to non existing user. "401": description: Missing or invalid access token provided. + "404": + description: Failed due to non existing user. + "415": + description: Missing or invalid content type. + "422": + description: Database can't process request. "500": $ref: "#/components/responses/ServiceError" /password/reset-request: post: + operationId: requestPasswordReset summary: User password reset request description: | Generates a reset token and sends and @@ -331,11 +393,14 @@ paths: description: Failed due to malformed JSON. "415": description: Missing or invalid content type. + "422": + description: Database can't process request. "500": $ref: "#/components/responses/ServiceError" /password/reset: put: + operationId: resetPassword summary: User password reset endpoint description: | When user gets reset token, after he submitted @@ -350,13 +415,18 @@ paths: description: User link . "400": description: Failed due to malformed JSON. + "401": + description: Missing or invalid access token provided. "415": description: Missing or invalid content type. + "422": + description: Database can't process request. "500": $ref: "#/components/responses/ServiceError" /groups/{groupID}/users: get: + operationId: listUsersInGroup tags: - Users summary: List users in a group @@ -383,6 +453,8 @@ paths: description: | Missing or invalid access token provided. This endpoint is available only for administrators. + "403": + description: Failed to perform authorization over the entity. "404": description: A non-existent entity request. "422": @@ -392,6 +464,7 @@ paths: /channels/{channelID}/users: get: + operationId: listUsersInChannel tags: - Users summary: List users in a channel @@ -418,6 +491,8 @@ paths: description: | Missing or invalid access token provided. This endpoint is available only for administrators. + "403": + description: Failed to perform authorization over the entity. "404": description: A non-existent entity request. "422": @@ -427,6 +502,7 @@ paths: /users/tokens/issue: post: + operationId: issueToken summary: Issue Token description: | Issue Access and Refresh Token used for authenticating into the system. @@ -437,8 +513,12 @@ paths: responses: "200": $ref: "#/components/responses/TokenRes" + "400": + description: Failed due to malformed JSON. "404": description: A non-existent entity request. + "415": + description: Missing or invalid content type. "422": description: Database can't process request. "500": @@ -446,6 +526,7 @@ paths: /users/tokens/refresh: post: + operationId: refreshToken summary: Refresh Token description: | Refreshes Access and Refresh Token used for authenticating into the system. @@ -456,8 +537,12 @@ paths: responses: "200": $ref: "#/components/responses/TokenRes" + "400": + description: Failed due to malformed JSON. "404": description: A non-existent entity request. + "415": + description: Missing or invalid content type. "422": description: Database can't process request. "500": @@ -465,6 +550,7 @@ paths: /groups: post: + operationId: createGroup tags: - Groups summary: Creates new group @@ -482,14 +568,19 @@ paths: description: Failed due to malformed JSON. "401": description: Missing or invalid access token provided. + "403": + description: Failed to perform authorization over the entity. "409": description: Failed due to using an existing identity. "415": description: Missing or invalid content type. + "422": + description: Database can't process request. "500": $ref: "#/components/responses/ServiceError" get: + operationId: listGroups summary: Lists groups. description: | Lists groups up to a max level of hierarchy that can be fetched in one @@ -515,13 +606,18 @@ paths: description: Failed due to malformed query parameters. "401": description: Missing or invalid access token provided. + "403": + description: Failed to perform authorization over the entity. "404": description: Group does not exist. + "422": + description: Database can't process request. "500": $ref: "#/components/responses/ServiceError" /groups/{groupID}: get: + operationId: getGroup summary: Gets group info. description: | Gets info on a group specified by id. @@ -538,12 +634,17 @@ paths: description: Failed due to malformed query parameters. "401": description: Missing or invalid access token provided. + "403": + description: Failed to perform authorization over the entity. "404": description: Group does not exist. + "422": + description: Database can't process request. "500": $ref: "#/components/responses/ServiceError" put: + operationId: updateGroup summary: Updates group data. description: | Updates Name, Description or Metadata of a group. @@ -562,8 +663,16 @@ paths: description: Failed due to malformed query parameters. "401": description: Missing or invalid access token provided. + "403": + description: Failed to perform authorization over the entity. "404": description: Group does not exist. + "409": + description: Failed due to using an existing identity. + "415": + description: Missing or invalid content type. + "422": + description: Database can't process request. "500": $ref: "#/components/responses/ServiceError" delete: @@ -589,6 +698,7 @@ paths: /groups/{groupID}/children: get: + operationId: listChildren summary: List children of a certain group description: | Lists groups up to a max level of hierarchy that can be fetched in one @@ -615,13 +725,18 @@ paths: description: Failed due to malformed query parameters. "401": description: Missing or invalid access token provided. + "403": + description: Failed to perform authorization over the entity. "404": description: Group does not exist. + "422": + description: Database can't process request. "500": $ref: "#/components/responses/ServiceError" /groups/{groupID}/parents: get: + operationId: listParents summary: List parents of a certain group description: | Lists groups up to a max level of hierarchy that can be fetched in one @@ -648,13 +763,18 @@ paths: description: Failed due to malformed query parameters. "401": description: Missing or invalid access token provided. + "403": + description: Failed to perform authorization over the entity. "404": description: Group does not exist. + "422": + description: Database can't process request. "500": $ref: "#/components/responses/ServiceError" /groups/{groupID}/enable: post: + operationId: enableGroup summary: Enables a group description: | Enables a specific group that is identifier by the group ID. @@ -671,8 +791,14 @@ paths: description: Failed due to malformed query parameters. "401": description: Missing or invalid access token provided. + "403": + description: Failed to perform authorization over the entity. "404": description: A non-existent entity request. + "409": + description: Failed due to already enabled group. + "415": + description: Missing or invalid content type. "422": description: Database can't process request. "500": @@ -680,6 +806,7 @@ paths: /groups/{groupID}/disable: post: + operationId: disableGroup summary: Disables a group description: | Disables a specific group that is identifier by the group ID. @@ -696,8 +823,14 @@ paths: description: Failed due to malformed query parameters. "401": description: Missing or invalid access token provided. + "403": + description: Failed to perform authorization over the entity. "404": description: A non-existent entity request. + "409": + description: Failed due to already disabled group. + "415": + description: Missing or invalid content type. "422": description: Database can't process request. "500": @@ -705,6 +838,7 @@ paths: /groups/{groupID}/users/assign: post: + operationId: assignUser summary: Assigns a user to a group description: | Assigns a specific user to a group that is identifier by the group ID. @@ -723,8 +857,12 @@ paths: description: Failed due to malformed group's ID. "401": description: Missing or invalid access token provided. + "403": + description: Failed to perform authorization over the entity. "404": description: A non-existent entity request. + "415": + description: Missing or invalid content type. "422": description: Database can't process request. "500": @@ -732,6 +870,7 @@ paths: /groups/{groupID}/users/unassign: post: + operationId: unassignUser summary: Unassigns a user to a group description: | Unassigns a specific user to a group that is identifier by the group ID. @@ -750,8 +889,12 @@ paths: description: Failed due to malformed group's ID. "401": description: Missing or invalid access token provided. + "403": + description: Failed to perform authorization over the entity. "404": description: A non-existent entity request. + "415": + description: Missing or invalid content type. "422": description: Database can't process request. "500": @@ -759,6 +902,7 @@ paths: /channels/{memberID}/groups: get: + operationId: listGroupsInChannel summary: Get group associated with the member description: | Gets groups associated with the channel member specified by id. @@ -780,13 +924,18 @@ paths: description: Failed due to malformed query parameters. "401": description: Missing or invalid access token provided. + "403": + description: Failed to perform authorization over the entity. "404": description: Group does not exist. + "422": + description: Database can't process request. "500": $ref: "#/components/responses/ServiceError" /users/{memberID}/groups: get: + operationId: listGroupsByUser summary: Get group associated with the member description: | Gets groups associated with the user member specified by id. @@ -808,8 +957,12 @@ paths: description: Failed due to malformed query parameters. "401": description: Missing or invalid access token provided. + "403": + description: Failed to perform authorization over the entity. "404": description: Group does not exist. + "422": + description: Database can't process request. "500": $ref: "#/components/responses/ServiceError" /domains/{domainID}/users: @@ -845,6 +998,7 @@ paths: $ref: "#/components/responses/ServiceError" /health: get: + operationId: health summary: Retrieves service health check info. tags: - health @@ -1128,7 +1282,7 @@ components: required: - groups - total - - level + - offset MembersPage: type: object @@ -1212,7 +1366,7 @@ components: properties: role: type: string - enum: ["admin","user"] + enum: ["admin", "user"] example: user description: User role example. required: @@ -1351,6 +1505,9 @@ components: schema: type: string format: uuid + minLength: 36 + maxLength: 36 + pattern: "^[a-f0-9]{8}-[a-f0-9]{4}-[1-5][a-f0-9]{3}-[89ab][a-f0-9]{3}-[a-f0-9]{12}$" required: true example: bb7edb32-2eac-4aad-aebe-ed96fe073879 @@ -1369,6 +1526,7 @@ components: in: query schema: type: string + pattern: "^[^\u0000-\u001F]*$" required: false example: "admin@example.com" @@ -1429,6 +1587,9 @@ components: schema: type: string format: uuid + minLength: 36 + maxLength: 36 + pattern: "^[a-f0-9]{8}-[a-f0-9]{4}-[1-5][a-f0-9]{3}-[89ab][a-f0-9]{3}-[a-f0-9]{12}$" required: true example: bb7edb32-2eac-4aad-aebe-ed96fe073879 @@ -1439,6 +1600,9 @@ components: schema: type: string format: uuid + minLength: 36 + maxLength: 36 + pattern: "^[a-f0-9]{8}-[a-f0-9]{4}-[1-5][a-f0-9]{3}-[89ab][a-f0-9]{3}-[a-f0-9]{12}$" required: true example: bb7edb32-2eac-4aad-aebe-ed96fe073879 @@ -1449,6 +1613,9 @@ components: schema: type: string format: uuid + minLength: 36 + maxLength: 36 + pattern: "^[a-f0-9]{8}-[a-f0-9]{4}-[1-5][a-f0-9]{3}-[89ab][a-f0-9]{3}-[a-f0-9]{12}$" required: true example: bb7edb32-2eac-4aad-aebe-ed96fe073879 @@ -1667,6 +1834,43 @@ components: application/json: schema: $ref: "#/components/schemas/User" + links: + get: + operationId: getUser + parameters: + userID: $response.body#/id + get_groups: + operationId: listUsersInGroup + parameters: + groupID: $response.body#/id + get_channels: + operationId: listUsersInChannel + parameters: + channelID: $response.body#/id + update: + operationId: updateUser + parameters: + userID: $response.body#/id + update_tags: + operationId: updateUserTags + parameters: + userID: $response.body#/id + update_identity: + operationId: updateUserIdentity + parameters: + userID: $response.body#/id + update_role: + operationId: updateUserRole + parameters: + userID: $response.body#/id + disable: + operationId: disableUser + parameters: + userID: $response.body#/id + enable: + operationId: enableUser + parameters: + userID: $response.body#/id UserRes: description: Data retrieved. @@ -1674,6 +1878,15 @@ components: application/json: schema: $ref: "#/components/schemas/User" + links: + get_groups: + operationId: listUsersInGroup + parameters: + groupID: $response.body#/id + get_channels: + operationId: listUsersInChannel + parameters: + channelID: $response.body#/id UserPageRes: description: Data retrieved. @@ -1694,6 +1907,47 @@ components: application/json: schema: $ref: "#/components/schemas/Group" + links: + get: + operationId: getGroup + parameters: + groupID: $response.body#/id + get_children: + operationId: listChildren + parameters: + groupID: $response.body#/id + get_parent: + operationId: listParents + parameters: + groupID: $response.body#/id + get_channels: + operationId: listGroupsInChannel + parameters: + memberID: $response.body#/id + get_users: + operationId: listGroupsByUser + parameters: + memberID: $response.body#/id + update: + operationId: updateGroup + parameters: + groupID: $response.body#/id + disable: + operationId: disableGroup + parameters: + groupID: $response.body#/id + enable: + operationId: enableGroup + parameters: + groupID: $response.body#/id + assign: + operationId: assignUser + parameters: + groupID: $response.body#/id + unassign: + operationId: unassignUser + parameters: + groupID: $response.body#/id GroupRes: description: Data retrieved. @@ -1701,6 +1955,31 @@ components: application/json: schema: $ref: "#/components/schemas/Group" + links: + get_children: + operationId: listChildren + parameters: + groupID: $response.body#/id + get_parent: + operationId: listParents + parameters: + groupID: $response.body#/id + get_channels: + operationId: listGroupsInChannel + parameters: + memberID: $response.body#/id + get_users: + operationId: listGroupsByUser + parameters: + memberID: $response.body#/id + assign: + operationId: assignUser + parameters: + groupID: $response.body#/id + unassign: + operationId: unassignUser + parameters: + groupID: $response.body#/id GroupPageRes: description: Data retrieved. diff --git a/auth/postgres/domains.go b/auth/postgres/domains.go index b3fe87566..de4624c20 100644 --- a/auth/postgres/domains.go +++ b/auth/postgres/domains.go @@ -107,13 +107,13 @@ func (repo domainRepo) RetrievePermissions(ctx context.Context, subject, id stri rows, err := repo.db.QueryxContext(ctx, q, id, subject) if err != nil { - return []string{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return []string{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } defer rows.Close() domains, err := repo.processRows(rows) if err != nil { - return []string{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return []string{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } permissions := []string{} @@ -142,18 +142,18 @@ func (repo domainRepo) RetrieveAllByIDs(ctx context.Context, pm auth.Page) (auth dbPage, err := toDBClientsPage(pm) if err != nil { - return auth.DomainsPage{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return auth.DomainsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } rows, err := repo.db.NamedQueryContext(ctx, q, dbPage) if err != nil { - return auth.DomainsPage{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return auth.DomainsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } defer rows.Close() domains, err := repo.processRows(rows) if err != nil { - return auth.DomainsPage{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return auth.DomainsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } cq := "SELECT COUNT(*) FROM domains d" @@ -163,7 +163,7 @@ func (repo domainRepo) RetrieveAllByIDs(ctx context.Context, pm auth.Page) (auth total, err := postgres.Total(ctx, repo.db, cq, dbPage) if err != nil { - return auth.DomainsPage{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return auth.DomainsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } pm.Total = total @@ -199,18 +199,18 @@ func (repo domainRepo) ListDomains(ctx context.Context, pm auth.Page) (auth.Doma dbPage, err := toDBClientsPage(pm) if err != nil { - return auth.DomainsPage{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return auth.DomainsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } rows, err := repo.db.NamedQueryContext(ctx, q, dbPage) if err != nil { - return auth.DomainsPage{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return auth.DomainsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } defer rows.Close() domains, err := repo.processRows(rows) if err != nil { - return auth.DomainsPage{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return auth.DomainsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } cq := "SELECT COUNT(*) FROM domains d JOIN policies pc ON pc.object_id = d.id" @@ -220,7 +220,7 @@ func (repo domainRepo) ListDomains(ctx context.Context, pm auth.Page) (auth.Doma total, err := postgres.Total(ctx, repo.db, cq, dbPage) if err != nil { - return auth.DomainsPage{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return auth.DomainsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } pm.Total = total diff --git a/auth/service.go b/auth/service.go index 8a1700a13..1c013131c 100644 --- a/auth/service.go +++ b/auth/service.go @@ -18,27 +18,6 @@ import ( const recoveryDuration = 5 * time.Minute var ( - errRollbackPolicy = errors.New("failed to rollback policy") - errRemoveLocalPolicy = errors.New("failed to remove from local policy copy") - errRemovePolicyEngine = errors.New("failed to remove from policy engine") -) - -var ( - // ErrFailedToRetrieveMembers failed to retrieve group members. - ErrFailedToRetrieveMembers = errors.New("failed to retrieve group members") - - // ErrFailedToRetrieveMembership failed to retrieve memberships. - ErrFailedToRetrieveMembership = errors.New("failed to retrieve memberships") - - // ErrFailedToRetrieveAll failed to retrieve groups. - ErrFailedToRetrieveAll = errors.New("failed to retrieve all groups") - - // ErrFailedToRetrieveParents failed to retrieve groups. - ErrFailedToRetrieveParents = errors.New("failed to retrieve all groups") - - // ErrFailedToRetrieveChildren failed to retrieve groups. - ErrFailedToRetrieveChildren = errors.New("failed to retrieve all groups") - // ErrExpiry indicates that the token is expired. ErrExpiry = errors.New("token is expired") @@ -51,6 +30,9 @@ var ( errCreateDomainPolicy = errors.New("failed to create domain policy") errAddPolicies = errors.New("failed to add policies") errRemovePolicies = errors.New("failed to remove the policies") + errRollbackPolicy = errors.New("failed to rollback policy") + errRemoveLocalPolicy = errors.New("failed to remove from local policy copy") + errRemovePolicyEngine = errors.New("failed to remove from policy engine") ) // Authn specifies an API that must be fullfiled by the domain service @@ -358,7 +340,12 @@ func (svc service) CountSubjects(ctx context.Context, pr PolicyReq) (int, error) } func (svc service) ListPermissions(ctx context.Context, pr PolicyReq, filterPermisions []string) (Permissions, error) { - return svc.agent.RetrievePermissions(ctx, pr, filterPermisions) + pers, err := svc.agent.RetrievePermissions(ctx, pr, filterPermisions) + if err != nil { + return []string{}, errors.Wrap(svcerr.ErrViewEntity, err) + } + + return pers, nil } func (svc service) tmpKey(duration time.Duration, key Key) (Token, error) { diff --git a/internal/api/common.go b/internal/api/common.go index 0f5193f16..eb22bcfa5 100644 --- a/internal/api/common.go +++ b/internal/api/common.go @@ -10,7 +10,6 @@ import ( "github.com/absmach/magistrala" "github.com/absmach/magistrala/internal/apiutil" - "github.com/absmach/magistrala/internal/postgres" mgclients "github.com/absmach/magistrala/pkg/clients" "github.com/absmach/magistrala/pkg/errors" svcerr "github.com/absmach/magistrala/pkg/errors/service" @@ -116,22 +115,34 @@ func EncodeError(_ context.Context, err error, w http.ResponseWriter) { errors.Contains(err, apiutil.ErrLimitSize), errors.Contains(err, apiutil.ErrBearerKey), errors.Contains(err, apiutil.ErrNameSize), - errors.Contains(err, svcerr.ErrInvalidStatus), errors.Contains(err, apiutil.ErrInvalidIDFormat), - errors.Contains(err, apiutil.ErrInvalidQueryParams), errors.Contains(err, apiutil.ErrInvalidStatus), + errors.Contains(err, svcerr.ErrInvalidStatus), + errors.Contains(err, apiutil.ErrInvitationState), + errors.Contains(err, apiutil.ErrInvalidRole), + errors.Contains(err, apiutil.ErrMissingEmail), + errors.Contains(err, apiutil.ErrMissingHost), + errors.Contains(err, apiutil.ErrMissingIdentity), + errors.Contains(err, apiutil.ErrMissingSecret), + errors.Contains(err, apiutil.ErrMissingPass), + errors.Contains(err, apiutil.ErrMissingConfPass), + errors.Contains(err, apiutil.ErrInvalidResetPass), errors.Contains(err, apiutil.ErrMissingRelation), + errors.Contains(err, errors.ErrPasswordFormat), + errors.Contains(err, apiutil.ErrInvalidLevel), + errors.Contains(err, apiutil.ErrInvalidQueryParams), errors.Contains(err, apiutil.ErrValidation): w.WriteHeader(http.StatusBadRequest) case errors.Contains(err, svcerr.ErrAuthentication), errors.Contains(err, errors.ErrAuthentication), + errors.Contains(err, errors.ErrLogin), errors.Contains(err, apiutil.ErrBearerToken): w.WriteHeader(http.StatusUnauthorized) case errors.Contains(err, svcerr.ErrNotFound): w.WriteHeader(http.StatusNotFound) case errors.Contains(err, svcerr.ErrConflict), - errors.Contains(err, postgres.ErrMemberAlreadyAssigned), - errors.Contains(err, errors.ErrConflict): + errors.Contains(err, errors.ErrConflict), + errors.Contains(err, errors.ErrStatusAlreadyAssigned): w.WriteHeader(http.StatusConflict) case errors.Contains(err, svcerr.ErrAuthorization), errors.Contains(err, errors.ErrAuthorization), @@ -141,9 +152,12 @@ func EncodeError(_ context.Context, err error, w http.ResponseWriter) { w.WriteHeader(http.StatusUnsupportedMediaType) case errors.Contains(err, svcerr.ErrCreateEntity), errors.Contains(err, svcerr.ErrUpdateEntity), + errors.Contains(err, svcerr.ErrFailedUpdateRole), errors.Contains(err, svcerr.ErrViewEntity), + errors.Contains(err, svcerr.ErrAddPolicies), + errors.Contains(err, svcerr.ErrDeletePolicies), errors.Contains(err, svcerr.ErrRemoveEntity): - w.WriteHeader(http.StatusInternalServerError) + w.WriteHeader(http.StatusUnprocessableEntity) default: w.WriteHeader(http.StatusInternalServerError) } diff --git a/internal/api/common_test.go b/internal/api/common_test.go index a29bba99c..abf97d119 100644 --- a/internal/api/common_test.go +++ b/internal/api/common_test.go @@ -294,14 +294,14 @@ func TestEncodeError(t *testing.T) { code: http.StatusUnsupportedMediaType, }, { - desc: "InternalServerError", + desc: "StatusUnprocessableEntity", errs: []error{ svcerr.ErrCreateEntity, svcerr.ErrUpdateEntity, svcerr.ErrViewEntity, svcerr.ErrRemoveEntity, }, - code: http.StatusInternalServerError, + code: http.StatusUnprocessableEntity, }, { desc: "InternalServerError", diff --git a/internal/apiutil/errors.go b/internal/apiutil/errors.go index bca1c710e..e2346edd2 100644 --- a/internal/apiutil/errors.go +++ b/internal/apiutil/errors.go @@ -114,12 +114,12 @@ var ( // ErrMissingRelation indicates missing relation. ErrMissingRelation = errors.New("missing relation") + // ErrInvalidRelation indicates an invalid relation. + ErrInvalidRelation = errors.New("invalid relation") + // ErrInvalidAPIKey indicates an invalid API key type. ErrInvalidAPIKey = errors.New("invalid api key type") - // ErrMaxLevelExceeded indicates an invalid group level. - ErrMaxLevelExceeded = errors.New("invalid group level (should be lower than 5)") - // ErrBootstrapState indicates an invalid bootstrap state. ErrBootstrapState = errors.New("invalid bootstrap state") diff --git a/internal/groups/api/responses.go b/internal/groups/api/responses.go index 08b4b314c..a9fbcc5ef 100644 --- a/internal/groups/api/responses.go +++ b/internal/groups/api/responses.go @@ -86,9 +86,9 @@ type groupPageRes struct { } type pageRes struct { - Limit uint64 `json:"limit"` + Limit uint64 `json:"limit,omitempty"` Offset uint64 `json:"offset"` - Total uint64 `json:"total,omitempty"` + Total uint64 `json:"total"` Level uint64 `json:"level,omitempty"` } diff --git a/internal/groups/postgres/groups.go b/internal/groups/postgres/groups.go index b653fb701..756d73164 100644 --- a/internal/groups/postgres/groups.go +++ b/internal/groups/postgres/groups.go @@ -160,17 +160,17 @@ func (repo groupRepository) RetrieveAll(ctx context.Context, gm mggroups.Page) ( dbPage, err := toDBGroupPage(gm) if err != nil { - return mggroups.Page{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return mggroups.Page{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } rows, err := repo.db.NamedQueryContext(ctx, q, dbPage) if err != nil { - return mggroups.Page{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return mggroups.Page{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } defer rows.Close() items, err := repo.processRows(rows) if err != nil { - return mggroups.Page{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return mggroups.Page{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } cq := "SELECT COUNT(*) FROM groups g" @@ -180,7 +180,7 @@ func (repo groupRepository) RetrieveAll(ctx context.Context, gm mggroups.Page) ( total, err := postgres.Total(ctx, repo.db, cq, dbPage) if err != nil { - return mggroups.Page{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return mggroups.Page{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } page := gm @@ -208,17 +208,17 @@ func (repo groupRepository) RetrieveByIDs(ctx context.Context, gm mggroups.Page, dbPage, err := toDBGroupPage(gm) if err != nil { - return mggroups.Page{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return mggroups.Page{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } rows, err := repo.db.NamedQueryContext(ctx, q, dbPage) if err != nil { - return mggroups.Page{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return mggroups.Page{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } defer rows.Close() items, err := repo.processRows(rows) if err != nil { - return mggroups.Page{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return mggroups.Page{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } cq := "SELECT COUNT(*) FROM groups g" @@ -228,7 +228,7 @@ func (repo groupRepository) RetrieveByIDs(ctx context.Context, gm mggroups.Page, total, err := postgres.Total(ctx, repo.db, cq, dbPage) if err != nil { - return mggroups.Page{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return mggroups.Page{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } page := gm diff --git a/internal/groups/service.go b/internal/groups/service.go index 8dd6d08f0..338d67c3e 100644 --- a/internal/groups/service.go +++ b/internal/groups/service.go @@ -13,6 +13,7 @@ import ( "github.com/absmach/magistrala/internal/apiutil" mgclients "github.com/absmach/magistrala/pkg/clients" "github.com/absmach/magistrala/pkg/errors" + svcerr "github.com/absmach/magistrala/pkg/errors/service" "github.com/absmach/magistrala/pkg/groups" "golang.org/x/sync/errgroup" ) @@ -20,8 +21,6 @@ import ( var ( errParentUnAuthz = errors.New("failed to authorize parent group") errMemberKind = errors.New("invalid member kind") - errAddPolicies = errors.New("failed to add policies") - errDeletePolicies = errors.New("failed to delete policies") errRetrieveGroups = errors.New("failed to retrieve groups") errGroupIDs = errors.New("invalid group ids") ) @@ -284,10 +283,10 @@ func (svc service) checkSuperAdmin(ctx context.Context, userID string) error { Object: auth.MagistralaObject, }) if err != nil { - return err + return errors.Wrap(svcerr.ErrAuthorization, err) } if !res.Authorized { - return errors.ErrAuthorization + return svcerr.ErrAuthorization } return nil } @@ -445,7 +444,7 @@ func (svc service) Assign(ctx context.Context, token, groupID, relation, memberK } if _, err := svc.auth.AddPolicies(ctx, &policies); err != nil { - return errors.Wrap(errAddPolicies, err) + return errors.Wrap(svcerr.ErrAddPolicies, err) } return nil @@ -596,7 +595,7 @@ func (svc service) Unassign(ctx context.Context, token, groupID, relation, membe } if _, err := svc.auth.DeletePolicies(ctx, &policies); err != nil { - return errors.Wrap(errDeletePolicies, err) + return errors.Wrap(svcerr.ErrDeletePolicies, err) } return nil } @@ -694,7 +693,7 @@ func (svc service) changeGroupStatus(ctx context.Context, token string, group gr return groups.Group{}, err } if dbGroup.Status == group.Status { - return groups.Group{}, mgclients.ErrStatusAlreadyAssigned + return groups.Group{}, errors.ErrStatusAlreadyAssigned } group.UpdatedBy = id @@ -723,10 +722,10 @@ func (svc service) authorizeToken(ctx context.Context, subjectType, subject, per } res, err := svc.auth.Authorize(ctx, req) if err != nil { - return "", err + return "", errors.Wrap(svcerr.ErrAuthorization, err) } if !res.GetAuthorized() { - return "", errors.ErrAuthorization + return "", svcerr.ErrAuthorization } return res.GetId(), nil } @@ -743,10 +742,10 @@ func (svc service) authorizeKind(ctx context.Context, domainID, subjectType, sub } res, err := svc.auth.Authorize(ctx, req) if err != nil { - return "", err + return "", errors.Wrap(svcerr.ErrAuthorization, err) } if !res.GetAuthorized() { - return "", errors.ErrAuthorization + return "", svcerr.ErrAuthorization } return res.GetId(), nil } diff --git a/internal/groups/service_test.go b/internal/groups/service_test.go index d28b92ef1..f158bc54a 100644 --- a/internal/groups/service_test.go +++ b/internal/groups/service_test.go @@ -17,7 +17,6 @@ import ( "github.com/absmach/magistrala/internal/groups" "github.com/absmach/magistrala/internal/testsutil" "github.com/absmach/magistrala/pkg/clients" - mgclients "github.com/absmach/magistrala/pkg/clients" "github.com/absmach/magistrala/pkg/errors" mggroups "github.com/absmach/magistrala/pkg/groups" "github.com/absmach/magistrala/pkg/groups/mocks" @@ -585,7 +584,7 @@ func TestEnableGroup(t *testing.T) { retrieveResp: mggroups.Group{ Status: clients.Status(groups.EnabledStatus), }, - err: mgclients.ErrStatusAlreadyAssigned, + err: errors.ErrStatusAlreadyAssigned, }, { desc: "with retrieve error", @@ -685,7 +684,7 @@ func TestDisableGroup(t *testing.T) { retrieveResp: mggroups.Group{ Status: clients.Status(groups.DisabledStatus), }, - err: mgclients.ErrStatusAlreadyAssigned, + err: errors.ErrStatusAlreadyAssigned, }, { desc: "with retrieve error", diff --git a/internal/postgres/common.go b/internal/postgres/common.go index 1c5b18371..c34ac9fb4 100644 --- a/internal/postgres/common.go +++ b/internal/postgres/common.go @@ -6,42 +6,9 @@ package postgres import ( "context" "encoding/json" - "errors" "fmt" ) -var ( - // ErrAssignToGroup indicates failure to assign member to a group. - ErrAssignToGroup = errors.New("failed to assign member to a group") - - // ErrUnassignFromGroup indicates failure to unassign member from a group. - ErrUnassignFromGroup = errors.New("failed to unassign member from a group") - - // ErrMissingParent indicates that parent can't be found. - ErrMissingParent = errors.New("failed to retrieve parent") - - // ErrGroupNotEmpty indicates group is not empty, can't be deleted. - ErrGroupNotEmpty = errors.New("group is not empty") - - // ErrMemberAlreadyAssigned indicates that members is already assigned. - ErrMemberAlreadyAssigned = errors.New("member is already assigned") - - // ErrFailedToRetrieveMembers failed to retrieve group members. - ErrFailedToRetrieveMembers = errors.New("failed to retrieve group members") - - // ErrFailedToRetrieveMembership failed to retrieve memberships. - ErrFailedToRetrieveMembership = errors.New("failed to retrieve memberships") - - // ErrFailedToRetrieveAll failed to retrieve groups. - ErrFailedToRetrieveAll = errors.New("failed to retrieve all groups") - - // ErrFailedToRetrieveParents failed to retrieve groups. - ErrFailedToRetrieveParents = errors.New("failed to retrieve all groups") - - // ErrFailedToRetrieveChildren failed to retrieve groups. - ErrFailedToRetrieveChildren = errors.New("failed to retrieve all groups") -) - func CreateMetadataQuery(entity string, um map[string]interface{}) (string, []byte, error) { if len(um) == 0 { return "", nil, nil diff --git a/invitations/api/endpoint_test.go b/invitations/api/endpoint_test.go index fbf0cd4d0..bf48b965e 100644 --- a/invitations/api/endpoint_test.go +++ b/invitations/api/endpoint_test.go @@ -266,7 +266,7 @@ func TestListInvitation(t *testing.T) { desc: "with invalid state", token: validToken, query: "state=invalid", - status: http.StatusInternalServerError, + status: http.StatusBadRequest, contentType: validContenType, svcErr: nil, }, diff --git a/invitations/api/requests_test.go b/invitations/api/requests_test.go index 1f1192a92..a2e22660a 100644 --- a/invitations/api/requests_test.go +++ b/invitations/api/requests_test.go @@ -4,7 +4,6 @@ package api import ( - "errors" "fmt" "testing" @@ -14,11 +13,7 @@ import ( "github.com/stretchr/testify/assert" ) -var ( - errMissingRelation = errors.New("missing relation") - errInvalidRelation = errors.New("invalid relation") - valid = "valid" -) +var valid = "valid" func TestSendInvitationReqValidation(t *testing.T) { cases := []struct { @@ -79,7 +74,7 @@ func TestSendInvitationReqValidation(t *testing.T) { Relation: "", Resend: true, }, - err: errMissingRelation, + err: apiutil.ErrMissingRelation, }, { desc: "invalid relation", @@ -90,7 +85,7 @@ func TestSendInvitationReqValidation(t *testing.T) { Relation: "invalid", Resend: true, }, - err: errInvalidRelation, + err: apiutil.ErrInvalidRelation, }, } diff --git a/invitations/invitations.go b/invitations/invitations.go index 354dd3108..0cf6c5a2a 100644 --- a/invitations/invitations.go +++ b/invitations/invitations.go @@ -6,15 +6,10 @@ package invitations import ( "context" "encoding/json" - "errors" "time" "github.com/absmach/magistrala/auth" -) - -var ( - errMissingRelation = errors.New("missing relation") - errInvalidRelation = errors.New("invalid relation") + "github.com/absmach/magistrala/internal/apiutil" ) // Invitation is an invitation to join a domain. @@ -122,7 +117,7 @@ type Repository interface { // It returns an error if the relation is empty or invalid. func CheckRelation(relation string) error { if relation == "" { - return errMissingRelation + return apiutil.ErrMissingRelation } if relation != auth.AdministratorRelation && relation != auth.EditorRelation && @@ -133,7 +128,7 @@ func CheckRelation(relation string) error { relation != auth.RoleGroupRelation && relation != auth.GroupRelation && relation != auth.PlatformRelation { - return errInvalidRelation + return apiutil.ErrInvalidRelation } return nil diff --git a/invitations/invitations_test.go b/invitations/invitations_test.go index 942eaec76..2c367d7a7 100644 --- a/invitations/invitations_test.go +++ b/invitations/invitations_test.go @@ -4,19 +4,14 @@ package invitations_test import ( - "errors" "fmt" "testing" + "github.com/absmach/magistrala/internal/apiutil" "github.com/absmach/magistrala/invitations" "github.com/stretchr/testify/assert" ) -var ( - errMissingRelation = errors.New("missing relation") - errInvalidRelation = errors.New("invalid relation") -) - func TestInvitation_MarshalJSON(t *testing.T) { cases := []struct { desc string @@ -60,8 +55,8 @@ func TestCheckRelation(t *testing.T) { relation string err error }{ - {"", errMissingRelation}, - {"admin", errInvalidRelation}, + {"", apiutil.ErrMissingRelation}, + {"admin", apiutil.ErrInvalidRelation}, {"editor", nil}, {"viewer", nil}, {"member", nil}, diff --git a/invitations/service_test.go b/invitations/service_test.go index 87fda2ef7..58b7593e7 100644 --- a/invitations/service_test.go +++ b/invitations/service_test.go @@ -5,7 +5,6 @@ package invitations_test import ( "context" - "errors" "math/rand" "testing" "time" @@ -13,6 +12,7 @@ import ( "github.com/absmach/magistrala" "github.com/absmach/magistrala/auth" authmocks "github.com/absmach/magistrala/auth/mocks" + "github.com/absmach/magistrala/internal/apiutil" "github.com/absmach/magistrala/internal/testsutil" "github.com/absmach/magistrala/invitations" "github.com/absmach/magistrala/invitations/mocks" @@ -79,7 +79,7 @@ func TestSendInvitation(t *testing.T) { token: validToken, tokenUserID: testsutil.GenerateUUID(t), req: invitations.Invitation{Relation: "invalid"}, - err: errors.New("invalid relation"), + err: apiutil.ErrInvalidRelation, authNErr: nil, domainErr: nil, adminErr: nil, diff --git a/pkg/clients/postgres/clients.go b/pkg/clients/postgres/clients.go index 6ca5255d1..1ada83f48 100644 --- a/pkg/clients/postgres/clients.go +++ b/pkg/clients/postgres/clients.go @@ -149,11 +149,11 @@ func (repo *Repository) RetrieveAll(ctx context.Context, pm clients.Page) (clien dbPage, err := ToDBClientsPage(pm) if err != nil { - return clients.ClientsPage{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return clients.ClientsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } rows, err := repo.DB.NamedQueryContext(ctx, q, dbPage) if err != nil { - return clients.ClientsPage{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return clients.ClientsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } defer rows.Close() @@ -197,12 +197,12 @@ func (repo *Repository) RetrieveAllBasicInfo(ctx context.Context, pm clients.Pag dbPage, err := ToDBClientsPage(pm) if err != nil { - return clients.ClientsPage{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return clients.ClientsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } rows, err := repo.DB.NamedQueryContext(ctx, q, dbPage) if err != nil { - return clients.ClientsPage{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return clients.ClientsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } defer rows.Close() @@ -255,11 +255,11 @@ func (repo *Repository) RetrieveAllByIDs(ctx context.Context, pm clients.Page) ( dbPage, err := ToDBClientsPage(pm) if err != nil { - return clients.ClientsPage{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return clients.ClientsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } rows, err := repo.DB.NamedQueryContext(ctx, q, dbPage) if err != nil { - return clients.ClientsPage{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return clients.ClientsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } defer rows.Close() diff --git a/pkg/clients/status.go b/pkg/clients/status.go index 9d4c59e2f..82a716d51 100644 --- a/pkg/clients/status.go +++ b/pkg/clients/status.go @@ -5,7 +5,6 @@ package clients import ( "encoding/json" - "errors" "strings" "github.com/absmach/magistrala/internal/apiutil" @@ -36,9 +35,6 @@ const ( Unknown = "unknown" ) -// ErrStatusAlreadyAssigned indicated that the client or group has already been assigned the status. -var ErrStatusAlreadyAssigned = errors.New("status already assigned") - // String converts client/group status to string literal. func (s Status) String() string { switch s { diff --git a/pkg/errors/repository/types.go b/pkg/errors/repository/types.go index 2637d79b3..c27124343 100644 --- a/pkg/errors/repository/types.go +++ b/pkg/errors/repository/types.go @@ -48,4 +48,7 @@ var ( // ErrInvalidSecret indicates invalid secret. ErrInvalidSecret = errors.New("missing secret") + + // ErrFailedToRetrieveAllGroups failed to retrieve groups. + ErrFailedToRetrieveAllGroups = errors.New("failed to retrieve all groups") ) diff --git a/pkg/errors/service/types.go b/pkg/errors/service/types.go index a0a3049f2..f02fde297 100644 --- a/pkg/errors/service/types.go +++ b/pkg/errors/service/types.go @@ -45,4 +45,22 @@ var ( // ErrInvalidPolicy indicates that an invalid policy. ErrInvalidPolicy = errors.New("invalid policy") + + // ErrRecoveryToken indicates error in generating password recovery token. + ErrRecoveryToken = errors.New("failed to generate password recovery token") + + // ErrFailedPolicyUpdate indicates a failure to update user policy. + ErrFailedPolicyUpdate = errors.New("failed to update user policy") + + // ErrAddPolicies indicates failed to add policies. + ErrAddPolicies = errors.New("failed to add policies") + + // ErrDeletePolicies indicates failed to delete policies. + ErrDeletePolicies = errors.New("failed to delete policies") + + // ErrPasswordFormat indicates weak password. + ErrPasswordFormat = errors.New("password does not meet the requirements") + + // ErrFailedUpdateRole indicates a failure to update user role. + ErrFailedUpdateRole = errors.New("failed to update user role") ) diff --git a/pkg/errors/types.go b/pkg/errors/types.go index 684c4dec6..f49e71a79 100644 --- a/pkg/errors/types.go +++ b/pkg/errors/types.go @@ -45,6 +45,9 @@ var ( // ErrLogin indicates wrong login credentials. ErrLogin = New("invalid user id or secret") + // ErrPasswordFormat indicates weak password. + ErrPasswordFormat = errors.New("password does not meet the requirements") + // ErrUnsupportedContentType indicates invalid content type. ErrUnsupportedContentType = errors.New("invalid content type") @@ -53,4 +56,7 @@ var ( // ErrEmptyPath indicates empty file path. ErrEmptyPath = errors.New("empty file path") + + // ErrStatusAlreadyAssigned indicated that the client or group has already been assigned the status. + ErrStatusAlreadyAssigned = errors.New("status already assigned") ) diff --git a/pkg/groups/errors.go b/pkg/groups/errors.go index 24b1a29d9..b6665fa0b 100644 --- a/pkg/groups/errors.go +++ b/pkg/groups/errors.go @@ -14,7 +14,4 @@ var ( // ErrDisableGroup indicates error in disabling group. ErrDisableGroup = errors.New("failed to disable group") - - // ErrStatusAlreadyAssigned indicated that the group has already been assigned the status. - ErrStatusAlreadyAssigned = errors.New("status already assigned") ) diff --git a/pkg/sdk/go/channels_test.go b/pkg/sdk/go/channels_test.go index a64749028..441a82804 100644 --- a/pkg/sdk/go/channels_test.go +++ b/pkg/sdk/go/channels_test.go @@ -109,7 +109,7 @@ func TestCreateChannel(t *testing.T) { Status: mgclients.EnabledStatus.String(), }, token: token, - err: errors.NewSDKErrorWithStatus(errors.ErrCreateEntity, http.StatusInternalServerError), + err: errors.NewSDKErrorWithStatus(errors.ErrCreateEntity, http.StatusUnprocessableEntity), }, { desc: "create channel with missing name", @@ -463,7 +463,7 @@ func TestUpdateChannel(t *testing.T) { }, response: sdk.Channel{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthorization, http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthorization, svcerr.ErrAuthorization), http.StatusForbidden), }, { desc: "update channel description with invalid token", @@ -473,7 +473,7 @@ func TestUpdateChannel(t *testing.T) { }, response: sdk.Channel{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthorization, http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthorization, svcerr.ErrAuthorization), http.StatusForbidden), }, { desc: "update channel metadata with invalid token", @@ -485,7 +485,7 @@ func TestUpdateChannel(t *testing.T) { }, response: sdk.Channel{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthorization, http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthorization, svcerr.ErrAuthorization), http.StatusForbidden), }, { desc: "update channel that can't be marshalled", diff --git a/pkg/sdk/go/groups_test.go b/pkg/sdk/go/groups_test.go index ca6f49a25..e196fb88d 100644 --- a/pkg/sdk/go/groups_test.go +++ b/pkg/sdk/go/groups_test.go @@ -94,7 +94,7 @@ func TestCreateGroup(t *testing.T) { ParentID: wrongID, Status: clients.EnabledStatus.String(), }, - err: errors.NewSDKErrorWithStatus(svcerr.ErrCreateEntity, http.StatusInternalServerError), + err: errors.NewSDKErrorWithStatus(svcerr.ErrCreateEntity, http.StatusUnprocessableEntity), }, { desc: "create group with missing name", @@ -717,7 +717,7 @@ func TestUpdateGroup(t *testing.T) { }, response: sdk.Group{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthorization, http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthorization, svcerr.ErrAuthorization), http.StatusForbidden), }, { desc: "update group description with invalid token", @@ -727,7 +727,7 @@ func TestUpdateGroup(t *testing.T) { }, response: sdk.Group{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthorization, http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthorization, svcerr.ErrAuthorization), http.StatusForbidden), }, { desc: "update group metadata with invalid token", @@ -739,7 +739,7 @@ func TestUpdateGroup(t *testing.T) { }, response: sdk.Group{}, token: invalidToken, - err: errors.NewSDKErrorWithStatus(svcerr.ErrAuthorization, http.StatusForbidden), + err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrAuthorization, svcerr.ErrAuthorization), http.StatusForbidden), }, { desc: "update a group that can't be marshalled", diff --git a/pkg/sdk/go/things_test.go b/pkg/sdk/go/things_test.go index 98defd07e..8e3fca196 100644 --- a/pkg/sdk/go/things_test.go +++ b/pkg/sdk/go/things_test.go @@ -97,7 +97,7 @@ func TestCreateThing(t *testing.T) { response: sdk.Thing{}, token: token, repoErr: sdk.ErrFailedCreation, - err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedCreation, repoerr.ErrCreateEntity), http.StatusInternalServerError), + err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedCreation, repoerr.ErrCreateEntity), http.StatusUnprocessableEntity), }, { desc: "register empty thing", @@ -246,7 +246,7 @@ func TestCreateThings(t *testing.T) { things: thingsList, response: []sdk.Thing{}, token: token, - err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedCreation, sdk.ErrFailedCreation), http.StatusInternalServerError), + err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedCreation, sdk.ErrFailedCreation), http.StatusUnprocessableEntity), }, { desc: "register empty things", @@ -725,7 +725,7 @@ func TestUpdateThing(t *testing.T) { thing: thing2, response: sdk.Thing{}, token: validToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusInternalServerError), + err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusUnprocessableEntity), }, { desc: "update thing that can't be marshalled", @@ -810,7 +810,7 @@ func TestUpdateThingTags(t *testing.T) { thing: thing2, response: sdk.Thing{}, token: validToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusInternalServerError), + err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusUnprocessableEntity), }, { desc: "update thing that can't be marshalled", diff --git a/pkg/sdk/go/tokens_test.go b/pkg/sdk/go/tokens_test.go index ebf6cfbd5..ad5f07a95 100644 --- a/pkg/sdk/go/tokens_test.go +++ b/pkg/sdk/go/tokens_test.go @@ -62,7 +62,7 @@ func TestIssueToken(t *testing.T) { desc: "issue token for an empty user", login: sdk.Login{}, token: &magistrala.Token{}, - err: errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, apiutil.ErrMissingIdentity), http.StatusInternalServerError), + err: errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, apiutil.ErrMissingIdentity), http.StatusBadRequest), }, { desc: "issue token for invalid identity", diff --git a/pkg/sdk/go/users_test.go b/pkg/sdk/go/users_test.go index 39d6aeacf..a3fa90cb4 100644 --- a/pkg/sdk/go/users_test.go +++ b/pkg/sdk/go/users_test.go @@ -86,7 +86,7 @@ func TestCreateClient(t *testing.T) { client: user, response: sdk.User{}, token: token, - err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedCreation, sdk.ErrFailedCreation), http.StatusInternalServerError), + err: errors.NewSDKErrorWithStatus(errors.Wrap(sdk.ErrFailedCreation, sdk.ErrFailedCreation), http.StatusUnprocessableEntity), }, { desc: "register empty user", @@ -354,7 +354,6 @@ func TestListClients(t *testing.T) { for _, tc := range cases { pm := sdk.PageMetadata{ Status: tc.status, - Total: total, Offset: tc.offset, Limit: tc.limit, Name: tc.name, @@ -556,7 +555,7 @@ func TestUpdateClient(t *testing.T) { client: client2, response: sdk.User{}, token: validToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusInternalServerError), + err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusUnprocessableEntity), }, { desc: "update a user that can't be marshalled", @@ -647,7 +646,7 @@ func TestUpdateClientTags(t *testing.T) { client: client2, response: sdk.User{}, token: validToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusInternalServerError), + err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusUnprocessableEntity), }, { desc: "update a user that can't be marshalled", @@ -737,7 +736,7 @@ func TestUpdateClientIdentity(t *testing.T) { client: client2, response: sdk.User{}, token: validToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusInternalServerError), + err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusUnprocessableEntity), }, { desc: "update a user that can't be marshalled", @@ -753,7 +752,7 @@ func TestUpdateClientIdentity(t *testing.T) { }, response: sdk.User{}, token: validToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusInternalServerError), + err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrUpdateEntity, svcerr.ErrUpdateEntity), http.StatusUnprocessableEntity), }, } @@ -905,7 +904,7 @@ func TestUpdateClientRole(t *testing.T) { client: client2, response: sdk.User{}, token: validToken, - err: errors.NewSDKErrorWithStatus(errors.Wrap(users.ErrFailedUpdateRole, users.ErrFailedUpdateRole), http.StatusInternalServerError), + err: errors.NewSDKErrorWithStatus(errors.Wrap(svcerr.ErrFailedUpdateRole, svcerr.ErrFailedUpdateRole), http.StatusUnprocessableEntity), }, { desc: "update a user that can't be marshalled", diff --git a/things/service.go b/things/service.go index 8130609c6..78d331d4d 100644 --- a/things/service.go +++ b/things/service.go @@ -507,7 +507,7 @@ func (svc service) changeClientStatus(ctx context.Context, token string, client return mgclients.Client{}, errors.Wrap(repoerr.ErrNotFound, err) } if dbClient.Status == client.Status { - return mgclients.Client{}, mgclients.ErrStatusAlreadyAssigned + return mgclients.Client{}, errors.ErrStatusAlreadyAssigned } client.UpdatedBy = userID diff --git a/things/service_test.go b/things/service_test.go index 644bd6c0b..097a4d425 100644 --- a/things/service_test.go +++ b/things/service_test.go @@ -1079,8 +1079,8 @@ func TestEnableClient(t *testing.T) { changeStatusResponse: enabledClient1, retrieveByIDResponse: enabledClient1, authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, - changeStatusErr: mgclients.ErrStatusAlreadyAssigned, - err: mgclients.ErrStatusAlreadyAssigned, + changeStatusErr: errors.ErrStatusAlreadyAssigned, + err: errors.ErrStatusAlreadyAssigned, }, { desc: "enable non-existing client", @@ -1236,8 +1236,8 @@ func TestDisableClient(t *testing.T) { changeStatusResponse: mgclients.Client{}, retrieveByIDResponse: disabledClient1, authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, - changeStatusErr: mgclients.ErrStatusAlreadyAssigned, - err: mgclients.ErrStatusAlreadyAssigned, + changeStatusErr: errors.ErrStatusAlreadyAssigned, + err: errors.ErrStatusAlreadyAssigned, }, { desc: "disable non-existing client", diff --git a/users/api/clients.go b/users/api/clients.go index 84283d4e7..a0da7c589 100644 --- a/users/api/clients.go +++ b/users/api/clients.go @@ -26,6 +26,7 @@ func clientsHandler(svc users.Service, r *chi.Mux, logger *slog.Logger) http.Han opts := []kithttp.ServerOption{ kithttp.ServerErrorEncoder(apiutil.LoggingErrorEncoder(logger, api.EncodeError)), } + r.Route("/users", func(r chi.Router) { r.Post("/", otelhttp.NewHandler(kithttp.NewServer( registrationEndpoint(svc), @@ -83,20 +84,6 @@ func clientsHandler(svc users.Service, r *chi.Mux, logger *slog.Logger) http.Han opts..., ), "update_client_identity").ServeHTTP) - r.Post("/password/reset-request", otelhttp.NewHandler(kithttp.NewServer( - passwordResetRequestEndpoint(svc), - decodePasswordResetRequest, - api.EncodeResponse, - opts..., - ), "password_reset_req").ServeHTTP) - - r.Put("/password/reset", otelhttp.NewHandler(kithttp.NewServer( - passwordResetEndpoint(svc), - decodePasswordReset, - api.EncodeResponse, - opts..., - ), "password_reset").ServeHTTP) - r.Patch("/{id}/role", otelhttp.NewHandler(kithttp.NewServer( updateClientRoleEndpoint(svc), decodeUpdateClientRole, @@ -133,6 +120,22 @@ func clientsHandler(svc users.Service, r *chi.Mux, logger *slog.Logger) http.Han ), "disable_client").ServeHTTP) }) + r.Route("/password", func(r chi.Router) { + r.Post("/reset-request", otelhttp.NewHandler(kithttp.NewServer( + passwordResetRequestEndpoint(svc), + decodePasswordResetRequest, + api.EncodeResponse, + opts..., + ), "password_reset_req").ServeHTTP) + + r.Put("/reset", otelhttp.NewHandler(kithttp.NewServer( + passwordResetEndpoint(svc), + decodePasswordReset, + api.EncodeResponse, + opts..., + ), "password_reset").ServeHTTP) + }) + // Ideal location: users service, groups endpoint. // Reason for placing here : // SpiceDB provides list of user ids in given user_group_id @@ -217,7 +220,6 @@ func decodeListClients(_ context.Context, r *http.Request) (interface{}, error) if err != nil { return nil, errors.Wrap(apiutil.ErrValidation, err) } - order, err := apiutil.ReadStringQuery(r, api.OrderKey, api.DefOrder) if err != nil { return nil, errors.Wrap(apiutil.ErrValidation, err) diff --git a/users/api/endpoint_test.go b/users/api/endpoint_test.go index 0691810aa..07d9bafb4 100644 --- a/users/api/endpoint_test.go +++ b/users/api/endpoint_test.go @@ -452,13 +452,6 @@ func TestListClients(t *testing.T) { status: http.StatusOK, err: nil, }, - { - desc: "list users with invalid name", - token: validToken, - query: "name=invalid", - status: http.StatusBadRequest, - err: apiutil.ErrValidation, - }, { desc: "list users with duplicate name", token: validToken, @@ -506,13 +499,6 @@ func TestListClients(t *testing.T) { status: http.StatusOK, err: nil, }, - { - desc: "list users with invalid tags", - token: validToken, - query: "tag=invalid", - status: http.StatusBadRequest, - err: apiutil.ErrValidation, - }, { desc: "list users with duplicate tags", token: validToken, @@ -560,13 +546,6 @@ func TestListClients(t *testing.T) { status: http.StatusOK, err: nil, }, - { - desc: "list users with invalid permissions", - token: validToken, - query: "permission=invalid", - status: http.StatusBadRequest, - err: apiutil.ErrValidation, - }, { desc: "list users with duplicate permissions", token: validToken, @@ -587,13 +566,6 @@ func TestListClients(t *testing.T) { status: http.StatusOK, err: nil, }, - { - desc: "list users with invalid list perms", - token: validToken, - query: "list_perms=invalid", - status: http.StatusBadRequest, - err: apiutil.ErrValidation, - }, { desc: "list users with duplicate list perms", token: validToken, @@ -616,13 +588,6 @@ func TestListClients(t *testing.T) { status: http.StatusOK, err: nil, }, - { - desc: "list users with invalid identity", - token: validToken, - query: "identity=invalid", - status: http.StatusBadRequest, - err: apiutil.ErrValidation, - }, { desc: "list users with duplicate identity", token: validToken, @@ -645,13 +610,6 @@ func TestListClients(t *testing.T) { status: http.StatusOK, err: nil, }, - { - desc: "list users with invalid order", - token: validToken, - query: "order=invalid", - status: http.StatusBadRequest, - err: apiutil.ErrValidation, - }, { desc: "list users with duplicate order", token: validToken, @@ -659,13 +617,6 @@ func TestListClients(t *testing.T) { status: http.StatusBadRequest, err: apiutil.ErrInvalidQueryParams, }, - { - desc: "list users with invalid order direction", - token: validToken, - query: "dir=invalid", - status: http.StatusInternalServerError, - err: apiutil.ErrValidation, - }, { desc: "list users with duplicate order direction", token: validToken, @@ -1078,7 +1029,7 @@ func TestPasswordResetRequest(t *testing.T) { desc: "password reset request with empty email", data: fmt.Sprintf(`{"email": "%s", "host": "%s"}`, "", testhost), contentType: contentType, - status: http.StatusInternalServerError, + status: http.StatusBadRequest, err: apiutil.ErrValidation, }, { @@ -1115,7 +1066,7 @@ func TestPasswordResetRequest(t *testing.T) { req := testRequest{ client: us.Client(), method: http.MethodPost, - url: fmt.Sprintf("%s/users/password/reset-request", us.URL), + url: fmt.Sprintf("%s/password/reset-request", us.URL), contentType: tc.contentType, body: strings.NewReader(tc.data), } @@ -1163,7 +1114,7 @@ func TestPasswordReset(t *testing.T) { data: fmt.Sprintf(`{"token": "%s", "password": "%s", "confirm_password": "%s"}`, validToken, "weak", "weak"), token: validToken, contentType: contentType, - status: http.StatusInternalServerError, + status: http.StatusBadRequest, err: ErrPasswordFormat, }, { @@ -1179,7 +1130,7 @@ func TestPasswordReset(t *testing.T) { data: fmt.Sprintf(`{"token": "%s", "password": "%s", "confirm_password": "%s"}`, validToken, "", ""), token: validToken, contentType: contentType, - status: http.StatusInternalServerError, + status: http.StatusBadRequest, err: apiutil.ErrValidation, }, { @@ -1203,7 +1154,7 @@ func TestPasswordReset(t *testing.T) { req := testRequest{ client: us.Client(), method: http.MethodPut, - url: fmt.Sprintf("%s/users/password/reset", us.URL), + url: fmt.Sprintf("%s/password/reset", us.URL), contentType: tc.contentType, token: tc.token, body: strings.NewReader(tc.data), @@ -1272,7 +1223,7 @@ func TestUpdateClientRole(t *testing.T) { clientID: client.ID, token: validToken, contentType: contentType, - status: http.StatusInternalServerError, + status: http.StatusBadRequest, err: svcerr.ErrInvalidRole, }, { @@ -1476,7 +1427,7 @@ func TestIssueToken(t *testing.T) { desc: "issue token with empty identity", data: fmt.Sprintf(`{"identity": "%s", "secret": "%s", "domainID": "%s"}`, "", secret, validID), contentType: contentType, - status: http.StatusInternalServerError, + status: http.StatusBadRequest, err: apiutil.ErrValidation, }, { diff --git a/users/api/responses.go b/users/api/responses.go index 716f5a912..fb05de493 100644 --- a/users/api/responses.go +++ b/users/api/responses.go @@ -25,8 +25,8 @@ var ( type pageRes struct { Limit uint64 `json:"limit,omitempty"` - Offset uint64 `json:"offset,omitempty"` - Total uint64 `json:"total,omitempty"` + Offset uint64 `json:"offset"` + Total uint64 `json:"total"` } type createClientRes struct { diff --git a/users/postgres/clients.go b/users/postgres/clients.go index 5a65de691..b504f2c8a 100644 --- a/users/postgres/clients.go +++ b/users/postgres/clients.go @@ -133,11 +133,11 @@ func (repo clientRepo) RetrieveAll(ctx context.Context, pm mgclients.Page) (mgcl dbPage, err := pgclients.ToDBClientsPage(pm) if err != nil { - return mgclients.ClientsPage{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return mgclients.ClientsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } rows, err := repo.DB.NamedQueryContext(ctx, q, dbPage) if err != nil { - return mgclients.ClientsPage{}, errors.Wrap(postgres.ErrFailedToRetrieveAll, err) + return mgclients.ClientsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } defer rows.Close() diff --git a/users/service.go b/users/service.go index ae057b03f..fefb6340b 100644 --- a/users/service.go +++ b/users/service.go @@ -19,18 +19,6 @@ import ( ) var ( - // ErrRecoveryToken indicates error in generating password recovery token. - ErrRecoveryToken = errors.New("failed to generate password recovery token") - - // ErrPasswordFormat indicates weak password. - ErrPasswordFormat = errors.New("password does not meet the requirements") - - // ErrFailedPolicyUpdate indicates a failure to update user policy. - ErrFailedPolicyUpdate = errors.New("failed to update user policy") - - // ErrFailedUpdateRole indicates a failure to update user role. - ErrFailedUpdateRole = errors.New("failed to update user role") - // ErrAddPolicies indictaed a failre to add policies. errAddPolicies = errors.New("failed to add policies") @@ -292,7 +280,7 @@ func (svc service) GenerateResetToken(ctx context.Context, email, host string) e } token, err := svc.auth.Issue(ctx, issueReq) if err != nil { - return errors.Wrap(ErrRecoveryToken, err) + return errors.Wrap(svcerr.ErrRecoveryToken, err) } return svc.SendPasswordReset(ctx, host, email, client.Name, token.AccessToken) @@ -311,7 +299,7 @@ func (svc service) ResetSecret(ctx context.Context, resetToken, secret string) e return errors.ErrNotFound } if !svc.passRegex.MatchString(secret) { - return ErrPasswordFormat + return errors.ErrPasswordFormat } secret, err = svc.hasher.Hash(secret) if err != nil { @@ -337,7 +325,7 @@ func (svc service) UpdateClientSecret(ctx context.Context, token, oldSecret, new return mgclients.Client{}, err } if !svc.passRegex.MatchString(newSecret) { - return mgclients.Client{}, ErrPasswordFormat + return mgclients.Client{}, errors.ErrPasswordFormat } dbClient, err := svc.clients.RetrieveByID(ctx, id) if err != nil { @@ -384,7 +372,7 @@ func (svc service) UpdateClientRole(ctx context.Context, token string, cli mgcli } if err := svc.updateClientPolicy(ctx, cli.ID, cli.Role); err != nil { - return mgclients.Client{}, errors.Wrap(ErrFailedPolicyUpdate, err) + return mgclients.Client{}, errors.Wrap(svcerr.ErrFailedPolicyUpdate, err) } client, err = svc.clients.UpdateRole(ctx, client) if err != nil { @@ -392,7 +380,7 @@ func (svc service) UpdateClientRole(ctx context.Context, token string, cli mgcli if errRollback := svc.updateClientPolicy(ctx, cli.ID, mgclients.UserRole); errRollback != nil { return mgclients.Client{}, errors.Wrap(err, errors.Wrap(repoerr.ErrRollbackTx, errRollback)) } - return mgclients.Client{}, errors.Wrap(ErrFailedUpdateRole, err) + return mgclients.Client{}, errors.Wrap(svcerr.ErrFailedUpdateRole, err) } return client, nil } @@ -438,7 +426,7 @@ func (svc service) changeClientStatus(ctx context.Context, token string, client return mgclients.Client{}, errors.Wrap(repoerr.ErrNotFound, err) } if dbClient.Status == client.Status { - return mgclients.Client{}, mgclients.ErrStatusAlreadyAssigned + return mgclients.Client{}, errors.ErrStatusAlreadyAssigned } client.UpdatedBy = tokenUserID @@ -580,7 +568,7 @@ func (svc *service) authorize(ctx context.Context, subjType, subjKind, subj, per } if !res.GetAuthorized() { - return "", errors.Wrap(svcerr.ErrAuthorization, err) + return "", svcerr.ErrAuthorization } return res.GetId(), nil } diff --git a/users/service_test.go b/users/service_test.go index ba54c53d8..f355d5fc0 100644 --- a/users/service_test.go +++ b/users/service_test.go @@ -1183,7 +1183,7 @@ func TestUpdateClientSecret(t *testing.T) { newSecret: "weak", token: validToken, identifyResponse: &magistrala.IdentityRes{UserId: client.ID}, - err: users.ErrPasswordFormat, + err: errors.ErrPasswordFormat, }, { desc: "update client secret with failed to retrieve client by ID", @@ -1349,7 +1349,7 @@ func TestEnableClient(t *testing.T) { identifyResponse: &magistrala.IdentityRes{UserId: enabledClient1.ID}, authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, retrieveByIDResponse: enabledClient1, - err: mgclients.ErrStatusAlreadyAssigned, + err: errors.ErrStatusAlreadyAssigned, }, { desc: "enable disabled client with failed to change status", @@ -1536,7 +1536,7 @@ func TestDisableClient(t *testing.T) { identifyResponse: &magistrala.IdentityRes{UserId: disabledClient1.ID}, authorizeResponse: &magistrala.AuthorizeRes{Authorized: true}, retrieveByIDResponse: disabledClient1, - err: mgclients.ErrStatusAlreadyAssigned, + err: errors.ErrStatusAlreadyAssigned, }, { desc: "disable enabled client with failed to change status", @@ -2278,7 +2278,7 @@ func TestGenerateResetToken(t *testing.T) { retrieveByIdentityResponse: client, issueResponse: &magistrala.Token{}, issueErr: svcerr.ErrAuthorization, - err: users.ErrRecoveryToken, + err: svcerr.ErrRecoveryToken, }, } @@ -2367,7 +2367,7 @@ func TestResetSecret(t *testing.T) { newSecret: "weak", identifyResponse: &magistrala.IdentityRes{UserId: client.ID}, retrieveByIDResponse: client, - err: users.ErrPasswordFormat, + err: errors.ErrPasswordFormat, }, { desc: "reset secret with failed to update secret",