Skip to content

Commit

Permalink
feature: Support max_age parameter
Browse files Browse the repository at this point in the history
  • Loading branch information
toupeira committed Nov 5, 2016
1 parent 741fff2 commit aabe3aa
Show file tree
Hide file tree
Showing 10 changed files with 137 additions and 25 deletions.
2 changes: 2 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,6 @@ en:
messages:
# Configuration error messages
resource_owner_from_access_token_not_configured: 'Failure due to Doorkeeper::OpenidConnect.configure.resource_owner_from_access_token missing configuration.'
auth_time_from_resource_owner_not_configured: 'Failure due to Doorkeeper::OpenidConnect.configure.auth_time_from_resource_owner missing configuration.'
reauthenticate_resource_owner_not_configured: 'Failure due to Doorkeeper::OpenidConnect.configure.reauthenticate_resource_owner missing configuration.'
subject_not_configured: 'ID Token generation failed due to Doorkeeper::OpenidConnect.configure.subject missing configuration.'
8 changes: 8 additions & 0 deletions lib/doorkeeper/openid_connect/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,14 @@ def extended(base)
fail ConfigurationError, I18n.translate('doorkeeper.openid_connect.errors.messages.resource_owner_from_access_token_not_configured')
}

option :auth_time_from_resource_owner, default: lambda { |*_|
fail ConfigurationError, I18n.translate('doorkeeper.openid_connect.errors.messages.auth_time_from_resource_owner_not_configured')
}

option :reauthenticate_resource_owner, default: lambda { |*_|
fail ConfigurationError, I18n.translate('doorkeeper.openid_connect.errors.messages.reauthenticate_resource_owner_not_configured')
}

option :subject, default: lambda { |*_|
fail ConfigurationError, I18n.translate('doorkeeper.openid_connect.errors.messages.subject_not_configured')
}
Expand Down
44 changes: 31 additions & 13 deletions lib/doorkeeper/openid_connect/helpers/controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,41 @@ module Controller

def authenticate_resource_owner!
owner = super

if prompt_values.include?('none') && (!owner || owner.is_a?(String))
# clear the previous response body to avoid a DoubleRenderError
# TODO: this is currently broken on Rails 5, see
# https://github.com/rails/rails/issues/25106
self.response_body = nil

error = ::Doorkeeper::OAuth::ErrorResponse.new(name: :login_required)
response.headers.merge!(error.headers)
render json: error.body, status: error.status
else
if validate_prompt_param!(owner) && validate_max_age_param!(owner)
owner
end
end

def prompt_values
@prompt_values ||= params[:prompt].to_s.split(/ +/)
def validate_prompt_param!(owner)
prompt_values ||= params[:prompt].to_s.split(/ +/)
return true unless prompt_values.include?('none') && !owner

# clear the previous response body to avoid a DoubleRenderError
# TODO: this is currently broken on Rails 5, see
# https://github.com/rails/rails/issues/25106
self.response_body = nil

error = ::Doorkeeper::OAuth::ErrorResponse.new(name: :login_required)
response.headers.merge!(error.headers)
render json: error.body, status: error.status

false
end

def validate_max_age_param!(owner)
max_age = params[:max_age].to_i
return true unless max_age.positive?

auth_time = instance_exec owner,
&Doorkeeper::OpenidConnect.configuration.auth_time_from_resource_owner

if !auth_time || (Time.zone.now - auth_time) > max_age
instance_exec owner,
&Doorkeeper::OpenidConnect.configuration.reauthenticate_resource_owner
false
else
true
end
end
end
end
Expand Down
12 changes: 12 additions & 0 deletions lib/generators/doorkeeper/openid_connect/templates/initializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,18 @@
# User.find_by(id: access_token.resource_owner_id)
end

auth_time_from_resource_owner do |resource_owner|
# Example implementation:
# resource_owner.current_sign_in_at
end

reauthenticate_resource_owner do |resource_owner|
# Example implementation:
# store_location_for resource_owner, request.fullpath
# sign_out resource_owner
# redirect_to new_user_session_url
end

