Disable auto admin mode in specs
What does this MR do?
In spec/spec_helper.rb rename the existing :do_not_mock_admin_mode
tag to :enable_admin_mode
and invert the logic: protect the mocking with an if
, and modify all specs that require administrative access to explicitly require it.
Adapt specs to either:
- mock the whole spec using the
:enable_admin_mode
metadata, or - test both cases with / without admin mode mocking individual specs using the
:enable_admin_mode
metadata, or - in feature specs use the login helper
LoginHelpers#gitlab_enable_admin_mode_sign_in
that actually enables admin mode via UI actions
Controller tests
- Controllers under
Admin::**
get:enable_admin_mode
for all specs in the topdescribe
block, since access to them is already limited to admin mode by concern EnforcesAdminAuthentication - Non-
Admin::**
controllers:- For some both variants get tested with/without admin mode in contexts
- There's others that need significant re-work and domain knowledge, we'd need to follow-up
Feature tests
- Feature tests use
LoginHelpers#gitlab_enable_admin_mode_sign_in
to go through forms input whenever they use an admin user. Since !27318 (merged) we do not check other sessions for admin mode, so:clean_gitlab_redis_shared_state
is not needed anymore in specs to make sure that the state of other previous sessions does not interfere between specs -
admin/**
feature tests they get the admin mode at the startbefore
block - Non-
admin/**
feature tests, some get admin/non-admin mode tested in contexts
Policies
- I started migrating some of them to test both with / without, but not all of them, it's a quite big task. Also requires some knowledge of basically the whole platform, so they need to be checked and also in the future followed-up to test all cases.
Lib, Finders, Models, Requests, Serializers, View, Workers Specs
- Those that use admin users get
:enable_admin_mode
in relevant contexts, some are tested with both admin mode enabled / disabled
Shared Examples
- Adapted some shared examples under
spec/support/shared_examples
to test both with / without admin mode - Some tests have been disabled with
xit
because I didn't fully grasp if I was testing the right thing, need to be re-checked.
Service Tests
- There's a lot of services that are tested using admin users and in those I have enabled admin mode via
:enable_admin_mode
, but we need to make sure this will work properly, e.g.:-
spec/services/ci/create_pipeline_service_spec.rb
looks like one of those cases where we might have sidekiq jobs expecting to have administrative rights - all the
CycleAnalitics
models and services
-
Frontend Fixtures
- For fixtures that use controllers and administrator users, I have directly used the
:enable_admin_mode
metadata instead of going through the whole UI process, it seems unnecessary
Fixes #31511 (closed)
Screenshots
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Edited by 🤖 GitLab Bot 🤖