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.

Sign up for GitHub

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-9389 Implement the EntitlementsService interface #1277

Merged
merged 5 commits into from
Jul 22, 2024
Merged

CSS-9389 Implement the EntitlementsService interface #1277

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c297be2
add entitlements api to rebac endpoint
SimoneDutto Jul 22, 2024
a85d537
fix pr comments
SimoneDutto Jul 22, 2024
3d89f93
update embed file names
SimoneDutto Jul 22, 2024
b89244b
remove test
SimoneDutto Jul 22, 2024
d7a23f3
remove static_entitlements file
SimoneDutto Jul 22, 2024
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
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ require (
require (
github.com/antonlindstrom/pgstore v0.0.0-20220421113606-e3a6e3fed12a
github.com/canonical/ofga v0.10.0
github.com/canonical/rebac-admin-ui-handlers v0.0.0-20240702080748-d1d377dd37b3
github.com/canonical/rebac-admin-ui-handlers v0.0.0-20240717130927-038848181576
github.com/coreos/go-oidc/v3 v3.9.0
github.com/dustinkirkland/golang-petname v0.0.0-20231002161417-6a283f1aaaf2
github.com/go-chi/chi/v5 v5.0.12
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ github.com/canonical/ofga v0.10.0 h1:DHXhG/DAXWWQT/I+2jzr4qm0uTIYrILmtMxd6ZqmEzE
github.com/canonical/ofga v0.10.0/go.mod h1:u4Ou8dbIhO7FmVlT7W3rX2roD9AOGz/CqmGh7AdF0Lo=
github.com/canonical/rebac-admin-ui-handlers v0.0.0-20240702080748-d1d377dd37b3 h1:BitPbyXN2gBaARUZt/KDrH7ekdBuyCm2NOI+WtOqHAs=
github.com/canonical/rebac-admin-ui-handlers v0.0.0-20240702080748-d1d377dd37b3/go.mod h1:EIdBoaTHWYPkzNeUeXUBueJkglN9nQz5HLIvaOT7o1k=
github.com/canonical/rebac-admin-ui-handlers v0.0.0-20240717130927-038848181576 h1:i4lyP7WRFB711aLMkBYiBN23njSFJV0DaHYJn7rwOGY=
github.com/canonical/rebac-admin-ui-handlers v0.0.0-20240717130927-038848181576/go.mod h1:EIdBoaTHWYPkzNeUeXUBueJkglN9nQz5HLIvaOT7o1k=
github.com/cenkalti/backoff/v3 v3.0.0/go.mod h1:cIeZDE3IrqwwJl6VUwCN6trj1oXrTS4rc0ij+ULvLYs=
github.com/cenkalti/backoff/v3 v3.2.2 h1:cfUAAO3yvKMYKPrvhDuHSwQnhZNk/RMHKdZqKTxfm6M=
github.com/cenkalti/backoff/v3 v3.2.2/go.mod h1:cIeZDE3IrqwwJl6VUwCN6trj1oXrTS4rc0ij+ULvLYs=
Expand Down
1 change: 1 addition & 0 deletions internal/rebac_admin/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ func SetupBackend(ctx context.Context) (*rebachandlers.ReBACAdminBackend, error)

rebacBackend, err := rebachandlers.NewReBACAdminBackend(rebachandlers.ReBACAdminBackendParams{
Authenticator: &Authenticator{},
Entitlements: &EntitlementsService{},
})
if err != nil {
zapctx.Error(ctx, "failed to create rebac admin backend", zap.Error(err))
Expand Down
22 changes: 22 additions & 0 deletions internal/rebac_admin/entitlements.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright 2023 canonical.
SimoneDutto marked this conversation as resolved.
Show resolved Hide resolved

package rebac_admin

import (
"context"

openfgastatic "github.com/canonical/jimm/openfga"
SimoneDutto marked this conversation as resolved.
Show resolved Hide resolved
"github.com/canonical/rebac-admin-ui-handlers/v1/resources"
)

type EntitlementsService struct{}
SimoneDutto marked this conversation as resolved.
Show resolved Hide resolved

// ListEntitlements returns the list of entitlements in JSON format.
func (s *EntitlementsService) ListEntitlements(ctx context.Context, params *resources.GetEntitlementsParams) ([]resources.EntityEntitlement, error) {
return EntitlementsList, nil
}

// RawEntitlements returns the list of entitlements as raw text.
func (s *EntitlementsService) RawEntitlements(ctx context.Context) (string, error) {
return string(openfgastatic.AuthModelFileDSL), nil
}
59 changes: 59 additions & 0 deletions internal/rebac_admin/static_entitlements.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Copyright 2023 canonical.
SimoneDutto marked this conversation as resolved.
Show resolved Hide resolved

package rebac_admin

import "github.com/canonical/rebac-admin-ui-handlers/v1/resources"

// For rebac v1 this list is kept manually
SimoneDutto marked this conversation as resolved.
Show resolved Hide resolved
var EntitlementsList = []resources.EntityEntitlement{
// applicationoffer
{EntitlementType: "administrator", EntityName: "user", EntityType: "applicationoffer"},
{EntitlementType: "administrator", EntityName: "user:*", EntityType: "applicationoffer"},
{EntitlementType: "administrator", EntityName: "group#member", EntityType: "applicationoffer"},
{EntitlementType: "consumer", EntityName: "user", EntityType: "applicationoffer"},
{EntitlementType: "consumer", EntityName: "user:*", EntityType: "applicationoffer"},
{EntitlementType: "consumer", EntityName: "group#member", EntityType: "applicationoffer"},
{EntitlementType: "reader", EntityName: "user", EntityType: "applicationoffer"},
{EntitlementType: "reader", EntityName: "user:*", EntityType: "applicationoffer"},
{EntitlementType: "reader", EntityName: "group#member", EntityType: "applicationoffer"},
{EntitlementType: "model", EntityName: "model", EntityType: "applicationoffer"},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be removed.


// cloud
{EntitlementType: "administrator", EntityName: "user", EntityType: "cloud"},
{EntitlementType: "administrator", EntityName: "user:*", EntityType: "cloud"},
{EntitlementType: "administrator", EntityName: "group#member", EntityType: "cloud"},
{EntitlementType: "can_addmodel", EntityName: "user", EntityType: "cloud"},
{EntitlementType: "can_addmodel", EntityName: "user:*", EntityType: "cloud"},
{EntitlementType: "can_addmodel", EntityName: "group#member", EntityType: "cloud"},
{EntitlementType: "controller", EntityName: "controller", EntityType: "cloud"},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be removed.


// controller
{EntitlementType: "administrator", EntityName: "user", EntityType: "controller"},
{EntitlementType: "administrator", EntityName: "user:*", EntityType: "controller"},
{EntitlementType: "administrator", EntityName: "group#member", EntityType: "controller"},
{EntitlementType: "audit_log_viewer", EntityName: "user", EntityType: "controller"},
{EntitlementType: "audit_log_viewer", EntityName: "user:*", EntityType: "controller"},
{EntitlementType: "audit_log_viewer", EntityName: "group#member", EntityType: "controller"},

// group
{EntitlementType: "member", EntityName: "user", EntityType: "group"},
{EntitlementType: "member", EntityName: "user:*", EntityType: "group"},
{EntitlementType: "member", EntityName: "group#member", EntityType: "group"},

// model
{EntitlementType: "administrator", EntityName: "user", EntityType: "model"},
{EntitlementType: "administrator", EntityName: "user:*", EntityType: "model"},
{EntitlementType: "administrator", EntityName: "group#member", EntityType: "model"},
{EntitlementType: "reader", EntityName: "user", EntityType: "model"},
{EntitlementType: "reader", EntityName: "user:*", EntityType: "model"},
{EntitlementType: "reader", EntityName: "group#member", EntityType: "model"},
{EntitlementType: "writer", EntityName: "user", EntityType: "model"},
{EntitlementType: "writer", EntityName: "user:*", EntityType: "model"},
{EntitlementType: "writer", EntityName: "group#member", EntityType: "model"},
{EntitlementType: "controller", EntityName: "controller", EntityType: "model"},
Copy link
Member

@babakks babakks Jul 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be removed. Generally, let's remove all entitlements that has a receiver other than user, user:*, or group#member.


// serviceaccount
{EntitlementType: "administrator", EntityName: "user", EntityType: "serviceaccount"},
{EntitlementType: "administrator", EntityName: "user:*", EntityType: "serviceaccount"},
{EntitlementType: "administrator", EntityName: "group#member", EntityType: "serviceaccount"},
}
3 changes: 3 additions & 0 deletions openfga/auth_model.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,6 @@ import (

//go:embed authorisation_model.json
var AuthModelFile []byte
kian99 marked this conversation as resolved.
Show resolved Hide resolved

//go:embed authorisation_model.fga
var AuthModelFileDSL []byte
26 changes: 26 additions & 0 deletions service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,32 @@ func TestRebacAdminApi(t *testing.T) {
c.Assert(response.StatusCode, qt.Equals, 200)
}

func TestRebacEntitlementsApi(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should be adding this test in here, otherwise we might start to test all our HTTP endpoints in here which is probably not what we want.
@ale8k @babakks @alesstimec What do you guys think about this?
In other packages like internal/jimmjwx/ we create a JIMM service by calling jimm.NewService. Should service layer tests exist here (in service_test.go) or in the package internal/rebac_admin?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimoneDutto Yeah, please remove this test.

@kian99 I agree. We already have a test for ReBAC Admin API in service_test.go but that's just for the sake of composition and to make sure we're correctly enabling the API.

Other endpoints should be tested inside the rebac_admin package. Also, I think we shouldn't test against the HTTP endpoints (i.e., calling /entitlements directly) because that's the responsibility of the rebac-admin-ui-handlers library (and we should know as little as possible about the HTTP API endpoints in JIMM). So, we can safely assume the library works as expected.

Generally, in the rebac_admin package tests we should just call the methods (like EntitlementsService.RawEntitlements) directly and assert the returned value against an expected value. However, in our static implementation of EntitlementsService interface, I don't see a point of having tests only to assert against a the same static values.

c := qt.New(t)

_, _, cofgaParams, err := jimmtest.SetupTestOFGAClient(c.Name())
c.Assert(err, qt.IsNil)

p := jimmtest.NewTestJimmParams(c)
p.InsecureSecretStorage = true
p.OpenFGAParams = cofgaParamsToJIMMOpenFGAParams(*cofgaParams)

svc, err := jimm.NewService(context.Background(), p)
c.Assert(err, qt.IsNil)
defer svc.Cleanup()

srv := httptest.NewTLSServer(svc)
c.Cleanup(srv.Close)

response, err := srv.Client().Get(srv.URL + "/rebac/v1/entitlements")
c.Assert(err, qt.IsNil)
c.Assert(response.StatusCode, qt.Equals, 200)

responseRaw, err := srv.Client().Get(srv.URL + "/rebac/v1/entitlements/raw")
c.Assert(err, qt.IsNil)
c.Assert(responseRaw.StatusCode, qt.Equals, 200)
}

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

Expand Down
Loading