fix issues

canonical · Sep 2, 2024 · 2e60e73 · 2e60e73
1 parent 36f8f4b
commit 2e60e73
Show file tree

Hide file tree

Showing 39 changed files with 103 additions and 92 deletions.
diff --git a/cmd/jimmsrv/service/service_test.go b/cmd/jimmsrv/service/service_test.go
@@ -292,6 +292,7 @@ func TestRebacAdminApi(t *testing.T) {
 
 	response, err := srv.Client().Get(srv.URL + "/rebac/v1/swagger.json")
 	c.Assert(err, qt.IsNil)
+	defer response.Body.Close()
 	c.Assert(response.StatusCode, qt.Equals, 401)
 }
 

diff --git a/internal/common/pagination/entitlement.go b/internal/common/pagination/entitlement.go
@@ -1,4 +1,4 @@
-// Copyright 2024 Canonical Ltd.
+// Copyright 2024 Canonical.
 
 package pagination
 

diff --git a/internal/common/pagination/entitlement_test.go b/internal/common/pagination/entitlement_test.go
@@ -1,4 +1,4 @@
-// Copyright 2024 Canonical Ltd.
+// Copyright 2024 Canonical.
 
 package pagination_test
 

diff --git a/internal/common/pagination/export_test.go b/internal/common/pagination/export_test.go
@@ -1,4 +1,4 @@
-// Copyright 2024 Canonical Ltd.
+// Copyright 2024 Canonical.
 
 package pagination
 

diff --git a/internal/common/pagination/pagination.go b/internal/common/pagination/pagination.go
@@ -1,4 +1,4 @@
-// Copyright 2024 Canonical Ltd.
+// Copyright 2024 Canonical.
 
 // pagination holds common pagination patterns.
 package pagination

diff --git a/internal/common/pagination/pagination_test.go b/internal/common/pagination/pagination_test.go
@@ -1,4 +1,4 @@
-// Copyright 2024 Canonical Ltd.
+// Copyright 2024 Canonical.
 
 package pagination_test
 

