8000 Group count and PartialExport permission fixes by rmartinc · Pull Request #24340 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

Group count and PartialExport permission fixes - #24340

Merged
pedroigor merged 1 commit into
keycloak:mainfrom
rmartinc:issue-12171
Oct 31, 2023
Merged

Group count and PartialExport permission fixes#24340
pedroigor merged 1 commit into
keycloak:mainfrom
rmartinc:issue-12171

Conversation

@rmartinc
@rmartinc rmartinc commented Oct 27, 2023
Copy link
Copy Markdown
Contributor

Closes #12171

  • Scenario 1: This is a change in permissions, previously it was view-realm and with this PR the export requires manage-realm. There were tests checking the old behavior. As the issue is directly requesting this permission I have just changed it. To include groups/clients in the export the same permissions as before are requested (list/view respectively). If anyone has any concern or we need some documentation, just let me know. @keycloak/ui I have modified the realm header to mark enable/disable toggle and the action options as read-only if the admin doesn't have manage-realm permission. Lint is OK, let's see the tests.

  • Scenario 2 was fixed yesterday by #22700 😄 . I needed to rebase because of the conflict. So it's fixed now.

PermissionsTest class modified to add tests for both modifications.

@rmartinc
rmartinc requested review from a team as code owners October 27, 2023 06:43
@rmartinc
rmartinc requested a review from a team October 27, 2023 06:43
@rmartinc
rmartinc requested a review from a team as a code owner October 27, 2023 06:43
@cypress
cypress Bot commented Oct 27, 2023 •
edited
Loading
Copy link
Copy Markdown

Passing run #9624 ↗︎

0 538 48 0 Flakiness 0

Details:

Merge f198f20 into 357f3b2...
Project: Keycloak Admin UI Commit: 8d689e2827 ℹ️
Status: Passed Duration: 11:44 💡
Started: Oct 30, 2023 8:17 AM Ended: Oct 30, 2023 8:28 AM

Review all test suite changes for PR #24340 ↗︎

@ghost ghost added the flaky-test label Oct 27, 2023
This was referenced Oct 27, 2023
Closed
Closed
Closed
@rmartinc
rmartinc commented Oct 27, 2023
Copy link
Copy Markdown
Contributor Author

That test works for me locally so maybe it's spureous:

CYPRESS_BASE_URL=http://localhost:8080/admin/ CYPRESS_KEYCLOAK_SERVER=http://localhost:8080 /home/rmartinc/.local/share/pnpm/pnpm run cy:run --browser chromium --spec ./cypress/e2e/realm_settings_general_tab_test.spec.ts

> admin-ui@ cy:run /home/rmartinc/keycloak/js/apps/admin-ui
> cypress run --browser chrome "--browser" "chromium" "--spec" "./cypress/e2e/realm_settings_general_tab_test.spec.ts"

  (Run Starting)
  ...                                                                                                 
  Running:  realm_settings_general_tab_test.spec.ts                                         (1 of 1)


  Realm settings general tab tests
    ✓ Test all general tab switches (9365ms)
    ✓ Modify Display name (2227ms)
    ✓ Check Display name value (1579ms)
    ✓ Modify front end URL (2211ms)
    ✓ Select SSL all requests (1851ms)
    ✓ Verify SSL all requests displays (1544ms)
    ✓ Select SSL external requests (1887ms)
    ✓ Verify SSL external requests displays (1671ms)
    ✓ Select SSL None (2021ms)
    ✓ Verify SSL None displays (1557ms)
    ✓ Check Access Endpoints OpenID Endpoint Configuration link (1563ms)
    ✓ Access Endpoints OpenID Endpoint Configuration link (1527ms)
    ✓ Check if Access Endpoints SAML 2.0 Identity Provider Metadata link exists (1568ms)
    ✓ Access Endpoints SAML 2.0 Identity Provider Metadata link (1661ms)
    ✓ Verify 'Revert' button works (2170ms)


  15 passing (35s)

jonkoops
jonkoops previously approved these changes Oct 27, 2023
View reviewed changes
@jonkoops jonkoops left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aside from a small nit this code looks good to me.

Comment thread js/apps/admin-ui/src/realm-settings/RealmSettingsTabs.tsx Resolved
pedroigor
pedroigor reviewed Oct 27, 2023
View reviewed changes
@pedroigor pedroigor left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@rmartinc We probably need release notes for this one.

@pedroigor
pedroigor enabled auto-merge (rebase) October 30, 2023 10:03
ghost
ghost reviewed Oct 30, 2023
View reviewed changes
@ghost ghost left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@ghost
ghost commented Oct 30, 2023
Copy link
Copy Markdown

Unreported flaky test detected

If the below flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.admin.concurrency.ConcurrentLoginTest#concurrentLoginSingleUser

Keycloak CI - New Store IT (hot-rod)

java.lang.RuntimeException: There were failures in threads. Failures count: 1
	at org.keycloak.testsuite.admin.concurrency.AbstractConcurrencyTest.run(AbstractConcurrencyTest.java:122)
	at org.keycloak.testsuite.admin.concurrency.AbstractConcurrencyTest.run(AbstractConcurrencyTest.java:63)
	at org.keycloak.testsuite.admin.concurrency.ConcurrentLoginTest.concurrentLoginSingleUser(ConcurrentLoginTest.java:144)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
...

Report flaky test

@pedroigor
pedroigor merged commit 7deb4ca into keycloak:main Oct 31, 2023
ghost
ghost reviewed Oct 31, 2023
View reviewed changes
@ghost ghost left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@ghost
ghost commented Oct 31, 2023
Copy link
Copy Markdown

Unreported flaky test detected

If the below flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.admin.concurrency.ConcurrentLoginTest#concurrentLoginSingleUser

Keycloak CI - New Store IT (hot-rod)

java.lang.RuntimeException: There were failures in thread
8000
s. Failures count: 1
	at org.keycloak.testsuite.admin.concurrency.AbstractConcurrencyTest.run(AbstractConcurrencyTest.java:122)
	at org.keycloak.testsuite.admin.concurrency.AbstractConcurrencyTest.run(AbstractConcurrencyTest.java:63)
	at org.keycloak.testsuite.admin.concurrency.ConcurrentLoginTest.concurrentLoginSingleUser(ConcurrentLoginTest.java:144)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
...

Report flaky test

@malexmave malexmave mentioned this pull request Feb 29, 2024
Closed
@malexmave malexmave mentioned this pull request Dec 10, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pedroigor pedroigor pedroigor approved these changes
+2 more reviewers
@jonkoops jonkoops jonkoops approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

flaky-test team/cloud-native

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Inconsistent authorization behavior when exporting data from a realm

3 participants

@rmartinc @jonkoops @pedroigor
0