Skip to content

Commit

Permalink
Extract verified session logic into a concern
Browse files Browse the repository at this point in the history
It was being duplicated a lot
  • Loading branch information
segiddins committed Nov 22, 2023
1 parent d630c5e commit 488077f
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 47 deletions.
10 changes: 3 additions & 7 deletions app/controllers/adoptions_controller.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
class AdoptionsController < ApplicationController
include SessionVerifiable

before_action :find_rubygem
before_action :verify_ownership_requestable
before_action :redirect_to_verify, if: :current_user_is_owner?
before_action :redirect_to_verify, if: -> { current_user_is_owner? && !password_session_active? }

def index
@ownership_call = @rubygem.ownership_call
Expand All @@ -18,10 +20,4 @@ def verify_ownership_requestable
def current_user_is_owner?
@rubygem.owned_by?(current_user)
end

def redirect_to_verify
return if password_session_active?
session[:redirect_uri] = rubygem_adoptions_path(@rubygem.slug)
redirect_to verify_session_path
end
end
25 changes: 16 additions & 9 deletions app/controllers/api_keys_controller.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
class ApiKeysController < ApplicationController
include ApiKeyable
before_action :redirect_to_signin, unless: :signed_in?
before_action :redirect_to_new_mfa, if: :mfa_required_not_yet_enabled?
before_action :redirect_to_settings_strong_mfa_required, if: :mfa_required_weak_level_enabled?
before_action :redirect_to_verify, unless: :password_session_active?

include SessionVerifiable
verify_session_before

def index
@api_key = session.delete(:api_key)
Expand Down Expand Up @@ -84,12 +83,20 @@ def reset

private

def api_key_params
params.require(:api_key).permit(:name, *ApiKey::API_SCOPES, :mfa, :rubygem_id)
def verify_session_redirect_path
case action_name
when "reset", "destroy"
profile_api_keys_path

Check warning on line 89 in app/controllers/api_keys_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/api_keys_controller.rb#L89

Added line #L89 was not covered by tests
when "create"
new_profile_api_key_path

Check warning on line 91 in app/controllers/api_keys_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/api_keys_controller.rb#L91

Added line #L91 was not covered by tests
when "update"
edit_profile_api_key_path(params.require(:id))

Check warning on line 93 in app/controllers/api_keys_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/api_keys_controller.rb#L93

Added line #L93 was not covered by tests
else
super
end
end

def redirect_to_verify
session[:redirect_uri] = profile_api_keys_path
redirect_to verify_session_path
def api_key_params
params.require(:api_key).permit(:name, *ApiKey::API_SCOPES, :mfa, :rubygem_id)
end
end
29 changes: 29 additions & 0 deletions app/controllers/concerns/session_verifiable.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
module SessionVerifiable
extend ActiveSupport::Concern

class_methods do
def verify_session_before(**opts)
before_action :redirect_to_signin, **opts, unless: :signed_in?
before_action :redirect_to_new_mfa, **opts, if: :mfa_required_not_yet_enabled?
before_action :redirect_to_settings_strong_mfa_required, **opts, if: :mfa_required_weak_level_enabled?
before_action :redirect_to_verify, **opts, unless: :password_session_active?
end
end

private

def verify_session_redirect_path
redirect_uri = request.path_info
redirect_uri += "?#{request.query_string}" if request.query_string.present?
redirect_uri
end

included do
private

def redirect_to_verify
session[:redirect_uri] = verify_session_redirect_path
redirect_to verify_session_path
end
end
end
23 changes: 14 additions & 9 deletions app/controllers/oidc/api_key_roles_controller.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
class OIDC::ApiKeyRolesController < ApplicationController
include ApiKeyable

include SessionVerifiable
verify_session_before

helper RubygemsHelper

