Просмотр исходного кода

Existing user session when requesting SSO session create endpoint will fail device check because of missing fingerprint param (which is required as soon as a user/session is present).

Thorsten Eckel 5 лет назад
Родитель
Сommit
d1ed72a071

+ 2 - 2
app/controllers/application_controller/authenticates.rb

@@ -143,10 +143,10 @@ module ApplicationController::Authenticates
               User.lookup(login: login&.downcase)
             end
 
-    raise Exceptions::NotAuthorized, 'no valid session' if !user
+    raise Exceptions::NotAuthorized, 'Missing SSO ENV REMOTE_USER' if !user
 
     session.delete(:switched_from_user_id)
-    authentication_check_prerequesits(user, 'session', {})
+    authentication_check_prerequesits(user, 'SSO', {})
   end
 
   def authentication_check_prerequesits(user, auth_type, auth_param)

+ 1 - 0
app/controllers/application_controller/handles_devices.rb

@@ -17,6 +17,7 @@ module ApplicationController::HandlesDevices
     return true if switched_from_user_id
     return true if !user
     return true if !user.permissions?('user_preferences.device')
+    return true if type == 'SSO'
 
     time_to_check = true
     user_device_updated_at = session[:user_device_updated_at]

+ 10 - 3
app/controllers/sessions_controller.rb

@@ -3,6 +3,7 @@
 class SessionsController < ApplicationController
   prepend_before_action :authentication_check, only: %i[switch_to_user list delete]
   skip_before_action :verify_csrf_token, only: %i[show destroy create_omniauth failure_omniauth]
+  skip_before_action :user_device_check, only: %i[create_sso]
 
   # "Create" a login, aka "log the user in"
   def create
@@ -14,15 +15,21 @@ class SessionsController < ApplicationController
            json:   SessionHelper.json_hash(user).merge(config: config_frontend)
   end
 
+  def create_sso
+    authenticate_with_sso
+
+    redirect_to '/#'
+  end
+
   def show
-    user = authentication_check_only || authenticate_with_sso
+    user = authentication_check_only
+    raise Exceptions::NotAuthorized, 'no valid session' if user.blank?
+
     initiate_session_for(user)
 
     # return current session
     render json: SessionHelper.json_hash(user).merge(config: config_frontend)
   rescue Exceptions::NotAuthorized => e
