diff --git a/config/locales/en.yml b/config/locales/en.yml index e7cfbcb..d0f75b5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -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.' diff --git a/lib/doorkeeper/openid_connect/config.rb b/lib/doorkeeper/openid_connect/config.rb index ee4588c..42d1b33 100644 --- a/lib/doorkeeper/openid_connect/config.rb +++ b/lib/doorkeeper/openid_connect/config.rb @@ -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') } diff --git a/lib/doorkeeper/openid_connect/helpers/controller.rb b/lib/doorkeeper/openid_connect/helpers/controller.rb index e2d9bfa..ae1c4de 100644 --- a/lib/doorkeeper/openid_connect/helpers/controller.rb +++ b/lib/doorkeeper/openid_connect/helpers/controller.rb @@ -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 diff --git a/lib/generators/doorkeeper/openid_connect/templates/initializer.rb b/lib/generators/doorkeeper/openid_connect/templates/initializer.rb index 1cb93a9..adbcfa1 100644 --- a/lib/generators/doorkeeper/openid_connect/templates/initializer.rb +++ b/lib/generators/doorkeeper/openid_connect/templates/initializer.rb @@ -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 diff --git a/spec/controllers/doorkeeper/authorizations_controller.rb b/spec/controllers/doorkeeper/authorizations_controller.rb index 8c2a6ef..421b8b8 100644 --- a/spec/controllers/doorkeeper/authorizations_controller.rb +++ b/spec/controllers/doorkeeper/authorizations_controller.rb @@ -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 @@ -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 diff --git a/spec/dummy/config/initializers/doorkeeper.rb b/spec/dummy/config/initializers/doorkeeper.rb index 2286587..6a225c9 100644 --- a/spec/dummy/config/initializers/doorkeeper.rb +++ b/spec/dummy/config/initializers/doorkeeper.rb @@ -4,6 +4,7 @@ User.new name: params[:current_user] else redirect_to('/login') + nil end end end diff --git a/spec/dummy/config/initializers/doorkeeper_openid_connect.rb b/spec/dummy/config/initializers/doorkeeper_openid_connect.rb index 087ed06..c3b224a 100644 --- a/spec/dummy/config/initializers/doorkeeper_openid_connect.rb +++ b/spec/dummy/config/initializers/doorkeeper_openid_connect.rb @@ -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 diff --git a/spec/dummy/db/migrate/20161102204540_add_current_sign_in_at_to_users.rb b/spec/dummy/db/migrate/20161102204540_add_current_sign_in_at_to_users.rb new file mode 100644 index 0000000..3e6720f --- /dev/null +++ b/spec/dummy/db/migrate/20161102204540_add_current_sign_in_at_to_users.rb @@ -0,0 +1,5 @@ +class AddCurrentSignInAtToUsers < ActiveRecord::Migration + def change + add_column :users, :current_sign_in_at, :datetime + end +end diff --git a/spec/dummy/db/schema.rb b/spec/dummy/db/schema.rb index 21d6dff..314ded4 100644 --- a/spec/dummy/db/schema.rb +++ b/spec/dummy/db/schema.rb @@ -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 @@ -67,6 +67,7 @@ t.datetime "created_at" t.datetime "updated_at" t.string "password" + t.datetime "current_sign_in_at" end end diff --git a/spec/lib/config_spec.rb b/spec/lib/config_spec.rb index c335a70..0737ddd 100644 --- a/spec/lib/config_spec.rb +++ b/spec/lib/config_spec.rb @@ -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 {}