Skip to content

Commit

Permalink
Fix issue 1043: Obfuscate AI data on request redirect (#5236)
Browse files Browse the repository at this point in the history
Fix for issue 1043 - obfuscate the AI data on request redirect.
cristinamanum authored Jan 10, 2018
1 parent 90e8ada commit 18bc2a1
Showing 7 changed files with 216 additions and 91 deletions.
24 changes: 16 additions & 8 deletions src/NuGetGallery/App_Start/Routes.cs
Original file line number Diff line number Diff line change
@@ -147,17 +147,20 @@ public static void RegisterUIRoutes(RouteCollection routes)
routes.MapRoute(
RouteName.PackageOwnerConfirmation,
"packages/{id}/owners/{username}/confirm/{token}",
new { controller = "Packages", action = "ConfirmPendingOwnershipRequest" });
new { controller = "Packages", action = "ConfirmPendingOwnershipRequest" },
new RouteExtensions.ObfuscatedMetadata(3, Obfuscator.DefaultTelemetryUserName));

routes.MapRoute(
RouteName.PackageOwnerRejection,
"packages/{id}/owners/{username}/reject/{token}",
new { controller = "Packages", action = "RejectPendingOwnershipRequest" });
new { controller = "Packages", action = "RejectPendingOwnershipRequest" },
new RouteExtensions.ObfuscatedMetadata(3, Obfuscator.DefaultTelemetryUserName));

routes.MapRoute(
RouteName.PackageOwnerCancellation,
"packages/{id}/owners/{username}/cancel/{token}",
new { controller = "Packages", action = "CancelPendingOwnershipRequest" });
new { controller = "Packages", action = "CancelPendingOwnershipRequest" },
new RouteExtensions.ObfuscatedMetadata(3, Obfuscator.DefaultTelemetryUserName));

// We need the following two routes (rather than just one) due to Routing's
// Consecutive Optional Parameter bug. :(
@@ -242,7 +245,8 @@ public static void RegisterUIRoutes(RouteCollection routes)
routes.MapRoute(
RouteName.Profile,
"profiles/{username}",
new { controller = "Users", action = "Profiles" });
new { controller = "Users", action = "Profiles" },
new RouteExtensions.ObfuscatedMetadata(1, Obfuscator.DefaultTelemetryUserName));

routes.MapRoute(
RouteName.RemovePassword,
@@ -257,17 +261,20 @@ public static void RegisterUIRoutes(RouteCollection routes)
routes.MapRoute(
RouteName.PasswordReset,
"account/forgotpassword/{username}/{token}",
new { controller = "Users", action = "ResetPassword", forgot = true });
new { controller = "Users", action = "ResetPassword", forgot = true },
new RouteExtensions.ObfuscatedMetadata(2, Obfuscator.DefaultTelemetryUserName));

routes.MapRoute(
RouteName.PasswordSet,
"account/setpassword/{username}/{token}",
new { controller = "Users", action = "ResetPassword", forgot = false });
new { controller = "Users", action = "ResetPassword", forgot = false },
new RouteExtensions.ObfuscatedMetadata(2, Obfuscator.DefaultTelemetryUserName));

routes.MapRoute(
RouteName.ConfirmAccount,
"account/confirm/{username}/{token}",
new { controller = "Users", action = "Confirm" });
new { controller = "Users", action = "Confirm" },
new RouteExtensions.ObfuscatedMetadata(2, Obfuscator.DefaultTelemetryUserName));

routes.MapRoute(
RouteName.ChangeEmailSubscription,
@@ -277,7 +284,8 @@ public static void RegisterUIRoutes(RouteCollection routes)
routes.MapRoute(
RouteName.AdminDeleteAccount,
"account/delete/{accountName}",
new { controller = "Users", action = "Delete" });
new { controller = "Users", action = "Delete" },
new RouteExtensions.ObfuscatedMetadata(2, Obfuscator.DefaultTelemetryUserName));

