From 9cc7983836b1f81580ccd729584eff4e4eb2742a Mon Sep 17 00:00:00 2001 From: Kian Parvin <46668016+kian99@users.noreply.github.com> Date: Mon, 2 Sep 2024 15:24:08 +0200 Subject: [PATCH] Tuple queries with UUID (#1335) * tweak how tags are resolved - allow resolving tags without hitting the db if a UUID is specified * simplify matcher and tests * fix tests - Additionally cleaned up duplicated logic in ToJAASTag() * improve error message --- cmd/jimmctl/cmd/relation_test.go | 6 +- internal/errors/errors.go | 2 +- internal/jimm/access.go | 208 ++++++++++------------ internal/jimm/access_test.go | 223 +++++++++--------------- internal/jujuapi/access_control.go | 7 +- internal/jujuapi/access_control_test.go | 63 ++++--- 6 files changed, 212 insertions(+), 297 deletions(-) diff --git a/cmd/jimmctl/cmd/relation_test.go b/cmd/jimmctl/cmd/relation_test.go index 1c9b7688b..a1e4d32fb 100644 --- a/cmd/jimmctl/cmd/relation_test.go +++ b/cmd/jimmctl/cmd/relation_test.go @@ -360,11 +360,11 @@ func (s *relationSuite) TestListRelations(c *gc.C) { }, { Object: "group-group-1#member", Relation: "administrator", - TargetObject: "model-" + env.controllers[0].Name + ":" + env.models[0].OwnerIdentityName + "/" + env.models[0].Name, + TargetObject: "model-" + env.models[0].OwnerIdentityName + "/" + env.models[0].Name, }, { Object: "user-" + env.users[1].Name, Relation: "administrator", - TargetObject: "applicationoffer-" + env.controllers[0].Name + ":" + env.applicationOffers[0].Model.OwnerIdentityName + "/" + env.applicationOffers[0].Model.Name + "." + env.applicationOffers[0].Name, + TargetObject: "applicationoffer-" + env.applicationOffers[0].URL, }, { Object: "user-" + env.users[0].Name, Relation: "administrator", @@ -573,7 +573,7 @@ func (s *relationSuite) TestCheckRelationViaSuperuser(c *gc.C) { // Test reader is OK userToCheck := "user-" + u.Name - modelToCheck := "model-" + controller.Name + ":" + u.Name + "/" + model.Name + modelToCheck := "model-" + u.Name + "/" + model.Name cmdCtx, err := cmdtesting.RunCommand( c, cmd.NewCheckRelationCommandForTesting(s.ClientStore(), bClient), diff --git a/internal/errors/errors.go b/internal/errors/errors.go index 08fbc7695..26f4d57bc 100644 --- a/internal/errors/errors.go +++ b/internal/errors/errors.go @@ -122,7 +122,7 @@ const ( CodeStillAlive Code = apiparams.CodeStillAlive CodeUnauthorized Code = jujuparams.CodeUnauthorized CodeUpgradeInProgress Code = jujuparams.CodeUpgradeInProgress - CodeFailedToParseTupleKey Code = "failed to parse tuple object key" + CodeFailedToParseTupleKey Code = "failed to parse tuple" CodeFailedToResolveTupleResource Code = "failed resolve resource" CodeOpenFGARequestFailed Code = "failed request to OpenFGA" CodeJWKSRetrievalFailed Code = "jwks retrieval failure" diff --git a/internal/jimm/access.go b/internal/jimm/access.go index 626e4e820..fb2978350 100644 --- a/internal/jimm/access.go +++ b/internal/jimm/access.go @@ -12,10 +12,8 @@ import ( "github.com/canonical/ofga" "github.com/google/uuid" - "github.com/juju/juju/core/crossmodel" jujuparams "github.com/juju/juju/rpc/params" "github.com/juju/names/v5" - "github.com/juju/zaputil" "github.com/juju/zaputil/zapctx" "go.uber.org/zap" @@ -34,28 +32,23 @@ const ( var ( // Matches juju uris, jimm user/group tags and UUIDs - // Performs a single match and breaks the juju URI into 10 groups, each successive group is XORD to ensure we can run - // this just once. - // The groups are as so: + // Performs a single match and breaks the juju URI into 4 groups. + // The groups are: // [0] - Entire match // [1] - tag - // [2] - A single "-", ignored - // [3] - Controller name OR user name OR group name - // [4] - A single ":", ignored - // [5] - Controller user / model owner - // [6] - A single "/", ignored - // [7] - Model name - // [8] - A single ".", ignored - // [9] - Application offer name - // [10] - Relation specifier (i.e., #member) + // [2] - trailer (i.e. resource identifier) + // [3] - Relation specifier (i.e., #member) // A complete matcher example would look like so with square-brackets denoting groups and paranthsis denoting index: - // (1)[controller](2)[-](3)[controller-1](4)[:](5)[alice@canonical.com-place](6)[/](7)[model-1](8)[.](9)[offer-1](10)[#relation-specifier]" - // In the case of something like: user-alice@wonderland or group-alices-wonderland#member, it would look like so: - // (1)[user](2)[-](3)[alices@wonderland] - // (1)[group](2)[-](3)[alices-wonderland](10)[#member] - // So if a group, user, UUID, controller name comes in, it will always be index 3 for them - // and if a relation specifier is present, it will always be index 10 - jujuURIMatcher = regexp.MustCompile(`([a-zA-Z0-9]*)(\-|\z)([a-zA-Z0-9-@.]*)(\:|)([a-zA-Z0-9-@.]*)(\/|)([a-zA-Z0-9-]*)(\.|)([a-zA-Z0-9-]*)([a-zA-Z#]*|\z)\z`) + // (1)[controller][-](2)[myFavoriteController][#](3)[relation-specifier]" + // An example without a relation: `user-alice@wonderland`: + // (1)[user][-](2)[alice@wonderland] + // An example with a relaton `group-alices-wonderland#member`: + // (1)[group][-](2)[alices-wonderland][#](3)[member] + jujuURIMatcher = regexp.MustCompile(`([a-zA-Z0-9]*)(?:-)([^#]+)(?:#([a-zA-Z]+)|\z)`) + + // modelOwnerAndNameMatcher matches a string based on the + // the expected form / + modelOwnerAndNameMatcher = regexp.MustCompile(`(.+)/(.+)`) ) // ToOfferAccessString maps relation to an application offer access string. @@ -400,9 +393,17 @@ func (j *JIMM) ToJAASTag(ctx context.Context, tag *ofganames.Tag, resolveUUIDs b return res, nil } + tagToString := func(kind, id string) string { + res := kind + "-" + id + if tag.Relation.String() != "" { + res += "#" + tag.Relation.String() + } + return res + } + switch tag.Kind { case names.UserTagKind: - return names.UserTagKind + "-" + tag.ID, nil + return tagToString(names.UserTagKind, tag.ID), nil case jimmnames.ServiceAccountTagKind: return jimmnames.ServiceAccountTagKind + "-" + tag.ID, nil case names.ControllerTagKind: @@ -416,11 +417,7 @@ func (j *JIMM) ToJAASTag(ctx context.Context, tag *ofganames.Tag, resolveUUIDs b if err != nil { return "", errors.E(err, fmt.Sprintf("failed to fetch controller information: %s", controller.UUID)) } - controllerString := names.ControllerTagKind + "-" + controller.Name - if tag.Relation.String() != "" { - controllerString = controllerString + "#" + tag.Relation.String() - } - return controllerString, nil + return tagToString(names.ControllerTagKind, controller.Name), nil case names.ModelTagKind: model := dbmodel.Model{ UUID: sql.NullString{ @@ -432,11 +429,8 @@ func (j *JIMM) ToJAASTag(ctx context.Context, tag *ofganames.Tag, resolveUUIDs b if err != nil { return "", errors.E(err, fmt.Sprintf("failed to fetch model information: %s", model.UUID.String)) } - modelString := names.ModelTagKind + "-" + model.Controller.Name + ":" + model.OwnerIdentityName + "/" + model.Name - if tag.Relation.String() != "" { - modelString = modelString + "#" + tag.Relation.String() - } - return modelString, nil + modelUserID := model.OwnerIdentityName + "/" + model.Name + return tagToString(names.ModelTagKind, modelUserID), nil case names.ApplicationOfferTagKind: ao := dbmodel.ApplicationOffer{ UUID: tag.ID, @@ -445,11 +439,7 @@ func (j *JIMM) ToJAASTag(ctx context.Context, tag *ofganames.Tag, resolveUUIDs b if err != nil { return "", errors.E(err, fmt.Sprintf("failed to fetch application offer information: %s", ao.UUID)) } - aoString := names.ApplicationOfferTagKind + "-" + ao.Model.Controller.Name + ":" + ao.Model.OwnerIdentityName + "/" + ao.Model.Name + "." + ao.Name - if tag.Relation.String() != "" { - aoString = aoString + "#" + tag.Relation.String() - } - return aoString, nil + return tagToString(names.ApplicationOfferTagKind, ao.URL), nil case jimmnames.GroupTagKind: group := dbmodel.GroupEntry{ UUID: tag.ID, @@ -458,11 +448,7 @@ func (j *JIMM) ToJAASTag(ctx context.Context, tag *ofganames.Tag, resolveUUIDs b if err != nil { return "", errors.E(err, fmt.Sprintf("failed to fetch group information: %s", group.UUID)) } - groupString := jimmnames.GroupTagKind + "-" + group.Name - if tag.Relation.String() != "" { - groupString = groupString + "#" + tag.Relation.String() - } - return groupString, nil + return tagToString(jimmnames.GroupTagKind, group.Name), nil case names.CloudTagKind: cloud := dbmodel.Cloud{ Name: tag.ID, @@ -471,59 +457,43 @@ func (j *JIMM) ToJAASTag(ctx context.Context, tag *ofganames.Tag, resolveUUIDs b if err != nil { return "", errors.E(err, fmt.Sprintf("failed to fetch cloud information: %s", cloud.Name)) } - cloudString := names.CloudTagKind + "-" + cloud.Name - if tag.Relation.String() != "" { - cloudString = cloudString + "#" + tag.Relation.String() - } - return cloudString, nil + return tagToString(names.CloudTagKind, cloud.Name), nil default: return "", errors.E(fmt.Sprintf("unexpected tag kind: %v", tag.Kind)) } } type tagResolver struct { - resourceUUID string - trailer string - controllerName string - userName string - modelName string - offerName string - relation ofga.Relation + resourceUUID string + trailer string + relation ofga.Relation } func newTagResolver(tag string) (*tagResolver, string, error) { matches := jujuURIMatcher.FindStringSubmatch(tag) + if len(matches) != 4 { + return nil, "", errors.E("tag is not properly formatted", errors.CodeBadRequest) + } tagKind := matches[1] resourceUUID := "" trailer := "" - // We first attempt to see if group3 is a uuid - if _, err := uuid.Parse(matches[3]); err == nil { + // We first attempt to see if group2 is a uuid + if _, err := uuid.Parse(matches[2]); err == nil { // We know it's a UUID - resourceUUID = matches[3] + resourceUUID = matches[2] } else { - // We presume it's a user or a group - trailer = matches[3] - } - - // Matchers along the way to determine segments of the string, they'll be empty - // if the match has failed - controllerName := matches[3] - userName := matches[5] - modelName := matches[7] - offerName := matches[9] - relationString := strings.TrimLeft(matches[10], "#") - relation, err := ofganames.ParseRelation(relationString) + // We presume the information the matcher needs is in the trailer + trailer = matches[2] + } + + relation, err := ofganames.ParseRelation(matches[3]) if err != nil { return nil, "", errors.E("failed to parse relation", errors.CodeBadRequest) } return &tagResolver{ - resourceUUID: resourceUUID, - trailer: trailer, - controllerName: controllerName, - userName: userName, - modelName: modelName, - offerName: offerName, - relation: relation, + resourceUUID: resourceUUID, + trailer: trailer, + relation: relation, }, tagKind, nil } @@ -548,12 +518,10 @@ func (t *tagResolver) groupTag(ctx context.Context, db *db.Database) (*ofga.Enti "Resolving JIMM tags to Juju tags for tag kind: group", zap.String("group-name", t.trailer), ) - var entry dbmodel.GroupEntry if t.resourceUUID != "" { - entry.UUID = t.resourceUUID - } else if t.trailer != "" { - entry.Name = t.trailer + return ofganames.ConvertTagWithRelation(jimmnames.NewGroupTag(t.resourceUUID), t.relation), nil } + entry := dbmodel.GroupEntry{Name: t.trailer} err := db.GetGroup(ctx, &entry) if err != nil { @@ -568,20 +536,14 @@ func (t *tagResolver) controllerTag(ctx context.Context, jimmUUID string, db *db ctx, "Resolving JIMM tags to Juju tags for tag kind: controller", ) - controller := dbmodel.Controller{} if t.resourceUUID != "" { - controller.UUID = t.resourceUUID - } else if t.controllerName != "" { - if t.controllerName == jimmControllerName { - return ofganames.ConvertTagWithRelation(names.NewControllerTag(jimmUUID), t.relation), nil - } - controller.Name = t.controllerName + return ofganames.ConvertTagWithRelation(names.NewControllerTag(t.resourceUUID), t.relation), nil } - - // NOTE (alesstimec) Do we need to special-case the - // controller-jimm case - jimm controller does not exist - // in the database, but has a clearly defined UUID? + if t.trailer == jimmControllerName { + return ofganames.ConvertTagWithRelation(names.NewControllerTag(jimmUUID), t.relation), nil + } + controller := dbmodel.Controller{Name: t.trailer} err := db.GetController(ctx, &controller) if err != nil { @@ -595,27 +557,25 @@ func (t *tagResolver) modelTag(ctx context.Context, db *db.Database) (*ofga.Enti ctx, "Resolving JIMM tags to Juju tags for tag kind: model", ) - model := dbmodel.Model{} if t.resourceUUID != "" { - model.UUID = sql.NullString{String: t.resourceUUID, Valid: true} - } else if t.controllerName != "" && t.userName != "" && t.modelName != "" { - controller := dbmodel.Controller{Name: t.controllerName} - err := db.GetController(ctx, &controller) - if err != nil { - return nil, errors.E("controller not found") - } - model.ControllerID = controller.ID - model.OwnerIdentityName = t.userName - model.Name = t.modelName + return ofganames.ConvertTagWithRelation(names.NewModelTag(t.resourceUUID), t.relation), nil + } + + model := dbmodel.Model{} + matches := modelOwnerAndNameMatcher.FindStringSubmatch(t.trailer) + if len(matches) != 3 { + return nil, errors.E("model name format incorrect, expected /") } + model.OwnerIdentityName = matches[1] + model.Name = matches[2] err := db.GetModel(ctx, &model) if err != nil { return nil, errors.E("model not found") } - return ofganames.ConvertTagWithRelation(names.NewModelTag(model.UUID.String), t.relation), nil + return ofganames.ConvertTagWithRelation(model.ResourceTag(), t.relation), nil } func (t *tagResolver) applicationOfferTag(ctx context.Context, db *db.Database) (*ofga.Entity, error) { @@ -623,23 +583,11 @@ func (t *tagResolver) applicationOfferTag(ctx context.Context, db *db.Database) ctx, "Resolving JIMM tags to Juju tags for tag kind: applicationoffer", ) - offer := dbmodel.ApplicationOffer{} if t.resourceUUID != "" { - offer.UUID = t.resourceUUID - } else if t.controllerName != "" && t.userName != "" && t.modelName != "" && t.offerName != "" { - offerURL, err := crossmodel.ParseOfferURL(fmt.Sprintf("%s:%s/%s.%s", t.controllerName, t.userName, t.modelName, t.offerName)) - if err != nil { - zapctx.Debug( - ctx, - "failed to parse application offer url", - zap.String("url", fmt.Sprintf("%s:%s/%s.%s", t.controllerName, t.userName, t.modelName, t.offerName)), - zaputil.Error(err), - ) - return nil, errors.E("failed to parse offer url", err) - } - offer.URL = offerURL.String() + return ofganames.ConvertTagWithRelation(names.NewApplicationOfferTag(t.resourceUUID), t.relation), nil } + offer := dbmodel.ApplicationOffer{URL: t.trailer} err := db.GetApplicationOffer(ctx, &offer) if err != nil { @@ -648,6 +596,26 @@ func (t *tagResolver) applicationOfferTag(ctx context.Context, db *db.Database) return ofganames.ConvertTagWithRelation(offer.ResourceTag(), t.relation), nil } + +func (t *tagResolver) cloudTag(ctx context.Context, db *db.Database) (*ofga.Entity, error) { + zapctx.Debug( + ctx, + "Resolving JIMM tags to Juju tags for tag kind: cloud", + ) + + if t.resourceUUID != "" { + return ofganames.ConvertTagWithRelation(names.NewCloudTag(t.resourceUUID), t.relation), nil + } + cloud := dbmodel.Cloud{Name: t.trailer} + + err := db.GetCloud(ctx, &cloud) + if err != nil { + return nil, errors.E("application offer not found") + } + + return ofganames.ConvertTagWithRelation(cloud.ResourceTag(), t.relation), nil +} + func (t *tagResolver) serviceAccountTag(ctx context.Context) (*ofga.Entity, error) { zapctx.Debug( ctx, @@ -672,7 +640,7 @@ func resolveTag(jimmUUID string, db *db.Database, tag string) (*ofganames.Tag, e ctx := context.Background() resolver, tagKind, err := newTagResolver(tag) if err != nil { - return nil, errors.E("failed to setup tag resolver", err) + return nil, errors.E(fmt.Errorf("failed to setup tag resolver: %w", err)) } switch tagKind { @@ -686,10 +654,12 @@ func resolveTag(jimmUUID string, db *db.Database, tag string) (*ofganames.Tag, e return resolver.modelTag(ctx, db) case names.ApplicationOfferTagKind: return resolver.applicationOfferTag(ctx, db) + case names.CloudTagKind: + return resolver.cloudTag(ctx, db) case jimmnames.ServiceAccountTagKind: return resolver.serviceAccountTag(ctx) } - return nil, errors.E("failed to map tag " + tagKind) + return nil, errors.E(errors.CodeBadRequest, fmt.Sprintf("failed to map tag, unknown kind: %s", tagKind)) } // ParseTag attempts to parse the provided key into a tag whilst additionally diff --git a/internal/jimm/access_test.go b/internal/jimm/access_test.go index 41d26bdf1..58292071e 100644 --- a/internal/jimm/access_test.go +++ b/internal/jimm/access_test.go @@ -5,6 +5,7 @@ package jimm_test import ( "context" "database/sql" + "fmt" "sort" "testing" "time" @@ -446,9 +447,9 @@ func TestParseTag(t *testing.T) { err = j.Database.Migrate(ctx, false) c.Assert(err, qt.IsNil) - user, _, controller, model, _, _, _ := createTestControllerEnvironment(ctx, c, j.Database) + user, _, _, model, _, _, _ := createTestControllerEnvironment(ctx, c, j.Database) - jimmTag := "model-" + controller.Name + ":" + user.Name + "/" + model.Name + "#administrator" + jimmTag := "model-" + user.Name + "/" + model.Name + "#administrator" // JIMM tag syntax for models tag, err := j.ParseTag(ctx, jimmTag) @@ -467,7 +468,7 @@ func TestParseTag(t *testing.T) { c.Assert(tag.Relation.String(), qt.Equals, "administrator") } -func TestResolveJIMM(t *testing.T) { +func TestResolveTags(t *testing.T) { c := qt.New(t) ctx := context.Background() @@ -482,145 +483,69 @@ func TestResolveJIMM(t *testing.T) { err := j.Database.Migrate(ctx, false) c.Assert(err, qt.IsNil) - jimmTag := "controller-jimm" + identity, group, controller, model, offer, cloud, _ := createTestControllerEnvironment(ctx, c, j.Database) - jujuTag, err := jimm.ResolveTag(j.UUID, &j.Database, jimmTag) - c.Assert(err, qt.IsNil) - c.Assert(jujuTag, qt.DeepEquals, ofganames.ConvertTag(names.NewControllerTag(j.UUID))) -} - -func TestResolveTupleObjectMapsApplicationOffersUUIDs(t *testing.T) { - c := qt.New(t) - ctx := context.Background() - - now := time.Now().UTC().Round(time.Millisecond) - j := &jimm.JIMM{ - UUID: uuid.NewString(), - Database: db.Database{ - DB: jimmtest.PostgresDB(c, func() time.Time { return now }), - }, - } - - err := j.Database.Migrate(ctx, false) - c.Assert(err, qt.IsNil) - - user, _, controller, model, offer, _, _ := createTestControllerEnvironment(ctx, c, j.Database) - - jimmTag := "applicationoffer-" + controller.Name + ":" + user.Name + "/" + model.Name + "." + offer.Name + "#administrator" - - jujuTag, err := jimm.ResolveTag(j.UUID, &j.Database, jimmTag) - c.Assert(err, qt.IsNil) - c.Assert(jujuTag, qt.DeepEquals, ofganames.ConvertTagWithRelation(names.NewApplicationOfferTag(offer.UUID), ofganames.AdministratorRelation)) -} - -func TestResolveTupleObjectMapsModelUUIDs(t *testing.T) { - c := qt.New(t) - ctx := context.Background() - - now := time.Now().UTC().Round(time.Millisecond) - j := &jimm.JIMM{ - UUID: uuid.NewString(), - Database: db.Database{ - DB: jimmtest.PostgresDB(c, func() time.Time { return now }), - }, - } - - err := j.Database.Migrate(ctx, false) - c.Assert(err, qt.IsNil) - - user, _, controller, model, _, _, _ := createTestControllerEnvironment(ctx, c, j.Database) - - jimmTag := "model-" + controller.Name + ":" + user.Name + "/" + model.Name + "#administrator" - - tag, err := jimm.ResolveTag(j.UUID, &j.Database, jimmTag) - c.Assert(err, qt.IsNil) - c.Assert(tag, qt.DeepEquals, ofganames.ConvertTagWithRelation(names.NewModelTag(model.UUID.String), ofganames.AdministratorRelation)) -} - -func TestResolveTupleObjectMapsControllerUUIDs(t *testing.T) { - c := qt.New(t) - ctx := context.Background() - - now := time.Now().UTC().Round(time.Millisecond) - j := &jimm.JIMM{ - UUID: uuid.NewString(), - Database: db.Database{ - DB: jimmtest.PostgresDB(c, func() time.Time { return now }), - }, - } - - err := j.Database.Migrate(ctx, false) - c.Assert(err, qt.IsNil) - - cloud := dbmodel.Cloud{ - Name: "test-cloud", - } - err = j.Database.AddCloud(context.Background(), &cloud) - c.Assert(err, qt.IsNil) - - uuid, _ := uuid.NewRandom() - controller := dbmodel.Controller{ - Name: "mycontroller", - UUID: uuid.String(), - CloudName: "test-cloud", - } - err = j.Database.AddController(ctx, &controller) - c.Assert(err, qt.IsNil) - - tag, err := jimm.ResolveTag(j.UUID, &j.Database, "controller-mycontroller#administrator") - c.Assert(err, qt.IsNil) - c.Assert(tag, qt.DeepEquals, ofganames.ConvertTagWithRelation(names.NewControllerTag(uuid.String()), ofganames.AdministratorRelation)) -} - -func TestResolveTupleObjectMapsGroups(t *testing.T) { - c := qt.New(t) - ctx := context.Background() - - now := time.Now().UTC().Round(time.Millisecond) - j := &jimm.JIMM{ - UUID: uuid.NewString(), - Database: db.Database{ - DB: jimmtest.PostgresDB(c, func() time.Time { return now }), - }, - } - - err := j.Database.Migrate(ctx, false) - c.Assert(err, qt.IsNil) - - _, err = j.Database.AddGroup(ctx, "myhandsomegroupofdigletts") - c.Assert(err, qt.IsNil) - group := &dbmodel.GroupEntry{ - Name: "myhandsomegroupofdigletts", - } - err = j.Database.GetGroup(ctx, group) - c.Assert(err, qt.IsNil) - // Test resolution via name and via UUID. - tag, err := jimm.ResolveTag(j.UUID, &j.Database, "group-"+group.Name+"#member") - c.Assert(err, qt.IsNil) - c.Assert(tag, qt.DeepEquals, ofganames.ConvertTagWithRelation(jimmnames.NewGroupTag(group.UUID), ofganames.MemberRelation)) - tag, err = jimm.ResolveTag(j.UUID, &j.Database, "group-"+group.UUID+"#member") - c.Assert(err, qt.IsNil) - c.Assert(tag, qt.DeepEquals, ofganames.ConvertTagWithRelation(jimmnames.NewGroupTag(group.UUID), ofganames.MemberRelation)) -} - -func TestResolveTagObjectMapsUsers(t *testing.T) { - c := qt.New(t) - ctx := context.Background() + testCases := []struct { + desc string + input string + expected *ofga.Entity + }{{ + desc: "map identity name with relation", + input: "user-" + identity.Name + "#member", + expected: ofganames.ConvertTagWithRelation(names.NewUserTag(identity.Name), ofganames.MemberRelation), + }, { + desc: "map group name with relation", + input: "group-" + group.Name + "#member", + expected: ofganames.ConvertTagWithRelation(jimmnames.NewGroupTag(group.UUID), ofganames.MemberRelation), + }, { + desc: "map group UUID", + input: "group-" + group.UUID, + expected: ofganames.ConvertTag(jimmnames.NewGroupTag(group.UUID)), + }, { + desc: "map group UUID with relation", + input: "group-" + group.UUID + "#member", + expected: ofganames.ConvertTagWithRelation(jimmnames.NewGroupTag(group.UUID), ofganames.MemberRelation), + }, { + desc: "map jimm controller", + input: "controller-" + "jimm", + expected: ofganames.ConvertTag(names.NewControllerTag(j.UUID)), + }, { + desc: "map controller", + input: "controller-" + controller.Name + "#administrator", + expected: ofganames.ConvertTagWithRelation(names.NewControllerTag(model.UUID.String), ofganames.AdministratorRelation), + }, { + desc: "map controller UUID", + input: "controller-" + controller.UUID, + expected: ofganames.ConvertTag(names.NewControllerTag(model.UUID.String)), + }, { + desc: "map model", + input: "model-" + model.OwnerIdentityName + "/" + model.Name + "#administrator", + expected: ofganames.ConvertTagWithRelation(names.NewModelTag(model.UUID.String), ofganames.AdministratorRelation), + }, { + desc: "map model UUID", + input: "model-" + model.UUID.String, + expected: ofganames.ConvertTag(names.NewModelTag(model.UUID.String)), + }, { + desc: "map offer", + input: "applicationoffer-" + offer.URL + "#administrator", + expected: ofganames.ConvertTagWithRelation(names.NewApplicationOfferTag(offer.UUID), ofganames.AdministratorRelation), + }, { + desc: "map offer UUID", + input: "applicationoffer-" + offer.UUID, + expected: ofganames.ConvertTag(names.NewApplicationOfferTag(offer.UUID)), + }, { + desc: "map cloud", + input: "cloud-" + cloud.Name + "#administrator", + expected: ofganames.ConvertTagWithRelation(names.NewCloudTag(cloud.Name), ofganames.AdministratorRelation), + }} - now := time.Now().UTC().Round(time.Millisecond) - j := &jimm.JIMM{ - UUID: uuid.NewString(), - Database: db.Database{ - DB: jimmtest.PostgresDB(c, func() time.Time { return now }), - }, + for _, tC := range testCases { + t.Run(tC.desc, func(t *testing.T) { + jujuTag, err := jimm.ResolveTag(j.UUID, &j.Database, tC.input) + c.Assert(err, qt.IsNil) + c.Assert(jujuTag, qt.DeepEquals, tC.expected) + }) } - - err := j.Database.Migrate(ctx, false) - c.Assert(err, qt.IsNil) - - tag, err := jimm.ResolveTag(j.UUID, &j.Database, "user-alex@canonical.com-werly#member") - c.Assert(err, qt.IsNil) - c.Assert(tag, qt.DeepEquals, ofganames.ConvertTagWithRelation(names.NewUserTag("alex@canonical.com-werly"), ofganames.MemberRelation)) } func TestResolveTupleObjectHandlesErrors(t *testing.T) { @@ -649,7 +574,7 @@ func TestResolveTupleObjectHandlesErrors(t *testing.T) { // Resolves bad tuple objects in general { input: "unknowntag-blabla", - want: "failed to map tag unknowntag", + want: "failed to map tag, unknown kind: unknowntag", }, // Resolves bad groups where they do not exist { @@ -669,17 +594,27 @@ func TestResolveTupleObjectHandlesErrors(t *testing.T) { // Resolves bad models where it cannot be found on the specified controller { input: "model-" + controller.Name + ":alex/", - want: "model not found", + want: "model name format incorrect, expected /", }, // Resolves bad applicationoffers where it cannot be found on the specified controller/model combo { input: "applicationoffer-" + controller.Name + ":alex/" + model.Name + "." + offer.Name + "fluff", want: "application offer not found", }, + { + input: "abc", + want: "failed to setup tag resolver: tag is not properly formatted", + }, + { + input: "model-test-unknowncontroller-1:alice@canonical.com/test-model-1", + want: "model not found", + }, } - for _, tc := range tests { - _, err := jimm.ResolveTag(j.UUID, &j.Database, tc.input) - c.Assert(err, qt.ErrorMatches, tc.want) + for i, tc := range tests { + t.Run(fmt.Sprintf("test %d", i), func(t *testing.T) { + _, err := jimm.ResolveTag(j.UUID, &j.Database, tc.input) + c.Assert(err, qt.ErrorMatches, tc.want) + }) } } diff --git a/internal/jujuapi/access_control.go b/internal/jujuapi/access_control.go index 5f8faf2d0..403359cc4 100644 --- a/internal/jujuapi/access_control.go +++ b/internal/jujuapi/access_control.go @@ -4,6 +4,7 @@ package jujuapi import ( "context" + "fmt" "strconv" "time" @@ -198,7 +199,7 @@ func (r *controllerRoot) parseTuple(ctx context.Context, tuple apiparams.Relatio // to be specific to the erroneous offender. parseTagError := func(msg string, key string, err error) error { zapctx.Debug(ctx, msg, zap.String("key", key), zap.Error(err)) - return errors.E(op, errors.CodeFailedToParseTupleKey, err, msg+" "+key) + return errors.E(op, errors.CodeFailedToParseTupleKey, fmt.Errorf("%s, key %s: %w", msg, key, err)) } if tuple.TargetObject == "" { @@ -207,14 +208,14 @@ func (r *controllerRoot) parseTuple(ctx context.Context, tuple apiparams.Relatio if tuple.TargetObject != "" { targetTag, err := r.jimm.ParseTag(ctx, tuple.TargetObject) if err != nil { - return nil, parseTagError("failed to parse tuple target object key", tuple.TargetObject, err) + return nil, parseTagError("failed to parse tuple target", tuple.TargetObject, err) } t.Target = targetTag } if tuple.Object != "" { objectTag, err := r.jimm.ParseTag(ctx, tuple.Object) if err != nil { - return nil, parseTagError("failed to parse tuple object key", tuple.Object, err) + return nil, parseTagError("failed to parse tuple object", tuple.Object, err) } t.Object = objectTag } diff --git a/internal/jujuapi/access_control_test.go b/internal/jujuapi/access_control_test.go index a5284718e..533d89dcf 100644 --- a/internal/jujuapi/access_control_test.go +++ b/internal/jujuapi/access_control_test.go @@ -337,7 +337,7 @@ func (s *accessControlSuite) TestAddRelation(c *gc.C) { }, // Test user -> model by name { - input: tuple{"user-" + user.Name, "writer", "model-" + controller.Name + ":" + user.Name + "/" + model.Name}, + input: tuple{"user-" + user.Name, "writer", "model-" + user.Name + "/" + model.Name}, want: createTuple( "user:"+user.Name, "writer", @@ -359,7 +359,7 @@ func (s *accessControlSuite) TestAddRelation(c *gc.C) { }, // Test user -> applicationoffer by name { - input: tuple{"user-" + user.Name, "consumer", "applicationoffer-" + controller.Name + ":" + user.Name + "/" + model.Name + "." + offer.Name}, + input: tuple{"user-" + user.Name, "consumer", "applicationoffer-" + offer.URL}, want: createTuple( "user:"+user.Name, "consumer", @@ -403,7 +403,7 @@ func (s *accessControlSuite) TestAddRelation(c *gc.C) { }, // Test group -> model by name { - input: tuple{"group-" + group.Name + "#member", "writer", "model-" + controller.Name + ":" + user.Name + "/" + model.Name}, + input: tuple{"group-" + group.Name + "#member", "writer", "model-" + user.Name + "/" + model.Name}, want: createTuple( "group:"+group.UUID+"#member", "writer", @@ -425,7 +425,7 @@ func (s *accessControlSuite) TestAddRelation(c *gc.C) { }, // Test group -> applicationoffer by name { - input: tuple{"group-" + group.Name + "#member", "consumer", "applicationoffer-" + controller.Name + ":" + user.Name + "/" + model.Name + "." + offer.Name}, + input: tuple{"group-" + group.Name + "#member", "consumer", "applicationoffer-" + offer.URL}, want: createTuple( "group:"+group.UUID+"#member", "consumer", @@ -599,7 +599,7 @@ func (s *accessControlSuite) TestRemoveRelation(c *gc.C) { Relation: "writer", Target: ofganames.ConvertTag(model.ResourceTag()), }, - toRemove: tuple{"user-" + user.Name, "writer", "model-" + controller.Name + ":" + user.Name + "/" + model.Name}, + toRemove: tuple{"user-" + user.Name, "writer", "model-" + user.Name + "/" + model.Name}, want: createTuple( "user:"+user.Name, "writer", @@ -631,7 +631,7 @@ func (s *accessControlSuite) TestRemoveRelation(c *gc.C) { Relation: "consumer", Target: ofganames.ConvertTag(offer.ResourceTag()), }, - toRemove: tuple{"user-" + user.Name, "consumer", "applicationoffer-" + controller.Name + ":" + user.Name + "/" + model.Name + "." + offer.Name}, + toRemove: tuple{"user-" + user.Name, "consumer", "applicationoffer-" + offer.URL}, want: createTuple( "user:"+user.Name, "consumer", @@ -695,7 +695,7 @@ func (s *accessControlSuite) TestRemoveRelation(c *gc.C) { Relation: "writer", Target: ofganames.ConvertTag(model.ResourceTag()), }, - toRemove: tuple{"group-" + group.Name + "#member", "writer", "model-" + controller.Name + ":" + user.Name + "/" + model.Name}, + toRemove: tuple{"group-" + group.Name + "#member", "writer", "model-" + user.Name + "/" + model.Name}, want: createTuple( "group:"+group.UUID+"#member", "writer", @@ -727,7 +727,7 @@ func (s *accessControlSuite) TestRemoveRelation(c *gc.C) { Relation: "consumer", Target: ofganames.ConvertTag(offer.ResourceTag()), }, - toRemove: tuple{"group-" + group.Name + "#member", "consumer", "applicationoffer-" + controller.Name + ":" + user.Name + "/" + model.Name + "." + offer.Name}, + toRemove: tuple{"group-" + group.Name + "#member", "consumer", "applicationoffer-" + offer.URL}, want: createTuple( "group:"+group.UUID+"#member", "consumer", @@ -819,10 +819,10 @@ func (s *accessControlSuite) TestJAASTag(c *gc.C) { expectedJAASTag: "controller-" + controller.Name, }, { tag: ofganames.ConvertTag(model.ResourceTag()), - expectedJAASTag: "model-" + controller.Name + ":" + user.Name + "/" + model.Name, + expectedJAASTag: "model-" + user.Name + "/" + model.Name, }, { tag: ofganames.ConvertTag(applicationOffer.ResourceTag()), - expectedJAASTag: "applicationoffer-" + controller.Name + ":" + user.Name + "/" + model.Name + "." + applicationOffer.Name, + expectedJAASTag: "applicationoffer-" + applicationOffer.URL, }, { tag: &ofganames.Tag{}, expectedError: "unexpected tag kind: ", @@ -891,7 +891,7 @@ func (s *accessControlSuite) TestJAASTagNoUUIDResolution(c *gc.C) { func (s *accessControlSuite) TestListRelationshipTuples(c *gc.C) { ctx := context.Background() - user, _, controller, model, applicationOffer, _, _, client, closeClient := createTestControllerEnvironment(ctx, c, s) + user, _, controller, _, applicationOffer, _, _, client, closeClient := createTestControllerEnvironment(ctx, c, s) defer closeClient() _, err := client.AddGroup(&apiparams.AddGroupRequest{Name: "yellow"}) @@ -914,7 +914,7 @@ func (s *accessControlSuite) TestListRelationshipTuples(c *gc.C) { }, { Object: "group-orange#member", Relation: "administrator", - TargetObject: "applicationoffer-" + controller.Name + ":" + user.Name + "/" + model.Name + "." + applicationOffer.Name, + TargetObject: "applicationoffer-" + applicationOffer.URL, }} err = client.AddRelation(&apiparams.AddRelationRequest{Tuples: tuples}) @@ -928,18 +928,27 @@ func (s *accessControlSuite) TestListRelationshipTuples(c *gc.C) { response, err = client.ListRelationshipTuples(&apiparams.ListRelationshipTuplesRequest{ Tuple: apiparams.RelationshipTuple{ - TargetObject: "applicationoffer-" + controller.Name + ":" + user.Name + "/" + model.Name + "." + applicationOffer.Name, + TargetObject: "applicationoffer-" + applicationOffer.URL, }, ResolveUUIDs: true, }) c.Assert(err, jc.ErrorIsNil) c.Assert(response.Tuples, jc.DeepEquals, []apiparams.RelationshipTuple{tuples[3]}) c.Assert(len(response.Errors), gc.Equals, 0) + + // Test error message when a resource is not found + _, err = client.ListRelationshipTuples(&apiparams.ListRelationshipTuplesRequest{ + Tuple: apiparams.RelationshipTuple{ + TargetObject: "applicationoffer-" + "fake-offer", + }, + ResolveUUIDs: true, + }) + c.Assert(err, gc.ErrorMatches, "failed to parse tuple target, key applicationoffer-fake-offer: application offer not found.*") } func (s *accessControlSuite) TestListRelationshipTuplesNoUUIDResolution(c *gc.C) { ctx := context.Background() - user, _, controller, model, applicationOffer, _, _, client, closeClient := createTestControllerEnvironment(ctx, c, s) + _, _, _, _, applicationOffer, _, _, client, closeClient := createTestControllerEnvironment(ctx, c, s) defer closeClient() _, err := client.AddGroup(&apiparams.AddGroupRequest{Name: "orange"}) @@ -964,7 +973,7 @@ func (s *accessControlSuite) TestListRelationshipTuplesNoUUIDResolution(c *gc.C) }} response, err := client.ListRelationshipTuples(&apiparams.ListRelationshipTuplesRequest{ Tuple: apiparams.RelationshipTuple{ - TargetObject: "applicationoffer-" + controller.Name + ":" + user.Name + "/" + model.Name + "." + applicationOffer.Name, + TargetObject: "applicationoffer-" + applicationOffer.URL, }, ResolveUUIDs: false, }) @@ -975,7 +984,7 @@ func (s *accessControlSuite) TestListRelationshipTuplesNoUUIDResolution(c *gc.C) func (s *accessControlSuite) TestListRelationshipTuplesAfterDeletingGroup(c *gc.C) { ctx := context.Background() - user, _, controller, model, applicationOffer, _, _, client, closeClient := createTestControllerEnvironment(ctx, c, s) + user, _, controller, _, applicationOffer, _, _, client, closeClient := createTestControllerEnvironment(ctx, c, s) defer closeClient() _, err := client.AddGroup(&apiparams.AddGroupRequest{Name: "yellow"}) @@ -998,7 +1007,7 @@ func (s *accessControlSuite) TestListRelationshipTuplesAfterDeletingGroup(c *gc. }, { Object: "group-orange#member", Relation: "administrator", - TargetObject: "applicationoffer-" + controller.Name + ":" + user.Name + "/" + model.Name + "." + applicationOffer.Name, + TargetObject: "applicationoffer-" + applicationOffer.URL, }} err = client.AddRelation(&apiparams.AddRelationRequest{Tuples: tuples}) @@ -1092,7 +1101,7 @@ func (s *accessControlSuite) TestCheckRelationOfferReaderFlow(c *gc.C) { ctx := context.Background() ofgaClient := s.JIMM.OpenFGAClient - user, group, controller, model, offer, _, _, client, closeClient := createTestControllerEnvironment(ctx, c, s) + user, group, _, _, offer, _, _, client, closeClient := createTestControllerEnvironment(ctx, c, s) defer closeClient() // Some tags (tuples) to assist in the creation of tuples within OpenFGA (such that they can be tested against) @@ -1102,7 +1111,7 @@ func (s *accessControlSuite) TestCheckRelationOfferReaderFlow(c *gc.C) { // JAAS style keys, to be translated and checked against UUIDs/users/groups userJAASKey := "user-" + user.Name - offerJAASKey := "applicationoffer-" + controller.Name + ":" + user.Name + "/" + model.Name + "." + offer.Name + offerJAASKey := "applicationoffer-" + offer.URL // Test direct relation to an applicationoffer from a user of a group via "reader" relation @@ -1163,7 +1172,7 @@ func (s *accessControlSuite) TestCheckRelationOfferConsumerFlow(c *gc.C) { ctx := context.Background() ofgaClient := s.JIMM.OpenFGAClient - user, group, controller, model, offer, _, _, client, closeClient := createTestControllerEnvironment(ctx, c, s) + user, group, _, _, offer, _, _, client, closeClient := createTestControllerEnvironment(ctx, c, s) defer closeClient() // Some keys to assist in the creation of tuples within OpenFGA (such that they can be tested against) @@ -1173,7 +1182,7 @@ func (s *accessControlSuite) TestCheckRelationOfferConsumerFlow(c *gc.C) { // JAAS style keys, to be translated and checked against UUIDs/users/groups userJAASKey := "user-" + user.Name - offerJAASKey := "applicationoffer-" + controller.Name + ":" + user.Name + "/" + model.Name + "." + offer.Name + offerJAASKey := "applicationoffer-" + offer.URL // Test direct relation to an applicationoffer from a user of a group via "consumer" relation userToGroupMember := openfga.Tuple{ @@ -1232,7 +1241,7 @@ func (s *accessControlSuite) TestCheckRelationModelReaderFlow(c *gc.C) { ctx := context.Background() ofgaClient := s.JIMM.OpenFGAClient - user, group, controller, model, _, _, _, client, closeClient := createTestControllerEnvironment(ctx, c, s) + user, group, _, model, _, _, _, client, closeClient := createTestControllerEnvironment(ctx, c, s) defer closeClient() // Some keys to assist in the creation of tuples within OpenFGA (such that they can be tested against) @@ -1244,7 +1253,7 @@ func (s *accessControlSuite) TestCheckRelationModelReaderFlow(c *gc.C) { // JAAS style keys, to be translated and checked against UUIDs/users/groups userJAASKey := "user-" + user.Name - modelJAASKey := "model-" + controller.Name + ":" + user.Name + "/" + model.Name + modelJAASKey := "model-" + user.Name + "/" + model.Name // Test direct relation to a model from a user of a group via "reader" relation userToGroupMember := openfga.Tuple{ @@ -1303,7 +1312,7 @@ func (s *accessControlSuite) TestCheckRelationModelWriterFlow(c *gc.C) { ctx := context.Background() ofgaClient := s.JIMM.OpenFGAClient - user, group, controller, model, _, _, _, client, closeClient := createTestControllerEnvironment(ctx, c, s) + user, group, _, model, _, _, _, client, closeClient := createTestControllerEnvironment(ctx, c, s) defer closeClient() // Some keys to assist in the creation of tuples within OpenFGA (such that they can be tested against) @@ -1325,7 +1334,7 @@ func (s *accessControlSuite) TestCheckRelationModelWriterFlow(c *gc.C) { // JAAS style keys, to be translated and checked against UUIDs/users/groups userJAASKey := "user-" + user.Name - modelJAASKey := "model-" + controller.Name + ":" + user.Name + "/" + model.Name + modelJAASKey := "model-" + user.Name + "/" + model.Name err := ofgaClient.AddRelation( ctx, @@ -1386,8 +1395,8 @@ func (s *accessControlSuite) TestCheckRelationControllerAdministratorFlow(c *gc. userJAASKey := "user-" + user.Name groupJAASKey := "group-" + group.Name controllerJAASKey := "controller-" + controller.Name - modelJAASKey := "model-" + controller.Name + ":" + user.Name + "/" + model.Name - offerJAASKey := "applicationoffer-" + controller.Name + ":" + user.Name + "/" + model.Name + "." + offer.Name + modelJAASKey := "model-" + user.Name + "/" + model.Name + offerJAASKey := "applicationoffer-" + offer.URL // Test the administrator flow of a group user being related to a controller via administrator relation userToGroup := openfga.Tuple{