Skip to content
Snippets Groups Projects
Unverified Commit bcbccd2c authored by Khemissi Amir's avatar Khemissi Amir Committed by GitHub
Browse files

Fixed bad OmniAuth flow when deployed on a subdirectory (Closes #5176). (#5197)

* Fixed static redirections on ExternalController#create API.
	+ Fixed static OIDC URLs on the FE codebase.

* Changed other host redirection handeling on external authN.
parent e05c5e16
No related branches found
No related tags found
No related merge requests found
......@@ -28,4 +28,9 @@ module ClientRoutable
def reset_password_url(token)
"#{root_url}reset_password/#{token}"
end
# Generates a client side pending url.
def pending_path
"#{root_path}pending"
end
end
......@@ -17,6 +17,8 @@
# frozen_string_literal: true
class ExternalController < ApplicationController
include ClientRoutable
skip_before_action :verify_authenticity_token
# GET 'auth/:provider/callback'
......@@ -25,13 +27,8 @@ class ExternalController < ApplicationController
provider = current_provider
credentials = request.env['omniauth.auth']
user_info = {
name: credentials['info']['name'],
email: credentials['info']['email'],
language: extract_language_code(credentials['info']['locale']),
external_id: credentials['uid'],
verified: true
}
user_info = build_user_info(credentials)
user = User.find_by(external_id: credentials['uid'], provider:)
new_user = user.blank?
......@@ -40,7 +37,7 @@ class ExternalController < ApplicationController
# Check if they have a valid token only if a new sign up
if new_user && registration_method == SiteSetting::REGISTRATION_METHODS[:invite] && !valid_invite_token(email: user_info[:email])
return redirect_to "/?error=#{Rails.configuration.custom_error_msgs[:invite_token_invalid]}"
return redirect_to root_path(error: Rails.configuration.custom_error_msgs[:invite_token_invalid])
end
# Create the user if they dont exist
......@@ -58,21 +55,25 @@ class ExternalController < ApplicationController
# Set to pending if registration method is approval
if registration_method == SiteSetting::REGISTRATION_METHODS[:approval]
user.pending! if new_user
return redirect_to '/pending' if user.pending?
return redirect_to pending_path if user.pending?
end
user.generate_session_token!
session[:session_token] = user.session_token
# TODO: - Ahmad: deal with errors
redirect_location = cookies[:location]
cookies.delete(:location)
return redirect_to redirect_location if redirect_location&.match?('\A\/rooms\/\w{3}-\w{3}-\w{3}-\w{3}\/join\z')
redirect_to '/'
redirect_location = cookies.delete(:location)
return redirect_to redirect_location, allow_other_host: false if redirect_location&.match?('\/rooms\/\w{3}-\w{3}-\w{3}-\w{3}\/join\z')
redirect_to root_path
rescue ActionController::Redirecting::UnsafeRedirectError => e
Rails.logger.error("Unsafe redirection attempt: #{e}")
redirect_to root_path
rescue StandardError => e
Rails.logger.error("Error during authentication: #{e}")
redirect_to '/?error=SignupError'
redirect_to root_path(error: Rails.configuration.custom_error_msgs[:external_signup_error])
end
# POST /recording_ready
......@@ -126,4 +127,14 @@ class ExternalController < ApplicationController
# Try to delete the invitation and return true if it succeeds
Invitation.destroy_by(email:, provider: current_provider, token:).present?
end
def build_user_info(credentials)
{
name: credentials['info']['name'],
email: credentials['info']['email'],
language: extract_language_code(credentials['info']['locale']),
external_id: credentials['uid'],
verified: true
}
end
end
......@@ -42,7 +42,7 @@ export default function AuthButtons({ direction }) {
if (env?.OPENID_CONNECT) {
return (
<Form action="/auth/openid_connect" method="POST" data-turbo="false">
<Form action={process.env.OMNIAUTH_PATH} method="POST" data-turbo="false">
<input type="hidden" name="authenticity_token" value={document.querySelector('meta[name="csrf-token"]').content} />
<input type="hidden" name="current_provider" value={env?.CURRENT_PROVIDER} />
<Stack direction={direction} gap={2}>
......
......@@ -39,7 +39,7 @@ export default function RequireAuthentication({ path }) {
<Card.Footer className="bg-white">
{
env?.OPENID_CONNECT ? (
<Form action="/auth/openid_connect" method="POST" data-turbo="false">
<Form action={process.env.OMNIAUTH_PATH} method="POST" data-turbo="false">
<input type="hidden" name="authenticity_token" value={document.querySelector('meta[name="csrf-token"]').content} />
<Button variant="brand-outline-color" className="btn btn-lg m-2" type="submit">{t('authentication.sign_up')}</Button>
<Button variant="brand" className="btn btn-lg m-2" type="submit">{t('authentication.sign_in')}</Button>
......
......@@ -18,7 +18,7 @@
import React, { useState, useEffect } from 'react';
import Card from 'react-bootstrap/Card';
import {
Navigate, Link, useLocation, useParams,
Navigate, Link, useParams,
} from 'react-router-dom';
import {
Button, Col, Row, Stack, Form as RegularForm,
......@@ -54,8 +54,7 @@ export default function RoomJoin() {
const { methods, fields } = useRoomJoinForm();
const location = useLocation();
const path = encodeURIComponent(location.pathname);
const path = encodeURIComponent(document.location.pathname);
useEffect(() => { // set cookie to return to if needed
const date = new Date();
......@@ -243,7 +242,7 @@ export default function RoomJoin() {
{!currentUser?.signed_in && (
env?.OPENID_CONNECT ? (
<Stack direction="horizontal" className="d-flex justify-content-center text-muted mt-3"> {t('authentication.already_have_account')}
<RegularForm action="/auth/openid_connect" method="POST" data-turbo="false">
<RegularForm action={process.env.OMNIAUTH_PATH} method="POST" data-turbo="false">
<input type="hidden" name="authenticity_token" value={document.querySelector('meta[name="csrf-token"]').content} />
<Button variant="link" className="btn-sm fs-6 cursor-pointer ms-2 ps-0" type="submit">{t('authentication.sign_in')}</Button>
</RegularForm>
......
......@@ -50,7 +50,8 @@ module Greenlight
room_limit: 'RoomLimitError',
pending_user: 'PendingUser',
banned_user: 'BannedUser',
unverified_user: 'UnverifiedUser'
unverified_user: 'UnverifiedUser',
external_signup_error: 'SignupError'
}
config.uploads = {
......
......@@ -20,6 +20,7 @@ await esbuild.build({
},
define: {
'process.env.RELATIVE_URL_ROOT': `"${relativeUrlRoot}"`,
'process.env.OMNIAUTH_PATH': `"${relativeUrlRoot}/auth/openid_connect"`,
},
});
......
......@@ -14,6 +14,7 @@ await esbuild.build({
},
define: {
'process.env.RELATIVE_URL_ROOT': `"${relativeUrlRoot}"`,
'process.env.OMNIAUTH_PATH': `"${relativeUrlRoot}/auth/openid_connect"`,
},
});
......
......@@ -54,7 +54,7 @@ RSpec.describe ExternalController, type: :controller do
get :create_user, params: { provider: 'openid_connect' }
expect(session[:session_token]).to eq(User.find_by(email: OmniAuth.config.mock_auth[:openid_connect][:info][:email]).session_token)
expect(response).to redirect_to('/')
expect(response).to redirect_to(root_path)
end
it 'assigns the User role to the user' do
......@@ -74,7 +74,7 @@ RSpec.describe ExternalController, type: :controller do
end
context 'redirect' do
it 'redirects to the location cookie if the format is valid' do
it 'redirects to the location cookie if a relative redirection 1' do
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
cookies[:location] = {
......@@ -86,19 +86,31 @@ RSpec.describe ExternalController, type: :controller do
expect(response).to redirect_to('/rooms/o5g-hvb-s44-p5t/join')
end
it 'doesnt redirect if it doesnt match a room joins format' do
it 'redirects to the location cookie if a relative redirection 2' do
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
cookies[:location] = {
value: 'https://google.com',
value: '/a/b/c/d/rooms/o5g-hvb-s44-p5t/join',
path: '/'
}
get :create_user, params: { provider: 'openid_connect' }
expect(response).to redirect_to('/')
expect(response).to redirect_to('/a/b/c/d/rooms/o5g-hvb-s44-p5t/join')
end
it 'doesnt redirect if it doesnt match a room joins format check 2' do
it 'doesnt redirect if NOT a relative redirection check 1' do
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
cookies[:location] = {
value: Faker::Internet.url,
path: '/'
}
get :create_user, params: { provider: 'openid_connect' }
expect(response).to redirect_to(root_path)
end
it 'doesnt redirect if it NOT a relative redirection format check 2' do
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
cookies[:location] = {
......@@ -107,10 +119,10 @@ RSpec.describe ExternalController, type: :controller do
}
get :create_user, params: { provider: 'openid_connect' }
expect(response).to redirect_to('/')
expect(response).to redirect_to(root_path)
end
it 'doesnt redirect if it doesnt match a room joins format check 3' do
it 'doesnt redirect if it NOT a relative redirection format check 3' do
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
cookies[:location] = {
......@@ -119,7 +131,31 @@ RSpec.describe ExternalController, type: :controller do
}
get :create_user, params: { provider: 'openid_connect' }
expect(response).to redirect_to('/')
expect(response).to redirect_to(root_path)
end
it 'doesnt redirect if NOT a relative redirection check 4' do
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
cookies[:location] = {
value: Faker::Internet.url(path: '/rooms/o5g-hvb-s44-p5t/join'),
path: '/'
}
get :create_user, params: { provider: 'openid_connect' }
expect(response).to redirect_to(root_path)
end
it 'doesnt redirect if NOT a valid room join link check 5' do
request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
cookies[:location] = {
value: '/romios/o5g-hvb-s44-p5t/join',
path: '/'
}
get :create_user, params: { provider: 'openid_connect' }
expect(response).to redirect_to(root_path)
end
it 'deletes the cookie after reading' do
......@@ -215,7 +251,7 @@ RSpec.describe ExternalController, type: :controller do
create(:user, external_id: OmniAuth.config.mock_auth[:openid_connect][:uid])
expect { get :create_user, params: { provider: 'openid_connect' } }.not_to raise_error
expect(response).to redirect_to('/')
expect(response).to redirect_to(root_path)
end
it 'returns an InviteInvalid error if no invite is passed' do
......@@ -223,7 +259,7 @@ RSpec.describe ExternalController, type: :controller do
get :create_user, params: { provider: 'openid_connect' }
expect(response).to redirect_to('/?error=InviteInvalid')
expect(response).to redirect_to(root_path(error: Rails.configuration.custom_error_msgs[:invite_token_invalid]))
end
it 'returns an InviteInvalid error if the token is wrong' do
......@@ -235,7 +271,7 @@ RSpec.describe ExternalController, type: :controller do
get :create_user, params: { provider: 'openid_connect' }
expect(response).to redirect_to('/?error=InviteInvalid')
expect(response).to redirect_to(root_path(error: Rails.configuration.custom_error_msgs[:invite_token_invalid]))
end
end
......@@ -252,6 +288,7 @@ RSpec.describe ExternalController, type: :controller do
expect { get :create_user, params: { provider: 'openid_connect' } }.to change(User, :count).by(1)
expect(User.find_by(email: OmniAuth.config.mock_auth[:openid_connect][:info][:email])).to be_pending
expect(response).to redirect_to(controller.pending_path)
end
end
end
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment