Merge pull request #32 from azul/feature/api-version-1-fixes

Feature: API version 1 fixes

9 files changed, 98 insertions, 12 deletions

diff --git a/.gitignore b/.gitignore
index 3b31e0b..5536c6f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -27,3 +27,4 @@ public/ca.crt
 public/config/*
 public/provider.json
 config/config.yml
+bin
diff --git a/users/app/controllers/sessions_controller.rb b/users/app/controllers/sessions_controller.rb
index 0345fbd..01ecff6 100644
--- a/users/app/controllers/sessions_controller.rb
+++ b/users/app/controllers/sessions_controller.rb
@@ -1,7 +1,5 @@
 class SessionsController < ApplicationController
 
-  skip_before_filter :verify_authenticity_token
-
   def new
     @session = Session.new
     if authentication_errors
diff --git a/users/app/controllers/v1/sessions_controller.rb b/users/app/controllers/v1/sessions_controller.rb
index 27d10fb..0551ca9 100644
--- a/users/app/controllers/v1/sessions_controller.rb
+++ b/users/app/controllers/v1/sessions_controller.rb
@@ -18,12 +18,20 @@ module V1
 
     def update
       authenticate!
-      render :json => session.delete(:handshake)
+      render :json => login_response
     end
 
     def destroy
       logout
       redirect_to root_path
     end
+
+    protected
+
+    def login_response
+      handshake = session.delete(:handshake)
+      handshake.to_hash.merge(:id => current_user.id)
+    end
+
   end
 end
diff --git a/users/app/views/v1/sessions/new.json.erb b/users/app/views/v1/sessions/new.json.erb
new file mode 100644
index 0000000..36154b8
--- /dev/null
+++ b/users/app/views/v1/sessions/new.json.erb
@@ -0,0 +1,3 @@
+{
+"errors": <%= raw @errors.to_json %>
+}
diff --git a/users/config/routes.rb b/users/config/routes.rb
index 2cd1740..c50cb15 100644
--- a/users/config/routes.rb
+++ b/users/config/routes.rb
@@ -1,17 +1,18 @@
 Rails.application.routes.draw do
 
   constraints :subdomain => "api" do
-    namespace "api", { module: "V1",
+    namespace "api", { module: "v1",
         path: "/1/",
         defaults: {format: 'json'} } do
-      resources :sessions, :only => [:new, :create, :update, :destroy]
+      resources :sessions, :only => [:new, :create, :update]
+      delete "logout" => "sessions#destroy", :as => "logout"
       resources :users, :only => [:create, :update]
     end
   end
 
   get "login" => "sessions#new", :as => "login"
-  get "logout" => "sessions#destroy", :as => "logout"
-  resources :sessions, :only => [:new, :create, :update, :destroy]
+  delete "logout" => "sessions#destroy", :as => "logout"
+  resources :sessions, :only => [:new, :create, :update]
 
   get "signup" => "users#new", :as => "signup"
   resources :users do
diff --git a/users/leap_web_users.gemspec b/users/leap_web_users.gemspec
index 0182c1f..c57937f 100644
--- a/users/leap_web_users.gemspec
+++ b/users/leap_web_users.gemspec
@@ -17,6 +17,6 @@ Gem::Specification.new do |s|
 
   s.add_dependency "leap_web_core", LeapWeb::VERSION
 
-  s.add_dependency "ruby-srp", "~> 0.1.5"
+  s.add_dependency "ruby-srp", "~> 0.1.6"
   s.add_dependency "rails_warden"
 end
diff --git a/users/lib/warden/strategies/secure_remote_password.rb b/users/lib/warden/strategies/secure_remote_password.rb
index 483336d..363e6a0 100644
--- a/users/lib/warden/strategies/secure_remote_password.rb
+++ b/users/lib/warden/strategies/secure_remote_password.rb
@@ -25,10 +25,15 @@ module Warden
       end
 
       def validate!
-        client = session[:handshake].authenticate(params['client_auth'].hex)
-        client ?
-          success!(User.find_by_login(client.username)) :
+        if client = validate
+          success!(User.find_by_login(client.username))
+        else
           fail!(:password => "wrong_password")
+        end
+      end
+
+      def validate
+        session[:handshake].authenticate(params['client_auth'].hex)
       end
 
       def initialize!
diff --git a/users/test/functional/sessions_controller_test.rb b/users/test/functional/sessions_controller_test.rb
index 9df4455..f99c0d7 100644
--- a/users/test/functional/sessions_controller_test.rb
+++ b/users/test/functional/sessions_controller_test.rb
@@ -47,10 +47,12 @@ class SessionsControllerTest < ActionController::TestCase
     request.env['warden'].expects(:authenticate!)
     handshake = stub(:to_json => "JSON")
     session[:handshake] = handshake
+
     post :update, :id => @user.login, :client_auth => @client_hex
+
     assert_nil session[:handshake]
     assert_response :success
-    assert_equal handshake.to_json, @response.body
+    assert_json_response handshake
   end
 
   test "logout should reset warden user" do
diff --git a/users/test/functional/v1/sessions_controller_test.rb b/users/test/functional/v1/sessions_controller_test.rb
new file mode 100644
index 0000000..be085ce
--- /dev/null
+++ b/users/test/functional/v1/sessions_controller_test.rb
@@ -0,0 +1,68 @@
+require 'test_helper'
+
+# This is a simple controller unit test.
+# We're stubbing out both warden and srp.
+# There's an integration test testing the full rack stack and srp
+class V1::SessionsControllerTest < ActionController::TestCase
+
+  setup do
+    @request.env['HTTP_HOST'] = 'api.lvh.me'
+    @user = stub :login => "me", :id => 123
+    @client_hex = 'a123'
+  end
+
+  test "renders json" do
+    request.env['warden'].expects(:winning_strategy)
+    get :new, :format => :json
+    assert_response :success
+    assert_json_error nil
+  end
+
+  test "renders warden errors" do
+    strategy = stub :message => {:field => :translate_me}
+    request.env['warden'].stubs(:winning_strategy).returns(strategy)
+    I18n.expects(:t).with(:translate_me).at_least_once.returns("translation stub")
+    get :new, :format => :json
+    assert_response 422
+    assert_json_error :field => "translation stub"
+  end
+
+  # Warden takes care of parsing the params and
+  # rendering the response. So not much to test here.
+  test "should perform handshake" do
+    request.env['warden'].expects(:authenticate!)
+    # make sure we don't get a template missing error:
+    @controller.stubs(:render)
+    post :create, :login => @user.login, 'A' => @client_hex
+  end
+
+  test "should authorize" do
+    request.env['warden'].expects(:authenticate!)
+    @controller.expects(:current_user).returns(@user)
+    handshake = stub(:to_hash => {h: "ash"})
+    session[:handshake] = handshake
+
+    post :update, :id => @user.login, :client_auth => @client_hex
+
+    assert_nil session[:handshake]
+    assert_response :success
+    assert_json_response handshake.to_hash.merge(id: @user.id)
+  end
+
+  test "logout should reset warden user" do
+    expect_warden_logout
+    delete :destroy
+    assert_response :redirect
+    assert_redirected_to root_url
+  end
+
+  def expect_warden_logout
+    raw = mock('raw session') do
+      expects(:inspect)
+    end
+    request.env['warden'].expects(:raw_session).returns(raw)
+    request.env['warden'].expects(:logout)
+  end
+
+
+end