Skip to content

Fix #13936. #13940

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.

Sign up for GitHub

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

Merged
dkocher merged 2 commits into from
Dec 5, 2022
Merged

Fix #13936. #13940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
156b47f
Review.
dkocher Dec 5, 2022
8040e6b
Fix #13936.
dkocher Dec 5, 2022
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
4 changes: 2 additions & 2 deletions cli/src/main/java/ch/cyberduck/cli/TerminalLoginService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public TerminalLoginService(final CommandLine input) {
}

@Override
public void validate(final Host bookmark, final String message, final LoginCallback prompt, final LoginOptions options) throws LoginCanceledException, LoginFailureException {
public void validate(final Host bookmark, final LoginCallback prompt, final LoginOptions options) throws LoginCanceledException, LoginFailureException {
final Credentials credentials = bookmark.getCredentials();
if(input.hasOption(TerminalOptionsBuilder.Params.anonymous.name())) {
credentials.setUsername(PreferencesFactory.get().getProperty("connection.login.anon.name"));
Expand All @@ -60,6 +60,6 @@ public void validate(final Host bookmark, final String message, final LoginCallb
if(StringUtils.isNotBlank(credentials.getUsername()) && StringUtils.isNotBlank(credentials.getPassword())) {
return;
}
super.validate(bookmark, message, prompt, options);
super.validate(bookmark, prompt, options);
}
}
83 changes: 49 additions & 34 deletions core/src/main/java/ch/cyberduck/core/KeychainLoginService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,16 @@ public class KeychainLoginService implements LoginService {

private final HostPasswordStore keychain;

public KeychainLoginService() {
this(PasswordStoreFactory.get());
}

public KeychainLoginService(final HostPasswordStore keychain) {
this.keychain = keychain;
}

@Override
public void validate(final Host bookmark, final String message, final LoginCallback prompt,
final LoginOptions options) throws LoginCanceledException, LoginFailureException {
public void validate(final Host bookmark, final LoginCallback prompt, final LoginOptions options) throws LoginCanceledException, LoginFailureException {
if(log.isDebugEnabled()) {
log.debug(String.format("Validate login credentials for %s", bookmark));
}
Expand Down Expand Up @@ -103,10 +106,17 @@ public void validate(final Host bookmark, final String message, final LoginCallb
}
}
if(!credentials.validate(bookmark.getProtocol(), options)) {
final StringAppender details = new StringAppender();
details.append(message);
details.append(LocaleFactory.localizedString("No login credentials could be found in the Keychain", "Credentials"));
this.prompt(bookmark, details.toString(), prompt, options);
final StringAppender message = new StringAppender();
if(options.password) {
message.append(MessageFormat.format(LocaleFactory.localizedString(
"Login {0} with username and password", "Credentials"), BookmarkNameProvider.toString(bookmark)));
}
if(options.publickey) {
message.append(LocaleFactory.localizedString(
"Select the private key in PEM or PuTTY format", "Credentials"));
}
message.append(LocaleFactory.localizedString("No login credentials could be found in the Keychain", "Credentials"));
this.prompt(bookmark, message.toString(), prompt, options);
}
}

Expand All @@ -124,19 +134,19 @@ public boolean prompt(final Host bookmark, final String message, final LoginCall
final Credentials credentials = bookmark.getCredentials();
if(options.password) {
final Credentials input = prompt.prompt(bookmark, credentials.getUsername(),
String.format("%s %s", LocaleFactory.localizedString("Login", "Login"), bookmark.getHostname()),
message,
options);
String.format("%s %s", LocaleFactory.localizedString("Login", "Login"), bookmark.getHostname()),
message,
options);
credentials.setSaved(input.isSaved());
credentials.setUsername(input.getUsername());
credentials.setPassword(input.getPassword());
credentials.setIdentity(input.getIdentity());
}
if(options.token) {
final Credentials input = prompt.prompt(bookmark,
LocaleFactory.localizedString("Provide additional login credentials", "Credentials"),
message,
options);
LocaleFactory.localizedString("Provide additional login credentials", "Credentials"),
message,
options);
credentials.setSaved(input.isSaved());
credentials.setToken(input.getPassword());
}
Expand All @@ -154,19 +164,19 @@ public boolean authenticate(final Proxy proxy, final Session session, final Prog
final Credentials credentials = bookmark.getCredentials();
if(credentials.isPasswordAuthentication()) {
listener.message(MessageFormat.format(LocaleFactory.localizedString("Authenticating as {0}", "Status"),
credentials.getUsername()));
credentials.getUsername()));
}
else if(credentials.isOAuthAuthentication()) {
listener.message(MessageFormat.format(LocaleFactory.localizedString("Authenticating as {0}", "Status"),
credentials.getOauth().getAccessToken()));
credentials.getOauth().getAccessToken()));
}
else if(credentials.isPublicKeyAuthentication()) {
listener.message(MessageFormat.format(LocaleFactory.localizedString("Authenticating as {0}", "Status"),
credentials.getIdentity().getName()));
credentials.getIdentity().getName()));
}
else if(credentials.isCertificateAuthentication()) {
listener.message(MessageFormat.format(LocaleFactory.localizedString("Authenticating as {0}", "Status"),
credentials.getCertificate()));
credentials.getCertificate()));
}
try {
if(log.isDebugEnabled()) {
Expand All @@ -177,24 +187,7 @@ else if(credentials.isCertificateAuthentication()) {
log.debug(String.format("Login successful for session %s", session));
}
listener.message(LocaleFactory.localizedString("Login successful", "Credentials"));
if(credentials.isSaved()) {
// Write credentials to keychain
try {
keychain.save(bookmark);
}
catch(LocalAccessDeniedException e) {
log.error(String.format("Failure saving credentials for %s in keychain. %s", bookmark, e));
}
}
else {
if(log.isInfoEnabled()) {
log.info(String.format("Skip writing credentials for bookmark %s", bookmark.getHostname()));
}
}
// Flag for successful authentication
credentials.setPassed(true);
// Nullify password and tokens
credentials.reset();
this.save(bookmark);
return true;
}
catch(LoginFailureException e) {
Expand All @@ -213,4 +206,26 @@ else if(credentials.isCertificateAuthentication()) {
throw e;
}
}

public void save(final Host bookmark) {
final Credentials credentials = bookmark.getCredentials();
if(credentials.isSaved()) {
// Write credentials to keychain
try {
keychain.save(bookmark);
}
catch(LocalAccessDeniedException e) {
log.error(String.format("Failure saving credentials for %s in keychain. %s", bookmark, e));
}
}
else {
if(log.isInfoEnabled()) {
log.info(String.format("Skip writing credentials for bookmark %s", bookmark.getHostname()));
}
}
// Flag for successful authentication
credentials.setPassed(true);
// Nullify password and tokens
credentials.reset();
}
}
12 changes: 1 addition & 11 deletions core/src/main/java/ch/cyberduck/core/LoginConnectionService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,17 +95,7 @@ public boolean check(final Session<?> session, final CancelCallback callback) th
}
// Obtain password from keychain or prompt
synchronized(login) {
final LoginOptions options = new LoginOptions(bookmark.getProtocol());
final StringAppender message = new StringAppender();
if(options.password) {
message.append(MessageFormat.format(LocaleFactory.localizedString(
"Login {0} with username and password", "Credentials"), BookmarkNameProvider.toString(bookmark)));
}
if(options.publickey) {
message.append(LocaleFactory.localizedString(
"Select the private key in PEM or PuTTY format", "Credentials"));
}
login.validate(bookmark, message.toString(), prompt, options);
login.validate(bookmark, prompt, new LoginOptions(bookmark.getProtocol()));
}
this.connect(session, callback);
return true;
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/ch/cyberduck/core/LoginService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,10 @@ public interface LoginService {
* Obtain password from keychain or prompt panel
*
* @param bookmark Credentials
* @param message Prompt message
* @param pompt Login prompt
* @param options Login mechanism features
*/
void validate(Host bookmark, String message, LoginCallback pompt, LoginOptions options) throws LoginCanceledException, LoginFailureException;
void validate(Host bookmark, LoginCallback pompt, LoginOptions options) throws LoginCanceledException, LoginFailureException;

/**
* Login and prompt on failure
Expand Down
4 changes: 2 additions & 2 deletions core/src/test/java/ch/cyberduck/core/KeychainLoginServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ else if(1 == i) {
@Test(expected = LoginCanceledException.class)
public void testCancel() throws Exception {
LoginService l = new KeychainLoginService(new DisabledPasswordStore());
l.validate(new Host(new TestProtocol(), "h"), "", new DisabledLoginCallback(), new LoginOptions());
l.validate(new Host(new TestProtocol(), "h"), new DisabledLoginCallback(), new LoginOptions());
}

@Test
Expand All @@ -68,7 +68,7 @@ public String findLoginPassword(final Host bookmark) {
final Credentials credentials = new Credentials();
credentials.setUsername("u");
final Host host = new Host(new TestProtocol(), "test.cyberduck.ch", credentials);
l.validate(host, "m", new DisabledLoginCallback(), new LoginOptions(host.getProtocol()));
l.validate(host, new DisabledLoginCallback(), new LoginOptions(host.getProtocol()));
assertTrue(keychain.get());
assertFalse(host.getCredentials().isSaved());
assertEquals("P", host.getCredentials().getPassword());
Expand Down
16 changes: 6 additions & 10 deletions ssh/src/main/java/ch/cyberduck/core/sftp/SFTPSession.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,21 +148,17 @@ protected SSHClient connect(final HostKeyCallback key, final LoginCallback promp
log.info(String.format("Connect using jump host configuration %s", proxy));
final SSHClient hop = this.toClient(key, configuration);
hop.connect(proxy.getHostname(), proxy.getPort());
proxy.setCredentials(new OpenSSHCredentialsConfigurator().configure(proxy));
final Credentials proxyCredentials = new OpenSSHCredentialsConfigurator().configure(proxy);
proxy.setCredentials(proxyCredentials);
final KeychainLoginService service = new KeychainLoginService();
service.validate(proxy, prompt, new LoginOptions(proxy.getProtocol()));
// Authenticate with jump host
this.authenticate(hop, proxy, prompt, new DisabledCancelCallback());
if(log.isDebugEnabled()) {
log.debug(String.format("Authenticated with jump host %s", proxy));
}
if(proxy.getCredentials().isSaved()) {
// Write credentials to keychain
try {
PasswordStoreFactory.get().save(proxy);
}
catch(LocalAccessDeniedException e) {
log.error(String.format("Failure saving credentials for %s in keychain. %s", proxy, e));
}
}
// Write credentials to keychain
service.save(proxy);
final DirectConnection tunnel = hop.newDirectConnection(
new OpenSSHHostnameConfigurator().getHostname(host.getHostname()), host.getPort());
// Connect to internal host
Expand Down