NOISSUE - Update Add and Delete Policies (#1792)

* Remove Policy Action Ranks

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Fix Rebase Issues

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Fix CI Test Errors

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Adding Check on Subject For Clients

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Remove Check Client Exists

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Check When Sharing Clients

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Only Add User to Group When Sharing Things

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Remove clientType

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Minor Fix on ShareClient and Fix Tests

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Fix Policies Tests

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Clean Up Things Authorization

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Fix Tests on RetrieveAll

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Test ShareThing

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Fix Merge Conflicts

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Remove Adding Policies. Only Use Ownership

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Check If Subject is same as Object

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Move Back To Union As Sometimes Policy is Empty and Fails to Evaluate on Ownership

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Fix Entity Type For Failing Tests

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Fix BUG in policy evaluation

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Fix Tests

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Add Comments Regarding checkAdmin

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Fix Tests On Rebase

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Combine Authorize For Things and Users

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Fix Tests On Rebase

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

* Error on Things SVC `unsupported protocol scheme`

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

---------

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>
Signed-off-by: dusanb94 <dusan.borovcanin@mainflux.com>

bootstrap/mocks/policies.go

@@ -30,19 +30,19 @@ func NewPoliciesService(auth upolicies.AuthServiceClient) tpolicies.Service {
 	}
 }
 