diff --git a/internal/common/utils/test_utils.go b/internal/common/utils/test_utils.go
@@ -1,3 +1,4 @@
+// Copyright 2024 Canonical.
 package utils
 
 func IntToPointer(i int) *int {

diff --git a/internal/jimm/admin.go b/internal/jimm/admin.go
@@ -4,6 +4,7 @@ package jimm
 
 import (
 	"context"
+	"net/http"
 
 	"golang.org/x/oauth2"
 
@@ -22,6 +23,10 @@ func (j *JIMM) LoginDevice(ctx context.Context) (*oauth2.DeviceAuthResponse, err
 	return resp, nil
 }
 
+func (j *JIMM) AuthenticateBrowserSession(ctx context.Context, w http.ResponseWriter, r *http.Request) (context.Context, error) {
+	return j.OAuthAuthenticator.AuthenticateBrowserSession(ctx, w, r)
+}
+
 // GetDeviceSessionToken polls an OIDC server while a user logs in and returns a session token scoped to the user's identity.
 func (j *JIMM) GetDeviceSessionToken(ctx context.Context, deviceOAuthResponse *oauth2.DeviceAuthResponse) (string, error) {
 	const op = errors.Op("jimm.GetDeviceSessionToken")

diff --git a/internal/jimm/identity.go b/internal/jimm/identity.go
@@ -1,4 +1,4 @@
-// Copyright 2020 Canonical Ltd.
+// Copyright 2024 Canonical.
 
 package jimm
 

diff --git a/internal/jimm/identity_test.go b/internal/jimm/identity_test.go
@@ -1,4 +1,4 @@
-// Copyright 2021 Canonical Ltd.
+// Copyright 2024 Canonical.
 
 package jimm_test
 

diff --git a/internal/jimm/relation.go b/internal/jimm/relation.go
@@ -1,4 +1,4 @@
-// Copyright 2024 Canonical Ltd.
+// Copyright 2024 Canonical.
 
 package jimm
 

diff --git a/internal/jimm/relation_test.go b/internal/jimm/relation_test.go
@@ -1,4 +1,4 @@
-// Copyright 2024 Canonical Ltd.
+// Copyright 2024 Canonical.
 
 package jimm_test
 

diff --git a/internal/jimmtest/mocks/jimm_controller_mock.go b/internal/jimmtest/mocks/jimm_controller_mock.go
@@ -1,13 +1,15 @@
+// Copyright 2024 Canonical.
 package mocks
 
 import (
 	"context"
 
+	jujuparams "github.com/juju/juju/rpc/params"
+	"github.com/juju/version"
+
 	"github.com/canonical/jimm/v3/internal/dbmodel"
 	"github.com/canonical/jimm/v3/internal/errors"
 	"github.com/canonical/jimm/v3/internal/openfga"
-	jujuparams "github.com/juju/juju/rpc/params"
-	"github.com/juju/version"
 )
 
 // ControllerService is an implementation of the jujuapi.ControllerService interface.

diff --git a/internal/jimmtest/mocks/jimm_group_mock.go b/internal/jimmtest/mocks/jimm_group_mock.go
@@ -1,4 +1,4 @@
-// Copyright 2024 Canonical Ltd.
+// Copyright 2024 Canonical.
 
 // This package contains mocks for each JIMM service.
 // Each file contains a struct providing tests with the ability to mock

diff --git a/internal/jimmtest/mocks/jimm_relation_mock.go b/internal/jimmtest/mocks/jimm_relation_mock.go
@@ -1,4 +1,4 @@
-// Copyright 2024 Canonical Ltd.
+// Copyright 2024 Canonical.
 
 package mocks
 

diff --git a/internal/jimmtest/mocks/login.go b/internal/jimmtest/mocks/login.go
@@ -3,6 +3,7 @@ package mocks
 
 import (
 	"context"
+	"net/http"
 
 	"golang.org/x/oauth2"
 
@@ -11,11 +12,19 @@ import (
 )
 
 type LoginService struct {
-	LoginDevice_            func(ctx context.Context) (*oauth2.DeviceAuthResponse, error)
-	GetDeviceSessionToken_  func(ctx context.Context, deviceOAuthResponse *oauth2.DeviceAuthResponse) (string, error)
-	LoginClientCredentials_ func(ctx context.Context, clientID string, clientSecret string) (*openfga.User, error)
-	LoginWithSessionToken_  func(ctx context.Context, sessionToken string) (*openfga.User, error)
-	LoginWithSessionCookie_ func(ctx context.Context, identityID string) (*openfga.User, error)
+	AuthenticateBrowserSession_ func(ctx context.Context, w http.ResponseWriter, req *http.Request) (context.Context, error)
+	LoginDevice_                func(ctx context.Context) (*oauth2.DeviceAuthResponse, error)
+	GetDeviceSessionToken_      func(ctx context.Context, deviceOAuthResponse *oauth2.DeviceAuthResponse) (string, error)
+	LoginClientCredentials_     func(ctx context.Context, clientID string, clientSecret string) (*openfga.User, error)
+	LoginWithSessionToken_      func(ctx context.Context, sessionToken string) (*openfga.User, error)
+	LoginWithSessionCookie_     func(ctx context.Context, identityID string) (*openfga.User, error)
+}
+
+func (j *LoginService) AuthenticateBrowserSession(ctx context.Context, w http.ResponseWriter, req *http.Request) (context.Context, error) {
+	if j.AuthenticateBrowserSession_ == nil {
+		return nil, errors.E(errors.CodeNotImplemented)
+	}
+	return j.AuthenticateBrowserSession_(ctx, w, req)
 }
 
 func (j *LoginService) LoginDevice(ctx context.Context) (*oauth2.DeviceAuthResponse, error) {

diff --git a/internal/jimmtest/mocks/model.go b/internal/jimmtest/mocks/model.go
@@ -30,7 +30,7 @@ type ModelManager struct {
 	ModelDefaultsForCloud_  func(ctx context.Context, user *dbmodel.Identity, cloudTag names.CloudTag) (jujuparams.ModelDefaultsResult, error)
 	ModelInfo_              func(ctx context.Context, u *openfga.User, mt names.ModelTag) (*jujuparams.ModelInfo, error)
 	ModelStatus_            func(ctx context.Context, u *openfga.User, mt names.ModelTag) (*jujuparams.ModelStatus, error)
-	QueryModelsJq_          func(ctx context.Context, models []dbmodel.Model, jqQuery string) (params.CrossModelQueryResponse, error)
+	QueryModelsJq_          func(ctx context.Context, models []string, jqQuery string) (params.CrossModelQueryResponse, error)
 	SetModelDefaults_       func(ctx context.Context, user *dbmodel.Identity, cloudTag names.CloudTag, region string, configs map[string]interface{}) error
 	UnsetModelDefaults_     func(ctx context.Context, user *dbmodel.Identity, cloudTag names.CloudTag, region string, keys []string) error
 	UpdateMigratedModel_    func(ctx context.Context, user *openfga.User, modelTag names.ModelTag, targetControllerName string) error
@@ -120,7 +120,7 @@ func (j *ModelManager) ModelStatus(ctx context.Context, u *openfga.User, mt name
 	return j.ModelStatus_(ctx, u, mt)
 }
 
-func (j *ModelManager) QueryModelsJq(ctx context.Context, models []dbmodel.Model, jqQuery string) (params.CrossModelQueryResponse, error) {
+func (j *ModelManager) QueryModelsJq(ctx context.Context, models []string, jqQuery string) (params.CrossModelQueryResponse, error) {
 	if j.QueryModelsJq_ == nil {
 		return params.CrossModelQueryResponse{}, errors.E(errors.CodeNotImplemented)
 	}

diff --git a/internal/jujuapi/admin.go b/internal/jujuapi/admin.go
@@ -4,6 +4,7 @@ package jujuapi
 
 import (
 	"context"
+	"net/http"
 	"sort"
 
 	"github.com/juju/juju/rpc"
@@ -18,6 +19,8 @@ import (
 
 // LoginService defines the set of methods used for login to JIMM.
 type LoginService interface {
+	// AuthenticateBrowserSession authenticates a session cookie is valid.
+	AuthenticateBrowserSession(ctx context.Context, w http.ResponseWriter, r *http.Request) (context.Context, error)
 	// LoginDevice is step 1 in the device flow and returns the OIDC server that the client should use for login.
 	LoginDevice(ctx context.Context) (*oauth2.DeviceAuthResponse, error)
 	// GetDeviceSessionToken polls the OIDC server waiting for the client to login and return a user scoped session token.

diff --git a/internal/jujuapi/jimm_relation.go b/internal/jujuapi/jimm_relation.go
@@ -1,4 +1,4 @@
-// Copyright 2024 Canonical Ltd.
+// Copyright 2024 Canonical.
 
 package jujuapi
 

diff --git a/internal/jujuapi/modelmanager.go b/internal/jujuapi/modelmanager.go
@@ -70,7 +70,7 @@ type ModelManager interface {
 	ModelDefaultsForCloud(ctx context.Context, user *dbmodel.Identity, cloudTag names.CloudTag) (jujuparams.ModelDefaultsResult, error)
 	ModelInfo(ctx context.Context, u *openfga.User, mt names.ModelTag) (*jujuparams.ModelInfo, error)
 	ModelStatus(ctx context.Context, u *openfga.User, mt names.ModelTag) (*jujuparams.ModelStatus, error)
-	QueryModelsJq(ctx context.Context, models []dbmodel.Model, jqQuery string) (params.CrossModelQueryResponse, error)
+	QueryModelsJq(ctx context.Context, models []string, jqQuery string) (params.CrossModelQueryResponse, error)
 	SetModelDefaults(ctx context.Context, user *dbmodel.Identity, cloudTag names.CloudTag, region string, configs map[string]interface{}) error
 	UnsetModelDefaults(ctx context.Context, user *dbmodel.Identity, cloudTag names.CloudTag, region string, keys []string) error
 	UpdateMigratedModel(ctx context.Context, user *openfga.User, modelTag names.ModelTag, targetControllerName string) error

diff --git a/internal/jujuapi/service_account.go b/internal/jujuapi/service_account.go
@@ -72,7 +72,7 @@ func (r *controllerRoot) getServiceAccount(ctx context.Context, clientID string)
 		return nil, errors.E(errors.CodeUnauthorized, "unauthorized")
 	}
 
-	return r.jimm.GetUser(ctx, clientIdWithDomain)
+	return r.jimm.UserLogin(ctx, clientIdWithDomain)
 }
 
 // UpdateServiceAccountCredentialsCheckModels updates a set of cloud credentials' content.

diff --git a/internal/jujuapi/service_account_test.go b/internal/jujuapi/service_account_test.go
@@ -175,7 +175,7 @@ func TestCopyServiceAccountCredential(t *testing.T) {
 					newCredTag := names.NewCloudCredentialTag(fmt.Sprintf("%s/%s/%s", test.args.CloudName, svcAcc.Name, test.args.CredentialName))
 					return newCredTag, nil, nil
 				},
-				GetUser_: func(ctx context.Context, email string) (*openfga.User, error) {
+				UserLogin_: func(ctx context.Context, email string) (*openfga.User, error) {
 					var u dbmodel.Identity
 					u.SetTag(names.NewUserTag(email))
 					return openfga.NewUser(&u, ofgaClient), nil
@@ -259,7 +259,7 @@ func TestGetServiceAccount(t *testing.T) {
 			err = pgDb.Migrate(context.Background(), false)
 			c.Assert(err, qt.IsNil)
 			jimm := &jimmtest.JIMM{
-				GetUser_: func(ctx context.Context, email string) (*openfga.User, error) {
+				UserLogin_: func(ctx context.Context, email string) (*openfga.User, error) {
 					var u dbmodel.Identity
 					u.SetTag(names.NewUserTag(email))
 					return openfga.NewUser(&u, ofgaClient), nil
@@ -453,7 +453,7 @@ func TestUpdateServiceAccountCredentials(t *testing.T) {
 			c.Assert(err, qt.IsNil)
 			jimm := &jimmtest.JIMM{
 				UpdateCloudCredential_: test.updateCloudCredential,
-				GetUser_:               func(ctx context.Context, email string) (*openfga.User, error) { return nil, nil },
+				UserLogin_:             func(ctx context.Context, email string) (*openfga.User, error) { return nil, nil },
 			}
 			var u dbmodel.Identity
 			u.SetTag(names.NewUserTag(test.username))
@@ -586,7 +586,7 @@ func TestListServiceAccountCredentials(t *testing.T) {
 				GetCloudCredential_:           test.getCloudCredential,
 				GetCloudCredentialAttributes_: test.getCloudCredentialAttributes,
 				ForEachUserCloudCredential_:   test.ForEachUserCloudCredential,
-				GetUser_: func(ctx context.Context, email string) (*openfga.User, error) {
+				UserLogin_: func(ctx context.Context, email string) (*openfga.User, error) {
 					var u dbmodel.Identity
 					u.SetTag(names.NewUserTag(email))
 					return openfga.NewUser(&u, ofgaClient), nil
@@ -702,7 +702,7 @@ func TestGrantServiceAccountAccess(t *testing.T) {
 			err = pgDb.Migrate(context.Background(), false)
 			c.Assert(err, qt.IsNil)
 			jimm := &jimmtest.JIMM{
-				GetUser_:                   func(ctx context.Context, email string) (*openfga.User, error) { return nil, nil },
+				UserLogin_:                 func(ctx context.Context, email string) (*openfga.User, error) { return nil, nil },
 				GrantServiceAccountAccess_: test.grantServiceAccountAccess,
 			}
 			var u dbmodel.Identity

diff --git a/internal/middleware/auth.go b/internal/middleware/auth.go
@@ -1,22 +1,22 @@
-// Copyright 2024 Canonical Ltd.
+// Copyright 2024 Canonical.
 
 package middleware
 
 import (
 	"net/http"
 
+	rebac_handlers "github.com/canonical/rebac-admin-ui-handlers/v1"
 	"github.com/juju/zaputil/zapctx"
 	"go.uber.org/zap"
 
 	"github.com/canonical/jimm/v3/internal/auth"
 	"github.com/canonical/jimm/v3/internal/jujuapi"
-	rebac_handlers "github.com/canonical/rebac-admin-ui-handlers/v1"
 )
 
 // AuthenticateViaCookie performs browser session authentication and puts an identity in the request's context
 func AuthenticateViaCookie(next http.Handler, jimm jujuapi.JIMM) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		ctx, err := jimm.OAuthAuthenticationService().AuthenticateBrowserSession(r.Context(), w, r)
+		ctx, err := jimm.AuthenticateBrowserSession(r.Context(), w, r)
 		if err != nil {
 			zapctx.Error(ctx, "failed to authenticate", zap.Error(err))
 			http.Error(w, "failed to authenticate", http.StatusUnauthorized)
@@ -41,21 +41,17 @@ func AuthenticateRebac(next http.Handler, jimm jujuapi.JIMM) http.Handler {
 			return
 		}
 
-		user, err := jimm.GetUser(ctx, identity)
+		user, err := jimm.UserLogin(ctx, identity)
 		if err != nil {
 			zapctx.Error(ctx, "failed to get openfga user", zap.Error(err))
 			http.Error(w, "internal authentication error", http.StatusInternalServerError)
 			return
 		}
 		if !user.JimmAdmin {
 			w.WriteHeader(http.StatusUnauthorized)
-			w.Write([]byte("user is not an admin"))
+			_, _ = w.Write([]byte("user is not an admin"))
 			return
 		}
-		err = jimm.UpdateUserLastLogin(ctx, identity)
-		if err != nil {
-			zapctx.Error(ctx, "failed to update user last login", zap.Error(err))
-		}
 
 		ctx = rebac_handlers.ContextWithIdentity(ctx, user)
 		next.ServeHTTP(w, r.WithContext(ctx))