From a92120e87338b4ce79effeb6bd282c034c269cae Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 17 Dec 2016 22:32:01 -0600 Subject: [PATCH] Fix #2785: Allow changing API key; require password to view or change key. --- .../stylesheets/specific/api_keys.css.scss | 11 +++ app/controllers/api_keys_controller.rb | 17 ---- .../maintenance/user/api_keys_controller.rb | 44 +++++++++++ app/models/api_key.rb | 4 + app/views/api_keys/new.html.erb | 19 ----- .../maintenance/user/api_keys/show.html.erb | 15 ++++ .../maintenance/user/api_keys/update.js.erb | 9 +++ .../maintenance/user/api_keys/view.html.erb | 32 ++++++++ app/views/users/_statistics.html.erb | 7 +- config/routes.rb | 7 +- test/functional/api_keys_controller_test.rb | 36 --------- .../user/api_keys_controller_test.rb | 78 +++++++++++++++++++ 12 files changed, 201 insertions(+), 78 deletions(-) create mode 100644 app/assets/stylesheets/specific/api_keys.css.scss delete mode 100644 app/controllers/api_keys_controller.rb create mode 100644 app/controllers/maintenance/user/api_keys_controller.rb delete mode 100644 app/views/api_keys/new.html.erb create mode 100644 app/views/maintenance/user/api_keys/show.html.erb create mode 100644 app/views/maintenance/user/api_keys/update.js.erb create mode 100644 app/views/maintenance/user/api_keys/view.html.erb delete mode 100644 test/functional/api_keys_controller_test.rb create mode 100644 test/functional/maintenance/user/api_keys_controller_test.rb diff --git a/app/assets/stylesheets/specific/api_keys.css.scss b/app/assets/stylesheets/specific/api_keys.css.scss new file mode 100644 index 000000000..0c43e98e2 --- /dev/null +++ b/app/assets/stylesheets/specific/api_keys.css.scss @@ -0,0 +1,11 @@ +@import "../common/020_base.css.scss"; + +#c-maintenance-user-api-keys { + #api-key { + @extend code; + } + + .button_to { + display: inline-block; + } +} diff --git a/app/controllers/api_keys_controller.rb b/app/controllers/api_keys_controller.rb deleted file mode 100644 index 706ccdc43..000000000 --- a/app/controllers/api_keys_controller.rb +++ /dev/null @@ -1,17 +0,0 @@ -class ApiKeysController < ApplicationController - before_filter :member_only - - def new - @api_key = ApiKey.new(:user_id => CurrentUser.user.id) - end - - def create - @api_key = ApiKey.generate!(CurrentUser.user) - - if @api_key.errors.empty? - redirect_to user_path(CurrentUser.user), :notice => "API key generated" - else - render :action => "new" - end - end -end diff --git a/app/controllers/maintenance/user/api_keys_controller.rb b/app/controllers/maintenance/user/api_keys_controller.rb new file mode 100644 index 000000000..8f4844be4 --- /dev/null +++ b/app/controllers/maintenance/user/api_keys_controller.rb @@ -0,0 +1,44 @@ +module Maintenance + module User + class ApiKeysController < ApplicationController + before_filter :member_only + before_filter :check_privilege + before_filter :authenticate!, :except => [:show] + rescue_from ::SessionLoader::AuthenticationFailure, :with => :authentication_failed + respond_to :html, :json, :xml + + def view + respond_with(CurrentUser.user, @api_key) + end + + def update + @api_key.regenerate! + respond_with(CurrentUser.user, @api_key) { |format| format.js } + end + + def destroy + @api_key.destroy + respond_with(CurrentUser.user, @api_key, location: CurrentUser.user) + end + + protected + + def check_privilege + raise ::User::PrivilegeError unless params[:user_id].to_i == CurrentUser.id + end + + def authenticate! + if ::User.authenticate(CurrentUser.user.name, params[:user][:password]) == CurrentUser.user + @api_key = CurrentUser.user.api_key || ApiKey.generate!(CurrentUser.user) + @password = params[:user][:password] + else + raise ::SessionLoader::AuthenticationFailure + end + end + + def authentication_failed + redirect_to(user_api_key_path(CurrentUser.user), :notice => "Password was incorrect.") + end + end + end +end diff --git a/app/models/api_key.rb b/app/models/api_key.rb index 58ac9ba8a..2037a89fa 100644 --- a/app/models/api_key.rb +++ b/app/models/api_key.rb @@ -7,4 +7,8 @@ class ApiKey < ActiveRecord::Base def self.generate!(user) create(:user_id => user.id, :key => SecureRandom.urlsafe_base64(32)) end + + def regenerate! + update!(:key => SecureRandom.urlsafe_base64(32)) + end end diff --git a/app/views/api_keys/new.html.erb b/app/views/api_keys/new.html.erb deleted file mode 100644 index 5fd33dd4f..000000000 --- a/app/views/api_keys/new.html.erb +++ /dev/null @@ -1,19 +0,0 @@ -
-
-

