User API keys: duplicate client_id will lead to internal server error

bug
1

When calling user-api-key/new with a client_id that is already used by another user the forum will throw a RecordNotUnique error and silently fail on an internal server error.

image
image1347×197 10.5 KB

ActiveRecord::RecordNotUnique (PG::UniqueViolation: ERROR:  duplicate key value violates unique constraint "index_user_api_keys_on_client_id"
DETAIL:  Key (client_id)=(893e0230d52455ea9b729334) already exists.
)
(eval):105:in `exec_params'
app/controllers/user_api_keys_controller.rb:66:in `create'

This might want to fail with something less silent informing the user that there already exists an API key with that client ID.

Though that brings me on the second question, are User API keys supposed to behave like that? Is the client ID supposed to be unique between all users?

5 Likes
2

Quick nudge for visibility. We’re still seeing these show up in our /logs endpoint fairly frequently:

image
image1359×70 9.59 KB

3 Likes
7

Thank you for reporting this, I have a couple of questions though in order to help me look into this.

Can you provide a basic repro for this so that I can debug this locally? What is your use case for user-api-keys? Are you using the discourse hub mobile app or something else?

1 Like
8

I’m not sure what you prefer as repro, but this bit of PHP code can repro it great.

Code 
<?php

// openssl genrsa -out keypair.pem 2048
$keypair = openssl_pkey_get_private(file_get_contents("keypair.pem"));

// Obtain the public key
$public = openssl_pkey_get_details($keypair)["key"];

// Build the query
$query = http_build_query([
    "auth_redirect" => "https://localhost/redirect",
    "application_name" => "Test repro",
    "client_id" => "7624a5376b7f52eb403a",
    "scopes" => "session_info",
    "nonce" => bin2hex(random_bytes(16)),
    "public_key" => $public
]);

$url = "https://forum.cfx.re/user-api-key/new?" . $query;
header("Location: " . $url);

The first and repeating authorization will succeed as the first user, when using it again for another user without changing the client_id it will fail.

image
image1252×63 8.55 KB

User API keys are used for allowing the user to use their forum account in the game client, so they’ll be able to post from ingame. We also have lot of users using them to authenticate with forum accounts to their own websites.

Whereas the client ID should be unique for the game clients, so each client is listed as seperate client in the apps screen. For website usecase you’d want to have one client ID so not each login is listed separately.

3 Likes
9

Was wondering if this was resolved