Pick `inactive_message` for inactive users from `devise.en.yml`
What does this MR do?
Currently, when an inactive user tries to login to GitLab, login is prevented and an alert message is shown. This is done using Devise.
Devise depends on the inactive_message
message method to choose which message to show to the user upon failure.
Problem:
This is not really a problem now, but it hinders progress on a feature we are currently building: &4491.
In this upcoming feature, we have the requirement to show a user, immediately after their sign up that "Your signup is successful, but you cannot login as your account is pending approval from admin", which cannot be done without this change.
Change:
If we check Devise, we can see that inactive_message
is overwritten in many places like here & here, and in these places it returns only the key
like :inactive
or :unconfirmed
. This key is further used internally by Devise to get the right, full message from devise.en.yml
However, in GitLab, we override inactive_message
, but instead of returning keys and defining the associated full message in devise.en.yml
, we were returning the whole message. This is being changed with this MR.
The natural question here would be: 'How was this working as expected till now?'
The answer is that in this specific place, Devise picks the full message via the "key -> devise.en.yml" route if the value is a key (like :inactive
) and if the value is not a key (ie, a Symbol
) it just considers it as a full message. Please note that this is applicable only in case of failed signins, and not signups.
However, this "trick" does not help in the use case we are going to build.
In our new use case, we would mandatorily require the message to be present in the devise.en.yml
, as we will be dealing with signups.
In this place, the flash message to be shown after a user signs up and is not active, is obtained from devise.en.yml
only.
If we go by the above Devise code, we'd have to add a new entry in the devise.en.yml
file named signed_up_but_blocked_pending_approval
and the inactive_message
method would turn to
def inactive_message
if blocked_pending_approval?
:blocked_pending_approval
elsif blocked?
:blocked
elsif internal?
:forbidden
else
super
end
end
to make the new feature show the notice as expected on a new signup.
Screenshots
Existing notices work as expected after the change.
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