-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Password Reset #181
Password Reset #181
Conversation
Signed-off-by: Suneeth Keerthy <skeerthy@rocketsoftware.com>
I think this is the right approach. Just awaiting ZSS stuff. |
lib/webapp.js
Outdated
@@ -1303,6 +1303,8 @@ WebApp.prototype = { | |||
|
|||
this._installRootService('/auth', 'post', this.auth.doLogin, | |||
{needJson: true, needAuth: false, isPseudoSso: true}); | |||
this._installRootService('/auth-password', 'post', this.auth.doPasswordReset, | |||
{needJson: true, needAuth: false, isPseudoSso: true}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isPseudoSso
may cause a problem. More below.
lib/webauth.js
Outdated
const _passwordResetInner = Promise.coroutine(function*(req, res, type) { | ||
const handlers = getRelevantHandlers(authManager, req.body); | ||
const result = new LoginResult(); | ||
for (const handler of handlers) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do this, it means that you are requesting a password reset from every handler.
The way this handler stuff is supposed to work is that maybe you have a handler for zos, facebook, and windows logins. You may want to login to all 3 at once. You do not want to password reset all 3 at once.
I think you should require that req.body defines exactly 1 handler, or that the URL contains a handler ID, or something like that.
Otherwise the user experience will be very strange.
Practically speaking, what will happen on 1.9 is you have 2 handlers: zss-auth and apiml-auth
zss-auth will do what you want.
apiml-auth will have canResetPassword == false, which will return success:false, which will make the UI show an error.
So, password reset will not work in that default setup. You need to limit it to zss for now.
lib/webauth.js
Outdated
@@ -277,7 +302,12 @@ module.exports = function(authManager, cookiePort, isSecurePort) { | |||
initZLUXSession(req); | |||
req.session.zlux.expirationTime = Date.now() + result.expms; | |||
} | |||
|
|||
if (result.categories.zss) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dont hardcode this.
Just make it so that if any plugin has reason "Expired password" that a 428 is set. You need to go through the list.
Also, you need 428 to be a constant.
Signed-off-by: Suneeth Keerthy <skeerthy@rocketsoftware.com>
Signed-off-by: Suneeth Keerthy <skeerthy@rocketsoftware.com>
Signed-off-by: Leanid Astrakou <lastrakou@rocketsoftware.com>
Changed a value to a const
lib/webauth.js
Outdated
@@ -207,6 +210,36 @@ module.exports = function(authManager, cookiePort, isSecurePort) { | |||
return; | |||
}); | |||
|
|||
const _passwordResetInner = Promise.coroutine(function*(req, res, type) { | |||
const handlers = getRelevantHandlers(authManager, req.body); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More correct to just use authManager.getAllHandlers() here since it is deliberate rather than accidental, since the behavior you observe is I think accidental currently.
lib/webauth.js
Outdated
} | ||
} | ||
} | ||
} | ||
result.defaultCategory = authManager.defaultType; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put this above your for loop, because it should be in every reponse from now on.
Signed-off-by: Suneeth Keerthy <skeerthy@rocketsoftware.com>
…mework into password-reset
Signed-off-by: Suneeth Keerthy <skeerthy@rocketsoftware.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me
WIP
Goes along with:
zowe/zss#161
zowe/zowe-common-c#128
zowe/zlux-app-server#106
zowe/zss-auth#29
zowe/zlux-platform#52
zowe/zlux-app-manager#193
Signed-off-by: Suneeth Keerthy skeerthy@rocketsoftware.com