Skip to content

Commit ae97492

Browse files
committed
Merge branch 'bugfix/GH-13936'
2 parents 614d33a + 8040e6b commit ae97492

File tree

6 files changed

+61
-61
lines changed

6 files changed

+61
-61
lines changed

cli/src/main/java/ch/cyberduck/cli/TerminalLoginService.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public TerminalLoginService(final CommandLine input) {
4343
}
4444

4545
@Override
46-
public void validate(final Host bookmark, final String message, final LoginCallback prompt, final LoginOptions options) throws LoginCanceledException, LoginFailureException {
46+
public void validate(final Host bookmark, final LoginCallback prompt, final LoginOptions options) throws LoginCanceledException, LoginFailureException {
4747
final Credentials credentials = bookmark.getCredentials();
4848
if(input.hasOption(TerminalOptionsBuilder.Params.anonymous.name())) {
4949
credentials.setUsername(PreferencesFactory.get().getProperty("connection.login.anon.name"));
@@ -60,6 +60,6 @@ public void validate(final Host bookmark, final String message, final LoginCallb
6060
if(StringUtils.isNotBlank(credentials.getUsername()) && StringUtils.isNotBlank(credentials.getPassword())) {
6161
return;
6262
}
63-
super.validate(bookmark, message, prompt, options);
63+
super.validate(bookmark, prompt, options);
6464
}
6565
}

core/src/main/java/ch/cyberduck/core/KeychainLoginService.java

Lines changed: 49 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,16 @@ public class KeychainLoginService implements LoginService {
3535

3636
private final HostPasswordStore keychain;
3737

38+
public KeychainLoginService() {
39+
this(PasswordStoreFactory.get());
40+
}
41+
3842
public KeychainLoginService(final HostPasswordStore keychain) {
3943
this.keychain = keychain;
4044
}
4145

4246
@Override
43-
public void validate(final Host bookmark, final String message, final LoginCallback prompt,
44-
final LoginOptions options) throws LoginCanceledException, LoginFailureException {
47+
public void validate(final Host bookmark, final LoginCallback prompt, final LoginOptions options) throws LoginCanceledException, LoginFailureException {
4548
if(log.isDebugEnabled()) {
4649
log.debug(String.format("Validate login credentials for %s", bookmark));
4750
}
@@ -103,10 +106,17 @@ public void validate(final Host bookmark, final String message, final LoginCallb
103106
}
104107
}
105108
if(!credentials.validate(bookmark.getProtocol(), options)) {
106-
final StringAppender details = new StringAppender();
107-
details.append(message);
108-
details.append(LocaleFactory.localizedString("No login credentials could be found in the Keychain", "Credentials"));
109-
this.prompt(bookmark, details.toString(), prompt, options);
109+
final StringAppender message = new StringAppender();
110+
if(options.password) {
111+
message.append(MessageFormat.format(LocaleFactory.localizedString(
112+
"Login {0} with username and password", "Credentials"), BookmarkNameProvider.toString(bookmark)));
113+
}
114+
if(options.publickey) {
115+
message.append(LocaleFactory.localizedString(
116+
"Select the private key in PEM or PuTTY format", "Credentials"));
117+
}
118+
message.append(LocaleFactory.localizedString("No login credentials could be found in the Keychain", "Credentials"));
119+
this.prompt(bookmark, message.toString(), prompt, options);
110120
}
111121
}
112122

@@ -124,19 +134,19 @@ public boolean prompt(final Host bookmark, final String message, final LoginCall
124134
final Credentials credentials = bookmark.getCredentials();
125135
if(options.password) {
126136
final Credentials input = prompt.prompt(bookmark, credentials.getUsername(),
127-
String.format("%s %s", LocaleFactory.localizedString("Login", "Login"), bookmark.getHostname()),
128-
message,
129-
options);
137+
String.format("%s %s", LocaleFactory.localizedString("Login", "Login"), bookmark.getHostname()),
138+
message,
139+
options);
130140
credentials.setSaved(input.isSaved());
131141
credentials.setUsername(input.getUsername());
132142
credentials.setPassword(input.getPassword());
133143
credentials.setIdentity(input.getIdentity());
134144
}
135145
if(options.token) {
136146
final Credentials input = prompt.prompt(bookmark,
137-
LocaleFactory.localizedString("Provide additional login credentials", "Credentials"),
138-
message,
139-
options);
147+
LocaleFactory.localizedString("Provide additional login credentials", "Credentials"),
148+
message,
149+
options);
140150
credentials.setSaved(input.isSaved());
141151
credentials.setToken(input.getPassword());
142152
}
@@ -154,19 +164,19 @@ public boolean authenticate(final Proxy proxy, final Session session, final Prog
154164
final Credentials credentials = bookmark.getCredentials();
155165
if(credentials.isPasswordAuthentication()) {
156166
listener.message(MessageFormat.format(LocaleFactory.localizedString("Authenticating as {0}", "Status"),
157-
credentials.getUsername()));
167+
credentials.getUsername()));
158168
}
159169
else if(credentials.isOAuthAuthentication()) {
160170
listener.message(MessageFormat.format(LocaleFactory.localizedString("Authenticating as {0}", "Status"),
161-
credentials.getOauth().getAccessToken()));
171+
credentials.getOauth().getAccessToken()));
162172
}
163173
else if(credentials.isPublicKeyAuthentication()) {
164174
listener.message(MessageFormat.format(LocaleFactory.localizedString("Authenticating as {0}", "Status"),
165-
credentials.getIdentity().getName()));
175+
credentials.getIdentity().getName()));
166176
}
167177
else if(credentials.isCertificateAuthentication()) {
168178
listener.message(MessageFormat.format(LocaleFactory.localizedString("Authenticating as {0}", "Status"),
169-
credentials.getCertificate()));
179+
credentials.getCertificate()));
170180
}
171181
try {
172182
if(log.isDebugEnabled()) {
@@ -177,24 +187,7 @@ else if(credentials.isCertificateAuthentication()) {
177187
log.debug(String.format("Login successful for session %s", session));
178188
}
179189
listener.message(LocaleFactory.localizedString("Login successful", "Credentials"));
180-
if(credentials.isSaved()) {
181-
// Write credentials to keychain
182-
try {
183-
keychain.save(bookmark);
184-
}
185-
catch(LocalAccessDeniedException e) {
186-
log.error(String.format("Failure saving credentials for %s in keychain. %s", bookmark, e));
187-
}
188-
}
189-
else {
190-
if(log.isInfoEnabled()) {
191-
log.info(String.format("Skip writing credentials for bookmark %s", bookmark.getHostname()));
192-
}
193-
}
194-
// Flag for successful authentication
195-
credentials.setPassed(true);
196-
// Nullify password and tokens
197-
credentials.reset();
190+
this.save(bookmark);
198191
return true;
199192
}
200193
catch(LoginFailureException e) {
@@ -213,4 +206,26 @@ else if(credentials.isCertificateAuthentication()) {
213206
throw e;
214207
}
215208
}
209+
210+
public void save(final Host bookmark) {
211+
final Credentials credentials = bookmark.getCredentials();
212+
if(credentials.isSaved()) {
213+
// Write credentials to keychain
214+
try {
215+
keychain.save(bookmark);
216+
}
217+
catch(LocalAccessDeniedException e) {
218+
log.error(String.format("Failure saving credentials for %s in keychain. %s", bookmark, e));
219+
}
220+
}
221+
else {
222+
if(log.isInfoEnabled()) {
223+
log.info(String.format("Skip writing credentials for bookmark %s", bookmark.getHostname()));
224+
}
225+
}
226+
// Flag for successful authentication
227+
credentials.setPassed(true);
228+
// Nullify password and tokens
229+
credentials.reset();
230+
}
216231
}