routes.MapRoute(
RouteName.UserDeleteAccount,
49 changes: 49 additions & 0 deletions src/NuGetGallery/Extensions/RouteExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Generic;
using System.Linq;
using System.Web.Mvc;
using System.Web.Routing;

namespace NuGetGallery
{
public static class RouteExtensions
{
public struct ObfuscatedMetadata
{
public int ObfuscatedSegment
{ get; }

public string ObfuscateValue
{ get; }

public ObfuscatedMetadata(int obfuscatedSegment, string obfuscateValue)
{
ObfuscatedSegment = obfuscatedSegment;
ObfuscateValue = obfuscateValue;
}
}

internal static Dictionary<string, ObfuscatedMetadata> ObfuscatedRouteMap = new Dictionary<string, ObfuscatedMetadata>();

public static void MapRoute(this RouteCollection routes, string name, string url, object defaults, ObfuscatedMetadata obfuscationMetadata)
{
routes.MapRoute(name, url, defaults);
if (!ObfuscatedRouteMap.ContainsKey(url)) { ObfuscatedRouteMap.Add(url, obfuscationMetadata); }
}

public static string ObfuscateUrlPath(this Route route, string urlPath)
{
var path = route.Url;
if (!ObfuscatedRouteMap.ContainsKey(path))
{
return null;
}
var metadata = ObfuscatedRouteMap[path];
string[] segments = urlPath.Split('/');
segments[metadata.ObfuscatedSegment] = metadata.ObfuscateValue;
return string.Join("/", segments);
}
}
}
1 change: 1 addition & 0 deletions src/NuGetGallery/NuGetGallery.csproj
Original file line number Diff line number Diff line change
@@ -784,6 +784,7 @@
<Compile Include="Controllers\PagesController.cs" />
<Compile Include="Extensions\OrganizationExtensions.cs" />
<Compile Include="Extensions\PrincipalExtensions.cs" />
<Compile Include="Extensions\RouteExtensions.cs" />
<Compile Include="Extensions\ScopeExtensions.cs" />
<Compile Include="Extensions\UserExtensions.cs" />
<Compile Include="Filters\ApiScopeRequiredAttribute.cs" />
42 changes: 21 additions & 21 deletions src/NuGetGallery/Telemetry/ClientTelemetryPIIProcessor.cs
Original file line number Diff line number Diff line change
@@ -2,8 +2,8 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;
using System.Web.Routing;
using Microsoft.ApplicationInsights.Channel;
using Microsoft.ApplicationInsights.DataContracts;
using Microsoft.ApplicationInsights.Extensibility;
@@ -30,29 +30,29 @@ private void ModifyItem(ITelemetry item)
var requestTelemetryItem = item as RequestTelemetry;
if(requestTelemetryItem != null)
{
requestTelemetryItem.Url = ObfuscateUri(requestTelemetryItem);
var route = GetCurrentRoute();
if(route == null)
{
return;
}
// Removes the first /
var requestPath = requestTelemetryItem.Url.AbsolutePath.TrimStart('/');
var obfuscatedPath = route.ObfuscateUrlPath(requestPath);
if(obfuscatedPath != null)
{
requestTelemetryItem.Url = new Uri(requestTelemetryItem.Url.ToString().Replace(requestPath, obfuscatedPath));
requestTelemetryItem.Name = requestTelemetryItem.Name.Replace(requestPath, obfuscatedPath);
if(requestTelemetryItem.Context.Operation?.Name != null)
{
requestTelemetryItem.Context.Operation.Name = requestTelemetryItem.Context.Operation.Name.Replace(requestPath, obfuscatedPath);
}
}
}
}

private Uri ObfuscateUri(RequestTelemetry telemetryItem)
public virtual Route GetCurrentRoute()
{
if(IsPIIOperation(telemetryItem.Context.Operation.Name))
{
// The new url form will be: https://nuget.org/ObfuscatedUserName
return new Uri(Obfuscator.DefaultObfuscatedUrl(telemetryItem.Url));
}
return telemetryItem.Url;
}

