From 348e2e3ef2df3511e30b5b54d5b72cf6e780cfe5 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 22 Oct 2013 15:53:08 -0400 Subject: [PATCH] Support for per-user API keys --- .../admin/controllers/admin_api_controller.js | 67 +++++++++++++++ .../controllers/admin_user_controller.js | 22 +++++ .../javascripts/admin/models/admin_api.js | 29 ------- .../javascripts/admin/models/admin_user.js | 28 +++++++ .../javascripts/admin/models/api_key.js | 81 +++++++++++++++++++ .../admin/routes/admin_api_route.js | 2 +- .../admin/templates/api.js.handlebars | 42 +++++++--- .../admin/templates/user.js.handlebars | 19 +++++ .../stylesheets/common/admin/admin_base.scss | 29 +++++++ app/controllers/admin/api_controller.rb | 26 +++++- app/controllers/admin/users_controller.rb | 26 +++++- app/controllers/application_controller.rb | 2 +- app/models/api_key.rb | 22 +++++ app/models/site_setting.rb | 10 --- app/models/user.rb | 15 ++++ .../admin_detailed_user_serializer.rb | 5 ++ app/serializers/api_key_serializer.rb | 12 +++ config/locales/client.en.yml | 14 +++- config/locales/server.cs.yml | 1 - config/locales/server.de.yml | 1 - config/locales/server.en.yml | 1 - config/locales/server.fr.yml | 1 - config/locales/server.it.yml | 1 - config/locales/server.ko.yml | 1 - config/locales/server.nl.yml | 1 - config/locales/server.pseudo.yml | 2 - config/locales/server.pt_BR.yml | 1 - config/locales/server.ru.yml | 1 - config/locales/server.sv.yml | 1 - config/locales/server.zh_CN.yml | 5 +- config/locales/server.zh_TW.yml | 1 - config/routes.rb | 6 +- db/migrate/20131022151218_create_api_keys.rb | 16 ++++ lib/auth/default_current_user_provider.rb | 13 ++- lib/tasks/api.rake | 6 +- spec/controllers/admin/api_controller_spec.rb | 56 +++++++++++++ .../admin/users_controller_spec.rb | 20 +++++ .../application_controller_spec.rb | 10 ++- spec/fabricators/api_key_fabricator.rb | 3 + spec/models/api_key_spec.rb | 16 ++++ spec/models/user_spec.rb | 51 ++++++++++++ test/fixtures/api_keys.yml | 9 +++ .../admin/models/admin_user_test.js | 30 +++++++ test/javascripts/admin/models/api_key_test.js | 45 +++++++++++ test/unit/api_key_test.rb | 7 ++ 45 files changed, 670 insertions(+), 87 deletions(-) create mode 100644 app/assets/javascripts/admin/controllers/admin_api_controller.js delete mode 100644 app/assets/javascripts/admin/models/admin_api.js create mode 100644 app/assets/javascripts/admin/models/api_key.js create mode 100644 app/models/api_key.rb create mode 100644 app/serializers/api_key_serializer.rb create mode 100644 db/migrate/20131022151218_create_api_keys.rb create mode 100644 spec/controllers/admin/api_controller_spec.rb create mode 100644 spec/fabricators/api_key_fabricator.rb create mode 100644 spec/models/api_key_spec.rb create mode 100644 test/fixtures/api_keys.yml create mode 100644 test/javascripts/admin/models/admin_user_test.js create mode 100644 test/javascripts/admin/models/api_key_test.js create mode 100644 test/unit/api_key_test.rb diff --git a/app/assets/javascripts/admin/controllers/admin_api_controller.js b/app/assets/javascripts/admin/controllers/admin_api_controller.js new file mode 100644 index 000000000..612d81d3e --- /dev/null +++ b/app/assets/javascripts/admin/controllers/admin_api_controller.js @@ -0,0 +1,67 @@ +/** + This controller supports the interface for dealing with API keys + + @class AdminApiController + @extends Ember.ArrayController + @namespace Discourse + @module Discourse +**/ +Discourse.AdminApiController = Ember.ArrayController.extend({ + + actions: { + /** + Generates a master api key + + @method generateMasterKey + @param {Discourse.ApiKey} the key to regenerate + **/ + generateMasterKey: function(key) { + var self = this; + Discourse.ApiKey.generateMasterKey().then(function (key) { + self.get('model').pushObject(key); + }); + }, + + /** + Creates an API key instance with internal user object + + @method regenerateKey + @param {Discourse.ApiKey} the key to regenerate + **/ + regenerateKey: function(key) { + bootbox.confirm(I18n.t("admin.api.confirm_regen"), I18n.t("no_value"), I18n.t("yes_value"), function(result) { + if (result) { + key.regenerate(); + } + }); + }, + + /** + Revokes an API key + + @method revokeKey + @param {Discourse.ApiKey} the key to revoke + **/ + revokeKey: function(key) { + var self = this; + bootbox.confirm(I18n.t("admin.api.confirm_revoke"), I18n.t("no_value"), I18n.t("yes_value"), function(result) { + if (result) { + key.revoke().then(function() { + self.get('model').removeObject(key); + }); + } + }); + } + }, + + /** + Has a master key already been generated? + + @property hasMasterKey + @type {Boolean} + **/ + hasMasterKey: function() { + return !!this.get('model').findBy('user', null); + }.property('model.@each') + +}); diff --git a/app/assets/javascripts/admin/controllers/admin_user_controller.js b/app/assets/javascripts/admin/controllers/admin_user_controller.js index 986e5129c..70a3d520e 100644 --- a/app/assets/javascripts/admin/controllers/admin_user_controller.js +++ b/app/assets/javascripts/admin/controllers/admin_user_controller.js @@ -27,6 +27,28 @@ Discourse.AdminUserController = Discourse.ObjectController.extend({ }); this.send('toggleTitleEdit'); + }, + + generateApiKey: function() { + this.get('model').generateApiKey(); + }, + + regenerateApiKey: function() { + var self = this; + bootbox.confirm(I18n.t("admin.api.confirm_regen"), I18n.t("no_value"), I18n.t("yes_value"), function(result) { + if (result) { + self.get('model').generateApiKey(); + } + }); + }, + + revokeApiKey: function() { + var self = this; + bootbox.confirm(I18n.t("admin.api.confirm_revoke"), I18n.t("no_value"), I18n.t("yes_value"), function(result) { + if (result) { + self.get('model').revokeApiKey(); + } + }); } } diff --git a/app/assets/javascripts/admin/models/admin_api.js b/app/assets/javascripts/admin/models/admin_api.js deleted file mode 100644 index b89c9a7eb..000000000 --- a/app/assets/javascripts/admin/models/admin_api.js +++ /dev/null @@ -1,29 +0,0 @@ -Discourse.AdminApi = Discourse.Model.extend({ - VALID_KEY_LENGTH: 64, - - keyExists: function(){ - var key = this.get('key') || ''; - return key && key.length === this.VALID_KEY_LENGTH; - }.property('key'), - - generateKey: function(){ - var adminApi = this; - Discourse.ajax('/admin/api/generate_key', {type: 'POST'}).then(function (result) { - adminApi.set('key', result.key); - }); - }, - - regenerateKey: function(){ - alert(I18n.t('not_implemented')); - } -}); - -Discourse.AdminApi.reopenClass({ - find: function() { - var model = Discourse.AdminApi.create(); - Discourse.ajax("/admin/api").then(function(data) { - model.setProperties(data); - }); - return model; - } -}); diff --git a/app/assets/javascripts/admin/models/admin_user.js b/app/assets/javascripts/admin/models/admin_user.js index 6ce899743..58f171df1 100644 --- a/app/assets/javascripts/admin/models/admin_user.js +++ b/app/assets/javascripts/admin/models/admin_user.js @@ -8,6 +8,34 @@ **/ Discourse.AdminUser = Discourse.User.extend({ + /** + Generates an API key for the user. Will regenerate if they already have one. + + @method generateApiKey + @returns {Promise} a promise that resolves to the newly generated API key + **/ + generateApiKey: function() { + var self = this; + return Discourse.ajax("/admin/users/" + this.get('id') + "/generate_api_key", {type: 'POST'}).then(function (result) { + var apiKey = Discourse.ApiKey.create(result.api_key); + self.set('api_key', apiKey); + return apiKey; + }); + }, + + /** + Revokes a user's current API key + + @method revokeApiKey + @returns {Promise} a promise that resolves when the API key has been deleted + **/ + revokeApiKey: function() { + var self = this; + return Discourse.ajax("/admin/users/" + this.get('id') + "/revoke_api_key", {type: 'DELETE'}).then(function (result) { + self.set('api_key', null); + }); + }, + deleteAllPosts: function() { this.set('can_delete_all_posts', false); var user = this; diff --git a/app/assets/javascripts/admin/models/api_key.js b/app/assets/javascripts/admin/models/api_key.js new file mode 100644 index 000000000..a7cb6f2cd --- /dev/null +++ b/app/assets/javascripts/admin/models/api_key.js @@ -0,0 +1,81 @@ +/** + Our data model for representing an API key in the system + + @class ApiKey + @extends Discourse.Model + @namespace Discourse + @module Discourse +**/ +Discourse.ApiKey = Discourse.Model.extend({ + + /** + Regenerates the api key + + @method regenerate + @returns {Promise} a promise that resolves to the key + **/ + regenerate: function() { + var self = this; + return Discourse.ajax('/admin/api/key', {type: 'PUT', data: {id: this.get('id')}}).then(function (result) { + self.set('key', result.api_key.key); + return self; + }); + }, + + /** + Revokes the current key + + @method revoke + @returns {Promise} a promise that resolves when the key has been revoked + **/ + revoke: function() { + var self = this; + return Discourse.ajax('/admin/api/key', {type: 'DELETE', data: {id: this.get('id')}}); + } + +}); + +Discourse.ApiKey.reopenClass({ + + /** + Creates an API key instance with internal user object + + @method create + @param {Object} the properties to create + @returns {Discourse.ApiKey} the ApiKey instance + **/ + create: function(apiKey) { + var result = this._super(apiKey); + if (result.user) { + result.user = Discourse.AdminUser.create(result.user); + } + return result; + }, + + /** + Finds a list of API keys + + @method find + @returns {Promise} a promise that resolves to the array of `Discourse.ApiKey` instances + **/ + find: function() { + return Discourse.ajax("/admin/api").then(function(keys) { + return keys.map(function (key) { + return Discourse.ApiKey.create(key); + }); + }); + }, + + /** + Generates a master api key and returns it. + + @method generateMasterKey + @returns {Promise} a promise that resolves to a master `Discourse.ApiKey` + **/ + generateMasterKey: function() { + return Discourse.ajax("/admin/api/key", {type: 'POST'}).then(function (result) { + return Discourse.ApiKey.create(result.api_key); + }); + } + +}); diff --git a/app/assets/javascripts/admin/routes/admin_api_route.js b/app/assets/javascripts/admin/routes/admin_api_route.js index 2da894f59..e13085fa1 100644 --- a/app/assets/javascripts/admin/routes/admin_api_route.js +++ b/app/assets/javascripts/admin/routes/admin_api_route.js @@ -9,7 +9,7 @@ Discourse.AdminApiRoute = Discourse.Route.extend({ model: function() { - return Discourse.AdminApi.find(); + return Discourse.ApiKey.find(); } }); diff --git a/app/assets/javascripts/admin/templates/api.js.handlebars b/app/assets/javascripts/admin/templates/api.js.handlebars index 2603c05ca..c5cb5616b 100644 --- a/app/assets/javascripts/admin/templates/api.js.handlebars +++ b/app/assets/javascripts/admin/templates/api.js.handlebars @@ -1,13 +1,33 @@ -

{{i18n admin.api.long_title}}

-{{#if keyExists}} - {{i18n admin.api.key}}: {{key}} - -

{{{i18n admin.api.note_html}}}

+{{#if model}} + + + + + + + {{#each model}} + + + + + + {{/each}} +
{{i18n admin.api.key}}{{i18n admin.api.user}} 
{{key}} + {{#if user}} + {{#link-to 'adminUser' user}} + {{avatar user imageSize="small"}} + {{/link-to}} + {{else}} + {{i18n admin.api.all_users}} + {{/if}} + + + +
{{else}} -

{{{i18n admin.api.info_html}}}

- +

{{i18n admin.api.none}}

{{/if}} + +{{#unless hasMasterKey}} + +{{/unless }} diff --git a/app/assets/javascripts/admin/templates/user.js.handlebars b/app/assets/javascripts/admin/templates/user.js.handlebars index 807fb398e..03255d817 100644 --- a/app/assets/javascripts/admin/templates/user.js.handlebars +++ b/app/assets/javascripts/admin/templates/user.js.handlebars @@ -125,6 +125,25 @@ +
+
{{i18n admin.api.key}}
+ + {{#if api_key}} +
+ {{api_key.key}} + + +
+ {{else}} +
+ — +
+
+ +
+ {{/if}} +
+
{{i18n admin.user.admin}}
{{admin}}
diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss index 7f3b67c89..7774772c8 100644 --- a/app/assets/stylesheets/common/admin/admin_base.scss +++ b/app/assets/stylesheets/common/admin/admin_base.scss @@ -206,6 +206,17 @@ table { float: left; margin-left: 12px; } + .long-value { + width: 800px; + float: left; + margin-left: 12px; + font-size: 13px; + + button { + margin-left: 10px; + } + } + .controls { .btn { margin-right: 5px; @@ -374,6 +385,24 @@ table { } } +table.api-keys { + margin-top: 10px; + width: 100%; + + th { + text-align: left; + padding: 5px; + } + + td { + padding: 5px; + } + + td.key { + font-size: 12px; + } +} + .dashboard-stats { margin-bottom: 30px; width: 460px; diff --git a/app/controllers/admin/api_controller.rb b/app/controllers/admin/api_controller.rb index d55c13c22..7861b2cfc 100644 --- a/app/controllers/admin/api_controller.rb +++ b/app/controllers/admin/api_controller.rb @@ -1,10 +1,28 @@ class Admin::ApiController < Admin::AdminController + def index - render json: {key: SiteSetting.api_key} + render_serialized(ApiKey.all, ApiKeySerializer) end - def generate_key - SiteSetting.generate_api_key! - render json: {key: SiteSetting.api_key} + def regenerate_key + api_key = ApiKey.where(id: params[:id]).first + raise Discourse::NotFound.new if api_key.blank? + + api_key.regenerate!(current_user) + render_serialized(api_key, ApiKeySerializer) end + + def revoke_key + api_key = ApiKey.where(id: params[:id]).first + raise Discourse::NotFound.new if api_key.blank? + + api_key.destroy + render nothing: true + end + + def create_master_key + api_key = ApiKey.create_master_key + render_serialized(api_key, ApiKeySerializer) + end + end diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 736a5d281..cbb654645 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -4,7 +4,21 @@ require_dependency 'boost_trust_level' class Admin::UsersController < Admin::AdminController - before_filter :fetch_user, only: [:ban, :unban, :refresh_browsers, :revoke_admin, :grant_admin, :revoke_moderation, :grant_moderation, :approve, :activate, :deactivate, :block, :unblock, :trust_level] + before_filter :fetch_user, only: [:ban, + :unban, + :refresh_browsers, + :revoke_admin, + :grant_admin, + :revoke_moderation, + :grant_moderation, + :approve, + :activate, + :deactivate, + :block, + :unblock, + :trust_level, + :generate_api_key, + :revoke_api_key] def index query = ::AdminUserIndexQuery.new(params) @@ -52,6 +66,16 @@ class Admin::UsersController < Admin::AdminController render nothing: true end + def generate_api_key + api_key = @user.generate_api_key(current_user) + render_serialized(api_key, ApiKeySerializer) + end + + def revoke_api_key + @user.revoke_api_key + render nothing: true + end + def grant_admin guardian.ensure_can_grant_admin!(@user) @user.grant_admin! diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3715481ce..5a7dad77f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -279,7 +279,7 @@ class ApplicationController < ActionController::Base protected def api_key_valid? - request["api_key"] && SiteSetting.api_key_valid?(request["api_key"]) + request["api_key"] && ApiKey.where(key: request["api_key"]).exists? end # returns an array of integers given a param key diff --git a/app/models/api_key.rb b/app/models/api_key.rb new file mode 100644 index 000000000..98eadd5c0 --- /dev/null +++ b/app/models/api_key.rb @@ -0,0 +1,22 @@ +class ApiKey < ActiveRecord::Base + belongs_to :user + belongs_to :created_by, class_name: User + + validates_presence_of :key + validates_uniqueness_of :user_id + + def regenerate!(updated_by) + self.key = SecureRandom.hex(32) + self.created_by = updated_by + save! + end + + def self.create_master_key + api_key = ApiKey.where('user_id IS NULL').first + if api_key.blank? + api_key = ApiKey.create(key: SecureRandom.hex(32), created_by: Discourse.system_user) + end + api_key + end + +end diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index ff32a727c..800126207 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -18,7 +18,6 @@ class SiteSetting < ActiveRecord::Base client_setting(:tos_url, '') client_setting(:faq_url, '') client_setting(:privacy_policy_url, '') - setting(:api_key, '') client_setting(:traditional_markdown_linebreaks, false) client_setting(:top_menu, 'latest|new|unread|favorited|categories') client_setting(:post_menu, 'like|edit|flag|delete|share|bookmark|reply') @@ -273,15 +272,6 @@ class SiteSetting < ActiveRecord::Base setting(:dominating_topic_minimum_percent, 20) - def self.generate_api_key! - self.api_key = SecureRandom.hex(32) - end - - def self.api_key_valid?(tested) - t = tested.strip - t.length == 64 && t == self.api_key - end - def self.call_discourse_hub? self.enforce_global_nicknames? && self.discourse_org_access_key.present? end diff --git a/app/models/user.rb b/app/models/user.rb index 74fc339b1..66e58ea68 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -43,6 +43,7 @@ class User < ActiveRecord::Base has_many :secure_categories, through: :groups, source: :categories has_one :user_search_data, dependent: :destroy + has_one :api_key, dependent: :destroy belongs_to :uploaded_avatar, class_name: 'Upload', dependent: :destroy @@ -479,6 +480,19 @@ class User < ActiveRecord::Base self.save! end + def generate_api_key(created_by) + if api_key.present? + api_key.regenerate!(created_by) + api_key + else + ApiKey.create(user: self, key: SecureRandom.hex(32), created_by: created_by) + end + end + + def revoke_api_key + ApiKey.where(user_id: self.id).delete_all + end + protected def cook @@ -567,6 +581,7 @@ class User < ActiveRecord::Base end end + private end diff --git a/app/serializers/admin_detailed_user_serializer.rb b/app/serializers/admin_detailed_user_serializer.rb index fad8927db..268d806f2 100644 --- a/app/serializers/admin_detailed_user_serializer.rb +++ b/app/serializers/admin_detailed_user_serializer.rb @@ -16,6 +16,7 @@ class AdminDetailedUserSerializer < AdminUserSerializer :can_be_deleted has_one :approved_by, serializer: BasicUserSerializer, embed: :objects + has_one :api_key, serializer: ApiKeySerializer, embed: :objects def can_revoke_admin scope.can_revoke_admin?(object) @@ -49,4 +50,8 @@ class AdminDetailedUserSerializer < AdminUserSerializer object.topics.count end + def include_api_key? + api_key.present? + end + end diff --git a/app/serializers/api_key_serializer.rb b/app/serializers/api_key_serializer.rb new file mode 100644 index 000000000..2fe1bb757 --- /dev/null +++ b/app/serializers/api_key_serializer.rb @@ -0,0 +1,12 @@ +class ApiKeySerializer < ApplicationSerializer + + attributes :id, + :key + + has_one :user, serializer: BasicUserSerializer, embed: :objects + + def include_user_id? + !object.user_id.nil? + end + +end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index f07144cc1..6ebb86129 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1169,12 +1169,18 @@ en: delete_failed: "Unable to delete group. If this is an automatic group, it cannot be destroyed." api: + generate_master: "Generate Master API Key" + none: "There are no active API keys right now." + user: "User" title: "API" - long_title: "API Information" - key: "Key" - generate: "Generate API Key" - regenerate: "Regenerate API Key" + key: "API Key" + generate: "Generate" + regenerate: "Regenerate" + revoke: "Revoke" + confirm_regen: "Are you sure you want to replace that API Key with a new one?" + confirm_revoke: "Are you sure you want to revoke that key?" info_html: "Your API key will allow you to create and update topics using JSON calls." + all_users: "All Users" note_html: "Keep this key secret, all users that have it may create arbitrary posts on the forum as any user." customize: diff --git a/config/locales/server.cs.yml b/config/locales/server.cs.yml index 7ce109e87..3919deb65 100644 --- a/config/locales/server.cs.yml +++ b/config/locales/server.cs.yml @@ -493,7 +493,6 @@ cs: company_full_name: "Plné jméno společnosti, která provozuje tento web, používá se v dokumentech jako je /tos" company_short_name: "Krátké jméno společnosti, která provozuje tento web, používá se v dokumentech jako je /tos" company_domain: "Doménové jméno vlastněné společností, která provozuje tento web, používá se v dokumentech jako je /tos" - api_key: "Zabezpečený API klíč, který se používá pro vytváření a aktualizaci témat, použijte sekci /admin/api k nastavení" queue_jobs: "Zařazovat úlohy do fronty v sidekiq, není-li nastaveno, jsou úlohy vyřizovány okamžitě" crawl_images: "Povolit získávání obrázků z webů třetích stran" ninja_edit_window: "Jak rychle smíte udělat změnu, aniž by se uložila jako nová verze" diff --git a/config/locales/server.de.yml b/config/locales/server.de.yml index 66464a903..5c6a088b6 100644 --- a/config/locales/server.de.yml +++ b/config/locales/server.de.yml @@ -463,7 +463,6 @@ de: company_full_name: "Voller Name des Unternehmens, das diese Seite betreibt. Wird in rechtlich relevanten Dokumenten wie den Nutzungsbestimmungen (/tos) verwendet." company_short_name: "Kurzname des Unternehmens, das diese Seite betreibt. Wird in rechtlich relevanten Dokumenten wie den Nutzungsbestimmungen (/tos) verwendet." company_domain: "Domainname des Unternehmens, das diese Seite betreibt. Wird in rechtlich relevanten Dokumenten wie den Nutzungsbestimmungen (/tos) verwendet." - api_key: "Sicherer API-Schlüssel, um Themen zu erstellen und zu aktualisieren. Benutze /admin/api, um ihn einzurichten." queue_jobs: "Benutze die Sidekiq-Queue, falls falsche Queues inline sind." crawl_images: "Lade Bilder von Dritten herunter, um ihre Höhe und Breite zu bestimmen." ninja_edit_window: "Sekunden nach Empfang eines Beitrag, in denen Bearbeitungen nicht als neue Version gelten." diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 7a8655e36..9f6cec0fd 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -495,7 +495,6 @@ en: company_full_name: "The full name of the company that runs this site, used in legal documents like the /tos" company_short_name: "The short name of the company that runs this site, used in legal documents like the /tos" company_domain: "The domain name owned by the company that runs this site, used in legal documents like the /tos" - api_key: "The secure API key used to create and update topics, use the /admin/api section to set it up" queue_jobs: "DEVELOPER ONLY! WARNING! By default, queue jobs in sidekiq. If disabled, your site will be broken." crawl_images: "Enable retrieving images from third party sources to insert width and height dimensions" ninja_edit_window: "Number of seconds after posting where edits do not create a new version" diff --git a/config/locales/server.fr.yml b/config/locales/server.fr.yml index f915b3abf..c8501fc25 100644 --- a/config/locales/server.fr.yml +++ b/config/locales/server.fr.yml @@ -460,7 +460,6 @@ fr: company_full_name: "Le nom complet de la société qui gère ce site, utilisé dans les documents légaux, tels que /tos" company_short_name: "Le nom de la société qui gère ce site, utilisé dans les documents légaux, tels que /tos" company_domain: "Le nom de domaine de la société qui gère ce site, utilisé dans les documents légaux, tels que /tos" - api_key: "La clé API sécurisé à utiliser pour créer et mettre à jour des discussions. Utilisez la section /admin/api pour la configurer." queue_jobs: "DÉVELOPPEURS SEULEMENT ! ATTENTION ! Par défaut, empiler les travaux dans sidekiq. Si désactivé, votre site sera cassé." crawl_images: "Permettre aux images provenant de sources tierces d'insérer la hauteur et la largeur de celles-ci" ninja_edit_window: "Temps d'édition avant de sauvegarder une nouvelle version, en secondes." diff --git a/config/locales/server.it.yml b/config/locales/server.it.yml index e330a40ac..ef70f655e 100644 --- a/config/locales/server.it.yml +++ b/config/locales/server.it.yml @@ -423,7 +423,6 @@ it: company_full_name: "Il nome completo di chi gestisce il sito, usato in documenti legali come /tos" company_short_name: "Il nome abbreviato di chi gestisce il sito, usato in documenti legali come /tos" company_domain: "Il dominio di chi gestisce il sito, usato in documenti legali come /tos" - api_key: "La chiave API segreta usata per creare e aggiornare topic, usa la sezione /admin/api per impostarla" queue_jobs: "Metti in coda diversi job in sidekiq, se false le code sono inline" crawl_images: "Abilita la ricezione di immagini da sorgenti terze parti" ninja_edit_window: "Numero di secondi trascorsi affinché una modifica del post appena inviato, non venga considerata come nuova revisione" diff --git a/config/locales/server.ko.yml b/config/locales/server.ko.yml index 3da6cc507..1fdd99e83 100644 --- a/config/locales/server.ko.yml +++ b/config/locales/server.ko.yml @@ -417,7 +417,6 @@ ko: company_full_name: "The full name of the company that runs this site, used in legal documents like the /tos" company_short_name: "The short name of the company that runs this site, used in legal documents like the /tos" company_domain: "The domain name owned by the company that runs this site, used in legal documents like the /tos" - api_key: "The secure API key used to create and update topics, use the /admin/api section to set it up" access_password: "When restricted access is enabled, this password must be entered" queue_jobs: "Queue various jobs in sidekiq, if false queues are inline" crawl_images: "Enable retrieving images from third party sources to insert width and height dimensions" diff --git a/config/locales/server.nl.yml b/config/locales/server.nl.yml index 8420c8aa9..e4fdbac8f 100644 --- a/config/locales/server.nl.yml +++ b/config/locales/server.nl.yml @@ -465,7 +465,6 @@ nl: company_full_name: "De volledige naam van het bedrijf dat deze site draait. Wordt gebruikt in juridische delen van de site, zoals /tos" company_short_name: "De korte naam van het bedrijf dat deze site draait. Wordt gebruikt in juridische delen van de site, zoals /tos" company_domain: "De domeinnaam van het bedrijf dat deze site draait. Wordt gebruikt in juridische delen van de site, zoals /tos" - api_key: "De beveiligde API-sleutel wordt gebruikt om topics te maken en bij te werken. Gebruik /admin/api om deze in te stellen" queue_jobs: "DEVELOPERS ONLY! WARNING! Zet verschillende taken in een queue binnen sidekiq, bij 'false' worden taken ineens uitgevoerd" crawl_images: Zet het ophalen van afbeeldingen van externe bronnen aan ninja_edit_window: "Hoe snel je een aanpassing kan maken zonder dat er een nieuwe versie wordt opgeslagen, in seconden." diff --git a/config/locales/server.pseudo.yml b/config/locales/server.pseudo.yml index 5c3b91450..a24e1ec58 100644 --- a/config/locales/server.pseudo.yml +++ b/config/locales/server.pseudo.yml @@ -543,8 +543,6 @@ pseudo: íɳ łéǧáł ďóčůɱéɳťš łíǩé ťĥé /ťóš ]]' company_domain: '[[ Ťĥé ďóɱáíɳ ɳáɱé óŵɳéď ƀý ťĥé čóɱƿáɳý ťĥáť řůɳš ťĥíš šíťé, ůšéď íɳ łéǧáł ďóčůɱéɳťš łíǩé ťĥé /ťóš ]]' - api_key: '[[ Ťĥé šéčůřé ÁРÍ ǩéý ůšéď ťó čřéáťé áɳď ůƿďáťé ťóƿíčš, ůšé ťĥé /áďɱíɳ/áƿí - šéčťíóɳ ťó šéť íť ůƿ ]]' queue_jobs: '[[ Ƣůéůé νáříóůš ʲóƀš íɳ šíďéǩíƣ, íƒ ƒáłšé ƣůéůéš ářé íɳłíɳé ]]' crawl_images: '[[ Éɳáƀłé řéťříéνíɳǧ íɱáǧéš ƒřóɱ ťĥířď ƿářťý šóůřčéš ťó íɳšéřť ŵíďťĥ áɳď ĥéíǧĥť ďíɱéɳšíóɳš ]]' diff --git a/config/locales/server.pt_BR.yml b/config/locales/server.pt_BR.yml index 5c9728977..51f27661d 100644 --- a/config/locales/server.pt_BR.yml +++ b/config/locales/server.pt_BR.yml @@ -489,7 +489,6 @@ pt_BR: company_full_name: "Nome completo da companhia que mantém este site, usada nos documentos legais como o /tos" company_short_name: "Nome curto da companhia que mantém este site, usada nos documentos legais como o /tos" company_domain: "Nome de domínio pertencente a companhia que mantém este site, usada nos documentos legais como o /tos" - api_key: "A chave de API segura usada para criar e modificar tópicos, acesse a seção /admin/api para defini-lá" queue_jobs: "APENAS DESENVOLVEDORES! ATENÇÃO! Por padrão, enfileira tarefas no sidekiq. Se desativado, seu site ficará defeituoso." crawl_images: "permitir mostrar imagens de sites terceiros" ninja_edit_window: "quão rápido é possivél fazer uma alteração sem guardar uma nova versão, em segundos." diff --git a/config/locales/server.ru.yml b/config/locales/server.ru.yml index a6cfba635..ddfc0b385 100644 --- a/config/locales/server.ru.yml +++ b/config/locales/server.ru.yml @@ -484,7 +484,6 @@ ru: company_full_name: 'Полное название компании, которой принадлежит сайт, используется в правовой документации как /tos' company_short_name: 'Короткое название компании, которой принадлежит сайт, используется в правовой документации как /tos' company_domain: 'Имя домена, принадлежащего компании, заведующей сайтом, используется в правовой документации как /tos' - api_key: 'Секретный API ключ, используемый для создания и обновления тем. Зайдите в секцию /admin/api , чтобы его задать' queue_jobs: 'ТОЛЬКО ДЛЯ РАЗРАБОТЧИКОВ! ВНИМАНИЕ! По умолчанию задачи обрабатываются асинхронно в очереди sidekiq. Если настройка выключена, ваш сайт может не работать.' crawl_images: 'Разрешить извлечение изображений из сторонних источников, ширина и высота' ninja_edit_window: 'Количество секунд после размещения сообщения, в течение которых внесение правок в сообщение не повлечет его изменение' diff --git a/config/locales/server.sv.yml b/config/locales/server.sv.yml index e0a2982ef..ed9bf4c30 100644 --- a/config/locales/server.sv.yml +++ b/config/locales/server.sv.yml @@ -346,7 +346,6 @@ sv: company_full_name: "Det fullständiga namnet för företaget som driver denna webbplats, används i juridiska dokument så som /tos" company_short_name: "Det korta namnet för företaget som driver denna webbplats, används i juridiska dokument så som /tos" company_domain: "Domännamnet som ägs av företaget som driver denna webbplats, används i juridiska dokument så som /tos" - api_key: "Den säkra API-nyckeln som används för att skapa och uppdatera trådar, använd /admin/api för att skapa en" queue_jobs: "Köa diverse jobb i sidekiq, om urkryssat så körs köer infogat" crawl_images: "Aktivera hämtning av bilder från tredjepartskällor för att infoga bredd och höjd" ninja_edit_window: "Antal sekunder efter ett inlägg när en ändring inte skapar en ny version" diff --git a/config/locales/server.zh_CN.yml b/config/locales/server.zh_CN.yml index bf41fadfd..3d1ab05b2 100644 --- a/config/locales/server.zh_CN.yml +++ b/config/locales/server.zh_CN.yml @@ -119,8 +119,8 @@ zh_CN: replace_paragraph: "[用一段简短的分类描述替换此第一段内容，请不要超过200个字符。]" post_template: "%{replace_paragraph}\n\n使用下面的空间输入分类的详细描述信息，可包括在此分类下讨论的规则、内容导向等等。" this_year: "今年" - - + + trust_levels: newuser: title: "访客" @@ -442,7 +442,6 @@ zh_CN: company_full_name: "运行本站点的公司全称，用于法律文档，例如服务条款 /tos" company_short_name: "运行本站点的公司短名，用于法律文档，例如服务条款 /tos" company_domain: "运行本站点的公司域名，用于法律文档，例如服务条款 /tos" - api_key: "加密的应用开发接口密钥（API key），用于创建和更新主题。使用 /admin/api 来对它进行设置。" queue_jobs: "如果失败队列在排队，使用 Sidekiq 消息引擎对不同的工作排队" crawl_images: "允许从第三方获取图片来插入宽、高数值" ninja_edit_window: "在多少秒钟之内，对帖子的多次编辑不生成新版本" diff --git a/config/locales/server.zh_TW.yml b/config/locales/server.zh_TW.yml index 5ee9994d7..485a6f4de 100644 --- a/config/locales/server.zh_TW.yml +++ b/config/locales/server.zh_TW.yml @@ -423,7 +423,6 @@ zh_TW: company_full_name: "運行本站點的公司全稱，用于法律文檔，例如服務條款 /tos" company_short_name: "運行本站點的公司短名，用于法律文檔，例如服務條款 /tos" company_domain: "運行本站點的公司域名，用于法律文檔，例如服務條款 /tos" - api_key: "加密的應用開發接口密鑰（API key），用于創建和更新主題。使用 /admin/api 來對它進行設置。" queue_jobs: "如果失敗隊列在排隊，使用 Sidekiq 消息引擎對不同的工作排隊" crawl_images: "允許從第三方獲取圖片來插入寬、高數值" ninja_edit_window: "在多少秒鍾之內，對帖子的多次編輯不生成新版本" diff --git a/config/routes.rb b/config/routes.rb index 38e17c294..583c9dcfc 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -43,6 +43,8 @@ Discourse::Application.routes.draw do put 'unban' put 'revoke_admin', constraints: AdminConstraint.new put 'grant_admin', constraints: AdminConstraint.new + post 'generate_api_key', constraints: AdminConstraint.new + delete 'revoke_api_key', constraints: AdminConstraint.new put 'revoke_moderation', constraints: AdminConstraint.new put 'grant_moderation', constraints: AdminConstraint.new put 'approve' @@ -89,7 +91,9 @@ Discourse::Application.routes.draw do end resources :api, only: [:index], constraints: AdminConstraint.new do collection do - post 'generate_key' + post 'key' => 'api#create_master_key' + put 'key' => 'api#regenerate_key' + delete 'key' => 'api#revoke_key' end end end diff --git a/db/migrate/20131022151218_create_api_keys.rb b/db/migrate/20131022151218_create_api_keys.rb new file mode 100644 index 000000000..684eb65c3 --- /dev/null +++ b/db/migrate/20131022151218_create_api_keys.rb @@ -0,0 +1,16 @@ +class CreateApiKeys < ActiveRecord::Migration + def change + create_table :api_keys do |t| + t.string :key, limit: 64, null: false + t.integer :user_id, null: true + t.integer :created_by_id + t.timestamps + end + + add_index :api_keys, :key + add_index :api_keys, :user_id, unique: true + + execute "INSERT INTO api_keys (key, created_at, updated_at) SELECT value, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP FROM site_settings WHERE name = 'api_key'" + execute "DELETE FROM site_settings WHERE name = 'api_key'" + end +end diff --git a/lib/auth/default_current_user_provider.rb b/lib/auth/default_current_user_provider.rb index 5a4e5df4e..217132c4c 100644 --- a/lib/auth/default_current_user_provider.rb +++ b/lib/auth/default_current_user_provider.rb @@ -38,12 +38,17 @@ class Auth::DefaultCurrentUserProvider # possible we have an api call, impersonate unless current_user - if api_key = request["api_key"] - if api_username = request["api_username"] - if SiteSetting.api_key_valid?(api_key) - @env[API_KEY] = true + if api_key_value = request["api_key"] + api_key = ApiKey.where(key: api_key_value).includes(:user).first + if api_key.present? + @env[API_KEY] = true + + if api_key.user.present? + current_user = api_key.user + elsif api_username = request["api_username"] current_user = User.where(username_lower: api_username.downcase).first end + end end end diff --git a/lib/tasks/api.rake b/lib/tasks/api.rake index d3279a8a5..ef42e8a7b 100644 --- a/lib/tasks/api.rake +++ b/lib/tasks/api.rake @@ -1,8 +1,6 @@ desc "generate api key if missing, return existing if already there" task "api_key:get" => :environment do - if SiteSetting.api_key.blank? - SiteSetting.generate_api_key! - end + api_key = ApiKey.create_master_key - puts SiteSetting.api_key + puts api_key.key end diff --git a/spec/controllers/admin/api_controller_spec.rb b/spec/controllers/admin/api_controller_spec.rb new file mode 100644 index 000000000..b7b61f2ad --- /dev/null +++ b/spec/controllers/admin/api_controller_spec.rb @@ -0,0 +1,56 @@ +require 'spec_helper' + +describe Admin::ApiController do + + it "is a subclass of AdminController" do + (Admin::ApiController < Admin::AdminController).should be_true + end + + let!(:user) { log_in(:admin) } + + context '.index' do + it "succeeds" do + xhr :get, :index + response.should be_success + end + end + + context '.regenerate_key' do + let(:api_key) { Fabricate(:api_key) } + + it "returns 404 when there is no key" do + xhr :put, :regenerate_key, id: 1234 + response.should_not be_success + response.status.should == 404 + end + + it "delegates to the api key's `regenerate!` method" do + ApiKey.any_instance.expects(:regenerate!) + xhr :put, :regenerate_key, id: api_key.id + end + end + + context '.revoke_key' do + let(:api_key) { Fabricate(:api_key) } + + it "returns 404 when there is no key" do + xhr :delete, :revoke_key, id: 1234 + response.should_not be_success + response.status.should == 404 + end + + it "delegates to the api key's `regenerate!` method" do + ApiKey.any_instance.expects(:destroy) + xhr :delete, :revoke_key, id: api_key.id + end + end + + context '.create_master_key' do + it "creates a record" do + lambda { + xhr :post, :create_master_key + }.should change(ApiKey, :count).by(1) + end + end + +end diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index ea21dd90a..358aefbe2 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -62,6 +62,26 @@ describe Admin::UsersController do end + context '.generate_api_key' do + let(:evil_trout) { Fabricate(:evil_trout) } + + it 'calls generate_api_key' do + User.any_instance.expects(:generate_api_key).with(@user) + xhr :post, :generate_api_key, user_id: evil_trout.id + end + end + + context '.revoke_api_key' do + + let(:evil_trout) { Fabricate(:evil_trout) } + + it 'calls revoke_api_key' do + User.any_instance.expects(:revoke_api_key) + xhr :delete, :revoke_api_key, user_id: evil_trout.id + end + + end + context '.approve' do let(:evil_trout) { Fabricate(:evil_trout) } diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 512928331..10ae98389 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -15,10 +15,18 @@ describe 'api' do Fabricate(:post) end + let(:api_key) { user.generate_api_key(user) } + let(:master_key) { ApiKey.create_master_key } + # choosing an arbitrarily easy to mock trusted activity it 'allows users with api key to bookmark posts' do PostAction.expects(:act).with(user, post, PostActionType.types[:bookmark]).once - put :bookmark, bookmarked: "true", post_id: post.id, api_key: SiteSetting.api_key, api_username: user.username, format: :json + put :bookmark, bookmarked: "true", post_id: post.id, api_key: api_key.key, format: :json + end + + it 'allows users with a master api key to bookmark posts' do + PostAction.expects(:act).with(user, post, PostActionType.types[:bookmark]).once + put :bookmark, bookmarked: "true", post_id: post.id, api_key: master_key.key, api_username: user.username, format: :json end it 'disallows phonies to bookmark posts' do diff --git a/spec/fabricators/api_key_fabricator.rb b/spec/fabricators/api_key_fabricator.rb new file mode 100644 index 000000000..34f3efdb5 --- /dev/null +++ b/spec/fabricators/api_key_fabricator.rb @@ -0,0 +1,3 @@ +Fabricator(:api_key) do + key '1dfb7d427400cb8ef18052fd412781af134cceca5725dd74f34bbc6b9e35ddc9' +end \ No newline at end of file diff --git a/spec/models/api_key_spec.rb b/spec/models/api_key_spec.rb new file mode 100644 index 000000000..7df92e574 --- /dev/null +++ b/spec/models/api_key_spec.rb @@ -0,0 +1,16 @@ +# encoding: utf-8 +require 'spec_helper' +require_dependency 'api_key' + +describe ApiKey do + it { should belong_to :user } + it { should belong_to :created_by } + + it { should validate_presence_of :key } + + it 'validates uniqueness of user_id' do + Fabricate(:api_key) + should validate_uniqueness_of(:user_id) + end + +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f2ce46809..dbeebb1b4 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -863,4 +863,55 @@ describe User do expect(user.update_avatar(upload)).to be_true end end + + describe 'api keys' do + let(:admin) { Fabricate(:admin) } + let(:other_admin) { Fabricate(:admin) } + let(:user) { Fabricate(:user) } + + describe '.generate_api_key' do + + it "generates an api key when none exists, and regenerates when it does" do + expect(user.api_key).to be_blank + + # Generate a key + api_key = user.generate_api_key(admin) + expect(api_key.user).to eq(user) + expect(api_key.key).to be_present + expect(api_key.created_by).to eq(admin) + + user.reload + expect(user.api_key).to eq(api_key) + + # Regenerate a key. Keeps the same record, updates the key + new_key = user.generate_api_key(other_admin) + expect(new_key.id).to eq(api_key.id) + expect(new_key.key).to_not eq(api_key.key) + expect(new_key.created_by).to eq(other_admin) + end + + end + + describe '.revoke_api_key' do + + it "revokes an api key when exists" do + expect(user.api_key).to be_blank + + # Revoke nothing does nothing + user.revoke_api_key + user.reload + expect(user.api_key).to be_blank + + # When a key is present it is removed + user.generate_api_key(admin) + user.reload + user.revoke_api_key + user.reload + expect(user.api_key).to be_blank + end + + end + + end + end diff --git a/test/fixtures/api_keys.yml b/test/fixtures/api_keys.yml new file mode 100644 index 000000000..1f28ffbb0 --- /dev/null +++ b/test/fixtures/api_keys.yml @@ -0,0 +1,9 @@ +# Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/Fixtures.html + +one: + key: MyString + user_id: 1 + +two: + key: MyString + user_id: 1 diff --git a/test/javascripts/admin/models/admin_user_test.js b/test/javascripts/admin/models/admin_user_test.js new file mode 100644 index 000000000..172bc481f --- /dev/null +++ b/test/javascripts/admin/models/admin_user_test.js @@ -0,0 +1,30 @@ +module("Discourse.AdminUser"); + + +asyncTestDiscourse('generate key', function() { + this.stub(Discourse, 'ajax').returns(Ember.RSVP.resolve({api_key: {id: 1234, key: 'asdfasdf'}})); + + var adminUser = Discourse.AdminUser.create({id: 333}); + + blank(adminUser.get('api_key'), 'it has no api key by default'); + adminUser.generateApiKey().then(function() { + start(); + ok(Discourse.ajax.calledWith("/admin/users/333/generate_api_key", { type: 'POST' }), "it POSTed to the url"); + present(adminUser.get('api_key'), 'it has an api_key now'); + }); +}); + +asyncTestDiscourse('revoke key', function() { + + var apiKey = Discourse.ApiKey.create({id: 1234, key: 'asdfasdf'}), + adminUser = Discourse.AdminUser.create({id: 333, api_key: apiKey}); + + this.stub(Discourse, 'ajax').returns(Ember.RSVP.resolve()); + + equal(adminUser.get('api_key'), apiKey, 'it has the api key in the beginning'); + adminUser.revokeApiKey().then(function() { + start(); + ok(Discourse.ajax.calledWith("/admin/users/333/revoke_api_key", { type: 'DELETE' }), "it DELETEd to the url"); + blank(adminUser.get('api_key'), 'it cleared the api_key'); + }); +}); \ No newline at end of file diff --git a/test/javascripts/admin/models/api_key_test.js b/test/javascripts/admin/models/api_key_test.js new file mode 100644 index 000000000..1cb658a08 --- /dev/null +++ b/test/javascripts/admin/models/api_key_test.js @@ -0,0 +1,45 @@ +module("Discourse.ApiKey"); + +test('create', function() { + var apiKey = Discourse.ApiKey.create({id: 123, user: {id: 345}}); + + present(apiKey, 'it creates the api key'); + present(apiKey.get('user'), 'it creates the user inside'); +}); + + +asyncTestDiscourse('find', function() { + this.stub(Discourse, 'ajax').returns(Ember.RSVP.resolve([])); + Discourse.ApiKey.find().then(function() { + start(); + ok(Discourse.ajax.calledWith("/admin/api"), "it GETs the keys"); + }); +}); + +asyncTestDiscourse('generateMasterKey', function() { + this.stub(Discourse, 'ajax').returns(Ember.RSVP.resolve([])); + Discourse.ApiKey.generateMasterKey().then(function() { + start(); + ok(Discourse.ajax.calledWith("/admin/api/key", {type: 'POST'}), "it POSTs to create a master key"); + }); +}); + +asyncTestDiscourse('regenerate', function() { + var apiKey = Discourse.ApiKey.create({id: 3456}); + + this.stub(Discourse, 'ajax').returns(Ember.RSVP.resolve({api_key: {id: 3456}})); + apiKey.regenerate().then(function() { + start(); + ok(Discourse.ajax.calledWith("/admin/api/key", {type: 'PUT', data: {id: 3456}}), "it PUTs the key"); + }); +}); + +asyncTestDiscourse('revoke', function() { + var apiKey = Discourse.ApiKey.create({id: 3456}); + + this.stub(Discourse, 'ajax').returns(Ember.RSVP.resolve([])); + apiKey.revoke().then(function() { + start(); + ok(Discourse.ajax.calledWith("/admin/api/key", {type: 'DELETE', data: {id: 3456}}), "it DELETES the key"); + }); +}); \ No newline at end of file diff --git a/test/unit/api_key_test.rb b/test/unit/api_key_test.rb new file mode 100644 index 000000000..2b1012784 --- /dev/null +++ b/test/unit/api_key_test.rb @@ -0,0 +1,7 @@ +require 'test_helper' + +class ApiKeyTest < ActiveSupport::TestCase + # test "the truth" do + # assert true + # end +end