From fc095acaaa3a46432a171dedfbb8ecb91d31c46e Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 15 Aug 2016 17:58:33 +1000 Subject: [PATCH] Feature: User API key support (server side implementation) - Supports throttled read and write - No support for push yet, but data is captured about intent --- app/controllers/user_api_keys_controller.rb | 68 +++++++++ app/models/user_api_key.rb | 30 ++++ config/locales/client.en.yml | 1 + config/locales/server.en.yml | 10 ++ config/routes.rb | 4 + config/site_settings.yml | 22 +++ .../20160815002002_add_user_api_keys.rb | 19 +++ lib/auth/default_current_user_provider.rb | 36 +++++ .../default_current_user_provider_spec.rb | 73 ++++++++++ .../user_api_keys_controller_spec.rb | 133 ++++++++++++++++++ 10 files changed, 396 insertions(+) create mode 100644 app/controllers/user_api_keys_controller.rb create mode 100644 app/models/user_api_key.rb create mode 100644 db/migrate/20160815002002_add_user_api_keys.rb create mode 100644 spec/controllers/user_api_keys_controller_spec.rb diff --git a/app/controllers/user_api_keys_controller.rb b/app/controllers/user_api_keys_controller.rb new file mode 100644 index 000000000..d870142b1 --- /dev/null +++ b/app/controllers/user_api_keys_controller.rb @@ -0,0 +1,68 @@ +class UserApiKeysController < ApplicationController + + skip_before_filter :redirect_to_login_if_required, only: [:new] + skip_before_filter :check_xhr + before_filter :ensure_logged_in, only: [:create] + + def new + end + + def create + + [ + :public_key, + :nonce, + :access, + :client_id, + :auth_redirect, + :application_name + ].each{|p| params.require(p)} + + unless SiteSetting.allowed_user_api_auth_redirects + .split('|') + .any?{|u| params[:auth_redirect] == u} + + raise Discourse::InvalidAccess + end + + raise Discourse::InvalidAccess if current_user.trust_level < SiteSetting.min_trust_level_for_user_api_key + + request_read = params[:access].include? 'r' + request_push = params[:access].include? 'p' + request_write = params[:access].include? 'w' + + raise Discourse::InvalidAccess unless request_read || request_push + raise Discourse::InvalidAccess if request_read && !SiteSetting.allow_read_user_api_keys + raise Discourse::InvalidAccess if request_write && !SiteSetting.allow_write_user_api_keys + raise Discourse::InvalidAccess if request_push && !SiteSetting.allow_push_user_api_keys + + if request_push && !SiteSetting.allowed_user_api_push_urls.split('|').any?{|u| params[:push_url] == u} + raise Discourse::InvalidAccess + end + + key = UserApiKey.create!( + application_name: params[:application_name], + client_id: params[:client_id], + read: request_read, + push: request_push, + user_id: current_user.id, + write: request_write, + key: SecureRandom.hex, + push_url: request_push ? params[:push_url] : nil + ) + + # we keep the payload short so it encrypts easily with public key + # it is often restricted to 128 chars + payload = { + key: key.key, + nonce: params[:nonce], + access: key.access + }.to_json + + public_key = OpenSSL::PKey::RSA.new(params[:public_key]) + payload = Base64.encode64(public_key.public_encrypt(payload)) + + redirect_to "#{params[:auth_redirect]}?payload=#{CGI.escape(payload)}" + end + +end diff --git a/app/models/user_api_key.rb b/app/models/user_api_key.rb new file mode 100644 index 000000000..9d518942d --- /dev/null +++ b/app/models/user_api_key.rb @@ -0,0 +1,30 @@ +class UserApiKey < ActiveRecord::Base + belongs_to :user + + def access + "#{read ? "r" : ""}#{write ? "w" : ""}#{push ? "p" : ""}" + end +end + +# == Schema Information +# +# Table name: user_api_keys +# +# id :integer not null, primary key +# user_id :integer not null +# client_id :string not null +# key :string not null +# application_name :string not null +# read :boolean not null +# write :boolean not null +# push :boolean not null +# push_url :string +# created_at :datetime +# updated_at :datetime +# +# Indexes +# +# index_user_api_keys_on_client_id (client_id) +# index_user_api_keys_on_key (key) UNIQUE +# index_user_api_keys_on_user_id (user_id) +# diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index e6225cede..aad28eab0 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3006,6 +3006,7 @@ en: developer: 'Developer' embedding: "Embedding" legal: "Legal" + user_api: 'User API' uncategorized: 'Other' backups: "Backups" login: "Login" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 4f8477943..2bb0fd464 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1365,6 +1365,16 @@ en: default_categories_tracking: "List of categories that are tracked by default." default_categories_muted: "List of categories that are muted by default." + max_user_api_reqs_per_day: "Maximum number of user API requests per key per day" + max_user_api_reqs_per_minute: "Maximum number of user API requests per key per minute" + allow_read_user_api_keys: "Allow generation of readonly user API keys" + allow_write_user_api_keys: "Allow generation of write user API keys" + allow_push_user_api_keys: "Allow generation of push user API keys" + max_api_keys_per_user: "Maximum number of user API keys per user" + min_trust_level_for_user_api_key: "Trust level required for generation of user API keys" + allowed_user_api_auth_redirects: "Allowed URL for authentication redirect for user API keys" + allowed_user_api_push_urls: "Allowed URLs for server push to user API" + tagging_enabled: "Enable tags on topics?" min_trust_to_create_tag: "The minimum trust level required to create a tag." max_tags_per_topic: "The maximum tags that can be applied to a topic." diff --git a/config/routes.rb b/config/routes.rb index 784e79598..3759a3469 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -661,5 +661,9 @@ Discourse::Application.routes.draw do # special case for top root to: "list#top", constraints: HomePageConstraint.new("top"), :as => "top_lists" + get "/user-api-key/new" => "user_api_keys#new" + post "/user-api-key/new" => "user_api_keys#create" + get "*url", to: 'permalinks#show', constraints: PermalinkConstraint.new + end diff --git a/config/site_settings.yml b/config/site_settings.yml index a0f37772f..f501aae40 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1243,6 +1243,28 @@ user_preferences: type: category_list default: '' +user_api: + max_user_api_reqs_per_day: + default: 2880 + max_user_api_reqs_per_minute: + default: 12 + allow_read_user_api_keys: + default: true + allow_write_user_api_keys: + default: false + allow_push_user_api_keys: + default: true + max_api_keys_per_user: + default: 10 + min_trust_level_for_user_api_key: + default: 1 + allowed_user_api_push_urls: + default: '' + type: list + allowed_user_api_auth_redirects: + default: 'https://api.discourse.org/api/auth_redirect' + type: list + tags: tagging_enabled: client: true diff --git a/db/migrate/20160815002002_add_user_api_keys.rb b/db/migrate/20160815002002_add_user_api_keys.rb new file mode 100644 index 000000000..0789781d8 --- /dev/null +++ b/db/migrate/20160815002002_add_user_api_keys.rb @@ -0,0 +1,19 @@ +class AddUserApiKeys < ActiveRecord::Migration + def change + create_table :user_api_keys do |t| + t.integer :user_id, null: false + t.string :client_id, null: false + t.string :key, null: false + t.string :application_name, null: false + t.boolean :read, null: false + t.boolean :write, null: false + t.boolean :push, null: false + t.string :push_url + t.timestamps + end + + add_index :user_api_keys, [:key], unique: true + add_index :user_api_keys, [:user_id] + add_index :user_api_keys, [:client_id] + end +end diff --git a/lib/auth/default_current_user_provider.rb b/lib/auth/default_current_user_provider.rb index d726be453..dc74a3000 100644 --- a/lib/auth/default_current_user_provider.rb +++ b/lib/auth/default_current_user_provider.rb @@ -5,6 +5,7 @@ class Auth::DefaultCurrentUserProvider CURRENT_USER_KEY ||= "_DISCOURSE_CURRENT_USER".freeze API_KEY ||= "api_key".freeze + USER_API_KEY ||= "USER_API_KEY".freeze API_KEY_ENV ||= "_DISCOURSE_API".freeze TOKEN_COOKIE ||= "_t".freeze PATH_INFO ||= "PATH_INFO".freeze @@ -75,10 +76,35 @@ class Auth::DefaultCurrentUserProvider @env[API_KEY_ENV] = true end + # user api key handling + if api_key = @env[USER_API_KEY] + + limiter_min = RateLimiter.new(nil, "user_api_min_#{api_key}", SiteSetting.max_user_api_reqs_per_minute, 60) + limiter_day = RateLimiter.new(nil, "user_api_day_#{api_key}", SiteSetting.max_user_api_reqs_per_day, 86400) + + unless limiter_day.can_perform? + raise RateLimiter::LimitExceeded, "User API calls per minute exceeded" + end + + unless limiter_min.can_perform? + raise RateLimiter::LimitExceeded, "User API calls per day exceeded" + end + + current_user = lookup_user_api_user(api_key) + raise Discourse::InvalidAccess unless current_user + + limiter_min.performed! + limiter_day.performed! + + @env[API_KEY_ENV] = true + end + @env[CURRENT_USER_KEY] = current_user end def refresh_session(user, session, cookies) + return if is_api? + if user && (!user.auth_token_updated_at || user.auth_token_updated_at <= 1.hour.ago) user.update_column(:auth_token_updated_at, Time.zone.now) cookies[TOKEN_COOKIE] = { value: user.auth_token, httponly: true, expires: SiteSetting.maximum_session_age.hours.from_now } @@ -150,6 +176,16 @@ class Auth::DefaultCurrentUserProvider protected + def lookup_user_api_user(user_api_key) + if api_key = UserApiKey.where(key: user_api_key).includes(:user).first + if !api_key.write && @env["REQUEST_METHOD"] != "GET" + raise Discourse::InvalidAccess + end + + api_key.user + end + end + def lookup_api_user(api_key_value, request) if api_key = ApiKey.where(key: api_key_value).includes(:user).first api_username = request["api_username"] diff --git a/spec/components/auth/default_current_user_provider_spec.rb b/spec/components/auth/default_current_user_provider_spec.rb index dd69a375b..4c38bbd9d 100644 --- a/spec/components/auth/default_current_user_provider_spec.rb +++ b/spec/components/auth/default_current_user_provider_spec.rb @@ -153,5 +153,78 @@ describe Auth::DefaultCurrentUserProvider do freeze_time 3.hours.from_now expect(provider("/", "HTTP_COOKIE" => "_t=#{user.auth_token}").current_user).to eq(nil) end + + context "user api" do + let :user do + Fabricate(:user) + end + + let :api_key do + UserApiKey.create!( + application_name: 'my app', + client_id: '1234', + read: true, + write: false, + push: false, + key: SecureRandom.hex, + user_id: user.id + ) + end + + it "allows user API access correctly" do + params = { + "REQUEST_METHOD" => "GET", + "USER_API_KEY" => api_key.key, + } + + expect(provider("/", params).current_user.id).to eq(user.id) + + expect { + provider("/", params.merge({"REQUEST_METHOD" => "POST"})).current_user + }.to raise_error(Discourse::InvalidAccess) + + end + + it "rate limits api usage" do + + RateLimiter.stubs(:disabled?).returns(false) + limiter1 = RateLimiter.new(nil, "user_api_day_#{api_key.key}", 10, 60) + limiter2 = RateLimiter.new(nil, "user_api_min_#{api_key.key}", 10, 60) + limiter1.clear! + limiter2.clear! + + SiteSetting.max_user_api_reqs_per_day = 3 + SiteSetting.max_user_api_reqs_per_minute = 4 + + params = { + "REQUEST_METHOD" => "GET", + "USER_API_KEY" => api_key.key, + } + + 3.times do + provider("/", params).current_user + end + + expect { + provider("/", params).current_user + }.to raise_error(RateLimiter::LimitExceeded) + + + SiteSetting.max_user_api_reqs_per_day = 4 + SiteSetting.max_user_api_reqs_per_minute = 3 + + limiter1.clear! + limiter2.clear! + + 3.times do + provider("/", params).current_user + end + + expect { + provider("/", params).current_user + }.to raise_error(RateLimiter::LimitExceeded) + + end + end end diff --git a/spec/controllers/user_api_keys_controller_spec.rb b/spec/controllers/user_api_keys_controller_spec.rb new file mode 100644 index 000000000..b4c86b10f --- /dev/null +++ b/spec/controllers/user_api_keys_controller_spec.rb @@ -0,0 +1,133 @@ +require 'rails_helper' + +describe UserApiKeysController do + + let :public_key do + <