Skip to content

Commit

Permalink
Add force_login option to OAuth authorize page (mastodon#8655)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Gargron authored and root committed Oct 12, 2018
1 parent d25d07f commit be46104
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 9 deletions.
4 changes: 0 additions & 4 deletions app/controllers/api/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@ def limit_param(default_limit)
[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
Expand Down
4 changes: 4 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ def after_sign_out_path_for(_resource_or_scope)

protected

def truthy_param?(key)
ActiveModel::Type::Boolean.new.cast(params[key])
end

def forbidden
respond_with_error(403)
end
Expand Down
8 changes: 8 additions & 0 deletions app/controllers/auth/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@ def create
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
Expand Down Expand Up @@ -124,8 +126,14 @@ def home_paths(resource)
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 changes: 14 additions & 0 deletions app/controllers/oauth/authorizations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion app/views/layouts/modal.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions spec/controllers/auth/sessions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@

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
Expand Down
32 changes: 28 additions & 4 deletions spec/controllers/oauth/authorizations_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,25 @@
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
Expand All @@ -35,6 +37,28 @@
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
Expand Down

0 comments on commit be46104

Please sign in to comment.