Ich überlege, ob es machbar ist, Clients mit Benutzer-API-Schlüsseln die Registrierung einer gültigen auth_redirect zum Zweck der Schlüsselerstellung zu ermöglichen.
Aktuelle Funktionalität
Derzeit müssen Benutzer-API-Schlüssel-Redirects vom Host-Site in der Site-Einstellung allowed_user_api_auth_redirects registriert werden. Jeder auth_redirect, der mit einer Schlüssel-Erstellungsanfrage gesendet wird, wird gegen diese Liste von Site-Einstellungen geprüft (siehe hier). Dies ist ein sinnvoller Schutz, um einen offenen Redirect zu vermeiden (der zu verschiedenen Sicherheitsrisiken führt).
Ursache
Die unmittelbare Ursache für diese Überlegung ist die Benutzerautorisierungsfunktion des ActivityPub-Plugins. Diese ermöglicht es einem Benutzer, den Besitz seines ActivityPub-Akteurs nachzuweisen, damit die Aktivitäten dieses Akteurs seinem Konto auf einer bestimmten Instanz zugeordnet werden können. So sieht das aus:
Dies funktioniert derzeit mit Mastodon, da jeder Mastodon-Server ein OAuth-Anbieter ist, der die programmatische Erstellung von OAuth-Clients ermöglicht. Der Ablauf funktioniert wie folgt:
Der Benutzer gibt die Mastodon-Domain an, auf der sich sein Konto befindet.
Wenn die Domain zum ersten Mal autorisiert wird, erstellt das Plugin eine OAuth-App auf diesem Mastodon-Server, registriert deren Redirects und erhält Client-Anmeldeinformationen für einen OAuth-Flow.
Das Plugin führt den OAuth-Flow mit dem Benutzer unter Verwendung der in 2 erstellten (oder zuvor erstellten) OAuth-App durch.
Das Ziel ist, mit Discourse mit Benutzer-API-Schlüsseln eine praktisch identische Funktionalität zu haben. Dies ist in diesem PR bereits implementiert:
Dieser Flow erfordert, dass die Host-Site die Domain oder den Redirect des Clients bereits zu allowed_user_api_auth_redirects hinzugefügt hat.
Mögliche Änderung
Ich denke derzeit darüber nach, einen PR für discourse/discourse zu erstellen, der Folgendes tun würde:
Erstellen einer user_api_key_client-Tabelle
Spalten:
client_id und application_name migriert von user_api_key
redirect_uri (optional)
public_key (optional)
Alle bestehenden Funktionalitäten, die client_id und application_name verwenden, würden wie bisher über user_api_key_client funktionieren.
Hinzufügen einer Route zur Registrierung von Clients
post "/user-api-key/register" => "user_api_keys#register"
Erforderliche Parameter:
application_name
client_id
public_key
auth_redirect
Aktion:
Parameter validieren und in user_api_key_client speichern
Bei Erfolg einen Erfolgscode zurückgeben
Zulassen der Verwendung registrierter Clients bei Erstellungsaktionen
Ändern Sie die create-Aktion des UserApiKeyController wie folgt:
Wenn client_id in user_api_key_client vorhanden ist und SOWOH auth_redirect als auch public_key hat:
Validieren Sie die Verwendung von auth_redirect mit dem gespeicherten auth_redirect
Verwenden Sie den gespeicherten public_key zur Verschlüsselung der Rückgabe-Payload
Andernfalls folgen Sie der bestehenden Funktionalität.
Insgesamt bin ich dafür, da dies den Workflow der Benutzerautorisierung im ActivityPub-Plugin und darüber hinaus ohne große Reibung für Administratoren ermöglichen würde.
Einige Fragen jedoch:
Ich bin kein großer Fan der Migration hier, ich würde es vorziehen, bestehende Benutzer-API-Schlüssel so zu belassen, wie sie sind, da die ID und der Name einzelner Benutzerschlüssel unabhängig von der ID und dem Namen der registrierten Clients sind. Vielleicht können wir stattdessen eine Spalte zu user_api_key für user_api_key_client_id hinzufügen. Die Schlüssel mit zugehörigen Clients können dann den vom Client konfigurierten Redirect verwenden (und bestehende Schlüssel behalten natürlich das aktuelle Verhalten bei).
Darüber hinaus, sollten Administratoren irgendwo die Liste der registrierten Clients überprüfen können?
Deshalb denke ich, dass die client_id migriert werden sollte
Derzeit ist die Tabelle user_api_keys eine Vermischung dessen, was vielleicht zwei separate Datenmodelle sein sollten: Benutzerschlüssel und Benutzer-API-Schlüssel-Clients. Sie können die Einschränkungen des aktuellen Ansatzes in der Methode zur Schlüsselgenerierung sehen (hier):
# destroy any old keys we had
UserApiKey.where(user_id: current_user.id, client_id: params[:client_id]).destroy_all
key =
UserApiKey.create!(
application_name: @application_name,
client_id: params[:client_id],
user_id: current_user.id,
push_url: params[:push_url],
scopes: scopes.map { |name| UserApiKeyScope.new(name: name) },
)
Um einen neuen Schlüssel zu erstellen, müssen Sie alle Schlüssel mit derselben client_id zerstören, um die client_id eindeutig zu halten. Anders ausgedrückt, das aktuelle Datenmodell erfordert eine 1:1-Beziehung zwischen Client und Schlüssel. Diese 1:1-Beziehung ist für eine Teilmenge von Anwendungsfällen nützlich, schränkt aber andere ein.
Tatsächlich ist diese 1:1-Einschränkung der Grund, warum der von Ihnen vorgeschlagene Ansatz wahrscheinlich nicht funktionieren würde
Damit ein Client einen Redirect für mehrere Schlüssel registrieren kann, muss die Eindeutigkeit der client_id unabhängig von den Schlüsseln selbst sein. Andernfalls funktioniert die Registrierung nicht für mehrere Schlüssel, da die client_id in der Registrierungsaktion zur Identifizierung des Redirects verwendet wird.
Wenn Sie die client_id und application_name in die dedizierte Tabelle user_api_key_clients verschieben, behalten Sie die 1:1-Fälle bei, ermöglichen aber auch 1:viele-Fälle und die damit verbundenen Funktionen, wie die Registrierung von redirect_uri für die Zwecke von ActivityPub.
Ich verstehe, was Sie sagen, aber durch die Migration der bestehenden client_id würden wir diese Vermischung in der neuen Tabelle beibehalten, da die migrierten IDs/Namen für Schlüssel gelten, die 1-zu-1-Clients sind. Besonders aus der Sicht eines Administrators ist es gut, eine Liste der registrierten Multi-zu-Eins-Clients zu haben.
Aber vielleicht ist die Behandlung dessen im Moment nicht angebracht.
Lass uns mit dem von Ihnen vorgeschlagenen Plan fortfahren, ich freue mich auf den PR. Danke!
Ich habe diesen PR rebased, da ich mich wieder auf ActivityPub konzentriere und dies ein potenzieller Rahmen für eine seiner Funktionen ist, wie in der OP besprochen.
Während des Rebasings fiel mir auf, dass die Trennung von Schlüsseln von Clients, wie dieser PR sie vornimmt, auch Probleme lösen würde, wie das kürzlich von @nat angesprochene.
Insbesondere die Notwendigkeit, diese Änderung vorzunehmen, um alle alten Schlüssel, die einem Client zugeordnet sind, unabhängig vom Benutzer zu zerstören, ergibt sich daraus, dass Schlüssel und Clients in derselben Tabelle stehen. Durch die Trennung können Sie einfach einen neuen Schlüssel für den alternativen Benutzer des Clients registrieren.
