Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions docs/documentation/upgrading/topics/changes/changes-26_6_5.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,3 @@ found, ensure the affected users and service accounts have the required admin ro
through group memberships. You can verify this in the Keycloak Admin Console by checking the user's role assignments and
group memberships in the target realm.

// ------------------------ Notable changes ------------------------ //
== Notable changes

Notable changes may include internal behavior changes that prevent common misconfigurations, bugs that are fixed, or changes to simplify running {project_name}.
It also lists significant changes to internal APIs.

=== <TODO>

// ------------------------ Deprecated features ------------------------ //
== Deprecated features

The following sections provide details on deprecated features.

=== <TODO>

// ------------------------ Removed features ------------------------ //
== Removed features

The following features have been removed from this release.

=== <TODO>
18 changes: 18 additions & 0 deletions docs/documentation/upgrading/topics/changes/changes-26_6_6.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// ------------------------ Breaking changes ------------------------ //
== Breaking changes

=== The `view-system` admin role no longer exists

Since version 26.5.4, when introduced, the `view-system` admin role was marked as deprecated. In this release, the role
has been removed for security reasons.

Access to full server information is now restricted to users in the `master` realm holding the `manage-realm` role.

If you are still relying on the `view-system` role, you should consider accessing the full server information using a user
(or service account) from the `master` realm and granted with `manage-realm` admin roles.

Before upgrading, you should check if any of your users, service accounts, or groups are still relying on the
`view-system` role and review their access controls to ensure they have the necessary permissions to access the server
information after the upgrade. The `manage-realm` admin role is a high-privileged role and should only be granted to
highly-trusted accounts.

4 changes: 4 additions & 0 deletions docs/documentation/upgrading/topics/changes/changes.adoc
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
[[migration-changes]]
== Migration Changes

=== Migrating to 26.6.6

include::changes-26_6_6.adoc[leveloffset=2]

=== Migrating to 26.6.5

