Skip to content

Refactor tests related to Omniauth users

Etienne Baqué requested to merge 338980-fix-oauth-ldap-tests into master

What does this MR do and why?

Related to #353534 (closed)

This MR is extracting changes initially introduced in !81272 (merged). In creating this code extraction via this MR, I'm taking a more conservative approach as I see the core changes in !81272 (merged) sensitive.

This MR primarily fixes the :omniauth_user factory, by introducing a :ldap trait. As outlined in this documentation, Omniauth login and LDAP login are two different mechanism and this is not currently reflected in the tests correctly, as we have the following:

user = create(:omniauth_user)

user.ldap_user? # currently returns `true`

From a definition point of view, having :ldap as a trait within :omniauth_user is not totally correct, but this change at least allows to introduce this difference of providers. To create a detached :ldap factory would be a bigger change that's outside the scope of what we're trying to fix initially.

This MR also focuses on refactoring some tests in spec/lib/gitlab/auth/o_auth/user_spec.rb into some shared_examples statements.

Finally, this MR also includes some small refactoring around the provider_name used in the OAuth::User related classes.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #338980 (closed)

Edited by Etienne Baqué

Merge request reports

Loading