What does this MR do and why?
- new users have to confirm the Terms of Service on first login using omnitauth (Closes #429070)
- all users have to confirm the updated Terms of Service if they are changed (Closes #345524 (closed))
Old behavior
- user logged in first time
- on some point method
sign_in_user_flow
is called - if the user was new, the former method
persist_accepted_terms_if_required
was used -
persist_accepted_terms_if_required
did accept the latest ToS automatically L319, L84 if: - the ToS Page wasn't displayed
Description of how and why
- the method
User.terms_accepted?
did only check if some ToS was accepted by the user not which version (Fixes #345524 (closed)) - the former method
persist_accepted_terms_if_required
- was implemented twice in two classes, doing exactly the same so i moved it to model
User
- was not named appropriate to that what it does, it accepts the ToS automatically, so i renamed it to
autoaccept_terms_if_not_enforced
- had a logical fail in
return unless Gitlab::CurrentSettings.current_application_settings.enforce_terms?
- the method should be left if its mandatory that the latest version of the ToS is accepted by the user, this leads in displaying the ToS page later (Fixes #429070)
- otherwise the ToS are accepted automatically (imo this is not the best solution (not now, not in the former implementation) but in doesn't break the former logic
- was implemented twice in two classes, doing exactly the same so i moved it to model
New behavior
- user log in first time
- on some point method
sign_in_user_flow
is called - if the user is new, the new instance method
@user.autoaccept_terms_if_not_enforced
is used -
@user.autoaccept_terms_if_not_enforced
accept the latest ToS automaticaly if: - the ToS Page is displayed
The ToS page is also displayed to the users (including admins, except *_bots) immediately if the admin updated the ToS text.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Edited by boontifex