Merge pull request #3442 from nebulab/kennyadsl/refactor-api-key-actions

Use RESTful routing for users' API key management

api/config/routes.rb

@@ -3,9 +3,11 @@
 Spree::Core::Engine.routes.draw do
   namespace :admin do
     resources :users do
+      resource :api_key, controller: 'users/api_key', only: [:create, :destroy]
+
       member do
-        put :generate_api_key
-        put :clear_api_key
+        put :generate_api_key # Deprecated
+        put :clear_api_key # Deprecated
       end
     end
   end

backend/app/controllers/spree/admin/users/api_key_controller.rb

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+module Spree
+  module Admin
+    module Users
+      class ApiKeyController < Spree::Admin::BaseController
+        def create
+          if user.generate_spree_api_key!
+            flash[:success] = t('spree.admin.api.key_generated')
+          end
+          redirect_to edit_admin_user_path(user)
+        end
+
+        def destroy
+          if user.clear_spree_api_key!
+            flash[:success] = t('spree.admin.api.key_cleared')
+          end
+          redirect_to edit_admin_user_path(user)
+        end
+
+        private
+
+        def user
+          @user ||= Spree.user_class.find(params[:user_id])
+        end
+      end
+    end
+  end
+end

backend/app/controllers/spree/admin/users_controller.rb

@@ -82,13 +82,33 @@ def items
       end
 
       def generate_api_key
+        Spree::Deprecation.warn <<-WARN.strip_heredoc, caller
+          The route or controller action you are using is deprecated.
+
+          Instead of:
+          generate_api_key_admin_user PUT    /admin/users/:id/generate_api_key
+
+          Please use:
+          admin_user_api_key          POST   /admin/users/:user_id/api_key
+        WARN
+
         if @user.generate_spree_api_key!
           flash[:success] = t('spree.admin.api.key_generated')
         end
         redirect_to edit_admin_user_path(@user)
       end
 
       def clear_api_key
+        Spree::Deprecation.warn <<-WARN.strip_heredoc, caller
+          The route or controller action you are using is deprecated.
+
+          Instead of:
+          clear_api_key_admin_user PUT    /admin/users/:id/clear_api_key
+
+          Please use:
+          admin_user_api_key       DELETE /admin/users/:user_id/api_key
+        WARN
+
         if @user.clear_spree_api_key!
           flash[:success] = t('spree.admin.api.key_cleared')
         end

backend/app/views/spree/admin/users/edit.html.erb

@@ -25,7 +25,7 @@
   </div>
 </fieldset>
 
-<% if can?(:update, @user) %>
+<% if can?(:update, @user) && can?(:manage, :api_key) %>
   <fieldset data-hook="admin_user_api_key" id="admin_user_edit_api_key">
     <legend><%= t('.api_access') %></legend>
 
@@ -40,16 +40,16 @@
 
       </div>
       <div class="filter-actions actions">
-        <%= button_to t('.clear_key'), spree.clear_api_key_admin_user_path(@user), method: :put, data: { confirm: t('.confirm_clear_key') }, class: 'btn btn-default' %>
-        <%= button_to t('.regenerate_key'), spree.generate_api_key_admin_user_path(@user), method: :put, data: { confirm: t('.confirm_regenerate_key') }, class: 'btn btn-default' %>
+        <%= button_to t('.clear_key'), spree.admin_user_api_key_path(@user), method: :delete, data: { confirm: t('.confirm_clear_key') }, class: 'btn btn-default' %>
+        <%= button_to t('.regenerate_key'), spree.admin_user_api_key_path(@user), method: :post, data: { confirm: t('.confirm_regenerate_key') }, class: 'btn btn-default' %>
       </div>
 
     <% else %>
 
       <div class="no-objects-found"><%= t('.no_key') %></div>
 
       <div class="filter-actions actions">
-        <%= button_to t('.generate_key'), spree.generate_api_key_admin_user_path(@user), method: :put, class: 'btn btn-primary' %>
+        <%= button_to t('.generate_key'), spree.admin_user_api_key_path(@user), method: :post, class: 'btn btn-primary' %>
       </div>
     <% end %>
   </fieldset>

backend/spec/controllers/spree/admin/users/api_key_controller_spec.rb

@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+describe Spree::Admin::Users::ApiKeyController, type: :controller do
+  let(:user) { create(:user) }
+
+  describe '#create' do
+    context "with ability to manage users and API keys" do
+      stub_authorization! do |_user|
+        can [:manage], Spree.user_class
+        can [:manage], :api_key
+      end
+
+      it "allows admins to create a new user's API key" do
+        post :create, params: { user_id: user.id }
+
+        expect(flash[:success]).to eq I18n.t('spree.admin.api.key_generated')
+        expect(response).to redirect_to(spree.edit_admin_user_path(user))
+      end
+    end
+
+    context "without ability to manage users and API keys" do
+      stub_authorization! do |_user|
+      end
+
+      it 'denies access' do
+        delete :destroy, params: { user_id: user.id }
+
+        expect(flash[:error]).to eq I18n.t('spree.authorization_failure')
+        expect(response).to redirect_to '/unauthorized'
+      end
+    end
+  end
+
+  describe '#destroy' do
+    context "with ability to manage users and API keys" do
+      stub_authorization! do |_user|
+        can [:manage], Spree.user_class
+        can [:manage], :api_key
+      end
+
+      it "allows admins to clear an existing user's API key" do
+        user.generate_spree_api_key!
+        delete :destroy, params: { user_id: user.id }
+
+        expect(flash[:success]).to eq I18n.t('spree.admin.api.key_cleared')
+        expect(response).to redirect_to(spree.edit_admin_user_path(user))
+      end
+    end
+
+    context "without ability to manage users and API keys" do
+      stub_authorization! do |_user|
+      end
+
+      it 'denies access' do
+        delete :destroy, params: { user_id: user.id }
+
+        expect(flash[:error]).to eq I18n.t('spree.authorization_failure')
+        expect(response).to redirect_to '/unauthorized'
+      end
+    end
+  end
+end

backend/spec/controllers/spree/admin/users_controller_spec.rb

@@ -42,13 +42,15 @@
       end
 
       it "allows admins to update a user's API key" do
+        expect(Spree::Deprecation).to receive(:warn)
         expect {
           put :generate_api_key, params: { id: user.id }
         }.to change { user.reload.spree_api_key }
         expect(response).to redirect_to(spree.edit_admin_user_path(user))
       end
 
       it "allows admins to clear a user's API key" do
+        expect(Spree::Deprecation).to receive(:warn)
         user.generate_spree_api_key!
         expect {
           put :clear_api_key, params: { id: user.id }

core/lib/spree/permission_sets/user_management.rb

@@ -18,6 +18,7 @@ def activate!
         cannot [:delete, :destroy], Spree.user_class
         can :manage, Spree::StoreCredit
         can :display, Spree::Role
+        can :manage, :api_key
       end
     end
   end