Skip to content

Commit 3d6fd21

Browse files
zhhyuzhhyu
authored
[Cookies] Expire and set the domain of Google Analytics cookies (#8263)
* Add custom GA * Set up the domain for Google Analytics cookies * More readable * Protect a port number * Add test coverage * Setting to none just works * Change to use Array
1 parent e56ad75 commit 3d6fd21

5 files changed

Lines changed: 27 additions & 83 deletions

File tree

‎src/NuGetGallery.Core/Cookies/CookieExpirationService.cs‎

Lines changed: 4 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -13,43 +13,29 @@ public class CookieExpirationService : ICookieExpirationService
1313
private static readonly DateTime CookieExpirationTime = new DateTime(2010, 1, 1);
1414

1515
// Google Analytics cookies
16-
private static readonly IReadOnlyList<string> GoogleAnalyticsCookies = new[]
16+
private static readonly string[] GoogleAnalyticsCookies = new[]
1717
{
1818
"_ga",
1919
"_gid",
2020
"_gat",
2121
};
2222

2323
// Application Insights cookies
24-
private static readonly IReadOnlyList<string> ApplicationInsightsCookies = new[]
24+
private static readonly string[] ApplicationInsightsCookies = new[]
2525
{
2626
"ai_user",
2727
"ai_session",
2828
};
2929

30-
private readonly string _domain;
31-
private readonly string _rootDomain;
32-
33-
public CookieExpirationService(string domain)
34-
{
35-
if (string.IsNullOrEmpty(domain))
36-
{
37-
throw new ArgumentException(CoreStrings.ArgumentCannotBeNullOrEmpty, nameof(domain));
38-
}
39-
40-
_domain = domain;
41-
_rootDomain = GetRootDomain(_domain);
42-
}
43-
4430
public void ExpireAnalyticsCookies(HttpContextBase httpContext)
4531
{
4632
if (httpContext == null)
4733
{
4834
throw new ArgumentNullException(nameof(httpContext));
4935
}
5036

51-
GoogleAnalyticsCookies.ToList().ForEach(cookieName => ExpireCookieByName(httpContext, cookieName, _rootDomain));
52-
ApplicationInsightsCookies.ToList().ForEach(cookieName => ExpireCookieByName(httpContext, cookieName));
37+
Array.ForEach(GoogleAnalyticsCookies, cookieName => ExpireCookieByName(httpContext, cookieName));
38+
Array.ForEach(ApplicationInsightsCookies, cookieName => ExpireCookieByName(httpContext, cookieName));
5339
}
5440

5541
public void ExpireSocialMediaCookies(HttpContextBase httpContext) { }
@@ -84,22 +70,5 @@ public void ExpireCookieByName(HttpContextBase httpContext, string cookieName, s
8470
}
8571
}
8672
}
87-
88-
private string GetRootDomain(string domain)
89-
{
90-
var index1 = domain.LastIndexOf('.');
91-
if (index1 < 0)
92-
{
93-
return domain;
94-
}
95-
96-
var index2 = domain.LastIndexOf('.', index1 - 1);
97-
if (index2 < 0)
98-
{
99-
return domain;
100-
}
101-
102-
return domain.Substring(index2 + 1);
103-
}
10473
}
10574
}

‎src/NuGetGallery/App_Code/ViewHelpers.cshtml‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
@using NuGetGallery
55
@using NuGetGallery.Helpers
66
@using NuGetGallery.Configuration
7-
@using NuGetGallery.Cookies
87
@using NuGet.Services.Entities
98
@using NuGet.Services.Licenses
109

@@ -334,7 +333,11 @@
334333
m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m)
335334
})(window,document,'script','https://www.google-analytics.com/analytics.js','ga');
336335

337-
ga('create', '@propertyId', 'auto');
336+
ga('create', '@propertyId', {
337+
'cookieDomain': 'none',
338+
'cookieExpires': 60 * 60 * 24 * 365,
339+
});
340+
338341
ga('set', 'anonymizeIp', true);
339342
ga('set', 'page', '@obfuscatedRequest');
340343
ga('set', 'title', '');

‎src/NuGetGallery/App_Start/DefaultDependenciesModule.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -446,10 +446,6 @@ protected override void Load(ContainerBuilder builder)
446446
.As<IPackageVulnerabilityService>()
447447
.InstancePerLifetimeScope();
448448

449-
builder.Register(c => new CookieExpirationService(domain: configuration.GetSiteRoot(useHttps: true)))
450-
.As<ICookieExpirationService>()
451-
.SingleInstance();
452-
453449
services.AddHttpClient();
454450
services.AddScoped<IGravatarProxyService, GravatarProxyService>();
455451

@@ -482,6 +478,10 @@ protected override void Load(ContainerBuilder builder)
482478

483479
RegisterCookieComplianceService(configuration, loggerFactory);
484480

481+
builder.RegisterType<CookieExpirationService>()
482+
.As<ICookieExpirationService>()
483+
.SingleInstance();
484+
485485
RegisterABTestServices(builder);
486486

487487
builder.RegisterType<MicrosoftTeamSubscription>()

‎tests/NuGetGallery.Core.Facts/Cookies/CookieExpirationServiceFacts.cs‎

Lines changed: 13 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -11,33 +11,13 @@ namespace NuGetGallery.Cookies
1111
{
1212
public class CookieExpirationServiceFacts
1313
{
14-
[Fact]
15-
public void CreateCookieExpirationService_ThrowsIfDomainNull()
16-
{
17-
// Arrange, Act & Assert
18-
var exception = Assert.Throws<ArgumentException>(() => new CookieExpirationService(domain: null));
19-
20-
Assert.Equal("domain", exception.ParamName);
21-
Assert.Contains("The argument cannot be null or empty", exception.Message);
22-
}
23-
24-
[Fact]
25-
public void CreateCookieExpirationService_ThrowsIfDomainEmpty()
26-
{
27-
// Arrange, Act & Assert
28-
var exception = Assert.Throws<ArgumentException>(() => new CookieExpirationService(domain: ""));
29-
30-
Assert.Equal("domain", exception.ParamName);
31-
Assert.Contains("The argument cannot be null or empty", exception.Message);
32-
}
33-
3414
public class TheExpireAnalyticsCookiesMethod
3515
{
3616
[Fact]
3717
public void ExpireAnalyticsCookies_ThrowsIfHttpContextNull()
3818
{
3919
// Arrange
40-
var cookieExpirationService = new CookieExpirationService("AnyDomain");
20+
var cookieExpirationService = new CookieExpirationService();
4121

4222
// Act & Assert
4323
var exception = Assert.Throws<ArgumentNullException>(() => cookieExpirationService.ExpireAnalyticsCookies(httpContext: null));
@@ -58,9 +38,8 @@ public void ExpireAnalyticsCookies()
5838
};
5939

6040
var httpContext = GetHttpContext(cookies);
61-
var domain = "subdomain.domain.test";
62-
var rootDomain = "domain.test";
63-
var cookieExpirationService = new CookieExpirationService(domain);
41+
42+
var cookieExpirationService = new CookieExpirationService();
6443

6544
// Act
6645
cookieExpirationService.ExpireAnalyticsCookies(httpContext);
@@ -72,16 +51,8 @@ public void ExpireAnalyticsCookies()
7251
Assert.NotNull(responseCookie);
7352
Assert.True(DateTime.Equals(new DateTime(2010, 1, 1), responseCookie.Expires));
7453
Assert.Equal(cookies[key], responseCookie.Value);
54+
Assert.Null(responseCookie.Domain);
7555
}
76-
77-
var _gaCookie = httpContext.Response.Cookies["_ga"];
78-
Assert.Equal(rootDomain, _gaCookie.Domain);
79-
var _gidCookie = httpContext.Response.Cookies["_gid"];
80-
Assert.Equal(rootDomain, _gidCookie.Domain);
81-
var _gatCookie = httpContext.Response.Cookies["_gat"];
82-
Assert.Equal(rootDomain, _gatCookie.Domain);
83-
Assert.Null(httpContext.Response.Cookies["ai_user"].Domain);
84-
Assert.Null(httpContext.Response.Cookies["ai_session"].Domain);
8556
}
8657
}
8758

@@ -91,7 +62,7 @@ public class TheExpireCookieByNameMethod
9162
public void ExpireCookieByName_ThrowsIfHttpContextNull()
9263
{
9364
// Arrange
94-
var cookieExpirationService = new CookieExpirationService("AnyDomain");
65+
var cookieExpirationService = new CookieExpirationService();
9566

9667
// Act & Assert
9768
var exception = Assert.Throws<ArgumentNullException>(() => cookieExpirationService.ExpireCookieByName(httpContext: null, cookieName: It.IsAny<string>()));
@@ -102,7 +73,7 @@ public void ExpireCookieByName_ThrowsIfHttpContextNull()
10273
public void ExpireCookieByName_ThrowsIfCookieNameNull()
10374
{
10475
// Arrange
105-
var cookieExpirationService = new CookieExpirationService("AnyDomain");
76+
var cookieExpirationService = new CookieExpirationService();
10677

10778
// Act & Assert
10879
var exception = Assert.Throws<ArgumentException>(() => cookieExpirationService.ExpireCookieByName(httpContext: Mock.Of<HttpContextBase>(), cookieName: null));
@@ -114,7 +85,7 @@ public void ExpireCookieByName_ThrowsIfCookieNameNull()
11485
public void ExpireCookieByName_ThrowsIfCookieNameEmpty()
11586
{
11687
// Arrange
117-
var cookieExpirationService = new CookieExpirationService("AnyDomain");
88+
var cookieExpirationService = new CookieExpirationService();
11889

11990
// Act & Assert
12091
var exception = Assert.Throws<ArgumentException>(() => cookieExpirationService.ExpireCookieByName(httpContext: Mock.Of<HttpContextBase>(), cookieName: ""));
@@ -127,7 +98,7 @@ public void ExpireCookieByName_ReturnsIfRequestNull()
12798
{
12899
// Arrange
129100
var httpContext = new Mock<HttpContextBase>();
130-
var cookieExpirationService = new CookieExpirationService("AnyDomain");
101+
var cookieExpirationService = new CookieExpirationService();
131102

132103
// Act & Assert
133104
cookieExpirationService.ExpireCookieByName(httpContext.Object, "AnyCookieName");
@@ -139,7 +110,7 @@ public void ExpireCookieByName_ReturnsIfResponseNull()
139110
// Arrange
140111
var httpContext = new Mock<HttpContextBase>();
141112
httpContext.Setup(c => c.Request).Returns(Mock.Of<HttpRequestBase>());
142-
var cookieExpirationService = new CookieExpirationService("AnyDomain");
113+
var cookieExpirationService = new CookieExpirationService();
143114

144115
// Act & Assert
145116
cookieExpirationService.ExpireCookieByName(httpContext.Object, "AnyCookieName");
@@ -152,7 +123,7 @@ public void ExpireCookieByName_ReturnsIfRequestCookiesNull()
152123
var httpContext = new Mock<HttpContextBase>();
153124
httpContext.Setup(c => c.Request).Returns(Mock.Of<HttpRequestBase>());
154125
httpContext.Setup(c => c.Response).Returns(Mock.Of<HttpResponseBase>());
155-
var cookieExpirationService = new CookieExpirationService("AnyDomain");
126+
var cookieExpirationService = new CookieExpirationService();
156127

157128
// Act & Assert
158129
cookieExpirationService.ExpireCookieByName(httpContext.Object, "AnyCookieName");
@@ -171,15 +142,15 @@ public void ExpireCookieByName_ReturnsIfResponseCookiesNull()
171142
httpContext.Setup(c => c.Request).Returns(httpRequest.Object);
172143
httpContext.Setup(c => c.Response).Returns(Mock.Of<HttpResponseBase>());
173144

174-
var cookieExpirationService = new CookieExpirationService("AnyDomain");
145+
var cookieExpirationService = new CookieExpirationService();
175146

176147
// Act & Assert
177148
cookieExpirationService.ExpireCookieByName(httpContext.Object, "AnyCookieName");
178149
}
179150

180151
[Theory]
181152
[InlineData(null)]
182-
[InlineData("AnyDomain")]
153+
[InlineData("anyspecificdomain")]
183154
public void ExpireCookieByName(string domain)
184155
{
185156
// Arrange
@@ -190,7 +161,7 @@ public void ExpireCookieByName(string domain)
190161
{ cookieName, cookieValue}
191162
};
192163
var httpContext = GetHttpContext(cookies);
193-
var cookieExpirationService = new CookieExpirationService("AnyDomain");
164+
var cookieExpirationService = new CookieExpirationService();
194165

195166
// Act
196167
cookieExpirationService.ExpireCookieByName(httpContext, cookieName, domain);

‎tests/NuGetGallery.Facts/App_Start/ConfigurationServiceFacts.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
34
using System;
45
using System.Configuration;
56
using System.Threading.Tasks;

0 commit comments

Comments
 (0)