New API Key

- -

You can generate a new API key to authenticate against <%= Danbooru.config.app_name %>.

- - <%= error_messages_for :api_key %> - - <%= simple_form_for(@api_key) do |f| %> - <%= submit_tag "Generate" %> - <% end %> -
-
- -<%= render "users/secondary_links" %> - -<% content_for(:page_title) do %> - New API Key - <%= Danbooru.config.app_name %> -<% end %> diff --git a/app/views/maintenance/user/api_keys/show.html.erb b/app/views/maintenance/user/api_keys/show.html.erb new file mode 100644 index 000000000..802b65073 --- /dev/null +++ b/app/views/maintenance/user/api_keys/show.html.erb @@ -0,0 +1,15 @@ +
+
+

API Key

+

You must re-enter your password to view or change your API key.

+ + <%= simple_form_for CurrentUser.user, url: view_user_api_key_path(CurrentUser.user), method: :post do |f| %> + <%= f.input :password, :as => :password, :input_html => {:autocomplete => "off"} %> + <%= f.button :submit, "Submit" %> + <% end %> +
+
+ +<% content_for(:page_title) do %> + API Key - <%= Danbooru.config.app_name %> +<% end %> diff --git a/app/views/maintenance/user/api_keys/update.js.erb b/app/views/maintenance/user/api_keys/update.js.erb new file mode 100644 index 000000000..13095d77e --- /dev/null +++ b/app/views/maintenance/user/api_keys/update.js.erb @@ -0,0 +1,9 @@ +<% if @api_key.errors.any? %> + Danbooru.error("<%= j @api_key.errors.full_messages.join(', ') %>"); +<% else %> + $("#api-key").text("<%= j @api_key.key %>"); + $("#api-key-created").html("<%= j compact_time @api_key.created_at %>"); + $("#api-key-updated").html("<%= j compact_time @api_key.updated_at %>"); + + Danbooru.notice("API key regenerated."); +<% end %> diff --git a/app/views/maintenance/user/api_keys/view.html.erb b/app/views/maintenance/user/api_keys/view.html.erb new file mode 100644 index 000000000..82fcc650a --- /dev/null +++ b/app/views/maintenance/user/api_keys/view.html.erb @@ -0,0 +1,32 @@ +
+
+

API Key

