diff --git a/internal/jimmtest/auth.go b/internal/jimmtest/auth.go index e0f98632f..49a79c063 100644 --- a/internal/jimmtest/auth.go +++ b/internal/jimmtest/auth.go @@ -125,8 +125,9 @@ func (m *MockOAuthAuthenticator) VerifySessionToken(token string) (jwt.Token, er return parsedToken, nil } -// ExtractAndVerifyIDToken return an ID token where the subject matches the subject obtained during the device flow. +// ExtractAndVerifyIDToken returns an ID token where the subject is equal to the username obtained during the device flow. // The auth token must match the one returned during the device flow. +// If the polled username is empty it indicates an error that the device flow was not run prior to calling this function. func (m *MockOAuthAuthenticator) ExtractAndVerifyIDToken(ctx context.Context, oauth2Token *oauth2.Token) (*oidc.IDToken, error) { if m.polledUsername == "" { return &oidc.IDToken{}, errors.New("unknown user for mock auth login") @@ -157,8 +158,6 @@ func (m *MockOAuthAuthenticator) AuthenticateBrowserSession(ctx context.Context, return ctx, errors.New("authentication failed") } -// Below are helper functions used for handling auth in tests. - func newUnsignedSessionToken(c SimpleTester, username string) string { email := convertUsernameToEmail(username) token, err := jwt.NewBuilder(). diff --git a/internal/jimmtest/suite.go b/internal/jimmtest/suite.go index abf6bca0c..b325f0f68 100644 --- a/internal/jimmtest/suite.go +++ b/internal/jimmtest/suite.go @@ -249,7 +249,7 @@ func (s *JIMMSuite) AddModel(c *gc.C, owner names.UserTag, name string, cloud na return names.NewModelTag(mi.UUID) } -// Call this non-blocking function before login to ensure device flow won't block. +// Call this non-blocking function before login to ensure the device flow won't block. // // This is necessary as the mock authenticator simulates polling an external OIDC server. func (s *JIMMSuite) ContinueDeviceFlow(username string) { diff --git a/internal/jujuapi/websocket_test.go b/internal/jujuapi/websocket_test.go index 627bcb210..95def8b69 100644 --- a/internal/jujuapi/websocket_test.go +++ b/internal/jujuapi/websocket_test.go @@ -13,6 +13,7 @@ import ( "net/url" "github.com/juju/juju/api" + "github.com/juju/juju/api/client/client" "github.com/juju/juju/rpc/jsoncodec" jujuparams "github.com/juju/juju/rpc/params" "github.com/juju/names/v5" @@ -99,13 +100,17 @@ func (s *websocketSuite) TearDownTest(c *gc.C) { s.BootstrapSuite.TearDownTest(c) } +type loginDetails struct { + info *api.Info + username string + lp api.LoginProvider + dialWebsocket func(ctx context.Context, urlStr string, tlsConfig *tls.Config, ipAddr string) (jsoncodec.JSONConn, error) +} + // openNoAssert creates a new websocket connection to the test server, using the // connection info specified in info, authenticating as the given user. // If info is nil then default values will be used. -func (s *websocketSuite) openNoAssert( - c *gc.C, - d loginDetails, -) (api.Connection, error) { +func (s *websocketSuite) openNoAssert(c *gc.C, d loginDetails) (api.Connection, error) { var inf api.Info if d.info != nil { inf = *d.info @@ -139,13 +144,6 @@ func (s *websocketSuite) openNoAssert( return api.Open(&inf, dialOpts) } -type loginDetails struct { - info *api.Info - username string - lp api.LoginProvider - dialWebsocket func(ctx context.Context, urlStr string, tlsConfig *tls.Config, ipAddr string) (jsoncodec.JSONConn, error) -} - func (s *websocketSuite) open(c *gc.C, info *api.Info, username string) api.Connection { ld := loginDetails{info: info, username: username} conn, err := s.openNoAssert(c, ld) @@ -209,10 +207,46 @@ func (s *proxySuite) TestDeviceFlowLogin(c *gc.C) { c.Check(cliOutput, gc.Matches, "Please visit .* and enter code.*") } +type logger struct{} + +func (l logger) Errorf(string, ...interface{}) {} + +func (s *proxySuite) TestModelStatus(c *gc.C) { + conn := s.open(c, &api.Info{ + ModelTag: s.Model.ResourceTag(), + SkipLogin: false, + }, "alice@canonical.com") + defer conn.Close() + jujuClient := client.NewClient(conn, logger{}) + status, err := jujuClient.Status(nil) + c.Check(err, gc.IsNil) + c.Check(status, gc.Not(gc.IsNil)) + c.Check(status.Model.Name, gc.Equals, s.Model.Name) +} + +func (s *proxySuite) TestModelStatusWithoutPermission(c *gc.C) { + fooUser := openfga.NewUser(&dbmodel.Identity{Name: "foo@canonical.com"}, s.JIMM.OpenFGAClient) + var cliOutput string + outputFunc := func(format string, a ...any) error { + cliOutput = fmt.Sprintf(format, a...) + return nil + } + s.JIMMSuite.ContinueDeviceFlow(fooUser.Name) + conn, err := s.openCustomLP(c, &api.Info{ + ModelTag: s.Model.ResourceTag(), + SkipLogin: false, + }, "foo", api.NewSessionTokenLoginProvider("", outputFunc, func(s string) error { return nil })) + c.Check(err, gc.ErrorMatches, "permission denied .*") + if conn != nil { + defer conn.Close() + } + c.Check(cliOutput, gc.Matches, "Please visit .* and enter code.*") +} + // TODO(Kian): This test aims to verify that JIMM gracefully handles clients that end their connection // during the login flow after JIMM starts polling the OIDC server. -// After https://github.com/juju/juju/pull/17606 lands We can refactor -// the API connection login method to use the loging provider on the state struct. +// After https://github.com/juju/juju/pull/17606 lands we can begin work on this. +// The API connection's login method should be refactored use the login provider stored on the state struct. // func (s *proxySuite) TestDeviceFlowCancelDuringPolling(c *gc.C) { // ctx := context.Background() // alice := names.NewUserTag("alice@canonical.com") diff --git a/internal/rpc/proxy.go b/internal/rpc/proxy.go index ae6a081b3..23e561c71 100644 --- a/internal/rpc/proxy.go +++ b/internal/rpc/proxy.go @@ -339,7 +339,7 @@ func (p *clientProxy) start(ctx context.Context) error { } // If there is a response for the client, send it to the client and continue. // If there is a message for the controller instead, use the normal path. - // We can't send the client a response from JIMM and send the client message to the controller. + // We can't send the client a response from JIMM and send a message to the controller. if toClient != nil { p.src.sendMessage(nil, toClient) continue