Skip to content

Commit

Permalink
CSS-6763 Introduce addServiceAccount method (#1127)
Browse files Browse the repository at this point in the history
* Introduce AddServiceAccount method

* Fixed auth model and added tests

* Update copyright date

* PR changes

* Correctly rename field

* Check if user has permission over service account

* Update service_account_test.go
  • Loading branch information
kian99 authored Jan 17, 2024
1 parent f9b3ef0 commit a996277
Show file tree
Hide file tree
Showing 9 changed files with 207 additions and 2 deletions.
8 changes: 8 additions & 0 deletions api/params/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,3 +393,11 @@ type MigrateModelInfo struct {
type MigrateModelRequest struct {
Specs []MigrateModelInfo `json:"specs"`
}

// Service Account related request parameters

// AddServiceAccountRequest holds a request to add a service account.
type AddServiceAccountRequest struct {
// ClientID holds the client id of the service account.
ClientID string `json:"client-id"`
}
53 changes: 53 additions & 0 deletions internal/jimm/service_account.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright 2024 Canonical Ltd.

package jimm

import (
"context"

"github.com/canonical/jimm/internal/errors"
"github.com/canonical/jimm/internal/openfga"
ofganames "github.com/canonical/jimm/internal/openfga/names"
jimmnames "github.com/canonical/jimm/pkg/names"
)

// AddServiceAccount checks that no one owns the service account yet
// and then adds a relation between the logged in user and the service account.
func (j *JIMM) AddServiceAccount(ctx context.Context, u *openfga.User, clientId string) error {
op := errors.Op("jimm.AddServiceAccount")
svcTag := jimmnames.NewServiceAccountTag(clientId)
key := openfga.Tuple{
Relation: ofganames.AdministratorRelation,
Target: ofganames.ConvertTag(svcTag),
}
keyWithUser := key
keyWithUser.Object = ofganames.ConvertTag(u.ResourceTag())

ok, err := j.OpenFGAClient.CheckRelation(ctx, keyWithUser, false)
if err != nil {
return errors.E(op, err)
}
// If the user already has administration permission over the
// service account then return early.
if ok {
return nil
}

tuples, _, err := j.OpenFGAClient.ReadRelatedObjects(ctx, key, 10, "")
if err != nil {
return errors.E(op, err)
}
if len(tuples) > 0 {
return errors.E(op, "service account already owned")
}
addTuple := openfga.Tuple{
Object: ofganames.ConvertTag(u.ResourceTag()),
Relation: ofganames.AdministratorRelation,
Target: ofganames.ConvertTag(svcTag),
}
err = j.AuthorizationClient().AddRelation(ctx, addTuple)
if err != nil {
return errors.E(op, err)
}
return nil
}
48 changes: 48 additions & 0 deletions internal/jimm/service_account_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright 2024 Canonical Ltd.

package jimm_test

import (
"context"
"testing"

qt "github.com/frankban/quicktest"

"github.com/canonical/jimm/internal/dbmodel"
"github.com/canonical/jimm/internal/jimm"
"github.com/canonical/jimm/internal/jimmtest"
"github.com/canonical/jimm/internal/openfga"
)

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

ctx := context.Background()
client, _, _, err := jimmtest.SetupTestOFGAClient(c.Name())
c.Assert(err, qt.IsNil)
j := &jimm.JIMM{
OpenFGAClient: client,
}
c.Assert(err, qt.IsNil)
user := openfga.NewUser(
&dbmodel.User{
Username: "bob@external",
DisplayName: "Bob",
},
client,
)
clientID := "39caae91-b914-41ae-83f8-c7b86ca5ad5a"
err = j.AddServiceAccount(ctx, user, clientID)
c.Assert(err, qt.IsNil)
err = j.AddServiceAccount(ctx, user, clientID)
c.Assert(err, qt.IsNil)
userAlice := openfga.NewUser(
&dbmodel.User{
Username: "alive@external",
DisplayName: "Alice",
},
client,
)
err = j.AddServiceAccount(ctx, userAlice, clientID)
c.Assert(err, qt.ErrorMatches, "service account already owned")
}
9 changes: 9 additions & 0 deletions internal/jimmtest/jimm_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type JIMM struct {
AddController_ func(ctx context.Context, u *openfga.User, ctl *dbmodel.Controller) error
AddHostedCloud_ func(ctx context.Context, user *openfga.User, tag names.CloudTag, cloud jujuparams.Cloud, force bool) error
AddModel_ func(ctx context.Context, u *openfga.User, args *jimm.ModelCreateArgs) (*jujuparams.ModelInfo, error)
AddServiceAccount_ func(ctx context.Context, u *openfga.User, clientId string) error
Authenticate_ func(ctx context.Context, req *jujuparams.LoginRequest) (*openfga.User, error)
AuthorizationClient_ func() *openfga.OFGAClient
ChangeModelCredential_ func(ctx context.Context, user *openfga.User, modelTag names.ModelTag, cloudCredentialTag names.CloudCredentialTag) error
Expand Down Expand Up @@ -130,6 +131,14 @@ func (j *JIMM) AddModel(ctx context.Context, u *openfga.User, args *jimm.ModelCr
}
return j.AddModel_(ctx, u, args)
}

func (j *JIMM) AddServiceAccount(ctx context.Context, u *openfga.User, clientId string) error {
if j.AddServiceAccount_ == nil {
return errors.E(errors.CodeNotImplemented)
}
return j.AddServiceAccount_(ctx, u, clientId)
}

func (j *JIMM) Authenticate(ctx context.Context, req *jujuparams.LoginRequest) (*openfga.User, error) {
if j.Authenticate_ == nil {
return nil, errors.E(errors.CodeNotImplemented)
Expand Down
1 change: 1 addition & 0 deletions internal/jujuapi/controllerroot.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type JIMM interface {
AddController(ctx context.Context, u *openfga.User, ctl *dbmodel.Controller) error
AddHostedCloud(ctx context.Context, user *openfga.User, tag names.CloudTag, cloud jujuparams.Cloud, force bool) error
AddModel(ctx context.Context, u *openfga.User, args *jimm.ModelCreateArgs) (_ *jujuparams.ModelInfo, err error)
AddServiceAccount(ctx context.Context, u *openfga.User, clientId string) error
Authenticate(ctx context.Context, req *jujuparams.LoginRequest) (*openfga.User, error)
AuthorizationClient() *openfga.OFGAClient
ChangeModelCredential(ctx context.Context, user *openfga.User, modelTag names.ModelTag, cloudCredentialTag names.CloudCredentialTag) error
Expand Down
3 changes: 3 additions & 0 deletions internal/jujuapi/jimm.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func init() {
crossModelQueryMethod := rpc.Method(r.CrossModelQuery)
purgeLogsMethod := rpc.Method(r.PurgeLogs)
migrateModel := rpc.Method(r.MigrateModel)
addServiceAccountMethod := rpc.Method(r.AddServiceAccount)

// JIMM Generic RPC
r.AddMethod("JIMM", 4, "AddController", addControllerMethod)
Expand Down Expand Up @@ -77,6 +78,8 @@ func init() {
r.AddMethod("JIMM", 4, "ListRelationshipTuples", listRelationshipTuplesMethod)
// JIMM Cross-model queries
r.AddMethod("JIMM", 4, "CrossModelQuery", crossModelQueryMethod)
// JIMM Service Accounts
r.AddMethod("JIMM", 4, "AddServiceAccount", addServiceAccountMethod)

return []int{4}
}
Expand Down
24 changes: 24 additions & 0 deletions internal/jujuapi/service_account.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2024 canonical.

package jujuapi

import (
"context"

apiparams "github.com/canonical/jimm/api/params"
"github.com/canonical/jimm/internal/errors"
jimmnames "github.com/canonical/jimm/pkg/names"
)

// service_acount contains the primary RPC commands for handling service accounts within JIMM via the JIMM facade itself.

// AddGroup creates a group within JIMMs DB for reference by OpenFGA.
func (r *controllerRoot) AddServiceAccount(ctx context.Context, req apiparams.AddServiceAccountRequest) error {
const op = errors.Op("jujuapi.AddGroup")

if !jimmnames.IsValidServiceAccountId(req.ClientID) {
return errors.E(op, errors.CodeBadRequest, "invalid client ID")
}

return r.jimm.AddServiceAccount(ctx, r.user, req.ClientID)
}
59 changes: 59 additions & 0 deletions internal/jujuapi/service_account_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Copyright 2024 Canonical Ltd.

package jujuapi_test

import (
"context"
"testing"

"github.com/canonical/jimm/api/params"
"github.com/canonical/jimm/internal/jimmtest"
"github.com/canonical/jimm/internal/jujuapi"
"github.com/canonical/jimm/internal/openfga"
qt "github.com/frankban/quicktest"
)

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

tests := []struct {
about string
addServiceAccount func(ctx context.Context, user *openfga.User, clientID string) error
args params.AddServiceAccountRequest
expectedError string
}{{
about: "Valid client ID",
addServiceAccount: func(ctx context.Context, user *openfga.User, clientID string) error {
return nil
},
args: params.AddServiceAccountRequest{
ClientID: "fca1f605-736e-4d1f-bcd2-aecc726923be",
},
}, {
about: "Invalid Client ID",
addServiceAccount: func(ctx context.Context, user *openfga.User, clientID string) error {
return nil
},
args: params.AddServiceAccountRequest{
ClientID: "_123_",
},
expectedError: "invalid client ID",
}}

for _, test := range tests {
test := test
c.Run(test.about, func(c *qt.C) {
jimm := &jimmtest.JIMM{
AddServiceAccount_: test.addServiceAccount,
}
cr := jujuapi.NewControllerRoot(jimm, jujuapi.Params{})

err := cr.AddServiceAccount(context.Background(), test.args)
if test.expectedError == "" {
c.Assert(err, qt.IsNil)
} else {
c.Assert(err, qt.ErrorMatches, test.expectedError)
}
})
}
}
4 changes: 2 additions & 2 deletions local/openfga/authorisation_model.json
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@
{
"metadata": {
"relations": {
"administator": {
"administrator": {
"directly_related_user_types": [
{
"type": "user"
Expand All @@ -438,7 +438,7 @@
}
},
"relations": {
"administator": {
"administrator": {
"this": {}
}
},
Expand Down

0 comments on commit a996277

Please sign in to comment.