core/src/main/java/ch/cyberduck/core/LoginConnectionService.java

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -95,17 +95,7 @@ public boolean check(final Session<?> session, final CancelCallback callback) th
9595
}
9696
// Obtain password from keychain or prompt
9797
synchronized(login) {
98-
final LoginOptions options = new LoginOptions(bookmark.getProtocol());
99-
final StringAppender message = new StringAppender();
100-
if(options.password) {
101-
message.append(MessageFormat.format(LocaleFactory.localizedString(
102-
"Login {0} with username and password", "Credentials"), BookmarkNameProvider.toString(bookmark)));
103-
}
104-
if(options.publickey) {
105-
message.append(LocaleFactory.localizedString(
106-
"Select the private key in PEM or PuTTY format", "Credentials"));
107-
}
108-
login.validate(bookmark, message.toString(), prompt, options);
98+
login.validate(bookmark, prompt, new LoginOptions(bookmark.getProtocol()));
10999
}
110100
this.connect(session, callback);
111101
return true;

core/src/main/java/ch/cyberduck/core/LoginService.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,10 @@ public interface LoginService {
2828
* Obtain password from keychain or prompt panel
2929
*
3030
* @param bookmark Credentials
31-
* @param message Prompt message
3231
* @param pompt Login prompt
3332
* @param options Login mechanism features
3433
*/
35-
void validate(Host bookmark, String message, LoginCallback pompt, LoginOptions options) throws LoginCanceledException, LoginFailureException;
34+
void validate(Host bookmark, LoginCallback pompt, LoginOptions options) throws LoginCanceledException, LoginFailureException;
3635

3736
/**
3837
* Login and prompt on failure

core/src/test/java/ch/cyberduck/core/KeychainLoginServiceTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ else if(1 == i) {
5151
@Test(expected = LoginCanceledException.class)
5252
public void testCancel() throws Exception {
5353
LoginService l = new KeychainLoginService(new DisabledPasswordStore());
54-
l.validate(new Host(new TestProtocol(), "h"), "", new DisabledLoginCallback(), new LoginOptions());
54+
l.validate(new Host(new TestProtocol(), "h"), new DisabledLoginCallback(), new LoginOptions());
5555
}
5656

5757
@Test
@@ -68,7 +68,7 @@ public String findLoginPassword(final Host bookmark) {
6868
final Credentials credentials = new Credentials();
6969
credentials.setUsername("u");
7070
final Host host = new Host(new TestProtocol(), "test.cyberduck.ch", credentials);
71-
l.validate(host, "m", new DisabledLoginCallback(), new LoginOptions(host.getProtocol()));
71+
l.validate(host, new DisabledLoginCallback(), new LoginOptions(host.getProtocol()));
7272
assertTrue(keychain.get());
7373
assertFalse(host.getCredentials().isSaved());
7474
assertEquals("P", host.getCredentials().getPassword());

ssh/src/main/java/ch/cyberduck/core/sftp/SFTPSession.java

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -148,21 +148,17 @@ protected SSHClient connect(final HostKeyCallback key, final LoginCallback promp
148148
log.info(String.format("Connect using jump host configuration %s", proxy));
149149
final SSHClient hop = this.toClient(key, configuration);
150150
hop.connect(proxy.getHostname(), proxy.getPort());
151-
proxy.setCredentials(new OpenSSHCredentialsConfigurator().configure(proxy));
151+
final Credentials proxyCredentials = new OpenSSHCredentialsConfigurator().configure(proxy);
152+
proxy.setCredentials(proxyCredentials);
153+
final KeychainLoginService service = new KeychainLoginService();
154+
service.validate(proxy, prompt, new LoginOptions(proxy.getProtocol()));
152155
// Authenticate with jump host
153156
this.authenticate(hop, proxy, prompt, new DisabledCancelCallback());
154157
if(log.isDebugEnabled()) {
155158
log.debug(String.format("Authenticated with jump host %s", proxy));
156159
}
157-
if(proxy.getCredentials().isSaved()) {
158-
// Write credentials to keychain
159-
try {
160-
PasswordStoreFactory.get().save(proxy);
161-
}
162-
catch(LocalAccessDeniedException e) {
163-
log.error(String.format("Failure saving credentials for %s in keychain. %s", proxy, e));
164-
}
165-
}
160+
// Write credentials to keychain
161+
service.save(proxy);
166162
final DirectConnection tunnel = hop.newDirectConnection(
167163
new OpenSSHHostnameConfigurator().getHostname(host.getHostname()), host.getPort());
168164
// Connect to internal host

0 commit comments

Comments
 (0)