Merge pull request #2469 from alphagov/move-user-show-action-into-oau…

…th-users-controller

Move UsersController#show -> OauthUsersController

app/controllers/oauth_users_controller.rb

@@ -0,0 +1,25 @@
+class OauthUsersController < ApplicationController
+  before_action :doorkeeper_authorize!
+  before_action :validate_token_matches_client_id
+  skip_after_action :verify_authorized
+
+  def show
+    current_resource_owner.permissions_synced!(application_making_request)
+    respond_to do |format|
+      format.json do
+        presenter = UserOAuthPresenter.new(current_resource_owner, application_making_request)
+        render json: presenter.as_hash.to_json
+      end
+    end
+  end
+
+private
+
+  def validate_token_matches_client_id
+    # FIXME: Once gds-sso is updated everywhere, this should always validate
+    # the client_id param.  It should 401 if no client_id is given.
+    if params[:client_id].present? && (params[:client_id] != doorkeeper_token.application.uid)
+      head :unauthorized
+    end
+  end
+end

app/controllers/users_controller.rb

@@ -5,27 +5,13 @@ class UsersController < ApplicationController
 
   layout "admin_layout", only: %w[index event_logs require_2sv]
 
-  before_action :authenticate_user!, except: :show
-  before_action :load_and_authorize_user, except: %i[index show]
+  before_action :authenticate_user!
+  before_action :load_and_authorize_user, except: %i[index]
   before_action :allow_no_application_access, only: [:update]
   before_action :redirect_legacy_filters, only: [:index]
   helper_method :applications_and_permissions, :filter_params
   respond_to :html
 
-  before_action :doorkeeper_authorize!, only: :show
-  before_action :validate_token_matches_client_id, only: :show
-  skip_after_action :verify_authorized, only: :show
-
-  def show
-    current_resource_owner.permissions_synced!(application_making_request)
-    respond_to do |format|
-      format.json do
-        presenter = UserOAuthPresenter.new(current_resource_owner, application_making_request)
-        render json: presenter.as_hash.to_json
-      end
-    end
-  end
-
   def index
     authorize User
 
@@ -108,14 +94,6 @@ def should_include_permissions?
     params[:format] == "csv"
   end
 
-  def validate_token_matches_client_id
-    # FIXME: Once gds-sso is updated everywhere, this should always validate
-    # the client_id param.  It should 401 if no client_id is given.
-    if params[:client_id].present? && (params[:client_id] != doorkeeper_token.application.uid)
-      head :unauthorized
-    end
-  end
-
   def export
     applications = Doorkeeper::Application.all
     CSV.generate do |csv|

config/routes.rb

@@ -46,7 +46,7 @@
       get :require_2sv
     end
   end
-  resource :user, only: [:show]
+  get "user", to: "oauth_users#show"
 
   resource :account, only: [:show]
   namespace :account do

test/controllers/oauth_users_controller_test.rb

@@ -0,0 +1,106 @@
+require "test_helper"
+
+class OauthUsersControllerTest < ActionController::TestCase
+  context "GET show (as OAuth client application)" do
+    setup do
+      @application = create(:application)
+    end
+
+    should "fetching json profile with a valid oauth token should succeed" do
+      user = create(:user)
+      user.grant_application_signin_permission(@application)
+      token = create(:access_token, application: @application, resource_owner_id: user.id)
+
+      @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}"
+      get :show, params: { client_id: @application.uid, format: :json }
+
+      assert_equal "200", response.code
+      presenter = UserOAuthPresenter.new(user, @application)
+      assert_equal presenter.as_hash.to_json, response.body
+    end
+
+    should "fetching json profile with a valid oauth token, but no client_id should succeed" do
+      # For now.  Once gds-sso is updated everywhere, this will 401.
+
+      user = create(:user)
+      user.grant_application_signin_permission(@application)
+      token = create(:access_token, application: @application, resource_owner_id: user.id)
+
+      @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}"
+      get :show, params: { format: :json }
+
+      assert_equal "200", response.code
+      presenter = UserOAuthPresenter.new(user, @application)
+      assert_equal presenter.as_hash.to_json, response.body
+    end
+
+    should "fetching json profile with an invalid oauth token should not succeed" do
+      user = create(:user)
+      token = create(:access_token, application: @application, resource_owner_id: user.id)
+
+      @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token.sub(/[0-9]/, 'x')}"
+      get :show, params: { client_id: @application.uid, format: :json }
+
+      assert_equal "401", response.code
+    end
+
+    should "fetching json profile with a token for another app should not succeed" do
+      other_application = create(:application)
+      user = create(:user)
+      token = create(:access_token, application: other_application, resource_owner_id: user.id)
+
+      @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token.sub(/[0-9]/, 'x')}"
+      get :show, params: { client_id: @application.uid, format: :json }
+
+      assert_equal "401", response.code
+    end
+
+    should "fetching json profile without any bearer header should not succeed" do
+      get :show, params: { client_id: @application.uid, format: :json }
+      assert_equal "401", response.code
+    end
+
+    should "fetching json profile should include permissions" do
+      user = create(:user, with_signin_permissions_for: [@application])
+      token = create(:access_token, application: @application, resource_owner_id: user.id)
+
+      @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}"
+      get :show, params: { client_id: @application.uid, format: :json }
+      json = JSON.parse(response.body)
+      assert_equal([SupportedPermission::SIGNIN_NAME], json["user"]["permissions"])
+    end
+
+    should "fetching json profile should include only permissions for the relevant app" do
+      other_application = create(:application)
+      user = create(:user, with_signin_permissions_for: [@application, other_application])
+
+      token = create(:access_token, application: @application, resource_owner_id: user.id)
+
+      @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}"
+      get :show, params: { client_id: @application.uid, format: :json }
+      json = JSON.parse(response.body)
+      assert_equal([SupportedPermission::SIGNIN_NAME], json["user"]["permissions"])
+    end
+
+    should "fetching json profile should update last_synced_at for the relevant app" do
+      user = create(:user)
+      user.grant_application_signin_permission(@application)
+      token = create(:access_token, application: @application, resource_owner_id: user.id)
+
+      @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}"
+      get :show, params: { client_id: @application.uid, format: :json }
+
+      assert_not_nil user.application_permissions.first.last_synced_at
+    end
+
+    should "fetching json profile should fail if no signin permission for relevant app" do
+      user = create(:user)
+      token = create(:access_token, application: @application, resource_owner_id: user.id)
+
+      @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}"
+      get :show, params: { client_id: @application.uid, format: :json }
+
+      assert_response :unauthorized
+    end
+  end
+end

test/controllers/users_controller_test.rb

@@ -3,109 +3,6 @@
 class UsersControllerTest < ActionController::TestCase
   include ActiveJob::TestHelper
 
-  context "GET show (as OAuth client application)" do
-    setup do
-      @application = create(:application)
-    end
-
-    should "fetching json profile with a valid oauth token should succeed" do
-      user = create(:user)
-      user.grant_application_signin_permission(@application)
-      token = create(:access_token, application: @application, resource_owner_id: user.id)
-
-      @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}"
-      get :show, params: { client_id: @application.uid, format: :json }
-
-      assert_equal "200", response.code
-      presenter = UserOAuthPresenter.new(user, @application)
-      assert_equal presenter.as_hash.to_json, response.body
-    end
-
-    should "fetching json profile with a valid oauth token, but no client_id should succeed" do
-      # For now.  Once gds-sso is updated everywhere, this will 401.
-
-      user = create(:user)
-      user.grant_application_signin_permission(@application)
-      token = create(:access_token, application: @application, resource_owner_id: user.id)
-
-      @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}"
-      get :show, params: { format: :json }
-
-      assert_equal "200", response.code
-      presenter = UserOAuthPresenter.new(user, @application)
-      assert_equal presenter.as_hash.to_json, response.body
-    end
-
-    should "fetching json profile with an invalid oauth token should not succeed" do
-      user = create(:user)
-      token = create(:access_token, application: @application, resource_owner_id: user.id)
-
-      @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token.sub(/[0-9]/, 'x')}"
-      get :show, params: { client_id: @application.uid, format: :json }
-
-      assert_equal "401", response.code
-    end
-
-    should "fetching json profile with a token for another app should not succeed" do
-      other_application = create(:application)
-      user = create(:user)
-      token = create(:access_token, application: other_application, resource_owner_id: user.id)
-
-      @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token.sub(/[0-9]/, 'x')}"
-      get :show, params: { client_id: @application.uid, format: :json }
-
-      assert_equal "401", response.code
-    end
-
-    should "fetching json profile without any bearer header should not succeed" do
-      get :show, params: { client_id: @application.uid, format: :json }
-      assert_equal "401", response.code
-    end
-
-    should "fetching json profile should include permissions" do
-      user = create(:user, with_signin_permissions_for: [@application])
-      token = create(:access_token, application: @application, resource_owner_id: user.id)
-
-      @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}"
-      get :show, params: { client_id: @application.uid, format: :json }
-      json = JSON.parse(response.body)
-      assert_equal([SupportedPermission::SIGNIN_NAME], json["user"]["permissions"])
-    end
-
-    should "fetching json profile should include only permissions for the relevant app" do
-      other_application = create(:application)
-      user = create(:user, with_signin_permissions_for: [@application, other_application])
-
-      token = create(:access_token, application: @application, resource_owner_id: user.id)
-
-      @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}"
-      get :show, params: { client_id: @application.uid, format: :json }
-      json = JSON.parse(response.body)
-      assert_equal([SupportedPermission::SIGNIN_NAME], json["user"]["permissions"])
-    end
-
-    should "fetching json profile should update last_synced_at for the relevant app" do
-      user = create(:user)
-      user.grant_application_signin_permission(@application)
-      token = create(:access_token, application: @application, resource_owner_id: user.id)
-
-      @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}"
-      get :show, params: { client_id: @application.uid, format: :json }
-
-      assert_not_nil user.application_permissions.first.last_synced_at
-    end
-
-    should "fetching json profile should fail if no signin permission for relevant app" do
-      user = create(:user)
-      token = create(:access_token, application: @application, resource_owner_id: user.id)
-
-      @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}"
-      get :show, params: { client_id: @application.uid, format: :json }
-
-      assert_response :unauthorized
-    end
-  end
-
   context "as Admin" do
     setup do
       @user = create(:admin_user, email: "[email protected]")