Skip to content

Commit

Permalink
CSS-6766 implement grant access to service account (#1138)
Browse files Browse the repository at this point in the history
* Implement GrantServiceAccountAccess

* Added godoc

* Fixed test
  • Loading branch information
kian99 authored Jan 25, 2024
1 parent a72e486 commit f70ba71
Show file tree
Hide file tree
Showing 10 changed files with 266 additions and 3 deletions.
10 changes: 10 additions & 0 deletions api/params/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,3 +417,13 @@ type ListServiceAccountCredentialsRequest struct {
// ClientID holds the client id of the service account.
ClientID string `json:"client-id"`
}

// ListServiceAccountCredentialsRequest holds a request to list
// a service accounts cloud credentials.
type GrantServiceAccountAccess struct {
// Entities holds a slice of entities (identities and groups)
// that should have administration access to the desired clientID.
Entities []string `json:"entities"`
// ClientID holds the client id of the service account.
ClientID string `json:"client-id"`
}
24 changes: 24 additions & 0 deletions internal/jimm/service_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"github.com/canonical/jimm/internal/openfga"
ofganames "github.com/canonical/jimm/internal/openfga/names"
jimmnames "github.com/canonical/jimm/pkg/names"
"github.com/juju/zaputil/zapctx"
"go.uber.org/zap"
)

// AddServiceAccount checks that no one owns the service account yet
Expand Down Expand Up @@ -51,3 +53,25 @@ func (j *JIMM) AddServiceAccount(ctx context.Context, u *openfga.User, clientId
}
return nil
}

// GrantServiceAccountAccess creates an administrator relation between the tags provided
// and the service account. The provided tags must be users or groups (with the member relation)
// otherwise OpenFGA will report an error.
func (j *JIMM) GrantServiceAccountAccess(ctx context.Context, u *openfga.User, svcAccTag jimmnames.ServiceAccountTag, tags []*ofganames.Tag) error {
op := errors.Op("jimm.GrantServiceAccountAccess")
tuples := make([]openfga.Tuple, 0, len(tags))
for _, tag := range tags {
tuple := openfga.Tuple{
Object: tag,
Relation: ofganames.AdministratorRelation,
Target: ofganames.ConvertTag(svcAccTag),
}
tuples = append(tuples, tuple)
}
err := j.AuthorizationClient().AddRelation(ctx, tuples...)
if err != nil {
zapctx.Error(ctx, "failed to add tuple(s)", zap.NamedError("add-relation-error", err))
return errors.E(op, errors.CodeOpenFGARequestFailed, err)
}
return nil
}
78 changes: 78 additions & 0 deletions internal/jimm/service_account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,16 @@ import (
"testing"

qt "github.com/frankban/quicktest"
"github.com/juju/names/v4"

"github.com/canonical/jimm/internal/db"
"github.com/canonical/jimm/internal/dbmodel"
"github.com/canonical/jimm/internal/jimm"
"github.com/canonical/jimm/internal/jimmtest"
"github.com/canonical/jimm/internal/openfga"
ofganames "github.com/canonical/jimm/internal/openfga/names"
jimmnames "github.com/canonical/jimm/pkg/names"
"github.com/canonical/ofga"
)