include::changes-26_6_5.adoc[leveloffset=2]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ public class AdminRoles {
public static final String VIEW_EVENTS = "view-events";
public static final String VIEW_IDENTITY_PROVIDERS = "view-identity-providers";
public static final String VIEW_AUTHORIZATION = "view-authorization";
@Deprecated(since = "26.4", forRemoval = true)
public static final String VIEW_SYSTEM = "view-system";

public static final String MANAGE_REALM = "manage-realm";
public static final String MANAGE_USERS = "manage-users";
Expand All @@ -71,7 +69,6 @@ public class AdminRoles {
ALL_ROLES.add(ADMIN);
ALL_ROLES.add(CREATE_REALM);
ALL_ROLES.add(REALM_ADMIN);
ALL_ROLES.add(VIEW_SYSTEM);
}

public static boolean isAdminRole(RoleModel role) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ public void deleteRole(final @Parameter(description = "role's name (not id!)") @
throw ErrorResponse.error(roleName + " is default role of the realm and cannot be removed.",
Response.Status.BAD_REQUEST);
}
auth.roles().requireManage(role);
RoleRepresentation roleRepresentation = new RoleRepresentation();
roleRepresentation.setId(role.getId());
roleRepresentation.setName(role.getName());
Expand Down Expand Up @@ -322,6 +323,7 @@ public Response updateRole(final @Parameter(description = "role's name (not id!)
if (role == null) {
throw new NotFoundException("Could not find role");
}
auth.roles().requireManage(role);
try {
updateRole(rep, role, realm, session);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

import jakarta.ws.rs.ForbiddenException;

import org.keycloak.Config;
import org.keycloak.authorization.AuthorizationProvider;
import org.keycloak.authorization.common.ClientModelIdentity;
import org.keycloak.authorization.common.DefaultEvaluationContext;
Expand All @@ -42,6 +43,7 @@
import org.keycloak.models.AdminRoles;
import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientScopeModel;
import org.keycloak.models.Constants;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.representations.AccessToken;
Expand Down Expand Up @@ -383,6 +385,9 @@ public Map<String, Collection<String>> getBaseAttributes() {

@Override
public boolean canManage(ClientModel client) {
if (isInternal(client)) {
return false;
}
if (canManageClientsDefault()) return true;
if (!root.isAdminSameRealm()) {
return false;
Expand Down Expand Up @@ -411,6 +416,9 @@ public boolean canManage(ClientModel client) {

@Override
public boolean canConfigure(ClientModel client) {
if (isInternal(client)) {
return false;
}
if (canManage(client)) return true;
if (!root.isAdminSameRealm()) {
return false;
Expand Down Expand Up @@ -708,4 +716,15 @@ private boolean hasPermission(Resource resource, String scope) {
return false;
}

protected boolean isInternal(ClientModel client) {
if (client == null) {
return false;
}

if (realm.getName().equals(Config.getAdminRealm())) {
return client.getClientId().endsWith(AdminRoles.APP_SUFFIX);
}

return Constants.REALM_MANAGEMENT_CLIENT_ID.equals(client.getClientId());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ public boolean canConfigure(ClientModel client) {

@Override
public boolean canManage(ClientModel client) {
if (isInternal(client)) {
return false;
}
if (root.hasOneAdminRole(AdminRoles.MANAGE_CLIENTS)) {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;

Expand Down Expand Up @@ -470,7 +471,7 @@ public void requireMapClientScope(RoleModel role) {
@Override
public boolean canManage(RoleModel role) {
if (role.getContainer() instanceof RealmModel) {
return root.realm().canManageRealm();
return root.realm().canManageRealm() && !isRealmAdminRole(role);
} else if (role.getContainer() instanceof ClientModel) {
ClientModel client = (ClientModel)role.getContainer();
return root.clients().canConfigure(client);
Expand Down Expand Up @@ -668,4 +669,7 @@ private ResourceServer getResourceServer(RoleModel role) {
}
return resourceServer;
}
private boolean isRealmAdminRole(RoleModel role) {
return role.getContainer() instanceof RealmModel && List.of(AdminRoles.ADMIN, AdminRoles.CREATE_REALM).contains(role.getName());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ class RolePermissionsV2 extends RolePermissions {
@Override
public boolean canMapRole(RoleModel role) {
if (isRealmAdminRole(role)) {
if (AdminRoles.VIEW_SYSTEM.equals(role.getName()) && !root.isAdmin(root.getMasterRealm())) {
return false;
}
if (realm.isAdminPermissionsEnabled()) {
// only server or realm admins can map roles if FGAP is enabled
return root.isRealmAdmin();
Expand All @@ -76,9 +73,6 @@ public boolean canMapRole(RoleModel role) {
@Override
public boolean canMapComposite(RoleModel role) {
if (isRealmAdminRole(role)) {
if (AdminRoles.VIEW_SYSTEM.equals(role.getName()) && !root.isAdmin(root.getMasterRealm())) {
return false;
}
if (realm.isAdminPermissionsEnabled()) {
// only server or realm admins can map roles if FGAP is enabled
return root.isRealmAdmin();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
import org.keycloak.events.EventType;
import org.keycloak.events.admin.OperationType;
import org.keycloak.events.admin.ResourceType;
import org.keycloak.models.AdminRoles;
import org.keycloak.models.GroupModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
Expand Down Expand Up @@ -126,17 +125,20 @@ public ServerInfoRepresentation getInfo() {
ServerInfoRepresentation info = new ServerInfoRepresentation();
RealmModel userRealm = session.getContext().getRealm();
AdminPermissionEvaluator adminEvaluator = AdminPermissions.evaluator(session, userRealm, auth);
if (RealmManager.isAdministrationRealm(userRealm) || adminEvaluator.hasOneAdminRole(AdminRoles.VIEW_SYSTEM)) {
// system information is only for admins in the administration realm or fallback view-system role
info.setSystemInfo(SystemInfoRepresentation.create(session.getKeycloakSessionFactory().getServerStartupTimestamp(), Version.VERSION));
info.setCpuInfo(CpuInfoRepresentation.create());
info.setMemoryInfo(MemoryInfoRepresentation.create());
} else if (adminEvaluator.realm().canManageRealm()) {
// If the user can manage his own realm just add the version information
SystemInfoRepresentation systemInfo = new SystemInfoRepresentation();
systemInfo.setVersion(Version.VERSION);
info.setSystemInfo(systemInfo);

if (adminEvaluator.realm().canManageRealm()) {
if (RealmManager.isAdministrationRealm(userRealm)) {
info.setSystemInfo(SystemInfoRepresentation.create(session.getKeycloakSessionFactory().getServerStartupTimestamp(), Version.VERSION));
info.setCpuInfo(CpuInfoRepresentation.create());
info.setMemoryInfo(MemoryInfoRepresentation.create());
} else {
// If the user can manage his own realm just add the version information
SystemInfoRepresentation systemInfo = new SystemInfoRepresentation();
systemInfo.setVersion(Version.VERSION);
info.setSystemInfo(systemInfo);
}
}

info.setProfileInfo(createProfileInfo());
info.setFeatures(createFeatureRepresentations());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,19 @@ public void beforeEach() { // todo rewrite
managedMasterRealm.admin().users().get(roleUserUuid).roles().clientLevel(clientUuid).add(Collections.singletonList(roleRep));
}

for (String role : AdminRoles.ALL_REALM_ROLES) {
response = managedMasterRealm.admin().users().create(UserConfigBuilder.create()
.username("master-user-" + role)
.password("password")
.build());
String roleUserUuid = ApiUtil.getCreatedId(response);
managedMasterRealm.cleanup().add(r -> r.users().delete(roleUserUuid).close());

String clientUuid = managedMasterRealm.admin().clients().findByClientId("master-realm").get(0).getId();
RoleRepresentation roleRep = managedMasterRealm.admin().clients().get(clientUuid).roles().get(role).toRepresentation();
managedMasterRealm.admin().users().get(roleUserUuid).roles().clientLevel(clientUuid).add(Collections.singletonList(roleRep));
}

clients.put(AdminRoles.REALM_ADMIN,
adminClientFactory.create().realm(REALM_NAME).username(AdminRoles.REALM_ADMIN).password("password").clientId("test-client").clientSecret("secret").build());

Expand All @@ -97,6 +110,10 @@ public void beforeEach() { // todo rewrite
adminClientFactory.create().realm("master").username("permissions-test-master-" + role).password("password").clientId(Constants.ADMIN_CLI_CLIENT_ID).build());
}

for (String role : AdminRoles.ALL_ROLES) {
clients.put("master-admin-" + role,
adminClientFactory.create().realm("master").username("master-user-" + role).password("password").clientId(Constants.ADMIN_CLI_CLIENT_ID).build());
}
}

protected void invoke(final Invocation invocation, AdminAuth.Resource resource, boolean manage) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Predicate;
import java.util.stream.Collectors;

import jakarta.ws.rs.BadRequestException;
import jakarta.ws.rs.ForbiddenException;
import jakarta.ws.rs.NotFoundException;
import jakarta.ws.rs.core.Response;

Expand Down Expand Up @@ -97,6 +99,7 @@
import static java.util.Arrays.asList;

import static org.keycloak.models.Constants.OIDC_PROTOCOL;
import static org.keycloak.models.Constants.REALM_MANAGEMENT_CLIENT_ID;
import static org.keycloak.models.Constants.defaultClients;

import static org.hamcrest.CoreMatchers.is;
Expand Down Expand Up @@ -145,7 +148,7 @@ public void getClients() {
public void getRealmClients() {
assertTrue(managedRealm.admin().clients().findAll().stream().filter(client -> client.getAttributes().get(Constants.REALM_CLIENT).equals("true"))
.map(ClientRepresentation::getClientId)
.allMatch(clientId -> clientId.equals(Constants.REALM_MANAGEMENT_CLIENT_ID) || clientId.equals(Constants.BROKER_SERVICE_CLIENT_ID) || clientId.endsWith("-realm")));
.allMatch(clientId -> clientId.equals(REALM_MANAGEMENT_CLIENT_ID) || clientId.equals(Constants.BROKER_SERVICE_CLIENT_ID) || clientId.endsWith("-realm")));
}

private ClientRepresentation createClient() {
Expand Down Expand Up @@ -569,13 +572,7 @@ public void removeClientWithDependentCompositeRoles() {

@Test
public void removeInternalClientExpectingBadRequestException() {
final String testRealmClientId = AdminApiUtil.findClientByClientId(managedMasterRealm.admin(), managedRealm.getName() + "-realm")
.toRepresentation().getId();

assertThrows(BadRequestException.class,
() -> managedMasterRealm.admin().clients().get(testRealmClientId).remove());

defaultClients.forEach(defaultClient -> {
defaultClients.stream().filter(Predicate.not(REALM_MANAGEMENT_CLIENT_ID::equals)).forEach(defaultClient -> {
final String defaultClientId = AdminApiUtil.findClientByClientId(managedRealm.admin(), defaultClient)
.toRepresentation().getId();

Expand All @@ -584,6 +581,23 @@ public void removeInternalClientExpectingBadRequestException() {
});
}

@Test
public void removeRealmManagementClientForbiddenException() {
assertThrows(ForbiddenException.class,
() -> {
String testRealmClientId = AdminApiUtil.findClientByClientId(managedMasterRealm.admin(), managedRealm.getName() + "-realm")
.toRepresentation().getId();
managedMasterRealm.admin().clients().get(testRealmClientId).remove();
});

assertThrows(ForbiddenException.class,
() -> {
String testRealmClientId = AdminApiUtil.findClientByClientId(managedRealm.admin(), REALM_MANAGEMENT_CLIENT_ID)
.toRepresentation().getId();
managedRealm.admin().clients().get(testRealmClientId).remove();
});
}

@Test
public void getClientRepresentation() {
String id = createClient().getId();
Expand Down
Loading
Loading