From 76788d3ae2ab111a583bdf22c858d04919acde56 Mon Sep 17 00:00:00 2001 From: b1ackd0t <28790446+rodneyosodo@users.noreply.github.com> Date: Thu, 11 Apr 2024 10:58:20 +0300 Subject: [PATCH] NOISSUE - Add property based testing to auth API (#2094) Signed-off-by: Rodney Osodo <28790446+rodneyosodo@users.noreply.github.com> --- .github/workflows/api-tests.yml | 11 ++ Makefile | 1 + api/openapi/auth.yml | 51 +++--- auth/api/http/domains/endpoint.go | 10 +- auth/api/http/domains/endpoint_test.go | 64 +++----- auth/api/http/domains/responses.go | 34 +--- auth/domains.go | 25 ++- auth/domains_test.go | 6 +- auth/postgres/domains.go | 10 +- auth/postgres/domains_test.go | 214 ++++++++----------------- internal/api/common.go | 6 +- internal/apiutil/errors.go | 3 - internal/groups/service.go | 2 +- internal/groups/service_test.go | 2 +- internal/groups/status.go | 4 +- internal/groups/status_test.go | 4 +- pkg/clients/status.go | 4 +- pkg/clients/status_test.go | 6 +- 18 files changed, 179 insertions(+), 278 deletions(-) diff --git a/.github/workflows/api-tests.yml b/.github/workflows/api-tests.yml index 5150fd7972..a0dd7f9cb2 100644 --- a/.github/workflows/api-tests.yml +++ b/.github/workflows/api-tests.yml @@ -31,6 +31,7 @@ env: USERS_URL: http://localhost:9002 THINGS_URL: http://localhost:9000 INVITATIONS_URL: http://localhost:9020 + AUTH_URL: http://localhost:8189 jobs: api-test: @@ -148,6 +149,16 @@ jobs: report: false args: '--header "Authorization: Bearer ${{ env.USER_TOKEN }}" --contrib-openapi-formats-uuid --hypothesis-suppress-health-check=filter_too_much --stateful=links' + - name: Run Auth API tests + if: steps.changes.outputs.auth == 'true' + uses: schemathesis/action@v1 + with: + schema: api/openapi/auth.yml + base-url: ${{ env.AUTH_URL }} + checks: all + report: false + args: '--header "Authorization: Bearer ${{ env.USER_TOKEN }}" --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/Makefile b/Makefile index 2f8b397692..1773c04f94 100644 --- a/Makefile +++ b/Makefile @@ -153,6 +153,7 @@ endef test_api_users: TEST_API_URL := http://localhost:9002 test_api_things: TEST_API_URL := http://localhost:9000 test_api_invitations: TEST_API_URL := http://localhost:9020 +test_api_auth: TEST_API_URL := http://localhost:8189 $(TEST_API): $(call test_api_service,$(@),$(TEST_API_URL)) diff --git a/api/openapi/auth.yml b/api/openapi/auth.yml index bd75c9e91e..83a90a4011 100644 --- a/api/openapi/auth.yml +++ b/api/openapi/auth.yml @@ -74,6 +74,8 @@ paths: responses: "200": $ref: "#/components/responses/DomainsPageRes" + "400": + description: Failed due to malformed query parameters. "401": description: Missing or invalid access token provided. "404": @@ -99,6 +101,8 @@ paths: $ref: "#/components/responses/DomainRes" "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": @@ -133,26 +137,7 @@ paths: description: Missing or invalid content type. "500": $ref: "#/components/responses/ServiceError" - delete: - summary: Delete domain for a domain with the given id. - description: | - Delete domain removes a domain with the given id from repo - and removes all the things, channels, assigned users, policies related to this domain. - tags: - - Domains - parameters: - - $ref: "#/components/parameters/DomainID" - security: - - bearerAuth: [] - responses: - "204": - description: Domain deleted. - "401": - description: Missing or invalid access token provided. - "403": - description: Unauthorized access to domain id. - "500": - $ref: "#/components/responses/ServiceError" + /domains/{domainID}/permissions: get: summary: Retrieves user permissions on domain. @@ -319,6 +304,7 @@ paths: $ref: "#/components/responses/ServiceError" /keys: post: + operationId: issueKey tags: - Keys summary: Issue API key @@ -341,6 +327,7 @@ paths: /keys/{keyID}: get: + operationId: getKey summary: Gets API key details. description: | Gets API key details for the given key. @@ -355,10 +342,13 @@ paths: description: Failed due to malformed query parameters. "401": description: Missing or invalid access token provided. + "404": + description: A non-existent entity request. "500": $ref: "#/components/responses/ServiceError" delete: + operationId: revokeKey summary: Revoke API key description: | Revoke API key identified by the given ID. @@ -371,11 +361,14 @@ paths: description: Key revoked. "401": description: Missing or invalid access token provided. + "404": + description: A non-existent entity request. "500": $ref: "#/components/responses/ServiceError" /policies: post: + operationId: addPolicies summary: Creates new policies. description: | Creates new policies. Only admin can use this endpoint. Therefore, you need an authentication token for the admin. @@ -393,6 +386,8 @@ paths: description: Missing or invalid access token provided. "403": description: Unauthorized access token provided. + "404": + description: A non-existent entity request. "409": description: Failed due to using an existing email address. "415": @@ -402,6 +397,7 @@ paths: /policies/delete: post: + operationId: deletePolicies summary: Deletes policies. description: | Deletes policies. Only admin can use this endpoint. Therefore, you need an authentication token for the admin. @@ -415,6 +411,8 @@ paths: description: Policies deleted. "400": description: Failed due to malformed JSON. + "404": + description: A non-existent entity request. "409": description: Failed due to using an existing email address. "415": @@ -441,7 +439,7 @@ paths: - bearerAuth: [] responses: "200": - $ref: "users.yml#/components/responses/UserPageRes" + $ref: "#/components/responses/DomainsPageRes" "400": description: Failed due to malformed query parameters. "401": @@ -569,7 +567,7 @@ components: example: 10 description: Maximum number of items to return in one page. required: - - domain + - domains - total - offset DomainUpdate: @@ -620,7 +618,7 @@ components: ] relation: type: string - enum: ["administrator", "editor","viewer","member"] + enum: ["administrator", "editor", "viewer", "member"] example: "administrator" description: Policy relations. required: @@ -867,11 +865,16 @@ components: application/json: schema: $ref: "#/components/schemas/Key" + links: + revoke: + operationId: revokeKey + parameters: + keyID: $response.body#/id HealthRes: description: Service Health Check. content: - application/json: + application/health+json: schema: $ref: "./schemas/HealthInfo.yml" diff --git a/auth/api/http/domains/endpoint.go b/auth/api/http/domains/endpoint.go index bfc4d80010..55fdd2c595 100644 --- a/auth/api/http/domains/endpoint.go +++ b/auth/api/http/domains/endpoint.go @@ -29,7 +29,7 @@ func createDomainEndpoint(svc auth.Service) endpoint.Endpoint { return nil, err } - return createDomainRes{Data: domain}, nil + return createDomainRes{domain}, nil } } @@ -44,7 +44,7 @@ func retrieveDomainEndpoint(svc auth.Service) endpoint.Endpoint { if err != nil { return nil, err } - return retrieveDomainRes{Data: domain}, nil + return retrieveDomainRes{domain}, nil } } @@ -85,7 +85,7 @@ func updateDomainEndpoint(svc auth.Service) endpoint.Endpoint { return nil, err } - return updateDomainRes{Data: domain}, nil + return updateDomainRes{domain}, nil } } @@ -111,7 +111,7 @@ func listDomainsEndpoint(svc auth.Service) endpoint.Endpoint { if err != nil { return nil, err } - return listDomainsRes{Data: dp}, nil + return listDomainsRes{dp}, nil } } @@ -219,6 +219,6 @@ func listUserDomainsEndpoint(svc auth.Service) endpoint.Endpoint { if err != nil { return nil, err } - return listUserDomainsRes{Data: dp}, nil + return listUserDomainsRes{dp}, nil } } diff --git a/auth/api/http/domains/endpoint_test.go b/auth/api/http/domains/endpoint_test.go index 4a53418b47..f584daadc7 100644 --- a/auth/api/http/domains/endpoint_test.go +++ b/auth/api/http/domains/endpoint_test.go @@ -119,7 +119,7 @@ func TestCreateDomain(t *testing.T) { }, token: validToken, contentType: contentType, - status: http.StatusOK, + status: http.StatusCreated, err: nil, }, { @@ -162,7 +162,7 @@ func TestCreateDomain(t *testing.T) { }, token: validToken, contentType: contentType, - status: http.StatusInternalServerError, + status: http.StatusBadRequest, err: apiutil.ErrMissingName, }, { @@ -241,9 +241,7 @@ func TestListDomains(t *testing.T) { token: validToken, status: http.StatusOK, listDomainsRequest: auth.DomainsPage{ - Page: auth.Page{ - Total: 1, - }, + Total: 1, Domains: []auth.Domain{domain}, }, err: nil, @@ -265,9 +263,7 @@ func TestListDomains(t *testing.T) { desc: "list domains with offset", token: validToken, listDomainsRequest: auth.DomainsPage{ - Page: auth.Page{ - Total: 1, - }, + Total: 1, Domains: []auth.Domain{domain}, }, query: "offset=1", @@ -285,9 +281,7 @@ func TestListDomains(t *testing.T) { desc: "list domains with limit", token: validToken, listDomainsRequest: auth.DomainsPage{ - Page: auth.Page{ - Total: 1, - }, + Total: 1, Domains: []auth.Domain{domain}, }, query: "limit=1", @@ -305,9 +299,7 @@ func TestListDomains(t *testing.T) { desc: "list domains with name", token: validToken, listDomainsRequest: auth.DomainsPage{ - Page: auth.Page{ - Total: 1, - }, + Total: 1, Domains: []auth.Domain{domain}, }, query: "name=domainname", @@ -332,9 +324,7 @@ func TestListDomains(t *testing.T) { desc: "list domains with status", token: validToken, listDomainsRequest: auth.DomainsPage{ - Page: auth.Page{ - Total: 1, - }, + Total: 1, Domains: []auth.Domain{domain}, }, query: "status=enabled", @@ -359,9 +349,7 @@ func TestListDomains(t *testing.T) { desc: "list domains with tags", token: validToken, listDomainsRequest: auth.DomainsPage{ - Page: auth.Page{ - Total: 1, - }, + Total: 1, Domains: []auth.Domain{domain}, }, query: "tag=tag1,tag2", @@ -386,9 +374,7 @@ func TestListDomains(t *testing.T) { desc: "list domains with metadata", token: validToken, listDomainsRequest: auth.DomainsPage{ - Page: auth.Page{ - Total: 1, - }, + Total: 1, Domains: []auth.Domain{domain}, }, query: "metadata=%7B%22domain%22%3A%20%22example.com%22%7D&", @@ -413,9 +399,7 @@ func TestListDomains(t *testing.T) { desc: "list domains with permissions", token: validToken, listDomainsRequest: auth.DomainsPage{ - Page: auth.Page{ - Total: 1, - }, + Total: 1, Domains: []auth.Domain{domain}, }, query: "permission=view", @@ -440,9 +424,7 @@ func TestListDomains(t *testing.T) { desc: "list domains with order", token: validToken, listDomainsRequest: auth.DomainsPage{ - Page: auth.Page{ - Total: 1, - }, + Total: 1, Domains: []auth.Domain{domain}, }, query: "order=name", @@ -466,9 +448,7 @@ func TestListDomains(t *testing.T) { desc: "list domains with dir", token: validToken, listDomainsRequest: auth.DomainsPage{ - Page: auth.Page{ - Total: 1, - }, + Total: 1, Domains: []auth.Domain{domain}, }, query: "dir=asc", @@ -1066,7 +1046,7 @@ func TestAssignDomainUsers(t *testing.T) { domainID: domain.ID, contentType: contentType, token: validToken, - status: http.StatusInternalServerError, + status: http.StatusBadRequest, err: apiutil.ErrValidation, }, { @@ -1075,7 +1055,7 @@ func TestAssignDomainUsers(t *testing.T) { domainID: domain.ID, contentType: contentType, token: validToken, - status: http.StatusInternalServerError, + status: http.StatusBadRequest, err: apiutil.ErrValidation, }, } @@ -1180,7 +1160,7 @@ func TestUnassignDomainUsers(t *testing.T) { domainID: domain.ID, contentType: contentType, token: validToken, - status: http.StatusInternalServerError, + status: http.StatusBadRequest, err: apiutil.ErrValidation, }, { @@ -1189,7 +1169,7 @@ func TestUnassignDomainUsers(t *testing.T) { domainID: domain.ID, contentType: contentType, token: validToken, - status: http.StatusInternalServerError, + status: http.StatusBadRequest, err: apiutil.ErrValidation, }, } @@ -1231,9 +1211,7 @@ func TestListDomainsByUserID(t *testing.T) { token: validToken, status: http.StatusOK, listDomainsRequest: auth.DomainsPage{ - Page: auth.Page{ - Total: 1, - }, + Total: 1, Domains: []auth.Domain{domain}, }, userID: validID, @@ -1264,9 +1242,7 @@ func TestListDomainsByUserID(t *testing.T) { desc: "list domains by user id with offset", token: validToken, listDomainsRequest: auth.DomainsPage{ - Page: auth.Page{ - Total: 1, - }, + Total: 1, Domains: []auth.Domain{domain}, }, query: "offset=1", @@ -1285,9 +1261,7 @@ func TestListDomainsByUserID(t *testing.T) { desc: "list domains by user id with limit", token: validToken, listDomainsRequest: auth.DomainsPage{ - Page: auth.Page{ - Total: 1, - }, + Total: 1, Domains: []auth.Domain{domain}, }, query: "limit=1", diff --git a/auth/api/http/domains/responses.go b/auth/api/http/domains/responses.go index 4a43a80b55..3eb277ef06 100644 --- a/auth/api/http/domains/responses.go +++ b/auth/api/http/domains/responses.go @@ -4,10 +4,10 @@ package domains import ( - "encoding/json" "net/http" "github.com/absmach/magistrala" + "github.com/absmach/magistrala/auth" ) var ( @@ -19,11 +19,11 @@ var ( ) type createDomainRes struct { - Data interface{} + auth.Domain } func (res createDomainRes) Code() int { - return http.StatusOK + return http.StatusCreated } func (res createDomainRes) Headers() map[string]string { @@ -34,12 +34,8 @@ func (res createDomainRes) Empty() bool { return false } -func (res createDomainRes) MarshalJSON() ([]byte, error) { - return json.Marshal(res.Data) -} - type retrieveDomainRes struct { - Data interface{} + auth.Domain } func (res retrieveDomainRes) Code() int { @@ -54,10 +50,6 @@ func (res retrieveDomainRes) Empty() bool { return false } -func (res retrieveDomainRes) MarshalJSON() ([]byte, error) { - return json.Marshal(res.Data) -} - type retrieveDomainPermissionsRes struct { Permissions []string `json:"permissions"` } @@ -75,7 +67,7 @@ func (res retrieveDomainPermissionsRes) Empty() bool { } type updateDomainRes struct { - Data interface{} + auth.Domain } func (res updateDomainRes) Code() int { @@ -90,12 +82,8 @@ func (res updateDomainRes) Empty() bool { return false } -func (res updateDomainRes) MarshalJSON() ([]byte, error) { - return json.Marshal(res.Data) -} - type listDomainsRes struct { - Data interface{} + auth.DomainsPage } func (res listDomainsRes) Code() int { @@ -110,10 +98,6 @@ func (res listDomainsRes) Empty() bool { return false } -func (res listDomainsRes) MarshalJSON() ([]byte, error) { - return json.Marshal(res.Data) -} - type enableDomainRes struct{} func (res enableDomainRes) Code() int { @@ -185,7 +169,7 @@ func (res unassignUsersRes) Empty() bool { } type listUserDomainsRes struct { - Data interface{} + auth.DomainsPage } func (res listUserDomainsRes) Code() int { @@ -199,7 +183,3 @@ func (res listUserDomainsRes) Headers() map[string]string { func (res listUserDomainsRes) Empty() bool { return false } - -func (res listUserDomainsRes) MarshalJSON() ([]byte, error) { - return json.Marshal(res.Data) -} diff --git a/auth/domains.go b/auth/domains.go index c4ac9a3329..c39dd46c8f 100644 --- a/auth/domains.go +++ b/auth/domains.go @@ -10,8 +10,8 @@ import ( "strings" "time" - "github.com/absmach/magistrala/internal/apiutil" "github.com/absmach/magistrala/pkg/clients" + svcerr "github.com/absmach/magistrala/pkg/errors/service" ) // Status represents Domain status. @@ -73,7 +73,7 @@ func ToStatus(status string) (Status, error) { case All: return AllStatus, nil } - return Status(0), apiutil.ErrInvalidStatus + return Status(0), svcerr.ErrInvalidStatus } // Custom Marshaller for Domains status. @@ -128,8 +128,25 @@ type Page struct { } type DomainsPage struct { - Page - Domains []Domain `json:"domains,omitempty"` + Total uint64 `json:"total"` + Offset uint64 `json:"offset"` + Limit uint64 `json:"limit"` + Domains []Domain `json:"domains"` +} + +func (page DomainsPage) MarshalJSON() ([]byte, error) { + type Alias DomainsPage + a := struct { + Alias + }{ + Alias: Alias(page), + } + + if a.Domains == nil { + a.Domains = make([]Domain, 0) + } + + return json.Marshal(a) } type Policy struct { diff --git a/auth/domains_test.go b/auth/domains_test.go index f24a18ff6e..82875bccd5 100644 --- a/auth/domains_test.go +++ b/auth/domains_test.go @@ -7,7 +7,7 @@ import ( "testing" "github.com/absmach/magistrala/auth" - "github.com/absmach/magistrala/internal/apiutil" + svcerr "github.com/absmach/magistrala/pkg/errors/service" "github.com/stretchr/testify/assert" ) @@ -87,7 +87,7 @@ func TestToStatus(t *testing.T) { desc: "Unknown", status: "unknown", expetcted: auth.Status(0), - err: apiutil.ErrInvalidStatus, + err: svcerr.ErrInvalidStatus, }, } @@ -171,7 +171,7 @@ func TestStatusUnmarshalJSON(t *testing.T) { desc: "Unknown", expected: auth.Status(0), status: []byte(`"unknown"`), - err: apiutil.ErrInvalidStatus, + err: svcerr.ErrInvalidStatus, }, } diff --git a/auth/postgres/domains.go b/auth/postgres/domains.go index 92b1b0b088..ad111e1a0e 100644 --- a/auth/postgres/domains.go +++ b/auth/postgres/domains.go @@ -166,9 +166,10 @@ func (repo domainRepo) RetrieveAllByIDs(ctx context.Context, pm auth.Page) (auth return auth.DomainsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } - pm.Total = total return auth.DomainsPage{ - Page: pm, + Total: total, + Offset: pm.Offset, + Limit: pm.Limit, Domains: domains, }, nil } @@ -226,9 +227,10 @@ func (repo domainRepo) ListDomains(ctx context.Context, pm auth.Page) (auth.Doma return auth.DomainsPage{}, errors.Wrap(repoerr.ErrFailedToRetrieveAllGroups, err) } - pm.Total = total return auth.DomainsPage{ - Page: pm, + Total: total, + Offset: pm.Offset, + Limit: pm.Limit, Domains: domains, }, nil } diff --git a/auth/postgres/domains_test.go b/auth/postgres/domains_test.go index 0755a631b7..b9d6d5fcff 100644 --- a/auth/postgres/domains_test.go +++ b/auth/postgres/domains_test.go @@ -367,12 +367,9 @@ func TestRetrieveAllByIDs(t *testing.T) { IDs: []string{items[1].ID, items[2].ID}, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 2, - Offset: 0, - Limit: 10, - IDs: []string{items[1].ID, items[2].ID}, - }, + Total: 2, + Offset: 0, + Limit: 10, Domains: []auth.Domain{items[1], items[2]}, }, err: nil, @@ -385,11 +382,9 @@ func TestRetrieveAllByIDs(t *testing.T) { IDs: []string{}, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 0, - Offset: 0, - Limit: 0, - }, + Total: 0, + Offset: 0, + Limit: 0, }, err: nil, }, @@ -401,12 +396,9 @@ func TestRetrieveAllByIDs(t *testing.T) { IDs: []string{inValid}, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 0, - Offset: 0, - Limit: 10, - IDs: []string{inValid}, - }, + Total: 0, + Offset: 0, + Limit: 10, }, err: nil, }, @@ -419,13 +411,9 @@ func TestRetrieveAllByIDs(t *testing.T) { Status: auth.DisabledStatus, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 1, - Offset: 0, - Limit: 10, - Status: auth.DisabledStatus, - IDs: []string{items[0].ID, items[1].ID}, - }, + Total: 1, + Offset: 0, + Limit: 10, Domains: []auth.Domain{items[0]}, }, }, @@ -438,13 +426,9 @@ func TestRetrieveAllByIDs(t *testing.T) { Status: 5, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 2, - Offset: 0, - Limit: 10, - Status: 5, - IDs: []string{items[0].ID, items[1].ID}, - }, + Total: 2, + Offset: 0, + Limit: 10, Domains: []auth.Domain{items[0], items[1]}, }, }, @@ -457,13 +441,9 @@ func TestRetrieveAllByIDs(t *testing.T) { Tag: "test", }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 1, - Offset: 0, - Limit: 10, - Tag: "test", - IDs: []string{items[0].ID, items[1].ID}, - }, + Total: 1, + Offset: 0, + Limit: 10, Domains: []auth.Domain{items[1]}, }, }, @@ -479,16 +459,9 @@ func TestRetrieveAllByIDs(t *testing.T) { Status: auth.EnabledStatus, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 2, - Offset: 0, - Limit: 10, - IDs: []string{items[1].ID, items[2].ID}, - Metadata: map[string]interface{}{ - "test": "test", - }, - Status: auth.EnabledStatus, - }, + Total: 2, + Offset: 0, + Limit: 10, Domains: items[1:3], }, }, @@ -504,16 +477,9 @@ func TestRetrieveAllByIDs(t *testing.T) { Status: auth.EnabledStatus, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 0, - Offset: 0, - Limit: 10, - IDs: []string{items[1].ID, items[2].ID}, - Metadata: map[string]interface{}{ - "test1": "test1", - }, - Status: auth.EnabledStatus, - }, + Total: 0, + Offset: 0, + Limit: 10, }, }, { @@ -527,10 +493,8 @@ func TestRetrieveAllByIDs(t *testing.T) { }, Status: auth.EnabledStatus, }, - response: auth.DomainsPage{ - Page: auth.Page{}, - }, - err: repoerr.ErrViewEntity, + response: auth.DomainsPage{}, + err: repoerr.ErrViewEntity, }, { desc: "retrieve all by ids and id", @@ -541,13 +505,9 @@ func TestRetrieveAllByIDs(t *testing.T) { IDs: []string{items[1].ID, items[2].ID}, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 1, - Offset: 0, - Limit: 10, - ID: items[1].ID, - IDs: []string{items[1].ID, items[2].ID}, - }, + Total: 1, + Offset: 0, + Limit: 10, Domains: []auth.Domain{items[1]}, }, }, @@ -560,13 +520,9 @@ func TestRetrieveAllByIDs(t *testing.T) { IDs: []string{items[1].ID, items[2].ID}, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 0, - Offset: 0, - Limit: 10, - ID: inValid, - IDs: []string{items[1].ID, items[2].ID}, - }, + Total: 0, + Offset: 0, + Limit: 10, }, }, { @@ -578,13 +534,9 @@ func TestRetrieveAllByIDs(t *testing.T) { IDs: []string{items[1].ID, items[2].ID}, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 1, - Offset: 0, - Limit: 10, - Name: items[1].Name, - IDs: []string{items[1].ID, items[2].ID}, - }, + Total: 1, + Offset: 0, + Limit: 10, Domains: []auth.Domain{items[1]}, }, }, @@ -667,12 +619,9 @@ func TestListDomains(t *testing.T) { Status: auth.AllStatus, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 10, - Offset: 0, - Limit: 10, - Status: auth.AllStatus, - }, + Total: 10, + Offset: 0, + Limit: 10, Domains: items, }, err: nil, @@ -684,11 +633,9 @@ func TestListDomains(t *testing.T) { Limit: 0, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 8, - Offset: 0, - Limit: 0, - }, + Total: 8, + Offset: 0, + Limit: 0, }, err: nil, }, @@ -700,12 +647,9 @@ func TestListDomains(t *testing.T) { Status: auth.EnabledStatus, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 8, - Offset: 0, - Limit: 10, - Status: auth.EnabledStatus, - }, + Total: 8, + Offset: 0, + Limit: 10, Domains: []auth.Domain{items[1], items[2], items[3], items[4], items[6], items[7], items[8], items[9]}, }, err: nil, @@ -718,12 +662,9 @@ func TestListDomains(t *testing.T) { Status: auth.DisabledStatus, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 2, - Offset: 0, - Limit: 10, - Status: auth.DisabledStatus, - }, + Total: 2, + Offset: 0, + Limit: 10, Domains: []auth.Domain{items[0], items[5]}, }, err: nil, @@ -737,13 +678,9 @@ func TestListDomains(t *testing.T) { Status: auth.AllStatus, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 10, - Offset: 0, - Limit: 10, - Status: auth.AllStatus, - SubjectID: userID, - }, + Total: 10, + Offset: 0, + Limit: 10, Domains: rDomains, }, err: nil, @@ -757,13 +694,9 @@ func TestListDomains(t *testing.T) { Status: auth.EnabledStatus, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 8, - Offset: 0, - Limit: 10, - Status: auth.EnabledStatus, - SubjectID: userID, - }, + Total: 8, + Offset: 0, + Limit: 10, Domains: []auth.Domain{rDomains[1], rDomains[2], rDomains[3], rDomains[4], rDomains[6], rDomains[7], rDomains[8], rDomains[9]}, }, err: nil, @@ -778,14 +711,9 @@ func TestListDomains(t *testing.T) { Status: auth.AllStatus, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 10, - Offset: 0, - Limit: 10, - Status: auth.AllStatus, - SubjectID: userID, - Permission: "domain", - }, + Total: 10, + Offset: 0, + Limit: 10, Domains: rDomains, }, err: nil, @@ -800,14 +728,9 @@ func TestListDomains(t *testing.T) { Status: auth.AllStatus, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 10, - Offset: 0, - Limit: 10, - Status: auth.AllStatus, - SubjectID: userID, - Tag: "test", - }, + Total: 10, + Offset: 0, + Limit: 10, Domains: rDomains, }, err: nil, @@ -824,16 +747,9 @@ func TestListDomains(t *testing.T) { Status: auth.AllStatus, }, response: auth.DomainsPage{ - Page: auth.Page{ - Total: 8, - Offset: 0, - Limit: 10, - Status: auth.AllStatus, - SubjectID: userID, - Metadata: map[string]interface{}{ - "test": "test", - }, - }, + Total: 8, + Offset: 0, + Limit: 10, Domains: []auth.Domain{rDomains[1], rDomains[2], rDomains[3], rDomains[4], rDomains[6], rDomains[7], rDomains[8], rDomains[9]}, }, }, @@ -848,10 +764,8 @@ func TestListDomains(t *testing.T) { }, Status: auth.AllStatus, }, - response: auth.DomainsPage{ - Page: auth.Page{}, - }, - err: repoerr.ErrViewEntity, + response: auth.DomainsPage{}, + err: repoerr.ErrViewEntity, }, } diff --git a/internal/api/common.go b/internal/api/common.go index ce7b5e3649..bad612c4be 100644 --- a/internal/api/common.go +++ b/internal/api/common.go @@ -115,7 +115,6 @@ func EncodeError(_ context.Context, err error, w http.ResponseWriter) { errors.Contains(err, apiutil.ErrBearerKey), errors.Contains(err, apiutil.ErrNameSize), errors.Contains(err, apiutil.ErrInvalidIDFormat), - errors.Contains(err, apiutil.ErrInvalidStatus), errors.Contains(err, svcerr.ErrInvalidStatus), errors.Contains(err, apiutil.ErrValidation), errors.Contains(err, apiutil.ErrInvitationState), @@ -130,7 +129,10 @@ func EncodeError(_ context.Context, err error, w http.ResponseWriter) { errors.Contains(err, apiutil.ErrMissingRelation), errors.Contains(err, apiutil.ErrPasswordFormat), errors.Contains(err, apiutil.ErrInvalidLevel), - errors.Contains(err, apiutil.ErrInvalidQueryParams): + errors.Contains(err, apiutil.ErrInvalidQueryParams), + errors.Contains(err, apiutil.ErrMalformedPolicy), + errors.Contains(err, apiutil.ErrInvalidAPIKey), + errors.Contains(err, apiutil.ErrMissingName): w.WriteHeader(http.StatusBadRequest) case errors.Contains(err, svcerr.ErrAuthentication), errors.Contains(err, svcerr.ErrLogin), diff --git a/internal/apiutil/errors.go b/internal/apiutil/errors.go index dabe5881d2..ed5c2cd77e 100644 --- a/internal/apiutil/errors.go +++ b/internal/apiutil/errors.go @@ -33,9 +33,6 @@ var ( // ErrEmailSize indicates that email size exceeds the max. ErrEmailSize = errors.New("invalid email size") - // ErrInvalidStatus indicates an invalid user account status. - ErrInvalidStatus = errors.New("invalid user account status") - // ErrInvalidRole indicates that an invalid role. ErrInvalidRole = errors.New("invalid client role") diff --git a/internal/groups/service.go b/internal/groups/service.go index 17acc0d2e5..78a6f621aa 100644 --- a/internal/groups/service.go +++ b/internal/groups/service.go @@ -54,7 +54,7 @@ func (svc service) CreateGroup(ctx context.Context, token, kind string, g groups return groups.Group{}, err } if g.Status != mgclients.EnabledStatus && g.Status != mgclients.DisabledStatus { - return groups.Group{}, apiutil.ErrInvalidStatus + return groups.Group{}, svcerr.ErrInvalidStatus } g.ID = groupID diff --git a/internal/groups/service_test.go b/internal/groups/service_test.go index 27748e62fd..2d513f04db 100644 --- a/internal/groups/service_test.go +++ b/internal/groups/service_test.go @@ -155,7 +155,7 @@ func TestCreateGroup(t *testing.T) { authzResp: &magistrala.AuthorizeRes{ Authorized: true, }, - err: apiutil.ErrInvalidStatus, + err: svcerr.ErrInvalidStatus, }, { desc: "successfully with parent", diff --git a/internal/groups/status.go b/internal/groups/status.go index 85807e959c..d967dbc0b4 100644 --- a/internal/groups/status.go +++ b/internal/groups/status.go @@ -3,7 +3,7 @@ package groups -import "github.com/absmach/magistrala/internal/apiutil" +import svcerr "github.com/absmach/magistrala/pkg/errors/service" // Status represents Group status. type Status uint8 @@ -54,5 +54,5 @@ func ToStatus(status string) (Status, error) { case All: return AllStatus, nil } - return Status(0), apiutil.ErrInvalidStatus + return Status(0), svcerr.ErrInvalidStatus } diff --git a/internal/groups/status_test.go b/internal/groups/status_test.go index e052538120..a715ee392d 100644 --- a/internal/groups/status_test.go +++ b/internal/groups/status_test.go @@ -6,8 +6,8 @@ package groups_test import ( "testing" - "github.com/absmach/magistrala/internal/apiutil" "github.com/absmach/magistrala/internal/groups" + svcerr "github.com/absmach/magistrala/pkg/errors/service" "github.com/stretchr/testify/assert" ) @@ -39,7 +39,7 @@ func TestToStatus(t *testing.T) { {"Enabled", "enabled", groups.EnabledStatus, nil}, {"Disabled", "disabled", groups.DisabledStatus, nil}, {"All", "all", groups.AllStatus, nil}, - {"Unknown", "unknown", groups.Status(0), apiutil.ErrInvalidStatus}, + {"Unknown", "unknown", groups.Status(0), svcerr.ErrInvalidStatus}, } for _, tc := range cases { diff --git a/pkg/clients/status.go b/pkg/clients/status.go index 82a716d511..7d9ffcb970 100644 --- a/pkg/clients/status.go +++ b/pkg/clients/status.go @@ -7,7 +7,7 @@ import ( "encoding/json" "strings" - "github.com/absmach/magistrala/internal/apiutil" + svcerr "github.com/absmach/magistrala/pkg/errors/service" ) // Status represents Client status. @@ -59,7 +59,7 @@ func ToStatus(status string) (Status, error) { case All: return AllStatus, nil } - return Status(0), apiutil.ErrInvalidStatus + return Status(0), svcerr.ErrInvalidStatus } // Custom Marshaller for Client/Groups. diff --git a/pkg/clients/status_test.go b/pkg/clients/status_test.go index 1d6e08f198..3e9540a1f5 100644 --- a/pkg/clients/status_test.go +++ b/pkg/clients/status_test.go @@ -6,8 +6,8 @@ package clients_test import ( "testing" - "github.com/absmach/magistrala/internal/apiutil" "github.com/absmach/magistrala/pkg/clients" + svcerr "github.com/absmach/magistrala/pkg/errors/service" "github.com/stretchr/testify/assert" ) @@ -76,7 +76,7 @@ func TestToStatus(t *testing.T) { desc: "Unknown", status: "unknown", expetcted: clients.Status(0), - err: apiutil.ErrInvalidStatus, + err: svcerr.ErrInvalidStatus, }, } @@ -160,7 +160,7 @@ func TestStatusUnmarshalJSON(t *testing.T) { desc: "Unknown", expected: clients.Status(0), status: []byte(`"unknown"`), - err: apiutil.ErrInvalidStatus, + err: svcerr.ErrInvalidStatus, }, }