Viabilidad de permitir que un cliente con User Api Key registre una auth_redirect válida

Desarrollador
1

Estoy considerando si es factible permitir que los clientes de clave de API de usuario registren un auth_redirect válido para la creación de claves.

Funcionalidad Actual

Actualmente, las redirecciones de claves de API de usuario deben ser registradas por el sitio anfitrión en la configuración del sitio allowed_user_api_auth_redirects. Cualquier auth_redirect enviado con una solicitud de creación de clave se verifica contra esa lista de configuración del sitio (ver aquí). Esta es una protección sensata para evitar una redirección abierta (que da lugar a varios riesgos de seguridad).

Causa Eficiente

La causa eficiente de esta consideración es la función de autorización de usuario del plugin ActivityPub. Esto permite a un usuario probar la propiedad de un actor de ActivityPub para que las actividades de ese actor puedan asociarse con su cuenta en una instancia determinada. Esto es lo que parece:

Actualmente funciona con Mastodon, ya que cada servidor Mastodon es un proveedor de OAuth que permite la creación programática de clientes de OAuth. El flujo funciona de la siguiente manera:

  1. El usuario identifica el dominio de Mastodon en el que se encuentra su cuenta.
  2. Si es la primera vez que se autoriza en ese dominio, el Plugin crea una aplicación OAuth en ese servidor Mastodon, registrando sus redirecciones y obteniendo credenciales de cliente para un Flujo OAuth.
  3. El Plugin realiza el flujo OAuth con el usuario utilizando la aplicación OAuth que creó (o creó previamente) en el punto 2.

El objetivo es tener una funcionalidad efectivamente idéntica con Discourse utilizando Claves de API de Usuario. Esto ya está implementado en este PR:

Este flujo requiere que el sitio anfitrión ya haya agregado el dominio o la redirección del cliente a allowed_user_api_auth_redirects.

Cambio Posible

Lo que estoy pensando actualmente es hacer un PR a discourse/discourse que haría lo siguiente:

Crear una tabla user_api_key_client

Columnas:

  • client_id y application_name migrados desde user_api_key
  • redirect_uri (opcional)
  • public_key (opcional)

Toda la funcionalidad existente que utiliza client_id y application_name funcionaría como antes, a través de user_api_key_client.

Agregar una ruta de registro de cliente

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

Parámetros requeridos:

  • application_name
  • client_id
  • public_key
  • auth_redirect

Acción:

  • Validar y guardar los parámetros en user_api_key_client
  • Devolver un código de éxito si es exitoso

Permitir el uso de clientes registrados en acciones de creación

Cambiar la acción create de UserApiKeyController de la siguiente manera:

  • Si client_id existe en user_api_key_client y tiene TANTO auth_redirect como public_key:
    • Validar el uso de auth_redirect con el auth_redirect almacenado
    • Usar la public_key almacenada para cifrar la carga útil de retorno
  • De lo contrario, seguir la funcionalidad existente.

cc @pmusaraj

2 Me gusta
2

En general, estoy a favor de esto porque permitiría el flujo de autorización de usuarios en el plugin ActivityPub y más allá sin mucha fricción para los administradores.

Sin embargo, algunas preguntas:

No soy un gran fanático de la migración aquí, preferiría dejar las claves API de usuario existentes como están porque el id y el nombre de las claves de usuario individuales son independientes del id y el nombre de los clientes registrados. Quizás en su lugar podamos agregar una columna a user_api_key para user_api_key_client_id. Las claves con clientes asociados podrán usar la redirección configurada por el cliente (y las claves existentes, por supuesto, mantendrán el comportamiento actual).

Además, ¿deberían los administradores poder revisar en algún lugar la lista de clientes registrados?

1 me gusta
4

Por eso creo que el client_id debería migrarse :slight_smile:

Actualmente, la tabla user_api_keys es una mezcla de lo que quizás deberían ser dos modelos de datos separados: claves API de usuario y clientes de claves API de usuario. Puede ver las limitaciones del enfoque actual en el método de creación de claves (aquí):

# destruir cualquier clave antigua que tuviéramos
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) },
  )

Para crear una nueva clave, debe destruir todas las claves con el mismo client_id para mantener el client_id único. En otras palabras, el modelo de datos actual requiere una relación 1:1 entre cliente y clave. Esa relación 1:1 es útil para un subconjunto de casos de uso, pero limita a otros.

De hecho, esta limitación 1:1 es la razón por la que el enfoque que está sugiriendo probablemente no funcionaría

Para que un cliente pueda registrar una redirección para múltiples claves, la unicidad del client_id debe ser independiente de las claves mismas. De lo contrario, el registro no funcionará para múltiples claves, ya que el client_id en la acción de registro es lo que se usa para identificar la redirección.

Si mueve el client_id y el application_name a la tabla dedicada user_api_key_clients, conserva los casos 1:1, pero también permite casos 1:muchos y las características que estos abren, como el registro de redirect_uri para fines de ActivityPub.

1 me gusta
6

Lo siento por la demora, Angus.

Entiendo lo que dices, pero al migrar el client_id existente, mantendríamos parte de esta mezcla en la nueva tabla porque los IDs/nombres migrados serían para claves que son clientes 1 a 1. Especialmente desde la perspectiva de un administrador en el futuro, es bueno tener una lista de los clientes registrados de muchos a uno.

Pero tal vez abordar eso no sea apropiado en este momento.

Sigamos con el plan que propones, espero el PR. ¡Gracias!

1 me gusta
7

He publicado un PR de borrador implementando esto

Tenga en cuenta que:

  • Toda la funcionalidad existente funciona como antes.
  • La migración es reversible.

Por hacer:

  • Corregir pruebas fallidas.
  • Actualmente, cualquier usuario puede registrar un cliente. ¿Debería restringirse de alguna manera(?)
1 me gusta
8

He vuelto a basar este PR ya que vuelvo a centrarme en ActivityPub, y este es un marco potencial para una de sus características, como se discutió en el OP.

Mientras volvía a basar, noté que separar las claves de los clientes como lo hace este PR también resolvería problemas como el abordado recientemente por @nat

En concreto, la necesidad de hacer este cambio, de destruir todas las claves antiguas asociadas a un cliente, independientemente del usuario, surge porque las claves y los clientes están en la misma tabla. Separarlos significa que puedes registrar una nueva clave para el usuario alternativo del cliente.

1 me gusta
9

Esto por fin se ha fusionado, muchas gracias @angus, espero que esto te desbloquee ahora.

1 me gusta
10

¡Muchas gracias! Me ha (desbloqueado) :slight_smile: Pronto habrá una implementación en una PR de autenticación de Discourse a Discourse de ActivityPub.

1 me gusta