-    raise if e.message != 'no valid session'
-
     render json: {
       error:       e.message,
       config:      config_frontend,

+ 3 - 0
config/routes/auth.rb

@@ -5,6 +5,9 @@ Zammad::Application.routes.draw do
   match '/auth/:provider/callback',         to: 'sessions#create_omniauth',      via: %i[post get puts delete]
   match '/auth/failure',                    to: 'sessions#failure_omniauth',     via: %i[post get]
 
+  # sso
+  match '/auth/sso',                        to: 'sessions#create_sso',           via: %i[get post]
+
   # sessions
   match api_path + '/signin',               to: 'sessions#create',               via: :post
   match api_path + '/signshow',             to: 'sessions#show',                 via: %i[get post]

+ 23 - 74
spec/requests/session_spec.rb

@@ -1,53 +1,38 @@
 require 'rails_helper'
 
 RSpec.describe 'Sessions endpoints', type: :request do
-  # The frontend sends a device fingerprint in the request parameters during authentication
-  # (as part of App.Auth.loginCheck() and App.WebSocket.auth()).
-  #
-  # Without this parameter, the controller will raise a 422 Unprocessable Entity error
-  # (in ApplicationController::HandlesDevices#user_device_log).
-  let(:fingerprint) { { fingerprint: 'foo' } }
-
-  describe 'GET /api/v1/signshow (single sign-on)' do
+
+  describe 'GET /auth/sso (single sign-on)' do
     context 'with invalid user login' do
       let(:login) { User.pluck(:login).max.next }
 
       context 'in "REMOTE_USER" request env var' do
         let(:env) { { 'REMOTE_USER' => login } }
 
-        it 'returns invalid session response' do
-          get '/api/v1/signshow', as: :json, env: env, params: fingerprint
+        it 'returns unauthorized response' do
+          get '/auth/sso', as: :json, env: env
 
-          expect(response).to have_http_status(:ok)
-          expect(json_response)
-            .to include('error' => 'no valid session')
-            .and not_include('session')
+          expect(response).to have_http_status(:unauthorized)
         end
       end
 
       context 'in "HTTP_REMOTE_USER" request env var' do
         let(:env) { { 'HTTP_REMOTE_USER' => login } }
 
-        it 'returns invalid session response' do
-          get '/api/v1/signshow', as: :json, env: env, params: fingerprint
+        it 'returns unauthorized response' do
+          get '/auth/sso', as: :json, env: env
 
-          expect(response).to have_http_status(:ok)
-          expect(json_response)
-            .to include('error' => 'no valid session')
-            .and not_include('session')
+          expect(response).to have_http_status(:unauthorized)
         end
       end
 
       context 'in "X-Forwarded-User" request header' do
         let(:headers) { { 'X-Forwarded-User' => login } }
 
-        it 'returns invalid session response' do
-          get '/api/v1/signshow', as: :json, headers: headers, params: fingerprint
+        it 'returns unauthorized response' do
+          get '/auth/sso', as: :json, headers: headers
 
-          expect(response).to have_http_status(:ok)
-          expect(json_response)
-            .to include('error' => 'no valid session')
-            .and not_include('session')
+          expect(response).to have_http_status(:unauthorized)
         end
       end
     end
@@ -63,7 +48,7 @@ RSpec.describe 'Sessions endpoints', type: :request do
           let(:env) { { 'REMOTE_USER' => login } }
 
           it 'returns 401 unauthorized' do
-            get '/api/v1/signshow', as: :json, env: env, params: fingerprint
+            get '/auth/sso', as: :json, env: env
 
             expect(response).to have_http_status(:unauthorized)
             expect(json_response).to include('error' => 'Maintenance mode enabled!')
@@ -74,7 +59,7 @@ RSpec.describe 'Sessions endpoints', type: :request do
           let(:env) { { 'HTTP_REMOTE_USER' => login } }
 
           it 'returns 401 unauthorized' do
-            get '/api/v1/signshow', as: :json, env: env, params: fingerprint
+            get '/auth/sso', as: :json, env: env
 
             expect(response).to have_http_status(:unauthorized)
             expect(json_response).to include('error' => 'Maintenance mode enabled!')
@@ -85,7 +70,7 @@ RSpec.describe 'Sessions endpoints', type: :request do
           let(:headers) { { 'X-Forwarded-User' => login } }
 
           it 'returns 401 unauthorized' do
-            get '/api/v1/signshow', as: :json, headers: headers, params: fingerprint
+            get '/auth/sso', as: :json, headers: headers
 
             expect(response).to have_http_status(:unauthorized)
             expect(json_response).to include('error' => 'Maintenance mode enabled!')
@@ -97,81 +82,45 @@ RSpec.describe 'Sessions endpoints', type: :request do
         let(:env) { { 'REMOTE_USER' => login } }
 
         it 'returns a new user-session response' do
-          get '/api/v1/signshow', as: :json, env: env, params: fingerprint
+          get '/auth/sso', as: :json, env: env
 
-          expect(json_response)
-            .to include('session' => hash_including('login' => login))
-            .and not_include('error')
+          expect(response).to redirect_to('/#')
         end
 
         it 'sets the :user_id session parameter' do
-          expect { get '/api/v1/signshow', as: :json, env: env, params: fingerprint }
+          expect { get '/auth/sso', as: :json, env: env }
             .to change { request&.session&.fetch(:user_id) }.to(user.id)
         end
-
-        it 'sets the :persistent session parameter' do
-          expect { get '/api/v1/signshow', as: :json, env: env, params: fingerprint }
-            .to change { request&.session&.fetch(:persistent) }.to(true)
-        end
-
-        it 'adds an activity stream entry for the user’s session' do
-          expect { get '/api/v1/signshow', as: :json, env: env, params: fingerprint }
-            .to change(ActivityStream, :count).by(1)
-        end
       end
 
       context 'in "HTTP_REMOTE_USER" request env var' do
         let(:env) { { 'HTTP_REMOTE_USER' => login } }
 
         it 'returns a new user-session response' do
-          get '/api/v1/signshow', as: :json, env: env, params: fingerprint
+          get '/auth/sso', as: :json, env: env
 
-          expect(json_response)
-            .to include('session' => hash_including('login' => login))
-            .and not_include('error')
+          expect(response).to redirect_to('/#')
         end
 
         it 'sets the :user_id session parameter' do
-          expect { get '/api/v1/signshow', as: :json, env: env, params: fingerprint }
+          expect { get '/auth/sso', as: :json, env: env }
             .to change { request&.session&.fetch(:user_id) }.to(user.id)
         end
-
-        it 'sets the :persistent session parameter' do
-          expect { get '/api/v1/signshow', as: :json, env: env, params: fingerprint }
-            .to change { request&.session&.fetch(:persistent) }.to(true)
-        end
-
-        it 'adds an activity stream entry for the user’s session' do
-          expect { get '/api/v1/signshow', as: :json, env: env, params: fingerprint }
-            .to change(ActivityStream, :count).by(1)
-        end
       end
 
       context 'in "X-Forwarded-User" request header' do
         let(:headers) { { 'X-Forwarded-User' => login } }
 
         it 'returns a new user-session response' do
-          get '/api/v1/signshow', as: :json, headers: headers, params: fingerprint
+          get '/auth/sso', as: :json, headers: headers
 
-          expect(json_response)
-            .to include('session' => hash_including('login' => login))
-            .and not_include('error')
+          expect(response).to redirect_to('/#')
         end
 
         it 'sets the :user_id session parameter on the client' do
-          expect { get '/api/v1/signshow', as: :json, headers: headers, params: fingerprint }
+          expect { get '/auth/sso', as: :json, headers: headers }
             .to change { request&.session&.fetch(:user_id) }.to(user.id)
         end
-
-        it 'sets the :persistent session parameter' do
-          expect { get '/api/v1/signshow', as: :json, headers: headers, params: fingerprint }
-            .to change { request&.session&.fetch(:persistent) }.to(true)
-        end
-
-        it 'adds an activity stream entry for the user’s session' do
-          expect { get '/api/v1/signshow', as: :json, headers: headers, params: fingerprint }
-            .to change(ActivityStream, :count).by(1)
-        end
       end
     end
   end