Skip to content

Commit

Permalink
[WFCORE-7102]: AccessDeniedException on Windows when using a read-onl…
Browse files Browse the repository at this point in the history
…y configuration dir.

* Replacing java.io.File.canWrite() by java.nio.file.Files.isWritable(Path) in:
   - ConfigurationFile
   - FilePersistenceUtils
   - ConfigurationFilePersistenceResource
* Enabling the test for Windows and chaning the temp folder creation

Jira: https://issues.redhat.com/browse/WFCORE-7102

Signed-off-by: Emmanuel Hugonnet <[email protected]>
  • Loading branch information
ehsavoie committed Dec 19, 2024
1 parent ebf13df commit 6f5d3fa
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ public ConfigurationFile(final File configurationDir, final String rawName, fina
this.configurationExtension = configurationExtension;
this.interactionPolicy = interactionPolicy == null ? InteractionPolicy.STANDARD : interactionPolicy;
// If we are in a read only policy and the configurationDir cannot be written, then we use the tmpDir for temporal files and history
this.historyRoot = new File(tmpDir != null && this.interactionPolicy.isReadOnly() && !configurationDir.canWrite() ? tmpDir : configurationDir,
this.historyRoot = new File(tmpDir != null && this.interactionPolicy.isReadOnly() && !Files.isWritable(configurationDir.toPath())? tmpDir : configurationDir,
rawName.replace('.', '_') + "_history");
this.currentHistory = new File(historyRoot, "current");
this.snapshotsDirectory = new File(historyRoot, "snapshot");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import java.io.File;
import java.io.InputStream;
import java.nio.file.Files;

import org.jboss.dmr.ModelNode;

Expand Down Expand Up @@ -37,7 +38,7 @@ protected void doCommit(InputStream in) {

if ( FilePersistenceUtils.isParentFolderWritable(fileName) ){
tempFileName = FilePersistenceUtils.createTempFile(fileName);
} else if (configurationFile.getConfigurationDir().canWrite()) {
} else if (Files.isWritable(configurationFile.getConfigurationDir().toPath())) {
tempFileName = FilePersistenceUtils.createTempFile(configurationFile.getConfigurationDir(), fileName.getName());
} else {
tempFileName = FilePersistenceUtils.createTempFile(configurationFile.getConfigurationTmpDir(), fileName.getName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,6 @@ static boolean isParentFolderWritable(File file){
if ( !file.exists() || file.getParentFile() == null ){
return false;
}
return file.getParentFile().canWrite();
return Files.isWritable(file.getParentFile().toPath());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ protected ProcessReloadHandler.ReloadContext<RunningModeControl> initializeReloa
if (serverConfig != null && !environment.getServerConfigurationFile().checkCanFindNewBootFile(serverConfig)) {
throw ServerLogger.ROOT_LOGGER.serverConfigForReloadNotFound(serverConfig);
}
ServerLogger.ROOT_LOGGER.warn("Reloading with config file " + serverConfig);
return new ReloadContext<RunningModeControl>() {

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@
import java.util.Set;

import jakarta.inject.Inject;
import java.io.File;
import java.nio.file.attribute.AclEntry;
import java.nio.file.attribute.AclEntryPermission;
import java.nio.file.attribute.AclEntryType;
import java.nio.file.attribute.AclFileAttributeView;
import java.nio.file.attribute.FileOwnerAttributeView;
import java.nio.file.attribute.UserPrincipal;
import java.util.List;

import org.jboss.as.controller.PathAddress;
import org.jboss.as.controller.PathElement;
Expand All @@ -28,7 +36,6 @@
import org.jboss.dmr.ModelNode;
import org.junit.After;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.wildfly.core.testrunner.ServerControl;
Expand Down Expand Up @@ -69,29 +76,37 @@ public void testConfigurationNotUpdated() throws Exception {

@Test
public void testReadOnlyConfigurationDirectory() throws Exception {
// We ignore the test on Windows to prevent in case of errors the pollution of %TMPDIR% with read only directories
// On unix machines, the /tmp dir is always deleted on each server boot by root user.
Assume.assumeFalse(TestSuiteEnvironment.isWindows());

final Path jbossHome = Paths.get(System.getProperty("jboss.home"));
final Path configDir = jbossHome.resolve("standalone").resolve("configuration");
final Path standaloneTmpDir = jbossHome.resolve("standalone").resolve("tmp");
final Path osTmpDir = Paths.get("/tmp");
final Path osTmpDir = TestSuiteEnvironment.isWindows() ? new File("target", "tmp").toPath().toAbsolutePath() : Paths.get("/tmp");
if(Files.notExists(osTmpDir)) {
Files.createDirectories(osTmpDir);
}
final Path roConfigDir = Files.createTempDirectory(osTmpDir, "wildfly-test-suite-");

PathUtil.copyRecursively(configDir, roConfigDir, true);

Set<PosixFilePermission> perms = new HashSet<>();

perms.add(PosixFilePermission.OWNER_READ);
perms.add(PosixFilePermission.OWNER_EXECUTE);
perms.add(PosixFilePermission.GROUP_READ);
perms.add(PosixFilePermission.GROUP_EXECUTE);
perms.add(PosixFilePermission.OTHERS_READ);
perms.add(PosixFilePermission.OTHERS_EXECUTE);

Files.getFileAttributeView(roConfigDir, PosixFileAttributeView.class).setPermissions(perms);

if (TestSuiteEnvironment.isWindows()) {
UserPrincipal owner = Files.getFileAttributeView(configDir, FileOwnerAttributeView.class).getOwner();
AclEntry entry = AclEntry.newBuilder()
.setPrincipal(owner)
.setType(AclEntryType.DENY)
.setPermissions(AclEntryPermission.WRITE_DATA, AclEntryPermission.APPEND_DATA)
.build();
AclFileAttributeView view = Files.getFileAttributeView(roConfigDir, AclFileAttributeView.class);
List<AclEntry> acl = view.getAcl();
acl.add(0, entry);
view.setAcl(acl);
} else {
Set<PosixFilePermission> perms = new HashSet<>();
perms.add(PosixFilePermission.OWNER_READ);
perms.add(PosixFilePermission.OWNER_EXECUTE);
perms.add(PosixFilePermission.GROUP_READ);
perms.add(PosixFilePermission.GROUP_EXECUTE);
perms.add(PosixFilePermission.OTHERS_READ);
perms.add(PosixFilePermission.OTHERS_EXECUTE);
Files.getFileAttributeView(roConfigDir, PosixFileAttributeView.class).setPermissions(perms);
}
assertFalse(roConfigDir.toString() + " is writeable", Files.isWritable(roConfigDir));

try {
Expand Down Expand Up @@ -120,13 +135,34 @@ public void testReadOnlyConfigurationDirectory() throws Exception {
}

} finally {
if (TestSuiteEnvironment.isWindows()) {
UserPrincipal owner = Files.getFileAttributeView(configDir, FileOwnerAttributeView.class).getOwner();
AclEntry entry = AclEntry.newBuilder()
.setPrincipal(owner)
.setType(AclEntryType.ALLOW)
.setPermissions(AclEntryPermission.WRITE_DATA, AclEntryPermission.APPEND_DATA)
.build();
AclFileAttributeView view = Files.getFileAttributeView(roConfigDir, AclFileAttributeView.class);
List<AclEntry> acl = view.getAcl();
acl.add(0, entry);
view.setAcl(acl);
} else {
Set<PosixFilePermission> perms = new HashSet<>();
perms.add(PosixFilePermission.OWNER_WRITE);
perms.add(PosixFilePermission.GROUP_WRITE);
perms.add(PosixFilePermission.OTHERS_WRITE);

perms.add(PosixFilePermission.OWNER_READ);
perms.add(PosixFilePermission.OWNER_EXECUTE);
perms.add(PosixFilePermission.GROUP_READ);
perms.add(PosixFilePermission.GROUP_EXECUTE);
perms.add(PosixFilePermission.OTHERS_READ);
perms.add(PosixFilePermission.OTHERS_EXECUTE);
Files.getFileAttributeView(roConfigDir, PosixFileAttributeView.class).setPermissions(perms);

}
PathUtil.deleteRecursively(roConfigDir);
if( TestSuiteEnvironment.isWindows()) {
PathUtil.deleteRecursively(osTmpDir);
}
}
}

Expand Down

0 comments on commit 6f5d3fa

Please sign in to comment.