+ + + + + + + + + + + + + + + + + + + +
API KeyCreatedUpdatedActions
<%= @api_key.key %><%= compact_time @api_key.created_at %><%= compact_time @api_key.updated_at %> + <%= button_to "Regenerate", user_api_key_path(CurrentUser.user), method: :put, params: { 'user[password]': @password }, remote: true %> + <%= button_to "Delete", user_api_key_path(CurrentUser.user), method: :delete, params: { 'user[password]': @password } %> +
+
+
+ +<% content_for(:page_title) do %> + API Key - <%= Danbooru.config.app_name %> +<% end %> diff --git a/app/views/users/_statistics.html.erb b/app/views/users/_statistics.html.erb index 55da1ba23..754a434ec 100644 --- a/app/views/users/_statistics.html.erb +++ b/app/views/users/_statistics.html.erb @@ -147,11 +147,8 @@ API Key - <% if CurrentUser.user.api_key %> - <%= CurrentUser.user.api_key.key %> - <% else %> - <%= link_to "Generate key", new_api_key_path %> - <% end %> + <%= link_to (CurrentUser.api_key ? "View" : "Generate"), user_api_key_path(CurrentUser.user) %> + (<%= link_to "help", wiki_pages_path(title: "help:api") %>) <% end %> diff --git a/config/routes.rb b/config/routes.rb index b69f2cd18..e4b2805dd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -65,13 +65,15 @@ Rails.application.routes.draw do resource :deletion, :only => [:show, :destroy] resource :email_change, :only => [:new, :create] resource :dmail_filter, :only => [:edit, :update] + resource :api_key, :only => [:show, :view, :update, :destroy] do + post :view + end end end resources :advertisements do resources :hits, :controller => "advertisement_hits", :only => [:create] end - resources :api_keys, :only => [:new, :create] resources :artists do member do put :revert @@ -278,6 +280,9 @@ Rails.application.routes.draw do end resources :users do resource :password, :only => [:edit], :controller => "maintenance/user/passwords" + resource :api_key, :only => [:show, :view, :update, :destroy], :controller => "maintenance/user/api_keys" do + post :view + end collection do get :search diff --git a/test/functional/api_keys_controller_test.rb b/test/functional/api_keys_controller_test.rb deleted file mode 100644 index 7238ec572..000000000 --- a/test/functional/api_keys_controller_test.rb +++ /dev/null @@ -1,36 +0,0 @@ -require 'test_helper' - -class ApiKeysControllerTest < ActionController::TestCase - context "An api keys controller" do - setup do - @user = FactoryGirl.create(:gold_user) - end - - context "#new" do - should "render" do - get :new, {}, {:user_id => @user.id} - assert_response :success - end - end - - context "#create" do - should "succeed" do - assert_difference("ApiKey.count", 1) do - post :create, {}, {:user_id => @user.id} - end - end - - context "when an api key already exists" do - setup do - ApiKey.generate!(@user) - end - - should "not create another api key" do - assert_difference("ApiKey.count", 0) do - post :create, {}, {:user_id => @user.id} - end - end - end - end - end -end \ No newline at end of file diff --git a/test/functional/maintenance/user/api_keys_controller_test.rb b/test/functional/maintenance/user/api_keys_controller_test.rb new file mode 100644 index 000000000..ae0a619c0 --- /dev/null +++ b/test/functional/maintenance/user/api_keys_controller_test.rb @@ -0,0 +1,78 @@ +require 'test_helper' + +module Maintenance + module User + class ApiKeysControllerTest < ActionController::TestCase + def params(password = "password") + { :user_id => @user.id, :user => { :password => password } } + end + + context "An api keys controller" do + setup do + @user = FactoryGirl.create(:gold_user, :password => "password") + CurrentUser.user = @user + CurrentUser.ip_addr = "127.0.0.1" + ApiKey.generate!(@user) + end + + teardown do + @user.api_key.destroy if @user.api_key + end + + context "#show" do + should "render" do + get :show, {:user_id => @user.id}, {:user_id => @user.id} + assert_response :success + end + end + + context "#view" do + context "with an incorrect password" do + should "redirect" do + post :view, params("hunter2"), { :user_id => @user.id } + assert_redirected_to(user_api_key_path(@user)) + end + end + + context "with a correct password" do + should "succeed" do + post :view, params, { :user_id => @user.id } + assert_response :success + end + + should "generate an API key if the user didn't already have one" do + @user.api_key.destroy + + assert_difference("ApiKey.count", 1) do + post :view, params, { :user_id => @user.id } + end + + assert_not_nil(@user.reload.api_key) + end + + should "not generate another API key if the user already has one" do + assert_difference("ApiKey.count", 0) do + post :view, params, { :user_id => @user.id } + end + end + end + end + + context "#update" do + should "regenerate the API key" do + old_key = @user.api_key + post :update, params, { :user_id => @user.id } + assert_not_equal(old_key.key, @user.reload.api_key.key) + end + end + + context "#destroy" do + should "delete the API key" do + post :destroy, params, { :user_id => @user.id } + assert_nil(@user.reload.api_key) + end + end + end + end + end +end