func TestAddServiceAccount(t *testing.T) {
Expand Down Expand Up @@ -46,3 +51,76 @@ func TestAddServiceAccount(t *testing.T) {
err = j.AddServiceAccount(ctx, userAlice, clientID)
c.Assert(err, qt.ErrorMatches, "service account already owned")
}

func TestGrantServiceAccountAccess(t *testing.T) {
c := qt.New(t)

tests := []struct {
about string
grantServiceAccountAccess func(ctx context.Context, user *openfga.User, tags []string) error
clientID string
tags []*ofganames.Tag
username string
expectedError string
}{{
about: "Valid request",
grantServiceAccountAccess: func(ctx context.Context, user *openfga.User, tags []string) error {
return nil
},
tags: []*ofganames.Tag{
&ofga.Entity{
Kind: "user",
ID: "alice",
},
&ofga.Entity{
Kind: "user",
ID: "bob",
},
&ofga.Entity{
Kind: "group",
ID: "1",
Relation: "member",
},
},
clientID: "fca1f605-736e-4d1f-bcd2-aecc726923be",
username: "alice",
}}

for _, test := range tests {
test := test
c.Run(test.about, func(c *qt.C) {
ofgaClient, _, _, err := jimmtest.SetupTestOFGAClient(c.Name())
c.Assert(err, qt.IsNil)
pgDb := db.Database{
DB: jimmtest.PostgresDB(c, nil),
}
err = pgDb.Migrate(context.Background(), false)
c.Assert(err, qt.IsNil)
jimm := &jimm.JIMM{
Database: pgDb,
OpenFGAClient: ofgaClient,
}
var u dbmodel.Identity
u.SetTag(names.NewUserTag(test.clientID))
svcAccountIdentity := openfga.NewUser(&u, ofgaClient)
svcAccountTag := jimmnames.NewServiceAccountTag(test.clientID)

err = jimm.GrantServiceAccountAccess(context.Background(), svcAccountIdentity, svcAccountTag, test.tags)
if test.expectedError == "" {
c.Assert(err, qt.IsNil)
for _, tag := range test.tags {
tuple := openfga.Tuple{
Object: tag,
Relation: ofganames.AdministratorRelation,
Target: ofganames.ConvertTag(jimmnames.NewServiceAccountTag(test.clientID)),
}
ok, err := jimm.AuthorizationClient().CheckRelation(context.Background(), tuple, false)
c.Assert(err, qt.IsNil)
c.Assert(ok, qt.IsTrue)
}
} else {
c.Assert(err, qt.ErrorMatches, test.expectedError)
}
})
}
}
11 changes: 11 additions & 0 deletions internal/jimmtest/jimm_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ import (
"github.com/canonical/jimm/internal/errors"
"github.com/canonical/jimm/internal/jimm"
"github.com/canonical/jimm/internal/openfga"
ofganames "github.com/canonical/jimm/internal/openfga/names"
"github.com/canonical/jimm/internal/pubsub"
jimmnames "github.com/canonical/jimm/pkg/names"
)

// JIMM is a default implementation of the jujuapi.JIMM interface. Every method
Expand Down Expand Up @@ -64,6 +66,7 @@ type JIMM struct {
GrantCloudAccess_ func(ctx context.Context, user *openfga.User, ct names.CloudTag, ut names.UserTag, access string) error
GrantModelAccess_ func(ctx context.Context, user *openfga.User, mt names.ModelTag, ut names.UserTag, access jujuparams.UserAccessPermission) error
GrantOfferAccess_ func(ctx context.Context, u *openfga.User, offerURL string, ut names.UserTag, access jujuparams.OfferAccessPermission) error
GrantServiceAccountAccess_ func(ctx context.Context, u *openfga.User, svcAccTag jimmnames.ServiceAccountTag, tags []*ofganames.Tag) error
ImportModel_ func(ctx context.Context, user *openfga.User, controllerName string, modelTag names.ModelTag, newOwner string) error
IdentityModelDefaults_ func(ctx context.Context, user *dbmodel.Identity) (map[string]interface{}, error)
InitiateMigration_ func(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec, targetControllerID uint) (jujuparams.InitiateMigrationResult, error)
Expand Down Expand Up @@ -330,6 +333,14 @@ func (j *JIMM) GrantOfferAccess(ctx context.Context, u *openfga.User, offerURL s
}
return j.GrantOfferAccess_(ctx, u, offerURL, ut, access)
}

func (j *JIMM) GrantServiceAccountAccess(ctx context.Context, u *openfga.User, svcAccTag jimmnames.ServiceAccountTag, tags []*ofganames.Tag) error {
if j.GrantServiceAccountAccess_ == nil {
return errors.E(errors.CodeNotImplemented)
}
return j.GrantServiceAccountAccess_(ctx, u, svcAccTag, tags)
}

func (j *JIMM) ImportModel(ctx context.Context, user *openfga.User, controllerName string, modelTag names.ModelTag, newOwner string) error {
if j.ImportModel_ == nil {
return errors.E(errors.CodeNotImplemented)
Expand Down
2 changes: 1 addition & 1 deletion internal/jujuapi/access_control.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func resolveTag(jimmUUID string, db *db.Database, tag string) (*ofganames.Tag, e
}
err := db.GetGroup(ctx, entry)
if err != nil {
return nil, errors.E("group not found")
return nil, errors.E(fmt.Sprintf("group %s not found", trailer))
}
return ofganames.ConvertTagWithRelation(jimmnames.NewGroupTag(strconv.FormatUint(uint64(entry.ID), 10)), relation), nil

Expand Down
2 changes: 1 addition & 1 deletion internal/jujuapi/access_control_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1334,7 +1334,7 @@ func (s *accessControlSuite) TestResolveTupleObjectHandlesErrors(c *gc.C) {
// Resolves bad groups where they do not exist
{
input: "group-myspecialpokemon-his-name-is-youguessedit-diglett",
want: "group not found",
want: "group myspecialpokemon-his-name-is-youguessedit-diglett not found",
},
// Resolves bad controllers where they do not exist
{
Expand Down
3 changes: 3 additions & 0 deletions internal/jujuapi/controllerroot.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import (
"github.com/canonical/jimm/internal/jimm"
"github.com/canonical/jimm/internal/jujuapi/rpc"
"github.com/canonical/jimm/internal/openfga"
ofganames "github.com/canonical/jimm/internal/openfga/names"
"github.com/canonical/jimm/internal/pubsub"
jimmnames "github.com/canonical/jimm/pkg/names"
)

type JIMM interface {
Expand Down Expand Up @@ -61,6 +63,7 @@ type JIMM interface {
GrantCloudAccess(ctx context.Context, user *openfga.User, ct names.CloudTag, ut names.UserTag, access string) error
GrantModelAccess(ctx context.Context, user *openfga.User, mt names.ModelTag, ut names.UserTag, access jujuparams.UserAccessPermission) error
GrantOfferAccess(ctx context.Context, u *openfga.User, offerURL string, ut names.UserTag, access jujuparams.OfferAccessPermission) error
GrantServiceAccountAccess(ctx context.Context, u *openfga.User, svcAccTag jimmnames.ServiceAccountTag, tags []*ofganames.Tag) error
IdentityModelDefaults(ctx context.Context, user *dbmodel.Identity) (map[string]interface{}, error)
ImportModel(ctx context.Context, user *openfga.User, controllerName string, modelTag names.ModelTag, newOwner string) error
InitiateInternalMigration(ctx context.Context, user *openfga.User, modelTag names.ModelTag, targetController string) (jujuparams.InitiateMigrationResult, error)
Expand Down
30 changes: 30 additions & 0 deletions internal/jujuapi/service_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/canonical/jimm/internal/errors"
"github.com/canonical/jimm/internal/jimm"
"github.com/canonical/jimm/internal/openfga"
ofganames "github.com/canonical/jimm/internal/openfga/names"
jimmnames "github.com/canonical/jimm/pkg/names"
)

Expand Down Expand Up @@ -103,3 +104,32 @@ func (r *controllerRoot) ListServiceAccountCredentials(ctx context.Context, req
}
return getIdentityCredentials(ctx, targetIdentity, r.jimm, req.CloudCredentialArgs)
}

// GrantServiceAccountAccess is the method handler for granting new users/groups with access
// to service accounts.
func (r *controllerRoot) GrantServiceAccountAccess(ctx context.Context, req apiparams.GrantServiceAccountAccess) error {
const op = errors.Op("jujuapi.GrantServiceAccountAccess")

targetUser, err := r.getServiceAccount(ctx, req.ClientID)
if err != nil {
return errors.E(op, err)
}
tags := make([]*ofganames.Tag, 0, len(req.Entities))
// Validate tags
for _, val := range req.Entities {
tag, err := parseTag(ctx, r.jimm.ResourceTag().Id(), r.jimm.DB(), val)
if err != nil {
return errors.E(op, err)
}
if tag.Kind != openfga.UserType && tag.Kind != openfga.GroupType {
return errors.E(op, "invalid entity - not user or group")
}
if tag.Kind == openfga.GroupType {
tag.Relation = ofganames.MemberRelation
}
tags = append(tags, tag)
}
svcAccTag := jimmnames.NewServiceAccountTag(req.ClientID)

return r.jimm.GrantServiceAccountAccess(ctx, targetUser, svcAccTag, tags)
}
107 changes: 107 additions & 0 deletions internal/jujuapi/service_account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,113 @@ func TestListServiceAccountCredentials(t *testing.T) {
}
}

func TestGrantServiceAccountAccess(t *testing.T) {
c := qt.New(t)

tests := []struct {
about string
grantServiceAccountAccess func(ctx context.Context, user *openfga.User, svcAccTag jimmnames.ServiceAccountTag, tags []*ofganames.Tag) error
params params.GrantServiceAccountAccess
tags []string
username string
addTuples []openfga.Tuple
expectedError string
}{{
about: "Valid request",
grantServiceAccountAccess: func(ctx context.Context, user *openfga.User, svcAccTag jimmnames.ServiceAccountTag, tags []*ofganames.Tag) error {
return nil
},
params: params.GrantServiceAccountAccess{
Entities: []string{
"user-alice",
"user-bob",
},
ClientID: "fca1f605-736e-4d1f-bcd2-aecc726923be",
},
username: "alice",
addTuples: []openfga.Tuple{{
Object: ofganames.ConvertTag(names.NewUserTag("alice")),
Relation: ofganames.AdministratorRelation,
Target: ofganames.ConvertTag(jimmnames.NewServiceAccountTag("fca1f605-736e-4d1f-bcd2-aecc726923be")),
}},
}, {
about: "Group that doesn't exist",
grantServiceAccountAccess: func(ctx context.Context, user *openfga.User, svcAccTag jimmnames.ServiceAccountTag, tags []*ofganames.Tag) error {
return nil
},
params: params.GrantServiceAccountAccess{
Entities: []string{
"user-alice",
"user-bob",
// This group doesn't exist.
"group-bar",
},
ClientID: "fca1f605-736e-4d1f-bcd2-aecc726923be",
},
username: "alice",
addTuples: []openfga.Tuple{{
Object: ofganames.ConvertTag(names.NewUserTag("alice")),
Relation: ofganames.AdministratorRelation,
Target: ofganames.ConvertTag(jimmnames.NewServiceAccountTag("fca1f605-736e-4d1f-bcd2-aecc726923be")),
}},
expectedError: "group bar not found",
}, {
about: "Invalid tags",
grantServiceAccountAccess: func(ctx context.Context, user *openfga.User, svcAccTag jimmnames.ServiceAccountTag, tags []*ofganames.Tag) error {
return nil
},
params: params.GrantServiceAccountAccess{
Entities: []string{
"user-alice",
"user-bob",
"controller-jimm",
},
ClientID: "fca1f605-736e-4d1f-bcd2-aecc726923be",
},
username: "alice",
addTuples: []openfga.Tuple{{
Object: ofganames.ConvertTag(names.NewUserTag("alice")),
Relation: ofganames.AdministratorRelation,
Target: ofganames.ConvertTag(jimmnames.NewServiceAccountTag("fca1f605-736e-4d1f-bcd2-aecc726923be")),
}},
expectedError: "invalid entity - not user or group",
}}

for _, test := range tests {
test := test
c.Run(test.about, func(c *qt.C) {
ofgaClient, _, _, err := jimmtest.SetupTestOFGAClient(c.Name())
c.Assert(err, qt.IsNil)
pgDb := db.Database{
DB: jimmtest.PostgresDB(c, nil),
}
err = pgDb.Migrate(context.Background(), false)
c.Assert(err, qt.IsNil)
jimm := &jimmtest.JIMM{
AuthorizationClient_: func() *openfga.OFGAClient { return ofgaClient },
GrantServiceAccountAccess_: test.grantServiceAccountAccess,
DB_: func() *db.Database { return &pgDb },
}
var u dbmodel.Identity
u.SetTag(names.NewUserTag(test.username))
user := openfga.NewUser(&u, ofgaClient)
cr := jujuapi.NewControllerRoot(jimm, jujuapi.Params{})
jujuapi.SetUser(cr, user)

if len(test.addTuples) > 0 {
ofgaClient.AddRelation(context.Background(), test.addTuples...)
}

err = cr.GrantServiceAccountAccess(context.Background(), test.params)
if test.expectedError == "" {
c.Assert(err, qt.IsNil)
} else {
c.Assert(err, qt.ErrorMatches, test.expectedError)
}
})
}
}

// Integration tests below.
type serviceAccountSuite struct {
websocketSuite
Expand Down
2 changes: 1 addition & 1 deletion internal/openfga/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (u *User) IsModelWriter(ctx context.Context, resource names.ModelTag) (bool
return isWriter, nil
}

// IsServiceAccountAdmin returns true if user has administrator relation to the service account.
// IsServiceAccountAdmin returns true if the user has administrator relation to the service account.
func (u *User) IsServiceAccountAdmin(ctx context.Context, clientID jimmnames.ServiceAccountTag) (bool, error) {
isAdmin, err := checkRelation(ctx, u, clientID, ofganames.AdministratorRelation)
if err != nil {
Expand Down

0 comments on commit f70ba71

Please sign in to comment.