Skip to content

Commit

Permalink
Revert "Add ownership requests to manage packages page (#4722 from Nu…
Browse files Browse the repository at this point in the history 
…Get/sb-ownerrequests2"

This reverts commit 11c0476.
  • Loading branch information
Scott Bommarito committed Sep 25, 2017
1 parent 11c0476 commit f0d0066
Show file tree
Hide file tree
Showing 36 changed files with 373 additions and 1,483 deletions.
5 changes: 2 additions & 3 deletions src/NuGetGallery.Core/Entities/PackageOwnerRequest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@ public class PackageOwnerRequest
: IEntity
{
public int PackageRegistrationKey { get; set; }
public virtual PackageRegistration PackageRegistration { get; set; }
public int NewOwnerKey { get; set; }
public virtual User NewOwner { get; set; }
public virtual User RequestingOwner { get; set; }
public User NewOwner { get; set; }
public User RequestingOwner { get; set; }
public int RequestingOwnerKey { get; set; }
public string ConfirmationCode { get; set; }
public DateTime RequestDate { get; set; }
Expand Down
5 changes: 0 additions & 5 deletions src/NuGetGallery/App_Start/DefaultDependenciesModule.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,11 +175,6 @@ protected override void Load(ContainerBuilder builder)
.AsSelf()
.InstancePerLifetimeScope();

builder.RegisterType<PackageOwnerRequestService>()
.AsSelf()
.As<IPackageOwnerRequestService>()
.InstancePerLifetimeScope();

builder.RegisterType<FormsAuthenticationService>()
.As<IFormsAuthenticationService>()
.InstancePerLifetimeScope();
Expand Down
12 changes: 1 addition & 11 deletions src/NuGetGallery/App_Start/Routes.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,17 +147,7 @@ public static void RegisterUIRoutes(RouteCollection routes)
routes.MapRoute(
RouteName.PackageOwnerConfirmation,
"packages/{id}/owners/{username}/confirm/{token}",
new { controller = "Packages", action = "ConfirmPendingOwnershipRequest" });

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

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

// We need the following two routes (rather than just one) due to Routing's
// Consecutive Optional Parameter bug. :(
Expand Down
4 changes: 3 additions & 1 deletion src/NuGetGallery/Controllers/AuthenticationController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,9 @@ public virtual async Task<ActionResult> Register(LogOnViewModel model, string re
{
_messageService.SendNewAccountEmail(
new MailAddress(user.User.UnconfirmedEmailAddress, user.User.Username),
Url.ConfirmEmail(
Url.ConfirmationUrl(
"Confirm",
"Users",
user.User.Username,
user.User.EmailConfirmationToken,
relativeUrl: false));
Expand Down
57 changes: 21 additions & 36 deletions src/NuGetGallery/Controllers/JsonApiController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public partial class JsonApiController
: AppController
{
private readonly IMessageService _messageService;
private readonly IPackageOwnerRequestService _packageOwnerRequestService;
private readonly IEntityRepository<PackageOwnerRequest> _packageOwnerRequestRepository;
private readonly IPackageService _packageService;
private readonly IUserService _userService;
private readonly IAppConfiguration _appConfiguration;
Expand All @@ -28,14 +28,14 @@ public partial class JsonApiController
public JsonApiController(
IPackageService packageService,
IUserService userService,
IPackageOwnerRequestService packageOwnerRequestService,
IEntityRepository<PackageOwnerRequest> packageOwnerRequestRepository,
IMessageService messageService,
IAppConfiguration appConfiguration,
ISecurityPolicyService policyService)
{
_packageService = packageService;
_userService = userService;
_packageOwnerRequestService = packageOwnerRequestService;
_packageOwnerRequestRepository = packageOwnerRequestRepository;
_messageService = messageService;
_appConfiguration = appConfiguration;
_policyService = policyService;
Expand Down Expand Up @@ -64,15 +64,15 @@ public virtual ActionResult GetPackageOwners(string id, string version)
Pending = false
};

var pending =
_packageOwnerRequestService.GetPackageOwnershipRequests(package: package.PackageRegistration)
.Select(r => new
{
Name = r.NewOwner.Username,
EmailAddress = r.NewOwner.EmailAddress,
Current = false,
Pending = true
});
var pending = from u in _packageOwnerRequestRepository.GetAll()
where u.PackageRegistrationKey == package.PackageRegistration.Key
select new
{
Name = u.NewOwner.Username,
EmailAddress = u.NewOwner.EmailAddress,
Current = false,
Pending = true
};

var result = owners.Union(pending).Select(o => new
{
Expand Down Expand Up @@ -113,27 +113,20 @@ public async Task<JsonResult> AddPackageOwner(string id, string username, string
if (TryGetManagePackageOwnerModel(id, username, out model))
{
var encodedMessage = HttpUtility.HtmlEncode(message);

var ownerRequest = await _packageOwnerRequestService.AddPackageOwnershipRequest(
var ownerRequest = await _packageService.CreatePackageOwnerRequestAsync(
model.Package, model.CurrentUser, model.User);

var confirmationUrl = Url.ConfirmPendingOwnershipRequest(
model.Package.Id,
model.User.Username,
ownerRequest.ConfirmationCode,
relativeUrl: false);

var rejectionUrl = Url.RejectPendingOwnershipRequest(
model.Package.Id,
var confirmationUrl = Url.ConfirmationUrl(
"ConfirmOwner",
"Packages",
model.User.Username,
ownerRequest.ConfirmationCode,
new { id = model.Package.Id },
relativeUrl: false);

var packageUrl = Url.Package(model.Package.Id, version: null, relativeUrl: false);
var policyMessage = GetNoticeOfPoliciesRequiredMessage(model.Package, model.User, model.CurrentUser);

_messageService.SendPackageOwnerRequest(model.CurrentUser, model.User, model.Package, packageUrl,
confirmationUrl, rejectionUrl, encodedMessage, policyMessage);
confirmationUrl, encodedMessage, policyMessage);

return Json(new
{
Expand All @@ -157,18 +150,9 @@ public async Task<JsonResult> RemovePackageOwner(string id, string username)
ManagePackageOwnerModel model;
if (TryGetManagePackageOwnerModel(id, username, out model))
{
var request = _packageOwnerRequestService.GetPackageOwnershipRequests(package: model.Package, newOwner: model.User).FirstOrDefault();

await _packageService.RemovePackageOwnerAsync(model.Package, model.User);

if (request == null)
{
_messageService.SendPackageOwnerRemovedNotice(model.CurrentUser, model.User, model.Package);
}
else
{
_messageService.SendPackageOwnerRequestCancellationNotice(model.CurrentUser, model.User, model.Package);
}
_messageService.SendPackageOwnerRemovedNotice(model.CurrentUser, model.User, model.Package);

return Json(new { success = true });
}
Expand Down Expand Up @@ -257,7 +241,8 @@ private IEnumerable<string> GetPropagatingOwners(PackageRegistration package)

private IEnumerable<string> GetPendingPropagatingOwners(PackageRegistration package)
{
return _packageOwnerRequestService.GetPackageOwnershipRequests(package: package)
return _packageOwnerRequestRepository.GetAll()
.Where(po => po.PackageRegistrationKey == package.Key)
.Select(po => po.NewOwner)
.Where(RequireSecurePushForCoOwnersPolicy.IsSubscribed)
.Select(po => po.Username);
Expand Down
93 changes: 11 additions & 82 deletions src/NuGetGallery/Controllers/PackagesController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,9 @@ public partial class PackagesController
private readonly IAppConfiguration _config;
private readonly IMessageService _messageService;
private readonly IPackageService _packageService;
private readonly IPackageOwnerRequestService _packageOwnerRequestService;
private readonly IPackageFileService _packageFileService;
private readonly ISearchService _searchService;
private readonly IUploadFileService _uploadFileService;
private readonly IUserService _userService;
private readonly IEntitiesContext _entitiesContext;
private readonly IIndexingService _indexingService;
private readonly ICacheService _cacheService;
Expand All @@ -64,9 +62,7 @@ public partial class PackagesController

public PackagesController(
IPackageService packageService,
IPackageOwnerRequestService packageOwnerRequestService,
IUploadFileService uploadFileService,
IUserService userService,
IMessageService messageService,
ISearchService searchService,
IAutomaticallyCuratePackageCommand autoCuratedPackageCmd,
Expand All @@ -86,9 +82,7 @@ public PackagesController(
IReadMeService readMeService)
{
_packageService = packageService;
_packageOwnerRequestService = packageOwnerRequestService;
_uploadFileService = uploadFileService;
_userService = userService;
_messageService = messageService;
_searchService = searchService;
_autoCuratedPackageCmd = autoCuratedPackageCmd;
Expand Down Expand Up @@ -1120,32 +1114,18 @@ public virtual async Task<JsonResult> Edit(string id, string version, VerifyPack
});
}

[HttpGet]
[Authorize]
[RequiresAccountConfirmation("accept ownership of a package")]
public virtual Task<ActionResult> ConfirmPendingOwnershipRequest(string id, string username, string token)
{
return HandleOwnershipRequest(id, username, token, accept: true);
}

[HttpGet]
[Authorize]
[RequiresAccountConfirmation("reject ownership of a package")]
public virtual Task<ActionResult> RejectPendingOwnershipRequest(string id, string username, string token)
public virtual async Task<ActionResult> ConfirmOwner(string id, string username, string token)
{
return HandleOwnershipRequest(id, username, token, accept: false);
}

private async Task<ActionResult> HandleOwnershipRequest(string id, string username, string token, bool accept)
{
if (string.IsNullOrEmpty(token))
if (String.IsNullOrEmpty(token))
{
return HttpNotFound();
}

if (!string.Equals(username, User.Identity.Name, StringComparison.OrdinalIgnoreCase))
if (!String.Equals(username, User.Identity.Name, StringComparison.OrdinalIgnoreCase))
{
return View("ConfirmOwner", new PackageOwnerConfirmationModel(id, username, ConfirmOwnershipResult.NotYourRequest));
return View(new PackageOwnerConfirmationModel(id, username, ConfirmOwnershipResult.NotYourRequest));
}

var package = _packageService.FindPackageRegistrationById(id);
Expand All @@ -1157,72 +1137,21 @@ private async Task<ActionResult> HandleOwnershipRequest(string id, string userna
var user = GetCurrentUser();
if (package.IsOwner(user))
{
return View("ConfirmOwner", new PackageOwnerConfirmationModel(id, username, ConfirmOwnershipResult.AlreadyOwner));
}

var request = _packageOwnerRequestService.GetPackageOwnershipRequest(package, user, token);
if (request == null)
{
return View("ConfirmOwner", new PackageOwnerConfirmationModel(id, username, ConfirmOwnershipResult.Failure));
}

if (accept)
{
var result = await HandleSecurePushPropagation(package, user);

await _packageService.AddPackageOwnerAsync(package, user);

SendAddPackageOwnerNotification(package, user, result.Item1, result.Item2);

return View("ConfirmOwner", new PackageOwnerConfirmationModel(id, username, ConfirmOwnershipResult.Success));
}
else
{
var requestingUser = request.RequestingOwner;

await _packageService.RemovePackageOwnerAsync(package, user);

_messageService.SendPackageOwnerRequestRejectionNotice(requestingUser, user, package);

return View("ConfirmOwner", new PackageOwnerConfirmationModel(id, username, ConfirmOwnershipResult.Rejected));
return View(new PackageOwnerConfirmationModel(id, username, ConfirmOwnershipResult.AlreadyOwner));
}
}

[HttpGet]
[Authorize]
[RequiresAccountConfirmation("cancel pending ownership request")]
public virtual async Task<ActionResult> CancelPendingOwnershipRequest(string id, string requestingUsername, string pendingUsername)
{
if (!string.Equals(requestingUsername, User.Identity.Name, StringComparison.OrdinalIgnoreCase))
if (!_packageService.IsValidPackageOwnerRequest(package, user, token))
{
return View("ConfirmOwner", new PackageOwnerConfirmationModel(id, requestingUsername, ConfirmOwnershipResult.NotYourRequest));
return View(new PackageOwnerConfirmationModel(id, username, ConfirmOwnershipResult.Failure));
}

var package = _packageService.FindPackageRegistrationById(id);
if (package == null)
{
return HttpNotFound();
}

var requestingUser = GetCurrentUser();

var pendingUser = _userService.FindByUsername(pendingUsername);
if (pendingUser == null)
{
return HttpNotFound();
}

var request = _packageOwnerRequestService.GetPackageOwnershipRequests(package, requestingUser, pendingUser).FirstOrDefault();
if (request == null)
{
return HttpNotFound();
}
var result = await HandleSecurePushPropagation(package, user);

await _packageOwnerRequestService.DeletePackageOwnershipRequest(request);
await _packageService.AddPackageOwnerAsync(package, user);

_messageService.SendPackageOwnerRequestCancellationNotice(requestingUser, pendingUser, package);
SendAddPackageOwnerNotification(package, user, result.Item1, result.Item2);

return View("ConfirmOwner", new PackageOwnerConfirmationModel(id, pendingUsername, ConfirmOwnershipResult.Cancelled));
return View(new PackageOwnerConfirmationModel(id, username, ConfirmOwnershipResult.Success));
}

/// <summary>
Expand Down
23 changes: 9 additions & 14 deletions src/NuGetGallery/Controllers/UsersController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ public partial class UsersController
private readonly IUserService _userService;
private readonly IMessageService _messageService;
private readonly IPackageService _packageService;
private readonly IPackageOwnerRequestService _packageOwnerRequestService;
private readonly IAppConfiguration _config;
private readonly AuthenticationService _authService;
private readonly ICredentialBuilder _credentialBuilder;
Expand All @@ -30,7 +29,6 @@ public UsersController(
ICuratedFeedService feedsQuery,
IUserService userService,
IPackageService packageService,
IPackageOwnerRequestService packageOwnerRequestService,
IMessageService messageService,
IAppConfiguration config,
AuthenticationService authService,
Expand All @@ -39,7 +37,6 @@ public UsersController(
_curatedFeedService = feedsQuery ?? throw new ArgumentNullException(nameof(feedsQuery));
_userService = userService ?? throw new ArgumentNullException(nameof(userService));
_packageService = packageService ?? throw new ArgumentNullException(nameof(packageService));
_packageOwnerRequestService = packageOwnerRequestService ?? throw new ArgumentNullException(nameof(packageOwnerRequestService));
_messageService = messageService ?? throw new ArgumentNullException(nameof(messageService));
_config = config ?? throw new ArgumentNullException(nameof(config));
_authService = authService ?? throw new ArgumentNullException(nameof(authService));
Expand All @@ -61,7 +58,8 @@ public virtual ActionResult ConfirmationRequired()
public virtual ActionResult ConfirmationRequiredPost()
{
User user = GetCurrentUser();
var confirmationUrl = Url.ConfirmEmail(user.Username, user.EmailConfirmationToken, relativeUrl: false);
var confirmationUrl = Url.ConfirmationUrl(
"Confirm", "Users", user.Username, user.EmailConfirmationToken, relativeUrl: false);

var alreadyConfirmed = user.UnconfirmedEmailAddress == null;

Expand Down Expand Up @@ -157,15 +155,9 @@ public virtual ActionResult Packages()
var packages = _packageService.FindPackagesByOwner(user, includeUnlisted: true)
.Select(p => new ListPackageItemViewModel(p)).OrderBy(p => p.Id).ToList();

var incoming = _packageOwnerRequestService.GetPackageOwnershipRequests(newOwner: user);
var outgoing = _packageOwnerRequestService.GetPackageOwnershipRequests(requestingOwner: user);

var ownerRequests = new OwnerRequestsViewModel(incoming, outgoing, user, _packageService);

var model = new ManagePackagesViewModel
{
Packages = packages,
OwnerRequests = ownerRequests
Packages = packages
};
return View(model);
}
Expand Down Expand Up @@ -397,7 +389,8 @@ public virtual async Task<ActionResult> ChangeEmail(AccountViewModel model)

if (user.Confirmed)
{
var confirmationUrl = Url.ConfirmEmail(user.Username, user.EmailConfirmationToken, relativeUrl: false);
var confirmationUrl = Url.ConfirmationUrl(
"Confirm", "Users", user.Username, user.EmailConfirmationToken, relativeUrl: false);
_messageService.SendEmailChangeConfirmationNotice(new MailAddress(user.UnconfirmedEmailAddress, user.Username), confirmationUrl);

TempData["Message"] = Strings.EmailUpdated_ConfirmationRequired;
Expand Down Expand Up @@ -758,10 +751,12 @@ private static int CountLoginCredentials(User user)

private ActionResult SendPasswordResetEmail(User user, bool forgotPassword)
{
var resetPasswordUrl = Url.ResetEmailOrPassword(
var resetPasswordUrl = Url.ConfirmationUrl(
"ResetPassword",
"Users",
user.Username,
user.PasswordResetToken,
forgotPassword,
new { forgot = forgotPassword },
relativeUrl: false);
_messageService.SendPasswordResetInstructions(user, resetPasswordUrl, forgotPassword);

Expand Down
Loading

0 comments on commit f0d0066

Please sign in to comment.