Skip to content

Commit c2cae83

Browse files
authored
Add logout logic when using external login (#6042)
* Add logout logic when using external login * Added missing test * Fix rspec issues
1 parent d539141 commit c2cae83

File tree

6 files changed

+114
-10
lines changed

6 files changed

+114
-10
lines changed

app/controllers/api/v1/sessions_controller.rb

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,28 @@ def create
6868
# DELETE /api/v1/sessions/signout
6969
# Clears the session cookie and signs the user out
7070
def destroy
71-
sign_out
72-
render_data status: :ok
71+
id_token = session.delete(:oidc_id_token)
72+
73+
# Make logout request to OIDC
74+
if id_token.present? && external_auth?
75+
issuer_url = if ENV.fetch('LOADBALANCER_ENDPOINT', nil).present?
76+
File.join(ENV.fetch('OPENID_CONNECT_ISSUER'), "/#{current_provider}")
77+
else
78+
ENV.fetch('OPENID_CONNECT_ISSUER')
79+
end
80+
81+
end_session = File.join(issuer_url, 'protocol', 'openid-connect', 'logout')
82+
83+
url = "#{end_session}?client_id=#{ENV.fetch('OPENID_CONNECT_CLIENT_ID', nil)}" \
84+
"&id_token_hint=#{id_token}" \
85+
"&post_logout_redirect_uri=#{CGI.escape(root_url(success: 'LogoutSuccessful'))}"
86+
87+
sign_out
88+
render_data data: url, status: :ok
89+
else
90+
sign_out
91+
render_data status: :ok
92+
end
7393
end
7494

7595
private

app/controllers/external_controller.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ def create_user
8383
handle_session_timeout(session_timeout.to_i, user) if session_timeout
8484

8585
session[:session_token] = user.session_token
86+
session[:oidc_id_token] = credentials.dig('credentials', 'id_token')
8687

8788
# TODO: - Ahmad: deal with errors
8889

app/javascript/components/home/HomePage.jsx

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ export default function HomePage() {
3535
const navigate = useNavigate();
3636
const [searchParams, setSearchParams] = useSearchParams();
3737
const error = searchParams.get('error');
38+
const success = searchParams.get('success');
3839
const { data: recordValue } = useRoomConfigValue('record');
3940
const { data: env } = useEnv();
4041

@@ -51,6 +52,16 @@ export default function HomePage() {
5152
[currentUser.signed_in],
5253
);
5354

55+
useEffect(() => {
56+
switch (success) {
57+
case 'LogoutSuccessful':
58+
toast.success(t('toast.success.session.signed_out'));
59+
break;
60+
default:
61+
}
62+
if (success) { setSearchParams(searchParams.delete('success')); }
63+
}, [success]);
64+
5465
useEffect(() => {
5566
switch (error) {
5667
case 'InviteInvalid':

app/javascript/hooks/mutations/sessions/useDeleteSession.jsx

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,19 @@ export default function useDeleteSession({ showToast = true }) {
3030
return useMutation(
3131
() => axios.delete('/sessions/signout.json'),
3232
{
33-
onSuccess: async () => {
34-
setStateChanging(true);
35-
queryClient.refetchQueries('useSessions');
36-
await navigate('/');
37-
if (showToast) { toast.success(t('toast.success.session.signed_out')); }
38-
setStateChanging(false);
33+
onSuccess: async ({ data }) => {
34+
const logoutUrl = data?.data;
35+
36+
if (typeof logoutUrl === 'string') {
37+
window.location.href = logoutUrl;
38+
} else {
39+
setStateChanging(true);
40+
queryClient.refetchQueries('useSessions');
41+
42+
await navigate('/');
43+
if (showToast) { toast.success(t('toast.success.session.signed_out')); }
44+
setStateChanging(false);
45+
}
3946
},
4047
onError: () => {
4148
toast.error(t('toast.error.problem_completing_action'));

spec/controllers/external_controller_spec.rb

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@
3030
info: {
3131
email: Faker::Internet.email,
3232
name: Faker::Name.name
33+
},
34+
credentials: {
35+
id_token: 'sample_id_token'
3336
}
3437
)
3538

@@ -53,7 +56,6 @@
5356

5457
get :create_user, params: { provider: 'openid_connect' }
5558

56-
expect(session[:session_token]).to eq(User.find_by(email: OmniAuth.config.mock_auth[:openid_connect][:info][:email]).session_token)
5759
expect(response).to redirect_to(root_path)
5860
end
5961

@@ -93,6 +95,15 @@
9395
end.not_to change(User, :count)
9496
end
9597

98+
it 'sets the correct session variables' do
99+
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
100+
101+
get :create_user, params: { provider: 'openid_connect' }
102+
103+
expect(session[:session_token]).to eq(User.find_by(email: OmniAuth.config.mock_auth[:openid_connect][:info][:email]).session_token)
104+
expect(session[:oidc_id_token]).to eq(OmniAuth.config.mock_auth[:openid_connect][:credentials][:id_token])
105+
end
106+
96107
context 'redirect' do
97108
it 'redirects to the location cookie if a relative redirection 1' do
98109
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]

spec/controllers/sessions_controller_spec.rb

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@
2828
end
2929

3030
describe '#create' do
31+
before do
32+
allow(controller).to receive(:external_auth?).and_return(false)
33+
end
34+
3135
it 'creates a regular session if the remember me checkbox is not selected' do
3236
post :create, params: {
3337
session: {
@@ -140,13 +144,63 @@
140144
end
141145

142146
describe '#destroy' do
143-
it 'signs off the user' do
147+
before do
144148
sign_in_user(user)
149+
end
145150

151+
it 'signs off the user' do
146152
delete :destroy
147153

148154
expect(cookies.encrypted[:_extended_session]).to be_nil
149155
expect(session[:session_token]).to be_nil
150156
end
157+
158+
context 'external auth' do
159+
before do
160+
session[:oidc_id_token] = 'sample_id_token'
161+
allow(controller).to receive(:external_auth?).and_return(true)
162+
ENV['OPENID_CONNECT_ISSUER'] = 'https://openid.example'
163+
end
164+
165+
after do
166+
ENV['OPENID_CONNECT_ISSUER'] = nil
167+
end
168+
169+
it 'returns the OIDC logout url' do
170+
delete :destroy
171+
172+
expect(response.parsed_body['data']).to match('protocol/openid-connect/logout')
173+
expect(response.parsed_body['data']).to match('id_token_hint=sample_id_token')
174+
expect(response.parsed_body['data']).to match("post_logout_redirect_uri=#{CGI.escape(root_url(success: 'LogoutSuccessful'))}")
175+
end
176+
177+
it 'removes both session tokens' do
178+
delete :destroy
179+
180+
expect(session[:session_token]).to be_nil
181+
expect(session[:oidc_id_token]).to be_nil
182+
end
183+
184+
context 'LB is set' do
185+
let!(:role_with_provider_test) { create(:role, provider: 'test-provider') }
186+
let!(:mt_user) { create(:user, provider: 'test-provider', role: role_with_provider_test) }
187+
188+
before do
189+
sign_in_user(mt_user)
190+
ENV['LOADBALANCER_ENDPOINT'] = 'http://test.com/'
191+
allow(controller).to receive(:current_provider).and_return('test-provider')
192+
end
193+
194+
after do
195+
ENV['LOADBALANCER_ENDPOINT'] = nil
196+
end
197+
198+
it 'returns the OIDC logout url' do
199+
delete :destroy
200+
201+
expect(response.parsed_body['data']).to start_with(File.join(ENV.fetch('OPENID_CONNECT_ISSUER', nil), "/#{controller.current_provider}"))
202+
end
203+
end
204+
end
151205
end
152206
end

0 commit comments

Comments
 (0)