hubzero / hubzero-cms

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

[NCN-438] Manage hub secret #1675

Closed jsperhac closed 4 months ago

jsperhac commented 11 months ago

Dependencies

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

Summary

As part of the Nanohub Epic [NCN-434] we will create and maintain a unique Hub secret (32 random characters). This PR provides an interface for the Admin user to reset the Hub secret.

Motivation

This development was undertaken for Nanohub, as part of the Epic "Salesforce Newsletter Expiration Token Rewrite", NCN-434. The card for the present development task is NCN-438. This is general enough functionality to justify its inclusion in core.

The goal is to enable the admin user to reset the unique Hub secret. This secret is stored in a database table (presently, badly named campaign_hub). This Hub secret can be hashed together with a user's unique secret and a unique email 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 access to a Hub newsletter.

Note that resetting the Hub secret will invalidate any existing campaigns. Resetting this secret will be a rare event!

Code Description

This development makes use of com_config. It adds a drop-down ('hub_secret') to the Global Configuration admin page. The Admin user can select 'Update Hub Secret' in the drop-down. This will trigger the change of the Hub secret on save of the page.

Several changed files provide text, explanation, and information about the interface.

This screenshot shows the default view of the control:

hub-secret-ui

In this work, the application controller in com_config is expanded to add a private function that generates and saves a 32-character random string. This is called if the user has prompted for a new Hub secret. If the save occurs correctly, the hub_secret config value is saved as 'noop' so that it is not changed again on subsequent load of the page.

The hub_secret setting and other Hub configuration values are saved both in /var/www/hubname/configuration.php and in app/config/. For all other config settings, only the files are used. In order to tag along with this mechanism, hub_secret is also saved as a file, but only the value 'noop' should be saved. Note that the Hub secret itself is not exposed on this page. It is present only in the database.

A migration file is included. This file creates the required campaign_hub database table and populates it with a single record, on up migration; it drops the table on down migration.

Testing

This feature was tested by accessing the Admin page's Global Configuration, and by viewing the configuration files and the single value of 'secret' in the database table campaign_hub.

When the Admin user selects "Reset Hub Secret" and saves the configuration page, the secret value saved in the campaign_hub table will be updated to a new random string. The hub_secret value saved in both /var/www/hubname/configuration.php and in app/config/hub_secret.php will be 'noop'.

If the Admin user accepts the default setting of this configuration in the Admin UI, the database secret value will not be changed. Again, the value of hub_secret in the files is 'noop'.

Up and down migration should also be tested:

Deployment

This should be deployed together with PR #1663 as they are part of the same Epic. Additional work associated with this is still to be completed. The campaign_hub table needs to exist. (Should the table creation be added to a migration script for this work?)

Revisions

This section is TBA as it depends on what happens in the code review. ;-)

Questions for code review:

nkissebe commented 11 months ago

Let's make a table jos_config and put a general purpose design to it for future use. Columns scope, key, value. This instance could be scope=global, key=secret, value=XXX. We can build a model around it, but for now this can be a one off for this variable and the next time we use it we can put in time for the general purpose infrastructure.

My only concern here is there is already a hub wide "secret" that is used to encrypt session info and I am afraid of confusing the two. We can't use the same value because changing the secret would mess up all active sessions and we wouldn't want that to be a side effect for this purpose. So maybe an alternate naming.

I would also like to avoid writing even the noop to the config file as thats a little confusing. Maybe in the xml we can add an option to indicate its a config value to only be stored in db and we can skip the file saving aspect. That might be a nice feature to have available. Not sure how feasible that is.

jsperhac commented 8 months ago

@nkissebe makes excellent points here. I've requested a meeting with him and @dbenham to talk over this work (and remainder of the Epic). TODO on this PR (1675):

jsperhac commented 4 months ago

Superceded by #1704