Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NOISSUE - Fix: Clients, Channels roles initialization and Channels connection Authz #2663

Merged
merged 1 commit into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions channels/api/grpc/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import (
func authorizeEndpoint(svc channels.Service) endpoint.Endpoint {
return func(ctx context.Context, request interface{}) (interface{}, error) {
req := request.(authorizeReq)
if err := req.validate(); err != nil {
return authorizeRes{}, err
}

if err := svc.Authorize(ctx, ch.AuthzReq{
DomainID: req.domainID,
Expand Down
16 changes: 15 additions & 1 deletion channels/api/grpc/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,13 @@

package grpc

import "github.com/absmach/supermq/pkg/connections"
import (
"github.com/absmach/supermq/pkg/connections"
"github.com/absmach/supermq/pkg/errors"
"github.com/absmach/supermq/pkg/policies"
)

var errDomainID = errors.New("domain id required for users")

type authorizeReq struct {
domainID string
Expand All @@ -12,6 +18,14 @@ type authorizeReq struct {
clientType string
connType connections.ConnType
}

func (req authorizeReq) validate() error {
if req.clientType == policies.UserType && req.domainID == "" {
return errDomainID
}
return nil
}

type removeClientConnectionsReq struct {
clientID string
}
Expand Down
8 changes: 8 additions & 0 deletions channels/api/http/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
apiutil "github.com/absmach/supermq/api/http/util"
"github.com/absmach/supermq/channels"
smqauthn "github.com/absmach/supermq/pkg/authn"
roleManagerHttp "github.com/absmach/supermq/pkg/roles/rolemanager/api"
"github.com/go-chi/chi/v5"
kithttp "github.com/go-kit/kit/transport/http"
"github.com/prometheus/client_golang/prometheus/promhttp"
Expand All @@ -22,6 +23,9 @@ func MakeHandler(svc channels.Service, authn smqauthn.Authentication, mux *chi.M
opts := []kithttp.ServerOption{
kithttp.ServerErrorEncoder(apiutil.LoggingErrorEncoder(logger, api.EncodeError)),
}

d := roleManagerHttp.NewDecoder("channelID")

mux.Route("/{domainID}/channels", func(r chi.Router) {
r.Use(api.AuthenticateMiddleware(authn, true))

Expand Down Expand Up @@ -60,6 +64,8 @@ func MakeHandler(svc channels.Service, authn smqauthn.Authentication, mux *chi.M
opts...,
), "disconnect").ServeHTTP)

r = roleManagerHttp.EntityAvailableActionsRouter(svc, d, r, opts)

r.Route("/{channelID}", func(r chi.Router) {
r.Get("/", otelhttp.NewHandler(kithttp.NewServer(
viewChannelEndpoint(svc),
Expand Down Expand Up @@ -130,6 +136,8 @@ func MakeHandler(svc channels.Service, authn smqauthn.Authentication, mux *chi.M
api.EncodeResponse,
opts...,
), "disconnect_channel_client").ServeHTTP)

roleManagerHttp.EntityRoleMangerRouter(svc, d, r, opts)
})
})

Expand Down
7 changes: 7 additions & 0 deletions channels/builtinroles.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Copyright (c) Abstract Machines
// SPDX-License-Identifier: Apache-2.0
package channels

import "github.com/absmach/supermq/pkg/roles"

const BuiltInRoleAdmin roles.BuiltInRoleName = "admin"
8 changes: 7 additions & 1 deletion channels/private/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package private
import (
"context"

"github.com/absmach/supermq/auth"
"github.com/absmach/supermq/channels"
"github.com/absmach/supermq/pkg/errors"
svcerr "github.com/absmach/supermq/pkg/errors/service"
Expand Down Expand Up @@ -35,10 +36,15 @@ func New(repo channels.Repository, evaluator policies.Evaluator, policy policies
func (svc service) Authorize(ctx context.Context, req channels.AuthzReq) error {
switch req.ClientType {
case policies.UserType:
permission, err := req.Type.Permission()
if err != nil {
return err
}
pr := policies.Policy{
Subject: req.ClientID,
Subject: auth.EncodeDomainUserID(req.DomainID, req.ClientID),
SubjectType: policies.UserType,
Object: req.ChannelID,
Permission: permission,
ObjectType: policies.ChannelType,
}
if err := svc.evaluator.CheckPolicy(ctx, pr); err != nil {
Expand Down
43 changes: 0 additions & 43 deletions channels/roleactions.go

This file was deleted.

4 changes: 2 additions & 2 deletions channels/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ type service struct {

var _ Service = (*service)(nil)

func New(repo Repository, policy policies.Service, idProvider supermq.IDProvider, clients grpcClientsV1.ClientsServiceClient, groups grpcGroupsV1.GroupsServiceClient, sidProvider supermq.IDProvider) (Service, error) {
rpms, err := roles.NewProvisionManageService(policies.ChannelType, repo, policy, sidProvider, AvailableActions(), BuiltInRoles())
func New(repo Repository, policy policies.Service, idProvider supermq.IDProvider, clients grpcClientsV1.ClientsServiceClient, groups grpcGroupsV1.GroupsServiceClient, sidProvider supermq.IDProvider, availableActions []roles.Action, builtInRoles map[roles.BuiltInRoleName][]roles.Action) (Service, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This signature looks messy. We have id provider twice, and we have way too many params. Can we group roles and actions (maybe something else as well) to a new config struct?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still not addressed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Roles need short ID provider, but now we are using uuid for role , it is making the url path longer.
Already we are domain id and entity id in url.

Example requests:
For instance if Client id is 87714e08-3087-476e-8a12-c7bee595b31d client role id is client_4tuyg19Dwd6AzwmnbWr3v6Wm and domain id is 960c325a-c973-45f8-9c0b-f9f686941c7b

  • Get All actions of client
    Method: GET URL: http://localhost/960c325a-c973-45f8-9c0b-f9f686941c7b/clients/87714e08-3087-476e-8a12-c7bee595b31d/roles/client_4tuyg19Dwd6AzwmnbWr3v6Wm/actions

  • Add All actions
    Method: POST URL: http://localhost/960c325a-c973-45f8-9c0b-f9f686941c7b/clients/87714e08-3087-476e-8a12-c7bee595b31d/roles/client_4tuyg19Dwd6AzwmnbWr3v6Wm/actions

  • Delete actions
    Method: POST URL: http://localhost/960c325a-c973-45f8-9c0b-f9f686941c7b/clients/87714e08-3087-476e-8a12-c7bee595b31d/roles/client_4tuyg19Dwd6AzwmnbWr3v6Wm/actions/delete

  • Delete all actions
    Method: POST URL: http://localhost/960c325a-c973-45f8-9c0b-f9f686941c7b/clients/87714e08-3087-476e-8a12-c7bee595b31d/roles/client_4tuyg19Dwd6AzwmnbWr3v6Wm/actions/delete-all

  • Get All members
    Method: GET URL: http://localhost/960c325a-c973-45f8-9c0b-f9f686941c7b/clients/87714e08-3087-476e-8a12-c7bee595b31d/roles/client_4tuyg19Dwd6AzwmnbWr3v6Wm/members

  • Add All members
    Method: POST URL: http://localhost/960c325a-c973-45f8-9c0b-f9f686941c7b/clients/87714e08-3087-476e-8a12-c7bee595b31d/roles/client_4tuyg19Dwd6AzwmnbWr3v6Wm/members

  • Delete members
    Method: POST URL: http://localhost/960c325a-c973-45f8-9c0b-f9f686941c7b/clients/87714e08-3087-476e-8a12-c7bee595b31d/roles/client_4tuyg19Dwd6AzwmnbWr3v6Wm/members

  • Delete all members
    Method: POST URL: http://localhost/960c325a-c973-45f8-9c0b-f9f686941c7b/clients/87714e08-3087-476e-8a12-c7bee595b31d/roles/client_4tuyg19Dwd6AzwmnbWr3v6Wm/members/delete-all

Here Role ID : client_4tuyg19Dwd6AzwmnbWr3v6Wm is encoded using https://github.com/sqids/sqids-go
The code is here https://github.com/absmach/supermq/blob/main/pkg/sid/sid.go

So something like this https://github.com/teris-io/shortid we need
Because of licenses i could decide which one to use

Related to AvailableActions and BuiltInRoles could not be combined because they are using for different purpose.
AvailableActions is list of string which helps user to understand which actions can be added to role.
BuiltInRole contains map of built in role with respective actions , which will automatically added to entity during it creation process

rpms, err := roles.NewProvisionManageService(policies.ChannelType, repo, policy, sidProvider, availableActions, builtInRoles)
if err != nil {
return nil, err
}
Expand Down
6 changes: 5 additions & 1 deletion channels/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@ func newService(t *testing.T) channels.Service {
policies = new(policymocks.Service)
clientsSvc = new(clmocks.ClientsServiceClient)
groupsSvc = new(gpmocks.GroupsServiceClient)
svc, err := channels.New(repo, policies, idProvider, clientsSvc, groupsSvc, idProvider)
availableActions := []roles.Action{}
builtInRoles := map[roles.BuiltInRoleName][]roles.Action{
clients.BuiltInRoleAdmin: availableActions,
}
svc, err := channels.New(repo, policies, idProvider, clientsSvc, groupsSvc, idProvider, availableActions, builtInRoles)
assert.Nil(t, err, fmt.Sprintf(" Unexpected error while creating service %v", err))
return svc
}
Expand Down
9 changes: 2 additions & 7 deletions clients/api/http/clients.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func clientsHandler(svc clients.Service, authn smqauthn.Authentication, r *chi.M
api.EncodeResponse,
opts...,
), "create_clients").ServeHTTP)

r = roleManagerHttp.EntityAvailableActionsRouter(svc, d, r, opts)

r.Route("/{clientID}", func(r chi.Router) {
Expand Down Expand Up @@ -111,16 +112,10 @@ func clientsHandler(svc clients.Service, authn smqauthn.Authentication, r *chi.M
api.EncodeResponse,
opts...,
), "delete_client").ServeHTTP)

roleManagerHttp.EntityRoleMangerRouter(svc, d, r, opts)
})
})

r.Get("/{domainID}/users/{userID}/clients", otelhttp.NewHandler(kithttp.NewServer(
listClientsEndpoint(svc),
decodeListClients,
api.EncodeResponse,
opts...,
), "list_user_clients").ServeHTTP)
})
return r
}
7 changes: 7 additions & 0 deletions clients/builtinroles.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Copyright (c) Abstract Machines
// SPDX-License-Identifier: Apache-2.0
package clients

import "github.com/absmach/supermq/pkg/roles"

const BuiltInRoleAdmin roles.BuiltInRoleName = "admin"
2 changes: 1 addition & 1 deletion clients/postgres/clients.go
Original file line number Diff line number Diff line change
Expand Up @@ -881,7 +881,7 @@ func ToClient(t DBClient) (clients.Client, error) {
updatedAt = t.UpdatedAt.Time
}

connTypes := []connections.ConnType{}
var connTypes []connections.ConnType
for _, ct := range t.ConnectionTypes {
connType, err := connections.NewType(uint(ct))
if err != nil {
Expand Down
44 changes: 0 additions & 44 deletions clients/roleactions.go

This file was deleted.

6 changes: 3 additions & 3 deletions clients/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ type service struct {
}

// NewService returns a new Clients service implementation.
func NewService(repo Repository, policy policies.Service, cache Cache, channels grpcChannelsV1.ChannelsServiceClient, groups grpcGroupsV1.GroupsServiceClient, idProvider smq.IDProvider, sIDProvider smq.IDProvider) (Service, error) {
rpms, err := roles.NewProvisionManageService(policies.ClientType, repo, policy, sIDProvider, AvailableActions(), BuiltInRoles())
func NewService(repo Repository, policy policies.Service, cache Cache, channels grpcChannelsV1.ChannelsServiceClient, groups grpcGroupsV1.GroupsServiceClient, idProvider smq.IDProvider, sIDProvider smq.IDProvider, availableActions []roles.Action, builtInRoles map[roles.BuiltInRoleName][]roles.Action) (Service, error) {
rpms, err := roles.NewProvisionManageService(policies.ClientType, repo, policy, sIDProvider, availableActions, builtInRoles)
if err != nil {
return service{}, err
}
Expand Down Expand Up @@ -95,7 +95,7 @@ func (svc service) CreateClients(ctx context.Context, session authn.Session, cls
}()

newBuiltInRoleMembers := map[roles.BuiltInRoleName][]roles.Member{
ClientBuiltInRoleAdmin: {roles.Member(session.UserID)},
BuiltInRoleAdmin: {roles.Member(session.UserID)},
}

optionalPolicies := []policies.Policy{}
Expand Down
6 changes: 5 additions & 1 deletion clients/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,11 @@ func newService() clients.Service {
repo = new(climocks.Repository)
chgRPCClient = new(chmocks.ChannelsServiceClient)
gpgRPCClient = new(gpmocks.GroupsServiceClient)
tsv, _ := clients.NewService(repo, pService, cache, chgRPCClient, gpgRPCClient, idProvider, sidProvider)
availableActions := []roles.Action{}
builtInRoles := map[roles.BuiltInRoleName][]roles.Action{
clients.BuiltInRoleAdmin: availableActions,
}
tsv, _ := clients.NewService(repo, pService, cache, chgRPCClient, gpgRPCClient, idProvider, sidProvider, availableActions, builtInRoles)
return tsv
}

Expand Down
31 changes: 26 additions & 5 deletions cmd/channels/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,12 @@ import (
pg "github.com/absmach/supermq/pkg/postgres"
pgclient "github.com/absmach/supermq/pkg/postgres"
"github.com/absmach/supermq/pkg/prometheus"
"github.com/absmach/supermq/pkg/roles"
"github.com/absmach/supermq/pkg/server"
grpcserver "github.com/absmach/supermq/pkg/server/grpc"
httpserver "github.com/absmach/supermq/pkg/server/http"
"github.com/absmach/supermq/pkg/sid"
spicedbdecoder "github.com/absmach/supermq/pkg/spicedb"
"github.com/absmach/supermq/pkg/uuid"
"github.com/authzed/authzed-go/v1"
"github.com/authzed/grpcutil"
Expand Down Expand Up @@ -78,11 +80,12 @@ type config struct {
JaegerURL url.URL `env:"SMQ_JAEGER_URL" envDefault:"http://localhost:4318/v1/traces"`
SendTelemetry bool `env:"SMQ_SEND_TELEMETRY" envDefault:"true"`
ESURL string `env:"SMQ_ES_URL" envDefault:"nats://localhost:4222"`
ESConsumerName string `env:"SMQ_CLIENTS_EVENT_CONSUMER" envDefault:"channels"`
ESConsumerName string `env:"SMQ_CHANNELS_EVENT_CONSUMER" envDefault:"channels"`
TraceRatio float64 `env:"SMQ_JAEGER_TRACE_RATIO" envDefault:"1.0"`
SpicedbHost string `env:"SMQ_SPICEDB_HOST" envDefault:"localhost"`
SpicedbPort string `env:"SMQ_SPICEDB_PORT" envDefault:"50051"`
SpicedbPreSharedKey string `env:"SMQ_SPICEDB_PRE_SHARED_KEY" envDefault:"12345678"`
SpicedbSchemaFile string `env:"SMQ_SPICEDB_SCHEMA_FILE" envDefault:"schema.zed"`
}

func main() {
Expand Down Expand Up @@ -222,7 +225,7 @@ func main() {
defer groupsHandler.Close()
logger.Info("Groups gRPC client successfully connected to groups gRPC server " + groupsHandler.Secure())

svc, psvc, err := newService(ctx, db, dbConfig, authz, policyEvaluator, policyService, cfg.ESURL, tracer, clientsClient, groupsClient, logger)
svc, psvc, err := newService(ctx, db, dbConfig, authz, policyEvaluator, policyService, cfg, tracer, clientsClient, groupsClient, logger)
if err != nil {
logger.Error(fmt.Sprintf("failed to create services: %s", err))
exitCode = 1
Expand Down Expand Up @@ -293,7 +296,7 @@ func main() {
}

func newService(ctx context.Context, db *sqlx.DB, dbConfig pgclient.Config, authz smqauthz.Authorization,
pe policies.Evaluator, ps policies.Service, esURL string, tracer trace.Tracer, clientsClient grpcClientsV1.ClientsServiceClient,
pe policies.Evaluator, ps policies.Service, cfg config, tracer trace.Tracer, clientsClient grpcClientsV1.ClientsServiceClient,
groupsClient grpcGroupsV1.GroupsServiceClient, logger *slog.Logger,
) (channels.Service, pChannels.Service, error) {
database := pg.NewDatabase(db, dbConfig, tracer)
Expand All @@ -305,12 +308,17 @@ func newService(ctx context.Context, db *sqlx.DB, dbConfig pgclient.Config, auth
return nil, nil, err
}

svc, err := channels.New(repo, ps, idp, clientsClient, groupsClient, sidp)
availableActions, buildInRoles, err := availableActionsAndBuiltInRoles(cfg.SpicedbSchemaFile)
if err != nil {
return nil, nil, err
}

svc, err = events.NewEventStoreMiddleware(ctx, svc, esURL)
svc, err := channels.New(repo, ps, idp, clientsClient, groupsClient, sidp, availableActions, buildInRoles)
if err != nil {
return nil, nil, err
}

svc, err = events.NewEventStoreMiddleware(ctx, svc, cfg.ESURL)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -344,3 +352,16 @@ func newSpiceDBPolicyServiceEvaluator(cfg config, logger *slog.Logger) (policies
pe := spicedb.NewPolicyEvaluator(client, logger)
return pe, ps, nil
}

func availableActionsAndBuiltInRoles(spicedbSchemaFile string) ([]roles.Action, map[roles.BuiltInRoleName][]roles.Action, error) {
availableActions, err := spicedbdecoder.GetActionsFromSchema(spicedbSchemaFile, policies.ChannelType)
if err != nil {
return []roles.Action{}, map[roles.BuiltInRoleName][]roles.Action{}, err
}

builtInRoles := map[roles.BuiltInRoleName][]roles.Action{
channels.BuiltInRoleAdmin: availableActions,
}

return availableActions, builtInRoles, err
}
Loading
Loading