hubzero / hubzero-cms

Platform for Scientific Collaboration
https://hubzero.org
GNU General Public License v2.0
47 stars 57 forks source link

[NCN-439] Enable Hub admin to perform per-user secret update #1683

Closed jsperhac closed 4 months ago

jsperhac commented 10 months ago

Dependencies

This work is part of Epic NCN-434, whose PRs should all be deployed together:

Summary

This hubzero-cms core code enables the Hub administrator to reset a Hub member's Secret. The Secret is a 32 character random string unique to each Hub user.

The changes shown here add a checkbox in the admin interface tab that is used for editing a member's password and related particulars. If the checkbox is checked, and the form is saved, a new user Secret is generated and saved in jos_users.secret for that user, replacing any existing Secret.

Navigation: Admin UI -> Users : Members -> select a specific User -> select Password tab -> scroll to bottom of page.

The following screenshot shows the new checkbox on the Password tab:

admin-members-edit-secret

Motivation

This development was undertaken for Nanohub, as part of the Epic "Salesforce Newsletter Expiration Token Rewrite", NCN-434. The specific card for this work is NCN-439.

Once this epic is deployed, each user on the Hub will have a unique secret. The user secret can then be hashed with a unique Hub secret and a unique Campaign secret to create a unique code. This code can be used to form a URL that will be emailed to the user to provide them with secure, individual access to a newsletter or other materials.

This development will be deployed with other changes from Epic NCN-434 that implement the Campaign secret, the Hub secret, the creation of all User secrets, and their management.

Code Description

These changes include:

Testing

This plugin was tested on a Hubzero AWS instance (jsperhac.aws.hubzero.org) running CentOS7. Tests included:

Note that there is no indication of the user secret in the UI. This is by design. Verification that the secret has changed must be done by querying the database table jos_users as shown below:

test-user-secret-reset

Deployment

This plugin should be deployed with other changes stemming from Nanohub epic NCN-434. Hotfixing should not be necessary.

TODO

Note: The author does not intend to merge these changes with PR 1693 (which moves the user secret plugin to Hubzero core plugin). Both of these PRs pertain to different components of the hub, both can be reviewed separately, and both contain the needed migration scripts to check for, create, and populate the secret column in jos_users table.

Recommend bundling the changes discussed in PR #1663 into this branch and this work. To summarize, the team concluded we should fold the user plugin developed in Jira NCN-435 into the com_members component.

The current PR has already borrowed the migration script from PR #1663. Consolidating these two PRs will facilitate testing, evaluation, and deployment of these changes.

nkissebe commented 10 months ago

Should the migration populate the secret field for all users that don't have one? I believe SQL has a built in function for generating random character sequences that could be used.

nkissebe commented 10 months ago

Is there any usefulness to actually showing the secret field? For salesforce debugging or anything like that? I'm not super happy with a checkbox. I envisioned showing the secret with a [Generate New User Secret] button next to it that would generate a new one to save. (fwiw, password should probably have something similar). Maybe at least add some hover over text or help text direct on the form saying what this is for.

jsperhac commented 8 months ago

@nkissebe and @dbenham: User generation of secrets for all users that have logged into the Hub in the last 1 year is now part of the migration script for this work. Same is done in PR #1693.

jsperhac commented 6 months ago

@nkissebe, I have replaced the PHP loop for updating user secrets with a one-liner that does the update on the RDBMS side. Thanks for the suggestion; it's a cleaner solution.

jsperhac commented 6 months ago

Replacement of the loop over users is discussed here: https://sdx-sdsc.atlassian.net/browse/NCN-743

jsperhac commented 4 months ago

Superceded by #1704