Skip to content

Commit

Permalink
Merge pull request #294 from betagouv/fix-notify-all-email
Browse files Browse the repository at this point in the history
fix: notify all email send after peer authentication, not before
  • Loading branch information
rdubigny authored Jul 28, 2023
2 parents 0114076 + 5c54e01 commit 09b3a13
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 34 deletions.
2 changes: 1 addition & 1 deletion cypress/e2e/join_org_with_verified_domain.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe('join organizations', () => {
// assert reception of notification email
.then(email => {
expect(email.body).to.match(
/.*Jean Nouveau.*\([email protected]\) a rejoint votre organisation.*Commune de clamart - Mairie.*sur .*MonComptePro/
/.*Jean Nouveau.*\(c6c64542-5601-43e0-b320-b20da72f6edc@mailslurp\.com\) a rejoint votre organisation.*Commune de clamart - Mairie.*sur .*MonComptePro/
);
});

Expand Down
15 changes: 15 additions & 0 deletions cypress/e2e/join_with_code_sent_to_official_contact_email.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,20 @@ describe('join organizations', () => {
});

cy.contains('Votre compte est créé');

cy.mailslurp()
.then(mailslurp =>
mailslurp.waitForLatestEmail(
'26ccc0fa-0dc3-4f12-9335-7bb00282920c',
60000,
true
)
)
// assert reception of notification email
.then(email => {
expect(email.body).to.match(
/.*Jean Nouveau.*\(c348a2c3-bf54-4f15-bb12-a2d7047c832f@mailslurp\.com\) a rejoint votre organisation.*Commune de lamalou-les-bains - Mairie.*sur .*MonComptePro/
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,16 @@ INSERT INTO users
(id, email, email_verified, email_verified_at, encrypted_password, created_at, updated_at, given_name, family_name,
phone_number, job)
VALUES
(1, '[email protected]', true, CURRENT_TIMESTAMP, '$2a$10$kzY3LINL6..50Fy9shWCcuNlRfYq0ft5lS.KCcJ5PzrhlWfKK4NIO', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP, 'Jean', 'Nouveau', '0123456789', 'Sbire');
(1, '[email protected]', true, CURRENT_TIMESTAMP, '$2a$10$kzY3LINL6..50Fy9shWCcuNlRfYq0ft5lS.KCcJ5PzrhlWfKK4NIO', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP, 'Jean', 'Officiel', '0123456789', 'Sbire'),
(2, '[email protected]', true, CURRENT_TIMESTAMP, '$2a$10$kzY3LINL6..50Fy9shWCcuNlRfYq0ft5lS.KCcJ5PzrhlWfKK4NIO', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP, 'Jean', 'Nouveau', '0123456789', 'Sbire');


INSERT INTO organizations
(id, siret, verified_email_domains, authorized_email_domains, created_at, updated_at)
VALUES
(1, '21340126800130', '{}', '{}', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP);

INSERT INTO users_organizations
(user_id, organization_id, is_external, verification_type, authentication_by_peers_type, has_been_greeted)
VALUES
(1, 1, false, null, 'all_members_notified', true)
3 changes: 0 additions & 3 deletions src/controllers/organization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
getOrganizationSuggestions,
joinOrganization,
} from '../managers/organization/join';
import { authenticateByPeers } from '../managers/organization/authentication-by-peers';

export const getJoinOrganizationController = async (
req: Request,
Expand Down Expand Up @@ -108,8 +107,6 @@ export const postJoinOrganizationMiddleware = async (
user_id: req.session.user!.id,
});

await authenticateByPeers(userOrganizationLink);

if (req.session.mustReturnOneOrganizationInPayload) {
await selectOrganization({
user_id: req.session.user!.id,
Expand Down
22 changes: 0 additions & 22 deletions src/managers/organization/authentication-by-peers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
NotFoundError,
UserAlreadyAskedForSponsorshipError,
} from '../../errors';
import { isEligibleToSponsorship } from '../../services/organization';
import { sendMail } from '../../connectors/sendinblue';
import { updateUserOrganizationLink } from '../../repositories/organization/setters';
import { getOrganizationsByUserId } from './main';
Expand All @@ -19,27 +18,6 @@ import {
} from '../../repositories/moderation';
import { SUPPORT_EMAIL_ADDRESS } from '../../env';

export const authenticateByPeers = async (
link: UserOrganizationLink
): Promise<{ hasBeenAuthenticated: boolean }> => {
const { organization_id, user_id, is_external } = link;
const organizationUsers = await getUsers(organization_id);
const user = organizationUsers.find(({ id }) => id === user_id);
const organization = await findOrganizationById(organization_id);

// The user should be in the organization already
if (isEmpty(user) || isEmpty(organization)) {
throw new NotFoundError();
}

if (isEligibleToSponsorship(organization)) {
return { hasBeenAuthenticated: false };
}

await notifyAllMembers(link);

return { hasBeenAuthenticated: true };
};
export const notifyAllMembers = async ({
organization_id,
user_id,
Expand Down
22 changes: 15 additions & 7 deletions src/middlewares/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ import { isEmpty } from 'lodash';
import { isUrlTrusted } from '../services/security';
import { updateEmailAddressVerificationStatus } from '../managers/user';
import { isEligibleToSponsorship } from '../services/organization';
import { NotImplemented } from 'http-errors';
import { getOrganizationsByUserId } from '../managers/organization/main';
import { greetForJoiningOrganization } from '../managers/organization/authentication-by-peers';
import {
greetForJoiningOrganization,
notifyAllMembers,
} from '../managers/organization/authentication-by-peers';
import { getSelectedOrganizationId } from '../repositories/redis/selected-organization';
import { getUserOrganizationLink } from '../repositories/organization/getters';

// redirect user to start sign in page if no email is available in session
export const checkEmailInSessionMiddleware = async (
Expand Down Expand Up @@ -227,15 +230,20 @@ export const checkUserHasBeenAuthenticatedByPeersMiddleware = (

if (!isEmpty(organizationThatNeedsAuthenticationByPeers)) {
if (
!isEligibleToSponsorship(organizationThatNeedsAuthenticationByPeers)
isEligibleToSponsorship(organizationThatNeedsAuthenticationByPeers)
) {
// this should never happen as all members are notified by default
return next(new NotImplemented());
return res.redirect(
`/users/choose-sponsor/${organizationThatNeedsAuthenticationByPeers.id}`
);
}

return res.redirect(
`/users/choose-sponsor/${organizationThatNeedsAuthenticationByPeers.id}`
const link = await getUserOrganizationLink(
organizationThatNeedsAuthenticationByPeers.id,
req.session.user!.id
);

// link exists because we get the organization id from getOrganizationsByUserId above
await notifyAllMembers(link!);
}

return next();
Expand Down

0 comments on commit 09b3a13

Please sign in to comment.