chore: add unit tests for controller authz failures by niazkhansap · Pull Request #262 · openkcm/cmk

niazkhansap · 2026-04-22T14:12:00Z

Summary by CodeRabbit

Release Notes

Tests
- Added authorization test suite with integration capabilities for verifying API access control across endpoints.

coderabbitai · 2026-04-22T14:12:08Z

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7a3f8909-62c1-4b9a-8255-ac11781a809e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Walkthrough

This change adds authorization testing infrastructure consisting of two components. First, a new integration test in authz_test.go that starts a test database and API server, then validates that restricted endpoints deny access with 403 Forbidden responses for unauthorized roles. Second, a test utility layer in internal/testutils/authz.go provides the RunAuthzFailureTests function and supporting helpers that derive authorization restrictions from policy data, enumerate blocked roles for each endpoint, and execute per-role authorization failure subtests. Together, these additions establish a framework for verifying authorization policy enforcement across the API.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

niazkhansap · 2026-04-23T08:51:26Z

@coderabbitai summary

coderabbitai · 2026-04-23T08:51:32Z

✅ Actions performed

Summary regeneration triggered.

minh-nghia · 2026-04-23T12:09:26Z

+}
+
+// randomUUID returns a new random UUID string for use in endpoint paths
+func randomUUID() string {


Seems to be overkill to me. Can we not use uuid.New().String() directly below?

minh-nghia · 2026-04-23T12:10:06Z

+}
+
+// jsonBody returns a Body function that provides the given JSON string
+func jsonBody(s string) func(testing.TB) io.Reader {


Should this not be in the testutils function? We can declare JSON strings in the test and let the utility do the json marshalling

minh-nghia · 2026-04-23T12:12:39Z

+
+	for _, ep := range endpoints {
+		pattern := ep.Pattern
+		if pattern == "" {


sv is created by startAPIServer so it's an instance of daemon.ServeMux which wraps http.ServeMux.
There is a function in http.ServeMux called Handler which should know how to do the pattern matching, we shouldn't have to do it again here

I would remove Pattern from the test fixtue and extend daemon.ServeMux with

func (m *ServeMux) Handler(r *http.Request) (http.Handler, string) { return m.httpServeMux.Handler(r) }

and derive the pattern as

func RunAuthzFailureTests( t *testing.T, sv *daemon.ServeMux, tenant string, r repo.Repo, ctx context.Context, endpoints []AuthzTestEndpoint, ) { t.Helper() for _, ep := range endpoints { req := NewHTTPRequest(t, RequestOptions{ //nolint:contextcheck Method: ep.Method, Endpoint: ep.Endpoint, }) _, pattern := sv.Handler(req) pattern = strings.Replace(pattern, sv.BaseURL, "", 1) // strip /cmk/v1/{tenant}

minh-nghia · 2026-04-24T08:52:47Z

+
+	sv, ok := testutils.NewAPIServer(t, db, testutils.TestAPIServerConfig{
+		Config: config.Config{Database: dbCfg},
+	}).(*daemon.ServeMux)


Np: Can be split to 2 steps for clarity

jmpTeixeira02 · 2026-04-24T10:37:30Z

+	sv, ok := server.(*daemon.ServeMux)
+	if !ok {
+		t.Fatal("expected *daemon.ServeMux")
+	}


Use assertify to remain consistant

jmpTeixeira02 · 2026-04-24T10:37:37Z

+		Config: config.Config{Database: dbCfg},
+	})
+
+	sv, ok := server.(*daemon.ServeMux)


Why the cast?

jmpTeixeira02 · 2026-04-24T10:39:22Z

+// RunAuthzFailureTests runs authorization failure tests for the provided
+// endpoints. For each endpoint it uses the ServeMux to resolve the registered
+// pattern, determines which roles should be blocked based on the policy data,
+// and asserts that each blocked role receives 403 Forbidden.
+func RunAuthzFailureTests(
+	t *testing.T,
+	sv *daemon.ServeMux,
+	tenant string,
+	r repo.Repo,
+	ctx context.Context,
+	endpoints []AuthzTestEndpoint,
+) {
+	t.Helper()
+
+	for _, ep := range endpoints {
+		req := NewHTTPRequest(t, RequestOptions{ //nolint:contextcheck
+			Method:   ep.Method,
+			Endpoint: ep.Endpoint,
+			Tenant:   tenant,
+		})
+
+		_, pattern := sv.Handler(req)
+		pattern = strings.Replace(pattern, sv.BaseURL, "", 1)
+
+		restriction, exists := authz.RestrictionsByAPI[pattern]
+		if !exists {
+			t.Fatalf(
+				"no authz restriction found for pattern %q on %s %s",
+				pattern, ep.Method, ep.Endpoint,
+			)
+		}
+
+		blockedRoles := getBlockedRoles(
+			restriction.APIResourceTypeName, restriction.APIAction,
+		)
+		if len(blockedRoles) == 0 {
+			t.Logf("all roles are allowed for %q (%s:%s), skipping",
+				pattern, restriction.APIResourceTypeName, restriction.APIAction)
+
+			continue
+		}
+
+		for _, role := range blockedRoles {
+			runAuthzBlockedSubtest(t, sv, tenant, r, ctx, ep, role)
+		}
+	}
+}
+
+// runAuthzBlockedSubtest executes a single subtest asserting that the given
+// role is blocked (403 Forbidden) for the endpoint.
+func runAuthzBlockedSubtest(
+	t *testing.T,
+	sv *daemon.ServeMux,
+	tenant string,
+	r repo.Repo,
+	ctx context.Context,
+	ep AuthzTestEndpoint,
+	role constants.Role,
+) {
+	t.Helper()
+
+	testName := fmt.Sprintf(
+		"%s_%s_blocked_for_%s", ep.Method, cleanPath(ep.Endpoint), role,
+	)
+
+	t.Run(testName, func(t *testing.T) {
+		authClient := NewAuthClient(ctx, t, r, roleAuthClientOpt(role))
+
+		w := MakeHTTPRequest(t, sv, RequestOptions{ //nolint:contextcheck
+			Method:            ep.Method,
+			Endpoint:          ep.Endpoint,
+			Tenant:            tenant,
+			Body:              WithBody(t, ep.Body),
+			AdditionalContext: authClient.GetClientMap(),
+		})
+
+		assert.Equal(t, http.StatusForbidden, w.Code,
+			"expected 403 for role %s on %s %s, got %d: %s",
+			role, ep.Method, ep.Endpoint, w.Code, w.Body.String())
+	})
+}


This shouldn't be on testutils, these are the tests themselves

petersbingham · 2026-04-27T15:55:52Z

+
+			continue
+		}
+


Would it be worth adding, for sanity, a single test for allowed role?

petersbingham · 2026-04-27T16:00:02Z

@@ -0,0 +1,359 @@
+//go:build !unit


Maybe unauthz_test.go would be a better name?
If the case be leave a commit for the sanity test suggested in other comment to leave a code comment to indicate that positive authz is for sanity test.

push-tags-from-workflow Bot added tests test labels Apr 22, 2026

niazkhansap force-pushed the task/authz-failures-tests branch 4 times, most recently from 925a0ea to d8a8264 Compare April 22, 2026 15:46

niazkhansap requested review from MarkMcAleese, bkieling, doanhluongsap, jmpTeixeira02, minh-nghia and petersbingham April 23, 2026 11:31

minh-nghia requested changes Apr 23, 2026

View reviewed changes

niazkhansap force-pushed the task/authz-failures-tests branch 3 times, most recently from bf27f31 to aea82f6 Compare April 23, 2026 17:46

anettlippert assigned niazkhansap Apr 24, 2026

minh-nghia previously approved these changes Apr 24, 2026

View reviewed changes

niazkhansap dismissed minh-nghia’s stale review via 83a3515 April 24, 2026 09:30

niazkhansap force-pushed the task/authz-failures-tests branch from aea82f6 to 83a3515 Compare April 24, 2026 09:30

niazkhansap requested review from anettlippert and minh-nghia April 24, 2026 09:45

minh-nghia previously approved these changes Apr 24, 2026

View reviewed changes

jmpTeixeira02 reviewed Apr 24, 2026

View reviewed changes

niazkhansap dismissed minh-nghia’s stale review via 24f8659 April 24, 2026 13:26

niazkhansap force-pushed the task/authz-failures-tests branch 2 times, most recently from 24f8659 to 1340f0b Compare April 24, 2026 14:11

niazkhansap requested review from jmpTeixeira02 and minh-nghia April 24, 2026 14:20

niazkhansap force-pushed the task/authz-failures-tests branch from 1340f0b to 6e5b102 Compare April 27, 2026 08:38

minh-nghia previously approved these changes Apr 27, 2026

View reviewed changes

petersbingham reviewed Apr 27, 2026

View reviewed changes

niazkhansap dismissed minh-nghia’s stale review via 04166df April 27, 2026 19:23

niazkhansap force-pushed the task/authz-failures-tests branch 2 times, most recently from 04166df to 5068e70 Compare April 27, 2026 19:49

niazkhansap requested review from minh-nghia and petersbingham April 27, 2026 19:55

anettlippert previously approved these changes Apr 28, 2026

View reviewed changes

niazkhansap dismissed anettlippert’s stale review via 836a61d April 28, 2026 08:06

niazkhansap force-pushed the task/authz-failures-tests branch from 5068e70 to 836a61d Compare April 28, 2026 08:06

niazkhansap requested a review from anettlippert April 28, 2026 08:12

niazkhansap force-pushed the task/authz-failures-tests branch from 836a61d to a1973f1 Compare April 28, 2026 08:48

chore: add unit tests for controller authz failures

a02c4ec

niazkhansap force-pushed the task/authz-failures-tests branch from a1973f1 to a02c4ec Compare April 28, 2026 09:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: add unit tests for controller authz failures#262

chore: add unit tests for controller authz failures#262
niazkhansap wants to merge 1 commit intomainfrom
task/authz-failures-tests

niazkhansap commented Apr 22, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 22, 2026 •

edited

Loading

Review skipped

Uh oh!

niazkhansap commented Apr 23, 2026

Uh oh!

coderabbitai Bot commented Apr 23, 2026

Uh oh!

minh-nghia Apr 23, 2026

Uh oh!

minh-nghia Apr 23, 2026

Uh oh!

minh-nghia Apr 23, 2026

Uh oh!

minh-nghia Apr 24, 2026

Uh oh!

jmpTeixeira02 Apr 24, 2026

Uh oh!

jmpTeixeira02 Apr 24, 2026

Uh oh!

jmpTeixeira02 Apr 24, 2026

Uh oh!

petersbingham Apr 27, 2026

Uh oh!

petersbingham Apr 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

niazkhansap commented Apr 22, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Uh oh!

niazkhansap commented Apr 23, 2026

Uh oh!

coderabbitai Bot commented Apr 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

niazkhansap commented Apr 22, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 22, 2026 •

edited

Loading