Browse Source

Add force_login option to OAuth authorize page (#8655)

* Add force_login option to OAuth authorize page

For when a user needs to sign into an app from multiple accounts
on the same server

* When logging out from modal header, redirect back after re-login
Eugen Rochko 5 years ago
parent
commit
2288d50a7b

+ 0 - 4
app/controllers/api/base_controller.rb

@@ -53,10 +53,6 @@ class Api::BaseController < ApplicationController
     [params[:limit].to_i.abs, default_limit * 2].min
   end
 
-  def truthy_param?(key)
-    ActiveModel::Type::Boolean.new.cast(params[key])
-  end
-
   def current_resource_owner
     @current_user ||= User.find(doorkeeper_token.resource_owner_id) if doorkeeper_token
   end

+ 4 - 0
app/controllers/application_controller.rb

@@ -58,6 +58,10 @@ class ApplicationController < ActionController::Base
 
   protected
 
+  def truthy_param?(key)
+    ActiveModel::Type::Boolean.new.cast(params[key])
+  end
+
   def forbidden
     respond_with_error(403)
   end

+ 8 - 0
app/controllers/auth/sessions_controller.rb

@@ -28,8 +28,10 @@ class Auth::SessionsController < Devise::SessionsController
   end
 
   def destroy
+    tmp_stored_location = stored_location_for(:user)
     super
     flash.delete(:notice)
+    store_location_for(:user, tmp_stored_location) if continue_after?
   end
 
   protected
@@ -124,8 +126,14 @@ class Auth::SessionsController < Devise::SessionsController
   end
 
   def clear_site_data
+    return if continue_after?
+
     # Should be '"*"' but that doen't work in Chrome (neither does '"executionContexts"')
     # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Clear-Site-Data
     response.headers['Clear-Site-Data'] = '"cache", "cookies", "storage"'
   end
+
+  def continue_after?
+    truthy_param?(:continue)
+  end
 end

+ 14 - 0
app/controllers/oauth/authorizations_controller.rb

@@ -13,4 +13,18 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController
   def store_current_location
     store_location_for(:user, request.url)
   end
+
+  def render_success
+    if skip_authorization? || (matching_token? && !truthy_param?('force_login'))
+      redirect_or_render authorize_response
+    elsif Doorkeeper.configuration.api_only
+      render json: pre_auth
+    else
+      render :new
+    end
+  end
+
+  def truthy_param?(key)
+    ActiveModel::Type::Boolean.new.cast(params[key])
+  end
 end

+ 1 - 1
app/views/layouts/modal.html.haml

@@ -8,7 +8,7 @@
       .name
         = t 'users.signed_in_as'
         %span.username @#{current_account.local_username_and_domain}
-      = link_to destroy_user_session_path, method: :delete, class: 'logout-link icon-button' do
+      = link_to destroy_user_session_path(continue: true), method: :delete, class: 'logout-link icon-button' do
         = fa_icon 'sign-out'
 
   .container-alt= yield

+ 7 - 0
spec/controllers/auth/sessions_controller_spec.rb

@@ -30,6 +30,13 @@ RSpec.describe Auth::SessionsController, type: :controller do
 
         expect(response).to redirect_to(new_user_session_path)
       end
+
+      it 'does not delete redirect location with continue=true' do
+        sign_in(user, scope: :user)
+        controller.store_location_for(:user, '/authorize')
+        delete :destroy, params: { continue: 'true' }
+        expect(controller.stored_location_for(:user)).to eq '/authorize'
+      end
     end
 
     context 'with a suspended user' do

+ 28 - 4
spec/controllers/oauth/authorizations_controller_spec.rb

@@ -5,23 +5,25 @@ require 'rails_helper'
 RSpec.describe Oauth::AuthorizationsController, type: :controller do
   render_views
 
-  let(:app) { Doorkeeper::Application.create!(name: 'test', redirect_uri: 'http://localhost/') }
+  let(:app) { Doorkeeper::Application.create!(name: 'test', redirect_uri: 'http://localhost/', scopes: 'read') }
 
   describe 'GET #new' do
     subject do
-      get :new, params: { client_id: app.uid, response_type: 'code', redirect_uri: 'http://localhost/' }
+      get :new, params: { client_id: app.uid, response_type: 'code', redirect_uri: 'http://localhost/', scope: 'read' }
     end
 
     shared_examples 'stores location for user' do
       it 'stores location for user' do
         subject
-        expect(controller.stored_location_for(:user)).to eq "/oauth/authorize?client_id=#{app.uid}&redirect_uri=http%3A%2F%2Flocalhost%2F&response_type=code"
+        expect(controller.stored_location_for(:user)).to eq "/oauth/authorize?client_id=#{app.uid}&redirect_uri=http%3A%2F%2Flocalhost%2F&response_type=code&scope=read"
       end
     end
 
     context 'when signed in' do
+      let!(:user) { Fabricate(:user) }
+
       before do
-        sign_in Fabricate(:user), scope: :user
+        sign_in user, scope: :user
       end
 
       it 'returns http success' do
@@ -35,6 +37,28 @@ RSpec.describe Oauth::AuthorizationsController, type: :controller do
       end
 
       include_examples 'stores location for user'
+
+      context 'when app is already authorized' do
+        before do
+          Doorkeeper::AccessToken.find_or_create_for(
+            app,
+            user.id,
+            app.scopes,
+            Doorkeeper.configuration.access_token_expires_in,
+            Doorkeeper.configuration.refresh_token_enabled?
+          )
+        end
+
+        it 'redirects to callback' do
+          subject
+          expect(response).to redirect_to(/\A#{app.redirect_uri}/)
+        end
+
+        it 'does not redirect to callback with force_login=true' do
+          get :new, params: { client_id: app.uid, response_type: 'code', redirect_uri: 'http://localhost/', scope: 'read', force_login: 'true' }
+          expect(response.body).to match(/Authorize/)
+        end
+      end
     end
 
     context 'when not signed in' do