Add RBAC schema implementation #9829

wweellddeerr · 2025-02-21T22:53:18Z

What does this PR change?

It is a port of #9802 containing the schema migration and the ORM implementation.

GUI diff

No difference.

DONE

Documentation

No documentation needed: only internal and user invisible changes
DONE

Test coverage

ℹ️ If a major new functionality is added, it is strongly recommended that tests for the new functionality are added to the Cucumber test suite

DONE

Links

https://github.com/SUSE/spacewalk/issues/26409

DONE

Changelogs

Make sure the changelogs entries you are adding are compliant with https://github.com/uyuni-project/uyuni/wiki/Contributing#changelogs and https://github.com/uyuni-project/uyuni/wiki/Contributing#uyuni-projectuyuni-repository

If you don't need a changelog check, please mark this checkbox:

No changelog needed

If you uncheck the checkbox after the PR is created, you will need to re-run changelog_test (see below)

Re-run a test

If you need to re-run a test, please mark the related checkbox, it will be unchecked automatically once it has re-run:

Re-run test "changelog_test"
Re-run test "backend_unittests_pgsql"
Re-run test "java_pgsql_tests"
Re-run test "schema_migration_test_pgsql"
Re-run test "susemanager_unittests"
Re-run test "javascript_lint"
Re-run test "spacecmd_unittests"

Before you merge

Check How to branch and merge properly!

github-actions · 2025-02-21T22:53:29Z

👋 Hello! Thanks for contributing to our project.
Acceptance tests will take some time (aprox. 1h), please be patient ☕

You can see the progress at the end of this page and at https://github.com/uyuni-project/uyuni/pull/9829/checks
Once tests finish, if they fail, you can check 👀 the cucumber report. See the link at the output of the action.
You can also check the artifacts section, which contains the logs at https://github.com/uyuni-project/uyuni/pull/9829/checks.

If you are unsure the failing tests are related to your code, you can check the "reference jobs". These are jobs that run on a scheduled time with code from master. If they fail for the same reason as your build, it means the tests or the infrastructure are broken. If they do not fail, but yours do, it means it is related to your code.

Reference tests:

KNOWN ISSUES

Sometimes the build can fail when pulling new jar files from download.opensuse.org . This is a known limitation. Given this happens rarely, when it does, all you need to do is rerun the test. Sorry for the inconvenience.

For more tips on troubleshooting, see the troubleshooting guide.

Happy hacking!
⚠️ You should not merge if acceptance tests fail to pass. ⚠️

rjmateus

Just a few notes, but overall looks good.

rjmateus · 2025-02-24T17:38:08Z

schema/spacewalk/common/views/userAccessTable.sql

