Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testcases for the Ominiauth controller openid connect #869

Merged
merged 21 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
eab41b0
Testcases for the Ominiauth controller openid connect
200455939-yashu Aug 29, 2024
050b7db
Code cleanup - specs controllers omniauth
200455939-yashu Aug 29, 2024
22fde67
Issue fix for the multiple accounts
200455939-yashu Aug 30, 2024
e95c10c
Resolving conflicts after the yashu-sso-link-accounts
200455939-yashu Aug 30, 2024
377f62c
Merge remote-tracking branch 'origin/yashu-sso-2user-accounts-issue-f…
200455939-yashu Aug 30, 2024
45e54e3
Resolving the conflicts after merge
200455939-yashu Aug 30, 2024
84c2ef7
adding test cases for the linked successfylly and 2 users condition
200455939-yashu Aug 30, 2024
58d4ee8
Translation related changes
200455939-yashu Aug 30, 2024
9678eda
Merge remote-tracking branch 'origin/yashu-sso-link-accounts' into ya…
200455939-yashu Aug 30, 2024
a3b5ab6
Spelling correction
200455939-yashu Aug 30, 2024
7f70edf
Removing the byebug and the updates related to translations.
200455939-yashu Sep 3, 2024
0aece6b
Adding the changelog
200455939-yashu Sep 3, 2024
3649013
commit after Resolving the conflicts in Changelog
200455939-yashu Sep 3, 2024
7eb53a1
Review Changes
200455939-yashu Sep 4, 2024
87b2fa3
Merge branch 'yashu-sso-link-accounts' into yashu-controller-spec
200455939-yashu Sep 5, 2024
c21e69d
Merge branch 'yashu-sso-link-accounts' into yashu-controller-spec
200455939-yashu Sep 6, 2024
8303962
Review changes 2 O
200455939-yashu Sep 9, 2024
8d3bd79
Resolving the conflicts after merging the link accounts branch
200455939-yashu Sep 9, 2024
bf0d279
Review changes 2.1
200455939-yashu Sep 10, 2024
ea4e25a
Removing conflict HEAD
200455939-yashu Sep 10, 2024
57da037
Make rubocop happy
aaronskiba Sep 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Added

- Test cases for CILogon(openid_connection) changes in Omniauth controller - [#869](https://github.com/portagenetwork/roadmap/pull/869/)
- Implemented openid_connection SSO with CILogon

### Changed
Expand Down
8 changes: 7 additions & 1 deletion app/controllers/users/omniauth_callbacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,14 @@ def openid_connect
attrs: auth,
identifiable: current_user)

flash[:notice] = 'Linked succesfully'
flash[:notice] = _('Linked successfully')
redirect_to root_path
elsif user.id != current_user.id
# If a user was found but does NOT match the current user then the identifier has
# already been attached to another account (likely the user has 2 accounts)
flash[:alert] = format(_("The current %{description} iD has been already linked to a user with email %{email}"),
description: identifier_scheme.description, email: user.email)
redirect_to edit_user_registration_path
end
end
# rubocop:enable Metrics/AbcSize, Metrics/MethodLength
Expand Down
1 change: 1 addition & 0 deletions app/views/translation_io_exports/_cilogon.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<%= _('CILogon') %>
194 changes: 134 additions & 60 deletions spec/controllers/omniauth_callbacks_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -1,62 +1,136 @@
# frozen_string_literal: true

require 'rails_helper'
require 'byebug'

# RSpec.describe Users::OmniauthCallbacksController, type: :controller do
# describe '#openid_connect' do
# let(:auth) do
# OmniAuth::AuthHash.new(
# provider: 'openid_connect',
# uid: '123545',
# info: {
# email: '[email protected]'
# }
# )
# end

# before do
# OmniAuth.config.test_mode = true
# OmniAuth.config.mock_auth[:openid_connect] = auth
# request.env['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
# request.env['devise.mapping'] = Devise.mappings[:user] # If using Devise
# end

# let(:user) { create(:user) } # Defining the user

# context 'when the email is missing and user does not exist' do
# before do
# allow(User).to receive(:from_omniauth).and_return(nil)
# allow(auth.info).to receive(:email).and_return(nil)
# get :openid_connect
# end

# it 'redirects to the registration page with a flash message' do
# expect(flash[:notice]).to eq('Something went wrong, Please try signing-up here.')
# expect(response).to redirect_to(new_user_registration_path)
# end
# end

# context 'with correct credentials' do
# before do
# create(:org, managed: false, is_other: true)
# @org = create(:org, managed: true)
# @identifier_scheme = create(:identifier_scheme,
# name: 'openid_connect',
# description: 'CILogon',
# active: true,
# identifier_prefix: 'https://www.cilogon.org/')

# Rails.application.env_config['devise.mapping'] = Devise.mappings[:user]
# Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[:openid_connect]
# allow(User).to receive(:from_omniauth).and_return(user)
# # get :openid_connect
# end