subject do |resource_owner|
# Example implementation:
# resource_owner.key
Expand Down
59 changes: 48 additions & 11 deletions spec/controllers/doorkeeper/authorizations_controller.rb
Original file line number Diff line number Diff line change
@@ -1,24 +1,26 @@
require 'rails_helper'

describe Doorkeeper::AuthorizationsController, type: :controller do
describe '#new' do
context 'without a prompt parameter' do
it 'renders the authorization form if logged in' do
get :new, current_user: 'Joe'
let(:user) { create :user }

expect(response).to be_successful
end
describe '#resource_owner_authenticator' do
it 'renders the authorization form if logged in' do
get :new, current_user: user.id

it 'redirects to login form when not logged in' do
get :new
expect(response).to be_successful
end

expect(response).to redirect_to '/login'
end
it 'redirects to login form when not logged in' do
get :new

expect(response).to redirect_to '/login'
end
end

describe '#validate_prompt_param!' do
context 'with a prompt=none parameter' do
it 'renders the authorization form if logged in' do
get :new, current_user: 'Joe', prompt: 'none'
get :new, current_user: user.id, prompt: 'none'

expect(response).to be_successful
end
Expand All @@ -34,4 +36,39 @@
end
end
end

describe '#validate_max_age_param!' do
context 'with an invalid max_age parameter' do
it 'renders the authorization form' do
%w[ 0 -1 -23 foobar ].each do |max_age|
get :new, current_user: user.id, max_age: max_age

expect(response).to be_successful
end
end
end

context 'with a max_age=10 parameter' do
it 'renders the authorization form if the users last login was within 10 seconds' do
user.update! current_sign_in_at: 5.seconds.ago
get :new, current_user: user.id, max_age: 10

expect(response).to be_successful
end

it 'calls reauthenticate_resource_owner if the last login was longer than 10 seconds ago' do
user.update! current_sign_in_at: 5.minutes.ago
get :new, current_user: user.id, max_age: 10

expect(response).to redirect_to '/reauthenticate'
end

it 'calls reauthenticate_resource_owner if the last login is unknown' do
user.update! current_sign_in_at: nil
get :new, current_user: user.id, max_age: 10

expect(response).to redirect_to '/reauthenticate'
end
end
end
end
1 change: 1 addition & 0 deletions spec/dummy/config/initializers/doorkeeper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
User.new name: params[:current_user]
else
redirect_to('/login')
nil
end
end
end
8 changes: 8 additions & 0 deletions spec/dummy/config/initializers/doorkeeper_openid_connect.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@
User.find_by(id: access_token.resource_owner_id)
end

auth_time_from_resource_owner do |resource_owner|
resource_owner.current_sign_in_at
end

reauthenticate_resource_owner do |_resource_owner|
redirect_to '/reauthenticate'
end

subject do |resource_owner|
resource_owner.id
end
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddCurrentSignInAtToUsers < ActiveRecord::Migration
def change
add_column :users, :current_sign_in_at, :datetime
end
end
3 changes: 2 additions & 1 deletion spec/dummy/db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20161031135842) do
ActiveRecord::Schema.define(version: 20161102204540) do

create_table "oauth_access_grants", force: :cascade do |t|
t.integer "resource_owner_id", null: false
Expand Down Expand Up @@ -67,6 +67,7 @@
t.datetime "created_at"
t.datetime "updated_at"
t.string "password"
t.datetime "current_sign_in_at"
end

end
20 changes: 20 additions & 0 deletions spec/lib/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,26 @@
end
end

describe 'auth_time_from_resource_owner' do
it 'sets the block that is accessible via auth_time_from_resource_owner' do
block = proc {}
Doorkeeper::OpenidConnect.configure do
auth_time_from_resource_owner(&block)
end
expect(subject.auth_time_from_resource_owner).to eq(block)
end
end

describe 'reauthenticate_resource_owner' do
it 'sets the block that is accessible via reauthenticate_resource_owner' do
block = proc {}
Doorkeeper::OpenidConnect.configure do
reauthenticate_resource_owner(&block)
end
expect(subject.reauthenticate_resource_owner).to eq(block)
end
end

describe 'subject' do
it 'sets the block that is accessible via subject' do
block = proc {}
Expand Down

0 comments on commit aabe3aa

Please sign in to comment.