Skip to content

Commit

Permalink
Fix org confirmation link and other fixes (#5427)
Browse files Browse the repository at this point in the history
  • Loading branch information
chenriksson authored Feb 15, 2018
1 parent acd5396 commit 62703f3
Show file tree
Hide file tree
Showing 20 changed files with 163 additions and 69 deletions.
44 changes: 31 additions & 13 deletions src/NuGetGallery/App_Code/ViewHelpers.cshtml
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,9 @@ var hlp = new AccordionHelper(name, formModelStatePrefix, expanded, page);
Func<MvcHtmlString, HelperResult> content,
bool expanded = true,
string expandedIcon = "ChevronDown",
string collapsedIcon = "ChevronRight")
string collapsedIcon = "ChevronRight",
string disabledIcon = "Lock",
bool disabled = false)
{
@Section(viewPage,
id,
Expand All @@ -515,29 +517,45 @@ var hlp = new AccordionHelper(name, formModelStatePrefix, expanded, page);
Func<MvcHtmlString, HelperResult> 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);
}

<div class="clearfix">
<div class="form-section-title">
<h2>
<a href="#" role="button" data-toggle="collapse" data-target="#@id-container"
aria-expanded="@expanded" aria-controls="@id-container" id="show-@id-container">
<i class="ms-Icon ms-Icon--@(expanded ? expandedIcon : collapsedIcon)"
aria-hidden="@(expanded ? "false" : "true")"></i>
@if (!disabled)
{
<a href="#" role="button" data-toggle="collapse" data-target="#@id-container"
aria-expanded="@expanded" aria-controls="@id-container" id="show-@id-container">
<i class="ms-Icon ms-Icon--@(expanded ? expandedIcon : collapsedIcon)"
aria-hidden="@(expanded ? "false" : "true")"></i>
<span>@title(MvcHtmlString.Empty)</span>
</a>
}
else
{
<i class="ms-Icon ms-Icon--@disabledIcon" aria-hidden="true"></i>
<span>@title(MvcHtmlString.Empty)</span>
</a>
}
</h2>
</div>
<div class="form-section-data">
@data(MvcHtmlString.Empty)
</div>
</div>
<div class="panel panel-default panel-collapse collapse @(expanded ? "in" : string.Empty)"
aria-expanded="@(expanded ? "true" : "false")" id="@id-container">
<div class="panel-body">
@content(MvcHtmlString.Empty)
if (!disabled)
{
<div class="panel panel-default panel-collapse collapse @(expanded ? "in" : string.Empty)"
aria-expanded="@(expanded ? "true" : "false")" id="@id-container">
<div class="panel-body">
@content(MvcHtmlString.Empty)
</div>
</div>
</div>
}
}
2 changes: 1 addition & 1 deletion src/NuGetGallery/App_Start/Routes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down
22 changes: 13 additions & 9 deletions src/NuGetGallery/Controllers/AccountsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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<ActionResult> Confirm(string username, string token)
public virtual async Task<ActionResult> 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)
Expand Down Expand Up @@ -236,8 +236,7 @@ public virtual async Task<ActionResult> 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;
}
Expand All @@ -249,6 +248,8 @@ public virtual async Task<ActionResult> ChangeEmail(TAccountViewModel model)
return RedirectToAction(AccountAction);
}

protected abstract void SendEmailChangedConfirmationNotice(User account);

[HttpPost]
[UIAuthorize]
[ValidateAntiForgeryToken]
Expand Down Expand Up @@ -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);
Expand All @@ -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)
Expand Down
14 changes: 14 additions & 0 deletions src/NuGetGallery/Controllers/OrganizationsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
14 changes: 14 additions & 0 deletions src/NuGetGallery/Controllers/UsersController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/NuGetGallery/NuGetGallery.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -2175,7 +2175,7 @@
<Content Include="Views\web.config" />
<Content Include="Views\_ViewStart.cshtml" />
<Content Include="Views\Errors\InternalError.cshtml" />
<Content Include="Views\Users\Confirm.cshtml" />
<Content Include="Views\Shared\Confirm.cshtml" />
<Content Include="Views\Users\Thanks.cshtml" />
<Content Include="Views\Users\Profiles.cshtml" />
<Content Include="Views\Authentication\Register.cshtml" />
Expand Down
11 changes: 9 additions & 2 deletions src/NuGetGallery/Scripts/gallery/page-manage-organization.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 + "'.";
Expand All @@ -93,7 +96,7 @@
var data = {
accountName: self.OrganizationViewModel.AccountName,
memberName: self.Username,
isAdmin: self.IsAdmin()
isAdmin: self.IsAdmin(),
};
addAntiForgeryToken(data);

