From 1e4ea1b464dc05ab4f9b539edd927a5f4a770cd0 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Fri, 29 Sep 2023 14:32:32 +0200 Subject: [PATCH 1/4] Add test for ParseRelation function - Ensure all valid relation strings can be parsed to a relation type --- internal/openfga/names/names.go | 8 ++++++++ internal/openfga/names/names_test.go | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/internal/openfga/names/names.go b/internal/openfga/names/names.go index ad869524f..32789883c 100644 --- a/internal/openfga/names/names.go +++ b/internal/openfga/names/names.go @@ -36,6 +36,8 @@ var ( NoRelation cofga.Relation = "" ) +var AllRelations = []cofga.Relation{MemberRelation, AdministratorRelation, ControllerRelation, ModelRelation, ConsumerRelation, ReaderRelation, WriterRelation, CanAddModelRelation, AuditLogViewerRelation, NoRelation} + // Tag represents an entity tag as used by JIMM in OpenFGA. type Tag = cofga.Entity @@ -128,6 +130,10 @@ func ParseRelation(relationString string) (cofga.Relation, error) { switch relationString { case "": return cofga.Relation(""), nil + case ControllerRelation.String(): + return ControllerRelation, nil + case ModelRelation.String(): + return ModelRelation, nil case MemberRelation.String(): return MemberRelation, nil case AdministratorRelation.String(): @@ -140,6 +146,8 @@ func ParseRelation(relationString string) (cofga.Relation, error) { return WriterRelation, nil case CanAddModelRelation.String(): return CanAddModelRelation, nil + case AuditLogViewerRelation.String(): + return AuditLogViewerRelation, nil default: return cofga.Relation(""), errors.E("unknown relation") diff --git a/internal/openfga/names/names_test.go b/internal/openfga/names/names_test.go index ae486fb32..566a8b9f1 100644 --- a/internal/openfga/names/names_test.go +++ b/internal/openfga/names/names_test.go @@ -76,3 +76,11 @@ func (s *namesSuite) TestConvertJujuRelation(c *gc.C) { } } } + +func (s *namesSuite) TestParseRelations(c *gc.C) { + for _, relation := range ofganames.AllRelations { + res, err := ofganames.ParseRelation(relation.String()) + c.Assert(err, gc.IsNil, gc.Commentf("testing relation %s", relation)) + c.Assert(res, gc.Equals, relation, gc.Commentf("testing relation %s", relation)) + } +} From 80a36265fd3e0ce1875ec18dc5fe4a54fcd4a338 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Fri, 29 Sep 2023 14:35:58 +0200 Subject: [PATCH 2/4] Add comment to ensure slice stays up to date --- internal/openfga/names/names.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/openfga/names/names.go b/internal/openfga/names/names.go index 32789883c..23d26026f 100644 --- a/internal/openfga/names/names.go +++ b/internal/openfga/names/names.go @@ -36,6 +36,8 @@ var ( NoRelation cofga.Relation = "" ) +// AllRelations contains a slice of all valid relations. +// NB: Add any new relations from the above to this slice. var AllRelations = []cofga.Relation{MemberRelation, AdministratorRelation, ControllerRelation, ModelRelation, ConsumerRelation, ReaderRelation, WriterRelation, CanAddModelRelation, AuditLogViewerRelation, NoRelation} // Tag represents an entity tag as used by JIMM in OpenFGA. From b091f8ef80153b212995ecfe73cd752b11a53364 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Fri, 29 Sep 2023 15:43:02 +0200 Subject: [PATCH 3/4] Fix docker compose --- cmd/jimmsrv/main.go | 5 +++++ docker-compose.yaml | 1 + internal/openfga/names/names.go | 12 ++++++++---- service.go | 6 +++++- 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/cmd/jimmsrv/main.go b/cmd/jimmsrv/main.go index 7bb63b6fd..f4b78adec 100644 --- a/cmd/jimmsrv/main.go +++ b/cmd/jimmsrv/main.go @@ -70,6 +70,10 @@ func start(ctx context.Context, s *service.Service) error { if _, ok := os.LookupEnv("INSECURE_SECRET_STORAGE"); ok { insecureSecretStorage = true } + insecureJwksLookup := false + if _, ok := os.LookupEnv("INSECURE_JWKS_LOOKUP"); ok { + insecureJwksLookup = true + } jimmsvc, err := jimm.NewService(ctx, jimm.Params{ ControllerUUID: os.Getenv("JIMM_UUID"), DSN: os.Getenv("JIMM_DSN"), @@ -97,6 +101,7 @@ func start(ctx context.Context, s *service.Service) error { MacaroonExpiryDuration: macaroonExpiryDuration, JWTExpiryDuration: jwtExpiryDuration, InsecureSecretStorage: insecureSecretStorage, + InsecureJwksLookup: insecureJwksLookup, }) if err != nil { return err diff --git a/docker-compose.yaml b/docker-compose.yaml index e4d11dfd9..332658e7a 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -55,6 +55,7 @@ services: VAULT_PATH: "/jimm-kv/" VAULT_SECRET_FILE: "/vault/approle.json" VAULT_AUTH_PATH: "/auth/approle/login" + INSECURE_JWKS_LOOKUP: "enabled" # Note: By default we should use Vault as that is the primary means of secret storage. # INSECURE_SECRET_STORAGE: "enabled" # JIMM_DASHBOARD_LOCATION: "" diff --git a/internal/openfga/names/names.go b/internal/openfga/names/names.go index 23d26026f..6e15cc189 100644 --- a/internal/openfga/names/names.go +++ b/internal/openfga/names/names.go @@ -5,6 +5,8 @@ package names import ( + "fmt" + "github.com/canonical/jimm/internal/errors" jimmnames "github.com/canonical/jimm/pkg/names" cofga "github.com/canonical/ofga" @@ -105,6 +107,7 @@ func BlankKindTag(kind string) (*Tag, error) { // ConvertJujuRelation takes a juju relation string and converts it to // one appropriate for use with OpenFGA. func ConvertJujuRelation(relation string) (cofga.Relation, error) { + const op = errors.Op("ConvertJujuRelation") switch relation { case string(permission.AdminAccess): return AdministratorRelation, nil @@ -119,16 +122,17 @@ func ConvertJujuRelation(relation string) (cofga.Relation, error) { // Below are controller specific permissions that // are not represented in JIMM's OpenFGA model. case string(permission.LoginAccess): - return NoRelation, errors.E("login access unused") + return NoRelation, errors.E(op, "login access unused") case string(permission.SuperuserAccess): - return NoRelation, errors.E("superuser access unused") + return NoRelation, errors.E(op, "superuser access unused") default: - return NoRelation, errors.E("unknown relation") + return NoRelation, errors.E(op, "unknown relation") } } // ParseRelation parses the relation string func ParseRelation(relationString string) (cofga.Relation, error) { + const op = errors.Op("ParseRelation") switch relationString { case "": return cofga.Relation(""), nil @@ -151,7 +155,7 @@ func ParseRelation(relationString string) (cofga.Relation, error) { case AuditLogViewerRelation.String(): return AuditLogViewerRelation, nil default: - return cofga.Relation(""), errors.E("unknown relation") + return cofga.Relation(""), errors.E(op, fmt.Sprintf("unknown relation %s", relationString)) } } diff --git a/service.go b/service.go index 4cb5afeca..1742c8832 100644 --- a/service.go +++ b/service.go @@ -159,6 +159,10 @@ type Params struct { // InsecureSecretStorage instructs JIMM to store secrets in its database // instead of dedicated secure storage. SHOULD NOT BE USED IN PRODUCTION. InsecureSecretStorage bool + + // InsecureJwksLookup instructs JIMM to lookup its JWKS value via + // http instead of https. Useful when running JIMM in a docker compose. + InsecureJwksLookup bool } // A Service is the implementation of a JIMM server. @@ -312,7 +316,7 @@ func NewService(ctx context.Context, p Params) (*Service, error) { s.jimm.JWKService = jimmjwx.NewJWKSService(s.jimm.CredentialStore) s.jimm.JWTService = jimmjwx.NewJWTService(jimmjwx.JWTServiceParams{ Host: p.PublicDNSName, - Secure: true, + Secure: !p.InsecureJwksLookup, Store: s.jimm.CredentialStore, Expiry: p.JWTExpiryDuration, }) From c682687befd92f3930d663064dd5a37cc803937e Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Fri, 29 Sep 2023 16:04:31 +0200 Subject: [PATCH 4/4] Increased compose timeout --- docker-compose.yaml | 2 +- internal/openfga/names/export_test.go | 4 ++++ internal/openfga/names/names.go | 4 ++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/docker-compose.yaml b/docker-compose.yaml index 332658e7a..c3fcd2542 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -83,7 +83,7 @@ services: test: [ "CMD", "curl", "http://jimm.localhost:80" ] interval: 5s timeout: 5s - retries: 40 + retries: 50 depends_on: db: condition: service_healthy diff --git a/internal/openfga/names/export_test.go b/internal/openfga/names/export_test.go index 99d9aa422..024e9e20e 100644 --- a/internal/openfga/names/export_test.go +++ b/internal/openfga/names/export_test.go @@ -4,6 +4,10 @@ package names import cofga "github.com/canonical/ofga" +var ( + AllRelations = allRelations +) + func NewTag(id, kind, relation string) *Tag { return &Tag{ ID: id, diff --git a/internal/openfga/names/names.go b/internal/openfga/names/names.go index 6e15cc189..d7d8a85ac 100644 --- a/internal/openfga/names/names.go +++ b/internal/openfga/names/names.go @@ -38,9 +38,9 @@ var ( NoRelation cofga.Relation = "" ) -// AllRelations contains a slice of all valid relations. +// allRelations contains a slice of all valid relations. // NB: Add any new relations from the above to this slice. -var AllRelations = []cofga.Relation{MemberRelation, AdministratorRelation, ControllerRelation, ModelRelation, ConsumerRelation, ReaderRelation, WriterRelation, CanAddModelRelation, AuditLogViewerRelation, NoRelation} +var allRelations = []cofga.Relation{MemberRelation, AdministratorRelation, ControllerRelation, ModelRelation, ConsumerRelation, ReaderRelation, WriterRelation, CanAddModelRelation, AuditLogViewerRelation, NoRelation} // Tag represents an entity tag as used by JIMM in OpenFGA. type Tag = cofga.Entity