Faisabilité de permettre à un client avec une clé API utilisateur de s'inscrire à un auth_redirect valide

J’envisage s’il est possible d’autoriser les clients clé API utilisateur à enregistrer un auth_redirect valide dans le but de créer des clés.

Fonctionnalité actuelle

Actuellement, les redirections de clés API utilisateur doivent être enregistrées par le site hôte dans le paramètre du site allowed_user_api_auth_redirects. Tout auth_redirect envoyé avec une demande de création de clé est vérifié par rapport à cette liste de paramètres du site (voir ici). C’est une protection judicieuse pour éviter une redirection ouverte (qui donne lieu à divers risques de sécurité).

Cause efficace

La cause efficace de cette considération est la fonctionnalité d’autorisation utilisateur du plugin ActivityPub. Cela permet à un utilisateur de prouver la propriété de son acteur ActivityPub afin que les activités de cet acteur puissent être associées à son compte sur une instance donnée. Voici à quoi cela ressemble :

Cela fonctionne actuellement avec Mastodon car chaque serveur Mastodon est un fournisseur OAuth qui permet la création programmatique de clients OAuth. Le flux fonctionne comme suit :

  1. L’utilisateur identifie le domaine Mastodon sur lequel son compte se trouve.
  2. Si c’est la première fois qu’il est autorisé sur ce domaine, le plugin crée une application OAuth sur ce serveur Mastodon, enregistre ses redirections et obtient les informations d’identification client pour un flux OAuth.
  3. Le plugin effectue un flux OAuth avec l’utilisateur en utilisant l’application OAuth qu’il a créée (ou créée précédemment) au point 2.

L’objectif est d’avoir une fonctionnalité pratiquement identique avec Discourse en utilisant les clés API utilisateur. Ceci est déjà implémenté dans cette PR :

Ce flux nécessite que le site hôte ait déjà ajouté le domaine ou la redirection du client à allowed_user_api_auth_redirects.

Changement possible

Ce à quoi je pense actuellement est de faire une PR à discourse/discourse qui ferait ce qui suit :

Créer une table user_api_key_client

Colonnes :

  • client_id et application_name migrés depuis user_api_key
  • redirect_uri (optionnel)
  • public_key (optionnel)

Toutes les fonctionnalités existantes utilisant client_id et application_name fonctionneraient comme avant, via user_api_key_client.

Ajouter une route d’enregistrement de client

post "/user-api-key/register" => "user_api_keys#register"

Paramètres requis :

  • application_name
  • client_id
  • public_key
  • auth_redirect

Action :

  • Valider et enregistrer les paramètres dans user_api_key_client
  • Retourner un code de succès si réussi

Permettre l’utilisation des clients enregistrés dans les actions de création

Modifier l’action create de UserApiKeyController comme suit :

  • Si client_id existe dans user_api_key_client et possède à la fois auth_redirect et public_key :
    • Valider l’utilisation de auth_redirect avec le auth_redirect stocké
    • Utiliser le public_key stocké pour chiffrer la charge utile de retour
  • Sinon, suivre la fonctionnalité existante.

cc @pmusaraj

2 « J'aime »

Dans l’ensemble, je suis favorable à cela car cela permettrait le flux d’autorisation des utilisateurs dans le plugin ActivityPub et au-delà sans beaucoup de friction pour les administrateurs.

Cependant, quelques questions :

Je ne suis pas un grand fan de la migration ici, je préférerais laisser les clés API utilisateur existantes telles quelles car l’identifiant et le nom des clés utilisateur individuelles sont indépendants de l’identifiant et du nom des clients enregistrés. Peut-être pourrions-nous plutôt ajouter une colonne à user_api_key pour user_api_key_client_id. Les clés associées à des clients pourront alors utiliser la redirection configurée par le client (et les clés existantes, bien sûr, conserveront le comportement actuel).

De plus, les administrateurs devraient-ils pouvoir examiner quelque part la liste des clients enregistrés ?

1 « J'aime »

C’est pourquoi je pense que le client_id devrait être migré :slight_smile:

Actuellement, la table user_api_keys est une combinaison de ce qui devrait peut-être être deux modèles de données distincts : les clés API utilisateur et les clients de clés API utilisateur. Vous pouvez voir les limitations de l’approche actuelle dans la méthode de création de clé (ici) :

# détruire toutes les anciennes clés que nous avions
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) },
  )

Pour créer une nouvelle clé, vous devez détruire toutes les clés ayant le même client_id afin de conserver l’unicité du client_id. En d’autres termes, le modèle de données actuel nécessite une relation 1:1 entre le client et la clé. Cette relation 1:1 est utile pour un sous-ensemble de cas d’utilisation, mais en limite d’autres.

En effet, cette limitation 1:1 est la raison pour laquelle l’approche que vous suggérez ne fonctionnerait probablement pas.

Pour qu’un client puisse enregistrer un redirect pour plusieurs clés, l’unicité du client_id doit être indépendante des clés elles-mêmes. Sinon, l’enregistrement ne fonctionnera pas pour plusieurs clés, car le client_id dans l’action d’enregistrement est ce qui est utilisé pour identifier le redirect.

Si vous déplacez le client_id et application_name vers la table dédiée user_api_key_clients, vous conservez les cas 1:1, mais vous autorisez également les cas 1:N, ainsi que les fonctionnalités qu’ils ouvrent, telles que l’enregistrement de redirect_uri à des fins d’ActivityPub.

1 « J'aime »

Désolé pour le retard, Angus.

Je vois ce que vous voulez dire, mais en migrant l’client_id existant, nous conserverions une partie de cette confusion dans la nouvelle table car les identifiants/noms migrés seraient pour des clés qui sont des clients 1-à-1. Surtout du point de vue d’un administrateur à l’avenir, il est bon d’avoir une liste des clients multi-à-un enregistrés.

Mais peut-être que s’attaquer à cela n’est pas approprié pour le moment.

Allons-y avec le plan que vous proposez, j’attends avec impatience la PR. Merci !

1 « J'aime »

J’ai publié une PR brouillon implémentant ceci

Notez que :

  • Toutes les fonctionnalités existantes fonctionnent comme avant.
  • La migration est réversible.

À faire :

  • Corriger les tests échoués.
  • Actuellement, n’importe quel utilisateur peut enregistrer un client. Faut-il restreindre cela d’une manière ou d’une autre (?)
1 « J'aime »

J’ai rebasé cette PR car je me concentre à nouveau sur ActivityPub, et ceci est un cadre potentiel pour l’une de ses fonctionnalités, comme discuté dans le OP.

En rebasant, j’ai remarqué que séparer les clés des clients, comme le fait cette PR, résoudrait également des problèmes tels que celui récemment abordé par @nat

Notamment, la nécessité de faire ce changement, de détruire toutes les anciennes clés associées à un client, quel que soit l’utilisateur, découle du fait que les clés et les clients se trouvent dans la même table. Les séparer signifie que vous pouvez simplement enregistrer une nouvelle clé pour l’utilisateur alternatif du client.

1 « J'aime »

Ceci est enfin fusionné, merci beaucoup @angus, j’espère que cela vous débloquera maintenant.

1 « J'aime »

Merci beaucoup ! Cela m’a (débloqué) :slight_smile: Il y aura bientôt une implémentation dans une PR d’authentification Discourse-à-Discourse ActivityPub.

1 « J'aime »