Expand All @@ -103,16 +106,20 @@
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 + "'.";
if (jqXHR.responseText) {
error = jqXHR.responseJSON;
}
parent.Error(error);
self.IsAdmin(!self.IsAdmin())
}
});
};
Expand Down
7 changes: 7 additions & 0 deletions src/NuGetGallery/Services/ActionsRequiringPermissions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,5 +94,12 @@ public static class ActionsRequiringPermissions
public static ActionRequiringAccountPermissions ManageAccount =
new ActionRequiringAccountPermissions(
accountPermissionsRequirement: RequireOwnerOrOrganizationAdmin);

/// <summary>
/// The action of viewing (read-only) a user or organization account.
/// </summary>
public static ActionRequiringAccountPermissions ViewAccount =
new ActionRequiringAccountPermissions(
accountPermissionsRequirement: RequireOwnerOrOrganizationMember);
}
}
17 changes: 16 additions & 1 deletion src/NuGetGallery/UrlExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions src/NuGetGallery/ViewModels/AccountViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ public bool IsOrganization

public string AccountName { get; set; }

public bool CanManage { get; set; }

public IList<string> CuratedFeeds { get; set; }

public ChangeEmailViewModel ChangeEmail { get; set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,23 @@
<script type="text/html" id="manage-members">
<div class="col-md-12">
<div class="panel-collapse collapse in" aria-expanded="true">
<div class="input-group col-md-12">
<div class="col-md-6">
<input class="form-control" placeholder="Add existing NuGet.org user" data-bind="textInput: NewMemberUsername, submit: AddMember" aria-label="Enter username to add member" />
</div>
<div class="col-md-6">
<select class="form-control col-md-2" data-bind="value: AddMemberRole, options: RoleNames" aria-label="Select new member role">
</select>
</div>
<span class="input-group-btn">
<button class="btn" type="submit" title="Add new member" name="Add new member" aria-label="Add" data-bind="click: AddMember">
<span class="ms-Icon ms-Icon--AddFriend" aria-hidden="true"></span>
</button>
</span>
</div><br />
@if (Model.CanManage)
{
<div class="input-group col-md-12">
<div class="col-md-6">
<input class="form-control" placeholder="Add existing NuGet.org user" data-bind="textInput: NewMemberUsername, submit: AddMember" aria-label="Enter username to add member" />
</div>
<div class="col-md-6">
<select class="form-control col-md-2" data-bind="value: AddMemberRole, options: RoleNames" aria-label="Select new member role">
</select>
</div>
<span class="input-group-btn">
<button class="btn" type="submit" title="Add new member" aria-label="Add new member" data-bind="click: AddMember">
<span class="ms-Icon ms-Icon--AddFriend" aria-hidden="true"></span>
</button>
</span>
</div><br />
}
<table class="table">
<tbody data-bind="foreach: Members">
<tr class="manage-members-listing">
Expand All @@ -55,19 +58,27 @@
<div data-bind="text: EmailAddress"></div>
</td>
<td class="align-middle member-username">
<select class="form-control" aria-label="Change member role"
data-bind="disable: IsCurrentUser, value: SelectedRole, options: OrganizationViewModel.RoleNames, event: { change: ToggleIsAdmin }">
</select>
</td>
<td class="text-right align-middle member-controls">
<!-- ko if: !IsCurrentUser -->
<span>
<a class="btn" data-bind="click: DeleteMember, attr: { 'aria-label': 'Delete Member' }">
<i class="ms-Icon ms-Icon--Cancel" aria-hidden="true"></i>
</a>
</span>
<!-- /ko -->
@if (Model.CanManage)
{
<select class="form-control" aria-label="Change member role"
data-bind="value: SelectedRole, options: OrganizationViewModel.RoleNames, event: { change: ToggleIsAdmin }">
</select>
}
else
{
<span data-bind="text: SelectedRole()"></span>
}
</td>
@if (Model.CanManage)
{
<td class="text-right align-middle member-controls">
<span>
<a class="btn" data-bind="click: DeleteMember, attr: { 'aria-label': 'Delete Member' }">
<i class="ms-Icon ms-Icon--Cancel" aria-hidden="true"></i>
</a>
</span>
</td>
}
</tr>
</tbody>
</table>
Expand Down
File renamed without changes.
3 changes: 2 additions & 1 deletion src/NuGetGallery/Views/Shared/_AccountChangeEmail.cshtml
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,5 @@
}
}
</text>,
ViewData.ModelState.Keys.Any(x => x.StartsWith("ChangeEmail")))
ViewData.ModelState.Keys.Any(x => x.StartsWith("ChangeEmail")),
disabled: !Model.CanManage)
Loading

0 comments on commit 62703f3

Please sign in to comment.