Skip to content

Commit

Permalink
Merge pull request #16560 from iterate-ch/bugfix/MD-22597
Browse files Browse the repository at this point in the history 
Refactor handling of security scoped bookmarks
  • Loading branch information
@dkocher
dkocher authored Nov 22, 2024
2 parents a36283c + f0f5cc4 commit 4f52cd2
Show file tree
Hide file tree
Showing 10 changed files with 133 additions and 176 deletions.
71 changes: 27 additions & 44 deletions core/dylib/src/main/java/ch/cyberduck/core/local/AbstractPromptBookmarkResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
import java.text.MessageFormat;
import java.util.concurrent.atomic.AtomicReference;

public abstract class AbstractPromptBookmarkResolver implements FilesystemBookmarkResolver<NSURL> {
public abstract class AbstractPromptBookmarkResolver implements FilesystemBookmarkResolver<NSData, NSURL> {
private static final Logger log = LogManager.getLogger(AbstractPromptBookmarkResolver.class);

private final int create;
Expand All @@ -62,66 +62,46 @@ public AbstractPromptBookmarkResolver(final int create, final int resolve) {
}

@Override
public String create(final Local file) throws AccessDeniedException {
if(this.skip(file)) {
public NSData create(final Local file) throws AccessDeniedException {
if(skip(file)) {
return null;
}
final ObjCObjectByReference error = new ObjCObjectByReference();
// Create new security scoped bookmark
final NSURL url = NSURL.fileURLWithPath(file.getAbsolute());
log.trace("Resolved file {} to url {}", file, url);
return this.create(url);
}

private NSData create(final NSURL url) throws LocalAccessDeniedException {
final ObjCObjectByReference error = new ObjCObjectByReference();
final NSData data = url.bookmarkDataWithOptions_includingResourceValuesForKeys_relativeToURL_error(
create, null, null, error);
if(null == data) {
log.warn("Failure getting bookmark data for file {}", file);
log.warn("Failure getting bookmark data for file {}", url.path());
final NSError f = error.getValueAs(NSError.class);
if(null == f) {
throw new LocalAccessDeniedException(file.getAbsolute());
throw new LocalAccessDeniedException(url.path());
}
throw new LocalAccessDeniedException(String.format("%s", f.localizedDescription()));
}
final String encoded = data.base64Encoding();
log.trace("Encoded bookmark for {} as {}", file, encoded);
return encoded;
log.trace("Created bookmark {} for {}", data.base64Encoding(), url.path());
return data;
}

@Override
public NSURL resolve(final Local file, final boolean interactive) throws AccessDeniedException {
if(this.skip(file)) {
public NSURL resolve(final NSData bookmark) throws AccessDeniedException {
if(null == bookmark) {
log.warn("Skip resolving null bookmark");
return null;
}
final NSData bookmark;
if(null == file.getBookmark()) {
if(interactive) {
if(!file.exists()) {
return null;
}
// Prompt user if no bookmark reference is available
log.warn("Missing security scoped bookmark for file {}", file);
final String reference = this.choose(file);
if(null == reference) {
// Prompt canceled by user
return null;
}
file.setBookmark(reference);
bookmark = NSData.dataWithBase64EncodedString(reference);
}
else {
log.warn("No security scoped bookmark for {}", file.getName());
return null;
}
}
else {
bookmark = NSData.dataWithBase64EncodedString(file.getBookmark());
}
final ObjCObjectByReference error = new ObjCObjectByReference();
final NSURL resolved = NSURL.URLByResolvingBookmarkData(bookmark, resolve, error);
if(null == resolved) {
log.warn("Error resolving bookmark for {} to URL", file);
final NSError f = error.getValueAs(NSError.class);
if(null == f) {
throw new LocalAccessDeniedException(file.getAbsolute());
throw new LocalAccessDeniedException();
}
log.warn("Error {} resolving bookmark", f);
throw new LocalAccessDeniedException(String.format("%s", f.localizedDescription()));
}
return resolved;
Expand All @@ -130,7 +110,7 @@ public NSURL resolve(final Local file, final boolean interactive) throws AccessD
/**
* Determine if creating security scoped bookmarks for file should be skipped
*/
private boolean skip(final Local file) {
private static boolean skip(final Local file) {
if(null != TEMPORARY) {
if(file.isChild(TEMPORARY)) {
// Skip prompt for file in temporary folder where access is not sandboxed
Expand All @@ -149,8 +129,13 @@ private boolean skip(final Local file) {
/**
* @return Security scoped bookmark
*/
public String choose(final Local file) throws AccessDeniedException {
final AtomicReference<Local> selected = new AtomicReference<Local>();
@Override
public NSData prompt(final Local file) throws AccessDeniedException {
if(!file.exists()) {
log.warn("Skip prompting for non existing file {}", file);
return null;
}
final AtomicReference<NSURL> selected = new AtomicReference<>();
log.warn("Prompt for file {} to obtain bookmark reference", file);
final DefaultMainAction action = new DefaultMainAction() {
@Override
Expand All @@ -160,15 +145,15 @@ public void run() {
panel.setCanChooseFiles(file.isFile());
panel.setAllowsMultipleSelection(false);
panel.setMessage(MessageFormat.format(LocaleFactory.localizedString("Select {0}", "Credentials"),
file.getAbbreviatedPath()));
file.getAbbreviatedPath()));
panel.setPrompt(LocaleFactory.localizedString("Choose"));
final NSInteger modal = panel.runModal(file.getParent().getAbsolute(), file.getName());
if(modal.intValue() == SheetCallback.DEFAULT_OPTION) {
final NSArray filenames = panel.URLs();
final NSEnumerator enumerator = filenames.objectEnumerator();
NSObject next;
while((next = enumerator.nextObject()) != null) {
selected.set(new FinderLocal(Rococoa.cast(next, NSURL.class).path()));
selected.set(Rococoa.cast(next, NSURL.class));
}
}
panel.orderOut(null);
Expand All @@ -179,8 +164,6 @@ public void run() {
log.warn("Prompt for {} canceled", file);
return null;
}
// Save Base64 encoded scoped reference
return this.create(selected.get());
}

}
11 changes: 3 additions & 8 deletions core/dylib/src/main/java/ch/cyberduck/core/local/FilesystemBookmarkResolverFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,15 @@
* GNU General Public License for more details.
*/

import ch.cyberduck.binding.foundation.NSURL;
import ch.cyberduck.core.Factory;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

public class FilesystemBookmarkResolverFactory extends Factory<FilesystemBookmarkResolver<NSURL>> {
private static final Logger log = LogManager.getLogger(FilesystemBookmarkResolverFactory.class);
public class FilesystemBookmarkResolverFactory<Bookmark, Resolved> extends Factory<FilesystemBookmarkResolver<Bookmark, Resolved>> {

private FilesystemBookmarkResolverFactory() {
super("factory.bookmarkresolver.class");
}

public static FilesystemBookmarkResolver<NSURL> get() {
return new FilesystemBookmarkResolverFactory().create();
public static <Bookmark, Resolved> FilesystemBookmarkResolver<Bookmark, Resolved> get() {
return new FilesystemBookmarkResolverFactory<Bookmark, Resolved>().create();
}
}
102 changes: 38 additions & 64 deletions core/dylib/src/main/java/ch/cyberduck/core/local/FinderLocal.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* dkocher@cyberduck.ch
*/

import ch.cyberduck.binding.foundation.NSData;
import ch.cyberduck.binding.foundation.NSFileManager;
import ch.cyberduck.binding.foundation.NSURL;
import ch.cyberduck.core.AttributedList;
Expand All @@ -27,11 +28,9 @@
import ch.cyberduck.core.exception.LocalAccessDeniedException;
import ch.cyberduck.core.library.Native;
import ch.cyberduck.core.preferences.PreferencesFactory;
import ch.cyberduck.core.serializer.Serializer;

import org.apache.commons.io.input.ProxyInputStream;
import org.apache.commons.io.output.ProxyOutputStream;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

Expand All @@ -49,40 +48,19 @@ public class FinderLocal extends Local {
Native.load("core");
}

private final FilesystemBookmarkResolver<NSURL> resolver;
private static final FilesystemBookmarkResolver<NSData, NSURL> resolver
= FilesystemBookmarkResolverFactory.get();

public FinderLocal(final Local parent, final String name) {
this(parent, name, FilesystemBookmarkResolverFactory.get());
}

public FinderLocal(final Local parent, final String name, final FilesystemBookmarkResolver<NSURL> resolver) {
super(parent, name);
this.resolver = resolver;
}

public FinderLocal(final String parent, final String name) {
this(parent, name, FilesystemBookmarkResolverFactory.get());
}

public FinderLocal(final String parent, final String name, final FilesystemBookmarkResolver<NSURL> resolver) {
super(parent, name);
this.resolver = resolver;
}

public FinderLocal(final String path) {
this(resolveAlias(new TildeExpander().expand(path)), FilesystemBookmarkResolverFactory.get());
}

public FinderLocal(final String name, final FilesystemBookmarkResolver<NSURL> resolver) {
public FinderLocal(final String name) {
super(name);
this.resolver = resolver;
}

@Override
public <T> T serialize(final Serializer<T> dict) {
dict.setStringForKey(this.getAbbreviatedPath(), "Path");
dict.setStringForKey(this.getBookmark(), "Bookmark");
return dict.getSerialized();
}

/**
Expand Down Expand Up @@ -133,40 +111,6 @@ public boolean exists(final LinkOption... options) {
}
}

/**
* @return Application scoped bookmark to access outside of sandbox
*/
@Override
public String getBookmark() {
final String path = this.getAbbreviatedPath();
String bookmark = PreferencesFactory.get().getProperty(String.format("local.bookmark.%s", path));
if(StringUtils.isBlank(bookmark)) {
try {
bookmark = resolver.create(this);
}
catch(AccessDeniedException e) {
log.warn("Failure resolving bookmark for {}. {}", this, e);
}
}
return bookmark;
}

/**
* Save bookmark reference for file in preferences
*
* @param data Security scoped bookmark to save for later retrieval of file reference or null to remove
*/
@Override
public void setBookmark(final String data) {
final String path = this.getAbbreviatedPath();
if(null == data) {
PreferencesFactory.get().deleteProperty(String.format("local.bookmark.%s", path));
}
else {
PreferencesFactory.get().setProperty(String.format("local.bookmark.%s", path), data);
}
}

@Override
public AttributedList<Local> list(final Filter<String> filter) throws AccessDeniedException {
final NSURL resolved;
Expand Down Expand Up @@ -206,13 +150,43 @@ public OutputStream getOutputStream(boolean append) throws AccessDeniedException
*/
@Override
public NSURL lock(final boolean interactive) throws AccessDeniedException {
final NSURL resolved = resolver.resolve(this, interactive);
return this.lock(interactive, resolver);
}

protected NSURL lock(final boolean interactive, final FilesystemBookmarkResolver<NSData, NSURL> resolver) throws AccessDeniedException {
final String path = this.getAbbreviatedPath();
NSURL resolved;
if(null != this.getBookmark()) {
resolved = resolver.resolve(NSData.dataWithBase64EncodedString(this.getBookmark()));
}
else {
try {
resolved = resolver.resolve(resolver.create(this));
}
catch(AccessDeniedException e) {
log.warn("Failure {} creating bookmark for {}", e, path);
if(interactive) {
log.warn("Missing security scoped bookmark for file {}", path);
// Prompt user if no bookmark reference is available
final NSData bookmark = resolver.prompt(this);
if(null == bookmark) {
// Prompt canceled by user
return null;
}
resolved = resolver.resolve(bookmark);
}
else {
log.warn("No security scoped bookmark for {}", path);
// Ignore failure resolving path
return null;
}
}
}
if(null == resolved) {
// Ignore failure resolving path
return null; // NSURL.fileURLWithPath(this.getAbsolute());
return null;
}
if(!resolved.startAccessingSecurityScopedResource()) {
throw new LocalAccessDeniedException(String.format("Failure accessing security scoped resource for %s", this));
throw new LocalAccessDeniedException(String.format("Failure accessing security scoped resource for %s", path));
}
return resolved;
}
Expand Down
10 changes: 5 additions & 5 deletions core/dylib/src/test/java/ch/cyberduck/core/local/AliasFilesystemBookmarkResolverTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* GNU General Public License for more details.
*/

import ch.cyberduck.binding.foundation.NSData;
import ch.cyberduck.binding.foundation.NSURL;
import ch.cyberduck.core.Local;
import ch.cyberduck.core.exception.LocalAccessDeniedException;
Expand Down Expand Up @@ -47,9 +48,9 @@ public void testCreateFileTemporary() throws Exception {
new DefaultLocalTouchFeature().touch(l);
try {
final AliasFilesystemBookmarkResolver resolver = new AliasFilesystemBookmarkResolver();
final String bookmark = resolver.create(l);
final NSData bookmark = resolver.create(l);
assertNull(bookmark);
final NSURL resolved = resolver.resolve(l, false);
final NSURL resolved = resolver.resolve(bookmark);
assertNull(resolved);
}
finally {
Expand All @@ -64,10 +65,9 @@ public void testCreateFileUserdir() throws Exception {
new DefaultLocalTouchFeature().touch(l);
try {
final AliasFilesystemBookmarkResolver resolver = new AliasFilesystemBookmarkResolver();
final String bookmark = resolver.create(l);
final NSData bookmark = resolver.create(l);
assertNotNull(bookmark);
l.setBookmark(bookmark);
final NSURL resolved = resolver.resolve(l, false);
final NSURL resolved = resolver.resolve(bookmark);
assertNotNull(resolved);
}
finally {
Expand Down
Loading

0 comments on commit 4f52cd2

Please sign in to comment.