protected virtual bool IsPIIOperation(string operationName)
{
if(string.IsNullOrEmpty(operationName))
{
return false;
}
// Remove the verb from the operation name.
// An example of operationName : GET Users/Profiles
return Obfuscator.ObfuscatedActions.Contains(operationName.Split(' ').Last());
return RouteTable.Routes.GetRouteData(new HttpContextWrapper(HttpContext.Current)).Route as Route;
}
}
}
58 changes: 58 additions & 0 deletions tests/NuGetGallery.Facts/Extensions/RouteExtensionsFacts.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Web.Routing;
using Xunit;

namespace NuGetGallery.Extensions
{
public class RouteExtensionsFacts
{
private static string _routeUrl = "test/{user}";
private static string _url = "test/user1";
private static int _segment = 1;
private static string _obfuscatedValue = "obfuscatedData";

[Fact]
public void MapRouteAddObfuscation()
{
// Arrange
var routes = new RouteCollection();
routes.MapRoute("test", _routeUrl, null, new RouteExtensions.ObfuscatedMetadata(_segment, _obfuscatedValue));

// Act + Assert
Assert.True(RouteExtensions.ObfuscatedRouteMap.ContainsKey(_routeUrl));
Assert.Equal(_segment, RouteExtensions.ObfuscatedRouteMap[_routeUrl].ObfuscatedSegment);
Assert.Equal(_obfuscatedValue, RouteExtensions.ObfuscatedRouteMap[_routeUrl].ObfuscateValue);
}

[Fact]
public void ObfuscateRoutePath_ReturnsNullWhenNotObfuscated()
{
//Arrange
var urlInput = "newtest/{user}";
var route = new Route(url: urlInput, routeHandler:null);

// Act
var obfuscated = route.ObfuscateUrlPath("newtest/user1");

//Assert
Assert.Null(obfuscated);
}

[Fact]
public void ObfuscateRoutePath_CorrectObfuscation()
{
//Arrange
var routes = new RouteCollection();
routes.MapRoute("test", _routeUrl, null, new RouteExtensions.ObfuscatedMetadata(_segment, _obfuscatedValue));
var route = new Route(url: _routeUrl, routeHandler: null);

// Act
var obfuscated = route.ObfuscateUrlPath(_url);

//Assert
Assert.Equal($"test/{_obfuscatedValue}", obfuscated);
}
}
}
1 change: 1 addition & 0 deletions tests/NuGetGallery.Facts/NuGetGallery.Facts.csproj
Original file line number Diff line number Diff line change
@@ -401,6 +401,7 @@
<Compile Include="Authentication\AuthenticationServiceFacts.cs" />
<Compile Include="Authentication\TestCredentialHelper.cs" />
<Compile Include="Controllers\SupportControllerFacts.cs" />
<Compile Include="Extensions\RouteExtensionsFacts.cs" />
<Compile Include="Extensions\OrganizationExtensionsFacts.cs" />
<Compile Include="Framework\MemberDataHelper.cs" />
<Compile Include="Infrastructure\Authentication\ApiKeyV4Facts.cs" />
132 changes: 70 additions & 62 deletions tests/NuGetGallery.Facts/Telemetry/ClientTelemetryPIIProcessorTests.cs
Original file line number Diff line number Diff line change
@@ -2,91 +2,87 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.IO;
using System.Security.Principal;
using System.Web;
using System.Web.Routing;
using Microsoft.ApplicationInsights.Channel;
using Microsoft.ApplicationInsights.DataContracts;
using Microsoft.ApplicationInsights.Extensibility;
using Microsoft.Owin;
using Microsoft.Owin.Security;
using Moq;
using Xunit;

