sergehuber commented on code in PR #760: URL: https://github.com/apache/unomi/pull/760#discussion_r3362921391
########## services-common/src/main/java/org/apache/unomi/services/common/security/KarafSecurityService.java: ########## @@ -0,0 +1,333 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.services.common.security; + +import org.apache.karaf.jaas.boot.principal.RolePrincipal; +import org.apache.karaf.jaas.boot.principal.UserPrincipal; +import org.apache.unomi.api.security.*; +import org.apache.unomi.api.tenants.AuditService; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.security.auth.Subject; +import java.security.AccessController; +import java.security.Principal; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +public class KarafSecurityService implements SecurityService { + private static final Logger LOGGER = LoggerFactory.getLogger(KarafSecurityService.class); + + public static final String SYSTEM_TENANT = "system"; + private final Subject SYSTEM_SUBJECT; + + private SecurityServiceConfiguration configuration; + private EncryptionService encryptionService; + private AuditService tenantAuditService; + + private final ThreadLocal<Subject> currentSubject = new ThreadLocal<>(); + private final ThreadLocal<Subject> privilegedSubject = new ThreadLocal<>(); + + public KarafSecurityService() { + SYSTEM_SUBJECT = createSystemSubject(); + } + + private Subject createSystemSubject() { + Subject subject = new Subject(); + subject.getPrincipals().add(new UserPrincipal("system")); + subject.getPrincipals().add(new RolePrincipal(UnomiRoles.ADMINISTRATOR)); + subject.getPrincipals().add(new RolePrincipal(UnomiRoles.TENANT_ADMINISTRATOR)); + subject.getPrincipals().add(new RolePrincipal(UnomiRoles.SYSTEM_MAINTENANCE)); + return subject; + } + + public void init() { + if (configuration == null) { + configuration = new SecurityServiceConfiguration(); + } + updateSystemSubject(); + } + + public void destroy() { + // Cleanup + } + + private void updateSystemSubject() { + SYSTEM_SUBJECT.getPrincipals().clear(); + SYSTEM_SUBJECT.getPrincipals().add(new TenantPrincipal(SYSTEM_TENANT)); + SYSTEM_SUBJECT.getPrincipals().add(new UserPrincipal("system")); + for (String role : configuration.getSystemRoles()) { + SYSTEM_SUBJECT.getPrincipals().add(new RolePrincipal(role)); + } + } + + public void setTenantAuditService(AuditService tenantAuditService) { + this.tenantAuditService = tenantAuditService; + } + + public void setConfiguration(SecurityServiceConfiguration configuration) { + this.configuration = configuration; + } + + public void bindEncryptionService(EncryptionService encryptionService) { + this.encryptionService = encryptionService; + } + + public void unbindEncryptionService(EncryptionService encryptionService) { + this.encryptionService = null; + } + + @Override + public Subject getCurrentSubject() { + // First check JAAS context + Subject jaasSubject = Subject.getSubject(AccessController.getContext()); + if (jaasSubject != null) { + return jaasSubject; + } + + // Then check privileged subject + Subject privSubject = privilegedSubject.get(); + if (privSubject != null) { + return privSubject; + } + + // Finally return current request subject + return currentSubject.get(); + } + + @Override + public Principal getCurrentPrincipal() { + Subject subject = getCurrentSubject(); + return subject != null ? getFirstPrincipal(subject) : null; + } + + @Override + public void setCurrentSubject(Subject subject) { + currentSubject.set(subject); + } + + @Override + public void clearCurrentSubject() { + currentSubject.remove(); + privilegedSubject.remove(); + } + + /** + * Sets a temporary privileged subject for operations that require elevated permissions. + * This subject will be used in addition to the current subject for permission checks. + * + * @param subject the privileged subject to set + */ + public void setPrivilegedSubject(Subject subject) { + privilegedSubject.set(subject); + } + + /** + * Clears the temporary privileged subject. + */ + public void clearPrivilegedSubject() { + privilegedSubject.remove(); + } + + @Override + public boolean hasRole(String role) { + // Check JAAS context first + Subject jaasSubject = Subject.getSubject(AccessController.getContext()); + if (jaasSubject != null && hasRoleInSubject(jaasSubject, role)) { + return true; + } + + // Then check privileged subject + Subject privileged = privilegedSubject.get(); + if (privileged != null && hasRoleInSubject(privileged, role)) { + return true; + } + + // Finally check current subject + Subject current = currentSubject.get(); + return current != null && hasRoleInSubject(current, role); + } + + @Override + public boolean isAdmin() { + return hasRole(UnomiRoles.ADMINISTRATOR); + } + + @Override + public boolean hasSystemAccess() { + return hasRole(UnomiRoles.ADMINISTRATOR) || hasRole(UnomiRoles.TENANT_ADMINISTRATOR); + } + + @Override + public boolean hasTenantAccess(String tenantId) { + if (hasRole(UnomiRoles.TENANT_ADMINISTRATOR)) { Review Comment: Fixed! The method now properly compares the requested tenantId against the subject's own tenant ID, so a tenant principal can only access their own tenant. We kept a blanket true for ADMINISTRATOR role only. We also took the opportunity to fix I6 at the same time — isOperatingOnSystemTenant() was a stub always returning false and now correctly checks against SYSTEM_TENANT. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
