Skip to content

Commit

Permalink
Allow special characters in credentials (LT-20157, LT-21275) (#316)
Browse files Browse the repository at this point in the history
* Escape credentials used in URLs
Some users use their email addresses as their logins. When we overhauled the Get from Internet UX, we inadvertently stopped escaping credentials.
* Allow users to type spaces in the password field
* Properly encode spaces in passwords (not usernames)
We are not allowing spaces in usernames, although we might could. I am unaware of any other services that allow spaces in usernames
* Use WebUtility instead of HttpUtility (in some places)
* Clean up HgServeRunner (indentation, change HttpUtility to WebUtility)
* Test that spaces in passwords are encoded properly
* Run more tests (some had been needlessly excluded)
* Foolproof a test against developer machines set to use the QA server

Addresses https://jira.sil.org/browse/LT-20157 and https://jira.sil.org/browse/LT-21275
  • Loading branch information
papeh authored Mar 2, 2023
1 parent 9547608 commit 7504501
Show file tree
Hide file tree
Showing 11 changed files with 129 additions and 87 deletions.
2 changes: 0 additions & 2 deletions src/Chorus/UI/Misc/ServerSettingsControl.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/Chorus/UI/Misc/ServerSettingsControl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ private void _textBox_KeyPress(object sender, KeyPressEventArgs e)

private void _password_TextChanged(object sender, EventArgs e)
{
Model.Password = _password.Text.Trim();
Model.Password = _password.Text;
UpdateDisplay();
}

Expand Down
51 changes: 28 additions & 23 deletions src/ChorusHub/HgServeRunner.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
using System;
using System.Diagnostics;
using System.IO;
using System.Net;
using System.Text;
using System.Threading;
using System.Web;
using Chorus.VcsDrivers.Mercurial;
using SIL.CommandLineProcessing;
using SIL.Progress;
Expand Down Expand Up @@ -140,40 +140,45 @@ public void CheckForFailedPushes()
try
{
if (!File.Exists(AccessLogPath))
{
return;
}

using (var stream = File.Open(AccessLogPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite))
using (TextReader reader = new StreamReader(stream))
{
while (true)
{
var line = reader.ReadLine();
if (line == null)
return;
var line = reader.ReadLine();
if (line == null)
{
return;
}

var start = line.IndexOf("GET /") + 5;
var end = line.IndexOf("?");
if (line.Contains("404") && start > 9 & end > 0)
var start = line.IndexOf("GET /", StringComparison.Ordinal) + 5;
var end = line.IndexOf("?", StringComparison.Ordinal);
if (line.Contains("404") && start > 9 & end > 0)
{
var name = line.Substring(start, end - start);
string directory = Path.Combine(_rootFolder, name);

directory = WebUtility.UrlDecode(directory); // convert %20 --> space
if (!Directory.Exists(directory))
{
var name = line.Substring(start, end - start);
string directory = Path.Combine(_rootFolder, name);

directory = HttpUtility.UrlDecode(directory); // convert %20 --> space
if (!Directory.Exists(directory))
{
//Progress.WriteMessage("Creating new folder '" + name + "'");
Directory.CreateDirectory(directory);
}
if (!Directory.Exists(Path.Combine(directory, ".hg")))
{
//Progress.WriteMessage("Initializing blank repository: " + name +
// ". Try Sending again in a few minutes, when hg notices the new directory.");
HgRepository.CreateRepositoryInExistingDir(directory, new ConsoleProgress());
}
//Progress.WriteMessage("Creating new folder '" + name + "'");
Directory.CreateDirectory(directory);
}

if (!Directory.Exists(Path.Combine(directory, ".hg")))
{
//Progress.WriteMessage("Initializing blank repository: " + name +
// ". Try Sending again in a few minutes, when hg notices the new directory.");
HgRepository.CreateRepositoryInExistingDir(directory, new ConsoleProgress());
}
}
}
}
}
}
catch
{
//EventLog.WriteEntry("Application", error.Message, EventLogEntryType.Error);
Expand Down
37 changes: 25 additions & 12 deletions src/LibChorus/Model/ServerSettingsModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
using System.Net;
using System.Security.Cryptography;
using System.Text;
using System.Web;
using Chorus.Properties;
using Chorus.Utilities;
using Chorus.VcsDrivers;
using Chorus.VcsDrivers.Mercurial;
Expand Down Expand Up @@ -91,8 +91,8 @@ static ServerSettingsModel()

public ServerSettingsModel()
{
Username = Properties.Settings.Default.LanguageForgeUser;
Password = DecryptPassword(Properties.Settings.Default.LanguageForgePass);
Username = Settings.Default.LanguageForgeUser;
Password = DecryptPassword(Settings.Default.LanguageForgePass);
RememberPassword = !string.IsNullOrEmpty(Password) || string.IsNullOrEmpty(Username);
}

Expand All @@ -118,13 +118,13 @@ public virtual void InitFromProjectPath(string path)

public virtual void InitFromUri(string url)
{
var urlUsername = HttpUtility.UrlDecode(UrlHelper.GetUserName(url));
var urlUsername = WebUtility.UrlDecode(UrlHelper.GetUserName(url));
if (!string.IsNullOrEmpty(urlUsername))
{
Username = urlUsername;
Password = HttpUtility.UrlDecode(UrlHelper.GetPassword(url));
Password = WebUtility.UrlDecode(UrlHelper.GetPassword(url));
}
ProjectId = HttpUtility.UrlDecode(UrlHelper.GetPathAfterHost(url));
ProjectId = WebUtility.UrlDecode(UrlHelper.GetPathAfterHost(url));
HasLoggedIn = !string.IsNullOrEmpty(ProjectId);
Bandwidth = new BandwidthItem(RepositoryAddress.IsKnownResumableRepository(url) ? BandwidthEnum.Low : BandwidthEnum.High);

Expand All @@ -146,7 +146,7 @@ public string URL
return CustomUrl;
}

return $"https://{Host}/{HttpUtility.UrlEncode(ProjectId)}";
return $"https://{Host}/{WebUtility.UrlEncode(ProjectId)}";
}
}

