Skip to content
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

Fix PR #6283: DB column value of "local backend" is 'client' and not 'local' #6496

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

nicorac
Copy link

@nicorac nicorac commented Mar 21, 2023

PR #6283 make it impossible for users to reset their password.
They now receive the error: "Unable to reset password. Contact your administrator"

This is because, in table ost_user the backend field contains client and not local for local authentication backend...

NOTE: this issue was already pointed out by other users here:
06e334f

nicorac referenced this pull request Mar 21, 2023
Reviewed-By: osTicket Devs <devs@osticket.com>
@JediKev JediKev added the Approved This pull has been reviewed, tested, and approved. label Mar 21, 2023
@JensEB
Copy link
Contributor

JensEB commented Mar 24, 2023

pull request #6283 should be removed again.
An agent or user has always a local account where they can change the password. Local accounts are not blocked, if users or agents access osticket via ldap for example. There are only two ways to login now. Users should every time able to change their password.

So I suggest to remove the changes on ./pwreset.php made by #6283 completely and change the changes on ./scp/pwreset.php to:
if (($bk=$staff->getAuthBackend()) && !($bk instanceofosTicketStaffAuthentication))
$msg = __('Unable to reset password. Contact your administrator');
elseif (!$staff->sendResetEmail())
`

So local accounts for users are always possible and for agents, if their account is not restricted to one method.

@TimzOwen
Copy link

This works much better to avoid confliction on the agents & Cred stufff

Copy link

@TimzOwen TimzOwen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. makes it easier for local and client management or Cred

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved This pull has been reviewed, tested, and approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants