Merge branch '326539-improve-resend-email-confirmation-banner-warning' into 'master'

Improve resend email confirmation banner warning

See merge request gitlab-org/gitlab!63322

app/assets/javascripts/pages/sessions/new/index.js

 import $ from 'jquery';
+import initVueAlerts from '~/vue_alerts';
 import NoEmojiValidator from '../../../emoji/no_emoji_validator';
 import LengthValidator from './length_validator';
 import OAuthRememberMe from './oauth_remember_me';
@@ -19,4 +20,5 @@ document.addEventListener('DOMContentLoaded', () => {
   // Save the URL fragment from the current window location. This will be present if the user was
   // redirected to sign-in after attempting to access a protected URL that included a fragment.
   preserveUrlFragment(window.location.hash);
+  initVueAlerts();
 });

app/assets/javascripts/vue_shared/components/dismissible_alert.vue

 <script>
-/* eslint-disable vue/no-v-html */
-import { GlAlert } from '@gitlab/ui';
+import { GlAlert, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui';
 
 export default {
+  name: 'DismissibleAlert',
   components: {
     GlAlert,
   },
+  directives: {
+    SafeHtml,
+  },
   props: {
     html: {
       type: String,
@@ -28,6 +31,6 @@ export default {
 
 <template>
   <gl-alert v-if="!isDismissed" v-bind="$attrs" @dismiss="dismiss" v-on="$listeners">
-    <div v-html="html"></div>
+    <div v-safe-html="html"></div>
   </gl-alert>
 </template>

app/views/layouts/_flash.html.haml

@@ -4,6 +4,8 @@
   - flash.each do |key, value|
     - if key == 'toast' && value
       .js-toast-message{ data: { message: value } }
+    - elsif value == I18n.t('devise.failure.unconfirmed')
+      = render 'shared/confirm_your_email_alert'
     - elsif value
       %div{ class: "flash-#{key} mb-2" }
         = sprite_icon(icons[key], css_class: 'align-middle mr-1') unless icons[key].nil?

app/views/shared/_confirm_your_email_alert.html.haml

+.js-vue-alert{ 'v-cloak': true,
+  data: { dismissible: 'true',
+  title: _('Please confirm your email address'),
+  primary_button_text: _('Resend confirmation email'),
+  primary_button_link: new_user_confirmation_path,
+  variant: 'warning'} }
+  = (_("To continue, you need to select the link in the confirmation email we sent to verify your email address. If you didn't get our email, select %{strongStart}Resend confirmation email.%{strongEnd}") % { strongStart: '<strong>', strongEnd: '</strong>' }).html_safe

locale/gitlab.pot

@@ -24290,6 +24290,9 @@ msgstr ""
 msgid "Please complete your profile with email address"
 msgstr ""
 
+msgid "Please confirm your email address"
+msgstr ""
+
 msgid "Please contact an admin to register runners."
 msgstr ""
 
@@ -33895,6 +33898,9 @@ msgstr ""
 msgid "To connect an SVN repository, check out %{svn_link}."
 msgstr ""
 
+msgid "To continue, you need to select the link in the confirmation email we sent to verify your email address. If you didn't get our email, select %{strongStart}Resend confirmation email.%{strongEnd}"
+msgstr ""
+
 msgid "To define internal users, first enable new users set to external"
 msgstr ""
 

spec/features/users/login_spec.rb

@@ -97,6 +97,8 @@ RSpec.describe 'Login' do
   describe 'with an unconfirmed email address' do
     let!(:user) { create(:user, confirmed_at: nil) }
     let(:grace_period) { 2.days }
+    let(:alert_title) { 'Please confirm your email address' }
+    let(:alert_message) { "To continue, you need to select the link in the confirmation email we sent to verify your email address. If you didn't get our email, select Resend confirmation email" }
 
     before do
       stub_application_setting(send_user_confirmation_email: true)
@@ -109,13 +111,14 @@ RSpec.describe 'Login' do
 
         gitlab_sign_in(user)
 
-        expect(page).not_to have_content(I18n.t('devise.failure.unconfirmed'))
+        expect(page).not_to have_content(alert_title)
+        expect(page).not_to have_content(alert_message)
         expect(page).not_to have_link('Resend confirmation email', href: new_user_confirmation_path)
       end
     end
 
     context 'when the confirmation grace period is expired' do
-      it 'prevents the user from logging in and renders a resend confirmation email link' do
+      it 'prevents the user from logging in and renders a resend confirmation email link', :js do
         travel_to((grace_period + 1.day).from_now) do
           expect(authentication_metrics)
             .to increment(:user_unauthenticated_counter)
@@ -123,7 +126,8 @@ RSpec.describe 'Login' do
 
           gitlab_sign_in(user)
 
-          expect(page).to have_content(I18n.t('devise.failure.unconfirmed'))
+          expect(page).to have_content(alert_title)
+          expect(page).to have_content(alert_message)
           expect(page).to have_link('Resend confirmation email', href: new_user_confirmation_path)
         end
       end
@@ -889,6 +893,8 @@ RSpec.describe 'Login' do
   context 'when sending confirmation email and not yet confirmed' do
     let!(:user) { create(:user, confirmed_at: nil) }
     let(:grace_period) { 2.days }
+    let(:alert_title) { 'Please confirm your email address' }
+    let(:alert_message) { "To continue, you need to select the link in the confirmation email we sent to verify your email address. If you didn't get our email, select Resend confirmation email" }
 
     before do
       stub_application_setting(send_user_confirmation_email: true)
@@ -906,7 +912,7 @@ RSpec.describe 'Login' do
     end
 
     context "when not having confirmed within Devise's allow_unconfirmed_access_for time" do
-      it 'does not allow login and shows a flash alert to confirm the email address' do
+      it 'does not allow login and shows a flash alert to confirm the email address', :js do
         travel_to((grace_period + 1.day).from_now) do
           expect(authentication_metrics)
             .to increment(:user_unauthenticated_counter)
@@ -915,7 +921,9 @@ RSpec.describe 'Login' do
           gitlab_sign_in(user)
 
           expect(current_path).to eq new_user_session_path
-          expect(page).to have_content(I18n.t('devise.failure.unconfirmed'))
+          expect(page).to have_content(alert_title)
+          expect(page).to have_content(alert_message)
+          expect(page).to have_link('Resend confirmation email', href: new_user_confirmation_path)
         end
       end
     end

spec/frontend/vue_alerts_spec.js

@@ -28,8 +28,8 @@ describe('VueAlerts', () => {
       alerts
         .map(
           (x) => `
-    <div class="js-vue-alert" 
-      data-dismissible="${x.dismissible}" 
+    <div class="js-vue-alert"
+      data-dismissible="${x.dismissible}"
       data-title="${x.title}"
       data-primary-button-text="${x.primaryButtonText}"
       data-primary-button-link="${x.primaryButtonLink}"

spec/frontend/vue_shared/components/dismissible_alert_spec.js

@@ -5,18 +5,12 @@ import DismissibleAlert from '~/vue_shared/components/dismissible_alert.vue';
 const TEST_HTML = 'Hello World! <strong>Foo</strong>';
 
 describe('vue_shared/components/dismissible_alert', () => {
-  const testAlertProps = {
-    primaryButtonText: 'Lorem ipsum',
-    primaryButtonLink: '/lorem/ipsum',
-  };
-
   let wrapper;
 
   const createComponent = (props = {}) => {
     wrapper = shallowMount(DismissibleAlert, {
       propsData: {
         html: TEST_HTML,
-        ...testAlertProps,
         ...props,
       },
     });
@@ -28,16 +22,13 @@ describe('vue_shared/components/dismissible_alert', () => {
 
   const findAlert = () => wrapper.find(GlAlert);
 
-  describe('with default', () => {
+  describe('default', () => {
     beforeEach(() => {
       createComponent();
     });
 
     it('shows alert', () => {
-      const alert = findAlert();
-
-      expect(alert.exists()).toBe(true);
-      expect(alert.props()).toEqual(expect.objectContaining(testAlertProps));
+      expect(findAlert().exists()).toBe(true);
     });
 
     it('shows given HTML', () => {
@@ -54,4 +45,32 @@ describe('vue_shared/components/dismissible_alert', () => {
       });
     });
   });
+
+  describe('with additional props', () => {
+    const testAlertProps = {
+      dismissible: true,
+      title: 'Mock Title',
+      primaryButtonText: 'Lorem ipsum',
+      primaryButtonLink: '/lorem/ipsum',
+      variant: 'warning',
+    };
+
+    beforeEach(() => {
+      createComponent(testAlertProps);
+    });
+
+    it('passes other props', () => {
+      expect(findAlert().props()).toEqual(expect.objectContaining(testAlertProps));
+    });
+  });
+
+  describe('with unsafe HTML', () => {
+    beforeEach(() => {
+      createComponent({ html: '<a onclick="alert("XSS")">Link</a>' });
+    });
+
+    it('removes unsafe HTML', () => {
+      expect(findAlert().html()).toContain('<a>Link</a>');
+    });
+  });
 });