Expand Down Expand Up @@ -264,10 +264,10 @@ protected RepositoryAddress CreateRepositoryAddress(string name)

public void SaveUserSettings()
{
Properties.Settings.Default.LanguageForgeUser = Username;
Properties.Settings.Default.LanguageForgePass = RememberPassword ? EncryptPassword(Password) : null;
Settings.Default.LanguageForgeUser = Username;
Settings.Default.LanguageForgePass = RememberPassword ? EncryptPassword(Password) : null;
PasswordForSession = Password;
Properties.Settings.Default.Save();
Settings.Default.Save();
}

public void LogIn(out string error)
Expand Down Expand Up @@ -377,6 +377,19 @@ internal static string DecryptPassword(string decryptMe)
return Encoding.Unicode.GetString(decryptedData);
}

/// <summary>
/// URL-encoded password to use for the current Send and Receive session. <see cref="PasswordForSession"/>
/// </summary>
/// <remarks>
/// UrlEncode encodes spaces as "+" and "+" as "%2b". LanguageDepot fails to decode plus-encoded spaces. Encode spaces as "%20"
/// </remarks>
public static string EncodedPasswordForSession => WebUtility.UrlEncode(PasswordForSession)?.Replace("+", "%20");

/// <summary>
/// URL-encoded language forge username
/// </summary>
public static string EncodedLanguageForgeUser => WebUtility.UrlEncode(Settings.Default.LanguageForgeUser);

private static string _passwordForSession;

/// <summary>
Expand All @@ -387,7 +400,7 @@ internal static string DecryptPassword(string decryptMe)
/// </summary>
public static string PasswordForSession
{
internal get { return _passwordForSession ?? DecryptPassword(Properties.Settings.Default.LanguageForgePass); }
internal get { return _passwordForSession ?? DecryptPassword(Settings.Default.LanguageForgePass); }
set { _passwordForSession = value; }
}

Expand All @@ -402,7 +415,7 @@ public static string PasswordForSession
/// <param name="clearString">Any string containing a URL with the <see cref="PasswordForSession"/> in clear text.</param>
internal static string RemovePasswordForLog(string clearString)
{
return clearString?.Replace($":{PasswordForSession}@", $":{PasswordAsterisks}@");
return clearString?.Replace($":{EncodedPasswordForSession}@", $":{PasswordAsterisks}@");
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions src/LibChorus/VcsDrivers/RepositoryAddress.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public abstract class RepositoryAddress
/// <summary>
/// This message is displayed when the user tries to create a new repository with the same name as an
/// unrelated, existing repository. Because of the way this string is used, two customizations are necessary:
/// - replace <see cref="MediumVariable"/> with the name of the syncronization medium
/// - replace <see cref="MediumVariable"/> with the name of the synchronization medium
/// - replace {0} and {0} with the URI's of the existing and new repositories, respectively
/// </summary>
public const string DuplicateWarningMessage = "Warning: There is a project repository on the " + MediumVariable
Expand Down Expand Up @@ -163,7 +163,7 @@ public override string GetPotentialRepoUri(string repoIdentifier, string project
// Our resumable API supports passing credentials in request headers; Mercurial requires them in the URL.
if (!IsResumable && string.IsNullOrEmpty(UrlHelper.GetUserName(uri)))
{
uri = uri.Replace("://", $"://{Properties.Settings.Default.LanguageForgeUser}:{ServerSettingsModel.PasswordForSession}@");
uri = uri.Replace("://", $"://{ServerSettingsModel.EncodedLanguageForgeUser}:{ServerSettingsModel.EncodedPasswordForSession}@");
}
return uri;
}
Expand Down
4 changes: 3 additions & 1 deletion src/LibChorusTests/Model/InternetCloneSettingsModelTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using NUnit.Framework;
using SIL.TestUtilities;
using Chorus.Model;
using Chorus.Utilities;

namespace LibChorus.Tests.Model
{
Expand All @@ -26,7 +27,8 @@ public void InitFromUri_GivenCompleteUri_AllPropertiesCorrect()
[Test]
public void URL_AfterConstruction_GoodDefault()
{
using (var testFolder = new TemporaryFolder("clonetest"))
using (var testFolder = new TemporaryFolder("cloneTest"))
using (new ShortTermEnvironmentalVariable(ServerSettingsModel.ServerEnvVar, null))
{
var model = new InternetCloneSettingsModel(testFolder.Path);
model.Username = "account";
Expand Down
Loading

0 comments on commit 7504501

Please sign in to comment.