From 58d722cd726aee17247e285eed139018c1ed40bc Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Tue, 14 Jan 2025 15:29:04 +0200 Subject: [PATCH 1/3] feat: add keyManager handler to model proxy The model proxy will intercept calls to the keyManager facade and persist user keys in JIMM rather than passing these calls along to the Juju controller. This is being done in order to support the SSH proxy efforts. Ideally, in Juju 4 these methods would be done on the controller api and all this logic would move into the jujuapi package. --- internal/jimm/jimm.go | 6 + internal/jujuapi/websocket.go | 1 + internal/rpcproxy/export_test.go | 5 +- internal/rpcproxy/rpcproxy.go | 96 +++++++++++++++ internal/rpcproxy/rpcproxy_test.go | 144 +++++++++++++++++++++++ internal/rpcproxy/rpcproxylogin_test.go | 2 + internal/rpcproxy/sshkeys.go | 123 ++++++++++++++++++++ internal/rpcproxy/sshkeys_test.go | 148 ++++++++++++++++++++++++ 8 files changed, 524 insertions(+), 1 deletion(-) create mode 100644 internal/rpcproxy/sshkeys.go create mode 100644 internal/rpcproxy/sshkeys_test.go diff --git a/internal/jimm/jimm.go b/internal/jimm/jimm.go index c7e2225cc..eda6a45b0 100644 --- a/internal/jimm/jimm.go +++ b/internal/jimm/jimm.go @@ -491,6 +491,12 @@ func (j *JIMM) ServiceAccountManager() ServiceAccountManager { return j.serviceAccountManager } +// SSHKeyManager returns a manager that enables operations +// related to ssh keys. +func (j *JIMM) SSHKeyManager() SSHKeyManager { + return j.sshKeyManager +} + type permission struct { resource string relation string diff --git a/internal/jujuapi/websocket.go b/internal/jujuapi/websocket.go index 325a0ed34..13cd8d813 100644 --- a/internal/jujuapi/websocket.go +++ b/internal/jujuapi/websocket.go @@ -185,6 +185,7 @@ func (s apiProxier) ServeWS(ctx context.Context, clientConn *websocket.Conn) { AuditLog: auditLogger, LoginService: s.jimm.LoginManager(), AuthenticatedIdentityID: auth.SessionIdentityFromContext(ctx), + SSHKeyManager: s.jimm.SSHKeyManager(), } if err := rpcproxy.ProxySockets(ctx, proxyHelpers); err != nil { zapctx.Error(ctx, "failed to start jimm model proxy", zap.Error(err)) diff --git a/internal/rpcproxy/export_test.go b/internal/rpcproxy/export_test.go index a58cb9249..0508f0985 100644 --- a/internal/rpcproxy/export_test.go +++ b/internal/rpcproxy/export_test.go @@ -2,4 +2,7 @@ package rpcproxy -type Message = message +type ( + Message = message + KeyManagerFacade = keyManagerFacade +) diff --git a/internal/rpcproxy/rpcproxy.go b/internal/rpcproxy/rpcproxy.go index e3375f0d4..f48ad7f32 100644 --- a/internal/rpcproxy/rpcproxy.go +++ b/internal/rpcproxy/rpcproxy.go @@ -22,6 +22,7 @@ import ( "github.com/canonical/jimm/v3/internal/dbmodel" "github.com/canonical/jimm/v3/internal/errors" + "github.com/canonical/jimm/v3/internal/jimm/sshkeys" "github.com/canonical/jimm/v3/internal/openfga" "github.com/canonical/jimm/v3/internal/servermon" "github.com/canonical/jimm/v3/internal/utils" @@ -32,6 +33,18 @@ const ( accessRequiredErrorCode = "access required" ) +// SSHKeyManager is an interface for managing SSH keys. +type SSHKeyManager interface { + // AddUserPublicKey saves a user's public key. + AddUserPublicKey(ctx context.Context, user *openfga.User, publicKey sshkeys.PublicKey) error + // ListUserPublicKeys lists a user's public keys. + ListUserPublicKeys(ctx context.Context, user *openfga.User) ([]sshkeys.PublicKey, error) + // RemoveUserKeyByComment removes a user's public key(s) by the key comment. + RemoveUserKeyByComment(ctx context.Context, user *openfga.User, comment string) error + // RemoveUserKeyByFingerprint removes a user's public key(s) by the key fingerprint. + RemoveUserKeyByFingerprint(ctx context.Context, user *openfga.User, fingerprint string) error +} + // TokenGenerator authenticates a user and generates a JWT token. type TokenGenerator interface { // MakeLoginToken returns a JWT containing claims about user's access @@ -77,6 +90,7 @@ type LoginService interface { // connection to a model. type ProxyHelpers struct { ConnClient WebsocketConnection + SSHKeyManager SSHKeyManager TokenGen TokenGenerator ConnectController func(context.Context) (WebsocketConnectionWithMetadata, error) AuditLog func(*dbmodel.AuditLogEntry) @@ -101,6 +115,10 @@ func ProxySockets(ctx context.Context, helpers ProxyHelpers) error { zapctx.Error(ctx, "Missing login service function") return errors.E(op, "Missing login service function") } + if helpers.SSHKeyManager == nil { + zapctx.Error(ctx, "Missing ssh key manager function") + return errors.E(op, "Missing ssh key manager function") + } errChan := make(chan error, 2) msgInFlight := inflightMsgs{messages: make(map[uint64]*message)} client := writeLockConn{conn: helpers.ConnClient} @@ -113,6 +131,7 @@ func ProxySockets(ctx context.Context, helpers ProxyHelpers) error { tokenGen: helpers.TokenGen, auditLog: helpers.AuditLog, conversationId: utils.NewConversationID(), + sshKeyManager: helpers.SSHKeyManager, loginService: helpers.LoginService, authenticatedIdentityID: helpers.AuthenticatedIdentityID, }, @@ -247,6 +266,7 @@ type modelProxy struct { msgs *inflightMsgs auditLog func(*dbmodel.AuditLogEntry) tokenGen TokenGenerator + sshKeyManager SSHKeyManager loginService LoginService modelName string conversationId string @@ -331,6 +351,7 @@ func unexpectedReadError(err error) bool { // clientProxy proxies messages from client->controller. type clientProxy struct { modelProxy + user *openfga.User wg sync.WaitGroup errChan chan error createControllerConn func(context.Context) (WebsocketConnectionWithMetadata, error) @@ -384,6 +405,19 @@ func (p *clientProxy) start(ctx context.Context) error { p.msgs.addLoginMessage(toController) } } + // This is a special case for the KeyManager facade. We handle it here + // because it is a model level facade. In Juju 4 we want to move this + // to a controller level facade and place the logic in jujuapi. + if msg.Type == "KeyManager" { + zapctx.Debug(ctx, "handling a KeyManager facade call") + toClient, err := p.handleKeyManagerFacade(ctx, msg) + if err != nil { + p.sendError(p.src, msg, err) + continue + } + p.src.sendMessage(nil, toClient) + continue + } p.msgs.addMessage(msg) zapctx.Debug(ctx, "Writing to controller") if err := p.dst.writeJson(msg); err != nil { @@ -624,6 +658,8 @@ func (p *clientProxy) handleAdminFacade(ctx context.Context, msg *message) (clie return nil, nil, err } controllerLoginMessageFnc := func(user *openfga.User) (*message, *message, error) { + // Store the logged in user for use with SSH key manager methods. + p.user = user jwt, err := p.tokenGen.MakeLoginToken(ctx, user) if err != nil { return errorFnc(err) @@ -710,3 +746,63 @@ func (p *clientProxy) handleAdminFacade(ctx context.Context, msg *message) (clie return nil, nil, nil } } + +// handleKeyManagerFacade processes the key manager facade call. +func (p *clientProxy) handleKeyManagerFacade(ctx context.Context, msg *message) (clientResponse *message, err error) { + if p.user == nil { + return nil, errors.E("user not authenticated") + } + clientRespF := func(data any) (*message, error) { + resp, err := json.Marshal(data) + if err != nil { + return nil, err + } + msg.Response = resp + return msg, nil + } + keyManager := keyManagerFacade{SSHKeyManager: p.sshKeyManager, user: p.user} + + switch msg.Request { + case "ListKeys": + var request params.ListSSHKeys + err := json.Unmarshal(msg.Params, &request) + if err != nil { + return nil, err + } + res, err := keyManager.ListKeys(ctx, request) + if err != nil { + return nil, err + } + return clientRespF(res) + + case "AddKeys": + var request params.ModifyUserSSHKeys + err := json.Unmarshal(msg.Params, &request) + if err != nil { + return nil, err + } + res, err := keyManager.AddKeys(ctx, request) + if err != nil { + return nil, err + } + return clientRespF(res) + + case "DeleteKeys": + var request params.ModifyUserSSHKeys + err := json.Unmarshal(msg.Params, &request) + if err != nil { + return nil, err + } + res, err := keyManager.DeleteKeys(ctx, request) + if err != nil { + return nil, err + } + return clientRespF(res) + + case "ImportKeys": + return nil, errors.E("ImportKeys not implemented", errors.CodeNotImplemented) + + default: + return nil, errors.E("unknown key manager request") + } +} diff --git a/internal/rpcproxy/rpcproxy_test.go b/internal/rpcproxy/rpcproxy_test.go index 03030e710..67566c960 100644 --- a/internal/rpcproxy/rpcproxy_test.go +++ b/internal/rpcproxy/rpcproxy_test.go @@ -10,11 +10,15 @@ import ( qt "github.com/frankban/quicktest" "github.com/gorilla/websocket" + jujuparams "github.com/juju/juju/rpc/params" "github.com/juju/names/v5" + "github.com/juju/utils/v3/ssh" "github.com/canonical/jimm/v3/internal/dbmodel" + "github.com/canonical/jimm/v3/internal/jimm/sshkeys" "github.com/canonical/jimm/v3/internal/openfga" "github.com/canonical/jimm/v3/internal/rpcproxy" + "github.com/canonical/jimm/v3/internal/testutils/jimmtest/mocks" "github.com/canonical/jimm/v3/internal/testutils/rpctest" ) @@ -58,6 +62,7 @@ func TestProxySockets(t *testing.T) { ConnectController: f, AuditLog: auditLogger, LoginService: &mockLoginService{}, + SSHKeyManager: &mocks.SSHKeyManager{}, } err := rpcproxy.ProxySockets(ctx, proxyHelpers) c.Check(err, qt.IsNil) @@ -117,6 +122,7 @@ func TestProxySocketsControllerConnectionFails(t *testing.T) { ConnectController: f, AuditLog: auditLogger, LoginService: &mockLoginService{}, + SSHKeyManager: &mocks.SSHKeyManager{}, } err := rpcproxy.ProxySockets(ctx, proxyHelpers) c.Check(err, qt.IsNil) @@ -176,6 +182,7 @@ func TestCancelProxySockets(t *testing.T) { ConnectController: f, AuditLog: auditLogger, LoginService: &mockLoginService{}, + SSHKeyManager: &mocks.SSHKeyManager{}, } err := rpcproxy.ProxySockets(ctx, proxyHelpers) c.Check(err, qt.ErrorMatches, "Context cancelled") @@ -217,6 +224,7 @@ func TestProxySocketsAuditLogs(t *testing.T) { ConnectController: f, AuditLog: auditLogger, LoginService: &mockLoginService{}, + SSHKeyManager: &mocks.SSHKeyManager{}, } err := rpcproxy.ProxySockets(ctx, proxyHelpers) c.Check(err, qt.IsNil) @@ -272,3 +280,139 @@ func TestProxySocketsAuditLogs(t *testing.T) { c.Assert(auditLogs, qt.DeepEquals, expectedEvents) } + +func TestProxySocketsSSHKeys(t *testing.T) { + c := qt.New(t) + + ctx := context.Background() + sshFacadeChan := make(chan (string), 1) + + srvController := rpctest.NewServer(rpctest.Echo) + + errChan := make(chan error) + srvJIMM := rpctest.NewServer(func(connClient *websocket.Conn) error { + defer connClient.Close() + testTokenGen := testTokenGenerator{} + connectControllerF := func(context.Context) (rpcproxy.WebsocketConnectionWithMetadata, error) { + connController := srvController.Dialer.DialWebsocket(c, srvController.URL) + return rpcproxy.WebsocketConnectionWithMetadata{ + Conn: connController, + ModelName: "TestModelName", + }, nil + } + proxyHelpers := rpcproxy.ProxyHelpers{ + ConnClient: connClient, + TokenGen: &testTokenGen, + ConnectController: connectControllerF, + AuditLog: func(ale *dbmodel.AuditLogEntry) {}, + LoginService: &mockLoginService{ + email: "alice@canonical.com", + }, + SSHKeyManager: &mocks.SSHKeyManager{ + AddUserPublicKey_: func(ctx context.Context, user *openfga.User, publicKey sshkeys.PublicKey) error { + sshFacadeChan <- "add-keys" + return nil + }, + ListUserPublicKeys_: func(ctx context.Context, user *openfga.User) ([]sshkeys.PublicKey, error) { + sshFacadeChan <- "list-keys" + return nil, nil + }, + RemoveUserKeyByComment_: func(ctx context.Context, user *openfga.User, comment string) error { + sshFacadeChan <- "remove-keys-comment" + return nil + }, + RemoveUserKeyByFingerprint_: func(ctx context.Context, user *openfga.User, fingerprint string) error { + sshFacadeChan <- "remove-keys-fingerprint" + return nil + }, + }, + } + err := rpcproxy.ProxySockets(ctx, proxyHelpers) + c.Check(err, qt.IsNil) + errChan <- err + return err + }) + + defer srvController.Close() + defer srvJIMM.Close() + ws := srvJIMM.Dialer.DialWebsocket(c, srvJIMM.URL) + defer ws.Close() + + // Perform login + p := json.RawMessage(`{"Key":"TestVal"}`) + msg := rpcproxy.Message{RequestID: 1, Type: "Admin", Request: "LoginWithSessionToken", Params: p} // #nosec G115 accept integer conversion + err := ws.WriteJSON(&msg) + c.Assert(err, qt.IsNil) + resp := rpcproxy.Message{} + err = ws.ReadJSON(&resp) + c.Assert(err, qt.IsNil) + c.Assert(resp.Error, qt.Equals, "") + + // Run sub-tests for all SSH Key methods + tests := []struct { + name string + request string + params []byte + expectedChanResult string + expectedErr string + }{ + { + name: "Add key method", + request: "AddKeys", + expectedChanResult: "add-keys", + params: mustMarshal(jujuparams.ModifyUserSSHKeys{Keys: []string{"type key comment"}}), + }, + { + name: "List keys method", + request: "ListKeys", + expectedChanResult: "list-keys", + params: mustMarshal(jujuparams.ListSSHKeys{Mode: ssh.Fingerprints}), + }, + { + name: "Delete keys by comment", + request: "DeleteKeys", + expectedChanResult: "remove-keys-comment", + params: mustMarshal(jujuparams.ModifyUserSSHKeys{Keys: []string{"comment"}}), + }, + { + name: "Delete keys by fingerprint", + request: "DeleteKeys", + expectedChanResult: "remove-keys-fingerprint", + params: mustMarshal(jujuparams.ModifyUserSSHKeys{Keys: []string{"79:fc:60:93:ec:ce:42:fe:15:61:f2:fb:d6:22:43:6e"}}), + }, + } + + for i, test := range tests { + c.Run(test.name, func(c *qt.C) { + msg := rpcproxy.Message{RequestID: uint64(i + 1), Type: "KeyManager", Request: test.request, Params: test.params} // #nosec G115 accept integer conversion + err := ws.WriteJSON(&msg) + c.Assert(err, qt.IsNil) + + resp := rpcproxy.Message{} + err = ws.ReadJSON(&resp) + c.Assert(err, qt.IsNil) + if test.expectedErr == "" { + c.Assert(resp.Error, qt.Equals, "") + } else { + c.Assert(err, qt.Matches, test.expectedErr) + } + + select { + case res := <-sshFacadeChan: + c.Assert(res, qt.Equals, test.expectedChanResult) + case <-time.After(100 * time.Millisecond): + c.Error("Expected SSH method was not called") + } + }) + } + ws.Close() + <-errChan // Ensure go routines are cleaned up +} + +func mustMarshal(data any) []byte { + out, err := json.Marshal(data) + if err != nil { + panic(err) + } + return out +} diff --git a/internal/rpcproxy/rpcproxylogin_test.go b/internal/rpcproxy/rpcproxylogin_test.go index 393de8c81..3c0fdeb43 100644 --- a/internal/rpcproxy/rpcproxylogin_test.go +++ b/internal/rpcproxy/rpcproxylogin_test.go @@ -20,6 +20,7 @@ import ( "github.com/canonical/jimm/v3/internal/errors" "github.com/canonical/jimm/v3/internal/openfga" "github.com/canonical/jimm/v3/internal/rpcproxy" + "github.com/canonical/jimm/v3/internal/testutils/jimmtest/mocks" apiparams "github.com/canonical/jimm/v3/pkg/api/params" jimmnames "github.com/canonical/jimm/v3/pkg/names" ) @@ -261,6 +262,7 @@ func TestProxySocketsAdminFacade(t *testing.T) { AuditLog: func(*dbmodel.AuditLogEntry) {}, LoginService: loginSvc, AuthenticatedIdentityID: test.authenticateEntityID, + SSHKeyManager: &mocks.SSHKeyManager{}, } var wg sync.WaitGroup wg.Add(1) diff --git a/internal/rpcproxy/sshkeys.go b/internal/rpcproxy/sshkeys.go new file mode 100644 index 000000000..2a995bf63 --- /dev/null +++ b/internal/rpcproxy/sshkeys.go @@ -0,0 +1,123 @@ +// Copyright 2025 Canonical. + +package rpcproxy + +import ( + "bytes" + "context" + "encoding/base64" + "fmt" + "regexp" + + jujuparams "github.com/juju/juju/rpc/params" + "github.com/juju/utils/v3/ssh" + gossh "golang.org/x/crypto/ssh" + + "github.com/canonical/jimm/v3/internal/jimm/sshkeys" + "github.com/canonical/jimm/v3/internal/openfga" +) + +var isFingerprintRegexp = regexp.MustCompile("^[0-9a-f]{2}(:[0-9a-f]{2}){15}$") + +// keyManagerFacade is intended to be a temporary struct used to emulate logic +// that will eventually live in jujuapi. This struct contains all +// the api layer logic for SSH key management methods that are currently +// used by the rpcProxy. +type keyManagerFacade struct { + SSHKeyManager + user *openfga.User +} + +func (s *keyManagerFacade) ListKeys(ctx context.Context, args jujuparams.ListSSHKeys) (jujuparams.StringsResults, error) { + keys, err := s.ListUserPublicKeys(ctx, s.user) + if err != nil { + return jujuparams.StringsResults{}, err + } + + var formatter func(key sshkeys.PublicKey) string + switch args.Mode { + case ssh.FullKeys: + formatter = marshalAuthorizedKeyWithComment + case ssh.Fingerprints: + formatter = fingerprintWithComment + default: + return jujuparams.StringsResults{}, fmt.Errorf("unknown mode (%v)", args.Mode) + } + + // The Juju CLI takes the first element from the slice of stringsResults. + res := jujuparams.StringsResult{} + for _, key := range keys { + res.Result = append(res.Result, formatter(key)) + } + return jujuparams.StringsResults{Results: []jujuparams.StringsResult{res}}, nil +} + +func (s *keyManagerFacade) AddKeys(ctx context.Context, args jujuparams.ModifyUserSSHKeys) (jujuparams.ErrorResults, error) { + var res []jujuparams.ErrorResult + errF := func(err error, msg string) jujuparams.ErrorResult { + return jujuparams.ErrorResult{Error: &jujuparams.Error{ + Message: fmt.Sprintf("%s: %s", msg, err.Error()), + }} + } + + for i, key := range args.Keys { + out, comment, _, _, err := gossh.ParseAuthorizedKey([]byte(key)) + if err != nil { + res = append(res, errF(err, fmt.Sprintf("Failed to parse key (entry %d)", i))) + } + jimmKey := sshkeys.PublicKey{ + PublicKey: out, + Comment: comment, + } + if err := s.AddUserPublicKey(ctx, s.user, jimmKey); err != nil { + res = append(res, errF(err, fmt.Sprintf("Failed to add key (comment %s)", comment))) + } + } + + return jujuparams.ErrorResults{Results: res}, nil +} + +func (s *keyManagerFacade) DeleteKeys(ctx context.Context, args jujuparams.ModifyUserSSHKeys) (jujuparams.ErrorResults, error) { + var res []jujuparams.ErrorResult + errF := func(err error, msg string) jujuparams.ErrorResult { + return jujuparams.ErrorResult{Error: &jujuparams.Error{ + Message: fmt.Sprintf("%s: %s", msg, err.Error()), + }} + } + + for _, key := range args.Keys { + if isFingerprintRegexp.MatchString(key) { + err := s.RemoveUserKeyByFingerprint(ctx, s.user, key) + if err != nil { + res = append(res, errF(err, fmt.Sprintf("Failed to remove key by fingerprint (%s)", key))) + } + } else { + err := s.RemoveUserKeyByComment(ctx, s.user, key) + if err != nil { + res = append(res, errF(err, fmt.Sprintf("Failed to remove key by comment (%s)", key))) + } + } + } + + return jujuparams.ErrorResults{Results: res}, nil +} + +func marshalAuthorizedKeyWithComment(key sshkeys.PublicKey) string { + // Copied from gossh.MarshalAuthorizedKey with an addition for the comment. + // Errors from the buffer's Write..() methods are always nil. + b := &bytes.Buffer{} + b.WriteString(key.Type()) + b.WriteByte(' ') + e := base64.NewEncoder(base64.StdEncoding, b) + _, _ = e.Write(key.Marshal()) + e.Close() + b.WriteByte(' ') + b.WriteString(key.Comment) + b.WriteByte('\n') + return b.String() +} + +func fingerprintWithComment(key sshkeys.PublicKey) string { + fingerprint := gossh.FingerprintLegacyMD5(key) + return fmt.Sprintf("%s (%s)", fingerprint, key.Comment) +} diff --git a/internal/rpcproxy/sshkeys_test.go b/internal/rpcproxy/sshkeys_test.go new file mode 100644 index 000000000..990e29945 --- /dev/null +++ b/internal/rpcproxy/sshkeys_test.go @@ -0,0 +1,148 @@ +// Copyright 2025 Canonical. + +package rpcproxy_test + +import ( + "bytes" + "context" + "crypto/rand" + "crypto/rsa" + "encoding/base64" + "fmt" + "regexp" + "testing" + + qt "github.com/frankban/quicktest" + "github.com/frankban/quicktest/qtsuite" + "github.com/juju/juju/rpc/params" + "github.com/juju/utils/v3/ssh" + gossh "golang.org/x/crypto/ssh" + + "github.com/canonical/jimm/v3/internal/jimm/sshkeys" + "github.com/canonical/jimm/v3/internal/openfga" + "github.com/canonical/jimm/v3/internal/rpcproxy" + "github.com/canonical/jimm/v3/internal/testutils/jimmtest/mocks" +) + +type keyManagerFacadeSuite struct { + keyManagerFacade rpcproxy.KeyManagerFacade + key1 sshkeys.PublicKey + key2 sshkeys.PublicKey + addKeyF func(ctx context.Context, user *openfga.User, publicKey sshkeys.PublicKey) error + removeKeyByCommentF func(ctx context.Context, user *openfga.User, comment string) error + removeKeyByFingerprintF func(ctx context.Context, user *openfga.User, fingerprint string) error +} + +func (k *keyManagerFacadeSuite) Init(c *qt.C) { + pk1, err := rsa.GenerateKey(rand.Reader, 2048) + c.Assert(err, qt.IsNil) + pk2, err := rsa.GenerateKey(rand.Reader, 2048) + c.Assert(err, qt.IsNil) + + pubKey1, err := gossh.NewPublicKey(&pk1.PublicKey) + c.Assert(err, qt.IsNil) + + pubKey2, err := gossh.NewPublicKey(&pk2.PublicKey) + c.Assert(err, qt.IsNil) + + k.key1 = sshkeys.PublicKey{ + PublicKey: pubKey1, + Comment: "comment-1", + } + + k.key2 = sshkeys.PublicKey{ + PublicKey: pubKey2, + Comment: "comment-2", + } + + keyManager := mocks.SSHKeyManager{ + ListUserPublicKeys_: func(ctx context.Context, user *openfga.User) ([]sshkeys.PublicKey, error) { + return []sshkeys.PublicKey{k.key1, k.key2}, nil + }, + AddUserPublicKey_: func(ctx context.Context, user *openfga.User, publicKey sshkeys.PublicKey) error { + return k.addKeyF(ctx, user, publicKey) + }, + RemoveUserKeyByComment_: func(ctx context.Context, user *openfga.User, comment string) error { + return k.removeKeyByCommentF(ctx, user, comment) + }, + RemoveUserKeyByFingerprint_: func(ctx context.Context, user *openfga.User, fingerprint string) error { + return k.removeKeyByFingerprintF(ctx, user, fingerprint) + }, + } + k.keyManagerFacade = rpcproxy.KeyManagerFacade{ + SSHKeyManager: &keyManager, + } +} + +var isFingerprintRegex = regexp.MustCompile(`[0-9a-f]{2}(:[0-9a-f]{2}){15}`) + +func (k *keyManagerFacadeSuite) TestListKeysShort(c *qt.C) { + c.Parallel() + ctx := context.Background() + + res, err := k.keyManagerFacade.ListKeys(ctx, params.ListSSHKeys{Mode: ssh.Fingerprints}) + c.Assert(err, qt.IsNil) + + c.Assert(res.Results[0].Result, qt.HasLen, 2) + c.Assert(res.Results[0].Result[0], qt.Matches, `.+ \(comment-1\)`) + c.Assert(isFingerprintRegex.MatchString(res.Results[0].Result[0]), qt.IsTrue) + c.Assert(res.Results[1].Result[1], qt.Matches, `.+ \(comment-2\)`) + c.Assert(isFingerprintRegex.MatchString(res.Results[0].Result[1]), qt.IsTrue) +} + +func (k *keyManagerFacadeSuite) TestListKeysFull(c *qt.C) { + c.Parallel() + ctx := context.Background() + + res, err := k.keyManagerFacade.ListKeys(ctx, params.ListSSHKeys{Mode: ssh.FullKeys}) + c.Assert(err, qt.IsNil) + + c.Assert(res.Results[0].Result, qt.HasLen, 2) + c.Assert(res.Results[0].Result[0], qt.Matches, `ssh-rsa .+ comment-1\n`) + c.Assert(res.Results[0].Result[1], qt.Matches, `ssh-rsa .+ comment-2\n`) +} + +func (k *keyManagerFacadeSuite) TestAddKeys(c *qt.C) { + c.Parallel() + ctx := context.Background() + + k.addKeyF = func(ctx context.Context, user *openfga.User, publicKey sshkeys.PublicKey) error { + c.Check(publicKey.Marshal(), qt.DeepEquals, k.key1.Marshal()) + c.Check(publicKey.Comment, qt.Equals, k.key1.Comment) + return nil + } + var b bytes.Buffer + e := base64.NewEncoder(base64.StdEncoding, &b) + _, _ = e.Write(k.key1.Marshal()) // Writes to a bytes buffer always return a nil error. + e.Close() + authorizedKey := fmt.Sprintf("%s %s %s", k.key1.Type(), &b, k.key1.Comment) + _, _ = k.keyManagerFacade.AddKeys(ctx, params.ModifyUserSSHKeys{Keys: []string{authorizedKey}}) +} + +func (k *keyManagerFacadeSuite) TestDeleteKeysByComment(c *qt.C) { + c.Parallel() + ctx := context.Background() + + k.removeKeyByCommentF = func(ctx context.Context, user *openfga.User, comment string) error { + c.Check(comment, qt.Equals, "comment-1") + return nil + } + _, _ = k.keyManagerFacade.DeleteKeys(ctx, params.ModifyUserSSHKeys{Keys: []string{"comment-1"}}) +} + +func (k *keyManagerFacadeSuite) TestDeleteKeysByFingerprint(c *qt.C) { + c.Parallel() + ctx := context.Background() + + fp := gossh.FingerprintLegacyMD5(k.key1) + + k.removeKeyByFingerprintF = func(ctx context.Context, user *openfga.User, fingerprint string) error { + c.Check(fingerprint, qt.Equals, fp) + return nil + } + _, _ = k.keyManagerFacade.DeleteKeys(ctx, params.ModifyUserSSHKeys{Keys: []string{fp}}) +} + +func TestKeyManagerFacade(t *testing.T) { + qtsuite.Run(qt.New(t), &keyManagerFacadeSuite{}) +} From 62a20e67d23f20269665c32df7e3d73c971a26ce Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Wed, 15 Jan 2025 14:12:02 +0200 Subject: [PATCH 2/3] chore: rename sshkeys and add godocs --- internal/rpcproxy/{sshkeys.go => keymanager.go} | 6 ++++++ internal/rpcproxy/{sshkeys_test.go => keymanager_test.go} | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) rename internal/rpcproxy/{sshkeys.go => keymanager.go} (92%) rename internal/rpcproxy/{sshkeys_test.go => keymanager_test.go} (98%) diff --git a/internal/rpcproxy/sshkeys.go b/internal/rpcproxy/keymanager.go similarity index 92% rename from internal/rpcproxy/sshkeys.go rename to internal/rpcproxy/keymanager.go index 2a995bf63..d048ac26a 100644 --- a/internal/rpcproxy/sshkeys.go +++ b/internal/rpcproxy/keymanager.go @@ -28,6 +28,8 @@ type keyManagerFacade struct { user *openfga.User } +// ListKeys lists the authenticated user's SSH keys +// in the format defined in args. func (s *keyManagerFacade) ListKeys(ctx context.Context, args jujuparams.ListSSHKeys) (jujuparams.StringsResults, error) { keys, err := s.ListUserPublicKeys(ctx, s.user) if err != nil { @@ -52,6 +54,8 @@ func (s *keyManagerFacade) ListKeys(ctx context.Context, args jujuparams.ListSSH return jujuparams.StringsResults{Results: []jujuparams.StringsResult{res}}, nil } +// AddKeys saves the SSH keys defined in args and associates them +// with the authenticated user. func (s *keyManagerFacade) AddKeys(ctx context.Context, args jujuparams.ModifyUserSSHKeys) (jujuparams.ErrorResults, error) { var res []jujuparams.ErrorResult errF := func(err error, msg string) jujuparams.ErrorResult { @@ -77,6 +81,8 @@ func (s *keyManagerFacade) AddKeys(ctx context.Context, args jujuparams.ModifyUs return jujuparams.ErrorResults{Results: res}, nil } +// DeleteKeys removes saved keys associated with the authenticated user +// and finds keys to remove either by comment or fingerprint. func (s *keyManagerFacade) DeleteKeys(ctx context.Context, args jujuparams.ModifyUserSSHKeys) (jujuparams.ErrorResults, error) { var res []jujuparams.ErrorResult errF := func(err error, msg string) jujuparams.ErrorResult { diff --git a/internal/rpcproxy/sshkeys_test.go b/internal/rpcproxy/keymanager_test.go similarity index 98% rename from internal/rpcproxy/sshkeys_test.go rename to internal/rpcproxy/keymanager_test.go index 990e29945..f7b36e792 100644 --- a/internal/rpcproxy/sshkeys_test.go +++ b/internal/rpcproxy/keymanager_test.go @@ -86,7 +86,7 @@ func (k *keyManagerFacadeSuite) TestListKeysShort(c *qt.C) { c.Assert(res.Results[0].Result, qt.HasLen, 2) c.Assert(res.Results[0].Result[0], qt.Matches, `.+ \(comment-1\)`) c.Assert(isFingerprintRegex.MatchString(res.Results[0].Result[0]), qt.IsTrue) - c.Assert(res.Results[1].Result[1], qt.Matches, `.+ \(comment-2\)`) + c.Assert(res.Results[0].Result[1], qt.Matches, `.+ \(comment-2\)`) c.Assert(isFingerprintRegex.MatchString(res.Results[0].Result[1]), qt.IsTrue) } From 113e69a078596009d991c1626b732b87bdc7b864 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Wed, 15 Jan 2025 15:02:40 +0200 Subject: [PATCH 3/3] chore: populate error code --- internal/rpcproxy/keymanager.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/rpcproxy/keymanager.go b/internal/rpcproxy/keymanager.go index d048ac26a..9b18b98c6 100644 --- a/internal/rpcproxy/keymanager.go +++ b/internal/rpcproxy/keymanager.go @@ -13,6 +13,7 @@ import ( "github.com/juju/utils/v3/ssh" gossh "golang.org/x/crypto/ssh" + "github.com/canonical/jimm/v3/internal/errors" "github.com/canonical/jimm/v3/internal/jimm/sshkeys" "github.com/canonical/jimm/v3/internal/openfga" ) @@ -60,6 +61,7 @@ func (s *keyManagerFacade) AddKeys(ctx context.Context, args jujuparams.ModifyUs var res []jujuparams.ErrorResult errF := func(err error, msg string) jujuparams.ErrorResult { return jujuparams.ErrorResult{Error: &jujuparams.Error{ + Code: string(errors.ErrorCode(err)), Message: fmt.Sprintf("%s: %s", msg, err.Error()), }} } @@ -87,6 +89,7 @@ func (s *keyManagerFacade) DeleteKeys(ctx context.Context, args jujuparams.Modif var res []jujuparams.ErrorResult errF := func(err error, msg string) jujuparams.ErrorResult { return jujuparams.ErrorResult{Error: &jujuparams.Error{ + Code: string(errors.ErrorCode(err)), Message: fmt.Sprintf("%s: %s", msg, err.Error()), }} }