# it 'links account from external credentials' do
# expect(flash[:notice]).to eq('Linked successfully')
# expect(response).to redirect_to(root_path)
# end
# end
# end
# end

RSpec.describe Users::OmniauthCallbacksController, type: :controller do
before do
# Enable test mode for OmniAuth
OmniAuth.config.test_mode = true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spec/spec_helper.rb:124 of yashu-sso-link-accounts also has OmniAuth.config.test_mode = true. Do we need the same statement here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated this


# Setup Devise mapping
@request.env["devise.mapping"] = Devise.mappings[:user]
create(:org, managed: false, is_other: true)
@org = create(:org, managed: true)
@identifier_scheme = create(:identifier_scheme,
name: 'openid_connect',
description: 'CILogon',
active: true,
identifier_prefix: 'https://www.cilogon.org/')

# Mock OmniAuth data for OpenID Connect with necessary info
OmniAuth.config.mock_auth[:openid_connect] = OmniAuth::AuthHash.new({
provider: 'openid_connect',
uid: '12345',
info: {
email: '[email protected]',
first_name: 'Test',
last_name: 'User',
name: 'Test User'
}
})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to define this here since it is already on spec_helper.rb

roadmap/spec/spec_helper.rb

Lines 126 to 136 in 4da5fdd

OmniAuth.config.mock_auth[:openid_connect] = OmniAuth::AuthHash.new(
{
provider: 'openid_connect',
uid: '12345',
info: {
email: '[email protected]',
first_name: 'John',
last_name: 'Doe'
}
}
)


# Assign the mocked authentication hash to the request environment
@request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:openid_connect]
end

describe 'GET #openid_connect' do
let(:auth) { request.env['omniauth.auth'] }
let!(:identifier_scheme) { IdentifierScheme.create(name: auth.provider) }

context 'when the email is missing and the user does not exist' do
before do
# Simulate missing email
OmniAuth.config.mock_auth[:openid_connect].info.email = nil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we reset the email after this test in case we use it in another test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this test case, we want to ensure it returns a 'nil' value. Replacing the values will not guarantee the nil results. 

@request.env["omniauth.auth"] = OmniAuth.config.mock_auth[:openid_connect]
end

it 'redirects to the registration page with a flash message' do
get :openid_connect

expect(response).to redirect_to(new_user_registration_path)
expect(flash[:notice]).to eq('Something went wrong, Please try signing-up here.')
end
end

context 'when the user is not signed in but already exists' do
# let!(:user) { User.create(email: auth.info.email, password: 'password123') }
let!(:user) { User.create(email: '[email protected]', firstname: 'Test', surname: 'User', org: @org) }

before do
def User.from_omniauth(_auth)
User.find_by(email: '[email protected]')
end
end

it 'signs in the existing user' do
get :openid_connect
# expect(subject.current_user).to eq(user)
expect(response).to redirect_to(root_path)
expect(flash[:notice]).to be_nil
end
end

context 'when the user is signed in and needs to link their OpenID Connect account' do
let!(:user) { User.create(email: '[email protected]', firstname: 'Test', surname: 'User', org: @org) }
let(:current_user) { create(:user) }

before do
sign_in current_user

# Ensure from_omniauth returns nil, indicating no user is associated with the auth
# User.define_singleton_method(:from_omniauth) do |_auth|
# nil
# end
end

it "links identifier to current user, sets flash notice, and redirects to root path" do
expect {
get :openid_connect
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused why get :openid_connect is working.
get :openid_connect seems to behave like post :openid_connect within these tests, but the same is not true outside of the tests.
Screenshot from 2024-09-04 16-13-46
Screenshot from 2024-09-04 16-13-11

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated this

current_user.reload # Ensure we have the latest state of the user
}.to change(current_user.identifiers, :count).by(1)

expect(flash[:notice]).to eq('Linked successfully')
expect(response).to redirect_to(root_path)
end
end

context 'when the user found via omniauth is different from the current_user' do
let(:current_user) { create(:user) }
let!(:different_user) { create(:user, email: '[email protected]') } # Ensure different_user is created before test runs

before do
sign_in current_user

# Mocking the from_omniauth method to return a different user
# We use `let!` to ensure `different_user` is accessible here
User.define_singleton_method(:from_omniauth) do |_auth|
User.find_by(email: '[email protected]')
end
end

it "sets flash alert and redirects to edit user registration path" do
get :openid_connect

expect(flash[:alert]).to eq(
"The current #{@identifier_scheme.description} iD has been already linked to a user with email #{different_user.email}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be updated to match the changes you made in app/controllers/users/omniauth_callbacks_controller.rb:57

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String value is going to be same as expected here. I have changed the way of passing the variable into this string.

)
expect(response).to redirect_to(edit_user_registration_path)
end
end



context 'when an unknown error occurs' do
before do
def User.from_omniauth(_auth)
raise StandardError.new('Unexpected error')
end
end

it 'handles the error and raises an exception' do
expect {
get :openid_connect
}.to raise_error(StandardError, 'Unexpected error')
end
end
end
end
Loading