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

CSS-6766 implement grant access to service account #1138

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
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 {
kian99 marked this conversation as resolved.
Show resolved Hide resolved
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 {
kian99 marked this conversation as resolved.
Show resolved Hide resolved
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) {
kian99 marked this conversation as resolved.
Show resolved Hide resolved
isAdmin, err := checkRelation(ctx, u, clientID, ofganames.AdministratorRelation)
if err != nil {
Expand Down