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

Add JImmAdmin field to User struct #1101

Merged
merged 5 commits into from
Nov 28, 2023
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
5 changes: 3 additions & 2 deletions cmd/jimmctl/cmd/jimmsuite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,9 @@ func (s *jimmSuite) AddController(c *gc.C, name string, info *api.Info) {
Port: hp.Port(),
}})
}
user := openfga.NewUser(s.AdminUser, s.OFGAClient)
err := s.JIMM.AddController(context.Background(), user, ctl)
adminUser := openfga.NewUser(s.AdminUser, s.OFGAClient)
adminUser.JimmAdmin = true
err := s.JIMM.AddController(context.Background(), adminUser, ctl)
c.Assert(err, gc.Equals, nil)
}

Expand Down
18 changes: 7 additions & 11 deletions internal/jimm/applicationoffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,10 +337,10 @@ func (j *JIMM) GetApplicationOffer(ctx context.Context, user *openfga.User, offe
}

// GrantOfferAccess grants rights for an application offer.
func (j *JIMM) GrantOfferAccess(ctx context.Context, u *openfga.User, offerURL string, ut names.UserTag, access jujuparams.OfferAccessPermission) error {
func (j *JIMM) GrantOfferAccess(ctx context.Context, user *openfga.User, offerURL string, ut names.UserTag, access jujuparams.OfferAccessPermission) error {
const op = errors.Op("jimm.GrantOfferAccess")

err := j.doApplicationOfferAdmin(ctx, u, offerURL, func(offer *dbmodel.ApplicationOffer, api API) error {
err := j.doApplicationOfferAdmin(ctx, user, offerURL, func(offer *dbmodel.ApplicationOffer, api API) error {
tUser := openfga.NewUser(&dbmodel.User{Username: ut.Id()}, j.OpenFGAClient)
currentRelation := tUser.GetApplicationOfferAccess(ctx, offer.ResourceTag())
currentAccessLevel := ToOfferAccessString(currentRelation)
Expand Down Expand Up @@ -720,21 +720,17 @@ func (j *JIMM) ListApplicationOffers(ctx context.Context, user *openfga.User, fi
return offers, nil
}

func (j *JIMM) listApplicationOffersForModel(ctx context.Context, u *openfga.User, m *dbmodel.Model, filters []jujuparams.OfferFilter) ([]jujuparams.ApplicationOfferAdminDetails, error) {
func (j *JIMM) listApplicationOffersForModel(ctx context.Context, user *openfga.User, m *dbmodel.Model, filters []jujuparams.OfferFilter) ([]jujuparams.ApplicationOfferAdminDetails, error) {
const op = errors.Op("jimm.listApplicationOffersForModel")

if err := j.Database.GetModel(ctx, m); err != nil {
return nil, errors.E(op, err)
}
isControllerAdmin, err := openfga.IsAdministrator(ctx, u, j.ResourceTag())
isModelAdmin, err := openfga.IsAdministrator(ctx, user, m.ResourceTag())
if err != nil {
return nil, errors.E(op, err)
}
isModelAdmin, err := openfga.IsAdministrator(ctx, u, m.ResourceTag())
if err != nil {
return nil, errors.E(op, err)
}
if !isControllerAdmin && !isModelAdmin {
if !user.JimmAdmin && !isModelAdmin {
return nil, errors.E(op, errors.CodeUnauthorized, "unauthorized")
}
api, err := j.dial(ctx, &m.Controller, names.ModelTag{})
Expand All @@ -756,7 +752,7 @@ func (j *JIMM) listApplicationOffersForModel(ctx context.Context, u *openfga.Use
// Note: The user does not need to have any access level on the offer itself.
// As long as they are model admins or controller superusers they can also
// manipulate the application offer as admins.
func (j *JIMM) doApplicationOfferAdmin(ctx context.Context, u *openfga.User, offerURL string, f func(offer *dbmodel.ApplicationOffer, api API) error) error {
func (j *JIMM) doApplicationOfferAdmin(ctx context.Context, user *openfga.User, offerURL string, f func(offer *dbmodel.ApplicationOffer, api API) error) error {
const op = errors.Op("jimm.doApplicationOfferAdmin")

offer := dbmodel.ApplicationOffer{
Expand All @@ -766,7 +762,7 @@ func (j *JIMM) doApplicationOfferAdmin(ctx context.Context, u *openfga.User, off
return errors.E(op, err)
}

isOfferAdmin, err := openfga.IsAdministrator(ctx, u, offer.ResourceTag())
isOfferAdmin, err := openfga.IsAdministrator(ctx, user, offer.ResourceTag())
if err != nil {
return errors.E(op, err)
}
Expand Down
26 changes: 11 additions & 15 deletions internal/jimm/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (j *JIMM) GetUserCloudAccess(ctx context.Context, user *openfga.User, cloud
// error with a code of CodeUnauthorized is returned. If the user only has
// add-model access to the cloud then the returned Users field will only
// contain the authentcated user.
func (j *JIMM) GetCloud(ctx context.Context, u *openfga.User, tag names.CloudTag) (dbmodel.Cloud, error) {
func (j *JIMM) GetCloud(ctx context.Context, user *openfga.User, tag names.CloudTag) (dbmodel.Cloud, error) {
const op = errors.Op("jimm.GetCloud")

var cl dbmodel.Cloud
Expand All @@ -52,7 +52,7 @@ func (j *JIMM) GetCloud(ctx context.Context, u *openfga.User, tag names.CloudTag
return cl, errors.E(op, err)
}

accessLevel, err := j.GetUserCloudAccess(ctx, u, tag)
accessLevel, err := j.GetUserCloudAccess(ctx, user, tag)
if err != nil {
if err != nil {
return dbmodel.Cloud{}, errors.E(op, err)
Expand Down Expand Up @@ -130,11 +130,7 @@ func (j *JIMM) ForEachUserCloud(ctx context.Context, user *openfga.User, f func(
func (j *JIMM) ForEachCloud(ctx context.Context, user *openfga.User, f func(*dbmodel.Cloud) error) error {
const op = errors.Op("jimm.ForEachCloud")

isControllerAdmin, err := openfga.IsAdministrator(ctx, user, j.ResourceTag())
if err != nil {
return errors.E(op, err)
}
if !isControllerAdmin {
if !user.JimmAdmin {
return errors.E(op, errors.CodeUnauthorized, "unauthorized")
}

Expand Down Expand Up @@ -473,7 +469,7 @@ func (j *JIMM) addControllerCloud(ctx context.Context, ctl *dbmodel.Controller,
// the cloud then the returned error will have the same code as the error
// returned from the dial operation. If the given function returns an error
// that error will be returned with the code unmasked.
func (j *JIMM) doCloudAdmin(ctx context.Context, u *openfga.User, ct names.CloudTag, f func(*dbmodel.Cloud, API) error) error {
func (j *JIMM) doCloudAdmin(ctx context.Context, user *openfga.User, ct names.CloudTag, f func(*dbmodel.Cloud, API) error) error {
const op = errors.Op("jimm.doCloudAdmin")

var c dbmodel.Cloud
Expand All @@ -483,7 +479,7 @@ func (j *JIMM) doCloudAdmin(ctx context.Context, u *openfga.User, ct names.Cloud
return errors.E(op, err)
}

isCloudAdministrator, err := openfga.IsAdministrator(ctx, u, c.ResourceTag())
isCloudAdministrator, err := openfga.IsAdministrator(ctx, user, c.ResourceTag())
if err != nil {
return errors.E(op, err)
}
Expand Down Expand Up @@ -657,10 +653,10 @@ func (j *JIMM) RevokeCloudAccess(ctx context.Context, user *openfga.User, ct nam
// authenticated user does not have admin access to the cloud then an error
// with the code CodeUnauthorized is returned. If the RemoveClouds API call
// retuns an error the error code is not masked.
func (j *JIMM) RemoveCloud(ctx context.Context, u *openfga.User, ct names.CloudTag) error {
func (j *JIMM) RemoveCloud(ctx context.Context, user *openfga.User, ct names.CloudTag) error {
const op = errors.Op("jimm.RemoveCloud")

err := j.doCloudAdmin(ctx, u, ct, func(c *dbmodel.Cloud, api API) error {
err := j.doCloudAdmin(ctx, user, ct, func(c *dbmodel.Cloud, api API) error {
// Note: JIMM doesn't attempt to determine if the cloud is
// used by any models before attempting to remove it. JIMM
// relies on the controller failing the RemoveClouds API
Expand Down Expand Up @@ -689,7 +685,7 @@ func (j *JIMM) RemoveCloud(ctx context.Context, u *openfga.User, ct names.CloudT
// an admin on the cloud an error is returned with a code of
// CodeUnauthorized. If the cloud with the given name cannot be found then
// an error with the code CodeNotFound is returned.
func (j *JIMM) UpdateCloud(ctx context.Context, u *openfga.User, ct names.CloudTag, cloud jujuparams.Cloud) error {
func (j *JIMM) UpdateCloud(ctx context.Context, user *openfga.User, ct names.CloudTag, cloud jujuparams.Cloud) error {
const op = errors.Op("jimm.UpdateCloud")

var c dbmodel.Cloud
Expand All @@ -698,7 +694,7 @@ func (j *JIMM) UpdateCloud(ctx context.Context, u *openfga.User, ct names.CloudT
if err := j.Database.GetCloud(ctx, &c); err != nil {
return errors.E(op, err)
}
cloudAccess, err := j.GetUserCloudAccess(ctx, u, c.ResourceTag())
cloudAccess, err := j.GetUserCloudAccess(ctx, user, c.ResourceTag())
if err != nil {
return errors.E(op, err)
}
Expand Down Expand Up @@ -762,7 +758,7 @@ func (j *JIMM) UpdateCloud(ctx context.Context, u *openfga.User, ct names.CloudT
// CodeNotFound is returned. If the authenticated user does not have admin
// access to the cloud then an error with the code CodeUnauthorized is returned.
// If the RemoveClouds API call retuns an error the error code is not masked.
func (j *JIMM) RemoveCloudFromController(ctx context.Context, u *openfga.User, controllerName string, ct names.CloudTag) error {
func (j *JIMM) RemoveCloudFromController(ctx context.Context, user *openfga.User, controllerName string, ct names.CloudTag) error {
const op = errors.Op("jimm.RemoveCloudFromController")

var cloud dbmodel.Cloud
Expand All @@ -772,7 +768,7 @@ func (j *JIMM) RemoveCloudFromController(ctx context.Context, u *openfga.User, c
return errors.E(op, err)
}

isAdministrator, err := openfga.IsAdministrator(ctx, u, ct)
isAdministrator, err := openfga.IsAdministrator(ctx, user, ct)
if err != nil {
return errors.E(op, err, errors.CodeUnauthorized, "unauthorized")
}
Expand Down
4 changes: 2 additions & 2 deletions internal/jimm/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,8 @@ func TestForEachCloud(t *testing.T) {
&dbmodel.User{Username: "daphne@external"},
client,
)
err = daphne.SetControllerAccess(context.Background(), names.NewControllerTag(j.UUID), ofganames.AdministratorRelation)
c.Assert(err, qt.IsNil)
daphne.JimmAdmin = true

everyone := openfga.NewUser(
&dbmodel.User{
Username: ofganames.EveryoneUser,
Expand Down
30 changes: 8 additions & 22 deletions internal/jimm/cloudcredential.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,14 @@ import (
func (j *JIMM) GetCloudCredential(ctx context.Context, user *openfga.User, tag names.CloudCredentialTag) (*dbmodel.CloudCredential, error) {
const op = errors.Op("jimm.GetCloudCredential")

isJIMMAdmin, err := openfga.IsAdministrator(ctx, user, j.ResourceTag())
if err != nil {
return nil, errors.E(op, errors.CodeUnauthorized, "unauthorized")
}
if !isJIMMAdmin && user.Username != tag.Owner().Id() {
if !user.JimmAdmin && user.Username != tag.Owner().Id() {
return nil, errors.E(op, errors.CodeUnauthorized, "unauthorized")
}

var credential dbmodel.CloudCredential
credential.SetTag(tag)

err = j.Database.GetCloudCredential(ctx, &credential)
err := j.Database.GetCloudCredential(ctx, &credential)
if err != nil {
return nil, errors.E(op, err)
}
Expand Down Expand Up @@ -134,17 +130,13 @@ type UpdateCloudCredentialArgs struct {
// UpdateCloudCredential checks that the credential can be updated
// and updates it in the local database and all controllers
// to which it is deployed.
func (j *JIMM) UpdateCloudCredential(ctx context.Context, u *openfga.User, args UpdateCloudCredentialArgs) ([]jujuparams.UpdateCredentialModelResult, error) {
func (j *JIMM) UpdateCloudCredential(ctx context.Context, user *openfga.User, args UpdateCloudCredentialArgs) ([]jujuparams.UpdateCredentialModelResult, error) {
const op = errors.Op("jimm.UpdateCloudCredential")

var resultMu sync.Mutex
var result []jujuparams.UpdateCredentialModelResult
if u.Tag() != args.CredentialTag.Owner() {
isJIMMAdmin, err := openfga.IsAdministrator(ctx, u, j.ResourceTag())
if err != nil {
return result, errors.E(op, errors.CodeUnauthorized, "unauthorized")
}
if !isJIMMAdmin {
if user.Tag() != args.CredentialTag.Owner() {
if !user.JimmAdmin {
return result, errors.E(op, errors.CodeUnauthorized, "unauthorized")
}
// ensure the user we are adding the credential for exists.
Expand Down Expand Up @@ -324,22 +316,16 @@ func (j *JIMM) ForEachUserCloudCredential(ctx context.Context, u *dbmodel.User,
// returned. Only the credential owner can retrieve hidden attributes any
// other user, including controller superusers, will recieve an error with
// the code CodeUnauthorized.
func (j *JIMM) GetCloudCredentialAttributes(ctx context.Context, u *openfga.User, cred *dbmodel.CloudCredential, hidden bool) (attrs map[string]string, redacted []string, err error) {
func (j *JIMM) GetCloudCredentialAttributes(ctx context.Context, user *openfga.User, cred *dbmodel.CloudCredential, hidden bool) (attrs map[string]string, redacted []string, err error) {
const op = errors.Op("jimm.GetCloudCredentialAttributes")

isControllerAdmin, err := openfga.IsAdministrator(ctx, u, j.ResourceTag())
if err != nil {
zapctx.Error(ctx, "failed to check for controller admin access", zap.Error(err))
return nil, nil, errors.E(op, err)
}

if hidden {
// Controller superusers cannot read hidden credential attributes.
if u.Username != cred.OwnerUsername {
if user.Username != cred.OwnerUsername {
return nil, nil, errors.E(op, errors.CodeUnauthorized, "unauthorized")
}
} else {
if !isControllerAdmin && u.Username != cred.OwnerUsername {
if !user.JimmAdmin && user.Username != cred.OwnerUsername {
return nil, nil, errors.E(op, errors.CodeUnauthorized, "unauthorized")
}
}
Expand Down
30 changes: 23 additions & 7 deletions internal/jimm/cloudcredential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@ func TestUpdateCloudCredential(t *testing.T) {
about string
checkCredentialErrors []error
updateCredentialErrors []error
jimmAdmin bool
createEnv func(*qt.C, *jimm.JIMM, *openfga.OFGAClient) (*dbmodel.User, jimm.UpdateCloudCredentialArgs, dbmodel.CloudCredential, string)
}{{
about: "all ok",
about: "all ok",
jimmAdmin: true,
createEnv: func(c *qt.C, j *jimm.JIMM, client *openfga.OFGAClient) (*dbmodel.User, jimm.UpdateCloudCredentialArgs, dbmodel.CloudCredential, string) {
u := dbmodel.User{
Username: "alice@external",
Expand Down Expand Up @@ -153,6 +155,7 @@ func TestUpdateCloudCredential(t *testing.T) {
},
}, {
about: "update credential error returned by controller",
jimmAdmin: true,
updateCredentialErrors: []error{nil, errors.E("test error")},
createEnv: func(c *qt.C, j *jimm.JIMM, client *openfga.OFGAClient) (*dbmodel.User, jimm.UpdateCloudCredentialArgs, dbmodel.CloudCredential, string) {
u := dbmodel.User{
Expand Down Expand Up @@ -242,6 +245,7 @@ func TestUpdateCloudCredential(t *testing.T) {
},
}, {
about: "check credential error returned by controller",
jimmAdmin: true,
checkCredentialErrors: []error{errors.E("test error")},
updateCredentialErrors: []error{nil},
createEnv: func(c *qt.C, j *jimm.JIMM, client *openfga.OFGAClient) (*dbmodel.User, jimm.UpdateCloudCredentialArgs, dbmodel.CloudCredential, string) {
Expand Down Expand Up @@ -326,14 +330,16 @@ func TestUpdateCloudCredential(t *testing.T) {
return &u, arg, dbmodel.CloudCredential{}, "test error"
},
}, {
about: "user is controller superuser",
about: "user is controller superuser",
jimmAdmin: true,
createEnv: func(c *qt.C, j *jimm.JIMM, client *openfga.OFGAClient) (*dbmodel.User, jimm.UpdateCloudCredentialArgs, dbmodel.CloudCredential, string) {
u := dbmodel.User{
Username: "alice@external",
}
c.Assert(j.Database.DB.Create(&u).Error, qt.IsNil)

alice := openfga.NewUser(&u, client)
alice.JimmAdmin = true

err := alice.SetControllerAccess(context.Background(), j.ResourceTag(), ofganames.AdministratorRelation)
c.Assert(err, qt.IsNil)
Expand Down Expand Up @@ -445,6 +451,7 @@ func TestUpdateCloudCredential(t *testing.T) {
}, {
about: "skip check, which would return an error",
checkCredentialErrors: []error{errors.E("test error")},
jimmAdmin: true,
createEnv: func(c *qt.C, j *jimm.JIMM, client *openfga.OFGAClient) (*dbmodel.User, jimm.UpdateCloudCredentialArgs, dbmodel.CloudCredential, string) {
u := dbmodel.User{
Username: "alice@external",
Expand Down Expand Up @@ -556,7 +563,8 @@ func TestUpdateCloudCredential(t *testing.T) {
return &u, arg, expectedCredential, ""
},
}, {
about: "skip update",
about: "skip update",
jimmAdmin: true,
createEnv: func(c *qt.C, j *jimm.JIMM, client *openfga.OFGAClient) (*dbmodel.User, jimm.UpdateCloudCredentialArgs, dbmodel.CloudCredential, string) {
u := dbmodel.User{
Username: "alice@external",
Expand Down Expand Up @@ -783,6 +791,8 @@ func TestUpdateCloudCredential(t *testing.T) {

u, arg, expectedCredential, expectedError := test.createEnv(c, j, client)
user := openfga.NewUser(u, client)
user.JimmAdmin = test.jimmAdmin

result, err := j.UpdateCloudCredential(ctx, user, arg)
if expectedError == "" {
c.Assert(err, qt.Equals, nil)
Expand Down Expand Up @@ -837,6 +847,7 @@ users:
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)
u := env.User("alice@external").DBObject(c, j.Database)
user := openfga.NewUser(&u, client)
user.JimmAdmin = true
_, err = j.UpdateCloudCredential(ctx, user, jimm.UpdateCloudCredentialArgs{
CredentialTag: names.NewCloudCredentialTag("test-cloud/bob@external/test"),
Credential: jujuparams.CloudCredential{
Expand Down Expand Up @@ -1532,13 +1543,15 @@ var getCloudCredentialAttributesTests = []struct {
name string
username string
hidden bool
jimmAdmin bool
expectAttributes map[string]string
expectRedacted []string
expectError string
expectErrorCode errors.Code
}{{
name: "OwnerNoHidden",
username: "bob@external",
name: "OwnerNoHidden",
username: "bob@external",
jimmAdmin: true,
expectAttributes: map[string]string{
"client-email": "[email protected]",
"client-id": "1234",
Expand All @@ -1556,8 +1569,9 @@ var getCloudCredentialAttributesTests = []struct {
"project-id": "5678",
},
}, {
name: "SuperUserNoHidden",
username: "alice@external",
name: "SuperUserNoHidden",
username: "alice@external",
jimmAdmin: true,
expectAttributes: map[string]string{
"client-email": "[email protected]",
"client-id": "1234",
Expand All @@ -1568,6 +1582,7 @@ var getCloudCredentialAttributesTests = []struct {
name: "SuperUserWithHiddenUnauthorized",
username: "alice@external",
hidden: true,
jimmAdmin: true,
expectError: `unauthorized`,
expectErrorCode: errors.CodeUnauthorized,
}, {
Expand Down Expand Up @@ -1608,6 +1623,7 @@ func TestGetCloudCredentialAttributes(t *testing.T) {

u = env.User(test.username).DBObject(c, j.Database)
userTest := openfga.NewUser(&u, client)
userTest.JimmAdmin = test.jimmAdmin
attr, redacted, err := j.GetCloudCredentialAttributes(ctx, userTest, cred, test.hidden)
if test.expectError != "" {
c.Check(err, qt.ErrorMatches, test.expectError)
Expand Down
Loading