NOISSUE - Remove Secret on Viewing User(s) and Things (#1884)

* Remove secret when returning user Signed-off-by: rodneyosodo <[email protected]> * Remove secret on list all users Signed-off-by: rodneyosodo <[email protected]> * Remove Secret on list all things Signed-off-by: rodneyosodo <[email protected]> --------- Signed-off-by: rodneyosodo <[email protected]>
absmach · Aug 9, 2023 · 896a74a · 896a74a
1 parent b4b625d
commit 896a74a
Show file tree

Hide file tree

Showing 8 changed files with 79 additions and 12 deletions.
diff --git a/api/openapi/things.yml b/api/openapi/things.yml
@@ -892,6 +892,63 @@ components:
       xml:
         name: thing
 
+    ThingWithEmptySecret:
+      type: object
+      properties:
+        id:
+          type: string
+          format: uuid
+          example: bb7edb32-2eac-4aad-aebe-ed96fe073879
+          description: Thing unique identifier.
+        name:
+          type: string
+          example: thingName
+          description: Thing name.
+        tags:
+          type: array
+          minItems: 0
+          items:
+            type: string
+          example: ['tag1', 'tag2']
+          description: Thing tags.
+        owner:
+          type: string
+          format: uuid
+          example: bb7edb32-2eac-4aad-aebe-ed96fe073879
+          description: Thing owner identifier.
+        credentials:
+          type: object
+          properties:
+            identity:
+              type: string
+              example: thingidentity
+              description: Thing Identity for example email address.
+            secret:
+              type: string
+              example: ""
+              description: Thing secret password.
+        metadata:
+          type: object
+          example: {"domain": "example.com"}
+          description: Arbitrary, object-encoded thing's data.
+        status:
+          type: string
+          description: Thing Status
+          format: string
+          example: enabled
+        created_at:
+          type: string
+          format: date-time
+          example: "2019-11-26 13:31:52"
+          description: Time when the channel was created.
+        updated_at:
+          type: string
+          format: date-time
+          example: "2019-11-26 13:31:52"
+          description: Time when the channel was created.
+      xml:
+        name: thing
+
     Channel:
       type: object
       properties:
@@ -1105,7 +1162,7 @@ components:
           minItems: 0
           uniqueItems: true
           items:
-            $ref: "#/components/schemas/Thing"
+            $ref: "#/components/schemas/ThingWithEmptySecret"
         total:
           type: integer
           example: 1

diff --git a/api/openapi/users.yml b/api/openapi/users.yml
@@ -931,10 +931,6 @@ components:
               type: string
               example: [email protected]
               description: User Identity for example email address.
-            secret:
-              type: string
-              example: password
-              description: User secret password.
         metadata:
           type: object
           example: {"domain": "example.com"}

diff --git a/pkg/clients/clients.go b/pkg/clients/clients.go
@@ -34,7 +34,7 @@ var (
 // and "secret" which can be a password or access token.
 type Credentials struct {
 	Identity string `json:"identity,omitempty"` // username or generated login ID
-	Secret   string `json:"secret"`             // password or token
+	Secret   string `json:"secret,omitempty"`   // password or token
 }
 
 // Client represents generic Client.
@@ -43,7 +43,7 @@ type Client struct {
 	Name        string      `json:"name,omitempty"`
 	Tags        []string    `json:"tags,omitempty"`
 	Owner       string      `json:"owner,omitempty"` // nullable
-	Credentials Credentials `json:"credentials"`
+	Credentials Credentials `json:"credentials,omitempty"`
 	Metadata    Metadata    `json:"metadata,omitempty"`
 	CreatedAt   time.Time   `json:"created_at"`
 	UpdatedAt   time.Time   `json:"updated_at,omitempty"`

diff --git a/pkg/clients/postgres/clients.go b/pkg/clients/postgres/clients.go
@@ -139,7 +139,7 @@ func (repo ClientRepository) RetrieveAll(ctx context.Context, pm clients.Page) (
 		return clients.ClientsPage{}, errors.Wrap(errors.ErrViewEntity, err)
 	}
 
-	q := fmt.Sprintf(`SELECT c.id, c.name, c.tags, c.identity, c.secret, c.metadata, COALESCE(c.owner_id, '') AS owner_id, c.status,
+	q := fmt.Sprintf(`SELECT c.id, c.name, c.tags, c.identity, c.metadata, COALESCE(c.owner_id, '') AS owner_id, c.status,
 					c.created_at, c.updated_at, COALESCE(c.updated_by, '') AS updated_by FROM clients c %s ORDER BY c.created_at LIMIT :limit OFFSET :offset;`, query)
 
 	dbPage, err := toDBClientsPage(pm)

diff --git a/pkg/clients/postgres/clients_test.go b/pkg/clients/postgres/clients_test.go
@@ -143,7 +143,7 @@ func TestClientsRetrieveAll(t *testing.T) {
 		if i%50 == 0 {
 			client.Status = mfclients.DisabledStatus
 		}
-		_, err := repo.Save(context.Background(), client)
+		client, err = repo.Save(context.Background(), client)
 		require.Nil(t, err, fmt.Sprintf("unexpected error: %s", err))
 		expectedClients = append(expectedClients, client)
 		var policy = policies.Policy{

diff --git a/pkg/sdk/go/users_test.go b/pkg/sdk/go/users_test.go
@@ -577,6 +577,7 @@ func TestClient(t *testing.T) {
 		repoCall2 := cRepo.On("RetrieveByID", mock.Anything, tc.clientID).Return(convertClient(tc.response), tc.err)
 		rClient, err := mfsdk.User(tc.clientID, tc.token)
 		assert.Equal(t, tc.err, err, fmt.Sprintf("%s: expected error %s, got %s", tc.desc, tc.err, err))
+		tc.response.Credentials.Secret = ""
 		assert.Equal(t, tc.response, rClient, fmt.Sprintf("%s: expected %v got %v\n", tc.desc, tc.response, rClient))
 		if tc.err == nil {
 			ok := repoCall1.Parent.AssertCalled(t, "CheckAdmin", mock.Anything, mock.Anything)
@@ -635,6 +636,7 @@ func TestProfile(t *testing.T) {
 		repoCall := cRepo.On("RetrieveByID", mock.Anything, mock.Anything).Return(convertClient(tc.response), tc.err)
 		rClient, err := mfsdk.UserProfile(tc.token)
 		assert.Equal(t, tc.err, err, fmt.Sprintf("%s: expected error %s, got %s", tc.desc, tc.err, err))
+		tc.response.Credentials.Secret = ""
 		assert.Equal(t, tc.response, rClient, fmt.Sprintf("%s: expected %v got %v\n", tc.desc, tc.response, rClient))
 		if tc.err == nil {
 			ok := repoCall.Parent.AssertCalled(t, "RetrieveByID", mock.Anything, mock.Anything)

diff --git a/users/clients/service.go b/users/clients/service.go
@@ -149,16 +149,27 @@ func (svc service) ViewClient(ctx context.Context, token string, id string) (mfc
 	if err := svc.authorize(ctx, ir, id, listRelationKey); err != nil {
 		return mfclients.Client{}, err
 	}
+	client, err := svc.clients.RetrieveByID(ctx, id)
+	if err != nil {
+		return mfclients.Client{}, err
+	}
+	client.Credentials.Secret = ""
 
-	return svc.clients.RetrieveByID(ctx, id)
+	return client, nil
 }
 
 func (svc service) ViewProfile(ctx context.Context, token string) (mfclients.Client, error) {
 	id, err := svc.Identify(ctx, token)
 	if err != nil {
 		return mfclients.Client{}, err
 	}
-	return svc.clients.RetrieveByID(ctx, id)
+	client, err := svc.clients.RetrieveByID(ctx, id)
+	if err != nil {
+		return mfclients.Client{}, err
+	}
+	client.Credentials.Secret = ""
+
+	return client, nil
 }
 
 func (svc service) ListClients(ctx context.Context, token string, pm mfclients.Page) (mfclients.ClientsPage, error) {

diff --git a/users/clients/service_test.go b/users/clients/service_test.go
@@ -43,7 +43,7 @@ var (
 	passRegex       = regexp.MustCompile("^.{8,}$")
 	accessDuration  = time.Minute * 1
 	refreshDuration = time.Minute * 10
-	myKey = "mine"
+	myKey           = "mine"
 )
 
 func TestRegisterClient(t *testing.T) {
@@ -307,6 +307,7 @@ func TestViewClient(t *testing.T) {
 		repoCall2 := cRepo.On("RetrieveByID", context.Background(), tc.clientID).Return(tc.response, tc.err)
 		rClient, err := svc.ViewClient(context.Background(), tc.token, tc.clientID)
 		assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s: expected %s got %s\n", tc.desc, tc.err, err))
+		tc.response.Credentials.Secret = ""
 		assert.Equal(t, tc.response, rClient, fmt.Sprintf("%s: expected %v got %v\n", tc.desc, tc.response, rClient))
 		if tc.err == nil {
 			ok := repoCall1.Parent.AssertCalled(t, "CheckAdmin", context.Background(), mock.Anything)