+-- userAccessTable view
+-- User access rules on endpoints either permitted directly or through an access group
+CREATE VIEW access.userAccessTable AS
+WITH endpoints AS (


I'm not sure if we need this initial query. We should test it in terms of performance when we have all the endpoints and see if it has any performance impact.

This one used to be for my own convenience, but I changed this view completely with the latest commit. This view is now queried from the Java filters and is an integral part of the feature, so its performance is critical and I optimized it with proper indexes and planning.

So far I tested it with what data I have, and it's running in microseconds (with about half of the predicted endpoint data).

schema/spacewalk/common/tables/namespace.sql

schema/spacewalk/common/tables/endpoint.sql

rjmateus · 2025-02-24T17:44:25Z

schema/spacewalk/common/tables/endpoint.sql

+    http_method     VARCHAR NOT NULL,
+    scope           CHAR(1) NOT NULL
+                        CHECK (scope in ('A', 'W')),
+    authorized      BOOLEAN NOT NULL DEFAULT true,


I would make it with a more explicit name, like auth_required, as proposed in the RFC.

schema/spacewalk/common/tables/namespace.sql

Rename 'authorized' to 'auth_required'

Checkstyle fixes

rjmateus

Some small nitpick, overall looks good to me.

rjmateus · 2025-02-24T19:20:49Z

java/code/src/com/redhat/rhn/domain/access/WebEndpoint.java

+)
+@NamedNativeQuery(name = "WebEndpoint_get_unauthenticated_webui",
+        // Get unauthenticated Web UI endpoints
+        query = "SELECT endpoint FROM access.endpoint WHERE auth_required = false"


The next query is filtering by API, this one is only filtering by auth_required false. Do we need the two methods? Is this needed because the default handler for the XML-RPC API?

This one is actually used with HTTP API routes as well (works the same way as a web UI endpoint). I'll fix the naming to reflect that.

rjmateus · 2025-02-24T19:25:59Z

java/code/src/com/redhat/rhn/domain/access/WebEndpointFactory.java

+    }
+
+    /**
+     * Get all unauthorized web UI endpoints


Suggested change

* Get all unauthorized web UI endpoints

* Get all web UI endpoints that don't require authorization check

I had a long argument with ChatGPT about this 🤣

Apparently, "unauthorized" means both "not authorized" and "doesn't require authorization"

But since you are also confused, I think it's better to change it like this

I not a native speaker, and even in my main language I not really good with "wording" so I would not argue on this regard :)

rjmateus · 2025-02-24T19:26:19Z

java/code/src/com/redhat/rhn/domain/access/WebEndpointFactory.java

+    }
+
+    /**
+     * Get all unauthorized API methods


Suggested change

* Get all unauthorized API methods

* Get all API methods that don't require authorization check

rjmateus · 2025-02-24T19:26:43Z

java/code/src/com/redhat/rhn/domain/access/WebEndpointFactory.java

+
+    /**
+     * Get all unauthorized API methods
+     * @return the set of unauthorized API methods as a string of qualified class name & method


Suggested change

* @return the set of unauthorized API methods as a string of qualified class name & method

* @return the set of API methods, that doesn't need authentication, as a string of qualified class name & method

rjmateus · 2025-02-24T19:27:01Z

java/code/src/com/redhat/rhn/domain/access/WebEndpointFactory.java

+
+    /**
+     * Get all unauthorized web UI endpoints
+     * @return the set of unauthorized web endpoints


Suggested change

* @return the set of unauthorized web endpoints

* @return the set of web endpoints that doesn't need authorization check

Address review issues

Fix schema idempotency issue

wweellddeerr requested a review from a team as a code owner February 21, 2025 22:53

wweellddeerr requested review from rjmateus and removed request for a team February 21, 2025 22:53

github-actions bot added java java_lint_checkstyle java_pgsql_tests schema_migration_test_pgsql database schema_migration_test_oracle data-model labels Feb 21, 2025

wweellddeerr requested review from cbbayburt and removed request for rjmateus February 21, 2025 22:53

wweellddeerr force-pushed the rbac-base-schema branch from 9aef9be to 46a5977 Compare February 22, 2025 01:58

cbbayburt force-pushed the rbac-base-schema branch 5 times, most recently from 0a3e5f9 to 293d8f8 Compare February 24, 2025 14:03

cbbayburt requested a review from rjmateus February 24, 2025 14:41

rjmateus reviewed Feb 24, 2025

View reviewed changes

cbbayburt added 4 commits February 24, 2025 18:59

RBAC DB schema

ec158f2

RBAC schema ORM

8278275

fixup! RBAC DB schema

293066a

fixup! RBAC schema ORM

600d1b4

Rename 'authorized' to 'auth_required'

cbbayburt force-pushed the rbac-base-schema branch from 293d8f8 to 600d1b4 Compare February 24, 2025 18:43

fixup! RBAC schema ORM

2f57975

Checkstyle fixes

rjmateus previously approved these changes Feb 24, 2025

View reviewed changes

cbbayburt dismissed rjmateus’s stale review via d636e17 February 24, 2025 19:38

cbbayburt removed their request for review February 24, 2025 19:40

cbbayburt added 2 commits February 24, 2025 20:47

fixup! RBAC schema ORM

eafeed9

Address review issues

fixup! RBAC DB schema

6540ae8

Fix schema idempotency issue

cbbayburt force-pushed the rbac-base-schema branch from d636e17 to 6540ae8 Compare February 24, 2025 19:47

cbbayburt merged commit 6540ae8 into uyuni-project:master Feb 24, 2025
27 of 30 checks passed

cbbayburt mentioned this pull request Feb 25, 2025

Add RBAC schema implementation #9802

Closed

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add RBAC schema implementation #9829

Add RBAC schema implementation #9829

wweellddeerr commented Feb 21, 2025 •

edited by cbbayburt

Loading

github-actions bot commented Feb 21, 2025

rjmateus left a comment

rjmateus Feb 24, 2025

cbbayburt Feb 24, 2025

rjmateus Feb 24, 2025

rjmateus left a comment

rjmateus Feb 24, 2025

cbbayburt Feb 24, 2025

rjmateus Feb 24, 2025

cbbayburt Feb 24, 2025

rjmateus Feb 24, 2025

rjmateus Feb 24, 2025

rjmateus Feb 24, 2025

rjmateus Feb 24, 2025

	* Get all unauthorized web UI endpoints
	* Get all web UI endpoints that don't require authorization check

	* Get all unauthorized API methods
	* Get all API methods that don't require authorization check

	* @return the set of unauthorized API methods as a string of qualified class name & method
	* @return the set of API methods, that doesn't need authentication, as a string of qualified class name & method

	* @return the set of unauthorized web endpoints
	* @return the set of web endpoints that doesn't need authorization check

Add RBAC schema implementation #9829

Add RBAC schema implementation #9829

Conversation

wweellddeerr commented Feb 21, 2025 • edited by cbbayburt Loading

What does this PR change?

GUI diff

Documentation

Test coverage

Links

Changelogs

Re-run a test

Before you merge

github-actions bot commented Feb 21, 2025

rjmateus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rjmateus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wweellddeerr commented Feb 21, 2025 •

edited by cbbayburt

Loading