CSS-4603 Refactor toward using canonical/ofga

README.md

@@ -18,6 +18,7 @@ dependencies. **Note: Go 1.11 or greater needed.**
 ## Development environment
 
 ### Local:
+
 A couple of system packages are required in order to set up a development
 environment. To install them, run the following:
 `make sysdeps`

cmd/jimmctl/cmd/jimmsuite_test.go

@@ -57,11 +57,11 @@ func (s *jimmSuite) SetUpTest(c *gc.C) {
 	s.CandidSuite.SetUpTest(c)
 	s.JujuConnSuite.SetUpTest(c)
 
-	ofgaAPI, ofgaClient, cfg, err := jimmtest.SetupTestOFGAClient(c.TestName())
+	ofgaClient, cofgaClient, cofgaParams, err := jimmtest.SetupTestOFGAClient(c.TestName())
 	c.Assert(err, gc.Equals, nil)
-	s.OFGAApi = ofgaAPI
 	s.OFGAClient = ofgaClient
-	s.OFGAConfig = cfg
+	s.COFGAClient = cofgaClient
+	s.COFGAParams = cofgaParams
 
 	s.JIMM = &jimm.JIMM{
 		UUID: "914487b5-60e7-42bb-bd63-1adc3fd3a388",
@@ -81,11 +81,12 @@ func (s *jimmSuite) SetUpTest(c *gc.C) {
 		ControllerAdmins: []string{"admin"},
 		DSN:              fmt.Sprintf("file:%s?mode=memory&cache=shared", c.TestName()),
 		OpenFGAParams: service.OpenFGAParams{
-			Scheme:    cfg.ApiScheme,
-			Host:      cfg.ApiHost,
-			Store:     cfg.StoreId,
-			Token:     cfg.Credentials.Config.ApiToken,
-			AuthModel: ofgaClient.AuthModelId,
+			Scheme:    cofgaParams.Scheme,
+			Host:      cofgaParams.Host,
+			Port:      cofgaParams.Port,
+			Store:     cofgaParams.StoreID,
+			Token:     cofgaParams.Token,
+			AuthModel: cofgaParams.AuthModelID,
 		},
 	}
 	srv, err := service.NewService(ctx, s.Params)

cmd/jimmctl/cmd/relation_test.go

@@ -16,7 +16,6 @@ import (
 	"github.com/juju/cmd/v3/cmdtesting"
 	jujuparams "github.com/juju/juju/rpc/params"
 	"github.com/juju/names/v4"
-	openfga "github.com/openfga/go-sdk"
 	gc "gopkg.in/check.v1"
 	yamlv2 "gopkg.in/yaml.v2"
 	"gopkg.in/yaml.v3"
@@ -25,7 +24,7 @@ import (
 	"github.com/canonical/jimm/cmd/jimmctl/cmd"
 	"github.com/canonical/jimm/internal/db"
 	"github.com/canonical/jimm/internal/dbmodel"
-	ofga "github.com/canonical/jimm/internal/openfga"
+	"github.com/canonical/jimm/internal/openfga"
 	ofganames "github.com/canonical/jimm/internal/openfga/names"
 	jimmnames "github.com/canonical/jimm/pkg/names"
 )
@@ -95,12 +94,13 @@ func (s *relationSuite) TestAddRelationSuperuser(c *gc.C) {
 			c.Assert(strings.Contains(err.Error(), tc.message), gc.Equals, true)
 		} else {
 			c.Assert(err, gc.IsNil)
-			resp, err := s.jimmSuite.JIMM.OpenFGAClient.ReadRelatedObjects(context.Background(), nil, 50, "")
+			tuples, ct, err := s.jimmSuite.JIMM.OpenFGAClient.ReadRelatedObjects(context.Background(), openfga.Tuple{}, 50, "")
 			c.Assert(err, gc.IsNil)
+			c.Assert(ct, gc.Equals, "")
 			// NOTE: this is a bad test because it relies on the number of related objects. So all the
 			// non-failing test cases must be executed before any of the failing tests - failing tests
 			// do not add any tuples therefore the following assertion fails.
-			c.Assert(len(resp.Tuples), gc.Equals, i+3)
+			c.Assert(len(tuples), gc.Equals, i+3)
 		}
 	}
 
@@ -143,9 +143,10 @@ func (s *relationSuite) TestAddRelationViaFileSuperuser(c *gc.C) {
 	_, err = cmdtesting.RunCommand(c, cmd.NewAddRelationCommandForTesting(s.ClientStore(), bClient), "-f", file.Name())
 	c.Assert(err, gc.IsNil)
 
-	resp, err := s.jimmSuite.JIMM.OpenFGAClient.ReadRelatedObjects(context.Background(), nil, 50, "")
+	tuples, ct, err := s.jimmSuite.JIMM.OpenFGAClient.ReadRelatedObjects(context.Background(), openfga.Tuple{}, 50, "")
 	c.Assert(err, gc.IsNil)
-	c.Assert(len(resp.Tuples), gc.Equals, 4)
+	c.Assert(ct, gc.Equals, "")
+	c.Assert(len(tuples), gc.Equals, 4)
 }
 
 func (s *relationSuite) TestAddRelationRejectsUnauthorisedUsers(c *gc.C) {
@@ -192,10 +193,11 @@ func (s *relationSuite) TestRemoveRelationSuperuser(c *gc.C) {
 			c.Assert(err, gc.ErrorMatches, tc.message)
 		} else {
 			c.Assert(err, gc.IsNil)
-			resp, err := s.jimmSuite.JIMM.OpenFGAClient.ReadRelatedObjects(context.Background(), nil, 50, "")
+			tuples, ct, err := s.jimmSuite.JIMM.OpenFGAClient.ReadRelatedObjects(context.Background(), openfga.Tuple{}, 50, "")
 			c.Assert(err, gc.IsNil)
+			c.Assert(ct, gc.Equals, "")
 			totalKeys--
-			c.Assert(len(resp.Tuples), gc.Equals, totalKeys)
+			c.Assert(len(tuples), gc.Equals, totalKeys)
 		}
 	}
 }
@@ -226,11 +228,12 @@ func (s *relationSuite) TestRemoveRelationViaFileSuperuser(c *gc.C) {
 	_, err = cmdtesting.RunCommand(c, cmd.NewRemoveRelationCommandForTesting(s.ClientStore(), bClient), "-f", file.Name())
 	c.Assert(err, gc.IsNil)
 
-	resp, err := s.jimmSuite.JIMM.OpenFGAClient.ReadRelatedObjects(context.Background(), nil, 50, "")
+	tuples, ct, err := s.jimmSuite.JIMM.OpenFGAClient.ReadRelatedObjects(context.Background(), openfga.Tuple{}, 50, "")
 	c.Assert(err, gc.IsNil)
-	c.Logf("existing relations %v", resp.Tuples)
+	c.Assert(ct, gc.Equals, "")
+	c.Logf("existing relations %v", tuples)
 	// Only two relations exist.
-	c.Assert(resp.Tuples, gc.DeepEquals, []ofga.Tuple{{
+	c.Assert(tuples, gc.DeepEquals, []openfga.Tuple{{
 		Object:   ofganames.ConvertTag(names.NewUserTag("admin")),
 		Relation: ofganames.AdministratorRelation,
 		Target:   ofganames.ConvertTag(names.NewControllerTag(s.Params.ControllerUUID)),
@@ -439,20 +442,6 @@ user-eve@external   	administrator	applicationoffer-test-controller-1:alice@exte
 	)
 }
 
-func createTupleKey(object, relation, target string) openfga.TupleKey {
-	k := openfga.NewTupleKey()
-	// in some cases specifying the object is not required
-	if object != "" {
-		k.SetUser(object)
-	}
-	// in some cases specifying the relation is not required
-	if relation != "" {
-		k.SetRelation(relation)
-	}
-	k.SetObject(target)
-	return *k
-}
-
 // TODO: remove boilerplate of env setup and use initialiseEnvironment
 func (s *relationSuite) TestCheckRelationViaSuperuser(c *gc.C) {
 	ctx := context.TODO()
@@ -527,13 +516,13 @@ func (s *relationSuite) TestCheckRelationViaSuperuser(c *gc.C) {
 	err = db.AddModel(ctx, &model)
 	c.Assert(err, gc.IsNil)
 
-	err = ofgaClient.AddRelations(ctx,
-		ofga.Tuple{
+	err = ofgaClient.AddRelation(ctx,
+		openfga.Tuple{
 			Object:   ofganames.ConvertTag(u.ResourceTag()),
 			Relation: "member",
 			Target:   ofganames.ConvertTag(group.Tag().(jimmnames.GroupTag)),
 		},
-		ofga.Tuple{
+		openfga.Tuple{
 			Object:   ofganames.ConvertTagWithRelation(group.Tag().(jimmnames.GroupTag), ofganames.MemberRelation),
 			Relation: "reader",
 			Target:   ofganames.ConvertTag(model.ResourceTag()),

go.mod

@@ -6,7 +6,7 @@ require (
 	github.com/canonical/candid v1.12.2
 	github.com/canonical/go-dqlite v1.11.9
 	github.com/canonical/go-service v1.0.0
-	github.com/frankban/quicktest v1.14.4
+	github.com/frankban/quicktest v1.14.6
 	github.com/go-macaroon-bakery/macaroon-bakery/v3 v3.0.1
 	github.com/gobwas/glob v0.2.4-0.20181002190808-e7a84e9525fe // indirect
 	github.com/google/go-cmp v0.5.9
@@ -46,6 +46,7 @@ require (
 )
 
 require (
+	github.com/canonical/ofga v0.5.0
 	github.com/dustinkirkland/golang-petname v0.0.0-20191129215211-8e5a1ed0cff0
 	github.com/go-chi/chi/v5 v5.0.8
 	github.com/go-chi/render v1.0.2

go.sum

@@ -285,6 +285,8 @@ github.com/canonical/go-dqlite v1.11.9 h1:aO7GG3QohddXsT+C7yEetdRHhhPUWNBKavz+/J
 github.com/canonical/go-dqlite v1.11.9/go.mod h1:Uvy943N8R4CFUAs59A1NVaziWY9nJ686lScY7ywurfg=
 github.com/canonical/go-service v1.0.0 h1:TF6TsEp04xAoI5pPoWjTYmEwLjbPATSnHEyeJCvzElg=
 github.com/canonical/go-service v1.0.0/go.mod h1:GzNLXpkGdglL0kjREXoLXj2rB2Qx+EvAGncRDqCENYQ=
+github.com/canonical/ofga v0.5.0 h1:ILIAsVtKM7/M4fi2CsWhOARyBizZTtXm5D+FBLBs3yU=
+github.com/canonical/ofga v0.5.0/go.mod h1:u4Ou8dbIhO7FmVlT7W3rX2roD9AOGz/CqmGh7AdF0Lo=
 github.com/casbin/casbin/v2 v2.1.2/go.mod h1:YcPU1XXisHhLzuxH9coDNf2FbKpjGlbCg3n9yuLkIJQ=
 github.com/cavaliercoder/go-cpio v0.0.0-20180626203310-925f9528c45e/go.mod h1:oDpT4efm8tSYHXV5tHSdRvBet/b/QzxZ+XyyPehvm3A=
 github.com/cenkalti/backoff v2.2.1+incompatible h1:tNowT99t7UNflLxfYYSlKYsBpXdEet03Pg2g16Swow4=
@@ -543,8 +545,8 @@ github.com/frankban/quicktest v1.7.2/go.mod h1:jaStnuzAqU1AJdCO0l53JDCJrVDKcS03D
 github.com/frankban/quicktest v1.10.0/go.mod h1:ui7WezCLWMWxVWr1GETZY3smRy0G4KWq9vcPtJmFl7Y=
 github.com/frankban/quicktest v1.11.3/go.mod h1:wRf/ReqHper53s+kmmSZizM8NamnL3IM0I9ntUbOk+k=
 github.com/frankban/quicktest v1.13.0/go.mod h1:qLE0fzW0VuyUAJgPU19zByoIr0HtCHN/r/VLSOOIySU=
-github.com/frankban/quicktest v1.14.4 h1:g2rn0vABPOOXmZUj+vbmUp0lPoXEMuhTpIluN0XL9UY=
-github.com/frankban/quicktest v1.14.4/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0=
+github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8=
+github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0=
 github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
 github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ=
 github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY=

internal/auth/jujuauth_test.go

@@ -25,7 +25,7 @@ import (
 func TestAuthenticateLogin(t *testing.T) {
 	c := qt.New(t)
 
-	_, ofgaclient, _, err := jimmtest.SetupTestOFGAClient(c.Name())
+	ofgaClient, _, _, err := jimmtest.SetupTestOFGAClient(c.Name())
 	c.Assert(err, qt.IsNil)
 
 	discharger := bakerytest.NewDischarger(nil)
@@ -36,7 +36,7 @@ func TestAuthenticateLogin(t *testing.T) {
 		},
 	)
 	authenticator := auth.JujuAuthenticator{
-		Client: ofgaclient,
+		Client: ofgaClient,
 		Bakery: identchecker.NewBakery(identchecker.BakeryParams{
 			Locator:        discharger,
 			Key:            bakery.MustGenerateKey(),
@@ -68,7 +68,7 @@ func TestAuthenticateLogin(t *testing.T) {
 func TestAuthenticateLoginWithDomain(t *testing.T) {
 	c := qt.New(t)
 
-	_, ofgaclient, _, err := jimmtest.SetupTestOFGAClient(c.Name())
+	ofgaClient, _, _, err := jimmtest.SetupTestOFGAClient(c.Name())
 	c.Assert(err, qt.IsNil)
 
 	discharger := bakerytest.NewDischarger(nil)
@@ -79,7 +79,7 @@ func TestAuthenticateLoginWithDomain(t *testing.T) {
 		},
 	)
 	authenticator := auth.JujuAuthenticator{
-		Client: ofgaclient,
+		Client: ofgaClient,
 		Bakery: identchecker.NewBakery(identchecker.BakeryParams{
 			Locator:        discharger,
 			Key:            bakery.MustGenerateKey(),
@@ -111,7 +111,7 @@ func TestAuthenticateLoginWithDomain(t *testing.T) {
 func TestAuthenticateLoginSuperuser(t *testing.T) {
 	c := qt.New(t)
 
-	_, ofgaclient, _, err := jimmtest.SetupTestOFGAClient(c.Name())
+	ofgaClient, _, _, err := jimmtest.SetupTestOFGAClient(c.Name())
 	c.Assert(err, qt.IsNil)
 
 	discharger := bakerytest.NewDischarger(nil)
@@ -122,7 +122,7 @@ func TestAuthenticateLoginSuperuser(t *testing.T) {
 		},
 	)
 	authenticator := auth.JujuAuthenticator{
-		Client: ofgaclient,
+		Client: ofgaClient,
 		Bakery: identchecker.NewBakery(identchecker.BakeryParams{
 			Locator:        discharger,
 			Key:            bakery.MustGenerateKey(),
@@ -156,7 +156,7 @@ func TestAuthenticateLoginSuperuser(t *testing.T) {
 func TestAuthenticateLoginInvalidUsernameDeclared(t *testing.T) {
 	c := qt.New(t)
 
-	_, ofgaclient, _, err := jimmtest.SetupTestOFGAClient(c.Name())
+	ofgaClient, _, _, err := jimmtest.SetupTestOFGAClient(c.Name())
 	c.Assert(err, qt.IsNil)
 
 	discharger := bakerytest.NewDischarger(nil)
@@ -167,7 +167,7 @@ func TestAuthenticateLoginInvalidUsernameDeclared(t *testing.T) {
 		},
 	)
 	authenticator := auth.JujuAuthenticator{
-		Client: ofgaclient,
+		Client: ofgaClient,
 		Bakery: identchecker.NewBakery(identchecker.BakeryParams{
 			Locator:        discharger,
 			Key:            bakery.MustGenerateKey(),

internal/jimm/access.go

@@ -17,7 +17,7 @@ import (
 )
 
 // ToOfferAccessString maps relation to an application offer access string.
-func ToOfferAccessString(relation ofganames.Relation) string {
+func ToOfferAccessString(relation openfga.Relation) string {
 	switch relation {
 	case ofganames.AdministratorRelation:
 		return string(jujuparams.OfferAdminAccess)
@@ -31,7 +31,7 @@ func ToOfferAccessString(relation ofganames.Relation) string {
 }
 
 // ToCloudAccessString maps relation to a cloud access string.
-func ToCloudAccessString(relation ofganames.Relation) string {
+func ToCloudAccessString(relation openfga.Relation) string {
 	switch relation {
 	case ofganames.AdministratorRelation:
 		return "admin"
@@ -43,7 +43,7 @@ func ToCloudAccessString(relation ofganames.Relation) string {
 }
 
 // ToModelAccessString maps relation to a model access string.
-func ToModelAccessString(relation ofganames.Relation) string {
+func ToModelAccessString(relation openfga.Relation) string {
 	switch relation {
 	case ofganames.AdministratorRelation:
 		return "admin"
@@ -57,7 +57,7 @@ func ToModelAccessString(relation ofganames.Relation) string {
 }
 
 // ToModelAccessString maps relation to a controller access string.
-func ToControllerAccessString(relation ofganames.Relation) string {
+func ToControllerAccessString(relation openfga.Relation) string {
 	switch relation {
 	case ofganames.AdministratorRelation:
 		return "superuser"
@@ -70,7 +70,7 @@ func ToControllerAccessString(relation ofganames.Relation) string {
 // string can be either "admin", in which case the administrator relation
 // is returned, or "add-model", in which case the can_addmodel relation is
 // returned.
-func ToCloudRelation(accessLevel string) (ofganames.Relation, error) {
+func ToCloudRelation(accessLevel string) (openfga.Relation, error) {
 	switch accessLevel {
 	case "admin":
 		return ofganames.AdministratorRelation, nil
@@ -82,7 +82,7 @@ func ToCloudRelation(accessLevel string) (ofganames.Relation, error) {
 }
 
 // ToModelRelation returns a valid relation for the model.
-func ToModelRelation(accessLevel string) (ofganames.Relation, error) {
+func ToModelRelation(accessLevel string) (openfga.Relation, error) {
 	switch accessLevel {
 	case "admin":
 		return ofganames.AdministratorRelation, nil
@@ -96,7 +96,7 @@ func ToModelRelation(accessLevel string) (ofganames.Relation, error) {
 }
 
 // ToOfferRelation returns a valid relation for the application offer.
-func ToOfferRelation(accessLevel string) (ofganames.Relation, error) {
+func ToOfferRelation(accessLevel string) (openfga.Relation, error) {
 	switch accessLevel {
 	case "":
 		return ofganames.NoRelation, nil
@@ -223,7 +223,7 @@ func checkPermission(ctx context.Context, user *openfga.User, cachedPerms map[st
 			if err != nil {
 				return cachedPerms, errors.E(op, fmt.Sprintf("failed to parse relation %s", stringVal), err)
 			}
-			check, _, err := openfga.CheckRelation(ctx, user, tag, relation)
+			check, err := openfga.CheckRelation(ctx, user, tag, relation)
 			if err != nil {
 				return cachedPerms, errors.E(op, err)
 			}

internal/jimm/access_test.go

@@ -17,41 +17,20 @@ import (
 	"github.com/google/uuid"
 )
 
-// TODO(Kian): We could use a test as below to unit test the
-// auth function returned by JwtGenerator and ensure it correctly
-// generates a JWT. The JWTGenerator requires the JIMM server to have
-// a JWT service and cache setup, so we could either turn this into
-// an interface and mock it or have the test start a full JIMM server.
-// Also required an interface for authentication, mocked or Candid.
-// This is already tested in an integration test in jujuapi/websocket_test.go
-// func TestJwtGenerator(t *testing.T) {
-// 	c := qt.New(t)
-
-// 	_, client, _, err := jimmtest.SetupTestOFGAClient(c.Name())
-// 	c.Assert(err, qt.IsNil)
-
-// 	j := &jimm.JIMM{
-// 		UUID: uuid.NewString(),
-// 		Database: db.Database{
-// 			DB: jimmtest.MemoryDB(c, nil),
-// 		},
-// 		OpenFGAClient: client,
-// 		JWTService:    jimmjwx.NewJWTService(,),
-// 	}
-// 	ctx := context.Background()
-// 	m := &dbmodel.Model{}
-// 	authFunc := j.JwtGenerator(ctx, m)
-// 	loginReq := new(jujuparams.LoginRequest)
-// 	desiredPerms := map[string]interface{}{"model-123": "writer", "applicationoffer-123": "consumer"}
-// 	token, err := authFunc(loginReq, desiredPerms)
-// 	c.Assert(err, qt.IsNil)
-// 	//Check token is valid and has correct assertions.
-// }
+/*
+TODO: (CSS-5247) We could use a test as below to unit test the
+auth function returned by JwtGenerator and ensure it correctly
+generates a JWT. The JWTGenerator requires the JIMM server to have
+a JWT service and cache setup, so we could either turn this into
+an interface and mock it or have the test start a full JIMM server.
+Also required an interface for authentication, mocked or Candid.
+This is already tested in an integration test in jujuapi/websocket_test.go
+*/
 
 func TestAuditLogAccess(t *testing.T) {
 	c := qt.New(t)
 
-	_, ofgaClient, _, err := jimmtest.SetupTestOFGAClient(c.Name())
+	ofgaClient, _, _, err := jimmtest.SetupTestOFGAClient(c.Name())
 	c.Assert(err, qt.IsNil)
 
 	now := time.Now().UTC().Round(time.Millisecond)

internal/jimm/applicationoffer.go

@@ -260,7 +260,7 @@ func (j *JIMM) listApplicationOfferUsers(ctx context.Context, offer names.Applic
 	users := make(map[string]string)
 
 	// we loop through relations in a decreasing order of access
-	for _, relation := range []ofganames.Relation{
+	for _, relation := range []openfga.Relation{
 		ofganames.AdministratorRelation,
 		ofganames.ConsumerRelation,
 		ofganames.ReaderRelation,