namespace NuGetGallery.Telemetry
{
public class ClientTelemetryPIIProcessorTests
public class ClientTelemetryPIIProcessorTests
{
private RouteCollection _currentRoutes ;

public ClientTelemetryPIIProcessorTests()
{
if (_currentRoutes == null)
{
_currentRoutes = new RouteCollection();
Routes.RegisterApiV2Routes(_currentRoutes);
Routes.RegisterUIRoutes(_currentRoutes);
}
}

[Fact]
public void NullTelemetryItemDoesNotThorw()
{
// Arange
string userName = "user1";
var piiProcessor = CreatePIIProcessor(false, userName);
var piiProcessor = CreatePIIProcessor();

// Act
piiProcessor.Process(null);
}

[Theory]
[InlineData(false)]
[InlineData(true)]
public void UrlIsUpdatedOnPIIAction(bool actionIsPII)
[MemberData(nameof(PIIUrlDataGenerator))]
public void UrlIsUpdatedOnPIIAction(string routePath, string inputUrl, string expectedOutputUrl)
{
// Arange
string userName = "user1";
var piiProcessor = CreatePIIProcessor(actionIsPII, userName);
var piiProcessor = CreatePIIProcessor(routePath);
RequestTelemetry telemetryItem = new RequestTelemetry();
telemetryItem.Url = new Uri("https://localhost/user1");
telemetryItem.Url = new Uri(inputUrl);
telemetryItem.Name = $"GET {telemetryItem.Url.AbsolutePath}";

// Act
piiProcessor.Process(telemetryItem);

// Assert
string expected = actionIsPII ? $"https://localhost/{Obfuscator.DefaultTelemetryUserName}" : telemetryItem.Url.ToString();
Assert.Equal(expected, telemetryItem.Url.ToString());
Assert.Equal(expectedOutputUrl, telemetryItem.Url.ToString());
Assert.Equal($"GET {(new Uri(expectedOutputUrl)).AbsolutePath}", $"GET {telemetryItem.Url.AbsolutePath}");
}

[Theory]
[MemberData(nameof(PIIOperationDataGenerator))]
public void TestValidPIIOperations(string operation)
{
// Arange
var piiProcessor = (TestClientTelemetryPIIProcessor)CreatePIIProcessor(false, "user");

// Act and Assert
Assert.True(piiProcessor.IsPIIOperationBase(operation));
}

[Theory]
[MemberData(nameof(InvalidPIIOPerationDataGenerator))]
public void TestInvalidPIIOperations(string operation)
[Fact]
public void ValidatePIIActions()
{
// Arange
var piiProcessor = (TestClientTelemetryPIIProcessor)CreatePIIProcessor(false, "user");
HashSet<string> existentPIIOperations = Obfuscator.ObfuscatedActions;
List<string> piiOperationsFromRoutes = GetPIIOperationsFromRoute();

// Act and Assert
Assert.False(piiProcessor.IsPIIOperationBase(operation));
Assert.True(existentPIIOperations.SetEquals(piiOperationsFromRoutes));
}

[Fact]
public void ValidatePIIRoutes()
{
// Arange
HashSet<string> existentPIIOperations = Obfuscator.ObfuscatedActions;
List<string> piiOperationsFromRoutes = GetPIIOperationsFromRoute();
List<string> piiUrlRoutes = GetPIIRoutesUrls();

// Act and Assert
Assert.True(existentPIIOperations.SetEquals(piiOperationsFromRoutes));
foreach (var route in piiUrlRoutes)
{
var expectedTrue = RouteExtensions.ObfuscatedRouteMap.ContainsKey(route);
Assert.True(expectedTrue, $"Route was not added to the obfuscated routeMap.");
}
}

private ClientTelemetryPIIProcessor CreatePIIProcessor(bool isPIIOperation, string userName)
private ClientTelemetryPIIProcessor CreatePIIProcessor(string url = "")
{
return new TestClientTelemetryPIIProcessor(new TestProcessorNext(), isPIIOperation, userName);
return new TestClientTelemetryPIIProcessor(new TestProcessorNext(), url );
}

private class TestProcessorNext : ITelemetryProcessor
@@ -98,59 +94,71 @@ public void Process(ITelemetry item)

private class TestClientTelemetryPIIProcessor : ClientTelemetryPIIProcessor
{
private User _testUser;
private bool _isPIIOperation;
private string _url = string.Empty;

public TestClientTelemetryPIIProcessor(ITelemetryProcessor next, bool isPIIOperation, string userName) : base(next)
public TestClientTelemetryPIIProcessor(ITelemetryProcessor next, string url) : base (next)
{
_isPIIOperation = isPIIOperation;
_testUser = new User(userName);
_url = url;
}

protected override bool IsPIIOperation(string operationName)
public override Route GetCurrentRoute()
{
return _isPIIOperation;
var handler = new Mock<IRouteHandler>();
return new Route(_url, handler.Object);
}

public bool IsPIIOperationBase(string operationName)
{
return base.IsPIIOperation(operationName);
}
}

private static List<string> GetPIIOperationsFromRoute()
private List<string> GetPIIOperationsFromRoute()
{
var currentRoutes = new RouteCollection();
NuGetGallery.Routes.RegisterApiV2Routes(currentRoutes);
NuGetGallery.Routes.RegisterUIRoutes(currentRoutes);

var piiRoutes = currentRoutes.Where((r) =>
var piiRoutes = _currentRoutes.Where((r) =>
{
Route webRoute = r as Route;
return webRoute != null ? IsPIIUrl(webRoute.Url.ToString()) : false;
}).Select((r)=> {
}).Select((r) => {
var dd = ((Route)r).Defaults;
return $"{dd["controller"]}/{dd["action"]}";
}).Distinct().ToList();

return piiRoutes;
}

private List<string> GetPIIRoutesUrls()
{
var piiRoutes = _currentRoutes.Where((r) =>
{
Route webRoute = r as Route;
return webRoute != null ? IsPIIUrl(webRoute.Url.ToString()) : false;
}).Select((r) => ((Route)r).Url).Distinct().ToList();

return piiRoutes;
}

private static bool IsPIIUrl(string url)
{
return url.ToLower().Contains("username") || url.ToLower().Contains("accountname");
}

public static IEnumerable<string[]> PIIOperationDataGenerator()
public static IEnumerable<string[]> PIIUrlDataGenerator()
{
return GetPIIOperationsFromRoute().Select(o => new string[]{$"GET {o}"});
foreach (var user in GenerateUserNames())
{
yield return new string[] { "packages/{id}/owners/{username}/confirm/{token}", $"https://localhost/packages/pack1/owners/user1/confirm/sometoken", $"https://localhost/packages/pack1/owners/ObfuscatedUserName/confirm/sometoken" };
yield return new string[] { "packages/{id}/owners/{username}/reject/{token}", $"https://localhost/packages/pack1/owners/user1/reject/sometoken", $"https://localhost/packages/pack1/owners/ObfuscatedUserName/reject/sometoken" };
yield return new string[] { "packages/{id}/owners/{username}/cancel/{token}", $"https://localhost/packages/pack1/owners/user1/cancel/sometoken", $"https://localhost/packages/pack1/owners/ObfuscatedUserName/cancel/sometoken" };

yield return new string[] { "account/confirm/{username}/{token}", $"https://localhost/account/confirm/user1/sometoken", $"https://localhost/account/confirm/ObfuscatedUserName/sometoken" };
yield return new string[] { "account/delete/{accountName}", "https://localhost/account/delete/user1", $"https://localhost/account/delete/ObfuscatedUserName" };

yield return new string[] { "profiles/{username}", $"https://localhost/profiles/user1", $"https://localhost/profiles/ObfuscatedUserName" };
yield return new string[] { "account/setpassword/{username}/{token}", $"https://localhost/account/setpassword/user1/sometoken", $"https://localhost/account/setpassword/ObfuscatedUserName/sometoken" };
yield return new string[] { "account/forgotpassword/{username}/{token}", $"https://localhost/account/forgotpassword/user1/sometoken", $"https://localhost/account/forgotpassword/ObfuscatedUserName/sometoken" };
}
}

public static IEnumerable<string[]> InvalidPIIOPerationDataGenerator()
public static List<string> GenerateUserNames()
{
yield return new string[]{ null };
yield return new string[]{ string.Empty };
yield return new string[]{"Some random data" };
return new List<string>{ "user1", "user.1", "user_1", "user-1"};
}
}
}

0 comments on commit 18bc2a1

Please sign in to comment.