Skip to content

feature 4877: Added auth middleware support for controller tests #266

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
davidbolet wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feature 4877: Added auth middleware support for controller tests #266

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
236735f
KMS20-4877_added_auth_middleware_support
davidbolet Apr 24, 2026
57b41c0
linter errors
davidbolet Apr 24, 2026
328f841
Increased sonarqube coverage
davidbolet Apr 24, 2026
6c353ca
Merge branch 'main' into feature/KMS20-4877-client_data_headers
davidbolet Apr 24, 2026
ed37ecb
Addressed comments
davidbolet Apr 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 100 additions & 31 deletions internal/controllers/cmk/tenant_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,42 @@
"strings"
"testing"

"github.com/google/uuid"
"github.com/openkcm/common-sdk/pkg/auth"
"github.com/stretchr/testify/assert"

multitenancy "github.com/bartventer/gorm-multitenancy/v8"

"github.com/openkcm/cmk/internal/api/cmkapi"
"github.com/openkcm/cmk/internal/config"
"github.com/openkcm/cmk/internal/constants"
"github.com/openkcm/cmk/internal/model"
"github.com/openkcm/cmk/internal/repo"
"github.com/openkcm/cmk/internal/repo/sql"
"github.com/openkcm/cmk/internal/testutils"
cmkContext "github.com/openkcm/cmk/utils/context"
)

func startAPITenant(t *testing.T) (*multitenancy.DB, cmkapi.ServeMux) {
func startAPITenant(t *testing.T) (*multitenancy.DB, cmkapi.ServeMux, *testutils.TestSigningKeyStorage) {
t.Helper()

db, _, dbCfg := testutils.NewTestDB(t, testutils.TestDBConfig{
CreateDatabase: true,
}, testutils.WithGenerateTenants(10))

keyStorage := testutils.NewTestSigningKeyStorage(t)
return db, testutils.NewAPIServer(t, db, testutils.TestAPIServerConfig{
Config: config.Config{Database: dbCfg},
})
Config: config.Config{Database: dbCfg},
EnableClientDataMW: true,
SigningKeyStorage: keyStorage,
}), keyStorage
}

func TestGetTenants(t *testing.T) {
db, sv := startAPITenant(t)
db, sv, keyStorage := startAPITenant(t)
r := sql.NewRepository(db)

var tenants []model.Tenant
var headers http.Header

err := r.List(t.Context(), model.Tenant{}, &tenants, *repo.NewQuery())
assert.NoError(t, err)
Expand All @@ -55,13 +60,26 @@
assert.NoError(t, err)
}

clientData := &auth.ClientData{
Identifier: "user-123",
Email: "bob@example.com",
GivenName: "Bob",
FamilyName: "Builder",
Groups: []string{"sysadmin", "some-other-group"},
}
// Get private key for signing test requests
privateKey, ok := keyStorage.GetPrivateKey(0)
assert.True(t, ok, "test key should exist")

headers = testutils.NewSignedClientDataHeadersFromStruct(t, clientData, privateKey, 0)
assert.NotEmpty(t, headers)

t.Run("Should 200 on list tenants", func(t *testing.T) {
w := testutils.MakeHTTPRequest(t, sv, testutils.RequestOptions{
Method: http.MethodGet,
Endpoint: "/tenants",
Tenant: tenants[0].ID,
AdditionalContext: testutils.GetClientMap("test",
[]string{"sysadmin", "othergroup"}),
Headers: headers,
})

assert.Equal(t, http.StatusOK, w.Code)
Expand All @@ -74,20 +92,32 @@
Method: http.MethodGet,
Endpoint: "/tenants",
Tenant: "non-existing-tenant-id",
AdditionalContext: testutils.GetClientMap("test",
[]string{"sysadmin", "othergroup"}),
Headers: headers,
})

assert.Equal(t, http.StatusForbidden, w.Code)
})

