Fix #2785: Allow changing API key; require password to view or change key.

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;
+  }
+}

app/controllers/api_keys_controller.rb

@@ -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

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

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

app/views/api_keys/new.html.erb

@@ -1,19 +0,0 @@
-<div id="c-api-keys">
-  <div id="a-new">
-    <h1>New API Key</h1>
-
-    <p>You can generate a new API key to authenticate against <%= Danbooru.config.app_name %>.</p>
-
-    <%= error_messages_for :api_key %>
-
-    <%= simple_form_for(@api_key) do |f| %>
-      <%= submit_tag "Generate" %>
-    <% end %>
-  </div>
-</div>
-
-<%= render "users/secondary_links" %>
-
-<% content_for(:page_title) do %>
-  New API Key - <%= Danbooru.config.app_name %>
-<% end %>

app/views/maintenance/user/api_keys/show.html.erb

@@ -0,0 +1,15 @@
+<div id="c-maintenance-user-api-keys">
+  <div id="a-show">
+    <h1>API Key</h1>
+    <p>You must re-enter your password to view or change your API key.</p>
+
+    <%= 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 %>
+  </div>
+</div>
+
+<% content_for(:page_title) do %>
+  API Key - <%= Danbooru.config.app_name %>
+<% end %>

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 %>

app/views/maintenance/user/api_keys/view.html.erb

@@ -0,0 +1,32 @@
+<div id="c-maintenance-user-api-keys">
+  <div id="a-view">
+    <h1>API Key</h1>
+
+    <table class="striped" width="100%">
+      <thead>
+        <tr>
+          <th>API Key</th>
+          <th>Created</th>
+          <th>Updated</th>
+          <th>Actions</th>
+        </tr>
+      </thead>
+
+      <tbody>
+        <tr>
+          <td id="api-key"><%= @api_key.key %></td>
+          <td id="api-key-created"><%= compact_time @api_key.created_at %></td>
+          <td id="api-key-updated"><%= compact_time @api_key.updated_at %></td>
+          <td>
+            <%= 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 } %>
+          </td>
+        </tr>
+      </tbody>
+    </table>
+  </div>
+</div>
+
+<% content_for(:page_title) do %>
+  API Key - <%= Danbooru.config.app_name %>
+<% end %>

app/views/users/_statistics.html.erb

@@ -147,11 +147,8 @@
         <tr>
           <th>API Key</th>
           <td>
-            <% 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") %>)
           </td>
         </tr>
       <% end %>

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

test/functional/api_keys_controller_test.rb

@@ -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

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