Fix Bug on Things Authorization Cache (#1810)

Signed-off-by: rodneyosodo <blackd0t@protonmail.com> Signed-off-by: dusanb94 <dusan.borovcanin@mainflux.com>
absmach · Jun 13, 2023 · f97df0a · f97df0a
1 parent 230ccb6
commit f97df0a
Show file tree

Hide file tree

Showing 9 changed files with 21 additions and 22 deletions.
diff --git a/pkg/auth/client.go b/pkg/auth/client.go
@@ -40,10 +40,10 @@ func (c client) Identify(ctx context.Context, thingKey string) (string, error) {
 	thingID, err := c.redisClient.Get(ctx, tkey).Result()
 	if err != nil {
 		t := &policies.Key{
-			Value: string(thingKey),
+			Value: thingKey,
 		}
 
-		thid, err := c.thingsClient.Identify(context.TODO(), t)
+		thid, err := c.thingsClient.Identify(ctx, t)
 		if err != nil {
 			return "", err
 		}
@@ -53,10 +53,6 @@ func (c client) Identify(ctx context.Context, thingKey string) (string, error) {
 }
 
 func (c client) Authorize(ctx context.Context, chanID, thingID, action string) error {
-	if c.redisClient.SIsMember(ctx, chanPrefix+":"+chanID, thingID).Val() {
-		return nil
-	}
-
 	ar := &policies.AuthorizeReq{
 		Sub:        thingID,
 		Obj:        chanID,

diff --git a/things/clients/clients.go b/things/clients/clients.go
@@ -51,8 +51,8 @@ type Service interface {
 	Identify(ctx context.Context, key string) (string, error)
 }
 
-// ClientCache contains thing caching interface.
-type ClientCache interface {
+// Cache contains thing caching interface.
+type Cache interface {
 	// Save stores pair thing secret, thing id.
 	Save(ctx context.Context, thingSecret, thingID string) error
 

diff --git a/things/clients/mocks/things.go b/things/clients/mocks/things.go
@@ -299,8 +299,8 @@ type clientCacheMock struct {
 	things map[string]string
 }
 
-// NewClientCache returns mock cache instance.
-func NewClientCache() clients.ClientCache {
+// NewCache returns mock cache instance.
+func NewCache() clients.Cache {
 	return &clientCacheMock{
 		things: make(map[string]string),
 	}

diff --git a/things/clients/redis/things.go b/things/clients/redis/things.go
@@ -17,14 +17,14 @@ const (
 	idPrefix  = "thing_id"
 )
 
-var _ clients.ClientCache = (*thingCache)(nil)
+var _ clients.Cache = (*thingCache)(nil)
 
 type thingCache struct {
 	client *redis.Client
 }
 
 // NewCache returns redis thing cache implementation.
-func NewCache(client *redis.Client) clients.ClientCache {
+func NewCache(client *redis.Client) clients.Cache {
 	return &thingCache{
 		client: client,
 	}

diff --git a/things/clients/service.go b/things/clients/service.go
@@ -31,13 +31,13 @@ type service struct {
 	uauth       upolicies.AuthServiceClient
 	policies    tpolicies.Service
 	clients     mfclients.Repository
-	clientCache ClientCache
+	clientCache Cache
 	idProvider  mainflux.IDProvider
 	grepo       mfgroups.Repository
 }
 
 // NewService returns a new Clients service implementation.
-func NewService(uauth upolicies.AuthServiceClient, policies tpolicies.Service, c mfclients.Repository, grepo mfgroups.Repository, tcache ClientCache, idp mainflux.IDProvider) Service {
+func NewService(uauth upolicies.AuthServiceClient, policies tpolicies.Service, c mfclients.Repository, grepo mfgroups.Repository, tcache Cache, idp mainflux.IDProvider) Service {
 	return service{
 		uauth:       uauth,
 		policies:    policies,

diff --git a/things/clients/service_test.go b/things/clients/service_test.go
@@ -43,7 +43,7 @@ var (
 func newService(tokens map[string]string) (clients.Service, *mocks.ClientRepository, *pmocks.PolicyRepository) {
 	adminPolicy := mocks.MockSubjectSet{Object: ID, Relation: clients.AdminRelationKey}
 	auth := mocks.NewAuthService(tokens, map[string][]mocks.MockSubjectSet{adminEmail: {adminPolicy}})
-	thingCache := mocks.NewClientCache()
+	thingCache := mocks.NewCache()
 	policiesCache := pmocks.NewChannelCache()
 	idProvider := uuid.NewMock()
 	cRepo := new(mocks.ClientRepository)

diff --git a/things/policies/service.go b/things/policies/service.go
@@ -86,14 +86,14 @@ func (svc service) Authorize(ctx context.Context, ar AccessRequest) (Policy, err
 		// Replace Subject since AccessRequest Subject is Thing Key,
 		// and Policy subject is Thing ID.
 		policy.Subject = ar.Subject
+		if err := svc.policyCache.Put(ctx, policy); err != nil {
+			return policy, err
+		}
+
 	default:
 		return Policy{}, ErrInvalidEntityType
 	}
 
-	if err := svc.policyCache.Put(ctx, policy); err != nil {
-		return policy, err
-	}
-
 	return policy, nil
 }
 
@@ -165,6 +165,10 @@ func (svc service) UpdatePolicy(ctx context.Context, token string, p Policy) (Po
 	p.UpdatedAt = time.Now()
 	p.UpdatedBy = userID
 
+	if err := svc.policyCache.Remove(ctx, p); err != nil {
+		return Policy{}, err
+	}
+
 	return svc.policies.Update(ctx, p)
 }
 

diff --git a/things/policies/tracing/tracing.go b/things/policies/tracing/tracing.go
@@ -21,7 +21,7 @@ func TracingMiddleware(psvc policies.Service, tracer trace.Tracer) policies.Serv
 }
 
 func (tm *tracingMiddleware) Authorize(ctx context.Context, ar policies.AccessRequest) (policies.Policy, error) {
-	ctx, span := tm.tracer.Start(ctx, "svc_authorize_by_key", trace.WithAttributes(attribute.String("subject", ar.Subject), attribute.String("object", ar.Object), attribute.String("action", ar.Action)))
+	ctx, span := tm.tracer.Start(ctx, "svc_authorize", trace.WithAttributes(attribute.String("subject", ar.Subject), attribute.String("object", ar.Object), attribute.String("action", ar.Action)))
 	defer span.End()
 
 	return tm.psvc.Authorize(ctx, ar)

diff --git a/things/postgres/init.go b/things/postgres/init.go
@@ -24,8 +24,7 @@ func Migration() *migrate.MemoryMigrationSource {
 						created_at	TIMESTAMP,
 						updated_at	TIMESTAMP,
 						updated_by  VARCHAR(254),
-						status		SMALLINT NOT NULL DEFAULT 0 CHECK (status >= 0),
-						UNIQUE		(owner_id, secret)
+						status		SMALLINT NOT NULL DEFAULT 0 CHECK (status >= 0)
 					)`,
 					`CREATE TABLE IF NOT EXISTS groups (
 						id			VARCHAR(36) PRIMARY KEY,