before_action :redirect_to_signin, unless: :signed_in?
before_action :redirect_to_new_mfa, if: :mfa_required_not_yet_enabled?
before_action :redirect_to_settings_strong_mfa_required, if: :mfa_required_weak_level_enabled?
before_action :redirect_to_verify, unless: :password_session_active?
before_action :find_api_key_role, except: %i[index new create]
before_action :redirect_for_deleted, only: %i[edit update destroy]
before_action :set_page, only: :index
Expand Down Expand Up @@ -90,17 +89,23 @@ def destroy

private

def verify_session_redirect_path
case action_name
when "create"
new_profile_api_key_path

Check warning on line 95 in app/controllers/oidc/api_key_roles_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/oidc/api_key_roles_controller.rb#L95

Added line #L95 was not covered by tests
when "update"
edit_profile_api_key_path

Check warning on line 97 in app/controllers/oidc/api_key_roles_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/oidc/api_key_roles_controller.rb#L97

Added line #L97 was not covered by tests
else
super
end
end

def find_api_key_role
@api_key_role = current_user.oidc_api_key_roles
.includes(:provider)
.find_by!(token: params.require(:token))
end

def redirect_to_verify
session[:redirect_uri] = request.path_info + (request.query_string.present? ? "?#{request.query_string}" : "")
redirect_to verify_session_path
end

def redirect_for_deleted
redirect_to profile_oidc_api_key_roles_path, flash: { error: t(".deleted") } if @api_key_role.deleted_at?
end
Expand Down
12 changes: 3 additions & 9 deletions app/controllers/oidc/id_tokens_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@
class OIDC::IdTokensController < ApplicationController
include ApiKeyable

before_action :redirect_to_signin, unless: :signed_in?
before_action :redirect_to_new_mfa, if: :mfa_required_not_yet_enabled?
before_action :redirect_to_settings_strong_mfa_required, if: :mfa_required_weak_level_enabled?
before_action :redirect_to_verify, unless: :password_session_active?
include SessionVerifiable
verify_session_before

before_action :find_id_token, except: %i[index]
before_action :set_page, only: :index

Expand All @@ -26,9 +25,4 @@ def show
def find_id_token
@id_token = current_user.oidc_id_tokens.find(params.require(:id))
end

def redirect_to_verify
session[:redirect_uri] = request.path_info
redirect_to verify_session_path
end
end
12 changes: 3 additions & 9 deletions app/controllers/oidc/providers_controller.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
# frozen_string_literal: true

class OIDC::ProvidersController < ApplicationController
before_action :redirect_to_signin, unless: :signed_in?
before_action :redirect_to_new_mfa, if: :mfa_required_not_yet_enabled?
before_action :redirect_to_settings_strong_mfa_required, if: :mfa_required_weak_level_enabled?
before_action :redirect_to_verify, unless: :password_session_active?
include SessionVerifiable
verify_session_before

before_action :find_provider, except: %i[index]
before_action :set_page, only: :index

Expand All @@ -22,9 +21,4 @@ def show
def find_provider
@provider = OIDC::Provider.find(params.require(:id))
end

def redirect_to_verify
session[:redirect_uri] = request.path_info
redirect_to verify_session_path
end
end
9 changes: 5 additions & 4 deletions app/controllers/owners_controller.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
class OwnersController < ApplicationController
include SessionVerifiable

before_action :find_rubygem, except: :confirm
before_action :render_forbidden, unless: :owner?, except: %i[confirm resend_confirmation]
before_action :redirect_to_verify, unless: :password_session_active?, only: %i[index create destroy]
verify_session_before only: %i[index create destroy]
before_action :verify_mfa_requirement, only: %i[create destroy]

def confirm
Expand Down Expand Up @@ -53,9 +55,8 @@ def destroy

private

def redirect_to_verify
session[:redirect_uri] = rubygem_owners_url(@rubygem.slug)
redirect_to verify_session_path
def verify_session_redirect_path
rubygem_owners_url(params[:rubygem_id])
end

def token_params
Expand Down

0 comments on commit 488077f

Please sign in to comment.