diff --git a/cmd/juju/sshkeys/remove_sshkeys.go b/cmd/juju/sshkeys/remove_sshkeys.go index a6dbe0eebf2..52466aa1e7e 100644 --- a/cmd/juju/sshkeys/remove_sshkeys.go +++ b/cmd/juju/sshkeys/remove_sshkeys.go @@ -21,9 +21,9 @@ var usageRemoveSSHKeyDetails = ` Juju maintains a per-model cache of public SSH keys which it copies to each unit. This command will remove a specified key (or space separated list of keys) from the model cache and all current units deployed in that -model. The keys to be removed may be specified by the key's fingerprint, -or by the text label associated with them. Invalid keys in the model cache -can be removed by specifying the key verbatim. +model. The keys to be removed may be specified by the key's fingerprint using a +sah256 sum, or by the text label associated with them. Invalid keys in the model +cache can be removed by specifying the key verbatim. `[1:] diff --git a/core/user/user.go b/core/user/user.go index 798cf892500..7ff34e8eb39 100644 --- a/core/user/user.go +++ b/core/user/user.go @@ -51,7 +51,8 @@ func NewUUID() (UUID, error) { return UUID(uuid.String()), nil } -// Validate returns an error if the UUID is invalid. +// Validate returns an error if the UUID is invalid. The error returned +// satisfies [errors.NotValid]. func (u UUID) Validate() error { if u == "" { return fmt.Errorf("empty uuid%w", errors.Hide(errors.NotValid)) diff --git a/domain/keymanager/doc.go b/domain/keymanager/doc.go new file mode 100644 index 00000000000..976509c7407 --- /dev/null +++ b/domain/keymanager/doc.go @@ -0,0 +1,10 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +// Package keys provides the domain needed for configuring public keys on a +// model for a user. +// +// Public keys for a user are per model based and will not follow a user between +// models. Currently under the covers we do not model the public keys and their +// user (owner) as an old legacy implementation details of Juju 3.x. +package keymanager diff --git a/domain/keymanager/errors/errors.go b/domain/keymanager/errors/errors.go new file mode 100644 index 00000000000..be18984eaaa --- /dev/null +++ b/domain/keymanager/errors/errors.go @@ -0,0 +1,31 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package errors + +import ( + "github.com/juju/errors" +) + +const ( + // PublicKeyAlreadyExists indicates that the authorised key already + // exists for the specified user. + PublicKeyAlreadyExists = errors.ConstError("public key already exists") + + // ImportSubjectNotFound indicates that when importing public keys for a + // subject the source of the public keys has told us that this subject + // does not exist. + ImportSubjectNotFound = errors.ConstError("import subject not found") + + // InvalidPublicKey indicates a problem with a public key where it + // was unable to be understood. + InvalidPublicKey = errors.ConstError("invalid public key") + + // ReservedCommentViolation indicates that a key contains a comment that is + // reserved within the Juju system and cannot be used. + ReservedCommentViolation = errors.ConstError("key contains a reserved comment") + + // UnknownImportSource indicates that an import operation cannot occur + // because the source of the information is unknown. + UnknownImportSource = errors.ConstError("unknown import source") +) diff --git a/domain/keymanager/service/package_test.go b/domain/keymanager/service/package_test.go new file mode 100644 index 00000000000..e4c519edd5f --- /dev/null +++ b/domain/keymanager/service/package_test.go @@ -0,0 +1,16 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package service + +import ( + "testing" + + gc "gopkg.in/check.v1" +) + +//go:generate go run go.uber.org/mock/mockgen -typed -package service -destination service_mock_test.go github.com/juju/juju/domain/keymanager/service PublicKeyImporter,State + +func TestPackage(t *testing.T) { + gc.TestingT(t) +} diff --git a/domain/keymanager/service/service.go b/domain/keymanager/service/service.go new file mode 100644 index 00000000000..974d7bd59c5 --- /dev/null +++ b/domain/keymanager/service/service.go @@ -0,0 +1,258 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package service + +import ( + "context" + "fmt" + "net/url" + + "github.com/juju/collections/set" + "github.com/juju/errors" + + "github.com/juju/juju/core/user" + "github.com/juju/juju/domain/keymanager" + keyserrors "github.com/juju/juju/domain/keymanager/errors" + "github.com/juju/juju/environs/config" + "github.com/juju/juju/internal/ssh" + importererrors "github.com/juju/juju/internal/ssh/importer/errors" +) + +// PublicKeyImporter describes a service that is capable of fetching and +// providing public keys for a subject from a set of well known sources that +// don't need to be understood by this service. +type PublicKeyImporter interface { + // FetchPublicKeysForSubject is responsible for gathering all of the + // public keys available for a specified subject. + // The following errors can be expected: + // - [importererrors.NoResolver] when there is import resolver the subject + // schema. + // - [importerrors.SubjectNotFound] when the resolver has reported that no + // subject exists. + FetchPublicKeysForSubject(context.Context, *url.URL) ([]string, error) +} + +// Service provides the means for interacting with a users underlying +// public keys for a model. +type Service struct { + // keyImporter is the [PublicKeyImporter] to use for fetching a users + // public key's for subject. + keyImporter PublicKeyImporter + + // st provides the state access layer to this service. + st State +} + +// State provides the access layer the [Service] needs for persisting and +// retrieving a user's public keys on a model. +type State interface { + // AddPublicKeysForUser adds a set of public keys for a user on + // this model. If one or more of the public keys to add for the user already + // exists a [keyserrors.PublicKeyAlreadyExists] error will be returned. + AddPublicKeysForUser(context.Context, user.UUID, []keymanager.PublicKey) error + + // AddPublicKeyForUserIfNotFound will attempt to add the given set of public + // keys to the user. If the user already contains the public key it will be + // skipped and no [keyserrors.PublicKeyAlreadyExists] error will be returned. + AddPublicKeyForUserIfNotFound(context.Context, user.UUID, []keymanager.PublicKey) error + + // GetPublicKeysForUser is responsible for returning all of the + // public keys for the current user in this model. + GetPublicKeysForUser(context.Context, user.UUID) ([]string, error) + + // DeletePublicKeysForUser is responsible for removing the keys from the + // users list of public keys where the string list represents one of + // the keys fingerprint, public key data or comment. + DeletePublicKeysForUser(context.Context, user.UUID, []string) error +} + +var ( + // reservedPublicKeyComments is the set of comments that can not be + // removed or added by a user. + reservedPublicKeyComments = set.NewStrings( + "juju-client-key", + config.JujuSystemKey, + ) +) + +// NewService constructs a new [Service] for interfacting with a users +// public keys. +func NewService(keyImporter PublicKeyImporter, state State) *Service { + return &Service{ + keyImporter: keyImporter, + st: state, + } +} + +// AddPublicKeysForUser is responsible for adding public keys for a user to a +// model. The following errors can be expected: +// - [errors.NotValid] when the user id is not valid +// - [github.com/juju/juju/domain/access/errors.UserNotFound] when the user does +// not exist. +// - [keyserrors.InvalidPublicKey] when a public key fails validation. +// - [keyserrors.ReservedCommentViolation] when a key being added contains a +// comment string that is reserved. +// - [keyserrors.PublicKeyAlreadyExists] when a public key being added +// for a user already exists. +func (s *Service) AddPublicKeysForUser( + ctx context.Context, + userID user.UUID, + keys ...string, +) error { + if err := userID.Validate(); err != nil { + return fmt.Errorf("validating user id %q when adding public keys: %w", userID, err) + } + + if len(keys) == 0 { + return nil + } + + toAdd := make([]keymanager.PublicKey, 0, len(keys)) + for i, keyToAdd := range keys { + parsedKey, err := ssh.ParsePublicKey(keyToAdd) + if err != nil { + return fmt.Errorf( + "%w %q at index %d: %w", + keyserrors.InvalidPublicKey, keyToAdd, i, err, + ) + } + + if reservedPublicKeyComments.Contains(parsedKey.Comment) { + return fmt.Errorf( + "public key %q at index %d contains a reserved comment %q that cannot be used: %w", + keyToAdd, + i, + parsedKey.Comment, + errors.Hide(keyserrors.ReservedCommentViolation), + ) + } + + toAdd = append(toAdd, keymanager.PublicKey{ + Comment: parsedKey.Comment, + Fingerprint: parsedKey.Fingerprint(), + Key: keyToAdd, + }) + } + + return s.st.AddPublicKeysForUser(ctx, userID, toAdd) +} + +// DeletePublicKeysForUser removes the keys associated with targets from the +// user's list of public keys. Targets can be an arbitrary list of a +// public key fingerprint (sha256), comment or full key value to be +// removed. Where a match is found the key will be removed. If no key exists for +// a target this will result in no operation. The following errors can be +// expected: +// - [errors.NotValid] when the user id is not valid +// - [accesserrors.UserNotFound] when the provided user does not exist. +func (s *Service) DeleteKeysForUser( + ctx context.Context, + userID user.UUID, + targets ...string, +) error { + if err := userID.Validate(); err != nil { + return fmt.Errorf( + "validating user id %q when deleting public keys: %w", + userID, err, + ) + } + + return s.st.DeletePublicKeysForUser(ctx, userID, targets) +} + +// ImportPublicKeysForUser will import all of the public keys available for a +// given subject and add them to the specified Juju user. If the user already +// has one or more of the public keys being imported they will safely be skipped +// with no errors being returned. +// The following errors can be expected: +// - [errors.NotValid] when the user id is not valid +// - [github.com/juju/juju/domain/access/errors.UserNotFound] when the user does +// not exist. +// - [keyserrors.InvalidPublicKey] when a key being imported fails validation. +// - [keyserrors.ReservedCommentViolation] when a key being added contains a +// comment string that is reserved. +// - [keyserrors.UnknownImportSource] when the source for the import operation +// is unknown to the service. +// - [keyserrors.ImportSubjectNotFound] when the source has indicated that the +// subject for the import operation does not exist. +func (s *Service) ImportPublicKeysForUser( + ctx context.Context, + userID user.UUID, + subject *url.URL, +) error { + if err := userID.Validate(); err != nil { + return fmt.Errorf( + "validating user id %q when importing public keys from %q: %w", + userID, subject.String(), err, + ) + } + + keys, err := s.keyImporter.FetchPublicKeysForSubject(ctx, subject) + + switch { + case errors.Is(err, importererrors.NoResolver): + return fmt.Errorf( + "cannot import public keys for user %q, unknown public key source %q%w", + userID, subject.Scheme, errors.Hide(keyserrors.UnknownImportSource), + ) + case errors.Is(err, importererrors.SubjectNotFound): + return fmt.Errorf( + "cannot import public keys for user %q, import subject %q not found%w", + userID, subject.String(), errors.Hide(keyserrors.ImportSubjectNotFound), + ) + case err != nil: + return fmt.Errorf( + "cannot import public keys for user %q using subject %q: %w", + userID, subject.String(), err, + ) + } + + keysToAdd := make([]keymanager.PublicKey, 0, len(keys)) + for i, key := range keys { + parsedKey, err := ssh.ParsePublicKey(key) + if err != nil { + return fmt.Errorf( + "cannot parse key %d for subject %q when importing keys for user %q: %w%w", + i, subject.String(), userID, err, errors.Hide(keyserrors.InvalidPublicKey), + ) + } + + if reservedPublicKeyComments.Contains(parsedKey.Comment) { + return fmt.Errorf( + "cannot import key %d for user %q with subject %q because the comment %q is reserved%w", + i, + userID, + subject.String(), + parsedKey.Comment, + errors.Hide(keyserrors.ReservedCommentViolation), + ) + } + + keysToAdd = append(keysToAdd, keymanager.PublicKey{ + Comment: parsedKey.Comment, + Key: key, + Fingerprint: parsedKey.Fingerprint(), + }) + } + + return s.st.AddPublicKeyForUserIfNotFound(ctx, userID, keysToAdd) +} + +// ListPublicKeysForUser is responsible for returning the public ssh keys for +// the specified user. The following errors can be expected: +// - [errors.NotValid] when the user id is not valid. +// - [usererrors.NotFound] when the given user does not exist. +func (s *Service) ListPublicKeysForUser( + ctx context.Context, + userID user.UUID, +) ([]string, error) { + if err := userID.Validate(); err != nil { + return nil, fmt.Errorf( + "validating user id %q when listing public keys: %w", + userID, err, + ) + } + + return s.st.GetPublicKeysForUser(ctx, userID) +} diff --git a/domain/keymanager/service/service_mock_test.go b/domain/keymanager/service/service_mock_test.go new file mode 100644 index 00000000000..23d4dd77964 --- /dev/null +++ b/domain/keymanager/service/service_mock_test.go @@ -0,0 +1,258 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/juju/juju/domain/keymanager/service (interfaces: PublicKeyImporter,State) +// +// Generated by this command: +// +// mockgen -typed -package service -destination service_mock_test.go github.com/juju/juju/domain/keymanager/service PublicKeyImporter,State +// + +// Package service is a generated GoMock package. +package service + +import ( + context "context" + url "net/url" + reflect "reflect" + + user "github.com/juju/juju/core/user" + keymanager "github.com/juju/juju/domain/keymanager" + gomock "go.uber.org/mock/gomock" +) + +// MockPublicKeyImporter is a mock of PublicKeyImporter interface. +type MockPublicKeyImporter struct { + ctrl *gomock.Controller + recorder *MockPublicKeyImporterMockRecorder +} + +// MockPublicKeyImporterMockRecorder is the mock recorder for MockPublicKeyImporter. +type MockPublicKeyImporterMockRecorder struct { + mock *MockPublicKeyImporter +} + +// NewMockPublicKeyImporter creates a new mock instance. +func NewMockPublicKeyImporter(ctrl *gomock.Controller) *MockPublicKeyImporter { + mock := &MockPublicKeyImporter{ctrl: ctrl} + mock.recorder = &MockPublicKeyImporterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockPublicKeyImporter) EXPECT() *MockPublicKeyImporterMockRecorder { + return m.recorder +} + +// FetchPublicKeysForSubject mocks base method. +func (m *MockPublicKeyImporter) FetchPublicKeysForSubject(arg0 context.Context, arg1 *url.URL) ([]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "FetchPublicKeysForSubject", arg0, arg1) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// FetchPublicKeysForSubject indicates an expected call of FetchPublicKeysForSubject. +func (mr *MockPublicKeyImporterMockRecorder) FetchPublicKeysForSubject(arg0, arg1 any) *MockPublicKeyImporterFetchPublicKeysForSubjectCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FetchPublicKeysForSubject", reflect.TypeOf((*MockPublicKeyImporter)(nil).FetchPublicKeysForSubject), arg0, arg1) + return &MockPublicKeyImporterFetchPublicKeysForSubjectCall{Call: call} +} + +// MockPublicKeyImporterFetchPublicKeysForSubjectCall wrap *gomock.Call +type MockPublicKeyImporterFetchPublicKeysForSubjectCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockPublicKeyImporterFetchPublicKeysForSubjectCall) Return(arg0 []string, arg1 error) *MockPublicKeyImporterFetchPublicKeysForSubjectCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockPublicKeyImporterFetchPublicKeysForSubjectCall) Do(f func(context.Context, *url.URL) ([]string, error)) *MockPublicKeyImporterFetchPublicKeysForSubjectCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockPublicKeyImporterFetchPublicKeysForSubjectCall) DoAndReturn(f func(context.Context, *url.URL) ([]string, error)) *MockPublicKeyImporterFetchPublicKeysForSubjectCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// MockState is a mock of State interface. +type MockState struct { + ctrl *gomock.Controller + recorder *MockStateMockRecorder +} + +// MockStateMockRecorder is the mock recorder for MockState. +type MockStateMockRecorder struct { + mock *MockState +} + +// NewMockState creates a new mock instance. +func NewMockState(ctrl *gomock.Controller) *MockState { + mock := &MockState{ctrl: ctrl} + mock.recorder = &MockStateMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockState) EXPECT() *MockStateMockRecorder { + return m.recorder +} + +// AddPublicKeyForUserIfNotFound mocks base method. +func (m *MockState) AddPublicKeyForUserIfNotFound(arg0 context.Context, arg1 user.UUID, arg2 []keymanager.PublicKey) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddPublicKeyForUserIfNotFound", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// AddPublicKeyForUserIfNotFound indicates an expected call of AddPublicKeyForUserIfNotFound. +func (mr *MockStateMockRecorder) AddPublicKeyForUserIfNotFound(arg0, arg1, arg2 any) *MockStateAddPublicKeyForUserIfNotFoundCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddPublicKeyForUserIfNotFound", reflect.TypeOf((*MockState)(nil).AddPublicKeyForUserIfNotFound), arg0, arg1, arg2) + return &MockStateAddPublicKeyForUserIfNotFoundCall{Call: call} +} + +// MockStateAddPublicKeyForUserIfNotFoundCall wrap *gomock.Call +type MockStateAddPublicKeyForUserIfNotFoundCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockStateAddPublicKeyForUserIfNotFoundCall) Return(arg0 error) *MockStateAddPublicKeyForUserIfNotFoundCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockStateAddPublicKeyForUserIfNotFoundCall) Do(f func(context.Context, user.UUID, []keymanager.PublicKey) error) *MockStateAddPublicKeyForUserIfNotFoundCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockStateAddPublicKeyForUserIfNotFoundCall) DoAndReturn(f func(context.Context, user.UUID, []keymanager.PublicKey) error) *MockStateAddPublicKeyForUserIfNotFoundCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// AddPublicKeysForUser mocks base method. +func (m *MockState) AddPublicKeysForUser(arg0 context.Context, arg1 user.UUID, arg2 []keymanager.PublicKey) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddPublicKeysForUser", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// AddPublicKeysForUser indicates an expected call of AddPublicKeysForUser. +func (mr *MockStateMockRecorder) AddPublicKeysForUser(arg0, arg1, arg2 any) *MockStateAddPublicKeysForUserCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddPublicKeysForUser", reflect.TypeOf((*MockState)(nil).AddPublicKeysForUser), arg0, arg1, arg2) + return &MockStateAddPublicKeysForUserCall{Call: call} +} + +// MockStateAddPublicKeysForUserCall wrap *gomock.Call +type MockStateAddPublicKeysForUserCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockStateAddPublicKeysForUserCall) Return(arg0 error) *MockStateAddPublicKeysForUserCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockStateAddPublicKeysForUserCall) Do(f func(context.Context, user.UUID, []keymanager.PublicKey) error) *MockStateAddPublicKeysForUserCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockStateAddPublicKeysForUserCall) DoAndReturn(f func(context.Context, user.UUID, []keymanager.PublicKey) error) *MockStateAddPublicKeysForUserCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// DeletePublicKeysForUser mocks base method. +func (m *MockState) DeletePublicKeysForUser(arg0 context.Context, arg1 user.UUID, arg2 []string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeletePublicKeysForUser", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeletePublicKeysForUser indicates an expected call of DeletePublicKeysForUser. +func (mr *MockStateMockRecorder) DeletePublicKeysForUser(arg0, arg1, arg2 any) *MockStateDeletePublicKeysForUserCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeletePublicKeysForUser", reflect.TypeOf((*MockState)(nil).DeletePublicKeysForUser), arg0, arg1, arg2) + return &MockStateDeletePublicKeysForUserCall{Call: call} +} + +// MockStateDeletePublicKeysForUserCall wrap *gomock.Call +type MockStateDeletePublicKeysForUserCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockStateDeletePublicKeysForUserCall) Return(arg0 error) *MockStateDeletePublicKeysForUserCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockStateDeletePublicKeysForUserCall) Do(f func(context.Context, user.UUID, []string) error) *MockStateDeletePublicKeysForUserCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockStateDeletePublicKeysForUserCall) DoAndReturn(f func(context.Context, user.UUID, []string) error) *MockStateDeletePublicKeysForUserCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// GetPublicKeysForUser mocks base method. +func (m *MockState) GetPublicKeysForUser(arg0 context.Context, arg1 user.UUID) ([]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetPublicKeysForUser", arg0, arg1) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetPublicKeysForUser indicates an expected call of GetPublicKeysForUser. +func (mr *MockStateMockRecorder) GetPublicKeysForUser(arg0, arg1 any) *MockStateGetPublicKeysForUserCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPublicKeysForUser", reflect.TypeOf((*MockState)(nil).GetPublicKeysForUser), arg0, arg1) + return &MockStateGetPublicKeysForUserCall{Call: call} +} + +// MockStateGetPublicKeysForUserCall wrap *gomock.Call +type MockStateGetPublicKeysForUserCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockStateGetPublicKeysForUserCall) Return(arg0 []string, arg1 error) *MockStateGetPublicKeysForUserCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockStateGetPublicKeysForUserCall) Do(f func(context.Context, user.UUID) ([]string, error)) *MockStateGetPublicKeysForUserCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockStateGetPublicKeysForUserCall) DoAndReturn(f func(context.Context, user.UUID) ([]string, error)) *MockStateGetPublicKeysForUserCall { + c.Call = c.Call.DoAndReturn(f) + return c +} diff --git a/domain/keymanager/service/service_test.go b/domain/keymanager/service/service_test.go new file mode 100644 index 00000000000..8cbc85b45aa --- /dev/null +++ b/domain/keymanager/service/service_test.go @@ -0,0 +1,480 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package service + +import ( + "context" + "net/url" + + "github.com/juju/errors" + "github.com/juju/testing" + jc "github.com/juju/testing/checkers" + gomock "go.uber.org/mock/gomock" + gc "gopkg.in/check.v1" + + "github.com/juju/juju/core/user" + usertesting "github.com/juju/juju/core/user/testing" + accesserrors "github.com/juju/juju/domain/access/errors" + "github.com/juju/juju/domain/keymanager" + keyserrors "github.com/juju/juju/domain/keymanager/errors" + "github.com/juju/juju/internal/ssh" + importererrors "github.com/juju/juju/internal/ssh/importer/errors" +) + +type serviceSuite struct { + testing.IsolationSuite + + keyImporter *MockPublicKeyImporter + state *MockState + userID user.UUID + subjectURI *url.URL +} + +var ( + _ = gc.Suite(&serviceSuite{}) + + existingUserPublicKeys = []string{ + "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAII4GpCvqUUYUJlx6d1kpUO9k/t4VhSYsf0yE0/QTqDzC existing1", + "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIJQJ9wv0uC3yytXM3d2sJJWvZLuISKo7ZHwafHVviwVe existing2", + } + + // testingPublicKeys represents a set of keys that can be used and are valid. + testingPublicKeys = []string{ + // ecdsa testing public key + "ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBG00bYFLb/sxPcmVRMg8NXZK/ldefElAkC9wD41vABdHZiSRvp+2y9BMNVYzE/FnzKObHtSvGRX65YQgRn7k5p0= juju@example.com", + + // ed25519 testing public key + "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIN8h8XBpjS9aBUG5cdoSWubs7wT2Lc/BEZIUQCqoaOZR juju@example.com", + + // rsa testing public key + "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDvplNOK3UBpULZKvZf/I5JHci/DufpSxj8yR4yKE2grescJxu6754jPT3xztSeLGD31/oJApJZGkMUAMRenvDqIaq+taRfOUo/l19AlGZc+Edv4bTlJzZ1Lzwex1vvL1doaLb/f76IIUHClGUgIXRceQH1ovHiIWj6nGltuLanG8YTWxlzzK33yhitmZt142DmpX1VUVF5c/Hct6Rav5lKmwej1TDed1KmHzXVoTHEsmWhKsOK27ue5yTuq0GX6LrAYDucF+2MqZCsuddXsPAW1tj5GNZSR7RrKW5q1CI0G7k9gSomuCsRMlCJ3BqID/vUSs/0qOWg4he0HUsYKQSrXIhckuZu+jYP8B80MoXT50ftRidoG/zh/PugBdXTk46FloVClQopG5A2fbqrphADcUUbRUxZ2lWQN+OVHKfEsfV2b8L2aSqZUGlryfW1cirB5JCTDvtv7rUy9/ny9iKA+8tAyKSDF0I901RDDqKc9dSkrHCg2bLnJZDoiRoWczE= juju@example.com", + } + + // reservedPublicKeys are keys with reserved comments that can not be added + // or removed via the service. + reservedPublicKeys = []string{ + "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIN8h8XBpjS9aBUG5cdoSWubs7wT2Lc/BEZIUQCqoaOZR juju-client-key", + "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIN8h8XBpjS9aBUG5cdoSWubs7wT2Lc/BEZIUQCqoaOZR juju-system-key", + } +) + +func (s *serviceSuite) setupMocks(c *gc.C) *gomock.Controller { + ctrl := gomock.NewController(c) + s.keyImporter = NewMockPublicKeyImporter(ctrl) + s.state = NewMockState(ctrl) + return ctrl +} + +func (s *serviceSuite) SetUpTest(c *gc.C) { + s.userID = usertesting.GenUserUUID(c) + + uri, err := url.Parse("gh:tlm") + c.Check(err, jc.ErrorIsNil) + s.subjectURI = uri +} + +// TestAddKeysForInvalidUser is asserting that if we pass in an invalid user id +// to [Service.AddKeysForUser] we get back a [errors.NotValid] error. +func (s *serviceSuite) TestAddKeysForInvalidUser(c *gc.C) { + defer s.setupMocks(c).Finish() + + err := NewService(s.keyImporter, s.state). + AddPublicKeysForUser(context.Background(), user.UUID("notvalid"), "key") + c.Check(err, jc.ErrorIs, errors.NotValid) +} + +// TestAddKeysForNonExistentUser is testing that if a user id doesn't exist that +// the services layer correctly passes back the [accesserrors.UserNotFound] +// error. +func (s *serviceSuite) TestAddKeysForNonExistentUser(c *gc.C) { + defer s.setupMocks(c).Finish() + + keyInfo, err := ssh.ParsePublicKey(testingPublicKeys[0]) + c.Assert(err, jc.ErrorIsNil) + expectedKeys := []keymanager.PublicKey{ + { + Comment: keyInfo.Comment, + Fingerprint: keyInfo.Fingerprint(), + Key: testingPublicKeys[0], + }, + } + + s.state.EXPECT().AddPublicKeysForUser( + gomock.Any(), s.userID, expectedKeys, + ).Return(accesserrors.UserNotFound) + + svc := NewService(s.keyImporter, s.state) + err = svc.AddPublicKeysForUser(context.Background(), s.userID, testingPublicKeys[0]) + c.Check(err, jc.ErrorIs, accesserrors.UserNotFound) +} + +// TestAddInvalidPublicKeys is testing that if we try and add one or more keys +// that are invalid we get back a [keyserrors.InvalidPublicKey] key error +// and no modificiation to state is performed. +func (s *serviceSuite) TestAddInvalidPublicKeys(c *gc.C) { + defer s.setupMocks(c).Finish() + + svc := NewService(s.keyImporter, s.state) + + err := svc.AddPublicKeysForUser(context.Background(), s.userID, "notvalid") + c.Check(err, jc.ErrorIs, keyserrors.InvalidPublicKey) + + err = svc.AddPublicKeysForUser(context.Background(), s.userID, "notvalid", testingPublicKeys[0]) + c.Check(err, jc.ErrorIs, keyserrors.InvalidPublicKey) + + err = svc.AddPublicKeysForUser(context.Background(), s.userID, testingPublicKeys[0], "notvalid") + c.Check(err, jc.ErrorIs, keyserrors.InvalidPublicKey) + + err = svc.AddPublicKeysForUser( + context.Background(), + s.userID, + testingPublicKeys[0], + "notvalid", + testingPublicKeys[1], + ) + c.Check(err, jc.ErrorIs, keyserrors.InvalidPublicKey) +} + +// TestAddReservedKeys is testing that if we try and add public keys with +// reserved comments we get back an error that satisfies +// [keyserrors.ReservedCommentViolation] +func (s *serviceSuite) TestAddReservedPublicKeys(c *gc.C) { + defer s.setupMocks(c).Finish() + + svc := NewService(s.keyImporter, s.state) + + err := svc.AddPublicKeysForUser(context.Background(), s.userID, reservedPublicKeys...) + c.Check(err, jc.ErrorIs, keyserrors.ReservedCommentViolation) + + err = svc.AddPublicKeysForUser( + context.Background(), + s.userID, + testingPublicKeys[0], + reservedPublicKeys[0], + ) + c.Check(err, jc.ErrorIs, keyserrors.ReservedCommentViolation) +} + +// TestAddExistingKeysForUser is asserting that when we try and add a key for a +// user that already exists we get back a +// [keyserrors.PublicKeyAlreadyExists] error. +func (s *serviceSuite) TestAddExistingKeysForUser(c *gc.C) { + defer s.setupMocks(c).Finish() + + svc := NewService(s.keyImporter, s.state) + + keyInfo1, err := ssh.ParsePublicKey(existingUserPublicKeys[1]) + c.Assert(err, jc.ErrorIsNil) + keyInfo2, err := ssh.ParsePublicKey(testingPublicKeys[1]) + c.Assert(err, jc.ErrorIsNil) + expectedKeys := []keymanager.PublicKey{ + { + Comment: keyInfo1.Comment, + Fingerprint: keyInfo1.Fingerprint(), + Key: existingUserPublicKeys[1], + }, + } + + s.state.EXPECT().AddPublicKeysForUser( + gomock.Any(), s.userID, expectedKeys, + ).Return(keyserrors.PublicKeyAlreadyExists) + + err = svc.AddPublicKeysForUser( + context.Background(), + s.userID, + existingUserPublicKeys[1], + ) + c.Check(err, jc.ErrorIs, keyserrors.PublicKeyAlreadyExists) + + expectedKeys = []keymanager.PublicKey{ + { + Comment: keyInfo2.Comment, + Fingerprint: keyInfo2.Fingerprint(), + Key: testingPublicKeys[1], + }, + { + Comment: keyInfo1.Comment, + Fingerprint: keyInfo1.Fingerprint(), + Key: existingUserPublicKeys[1], + }, + } + + s.state.EXPECT().AddPublicKeysForUser( + gomock.Any(), + s.userID, + expectedKeys, + ).Return(keyserrors.PublicKeyAlreadyExists) + + err = svc.AddPublicKeysForUser( + context.Background(), + s.userID, + testingPublicKeys[1], + existingUserPublicKeys[1], + ) + c.Check(err, jc.ErrorIs, keyserrors.PublicKeyAlreadyExists) +} + +// TestAddKeysForUser is testing the happy path of [Service.AddKeysForUser] +func (s *serviceSuite) TestAddKeysForUser(c *gc.C) { + defer s.setupMocks(c).Finish() + + expectedKeys := make([]keymanager.PublicKey, 0, len(testingPublicKeys)) + for _, key := range testingPublicKeys { + keyInfo, err := ssh.ParsePublicKey(key) + c.Assert(err, jc.ErrorIsNil) + expectedKeys = append(expectedKeys, keymanager.PublicKey{ + Comment: keyInfo.Comment, + Fingerprint: keyInfo.Fingerprint(), + Key: key, + }) + } + + s.state.EXPECT().AddPublicKeysForUser( + gomock.Any(), + s.userID, + expectedKeys, + ).Return(nil) + + svc := NewService(s.keyImporter, s.state) + + err := svc.AddPublicKeysForUser(context.Background(), s.userID, testingPublicKeys...) + c.Check(err, jc.ErrorIsNil) +} + +// TestListKeysForInvalidUserId is testing that if we pass in a junk non valid +// user id to [Service.ListKeysForUser] we get back a [errors.NotValid] error. +func (s *serviceSuite) TestListKeysForInvalidUserId(c *gc.C) { + defer s.setupMocks(c).Finish() + + svc := NewService(s.keyImporter, s.state) + + _, err := svc.ListPublicKeysForUser(context.Background(), user.UUID("not-valid")) + c.Check(err, jc.ErrorIs, errors.NotValid) +} + +// TestListKeysForNonExistentUser is testing that if we ask for the keys for a +// non existent user we get back an error that satisfies +// [accesserrors.UserNotFound]. +func (s *serviceSuite) TestListKeysForNonExistentUser(c *gc.C) { + defer s.setupMocks(c).Finish() + + s.state.EXPECT().GetPublicKeysForUser(gomock.Any(), s.userID). + Return(nil, accesserrors.UserNotFound).AnyTimes() + svc := NewService(s.keyImporter, s.state) + + _, err := svc.ListPublicKeysForUser(context.Background(), s.userID) + c.Check(err, jc.ErrorIs, accesserrors.UserNotFound) +} + +// TestListKeysForUser is testing the happy path for +// [Service.ListPublicKeysForUser]. +func (s *serviceSuite) TestListKeysForUser(c *gc.C) { + defer s.setupMocks(c).Finish() + + s.state.EXPECT().GetPublicKeysForUser(gomock.Any(), s.userID). + Return(existingUserPublicKeys, nil) + svc := NewService(s.keyImporter, s.state) + + keys, err := svc.ListPublicKeysForUser(context.Background(), s.userID) + c.Check(err, jc.ErrorIsNil) + c.Check(keys, gc.DeepEquals, existingUserPublicKeys) +} + +// TestDeleteKeysForInvalidUser is asserting that if we pass an invalid user id +// to [Service.DeleteKeysForUser] we get back an [errors.NotValid] error. +func (s *serviceSuite) TestDeleteKeysForInvalidUser(c *gc.C) { + defer s.setupMocks(c).Finish() + + err := NewService(s.keyImporter, s.state). + DeleteKeysForUser(context.Background(), user.UUID("notvalid"), "key") + c.Check(err, jc.ErrorIs, errors.NotValid) +} + +// TestDeleteKeysForUserNotFound is asserting that if the state layer +// propogrates a [accesserrors.UserNotFound] that the service layer returns the +// error back up. +func (s *serviceSuite) TestDeleteKeysForUserNotFound(c *gc.C) { + defer s.setupMocks(c).Finish() + + s.state.EXPECT().DeletePublicKeysForUser( + gomock.Any(), + s.userID, + []string{testingPublicKeys[0]}, + ).Return(accesserrors.UserNotFound) + + err := NewService(s.keyImporter, s.state). + DeleteKeysForUser(context.Background(), s.userID, testingPublicKeys[0]) + c.Check(err, jc.ErrorIs, accesserrors.UserNotFound) +} + +// TestDeleteKeysForUserWithFingerprint is asserting that we can remove keys for +// a user based on the key fingerprint. +func (s *serviceSuite) TestDeleteKeysForUserWithFingerprint(c *gc.C) { + defer s.setupMocks(c).Finish() + + key, err := ssh.ParsePublicKey(existingUserPublicKeys[0]) + c.Assert(err, jc.ErrorIsNil) + + s.state.EXPECT().DeletePublicKeysForUser( + gomock.Any(), s.userID, []string{key.Fingerprint()}, + ).Return(nil) + + err = NewService(s.keyImporter, s.state). + DeleteKeysForUser(context.Background(), s.userID, key.Fingerprint()) + c.Check(err, jc.ErrorIsNil) +} + +// TestDeleteKeysForUserWithComment is asserting that we can remove keys for a +// user based on the key comment. +func (s *serviceSuite) TestDeleteKeysForUserWithComment(c *gc.C) { + defer s.setupMocks(c).Finish() + + key, err := ssh.ParsePublicKey(existingUserPublicKeys[0]) + c.Assert(err, jc.ErrorIsNil) + + s.state.EXPECT().DeletePublicKeysForUser( + gomock.Any(), + s.userID, + []string{key.Comment}, + ).Return(nil) + + err = NewService(s.keyImporter, s.state). + DeleteKeysForUser(context.Background(), s.userID, key.Comment) + c.Check(err, jc.ErrorIsNil) +} + +// TestDeleteKeysForUserData is asserting that we can remove ssh keys for a user +// based on the raw key data. +func (s *serviceSuite) TestDeleteKeysForUserData(c *gc.C) { + defer s.setupMocks(c).Finish() + + s.state.EXPECT().DeletePublicKeysForUser( + gomock.Any(), + s.userID, + []string{existingUserPublicKeys[0]}, + ).Return(nil) + + err := NewService(s.keyImporter, s.state). + DeleteKeysForUser(context.Background(), s.userID, existingUserPublicKeys[0]) + c.Check(err, jc.ErrorIsNil) +} + +// TestDeleteKeysForUserCombination is asserting multiple deletes using +// different targets. We want to see that with a set of different target types +// all the correct keys are removed. +func (s *serviceSuite) TestDeleteKeysForUserCombination(c *gc.C) { + defer s.setupMocks(c).Finish() + + key, err := ssh.ParsePublicKey(existingUserPublicKeys[0]) + c.Assert(err, jc.ErrorIsNil) + + s.state.EXPECT().DeletePublicKeysForUser( + gomock.Any(), + s.userID, + []string{key.Comment, existingUserPublicKeys[1]}, + ).Return(nil) + + err = NewService(s.keyImporter, s.state). + DeleteKeysForUser( + context.Background(), + s.userID, + key.Comment, + existingUserPublicKeys[1], + ) + c.Check(err, jc.ErrorIsNil) +} + +// TestImportKeyForUnknownSource is asserting that if we try and import keys for +// a subject where the source is unknown. +func (s *serviceSuite) TestImportKeysForUnknownSource(c *gc.C) { + defer s.setupMocks(c).Finish() + + s.keyImporter.EXPECT().FetchPublicKeysForSubject( + gomock.Any(), + s.subjectURI, + ).Return(nil, importererrors.NoResolver) + + err := NewService(s.keyImporter, s.state). + ImportPublicKeysForUser(context.Background(), s.userID, s.subjectURI) + c.Check(err, jc.ErrorIs, keyserrors.UnknownImportSource) +} + +// TestImportKeyForUnknownSubject is asserting that if we ask to import keys for +// a subject that doesn't exist we get back a [keyserrors.ImportSubjectNotFound] +// error. +func (s *serviceSuite) TestImportKeysForUnknownSubject(c *gc.C) { + defer s.setupMocks(c).Finish() + + s.keyImporter.EXPECT().FetchPublicKeysForSubject( + gomock.Any(), + s.subjectURI, + ).Return(nil, importererrors.SubjectNotFound) + + err := NewService(s.keyImporter, s.state). + ImportPublicKeysForUser(context.Background(), s.userID, s.subjectURI) + c.Check(err, jc.ErrorIs, keyserrors.ImportSubjectNotFound) +} + +// TestImportKeysInvalidPublicKeys is asserting that if the key importer returns +// invalid public keys a [keyserrors.InvalidPublicKey] error is returned. +func (s *serviceSuite) TestImportKeysInvalidPublicKeys(c *gc.C) { + defer s.setupMocks(c).Finish() + + s.keyImporter.EXPECT().FetchPublicKeysForSubject( + gomock.Any(), + s.subjectURI, + ).Return([]string{"bad"}, nil) + + err := NewService(s.keyImporter, s.state). + ImportPublicKeysForUser(context.Background(), s.userID, s.subjectURI) + c.Check(err, jc.ErrorIs, keyserrors.InvalidPublicKey) +} + +// TestImportKeysWithReservedComment is asserting that if we import keys where +// one or more of the keys has a reserved comment we return an error that +// satisfies [keyserrors.ReservedCommentViolation]. +func (s *serviceSuite) TestImportKeysWithReservedComment(c *gc.C) { + defer s.setupMocks(c).Finish() + + s.keyImporter.EXPECT().FetchPublicKeysForSubject( + gomock.Any(), + s.subjectURI, + ).Return(reservedPublicKeys, nil) + + err := NewService(s.keyImporter, s.state). + ImportPublicKeysForUser(context.Background(), s.userID, s.subjectURI) + c.Check(err, jc.ErrorIs, keyserrors.ReservedCommentViolation) +} + +// TestImportPublicKeysForUser is asserting the happy path of +// [Service.ImportPublicKeysForUser]. +func (s *serviceSuite) TestImportPublicKeysForUser(c *gc.C) { + defer s.setupMocks(c).Finish() + + s.keyImporter.EXPECT().FetchPublicKeysForSubject( + gomock.Any(), + s.subjectURI, + ).Return(testingPublicKeys, nil) + + expectedKeys := make([]keymanager.PublicKey, 0, len(testingPublicKeys)) + for _, key := range testingPublicKeys { + keyInfo, err := ssh.ParsePublicKey(key) + c.Assert(err, jc.ErrorIsNil) + expectedKeys = append(expectedKeys, keymanager.PublicKey{ + Comment: keyInfo.Comment, + Fingerprint: keyInfo.Fingerprint(), + Key: key, + }) + } + + s.state.EXPECT().AddPublicKeyForUserIfNotFound( + gomock.Any(), + s.userID, + expectedKeys, + ).Return(nil) + + err := NewService(s.keyImporter, s.state). + ImportPublicKeysForUser(context.Background(), s.userID, s.subjectURI) + c.Check(err, jc.ErrorIsNil) +} diff --git a/domain/keymanager/types.go b/domain/keymanager/types.go new file mode 100644 index 00000000000..2d0db75bf26 --- /dev/null +++ b/domain/keymanager/types.go @@ -0,0 +1,12 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package keymanager + +// PublicKey represents the domains understand of what a public key is and the +// indiviual parts the domain cares about. +type PublicKey struct { + Comment string + Fingerprint string + Key string +} diff --git a/internal/ssh/authorisedkey.go b/internal/ssh/authorisedkey.go new file mode 100644 index 00000000000..dda11720a0e --- /dev/null +++ b/internal/ssh/authorisedkey.go @@ -0,0 +1,41 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package ssh + +import ( + "fmt" + + "golang.org/x/crypto/ssh" +) + +// PublicKey represents a single authorised key line that would commonly be +// found in a authorized_keys file. http://man.he.net/man5/authorized_keys +type PublicKey struct { + // Key holds the parse key data for the public key. + Key ssh.PublicKey + + // Comment is the comment string attached to the authorised key. + Comment string +} + +// Fingerprint returns the SHA256 fingerprint of the public key. +func (a *PublicKey) Fingerprint() string { + return ssh.FingerprintSHA256(a.Key) +} + +// ParsePublicKey parses a single line from an authorised keys file +// returning a [PublicKey] representation of the data. +// [ssh.ParseAuthorizedKey] is used to perform the underlying validating and +// parsing. +func ParsePublicKey(key string) (PublicKey, error) { + parsedKey, comment, _, _, err := ssh.ParseAuthorizedKey([]byte(key)) + if err != nil { + return PublicKey{}, fmt.Errorf("parsing public key %q: %w", key, err) + } + + return PublicKey{ + Key: parsedKey, + Comment: comment, + }, nil +} diff --git a/internal/ssh/importer/errors.go b/internal/ssh/importer/errors/errors.go similarity index 96% rename from internal/ssh/importer/errors.go rename to internal/ssh/importer/errors/errors.go index 7278b5d3afb..fe71ff23f56 100644 --- a/internal/ssh/importer/errors.go +++ b/internal/ssh/importer/errors/errors.go @@ -1,7 +1,7 @@ // Copyright 2024 Canonical Ltd. // Licensed under the AGPLv3, see LICENCE file for details. -package importer +package errors import ( "github.com/juju/errors" diff --git a/internal/ssh/importer/github.go b/internal/ssh/importer/github.go index f3c76ef2fe8..d28f537d80a 100644 --- a/internal/ssh/importer/github.go +++ b/internal/ssh/importer/github.go @@ -11,6 +11,8 @@ import ( "net/url" "github.com/juju/errors" + + importererrors "github.com/juju/juju/internal/ssh/importer/errors" ) // githubKeyResponse represents the response returned by Github for fetching a @@ -48,8 +50,8 @@ const ( // Github subject in this case a user and returning all of the public ssh keys // the user has for their profile. // The following errors can be expected: -// - [SubjectNotFound] when the subject being asked for does not exist in -// the resolvers domain. +// - [importererrors.SubjectNotFound] when the subject being asked for does not +// exist in the resolvers domain. func (g *GithubResolver) PublicKeysForSubject( ctx context.Context, subject string, @@ -81,7 +83,7 @@ func (g *GithubResolver) PublicKeysForSubject( if res.StatusCode == http.StatusNotFound { return nil, fmt.Errorf( "cannot find github user %q%w", - subject, errors.Hide(SubjectNotFound), + subject, errors.Hide(importererrors.SubjectNotFound), ) } else if res.StatusCode != http.StatusOK { return nil, fmt.Errorf( diff --git a/internal/ssh/importer/github_test.go b/internal/ssh/importer/github_test.go index 9abb8a3e34c..27a36387e80 100644 --- a/internal/ssh/importer/github_test.go +++ b/internal/ssh/importer/github_test.go @@ -15,6 +15,8 @@ import ( jc "github.com/juju/testing/checkers" gomock "go.uber.org/mock/gomock" gc "gopkg.in/check.v1" + + importererrors "github.com/juju/juju/internal/ssh/importer/errors" ) type githubSuite struct { @@ -32,7 +34,7 @@ func (s *githubSuite) setupMocks(c *gc.C) *gomock.Controller { } // TestSubjectNotFound is asserting that if the [GithubResolver] gets a 404 -// return it propagates a [SubjectNotFound] error. +// return it propagates a [importererrors.SubjectNotFound] error. func (g *githubSuite) TestSubjectNotFound(c *gc.C) { defer g.setupMocks(c).Finish() @@ -49,7 +51,7 @@ func (g *githubSuite) TestSubjectNotFound(c *gc.C) { gh := GithubResolver{g.client} _, err := gh.PublicKeysForSubject(context.Background(), "tlm") - c.Check(err, jc.ErrorIs, SubjectNotFound) + c.Check(err, jc.ErrorIs, importererrors.SubjectNotFound) } // TestSubjectPublicKeys is asserting the happy path for the [GithubResolver]. diff --git a/internal/ssh/importer/importer.go b/internal/ssh/importer/importer.go index 3cbb6233e1f..9618d81d700 100644 --- a/internal/ssh/importer/importer.go +++ b/internal/ssh/importer/importer.go @@ -9,6 +9,8 @@ import ( "net/url" "github.com/juju/errors" + + importererrors "github.com/juju/juju/internal/ssh/importer/errors" ) // Importer is responsible for providing a pattern for importing a subjects @@ -27,15 +29,20 @@ type Importer struct { // // The following errors can be expected: // - [errors.NotValid] when the opaque data for the URI is empty. -// - [NoResolver] when no resolver is defined for the schema of the URI. -// - [SubjectNotFound] when no subject exists for the [Resolver]. +// - [importererrors.NoResolver] when no resolver is defined for the schema of +// the URI. +// - [importererrors.SubjectNotFound] when no subject exists for the [Resolver]. func (i *Importer) FetchPublicKeysForSubject( ctx context.Context, subject *url.URL, ) ([]string, error) { resolver, has := i.resolvers[subject.Scheme] if !has { - return nil, fmt.Errorf("%w for subject scheme %q", NoResolver, subject.Scheme) + return nil, fmt.Errorf( + "%w for subject scheme %q", + importererrors.NoResolver, + subject.Scheme, + ) } if subject.Opaque == "" { diff --git a/internal/ssh/importer/importer_test.go b/internal/ssh/importer/importer_test.go index 63d43c252f6..1c773f4932f 100644 --- a/internal/ssh/importer/importer_test.go +++ b/internal/ssh/importer/importer_test.go @@ -12,6 +12,8 @@ import ( jc "github.com/juju/testing/checkers" gomock "go.uber.org/mock/gomock" gc "gopkg.in/check.v1" + + importererrors "github.com/juju/juju/internal/ssh/importer/errors" ) type importerSuite struct { @@ -45,7 +47,7 @@ func (i *importerSuite) TestInvalidURI(c *gc.C) { } // TestNoResolver is testing that if we ask for a subjects public key using a -// resolver that dosn't exist we get back a [NoResolver] error. +// resolver that dosn't exist we get back a [importererrors.NoResolver] error. func (i *importerSuite) TestNoResolver(c *gc.C) { defer i.setupMocks(c).Finish() @@ -57,15 +59,16 @@ func (i *importerSuite) TestNoResolver(c *gc.C) { }, } _, err = importer.FetchPublicKeysForSubject(context.Background(), uri) - c.Check(err, jc.ErrorIs, NoResolver) + c.Check(err, jc.ErrorIs, importererrors.NoResolver) } // TestSubjectNotFound is testing that if the resolver tells a subject does not -// exist we return a [SubjectNotFound] error. +// exist we return a [importererrors.SubjectNotFound] error. func (i *importerSuite) TestSubjectNotFound(c *gc.C) { defer i.setupMocks(c).Finish() - i.resolver.EXPECT().PublicKeysForSubject(gomock.Any(), "tlm").Return(nil, SubjectNotFound) + i.resolver.EXPECT().PublicKeysForSubject(gomock.Any(), "tlm"). + Return(nil, importererrors.SubjectNotFound) uri, err := url.Parse("gh:tlm") c.Assert(err, jc.ErrorIsNil) @@ -75,7 +78,7 @@ func (i *importerSuite) TestSubjectNotFound(c *gc.C) { }, } _, err = importer.FetchPublicKeysForSubject(context.Background(), uri) - c.Check(err, jc.ErrorIs, SubjectNotFound) + c.Check(err, jc.ErrorIs, importererrors.SubjectNotFound) } // TestFetchPublicKeysForSubject is testing the happy path for diff --git a/internal/ssh/importer/launchpad.go b/internal/ssh/importer/launchpad.go index 5eac4608963..50a8986c967 100644 --- a/internal/ssh/importer/launchpad.go +++ b/internal/ssh/importer/launchpad.go @@ -11,6 +11,8 @@ import ( "net/url" "github.com/juju/errors" + + importererrors "github.com/juju/juju/internal/ssh/importer/errors" ) // LaunchpadResolver is an implementation of [Resolver] for retrieving the @@ -33,8 +35,8 @@ const ( // Launchpad subject in this case a user and returning all of the public ssh // keys the user has for their profile. // The following errors can be expected: -// - [SubjectNotFound] when the subject being asked for does not exist in -// the resolvers domain. +// - [importererrors.SubjectNotFound] when the subject being asked for does not +// exist in the resolvers domain. func (l *LaunchpadResolver) PublicKeysForSubject( ctx context.Context, subject string, @@ -66,7 +68,7 @@ func (l *LaunchpadResolver) PublicKeysForSubject( if res.StatusCode == http.StatusNotFound { return nil, fmt.Errorf( "cannot find launchpad user %q%w", - subject, errors.Hide(SubjectNotFound), + subject, errors.Hide(importererrors.SubjectNotFound), ) } else if res.StatusCode != http.StatusOK { return nil, fmt.Errorf( diff --git a/internal/ssh/importer/launchpad_test.go b/internal/ssh/importer/launchpad_test.go index d25f25b57cd..2bb67cb7e55 100644 --- a/internal/ssh/importer/launchpad_test.go +++ b/internal/ssh/importer/launchpad_test.go @@ -13,6 +13,8 @@ import ( jc "github.com/juju/testing/checkers" gomock "go.uber.org/mock/gomock" gc "gopkg.in/check.v1" + + importererrors "github.com/juju/juju/internal/ssh/importer/errors" ) type launchpadSuite struct { @@ -30,7 +32,7 @@ func (s *launchpadSuite) setupMocks(c *gc.C) *gomock.Controller { } // TestSubjectNotFound is asserting that if the [LaunchpadResolver] gets a 404 -// return it propagates a [SubjectNotFound] error. +// return it propagates a [importererrors.SubjectNotFound] error. func (l *launchpadSuite) TestSubjectNotFound(c *gc.C) { defer l.setupMocks(c).Finish() @@ -47,7 +49,7 @@ func (l *launchpadSuite) TestSubjectNotFound(c *gc.C) { lp := LaunchpadResolver{l.client} _, err := lp.PublicKeysForSubject(context.Background(), "~tlm") - c.Check(err, jc.ErrorIs, SubjectNotFound) + c.Check(err, jc.ErrorIs, importererrors.SubjectNotFound) } // TestSubjectPublicKeys is asserting the happy path for the [LaunchpadResolver].