t.Run("Should 403 on list tenants without permission", func(t *testing.T) {

Check failure on line 101 in internal/controllers/cmk/tenant_controller_test.go

View workflow job for this annotation

GitHub Actions / check / validate-and-testing 

unnecessary leading newline (whitespace)

notAllowedClientData := &auth.ClientData{
Identifier: "user-123",
Email: "bob@example.com",
GivenName: "Bob",
FamilyName: "Builder",
Groups: []string{"test", "some-other-test-group"},
}
// Get private key for signing test requests
privateKey, ok := keyStorage.GetPrivateKey(0)
assert.True(t, ok, "test key should exist")

headersNotAllowed := testutils.NewSignedClientDataHeadersFromStruct(t, notAllowedClientData, privateKey, 0)

w := testutils.MakeHTTPRequest(t, sv, testutils.RequestOptions{
Method: http.MethodGet,
Endpoint: "/tenants",
Tenant: tenants[0].ID,
AdditionalContext: testutils.GetClientMap("test",
[]string{"othergroup"}),
Headers: headersNotAllowed,
})

assert.Equal(t, http.StatusForbidden, w.Code)
Expand All @@ -98,81 +128,120 @@
}

func TestGetTenantInfo(t *testing.T) {
db, sv := startAPITenant(t)
db, sv, keyStorage := startAPITenant(t)
r := sql.NewRepository(db)

var tenant model.Tenant
var headers http.Header

_, err := r.First(t.Context(), &tenant, *repo.NewQuery())
assert.NoError(t, err)

tenantCtx := cmkContext.CreateTenantContext(t.Context(), tenant.ID)

authClient := testutils.NewAuthClient(tenantCtx, t, r, testutils.WithTenantAdminRole())
tenant.IssuerURL = "https://testissuer.example.com"
_, err = r.Patch(tenantCtx, &tenant, *repo.NewQuery())
assert.NoError(t, err)

group := testutils.NewGroup(func(group *model.Group) {
group.IAMIdentifier = "sysadmin"
group.Role = constants.TenantAdminRole
})

err = r.Create(tenantCtx, group)
assert.NoError(t, err)

t.Run("Should 403 on get tenant info that does not exist", func(t *testing.T) {
w := testutils.MakeHTTPRequest(t, sv, testutils.RequestOptions{
Method: http.MethodGet,
Endpoint: "/tenantInfo",
Tenant: "nonexistent-tenant-id",
AdditionalContext: authClient.GetClientMap(
testutils.WithAdditionalGroup(uuid.NewString())),
})
clientData := &auth.ClientData{
Identifier: "user-123",
Email: "bob@example.com",
GivenName: "Bob",
FamilyName: "Builder",
Groups: []string{group.IAMIdentifier, "some-other-test-group"},
}
// Get private key for signing test requests
privateKey, ok := keyStorage.GetPrivateKey(0)
assert.True(t, ok, "test key should exist")
headers = testutils.NewSignedClientDataHeadersFromStruct(t, clientData, privateKey, 0)
assert.NotEmpty(t, headers)

clientDataNoGroups := &auth.ClientData{
Identifier: "user-123",
Email: "bob@example.com",
GivenName: "Bob",
FamilyName: "Builder",
Groups: []string{},
}
headersNoGroups := testutils.NewSignedClientDataHeadersFromStruct(t, clientDataNoGroups, privateKey, 0)
assert.NotEmpty(t, headersNoGroups)

clientDataInvalidGroup := &auth.ClientData{
Identifier: "user-123",
Email: "bob@example.com",
GivenName: "Bob",
FamilyName: "Builder",
Groups: []string{"not-existing-group"},
}
headersInvalidGroup := testutils.NewSignedClientDataHeadersFromStruct(t, clientDataInvalidGroup, privateKey, 0)
assert.NotEmpty(t, headersInvalidGroup)

assert.Equal(t, http.StatusForbidden, w.Code)
})
t.Run("Should 403 on get tenant info that does not exist", func(t *testing.T) {

Check failure on line 187 in internal/controllers/cmk/tenant_controller_test.go

View workflow job for this annotation

GitHub Actions / check / validate-and-testing 

unnecessary leading newline (whitespace)

t.Run("Should 403 on get tenant info without a user group", func(t *testing.T) {
w := testutils.MakeHTTPRequest(t, sv, testutils.RequestOptions{
Method: http.MethodGet,
Endpoint: "/tenantInfo",
Tenant: "nonexistent-tenant-id",
Headers: headers,
})

assert.Equal(t, http.StatusForbidden, w.Code)
})

t.Run("Should 200 on get tenant by valid ID and client data", func(t *testing.T) {

Check failure on line 199 in internal/controllers/cmk/tenant_controller_test.go

View workflow job for this annotation

GitHub Actions / check / validate-and-testing 

unnecessary leading newline (whitespace)

w := testutils.MakeHTTPRequest(t, sv, testutils.RequestOptions{
Method: http.MethodGet,
Endpoint: "/tenantInfo",
Tenant: tenant.ID,
AdditionalContext: authClient.GetClientMap(
testutils.WithAdditionalGroup(uuid.NewString())),
Headers: headers,
})

assert.Equal(t, http.StatusOK, w.Code)
resp := testutils.GetJSONBody[cmkapi.Tenant](t, w)
assert.NotNil(t, resp.Id)
assert.Equal(t, tenant.ID, *resp.Id)
assert.NotNil(t, resp.Role)
expectedRole := strings.TrimPrefix(string(tenant.Role), "ROLE_")
assert.Equal(t, cmkapi.TenantRole(expectedRole), *resp.Role)
assert.Equal(t, tenant.Name, resp.Name)
})

t.Run("Should 403 on get tenant by valid ID and no client data", func(t *testing.T) {
t.Run("Should 403 on get tenant info without a user group", func(t *testing.T) {
w := testutils.MakeHTTPRequest(t, sv, testutils.RequestOptions{
Method: http.MethodGet,
Endpoint: "/tenantInfo",
Tenant: tenant.ID,
Headers: headersNoGroups,
})

assert.Equal(t, http.StatusForbidden, w.Code)
})

t.Run("Should 500 on get tenant by valid ID and no client data", func(t *testing.T) {
w := testutils.MakeHTTPRequest(t, sv, testutils.RequestOptions{
Method: http.MethodGet,
Endpoint: "/tenantInfo",
Tenant: tenant.ID,
})

assert.Equal(t, http.StatusInternalServerError, w.Code)
Comment on lines +226 to +233
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test now asserts that missing client data yields HTTP 500. Since this is an authentication/authorization precondition, a 4xx status (typically 401/403) is usually more appropriate than a server error. If the 500 comes from apierrors.ErrNoClientData, consider changing that API error’s Status (and then update this test accordingly) so clients can distinguish auth failures from internal faults.

Suggested change
t.Run("Should 500 on get tenant by valid ID and no client data", func(t *testing.T) {
w := testutils.MakeHTTPRequest(t, sv, testutils.RequestOptions{
Method: http.MethodGet,
Endpoint: "/tenantInfo",
Tenant: tenant.ID,
})
assert.Equal(t, http.StatusInternalServerError, w.Code)
t.Run("Should 401 on get tenant by valid ID and no client data", func(t *testing.T) {
w := testutils.MakeHTTPRequest(t, sv, testutils.RequestOptions{
Method: http.MethodGet,
Endpoint: "/tenantInfo",
Tenant: tenant.ID,
})
assert.Equal(t, http.StatusUnauthorized, w.Code)

Copilot uses AI. Check for mistakes.
})

t.Run("Should 403 on get tenant by valid ID and no valid group", func(t *testing.T) {
w := testutils.MakeHTTPRequest(t, sv, testutils.RequestOptions{
Method: http.MethodGet,
Endpoint: "/tenantInfo",
Tenant: tenant.ID,
AdditionalContext: authClient.GetClientMap(testutils.WithOverriddenGroup(1)),
Method: http.MethodGet,
Endpoint: "/tenantInfo",
Tenant: tenant.ID,
Headers: headersInvalidGroup,
})

assert.Equal(t, http.StatusForbidden, w.Code)
Expand Down
55 changes: 32 additions & 23 deletions internal/controllers/cmk/userinfo_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,48 +11,56 @@ import (

"github.com/openkcm/cmk/internal/api/cmkapi"
"github.com/openkcm/cmk/internal/config"
"github.com/openkcm/cmk/internal/constants"
"github.com/openkcm/cmk/internal/model"
"github.com/openkcm/cmk/internal/repo/sql"
"github.com/openkcm/cmk/internal/testutils"
cmkcontext "github.com/openkcm/cmk/utils/context"
)

func startAPIUserInfo(t *testing.T) (*multitenancy.DB, cmkapi.ServeMux, string) {
func startAPIUserInfo(t *testing.T) (*multitenancy.DB, cmkapi.ServeMux, string, *testutils.TestSigningKeyStorage) {
t.Helper()

db, tenants, dbCfg := testutils.NewTestDB(t, testutils.TestDBConfig{
CreateDatabase: true,
})

keyStorage := testutils.NewTestSigningKeyStorage(t)

return db, testutils.NewAPIServer(t, db, testutils.TestAPIServerConfig{
Config: config.Config{Database: dbCfg},
}), tenants[0]
Config: config.Config{Database: dbCfg},
EnableClientDataMW: true,
SigningKeyStorage: keyStorage,
}), tenants[0], keyStorage
}

func TestGetUserInfo(t *testing.T) {
db, sv, tenant := startAPIUserInfo(t)
db, sv, tenant, keyStorage := startAPIUserInfo(t)
r := sql.NewRepository(db)

ctx := cmkcontext.CreateTenantContext(t.Context(), tenant)

// Get private key for signing test requests
privateKey, ok := keyStorage.GetPrivateKey(0)
assert.True(t, ok, "test key should exist")

t.Run("Should 200 on get user info with good client data", func(t *testing.T) {
group := testutils.NewGroup(func(_ *model.Group) {})
testutils.CreateTestEntities(ctx, t, r, group)

clientData := &auth.ClientData{
Identifier: "user-123",
Email: "bob@example.com",
GivenName: "Bob",
FamilyName: "Builder",
Groups: []string{group.IAMIdentifier, "some-other-group"},
}
headers := testutils.NewSignedClientDataHeadersFromStruct(t, clientData, privateKey, 0)

w := testutils.MakeHTTPRequest(t, sv, testutils.RequestOptions{
Method: http.MethodGet,
Endpoint: "/userInfo",
Tenant: tenant,
AdditionalContext: map[any]any{
constants.ClientData: &auth.ClientData{
Identifier: "user-123",
Email: "bob@example.com",
GivenName: "Bob",
FamilyName: "Builder",
Groups: []string{group.IAMIdentifier, "some-other-group"},
},
},
Headers: headers,
})

assert.Equal(t, http.StatusOK, w.Code)
Expand All @@ -66,19 +74,20 @@ func TestGetUserInfo(t *testing.T) {
})

t.Run("Should 200 on get user info without group", func(t *testing.T) {
clientData := &auth.ClientData{
Identifier: "user-123",
Email: "bob@example.com",
GivenName: "Bob",
FamilyName: "Builder",
Groups: []string{"some-other-group"},
}
headers := testutils.NewSignedClientDataHeadersFromStruct(t, clientData, privateKey, 0)

w := testutils.MakeHTTPRequest(t, sv, testutils.RequestOptions{
Comment thread
davidbolet marked this conversation as resolved.
Method: http.MethodGet,
Endpoint: "/userInfo",
Tenant: tenant,
AdditionalContext: map[any]any{
constants.ClientData: &auth.ClientData{
Identifier: "user-123",
Email: "bob@example.com",
GivenName: "Bob",
FamilyName: "Builder",
Groups: []string{"some-other-group"},
},
},
Headers: headers,
})

assert.Equal(t, http.StatusOK, w.Code)
Expand Down
Loading
Loading