Skip to content

Mask passwords while error logging #5

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

Open
SOVALINUX wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Mask passwords while error logging #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c3f7fc
Mask passwords while error logging
SOVALINUX Aug 27, 2015
d761887
Display fixed length masked password
SOVALINUX Aug 28, 2015
a00a2e7
Fix git config test for fixed length passwords
SOVALINUX Aug 28, 2015
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
14 changes: 9 additions & 5 deletions src/com/tw/go/plugin/cmd/Console.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ public static CommandLine createCommand(String... args) {
gitCmd.addArguments(args);
return gitCmd;
}

public static ConsoleResult runOrBomb(CommandLine commandLine, File workingDir, ProcessOutputStreamConsumer stdOut, ProcessOutputStreamConsumer stdErr) {
return runOrBomb(commandLine, workingDir, stdOut, stdErr, commandLine.toString());
Copy link
Owner

@srinivasupadhya srinivasupadhya Aug 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if i understand this right, you are passing commandLine.toString() as prettyMessage and using it in getMessage(). But isn't that what's happening anyway?

Copy link
Author

@SOVALINUX SOVALINUX Aug 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saved previous behaviour in method with 4 params and added new one with 5 and pass different prettyMessage only where needed

}

public static ConsoleResult runOrBomb(CommandLine commandLine, File workingDir, ProcessOutputStreamConsumer stdOut, ProcessOutputStreamConsumer stdErr, String prettyMessage) {
Executor executor = new DefaultExecutor();
executor.setStreamHandler(new PumpStreamHandler(stdOut, stdErr));
if (workingDir != null) {
Expand All @@ -25,16 +29,16 @@ public static ConsoleResult runOrBomb(CommandLine commandLine, File workingDir,
int exitCode = executor.execute(commandLine);

if (exitCode != 0) {
throw new RuntimeException(getMessage("Error", commandLine, workingDir));
throw new RuntimeException(getMessage("Error", prettyMessage, workingDir));
}

return new ConsoleResult(exitCode, stdOut.output(), stdErr.output());
} catch (Exception e) {
throw new RuntimeException(getMessage("Exception", commandLine, workingDir), e);
throw new RuntimeException(getMessage("Exception", prettyMessage, workingDir), e);
}
}

private static String getMessage(String type, CommandLine commandLine, File workingDir) {
return String.format("%s Occurred: %s - %s", type, commandLine.toString(), workingDir);
private static String getMessage(String type, String prettyMessage, File workingDir) {
return String.format("%s Occurred: %s - %s", type, prettyMessage, workingDir);
}
}
19 changes: 16 additions & 3 deletions src/com/tw/go/plugin/git/GitCmdHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ public String version() {
@Override
public void checkConnection() {
CommandLine gitCmd = Console.createCommand("ls-remote", gitConfig.getEffectiveUrl());
runAndGetOutput(gitCmd);
CommandLine gitCmdMasked = Console.createCommand("ls-remote", gitConfig.getEffectiveMaskedUrl());
Copy link
Owner

@srinivasupadhya srinivasupadhya Aug 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are replacing gitConfig.getEffectiveUrl() with gitConfig.getEffectiveMaskedUrl()? does ls-remote work with password masked?

Copy link
Author

@SOVALINUX SOVALINUX Aug 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gitCmdMasked here only for output in case of errors, to real execution we still pass gitCmd. Difference only in output while error

runAndGetOutput(gitCmd, workingDir, gitCmdMasked.toString());
}

@Override
Expand All @@ -49,10 +50,14 @@ public void cloneRepository() {
if (gitConfig.isShallowClone()) {
args.add("--depth=1");
}
List<String> maskedArgs = new ArrayList<String>(args);
args.add(gitConfig.getEffectiveUrl());
maskedArgs.add(gitConfig.getEffectiveMaskedUrl());
args.add(workingDir.getAbsolutePath());
maskedArgs.add(workingDir.getAbsolutePath());
CommandLine gitClone = Console.createCommand(ListUtil.toArray(args));
runAndGetOutput(gitClone, null, stdOut, stdErr);
CommandLine gitCloneMasked = Console.createCommand(ListUtil.toArray(maskedArgs));
runAndGetOutput(gitClone, null, stdOut, stdErr, gitCloneMasked.toString());
}

@Override
Expand Down Expand Up @@ -388,12 +393,20 @@ private ConsoleResult runOrBomb(CommandLine gitCmd) {
private ConsoleResult runAndGetOutput(CommandLine gitCmd) {
return runAndGetOutput(gitCmd, workingDir);
}

private ConsoleResult runAndGetOutput(CommandLine gitCmd, File workingDir, String prettyMessage) {
return runAndGetOutput(gitCmd, workingDir, new ProcessOutputStreamConsumer(new InMemoryConsumer()), new ProcessOutputStreamConsumer(new InMemoryConsumer()), prettyMessage);
}

private ConsoleResult runAndGetOutput(CommandLine gitCmd, File workingDir) {
return runAndGetOutput(gitCmd, workingDir, new ProcessOutputStreamConsumer(new InMemoryConsumer()), new ProcessOutputStreamConsumer(new InMemoryConsumer()));
}

private ConsoleResult runAndGetOutput(CommandLine gitCmd, File workingDir, ProcessOutputStreamConsumer stdOut, ProcessOutputStreamConsumer stdErr) {
return Console.runOrBomb(gitCmd, workingDir, stdOut, stdErr);
return runAndGetOutput(gitCmd, workingDir, stdOut, stdErr, gitCmd.toString());
}

private ConsoleResult runAndGetOutput(CommandLine gitCmd, File workingDir, ProcessOutputStreamConsumer stdOut, ProcessOutputStreamConsumer stdErr, String prettyMessage) {
return Console.runOrBomb(gitCmd, workingDir, stdOut, stdErr, prettyMessage);
}
}
21 changes: 19 additions & 2 deletions src/com/tw/go/plugin/model/GitConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import com.tw.go.plugin.util.StringUtil;

public class GitConfig {
private static final String MASKED_PASSWORD = "*****";

private String url;
private String username;
private String password;
Expand Down Expand Up @@ -42,10 +44,25 @@ public String getEffectiveUrl() {
}
return getUrl();
}

public String getEffectiveMaskedUrl() {
if (isRemoteUrl() && hasCredentials()) {
return getUrlWithMaskedCredentials();
}
return getUrl();
}

private String getUrlWithCredentials(String user, String pass) {
String[] parts = url.split("://");
return String.format("%s://%s:%s@%s", parts[0], user, pass, parts[1]);
}

public String getUrlWithCredentials() {
String[] parts = url.split("://");
return String.format("%s://%s:%s@%s", parts[0], username, password, parts[1]);
return getUrlWithCredentials(username, password);
}

public String getUrlWithMaskedCredentials() {
return getUrlWithCredentials(username, MASKED_PASSWORD);
}

public String getUrl() {
Expand Down
8 changes: 8 additions & 0 deletions src/com/tw/go/plugin/util/StringUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,12 @@ public class StringUtil {
public static boolean isEmpty(String str) {
return str == null || str.trim().isEmpty();
}

public static String repeat(String str, int count) {
StringBuilder result = new StringBuilder();
for (int i = 0; i < count; i++) {
result.append(str);
}
return result.toString();
}
}
9 changes: 9 additions & 0 deletions test/com/tw/go/plugin/model/GitConfigTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ public void shouldGetEffectiveUrl() throws Exception {
assertThat(new GitConfig("http://github.com/gocd/gocd", "username", "password", null).getEffectiveUrl(), is("http://username:password@github.com/gocd/gocd"));
assertThat(new GitConfig("https://github.com/gocd/gocd", "username", "password", null).getEffectiveUrl(), is("https://username:password@github.com/gocd/gocd"));
}

@Test
public void shouldGetEffectiveMaskedUrl() throws Exception {
assertThat(new GitConfig("/tmp/git-repo", null, null, null).getEffectiveMaskedUrl(), is("/tmp/git-repo"));
assertThat(new GitConfig("/tmp/git-repo", "username", "password", null).getEffectiveMaskedUrl(), is("/tmp/git-repo"));
assertThat(new GitConfig("http://github.com/gocd/gocd", null, null, null).getEffectiveMaskedUrl(), is("http://github.com/gocd/gocd"));
assertThat(new GitConfig("http://github.com/gocd/gocd", "username", "password", null).getEffectiveMaskedUrl(), is("http://username:*****@github.com/gocd/gocd"));
assertThat(new GitConfig("https://github.com/gocd/gocd", "username", "password", null).getEffectiveMaskedUrl(), is("https://username:*****@github.com/gocd/gocd"));
}

@Test
public void shouldGetEffectiveBranch() throws Exception {
Expand Down