Skip to content
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.

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

Do not return groups associated with organizations from the Admin Gro… #29046

Merged
merged 1 commit into from May 7, 2024

Conversation

martin-kanis
Copy link
Contributor

…up API

Closes #28734

@martin-kanis martin-kanis force-pushed the 28734-filter-out-org-groups branch 3 times, most recently from 4303ea5 to 78b469a Compare April 24, 2024 07:47
Copy link
Contributor

@vramik vramik left a comment

Choose a reason for hiding this comment

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

Thank you @martin-kanis for this PR.

I've noticed there is no description on the issue which would provide more clarity what should be the scope of this task. When I read #28133 's description there are following points related to groups

  • These groups should not be manageable by the Group API
  • Make sure the group name used and its format are reserved and can not be set to regular groups
  • Any attribute set to groups by the organization provider should be read only for both users and administrators

Does this PR aim to solve the first point or will there be a follow-ups?

@pedroigor Could you please confirm what operations on groups associated with organizations in group API should be prevented?

This was referenced Apr 24, 2024
@pedroigor
Copy link
Contributor

@vramik @martin-kanis The goal is to block managing the group associated with an organization. The way we are doing this here seems to be too much and we don't want so drastic changes to groups now.

As discussed, let's move forward by blocking accessing those groups via REST so that we have fewer changes to groups (and potentially making it worse) while still blocking them from being managed by the Groups API.

We have an entire epic to deal with reviewing groups and their API anyways.

@martin-kanis martin-kanis force-pushed the 28734-filter-out-org-groups branch 2 times, most recently from ed6a1ea to 7e6cd94 Compare April 26, 2024 12:19
@martin-kanis martin-kanis marked this pull request as ready for review April 29, 2024 08:07
@martin-kanis martin-kanis requested review from a team as code owners April 29, 2024 08:07
@keycloak-github-bot
Copy link

Unreported flaky test detected

If the 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.x509.X509BrowserCRLTest#loginFailedWithIntermediateRevocationListFromHttp

Keycloak CI - FIPS IT (non-strict)

java.lang.RuntimeException: Could not create statement
	at org.jboss.arquillian.junit.Arquillian.methodBlock(Arquillian.java:307)
	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)
...

Report flaky test

Copy link

@keycloak-github-bot keycloak-github-bot bot left a comment

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

Copy link

@keycloak-github-bot keycloak-github-bot bot left a comment

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

@keycloak-github-bot
Copy link

Unreported flaky test detected

If the 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.adapter.servlet.BrokerLinkAndTokenExchangeTest#testExternalExchangeCreateNewUserUsingMappers

Keycloak CI - Base IT

java.lang.AssertionError: expected:<200> but was:<400>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at org.junit.Assert.assertEquals(Assert.java:633)
...

Report flaky test

org.keycloak.testsuite.adapter.servlet.BrokerLinkAndTokenExchangeTest#testExternalExchange

Keycloak CI - Base IT

java.lang.AssertionError: expected:<200> but was:<400>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at org.junit.Assert.assertEquals(Assert.java:633)
...

Report flaky test

org.keycloak.testsuite.adapter.servlet.BrokerLinkAndTokenExchangeTest#testAccountLinkNoTokenStore

Keycloak CI - Base IT

java.lang.AssertionError: Unexpected page. Current Page URL: http://localhost:8280/exchange-linking/link?realm=child&provider=parent-idp
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.adapter.servlet.BrokerLinkAndTokenExchangeTest.testAccountLinkNoTokenStore(BrokerLinkAndTokenExchangeTest.java:494)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
...

Report flaky test

org.keycloak.testsuite.client.ClientTypesTest#testUpdateClientWithClientType

Keycloak CI - Base IT (4)

java.lang.AssertionError: Not expected to update client
	at org.junit.Assert.fail(Assert.java:89)
	at org.keycloak.testsuite.client.ClientTypesTest.testUpdateClientWithClientType(ClientTypesTest.java:112)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
...

Report flaky test

org.keycloak.testsuite.oauth.ClientTokenExchangeTest#testExchangeWithDynamicScopesEnabled

Keycloak CI - Base IT (6)

org.keycloak.common.VerificationException: Token not set
	at org.keycloak.TokenVerifier.parse(TokenVerifier.java:401)
	at org.keycloak.testsuite.oauth.ClientTokenExchangeTest.testExchange(ClientTokenExchangeTest.java:309)
	at org.keycloak.testsuite.oauth.ClientTokenExchangeTest.testExchangeWithDynamicScopesEnabled(ClientTokenExchangeTest.java:1021)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
...

Report flaky test

org.keycloak.testsuite.oauth.ClientTokenExchangeTest#testClientExchange

Keycloak CI - Base IT (6)

java.lang.AssertionError: expected:<200> but was:<400>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at org.junit.Assert.assertEquals(Assert.java:633)
...

Report flaky test

org.keycloak.testsuite.oauth.ClientTokenExchangeTest#testIntrospectTokenAfterImpersonation

Keycloak CI - Base IT (6)

java.lang.AssertionError: expected:<200> but was:<400>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at org.junit.Assert.assertEquals(Assert.java:633)
...

Report flaky test

org.keycloak.testsuite.oauth.ClientTokenExchangeTest#testPublicClientNotAllowed

Keycloak CI - Base IT (6)

java.lang.AssertionError: expected:<403> but was:<400>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at org.junit.Assert.assertEquals(Assert.java:633)
...

Report flaky test

org.keycloak.testsuite.oauth.ClientTokenExchangeTest#testExchangeUsingServiceAccount

Keycloak CI - Base IT (6)

org.keycloak.common.VerificationException: Token not set
	at org.keycloak.TokenVerifier.parse(TokenVerifier.java:401)
	at org.keycloak.testsuite.oauth.ClientTokenExchangeTest.testExchangeUsingServiceAccount(ClientTokenExchangeTest.java:347)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
...

Report flaky test

org.keycloak.testsuite.oauth.ClientTokenExchangeTest#testImpersonation

Keycloak CI - Base IT (6)

java.lang.AssertionError: expected:<200> but was:<400>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at org.junit.Assert.assertEquals(Assert.java:633)
...

Report flaky test

org.keycloak.testsuite.oauth.ClientTokenExchangeTest#testImpersonationUsingPublicClient

Keycloak CI - Base IT (6)

java.lang.AssertionError: expected:<200> but was:<400>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at org.junit.Assert.assertEquals(Assert.java:633)
...

Report flaky test

@martin-kanis martin-kanis force-pushed the 28734-filter-out-org-groups branch 3 times, most recently from b6b4acc to 32b355d Compare May 7, 2024 11:38
… APIs

Closes keycloak#28734

Signed-off-by: Martin Kanis <mkanis@redhat.com>
@pedroigor pedroigor merged commit d4b7e1a into keycloak:main May 7, 2024
69 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not return groups associated with organizations from the Admin Group API
4 participants