Commit b9adf92f authored by Tiago Botelho's avatar Tiago Botelho

Prevent users from disconnecting gitlab account from CAS

parent 19a44034
class Profiles::AccountsController < Profiles::ApplicationController class Profiles::AccountsController < Profiles::ApplicationController
include AuthHelper
def show def show
@user = current_user @user = current_user
end end
def unlink def unlink
provider = params[:provider] provider = params[:provider]
current_user.identities.find_by(provider: provider).destroy unless provider.to_s == 'saml' identity = current_user.identities.find_by(provider: provider)
return render_404 unless identity
if unlink_allowed?(provider)
identity.destroy
else
flash[:alert] = "You are not allowed to unlink your primary login account"
end
redirect_to profile_account_path redirect_to profile_account_path
end end
end end
...@@ -76,5 +76,9 @@ module AuthHelper ...@@ -76,5 +76,9 @@ module AuthHelper
(current_user.otp_grace_period_started_at + current_application_settings.two_factor_grace_period.hours) < Time.current (current_user.otp_grace_period_started_at + current_application_settings.two_factor_grace_period.hours) < Time.current
end end
def unlink_allowed?(provider)
%w(saml cas3).exclude?(provider.to_s)
end
extend self extend self
end end
...@@ -75,12 +75,12 @@ ...@@ -75,12 +75,12 @@
.provider-btn-image .provider-btn-image
= provider_image_tag(provider) = provider_image_tag(provider)
- if auth_active?(provider) - if auth_active?(provider)
- if provider.to_s == 'saml' - if unlink_allowed?(provider)
%a.provider-btn
Active
- else
= link_to unlink_profile_account_path(provider: provider), method: :delete, class: 'provider-btn' do = link_to unlink_profile_account_path(provider: provider), method: :delete, class: 'provider-btn' do
Disconnect Disconnect
- else
%a.provider-btn
Active
- else - else
= link_to omniauth_authorize_path(:user, provider), method: :post, class: 'provider-btn not-active' do = link_to omniauth_authorize_path(:user, provider), method: :post, class: 'provider-btn not-active' do
Connect Connect
......
---
title: Prevent users from disconnecting GitLab account from CAS
merge_request: 10282
author:
require 'spec_helper' require 'spec_helper'
describe Profiles::AccountsController do describe Profiles::AccountsController do
let(:user) { create(:omniauth_user, provider: 'saml') } describe 'DELETE unlink' do
let(:user) { create(:omniauth_user) }
before do before do
sign_in(user) sign_in(user)
end end
it 'does not allow to unlink SAML connected account' do it 'renders 404 if someone tries to unlink a non existent provider' do
delete :unlink, provider: 'github'
expect(response).to have_http_status(404)
end
[:saml, :cas3].each do |provider|
describe "#{provider} provider" do
let(:user) { create(:omniauth_user, provider: provider.to_s) }
it "does not allow to unlink connected account" do
identity = user.identities.last identity = user.identities.last
delete :unlink, provider: 'saml'
updated_user = User.find(user.id) delete :unlink, provider: provider.to_s
expect(response).to have_http_status(302) expect(response).to have_http_status(302)
expect(updated_user.identities.size).to eq(1) expect(user.reload.identities).to include(identity)
expect(updated_user.identities).to include(identity) end
end
end end
it 'does allow to delete other linked accounts' do [:twitter, :facebook, :google_oauth2, :gitlab, :github, :bitbucket, :crowd, :auth0].each do |provider|
user.identities.create(provider: 'twitter', extern_uid: 'twitter_123') describe "#{provider} provider" do
let(:user) { create(:omniauth_user, provider: provider.to_s) }
expect { delete :unlink, provider: 'twitter' }.to change(Identity.all, :size).by(-1) it 'allows to unlink connected account' do
identity = user.identities.last
delete :unlink, provider: provider.to_s
expect(response).to have_http_status(302)
expect(user.reload.identities).not_to include(identity)
end
end
end
end end
end end
...@@ -62,4 +62,18 @@ describe AuthHelper do ...@@ -62,4 +62,18 @@ describe AuthHelper do
end end
end end
end end
describe 'unlink_allowed?' do
[:saml, :cas3].each do |provider|
it "returns true if the provider is #{provider}" do
expect(helper.unlink_allowed?(provider)).to be false
end
end
[:twitter, :facebook, :google_oauth2, :gitlab, :github, :bitbucket, :crowd, :auth0].each do |provider|
it "returns false if the provider is #{provider}" do
expect(helper.unlink_allowed?(provider)).to be true
end
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment