From 2770c2e3c45d78ec58ba173ef484e4f79ac0272b Mon Sep 17 00:00:00 2001 From: jolavillette Date: Sat, 8 Aug 2026 08:38:10 +0200 Subject: [PATCH] gui(jsonapi): make the token Remove button actually work Removing an authorized token from the JSON API settings page could silently do nothing, leaving the token in the list across restarts. - Remove now acts on the selection of the token list, the token line edit being only a fallback for hand-typed tokens. A click below the last item yields an invalid index, which used to clear the line edit and turn Remove into a no-op on an empty user name. - Report failures instead of ignoring them: revokeAuthToken() and authorizeUser() return values are now shown to the user, as is an attempt to remove nothing. - Token management widgets are no longer disabled along with the server: authorizations are persistent configuration, they must stay editable while the server is stopped. Only port, listen address and Apply follow the server state, and load() now syncs them explicitly, since the check box is set whileBlocking and therefore commands nothing at page opening. - Refresh the list from the tokens actually authorized in the core. It was repopulated from Settings->getJsonApiAuthTokens(), a QSettings key that nothing ever writes: the matching setter had no caller at all. Both accessors are removed, tokens belong to jsonapi.cfg only. - Prevent in-place edition of the tokens, and stop leaking the list models. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/gui/settings/JsonApiPage.cc | 108 ++++++++++++------ retroshare-gui/src/gui/settings/JsonApiPage.h | 12 ++ .../src/gui/settings/rsharesettings.cpp | 9 -- .../src/gui/settings/rsharesettings.h | 4 +- 4 files changed, 90 insertions(+), 43 deletions(-) diff --git a/retroshare-gui/src/gui/settings/JsonApiPage.cc b/retroshare-gui/src/gui/settings/JsonApiPage.cc index a7ad05574..e9d542260 100644 --- a/retroshare-gui/src/gui/settings/JsonApiPage.cc +++ b/retroshare-gui/src/gui/settings/JsonApiPage.cc @@ -26,6 +26,7 @@ #include "util/qtthreadsutils.h" #include +#include #include #include #include @@ -64,6 +65,9 @@ JsonApiPage::JsonApiPage(QWidget */*parent*/, Qt::WindowFlags /*flags*/) ui.listenAddressLineEdit->setValidator(ipValidator); ui.providersListView->setSelectionMode(QAbstractItemView::NoSelection); // prevents edition. + ui.tokensListView->setSelectionMode(QAbstractItemView::SingleSelection); + ui.tokensListView->setEditTriggers(QAbstractItemView::NoEditTriggers); // prevents in-place edition of tokens + mEventHandlerId = 0; rsEvents->registerEventsHandler( [this](std::shared_ptr e) @@ -97,14 +101,16 @@ QString JsonApiPage::helpText() const The web interface for instance will automatically register its own token to the JSON API which will be visible \ in the list of authenticated tokens after you enable it.

"); } +void JsonApiPage::setServerWidgetsEnabled(bool enabled) +{ + ui.applyConfigPushButton->setEnabled(enabled); + ui.portSpinBox->setEnabled(enabled); + ui.listenAddressLineEdit->setEnabled(enabled); +} + void JsonApiPage::enableJsonApi(bool checked) { - ui.addTokenPushButton->setEnabled(checked); - ui.applyConfigPushButton->setEnabled(checked); - ui.removeTokenPushButton->setEnabled(checked); - ui.tokensListView->setEnabled(checked); - ui.portSpinBox->setEnabled(checked); - ui.listenAddressLineEdit->setEnabled(checked); + setServerWidgetsEnabled(checked); Settings->setJsonApiEnabled(checked); @@ -128,27 +134,41 @@ bool JsonApiPage::updateParams() return ok; } -void JsonApiPage::load() +void JsonApiPage::updateTokenList() { - whileBlocking(ui.portSpinBox)->setValue(rsJsonApi->listeningPort()); - whileBlocking(ui.listenAddressLineEdit)->setText(QString::fromStdString(rsJsonApi->getBindingAddress())); - whileBlocking(ui.enableCheckBox)->setChecked(rsJsonApi->isRunning()); - - QStringList newTk; + QStringList newTk; for(const auto& it : rsJsonApi->getAuthorizedTokens()) newTk.push_back( QString::fromStdString(it.first) + ":" + QString::fromStdString(it.second) ); - whileBlocking(ui.tokensListView)->setModel(new QStringListModel(newTk)); + QAbstractItemModel *oldModel = ui.tokensListView->model(); + whileBlocking(ui.tokensListView)->setModel(new QStringListModel(newTk,ui.tokensListView)); + delete oldModel; +} + +void JsonApiPage::load() +{ + whileBlocking(ui.portSpinBox)->setValue(rsJsonApi->listeningPort()); + whileBlocking(ui.listenAddressLineEdit)->setText(QString::fromStdString(rsJsonApi->getBindingAddress())); + whileBlocking(ui.enableCheckBox)->setChecked(rsJsonApi->isRunning()); + + // the check box is set whileBlocking, so the widgets it commands need to be + // synced explicitly, otherwise the page opens in an inconsistent state. + + setServerWidgetsEnabled(rsJsonApi->isRunning()); + + updateTokenList(); QStringList newTk2; for(const auto& it : rsJsonApi->getResourceProviders()) newTk2.push_back( QString::fromStdString(it.get().getName())) ; - whileBlocking(ui.providersListView)->setModel(new QStringListModel(newTk2)); + QAbstractItemModel *oldProvidersModel = ui.providersListView->model(); + whileBlocking(ui.providersListView)->setModel(new QStringListModel(newTk2,ui.providersListView)); + delete oldProvidersModel; if(rsJsonApi->isRunning()) ui.statusLabelLED->setPixmap(FilesDefs::getPixmapFromQtResourcePath(IMAGE_LEDON)) ; @@ -220,37 +240,61 @@ void JsonApiPage::addTokenClicked() QString token(ui.tokenLineEdit->text()); std::string user,passwd; - if(!RsJsonApi::parseToken(token.toStdString(),user,passwd)) return; + if(!RsJsonApi::parseToken(token.toStdString(),user,passwd)) + { + QMessageBox::warning( this, tr("Invalid token"), + tr("Tokens should spell as \"user:password\", where both " + "user and password are alphanumeric strings.") ); + return; + } - rsJsonApi->authorizeUser(user,passwd); + auto err = rsJsonApi->authorizeUser(user,passwd); - QStringList newTk; + if(err) + QMessageBox::warning( this, tr("Cannot add token"), + QString::fromStdString(err.message()) ); - for(const auto& it : rsJsonApi->getAuthorizedTokens()) - newTk.push_back( - QString::fromStdString(it.first) + ":" + - QString::fromStdString(it.second) ); + updateTokenList(); +} - whileBlocking(ui.tokensListView)->setModel(new QStringListModel(newTk)); +QString JsonApiPage::selectedTokenUser() const +{ + // The list selection is what the user actually points at. The token line + // edit is only used as a fallback, for tokens typed by hand. + + QString token; + const QModelIndex index = ui.tokensListView->currentIndex(); + + if(index.isValid() && ui.tokensListView->selectionModel()->isSelected(index)) + token = index.data().toString(); + else + token = ui.tokenLineEdit->text(); + + return token.section(':',0,0); // the core indexes tokens by user, not by "user:password" } void JsonApiPage::removeTokenClicked() { - QString token(ui.tokenLineEdit->text()); - std::string tokenStr = token.toStdString(); - rsJsonApi->revokeAuthToken(tokenStr.substr(0, tokenStr.find_first_of(":"))); + const QString user = selectedTokenUser(); - QStringList newTk; + if(user.isEmpty()) + { + QMessageBox::information( this, tr("No token selected"), + tr("Please select in the list below the token to remove.") ); + return; + } - for(const auto& it : rsJsonApi->getAuthorizedTokens()) - newTk.push_back( - QString::fromStdString(it.first) + ":" + - QString::fromStdString(it.second) ); + if(!rsJsonApi->revokeAuthToken(user.toStdString())) + QMessageBox::warning( this, tr("Cannot remove token"), + tr("No authorization was found for user \"%1\".").arg(user) ); - whileBlocking(ui.tokensListView)->setModel(new QStringListModel(Settings->getJsonApiAuthTokens()) ); + ui.tokenLineEdit->clear(); + + updateTokenList(); } void JsonApiPage::tokenClicked(const QModelIndex& index) { - ui.tokenLineEdit->setText(ui.tokensListView->model()->data(index).toString()); + if(index.isValid()) // a click below the last item gives an invalid index + ui.tokenLineEdit->setText(index.data().toString()); } diff --git a/retroshare-gui/src/gui/settings/JsonApiPage.h b/retroshare-gui/src/gui/settings/JsonApiPage.h index 9d5755213..548edd1bf 100644 --- a/retroshare-gui/src/gui/settings/JsonApiPage.h +++ b/retroshare-gui/src/gui/settings/JsonApiPage.h @@ -64,6 +64,18 @@ public slots: void checkToken(QString); private: + /// Refresh the token list view from the tokens actually authorized in the core + void updateTokenList(); + + /// Enable/disable the widgets that only make sense while the server runs. + /// Token management is *not* part of them: authorizations are persistent + /// configuration, they must stay editable while the server is stopped. + void setServerWidgetsEnabled(bool enabled); + + /// Token currently designated for removal: the list selection if any, + /// the token line edit otherwise. Returns the user part only. + QString selectedTokenUser() const; + Ui::JsonApiPage ui; /// Qt Designer generated object RsEventsHandlerId_t mEventHandlerId; }; diff --git a/retroshare-gui/src/gui/settings/rsharesettings.cpp b/retroshare-gui/src/gui/settings/rsharesettings.cpp index d40f6db81..d1a47be67 100644 --- a/retroshare-gui/src/gui/settings/rsharesettings.cpp +++ b/retroshare-gui/src/gui/settings/rsharesettings.cpp @@ -1276,15 +1276,6 @@ void RshareSettings::setJsonApiListenAddress(const QString& listenAddress) setValueToGroup("JsonApi", "listenAddress", listenAddress); } -QStringList RshareSettings::getJsonApiAuthTokens() -{ - return valueFromGroup("JsonApi", "authTokens", QStringList()).toStringList(); -} - -void RshareSettings::setJsonApiAuthTokens(const QStringList& authTokens) -{ - setValueToGroup("JsonApi", "authTokens", authTokens); -} #endif // RS_JSONAPI int RshareSettings::getDateFormat() diff --git a/retroshare-gui/src/gui/settings/rsharesettings.h b/retroshare-gui/src/gui/settings/rsharesettings.h index 3dcb54c5b..6b428d67f 100644 --- a/retroshare-gui/src/gui/settings/rsharesettings.h +++ b/retroshare-gui/src/gui/settings/rsharesettings.h @@ -403,8 +403,8 @@ public: QString getJsonApiListenAddress(); void setJsonApiListenAddress(const QString& listenAddress); - QStringList getJsonApiAuthTokens(); - void setJsonApiAuthTokens(const QStringList& authTokens); + // Authorized tokens are *not* stored here: they belong to the core config + // (jsonapi.cfg), and are managed through rsJsonApi. #endif // ifdef RS_JSONAPI protected: