Skip to content

Commit

Permalink
Display custom login errors (OrchardCMS#7734)
Browse files Browse the repository at this point in the history
  • Loading branch information
jardg authored and sebastienros committed Sep 14, 2017
1 parent 70adb18 commit 8036e53
Show file tree
Hide file tree
Showing 12 changed files with 98 additions and 70 deletions.
18 changes: 11 additions & 7 deletions src/Orchard.Tests.Modules/Users/Services/MembershipServiceTests.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.Web.Security;
using Autofac;
using Moq;
Expand All @@ -21,6 +22,7 @@
using Orchard.Environment;
using Orchard.Environment.Configuration;
using Orchard.Environment.Extensions;
using Orchard.Localization;
using Orchard.Security;
using Orchard.Services;
using Orchard.Settings;
Expand All @@ -32,7 +34,7 @@
using Orchard.Users.Models;
using Orchard.Users.Services;

namespace Orchard.Tests.Modules.Users.Services
namespace Orchard.Tests.Modules.Users.Services
{
[TestFixture]
public class MembershipServiceTests {
Expand Down Expand Up @@ -146,8 +148,9 @@ public void SaltAndPasswordShouldBeDifferentEvenWithSameSourcePassword() {
Assert.That(user1Record.PasswordSalt, Is.Not.EqualTo(user2Record.PasswordSalt));
Assert.That(user1Record.Password, Is.Not.EqualTo(user2Record.Password));

Assert.That(_membershipService.ValidateUser("a", "b"), Is.Not.Null);
Assert.That(_membershipService.ValidateUser("d", "b"), Is.Not.Null);
List<LocalizedString> validationErrors;
Assert.That(_membershipService.ValidateUser("a", "b", out validationErrors), Is.Not.Null);
Assert.That(_membershipService.ValidateUser("d", "b", out validationErrors), Is.Not.Null);
}

[Test]
Expand All @@ -156,9 +159,10 @@ public void ValidateUserShouldReturnNullIfUserOrPasswordIsIncorrect() {
_session.Flush();
_session.Clear();

var validate1 = _membershipService.ValidateUser("test-user", "bad-password");
var validate2 = _membershipService.ValidateUser("bad-user", "test-password");
var validate3 = _membershipService.ValidateUser("test-user", "test-password");
List<LocalizedString> validationErrors;
var validate1 = _membershipService.ValidateUser("test-user", "bad-password", out validationErrors);
var validate2 = _membershipService.ValidateUser("bad-user", "test-password", out validationErrors);
var validate3 = _membershipService.ValidateUser("test-user", "test-password", out validationErrors);

Assert.That(validate1, Is.Null);
Assert.That(validate2, Is.Null);
Expand All @@ -168,7 +172,7 @@ public void ValidateUserShouldReturnNullIfUserOrPasswordIsIncorrect() {
[Test]
public void UsersWhoHaveNeverLoggedInCanBeAuthenticated() {
var user = (UserPart)_membershipService.CreateUser(new CreateUserParams("a", "b", "c", null, null, true));

Assert.That(_membershipValidationService.CanAuthenticateWithCookie(user), Is.True);
}

Expand Down
21 changes: 11 additions & 10 deletions src/Orchard.Web/Modules/Orchard.Blogs/Services/XmlRpcHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
using Orchard.Blogs.Extensions;
using Orchard.Mvc.Html;
using Orchard.Core.Title.Models;
using System.Linq;

namespace Orchard.Blogs.Services {
[OrchardFeature("Orchard.Blogs.RemotePublishing")]
Expand All @@ -30,7 +31,7 @@ public class XmlRpcHandler : IXmlRpcHandler {
private readonly RouteCollection _routeCollection;

public XmlRpcHandler(IBlogService blogService, IBlogPostService blogPostService, IContentManager contentManager,
IAuthorizationService authorizationService, IMembershipService membershipService,
IAuthorizationService authorizationService, IMembershipService membershipService,
RouteCollection routeCollection) {
_blogService = blogService;
_blogPostService = blogPostService;
Expand Down Expand Up @@ -205,10 +206,10 @@ private int MetaWeblogNewPost(
if (blogPost.Is<TitlePart>()) {
blogPost.As<TitlePart>().Title = HttpUtility.HtmlDecode(title);
}

//AutoroutePart
dynamic dBlogPost = blogPost;
if (dBlogPost.AutoroutePart!=null){
if (dBlogPost.AutoroutePart!=null) {
dBlogPost.AutoroutePart.DisplayAlias = slug;
}

Expand Down Expand Up @@ -340,11 +341,11 @@ private bool MetaWeblogDeletePost(
}

private IUser ValidateUser(string userName, string password) {
IUser user = _membershipService.ValidateUser(userName, password);
if (user == null) {
throw new OrchardCoreException(T("The username or e-mail or password provided is incorrect."));
List<LocalizedString> validationErrors;
IUser user = _membershipService.ValidateUser(userName, password,out validationErrors);
if (validationErrors.Any()) {
throw new OrchardCoreException(validationErrors.FirstOrDefault());
}

return user;
}

Expand All @@ -361,13 +362,13 @@ private static XRpcStruct CreateBlogStruct(
var blogStruct = new XRpcStruct()
.Set("postid", blogPostPart.Id)
.Set("title", HttpUtility.HtmlEncode(blogPostPart.Title))

.Set("description", blogPostPart.Text)
.Set("link", url)
.Set("permaLink", url);

blogStruct.Set("wp_slug", blogPostPart.As<IAliasAspect>().Path);


if (blogPostPart.PublishedUtc != null) {
blogStruct.Set("dateCreated", blogPostPart.PublishedUtc);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Web.Mvc;
using System.Web.Routing;
Expand Down Expand Up @@ -55,7 +56,8 @@ private XRpcStruct MetaWeblogNewMediaObject(
XRpcStruct file,
UrlHelper url) {

var user = _membershipService.ValidateUser(userName, password);
List<LocalizedString> validationErrors;
var user = _membershipService.ValidateUser(userName, password, out validationErrors);
if (!_authorizationService.TryCheckAccess(Permissions.ManageMedia, user, null)) {
throw new OrchardCoreException(T("Access denied"));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Web.Mvc;
using System.Web.Routing;
Expand Down Expand Up @@ -59,7 +60,8 @@ private XRpcStruct MetaWeblogNewMediaObject(
XRpcStruct file,
UrlHelper url) {

var user = _membershipService.ValidateUser(userName, password);
List<LocalizedString> validationErrors;
var user = _membershipService.ValidateUser(userName, password, out validationErrors);
if (!_authorizationService.TryCheckAccess(Permissions.ManageOwnMedia, user, null)) {
throw new OrchardCoreException(T("Access denied"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,16 +162,17 @@ private IUser ValidateLogOn(string userNameOrEmail, string password) {
if (!validate)
return null;

var user = _membershipService.ValidateUser(userNameOrEmail, password);
List<LocalizedString> validationErrors;
var user = _membershipService.ValidateUser(userNameOrEmail, password, out validationErrors);
if (user == null) {
ModelState.AddModelError("password", T("The username or e-mail or password provided is incorrect."));
}

return user;
}

private string GetCallbackPath(WorkContext workContext)
{
private string GetCallbackPath(WorkContext workContext)
{
var shellSettings = workContext.Resolve<ShellSettings>();
var tenantPrefix = shellSettings.RequestUrlPrefix;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Xml.Linq;
using Orchard.ContentManagement;
using Orchard.Core.Contents;
Expand Down Expand Up @@ -147,9 +148,10 @@ private static int GetId(XRpcMethodResponse response) {
}

private IUser ValidateUser(string userName, string password) {
IUser user = _membershipService.ValidateUser(userName, password);
if (user == null) {
throw new OrchardCoreException(T("The username or e-mail or password provided is incorrect."));
List<LocalizedString> validationErrors;
IUser user = _membershipService.ValidateUser(userName, password, out validationErrors);
if (validationErrors.Any()) {
throw new OrchardCoreException(validationErrors.FirstOrDefault());
}

return user;
Expand Down
18 changes: 11 additions & 7 deletions src/Orchard.Web/Modules/Orchard.Tags/Services/XmlRpcHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using Orchard.ContentManagement;
using Orchard.Core.XmlRpc;
using Orchard.Core.XmlRpc.Models;
using Orchard.Localization;
using Orchard.Security;
using Orchard.Tags.Helpers;
using Orchard.Tags.Models;
Expand Down Expand Up @@ -88,7 +89,8 @@ private void MetaWeblogAttachTagsToPost(XRpcStruct postStruct, int postId, strin
if (postId < 1)
return;

var user = _membershipService.ValidateUser(userName, password);
List<LocalizedString> validationErrors;
var user = _membershipService.ValidateUser(userName, password, out validationErrors);
_authorizationService.CheckAccess(StandardPermissions.AccessAdminPanel, user, null);

var driver = new XmlRpcDriver(item => {
Expand Down Expand Up @@ -117,7 +119,8 @@ private static XRpcStruct GetPost(XRpcMethodResponse response) {
}

private XRpcArray MetaWeblogGetTags(string appKey, string userName, string password) {
var user = _membershipService.ValidateUser(userName, password);
List<LocalizedString> validationErrors;
var user = _membershipService.ValidateUser(userName, password, out validationErrors);
_authorizationService.CheckAccess(StandardPermissions.AccessAdminPanel, user, null);

var array = new XRpcArray();
Expand All @@ -127,17 +130,18 @@ private XRpcArray MetaWeblogGetTags(string appKey, string userName, string passw
.Set("tag_id", thisTag.TagName)
.Set("name", thisTag.TagName));
// nyi - not yet implemented
//.Set("count", "")
//.Set("slug", "")
//.Set("html_url", "")
//.Set("rss_url", ""));
//.Set("count", "")
//.Set("slug", "")
//.Set("html_url", "")
//.Set("rss_url", ""));
}

return array;
}

private void MetaWeblogUpdateTags(int contentItemId, string userName, string password, XRpcStruct content, bool publish, ICollection<IXmlRpcDriver> drivers) {
var user = _membershipService.ValidateUser(userName, password);
List<LocalizedString> validationErrors;
var user = _membershipService.ValidateUser(userName, password, out validationErrors);

var rawTags = content.Optional<string>("mt_keywords");
if (string.IsNullOrWhiteSpace(rawTags))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ public override IEnumerable<LocalizedString> Execute(WorkflowContext workflowCon
yield return T("IncorrectUserNameOrPassword");
yield break;
}

user = _membershipService.ValidateUser(userNameOrEmail, password);
List<LocalizedString> validationErrors;
user = _membershipService.ValidateUser(userNameOrEmail, password, out validationErrors);
}

if (user == null) {
Expand All @@ -71,7 +71,7 @@ public override IEnumerable<LocalizedString> Execute(WorkflowContext workflowCon

_userEventHandler.LoggingIn(userNameOrEmail, password);
_authenticationService.SignIn(user, createPersistentCookie);
_userEventHandler.LoggedIn(user);
_userEventHandler.LoggedIn(user);

yield return T("Done");
}
Expand All @@ -80,7 +80,7 @@ private bool IsTrueish(string value) {
if (String.IsNullOrWhiteSpace(value))
return false;

var falseValues = new[] {"false", "off", "no"};
var falseValues = new[] { "false", "off", "no" };
return falseValues.All(x => !String.Equals(x, value, StringComparison.OrdinalIgnoreCase));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using System.Web.Security;
using Orchard.Services;
using System.Collections.Generic;
using System.Linq;

namespace Orchard.Users.Controllers {
[HandleError, Themed]
Expand Down Expand Up @@ -289,7 +290,8 @@ public ActionResult ChangeExpiredPassword(string currentPassword, string newPass

private bool PasswordChangeIsSuccess(string currentPassword, string newPassword, string username) {
try {
var validated = _membershipService.ValidateUser(username, currentPassword);
List<LocalizedString> validationErrors;
var validated = _membershipService.ValidateUser(username, currentPassword, out validationErrors);

if (validated != null) {
_membershipService.SetPassword(validated, newPassword);
Expand All @@ -311,7 +313,7 @@ private bool PasswordChangeIsSuccess(string currentPassword, string newPassword,

[AlwaysAccessible]
public ActionResult LostPassword(string nonce) {
if ( _userService.ValidateLostPassword(nonce) == null ) {
if ( _userService.ValidateLostPassword(nonce) == null) {
return RedirectToAction("LogOn");
}

Expand All @@ -326,7 +328,7 @@ public ActionResult LostPassword(string nonce) {
[ValidateInput(false)]
public ActionResult LostPassword(string nonce, string newPassword, string confirmPassword) {
IUser user;
if ( (user = _userService.ValidateLostPassword(nonce)) == null ) {
if ( (user = _userService.ValidateLostPassword(nonce)) == null) {
return Redirect("~/");
}

Expand Down Expand Up @@ -374,7 +376,7 @@ public ActionResult ChallengeEmailFail() {
public ActionResult ChallengeEmail(string nonce) {
var user = _userService.ValidateChallenge(nonce);

if ( user != null ) {
if ( user != null) {
_userEventHandler.ConfirmedEmail(user);

return RedirectToAction("ChallengeEmailSuccess");
Expand All @@ -385,7 +387,7 @@ public ActionResult ChallengeEmail(string nonce) {

#region Validation Methods
private bool ValidateChangePassword(string currentPassword, string newPassword, string confirmPassword) {
if ( String.IsNullOrEmpty(currentPassword) ) {
if ( String.IsNullOrEmpty(currentPassword)) {
ModelState.AddModelError("currentPassword", T("You must specify a current password."));
}

Expand All @@ -395,7 +397,7 @@ private bool ValidateChangePassword(string currentPassword, string newPassword,

ValidatePassword(newPassword);

if ( !String.Equals(newPassword, confirmPassword, StringComparison.Ordinal) ) {
if ( !String.Equals(newPassword, confirmPassword, StringComparison.Ordinal)) {
ModelState.AddModelError("_FORM", T("The new password and confirmation password do not match."));
}

Expand All @@ -418,13 +420,17 @@ private IUser ValidateLogOn(string userNameOrEmail, string password) {
if (!validate)
return null;

var user = _membershipService.ValidateUser(userNameOrEmail, password);
if (user == null) {
List<LocalizedString> validationErrors;
var validationResult = _membershipService.ValidateUser(userNameOrEmail, password, out validationErrors);
if (validationResult == null) {
_userEventHandler.LogInFailed(userNameOrEmail, password);
ModelState.AddModelError("_FORM", T("The username or e-mail or password provided is incorrect."));
}

return user;
foreach (var error in validationErrors) {
ModelState.AddModelError("_FORM", error);
}

return validationResult;
}

private bool ValidateRegistration(string userName, string email, string password, string confirmPassword) {
Expand Down
Loading

0 comments on commit 8036e53

Please sign in to comment.