adjust routers in openid module #7528
Conversation
|
Looks good. @ThaerAlAjlouni do you agree with this change? |
| if (user != null && | ||
| membershipSettings.EnableCustomPasswordPolicy && | ||
| membershipSettings.EnablePasswordExpiration && | ||
| _membershipService.PasswordIsExpired(user, membershipSettings.PasswordExpirationTimeInDays)) { | ||
| return RedirectToAction("ChangeExpiredPassword", new { username = user.UserName }); | ||
| return RedirectToAction("ChangeExpiredPassword", "Account", new { Area = "Orchard.Users", username = user.UserName }); |
There was a problem hiding this comment.
In the same PR you change this to the Users module, but you also create routes for the OpenId module, on the same Controller/Action names, is that on purpose?
There was a problem hiding this comment.
this will redirect action to ChangeExpiredPassword in Orchard.Users because there is no ChangeExpiredPassword in opened module.
The purpose of the routes will redirect other action.(and also consistent to original code)
For this specific ChangeExpiredPassword action, considerate both of the above, it will redirect the ChangeExpiredPassword in Orchard.Users module with my test.
There was a problem hiding this comment.
But if the user is logged in with one of the OpenId providers, we can't change his/her password, because we don't own it!
So I think we need to implement custom logic to detect whether the user is logged in using a local account and then redirect to the change password action.
| @@ -44,6 +44,57 @@ public class OpenIdRoutes : IRouteProvider { | |||
| new RouteDescriptor { | |||
| Priority = 10, | |||
| Route = new Route( | |||
| "Users/Account/LogOn", | |||
There was a problem hiding this comment.
Is the goal to replace the one from Users ?
There was a problem hiding this comment.
No need to replace the routes, once the user sets the settings correctly of one of the enabled OpenId features the app will register the routes automatically after the first restart.
This was intended, no need to override the routes always, only override when the replacement is valid.
I think a more proper solution will be notifying the user that an app restart is required as @sebastienros earlier suggested.
|
@sebastienros what's the next step for this PR? |
It fixes this error:
The resource cannot be found.
Description: HTTP 404. The resource you are looking for (or one of its dependencies) could have been removed, had its name changed, or is temporarily unavailable. Please review the following URL and make sure that it is spelled correctly.
Requested URL: /OrchardLocal/Users/Account/ChangeExpiredPassword