-func (svc *mainfluxPolicies) AddPolicy(_ context.Context, token string, p tpolicies.Policy) (tpolicies.Policy, error) {
+func (svc *mainfluxPolicies) AddPolicy(ctx context.Context, token string, p tpolicies.Policy) (tpolicies.Policy, error) {
 	svc.mu.Lock()
 	defer svc.mu.Unlock()
 
-	if _, err := svc.auth.Identify(context.Background(), &upolicies.Token{Value: token}); err != nil {
+	if _, err := svc.auth.Identify(ctx, &upolicies.Token{Value: token}); err != nil {
 		return tpolicies.Policy{}, errors.ErrAuthentication
 	}
 	svc.connections[fmt.Sprintf("%s:%s", p.Subject, p.Object)] = p
 
 	return p, nil
 }
 
-func (svc *mainfluxPolicies) DeletePolicy(_ context.Context, token string, p tpolicies.Policy) error {
+func (svc *mainfluxPolicies) DeletePolicy(ctx context.Context, token string, p tpolicies.Policy) error {
 	svc.mu.Lock()
 	defer svc.mu.Unlock()
 
@@ -62,7 +62,7 @@ func (svc *mainfluxPolicies) UpdatePolicy(context.Context, string, tpolicies.Pol
 	panic("not implemented")
 }
 
-func (svc *mainfluxPolicies) Authorize(context.Context, tpolicies.AccessRequest, string) (string, error) {
+func (svc *mainfluxPolicies) Authorize(context.Context, tpolicies.AccessRequest) (tpolicies.Policy, error) {
 	panic("not implemented")
 }
 

bootstrap/mocks/things.go

@@ -134,6 +134,6 @@ func (svc *mainfluxThings) Identify(context.Context, string) (string, error) {
 	panic("not implemented")
 }
 
-func (svc *mainfluxThings) ShareClient(ctx context.Context, token, thingID string, actions, userIDs []string) error {
+func (svc *mainfluxThings) ShareClient(ctx context.Context, token, userID, groupID, thingID string, actions []string) error {
 	panic("not implemented")
 }

cmd/things/main.go

@@ -118,7 +118,7 @@ func main() {
 		auth = localusers.NewAuthService(cfg.StandaloneID, cfg.StandaloneToken)
 		logger.Info("Using standalone auth service")
 	default:
-		authServiceClient, authHandler, err := authClient.Setup(envPrefix, svcName, cfg.JaegerURL)
+		authServiceClient, authHandler, err := authClient.Setup(envPrefix, cfg.JaegerURL, svcName)
 		if err != nil {
 			logger.Fatal(err.Error())
 		}
@@ -176,9 +176,9 @@ func newService(db *sqlx.DB, auth upolicies.AuthServiceClient, cacheClient *redi
 	policyCache := redispcache.NewCache(cacheClient)
 	thingCache := redisthcache.NewCache(cacheClient)
 
-	csvc := clients.NewService(auth, cRepo, thingCache, idp)
-	gsvc := groups.NewService(auth, gRepo, idp)
-	psvc := tpolicies.NewService(auth, pRepo, thingCache, policyCache, idp)
+	psvc := tpolicies.NewService(auth, pRepo, policyCache, idp)
+	csvc := clients.NewService(auth, psvc, cRepo, gRepo, thingCache, idp)
+	gsvc := groups.NewService(auth, psvc, gRepo, idp)
 
 	csvc = redisthcache.NewEventStoreMiddleware(csvc, esClient)
 	gsvc = redischcache.NewEventStoreMiddleware(gsvc, esClient)

coap/adapter.go

@@ -60,7 +60,7 @@ func (svc *adapterService) Publish(ctx context.Context, key string, msg *messagi
 		Sub:        key,
 		Obj:        msg.Channel,
 		Act:        policies.WriteAction,
-		EntityType: policies.GroupEntityType,
+		EntityType: policies.ThingEntityType,
 	}
 	res, err := svc.auth.Authorize(ctx, ar)
 	if err != nil {

go.mod

@@ -33,7 +33,6 @@ require (
 	github.com/mitchellh/mapstructure v1.5.0
 	github.com/nats-io/nats.go v1.18.0
 	github.com/oklog/ulid/v2 v2.1.0
-	github.com/opentracing/opentracing-go v1.2.0
 	github.com/ory/dockertest/v3 v3.9.1
 	github.com/pelletier/go-toml v1.9.5
 	github.com/plgd-dev/go-coap/v2 v2.6.0

go.sum

@@ -619,8 +619,6 @@ github.com/opencontainers/runc v1.1.5/go.mod h1:1J5XiS+vdZ3wCyZybsuxXZWGrgSr8fFJ
 github.com/opencontainers/runtime-spec v1.0.3-0.20210326190908-1c3f411f0417/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0=
 github.com/opencontainers/selinux v1.10.0/go.mod h1:2i0OySw99QjzBBQByd1Gr9gSjvuho1lHsJxIJ3gGbJI=
 github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o=
-github.com/opentracing/opentracing-go v1.2.0 h1:uEJPy/1a5RIPAJ0Ov+OIO8OxWu77jEv+1B0VhjKrZUs=
-github.com/opentracing/opentracing-go v1.2.0/go.mod h1:GxEUsuufX4nBwe+T+Wl9TAgYrxe9dPLANfrWvHYVTgc=
 github.com/ory/dockertest/v3 v3.9.1 h1:v4dkG+dlu76goxMiTT2j8zV7s4oPPEppKT8K8p2f1kY=
 github.com/ory/dockertest/v3 v3.9.1/go.mod h1:42Ir9hmvaAPm0Mgibk6mBPi7SFvTXxEcnztDYOJ//uM=
 github.com/panjf2000/ants/v2 v2.4.3/go.mod h1:f6F0NZVFsGCp5A7QW/Zj/m92atWwOkY0OIhFxRNFr4A=

http/adapter.go

@@ -39,7 +39,7 @@ func (as *adapterService) Publish(ctx context.Context, token string, msg *messag
 		Sub:        token,
 		Obj:        msg.Channel,
 		Act:        policies.WriteAction,
-		EntityType: policies.GroupEntityType,
+		EntityType: policies.ThingEntityType,
 	}
 	res, err := as.things.Authorize(ctx, ar)
 	if err != nil {

internal/apiutil/errors.go

@@ -120,9 +120,6 @@ var (
 	// ErrMissingPolicyEntityType indicates malformed policy entity type
 	ErrMissingPolicyEntityType = errors.New("malformed or missing entity type")
 
-	// ErrHigherPolicyRank indicates that policies is not the same ranking with parsed policy.
-	ErrHigherPolicyRank = errors.New("policy is of a higher rank that of the client")
-
 	// ErrMissingName indicates missing identity name.
 	ErrMissingName = errors.New("missing identity name")
 

pkg/auth/client.go

@@ -61,7 +61,7 @@ func (c client) Authorize(ctx context.Context, chanID, thingID, action string) e
 		Sub:        thingID,
 		Obj:        chanID,
 		Act:        action,
-		EntityType: policies.GroupEntityType,
+		EntityType: policies.ThingEntityType,
 	}
 	res, err := c.thingsClient.Authorize(ctx, ar)
 	if err != nil {

pkg/sdk/go/policies_test.go

@@ -91,7 +91,7 @@ func TestCreatePolicy(t *testing.T) {
 			page: sdk.PolicyPage{},
 			policy: sdk.Policy{
 				Object:  "obj2",
-				Actions: []string{"c_delete", "c_update", "c_add", "c_list"},
+				Actions: []string{"c_delete", "c_update", "c_list"},
 				Subject: "sub2",
 			},
 			err:   nil,
@@ -141,9 +141,10 @@ func TestCreatePolicy(t *testing.T) {
 	}
 
 	for _, tc := range cases {
-		repoCall := pRepo.On("Retrieve", mock.Anything, mock.Anything).Return(convertPolicyPage(tc.page), nil)
-		repoCall1 := pRepo.On("Update", mock.Anything, mock.Anything).Return(tc.err)
-		repoCall2 := pRepo.On("Save", mock.Anything, mock.Anything).Return(tc.err)
+		repoCall := pRepo.On("CheckAdmin", mock.Anything, mock.Anything).Return(nil)
+		repoCall1 := pRepo.On("Retrieve", mock.Anything, mock.Anything).Return(convertPolicyPage(tc.page), nil)
+		repoCall2 := pRepo.On("Update", mock.Anything, mock.Anything).Return(tc.err)
+		repoCall3 := pRepo.On("Save", mock.Anything, mock.Anything).Return(tc.err)
 		err := policySDK.CreatePolicy(tc.policy, tc.token)
 		assert.Equal(t, tc.err, err, fmt.Sprintf("%s: expected error %s, got %s", tc.desc, tc.err, err))
 		if tc.err == nil {
@@ -159,6 +160,7 @@ func TestCreatePolicy(t *testing.T) {
 		repoCall.Unset()
 		repoCall1.Unset()
 		repoCall2.Unset()
+		repoCall3.Unset()
 	}
 }
 
@@ -212,14 +214,16 @@ func TestUpdatePolicy(t *testing.T) {
 	for _, tc := range cases {
 		policy.Actions = tc.action
 		policy.CreatedAt = time.Now()
-		repoCall := pRepo.On("Retrieve", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(policies.PolicyPage{}, nil)
-		repoCall1 := pRepo.On("Update", mock.Anything, mock.Anything).Return(tc.err)
+		repoCall := pRepo.On("CheckAdmin", mock.Anything, mock.Anything).Return(nil)
+		repoCall1 := pRepo.On("Retrieve", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(policies.PolicyPage{}, nil)
+		repoCall2 := pRepo.On("Update", mock.Anything, mock.Anything).Return(tc.err)
 		err := policySDK.UpdatePolicy(policy, tc.token)
 		assert.Equal(t, tc.err, err, fmt.Sprintf("%s: expected error %s, got %s", tc.desc, tc.err, err))
 		ok := repoCall1.Parent.AssertCalled(t, "Update", mock.Anything, mock.Anything)
 		assert.True(t, ok, fmt.Sprintf("Update was not called on %s", tc.desc))
 		repoCall.Unset()
 		repoCall1.Unset()
+		repoCall2.Unset()
 	}
 }
 
@@ -376,21 +380,25 @@ func TestDeletePolicy(t *testing.T) {
 	pr := sdk.Policy{Object: authoritiesObj, Actions: []string{"m_read", "g_add", "c_delete"}, Subject: sub}
 	cpr := sdk.Policy{Object: authoritiesObj, Actions: []string{"m_read", "g_add", "c_delete"}, Subject: sub}
 
-	repoCall := pRepo.On("Retrieve", mock.Anything, mock.Anything).Return(convertPolicyPage(sdk.PolicyPage{Policies: []sdk.Policy{cpr}}), nil)
-	repoCall1 := pRepo.On("Delete", mock.Anything, mock.Anything).Return(nil)
+	repoCall := pRepo.On("CheckAdmin", mock.Anything, mock.Anything).Return(nil)
+	repoCall1 := pRepo.On("Retrieve", mock.Anything, mock.Anything).Return(convertPolicyPage(sdk.PolicyPage{Policies: []sdk.Policy{cpr}}), nil)
+	repoCall2 := pRepo.On("Delete", mock.Anything, mock.Anything).Return(nil)
 	err := policySDK.DeletePolicy(pr, generateValidToken(t, csvc, cRepo))
 	assert.Nil(t, err, fmt.Sprintf("got unexpected error: %s", err))
 	ok := repoCall1.Parent.AssertCalled(t, "Delete", mock.Anything, mock.Anything)
 	assert.True(t, ok, "Delete was not called on valid policy")
+	repoCall2.Unset()
 	repoCall1.Unset()
 	repoCall.Unset()
 
-	repoCall = pRepo.On("Retrieve", mock.Anything, mock.Anything).Return(convertPolicyPage(sdk.PolicyPage{Policies: []sdk.Policy{cpr}}), nil)
-	repoCall1 = pRepo.On("Delete", mock.Anything, mock.Anything).Return(sdk.ErrFailedRemoval)
+	repoCall = pRepo.On("CheckAdmin", mock.Anything, mock.Anything).Return(nil)
+	repoCall1 = pRepo.On("Retrieve", mock.Anything, mock.Anything).Return(convertPolicyPage(sdk.PolicyPage{Policies: []sdk.Policy{cpr}}), nil)
+	repoCall2 = pRepo.On("Delete", mock.Anything, mock.Anything).Return(sdk.ErrFailedRemoval)
 	err = policySDK.DeletePolicy(pr, invalidToken)
 	assert.Equal(t, err, errors.NewSDKErrorWithStatus(errors.ErrAuthentication, http.StatusUnauthorized), fmt.Sprintf("expected %s got %s", pr, err))
 	ok = repoCall.Parent.AssertCalled(t, "Delete", mock.Anything, mock.Anything)
 	assert.True(t, ok, "Delete was not called on invalid policy")
+	repoCall2.Unset()
 	repoCall1.Unset()
 	repoCall.Unset()
 }

pkg/sdk/go/users_test.go

@@ -50,6 +50,7 @@ func TestCreateClient(t *testing.T) {
 		UsersURL: ts.URL,
 	}
 	clientSDK := sdk.NewSDK(conf)
+	token := testsutil.GenerateValidToken(t, testsutil.GenerateUUID(t, idProvider), svc, cRepo, phasher)
 
 	cases := []struct {
 		desc     string
@@ -169,6 +170,7 @@ func TestCreateClient(t *testing.T) {
 		repoCall := cRepo.On("Save", mock.Anything, mock.Anything).Return(tc.response, tc.err)
 		rClient, err := clientSDK.CreateUser(tc.client, tc.token)
 		tc.response.ID = rClient.ID
+		tc.response.Owner = rClient.Owner
 		tc.response.CreatedAt = rClient.CreatedAt
 		tc.response.UpdatedAt = rClient.UpdatedAt
 		rClient.Credentials.Secret = tc.response.Credentials.Secret
@@ -1080,7 +1082,6 @@ func TestEnableClient(t *testing.T) {
 			Limit:  100,
 			Status: tc.status,
 		}
-
 		repoCall := pRepo.On("CheckAdmin", mock.Anything, mock.Anything).Return(nil)
 		repoCall1 := cRepo.On("RetrieveAll", mock.Anything, mock.Anything).Return(convertClientsPage(tc.response), nil)
 		clientsPage, err := clientSDK.Users(pm, generateValidToken(t, svc, cRepo))

things/clients/api/endpoints.go

@@ -162,7 +162,7 @@ func shareClientEndpoint(svc clients.Service) endpoint.Endpoint {
 		if err := req.validate(); err != nil {
 			return nil, err
 		}
-		if err := svc.ShareClient(ctx, req.token, req.clientID, req.Policies, req.UserIDs); err != nil {
+		if err := svc.ShareClient(ctx, req.token, req.UserID, req.GroupID, req.clientID, req.Policies); err != nil {
 			return nil, err
 		}
 		return shareClientRes{}, nil

things/clients/api/logging.go

@@ -141,16 +141,16 @@ func (lm *loggingMiddleware) ListClientsByGroup(ctx context.Context, token, chan
 	return lm.svc.ListClientsByGroup(ctx, token, channelID, cp)
 }
 
-func (lm *loggingMiddleware) ShareClient(ctx context.Context, token, id string, actions, userIDs []string) (err error) {
+func (lm *loggingMiddleware) ShareClient(ctx context.Context, token, userID, groupID, thingID string, actions []string) (err error) {
 	defer func(begin time.Time) {
-		message := fmt.Sprintf("Method share_thing for thing with id %s using token %s took %s to complete", id, token, time.Since(begin))
+		message := fmt.Sprintf("Method share_thing for thing with id %s using token %s took %s to complete", thingID, token, time.Since(begin))
 		if err != nil {
 			lm.logger.Warn(fmt.Sprintf("%s with error: %s.", message, err))
 			return
 		}
 		lm.logger.Info(fmt.Sprintf("%s without errors.", message))
 	}(time.Now())
-	return lm.svc.ShareClient(ctx, token, id, actions, userIDs)
+	return lm.svc.ShareClient(ctx, token, userID, groupID, thingID, actions)
 }
 
 func (lm *loggingMiddleware) Identify(ctx context.Context, key string) (id string, err error) {

things/clients/api/metrics.go

@@ -66,12 +66,12 @@ func (ms *metricsMiddleware) UpdateClientTags(ctx context.Context, token string,
 	return ms.svc.UpdateClientTags(ctx, token, client)
 }
 
-func (ms *metricsMiddleware) ShareClient(ctx context.Context, token, id string, actions, userIDs []string) error {
+func (ms *metricsMiddleware) ShareClient(ctx context.Context, token, userID, groupID, thingID string, actions []string) error {
 	defer func(begin time.Time) {
 		ms.counter.With("method", "share_thing").Add(1)
 		ms.latency.With("method", "share_thing").Observe(time.Since(begin).Seconds())
 	}(time.Now())
-	return ms.svc.ShareClient(ctx, token, id, actions, userIDs)
+	return ms.svc.ShareClient(ctx, token, userID, groupID, thingID, actions)
 }
 
 func (ms *metricsMiddleware) UpdateClientSecret(ctx context.Context, token, oldSecret, newSecret string) (mfclients.Client, error) {

things/clients/api/requests.go

@@ -209,7 +209,8 @@ func (req changeClientStatusReq) validate() error {
 type shareClientReq struct {
 	token    string
 	clientID string
-	UserIDs  []string `json:"user_ids"`
+	GroupID  string   `json:"group_id"`
+	UserID   string   `json:"user_id"`
 	Policies []string `json:"policies"`
 }
 
@@ -218,7 +219,7 @@ func (req shareClientReq) validate() error {
 		return apiutil.ErrBearerToken
 	}
 
-	if req.clientID == "" || len(req.UserIDs) == 0 {
+	if req.clientID == "" || req.GroupID == "" || req.UserID == "" {
 		return apiutil.ErrMissingID
 	}
 

things/clients/clients.go

@@ -45,7 +45,7 @@ type Service interface {
 	// ShareClient gives actions associated with the thing to the given user IDs.
 	// The requester user identified by the token has to have a "write" relation
 	// on the thing in order to share the thing.
-	ShareClient(ctx context.Context, token, clientID string, actions, userIDs []string) error
+	ShareClient(ctx context.Context, token, userID, groupID, thingID string, actions []string) error
 
 	// Identify returns thing ID for given thing key.
 	Identify(ctx context.Context, key string) (string, error)

things/clients/redis/events.go

@@ -136,15 +136,17 @@ func (rce removeClientEvent) Encode() (map[string]interface{}, error) {
 type shareClientEvent struct {
 	thingID string
 	actions string
-	userIDs string
+	userID  string
+	groupID string
 }
 
 func (sce shareClientEvent) Encode() (map[string]interface{}, error) {
 	return map[string]interface{}{
 		"operation": clientShare,
 		"thing_id":  sce.thingID,
 		"actions":   sce.actions,
-		"user_ids":  sce.userIDs,
+		"user_id":   sce.userID,
+		"group_id":  sce.groupID,
 	}, nil
 }
 

things/clients/redis/streams.go

@@ -116,14 +116,15 @@ func (es eventStore) update(ctx context.Context, operation string, cli mfclients
 	return cli, nil
 }
 
-func (es eventStore) ShareClient(ctx context.Context, token, thingID string, actions, userIDs []string) error {
-	if err := es.svc.ShareClient(ctx, token, thingID, actions, userIDs); err != nil {
+func (es eventStore) ShareClient(ctx context.Context, token, userID, groupID, thingID string, actions []string) error {
+	if err := es.svc.ShareClient(ctx, token, userID, groupID, thingID, actions); err != nil {
 		return err
 	}
 	event := shareClientEvent{
 		thingID: thingID,
+		userID:  userID,
+		groupID: groupID,
 		actions: fmt.Sprintf("[%s]", strings.Join(actions, ",")),
-		userIDs: fmt.Sprintf("[%s]", strings.Join(userIDs, ",")),
 	}
 	values, err := event.Encode()
 	if err != nil {