[JENKINS-69869] Categorize the user properties

[Wadeck]

See JENKINS-69869.

This PR is a proposal to categorize the user properties. It follows the same idea as with the Global configuration pages.

Originally I was working on JENKINS-69853 but I wanted to put them in a separate page and end up creating this instead. That's also why you can see the UserPropertyCategory.Experimental as a WIP stuff.

If you test this PR live, you can look at the Experimental category to see the behavior in the case there is no enabled properties in a given category.

The category system is meant to be used by plugins without requiring to have the most recent version of core, with the String based method.

Screenshots

Before

After

If there is no enabled properties

For reviewers

This PR is structured to ease your review. You can review commit by commit to ease the understanding of the intent.

Structure of this PR in terms of commit:

1. Logic change to support new categories (= main part of the PR)
2. Update the existing properties in Jenkins core
3. Copy-paste the translation files used for configure.jelly in User

Testing done

• Saved config on each page and verified that it has correctly updated
• Adapted core with explicit compile time category
• Adapted plugins via string based category, e.g Move ssh keys to security user page when available sshd-plugin#172 and verified they moved and saving worked

Proposed changelog entries

• User properties are now categorized in different pages.

Proposed upgrade guidelines

N/A

Submitter checklist

• The Jira issue, if it exists, is well-described.
• [x]The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples).
  • Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
• There is automated testing or an explanation as to why this change has no tests.
• New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
• New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
• [n/a] New or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
• [n/a] For dependency updates, there are links to external changelogs and, if possible, full differentials.
• For new APIs and extension points, there is a link to at least one consumer.

Desired reviewers

@daniel-beck @basil As we were quickly discussing about the feature flag topic, this could be a first step in that direction. But not required, as the flags could be a simple UserProperty.

Maintainer checklist

Before the changes are marked as ready-for-merge:

• There are at least two (2) approvals for the pull request and no outstanding requests for change.
• Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
• Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
• Proper changelog labels are set so that the changelog can be generated automatically.
• If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
• If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

[timja]

@Wadeck are you interested in finishing this off?

It's definitely a great idea and a needed feature

Get "approval" about the direction

Looks good

Add a system property to keep the previous approach in place

Why?

(not sure about the process) After merge, updating the ATH to pass with the new approach

All ATH tests need to be passing pre-merge, tbh I'm not aware (as-in had to touch any, not searched) of any tests specifically that touch user config there may not be many to fix.

[janfaracik]

+1 on this - I think it's a great idea. Apologies for not seeing it sooner.

[Wadeck]

👋 Interested yes for sure, having time "soon", less sure :)
Anyone who would like to take over is very welcome, otherwise I will do it when time permits.

Add a system property to keep the previous approach in place

Why?

I really like the experience I have with some applications that proposes a way to "rollback" UI changes. If you think it's an overkill (in general or in this particular case), please tell me ;-)

[timja]

I've:

• Refactored out common code
• Polished user facing terminology
• Removed the half done feature flagging for falling back to the old method
• Adapted the merged feature flagging capability to this
• Added appearance category
• Adapted theme manager, custom header and sshd plugins

I'm happy with this UX and code wise now.

TODO:

• fix any failing tests in core
• run PCT - running again tests should be fixed though
• run ATH

[janfaracik]

Clicking the 'Configure' button on the Users (http://localhost:8080/jenkins/manage/securityRealm/) page results in a 404

[timja]

@jenkinsci/core-security-review possible to get a review please?

[Kevin-CB]

@jenkinsci/core-security-review possible to get a review please?

Yeah, we're aware of this PR and are planning to review it soonish

[Kevin-CB]

I tested this PR locally, and reviewed code, I don't see any security concern!

[timja]

Thanks! Addressed and retested with an admin and read only user, read only user can now see their own everything and the basic ones that they were allowed to before of admins, and admin can see everything.

---

/label ready-for-merge

---

This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

[Wadeck]

Thanks @timja to have worked on this one <3

[basil]

Thanks @timja! This is creating conflicts on the jakarta branch; could you please merge it there?

[timja]

Done

[basil]

Thanks @timja! We're back up-to-date again. 🙌

[basil]

Hi @timja, unfortunately this is causing a new ATH failure in core.JenkinsDatabaseSecurityRealmTest#create_update_delete. I think that test needs to be updated to use account instead of configure as was done in this PR.

org.openqa.selenium.TimeoutException: Expected condition failed: Element matching By.xpath: //form[contains(@name, 'config')] is present (tried for 10 second(s) with 500 milliseconds interval)
	at org.openqa.selenium.support.ui.FluentWait.timeoutException(FluentWait.java:265)
	at org.jenkinsci.test.acceptance.junit.Wait.timeoutException(Wait.java:188)
	at org.openqa.selenium.support.ui.FluentWait.until(FluentWait.java:228)
	at org.jenkinsci.test.acceptance.junit.Wait.until(Wait.java:119)
	at org.jenkinsci.test.acceptance.po.CapybaraPortingLayerImpl.waitFor(CapybaraPortingLayerImpl.java:137)
	at org.jenkinsci.test.acceptance.po.ConfigurablePageObject.configure(ConfigurablePageObject.java:113)
	at core.JenkinsDatabaseSecurityRealmTest.create_update_delete(JenkinsDatabaseSecurityRealmTest.java:83)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:61)
	at org.jenkinsci.test.acceptance.junit.WithPlugins$RuleImpl$1.evaluate(WithPlugins.java:160)
	at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:61)
	at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:61)
	at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:61)
	at org.jenkinsci.test.acceptance.junit.JenkinsAcceptanceTestRule$1$1.evaluate(JenkinsAcceptanceTestRule.java:153)
	at org.jenkinsci.test.acceptance.junit.FilterRule$1.evaluate(FilterRule.java:61)
	at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:61)
	at org.jenkinsci.test.acceptance.junit.JenkinsAcceptanceTestRule$1.evaluate(JenkinsAcceptanceTestRule.java:53)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:316)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:240)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:214)
	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:155)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:385)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
	at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:507)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:495)

[timja]

There is a PR that was waiting for release of this change for that, I think it will work now as well that its merged and not need to wait till Tuesday: jenkinsci/acceptance-test-harness#1584

because of where the change was forward compat was awkward and its doing version detection.

[basil]

Broke org.jenkinsci.plugins.oic.PluginTest#testLastGrantedAuthoritiesProperty.