Skip to content

Conversation

@muisit
Copy link
Contributor

@muisit muisit commented Aug 29, 2018

Our use case is that we set the OrgIdentity using OIS based on SAML verified data, so all OrgIdentity information is considered validated and email addresses do not need to be verified further. However, users may enter a preferred external email address (if so configured for the flow), which is linked to the CoPerson. That address does need to be verified, instead of the OrgIdentity address.

This fix allows skipping the email confirmation step if a verified address exists for (either) OrgIdentity or CoPerson. It makes sure an EmailAddress exists for OrgIdentity and throws an ugly exception elsewise (like in the main branch).

This PR also selects the first unverified CoPerson email address for verification if such an address exists. The use case is to allow users to enter an optional preferred email address during enrollment. If not entered, the whole step can be skipped (as we have a SAML verified email address on the OI), but if entered, we want to verify exactly that address.

This fix does not check multiple CoPerson email addresses, nor multiple email addresses linked to OrgIdentity records created through other OIS or pipelines. The use case seems far fetched and the proper way of handling that is unclear at the moment (ie: sending out multiple invites at once or one-at-a-time repeatedly).
Unfortunately, it seems like users cannot verify email addresses themselves at the moment after enrollment (see CO-1648).

As for the actual patch: I decided to link the email address id to the invite for this enrollment flow verification step. This allows me to reuse a simpler code path in the CoInvitesController and remove the part where OrgIdentities are determined based on the petition: the OrgIdentity or CoPersonId are stored conveniently on the EmailAddress record and the code comments did not give any hint whatsoever as to why linking to the EmailAddress record might be considered a security problem.

During the petition, I check for 'needConfirmation' to see if there is any email address unverified. sendConfirmation than sends a confirmation to exactly that address, instead of having to re-establish the OrgIdentity and picking the first email address of the OrgIdentity.

I refactored creating an enrolleeToken and removed it from the (unexpected) CoInvitesController to the (imho more logical) CoPetition model. In this way you can retrieve-or-create the token easily from other places. I needed this, because the code switches petition tokens between the sendConfirmation and processConfirmation steps.

…esses present are already considered verified. Also, this fix allows verifying a CoPerson email address if such an address was entered during enrollment and the OrgIdentity email address was already considered verified.
ioigoume pushed a commit to ioigoume/comanage-registry that referenced this pull request Aug 26, 2020
…_related_to_rcauth_initial_implementation

FIX_remove_core_changes_related_to_rcauth_initial_implementation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant