From 62703f32d1f1f46607d0b971bcb1eb9fd5fa7a78 Mon Sep 17 00:00:00 2001 From: Christy Henriksson Date: Thu, 15 Feb 2018 11:45:50 -0800 Subject: [PATCH] Fix org confirmation link and other fixes (#5427) --- src/NuGetGallery/App_Code/ViewHelpers.cshtml | 44 +++++++++---- src/NuGetGallery/App_Start/Routes.cs | 2 +- .../Controllers/AccountsController.cs | 22 ++++--- .../Controllers/OrganizationsController.cs | 14 +++++ .../Controllers/UsersController.cs | 14 +++++ src/NuGetGallery/NuGetGallery.csproj | 2 +- .../gallery/page-manage-organization.js | 11 +++- .../Services/ActionsRequiringPermissions.cs | 7 +++ src/NuGetGallery/UrlExtensions.cs | 17 ++++- .../ViewModels/AccountViewModel.cs | 2 + .../_OrganizationAccountManageMembers.cshtml | 63 +++++++++++-------- .../Views/{Users => Shared}/Confirm.cshtml | 0 .../Views/Shared/_AccountChangeEmail.cshtml | 3 +- .../Shared/_AccountChangeNotifications.cshtml | 3 +- .../Shared/_AccountProfilePicture.cshtml | 3 +- .../Views/Users/Organizations.cshtml | 9 +-- src/NuGetGallery/Views/Users/Transform.cshtml | 2 +- .../Controllers/AccountsControllerFacts.cs | 4 +- .../OrganizationsControllerFacts.cs | 8 +-- .../ClientTelemetryPIIProcessorTests.cs | 2 +- 20 files changed, 163 insertions(+), 69 deletions(-) rename src/NuGetGallery/Views/{Users => Shared}/Confirm.cshtml (100%) diff --git a/src/NuGetGallery/App_Code/ViewHelpers.cshtml b/src/NuGetGallery/App_Code/ViewHelpers.cshtml index 9ab9e75de4..7831577342 100644 --- a/src/NuGetGallery/App_Code/ViewHelpers.cshtml +++ b/src/NuGetGallery/App_Code/ViewHelpers.cshtml @@ -495,7 +495,9 @@ var hlp = new AccordionHelper(name, formModelStatePrefix, expanded, page); Func content, bool expanded = true, string expandedIcon = "ChevronDown", - string collapsedIcon = "ChevronRight") + string collapsedIcon = "ChevronRight", + string disabledIcon = "Lock", + bool disabled = false) { @Section(viewPage, id, @@ -515,29 +517,45 @@ var hlp = new AccordionHelper(name, formModelStatePrefix, expanded, page); Func content, bool expanded = false, string expandedIcon = "ChevronDown", - string collapsedIcon = "ChevronRight") + string collapsedIcon = "ChevronRight", + string disabledIcon = "Lock", + bool disabled = false) { - AddSection(viewPage.ViewBag, id); + if (!disabled) + { + AddSection(viewPage.ViewBag, id); + }

- - + @if (!disabled) + { + + + @title(MvcHtmlString.Empty) + + } + else + { + @title(MvcHtmlString.Empty) - + }

@data(MvcHtmlString.Empty)
-
-
- @content(MvcHtmlString.Empty) + if (!disabled) + { +
+
+ @content(MvcHtmlString.Empty) +
-
+ } } \ No newline at end of file diff --git a/src/NuGetGallery/App_Start/Routes.cs b/src/NuGetGallery/App_Start/Routes.cs index cf63407109..2722ea5b20 100644 --- a/src/NuGetGallery/App_Start/Routes.cs +++ b/src/NuGetGallery/App_Start/Routes.cs @@ -272,7 +272,7 @@ public static void RegisterUIRoutes(RouteCollection routes) routes.MapRoute( RouteName.ConfirmAccount, - "account/confirm/{username}/{token}", + "account/confirm/{accountName}/{token}", new { controller = "Users", action = "Confirm" }, new RouteExtensions.ObfuscatedMetadata(2, Obfuscator.DefaultTelemetryUserName)); diff --git a/src/NuGetGallery/Controllers/AccountsController.cs b/src/NuGetGallery/Controllers/AccountsController.cs index b0f54a31f0..5f581ace99 100644 --- a/src/NuGetGallery/Controllers/AccountsController.cs +++ b/src/NuGetGallery/Controllers/AccountsController.cs @@ -84,15 +84,13 @@ public virtual ActionResult ConfirmationRequiredPost(string accountName = null) { return new HttpStatusCodeResult(HttpStatusCode.Forbidden, Strings.Unauthorized); } - - var confirmationUrl = Url.ConfirmEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false); - + var alreadyConfirmed = account.UnconfirmedEmailAddress == null; ConfirmationViewModel model; if (!alreadyConfirmed) { - MessageService.SendNewAccountEmail(new MailAddress(account.UnconfirmedEmailAddress, account.Username), confirmationUrl); + SendNewAccountEmail(account); model = new ConfirmationViewModel(account) { @@ -106,14 +104,16 @@ public virtual ActionResult ConfirmationRequiredPost(string accountName = null) return View(model); } + protected abstract void SendNewAccountEmail(User account); + [UIAuthorize(allowDiscontinuedLogins: true)] - public virtual async Task Confirm(string username, string token) + public virtual async Task Confirm(string accountName, string token) { // We don't want Login to go to this page as a return URL // By having this value present in the dictionary BUT null, we don't put "returnUrl" on the Login link at all ViewData[Constants.ReturnUrlViewDataKey] = null; - var account = GetAccount(username); + var account = GetAccount(accountName); if (account == null || ActionsRequiringPermissions.ManageAccount.CheckPermissions(GetCurrentUser(), account) @@ -236,8 +236,7 @@ public virtual async Task ChangeEmail(TAccountViewModel model) if (account.Confirmed) { - var confirmationUrl = Url.ConfirmEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false); - MessageService.SendEmailChangeConfirmationNotice(new MailAddress(account.UnconfirmedEmailAddress, account.Username), confirmationUrl); + SendEmailChangedConfirmationNotice(account); TempData["Message"] = Messages.EmailUpdatedWithConfirmationRequired; } @@ -249,6 +248,8 @@ public virtual async Task ChangeEmail(TAccountViewModel model) return RedirectToAction(AccountAction); } + protected abstract void SendEmailChangedConfirmationNotice(User account); + [HttpPost] [UIAuthorize] [ValidateAntiForgeryToken] @@ -283,7 +284,7 @@ protected virtual TUser GetAccount(string accountName) protected virtual ActionResult AccountView(TUser account, TAccountViewModel model = null) { if (account == null - || ActionsRequiringPermissions.ManageAccount.CheckPermissions(GetCurrentUser(), account) + || ActionsRequiringPermissions.ViewAccount.CheckPermissions(GetCurrentUser(), account) != PermissionsCheckResult.Allowed) { return new HttpStatusCodeResult(HttpStatusCode.Forbidden, Strings.Unauthorized); @@ -301,6 +302,9 @@ protected virtual void UpdateAccountViewModel(TUser account, TAccountViewModel m model.Account = account; model.AccountName = account.Username; + model.CanManage = ActionsRequiringPermissions.ManageAccount.CheckPermissions( + GetCurrentUser(), account) == PermissionsCheckResult.Allowed; + model.CuratedFeeds = CuratedFeedService .GetFeedsForManager(account.Key) .Select(f => f.Name) diff --git a/src/NuGetGallery/Controllers/OrganizationsController.cs b/src/NuGetGallery/Controllers/OrganizationsController.cs index 914ff58dd6..b0a7a418d3 100644 --- a/src/NuGetGallery/Controllers/OrganizationsController.cs +++ b/src/NuGetGallery/Controllers/OrganizationsController.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Net; +using System.Net.Mail; using System.Threading.Tasks; using System.Web.Mvc; using NuGetGallery.Authentication; @@ -33,6 +34,19 @@ public OrganizationsController( EmailUpdatedWithConfirmationRequired = Strings.OrganizationEmailUpdatedWithConfirmationRequired }; + protected override void SendNewAccountEmail(User account) + { + var confirmationUrl = Url.ConfirmOrganizationEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false); + + MessageService.SendNewAccountEmail(new MailAddress(account.UnconfirmedEmailAddress, account.Username), confirmationUrl); + } + + protected override void SendEmailChangedConfirmationNotice(User account) + { + var confirmationUrl = Url.ConfirmOrganizationEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false); + MessageService.SendEmailChangeConfirmationNotice(new MailAddress(account.UnconfirmedEmailAddress, account.Username), confirmationUrl); + } + [HttpGet] [UIAuthorize] public virtual ActionResult ManageOrganization(string accountName) diff --git a/src/NuGetGallery/Controllers/UsersController.cs b/src/NuGetGallery/Controllers/UsersController.cs index be05fcde87..09b9cb63a2 100644 --- a/src/NuGetGallery/Controllers/UsersController.cs +++ b/src/NuGetGallery/Controllers/UsersController.cs @@ -6,6 +6,7 @@ using System.Globalization; using System.Linq; using System.Net; +using System.Net.Mail; using System.Threading.Tasks; using System.Web.Mvc; using NuGetGallery.Areas.Admin; @@ -60,6 +61,19 @@ public UsersController( EmailUpdatedWithConfirmationRequired = Strings.UserEmailUpdatedWithConfirmationRequired }; + protected override void SendNewAccountEmail(User account) + { + var confirmationUrl = Url.ConfirmEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false); + + MessageService.SendNewAccountEmail(new MailAddress(account.UnconfirmedEmailAddress, account.Username), confirmationUrl); + } + + protected override void SendEmailChangedConfirmationNotice(User account) + { + var confirmationUrl = Url.ConfirmEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false); + MessageService.SendEmailChangeConfirmationNotice(new MailAddress(account.UnconfirmedEmailAddress, account.Username), confirmationUrl); + } + protected override User GetAccount(string accountName) { var currentUser = GetCurrentUser(); diff --git a/src/NuGetGallery/NuGetGallery.csproj b/src/NuGetGallery/NuGetGallery.csproj index 97c1e6f93d..1006065845 100644 --- a/src/NuGetGallery/NuGetGallery.csproj +++ b/src/NuGetGallery/NuGetGallery.csproj @@ -2175,7 +2175,7 @@ - + diff --git a/src/NuGetGallery/Scripts/gallery/page-manage-organization.js b/src/NuGetGallery/Scripts/gallery/page-manage-organization.js index 11a0af64bd..33274555b0 100644 --- a/src/NuGetGallery/Scripts/gallery/page-manage-organization.js +++ b/src/NuGetGallery/Scripts/gallery/page-manage-organization.js @@ -77,6 +77,9 @@ success: function () { parent.Error(null); parent.Members.remove(self); + if (self.IsCurrentUser) { + document.location.href = "/"; + } }, error: function (jqXHR, textStatus, errorThrown) { var error = "Unknown error when trying to delete member '" + self.Username + "'."; @@ -93,7 +96,7 @@ var data = { accountName: self.OrganizationViewModel.AccountName, memberName: self.Username, - isAdmin: self.IsAdmin() + isAdmin: self.IsAdmin(), }; addAntiForgeryToken(data); @@ -103,9 +106,12 @@ type: 'POST', dataType: 'json', data: data, - success: function () { + success: function (data) { parent.Error(null); parent.UpdateMemberCounts(); + if (self.IsCurrentUser) { + window.location.reload(); + } }, error: function (jqXHR, textStatus, errorThrown) { var error = "Unknown error when trying to update member '" + self.Username + "'."; @@ -113,6 +119,7 @@ error = jqXHR.responseJSON; } parent.Error(error); + self.IsAdmin(!self.IsAdmin()) } }); }; diff --git a/src/NuGetGallery/Services/ActionsRequiringPermissions.cs b/src/NuGetGallery/Services/ActionsRequiringPermissions.cs index e67fda9e2e..e36bf46b83 100644 --- a/src/NuGetGallery/Services/ActionsRequiringPermissions.cs +++ b/src/NuGetGallery/Services/ActionsRequiringPermissions.cs @@ -94,5 +94,12 @@ public static class ActionsRequiringPermissions public static ActionRequiringAccountPermissions ManageAccount = new ActionRequiringAccountPermissions( accountPermissionsRequirement: RequireOwnerOrOrganizationAdmin); + + /// + /// The action of viewing (read-only) a user or organization account. + /// + public static ActionRequiringAccountPermissions ViewAccount = + new ActionRequiringAccountPermissions( + accountPermissionsRequirement: RequireOwnerOrOrganizationMember); } } \ No newline at end of file diff --git a/src/NuGetGallery/UrlExtensions.cs b/src/NuGetGallery/UrlExtensions.cs index b1e3ae4ae3..879422e2a3 100644 --- a/src/NuGetGallery/UrlExtensions.cs +++ b/src/NuGetGallery/UrlExtensions.cs @@ -924,13 +924,28 @@ public static string ConfirmEmail( { var routeValues = new RouteValueDictionary { - ["username"] = username, + ["accountName"] = username, ["token"] = token }; return GetActionLink(url, "Confirm", "Users", relativeUrl, routeValues); } + public static string ConfirmOrganizationEmail( + this UrlHelper url, + string username, + string token, + bool relativeUrl = true) + { + var routeValues = new RouteValueDictionary + { + ["accountName"] = username, + ["token"] = token + }; + + return GetActionLink(url, "Confirm", "Organizations", relativeUrl, routeValues); + } + public static string ResetEmailOrPassword( this UrlHelper url, string username, diff --git a/src/NuGetGallery/ViewModels/AccountViewModel.cs b/src/NuGetGallery/ViewModels/AccountViewModel.cs index 682faddd84..d5d5ac31ce 100644 --- a/src/NuGetGallery/ViewModels/AccountViewModel.cs +++ b/src/NuGetGallery/ViewModels/AccountViewModel.cs @@ -19,6 +19,8 @@ public bool IsOrganization public string AccountName { get; set; } + public bool CanManage { get; set; } + public IList CuratedFeeds { get; set; } public ChangeEmailViewModel ChangeEmail { get; set; } diff --git a/src/NuGetGallery/Views/Organizations/_OrganizationAccountManageMembers.cshtml b/src/NuGetGallery/Views/Organizations/_OrganizationAccountManageMembers.cshtml index 912293f75a..93199de291 100644 --- a/src/NuGetGallery/Views/Organizations/_OrganizationAccountManageMembers.cshtml +++ b/src/NuGetGallery/Views/Organizations/_OrganizationAccountManageMembers.cshtml @@ -24,20 +24,23 @@