add automatic verification threshold field

[mfrancisc]

Description

Add a new field for configuring the automatic verification threshold.

Jira: https://issues.redhat.com/browse/SANDBOX-495

Checks

1. Did you run make generate target? yes

2. Did make generate change anything in other projects (host-operator)? yes

3. In case of new CRD, did you the following? N/A

4. In case other projects are changed, please provides PR links.

   • host-operator: add automatic verification threshold field host-operator#942

[MatousJobanek]

I would maybe call it
MinimalRequiredScoreThreshold or just MinimalRequiredScore because if the user gets a low score, then we don't let the user even try to complete the phone verification. In other words, it's not only about automatic approval - that's what the ScoreThreshold field is used for actually:

// ScoreThreshold defines the captcha assessment score threshold. A score equal to or above the threshold means the user is most likely human and
// can proceed signing up but a score below the threshold means the score is suspicious and further verification may be required.
// +optional
ScoreThreshold *string `json:"scoreThreshold,omitempty"`

but even not letting users try to proceed with additional verification steps nor the signup process at all.

[mfrancisc]

Thanks for your suggestion @MatousJobanek !

I was unsure about the naming (as usual 😅 ) , I thought about something using the words minimal required . But TBH to me it doesn't explain the purpose of the configuration. It may not be clear what happens if the score is lower then the config. While the "automatic verification" implies that the user can proceed with the verification (phone, email, 2fa ... ) without an admin approval/intervention. Maybe autonomous or self verification would be better words 🤔

But again I don't have a strong opinion on the naming, whatever makes more sense to people 🤷‍♂️

[alexeykazakov]

In other words, it's not only about automatic approval - that's what the ScoreThreshold field is used for actually

Well... automatic approval is configured via AutomaticApprovalConfig. Not via ScoreThrershold. Even if PhoneVerification is required it's still auto approval if it's enabled in AutomaticApprovalConfiguration. Vs. a manual approval by an admin.
We could move this new field to AutomaticApprovalConfig and call it something like MinimalRequiredCaptchaScore or keep it under the Captcha configuration and call it something like MinimalRequiredScoreForAutoApproval if it's not too long :)
Otherwise I would probably prefer @mfrancisc's variant.

[MatousJobanek]

I'm idiot, I read "AutomaticApprovalThreshold" instead of what is there "AutomaticVerificationThreshold". I have some issues with my eyes in the last two days, so it's sometimes hard to focus on the words on the display, or I just maybe need a coffee.

I got maybe also confused by the comment talking about "automatic approval:

// AutomaticVerificationThreshold defines the lowest captcha score for automatic approval to be enabled.
// If the captcha score is below this threshold then manual approval is required in order for the user to be provisioned.

Because as Alexey mentioned, this threshold doesn't have anything to do with the automatic/manual approval, it's about proceeding with the signup process.
I would at least drop the "Automatic" prefix to not confuse more people.

so other ideas:

• SignupProcessThreshold
• RequiredScore
• HardStopThreshold
• GatekeeperScore
• SignupConfidenceLimit

Or just go with your name and update the comments so it doesn't talk about the automatic/manual approval, but rather that the user cannot proceed with the signup process at all

[mfrancisc]

No worries, and yeah the comment it's unclear.

Thanks for your suggestions, I like both SignupProcessThreshold and RequiredScore.
Maybe we can go with RequiredScore since it's also closer to what @alexeykazakov is suggesting ?

[mfrancisc]

it will be config.Spec.Host.RegistrationService.Verification.Captcha.RequiredScore

[MatousJobanek]

sounds good 👍

[mfrancisc]

renamed in 9247ee8

I've also added a new field in baa8ab5 as per slack discussion.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[MatousJobanek]

Nice, thanks 👍

[mfrancisc]

@alexeykazakov could you PTAL when you have some time?

[alexeykazakov]

Looks good to me!