Repository navigation
STF-2006: Add email verification inputs #305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
3ab673a
2277b09
9b75471
f97e800
58d0b12
6a19605
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,12 +31,32 @@ class Email < Base | |
| # @return [Boolean, nil] | ||
| attr_accessor :hash_address | ||
|
|
||
| # The date and time of the most recent attempt to verify the email | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: the email and phone time docs now use different wording. This says "the most recent attempt to verify the email address, if known". The two inputs have the same meaning: the time also applies when 🤖 Comment by Claude Opus 5.5.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given it's about phone, we can leave it |
||
| # address, if known. The string must be in the RFC 3339 date-time | ||
| # format, e.g., "2012-04-12T23:20:50.52Z". The time must not be before | ||
| # 1970-01-01T00:00:00Z or more than two days in the future. The web | ||
| # service ignores a time outside this range and returns a warning. | ||
| # | ||
| # @see https://datatracker.ietf.org/doc/html/rfc3339 | ||
| # | ||
| # @return [String, nil] | ||
| attr_accessor :verification_time | ||
|
|
||
| # Whether the most recent verification that the end user controls the | ||
| # email address succeeded, for example by confirming a link sent to the | ||
| # address. Do not include this field if no verification was attempted. | ||
| # | ||
| # @return [Boolean, nil] | ||
| attr_accessor :was_verification_successful | ||
|
|
||
| # @param params [Hash] Hash of parameters. Each key/value should | ||
| # correspond to one of the available attributes. | ||
| def initialize(params = {}) | ||
| @address = params[:address] | ||
| @domain = params[:domain] | ||
| @hash_address = params[:hash_address] | ||
| @address = params[:address] | ||
| @domain = params[:domain] | ||
| @hash_address = params[:hash_address] | ||
| @verification_time = params[:verification_time] | ||
| @was_verification_successful = params[:was_verification_successful] | ||
|
|
||
| validate | ||
| end | ||
|
|
@@ -72,6 +92,8 @@ def validate | |
|
|
||
| validate_email('email', @address) | ||
| validate_string('domain', 255, @domain) | ||
| validate_rfc3339('verification_time', @verification_time) | ||
| validate_boolean('was_verification_successful', @was_verification_successful) | ||
| end | ||
|
|
||
| def hash_email_address(address) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| # The prefix is the text before verification_time and | ||
| # was_verification_successful in the field names, such as 'phone_'. | ||
| RSpec.shared_examples 'a component with verification inputs' do |prefix| | ||
| let(:time_field) { :"#{prefix}verification_time" } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Suggest block-local variables, such as 🤖 Comment by Claude Opus 5.5.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reply by Claude Opus 5.5 on Greg's behalf. We will keep the |
||
| let(:successful_field) { :"#{prefix}was_verification_successful" } | ||
|
|
||
| [true, false].each do |successful| | ||
| it "sends #{prefix}was_verification_successful #{successful} as a JSON boolean" do | ||
| component = described_class.new( | ||
| time_field => '2026-10-01T14:30:00Z', | ||
| successful_field => successful, | ||
| ) | ||
| expect(component.to_json).to eq( | ||
| time_field.to_s => '2026-10-01T14:30:00Z', | ||
| successful_field.to_s => successful, | ||
| ) | ||
| end | ||
| end | ||
|
|
||
| context 'with verification validation' do | ||
| around do |example| | ||
| previous = Minfraud.enable_validation | ||
|
|
||
| Minfraud.configure { |c| c.enable_validation = 1 } | ||
| example.run | ||
| ensure | ||
| Minfraud.configure { |c| c.enable_validation = previous } | ||
| end | ||
|
|
||
| it "raises an exception for an invalid #{prefix}verification_time" do | ||
| expect do | ||
| described_class.new(time_field => '2026-10-01 14:30:00') | ||
| end.to raise_exception(Minfraud::InvalidInputError) | ||
| end | ||
|
|
||
| it "raises an exception for a boolean #{prefix}verification_time" do | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: These boolean checks repeat the tables in This example checks only for Suggest: add 🤖 Comment by Claude Opus 5.5.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reply by Claude Opus 5.5 on Greg's behalf. Fixed in 58d0b12. The example now matches |
||
| expect do | ||
| described_class.new(time_field => true) | ||
| end.to raise_exception(Minfraud::InvalidInputError, /must not be a boolean/) | ||
| end | ||
|
|
||
| it "raises an exception for an invalid #{prefix}was_verification_successful" do | ||
| expect do | ||
| described_class.new(successful_field => 'true') | ||
| end.to raise_exception(Minfraud::InvalidInputError) | ||
| end | ||
|
|
||
| it "raises an exception for a non-boolean #{prefix}was_verification_successful" do | ||
| expect do | ||
| described_class.new(successful_field => 1) | ||
| end.to raise_exception(Minfraud::InvalidInputError) | ||
| end | ||
|
|
||
| it 'does not raise an exception for valid verification values' do | ||
| described_class.new( | ||
| time_field => '2026-10-01T14:30:00Z', | ||
| successful_field => false, | ||
| ) | ||
| end | ||
| end | ||
| end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: in this example,
verification_time(2026-10-01) is 14 years after the example eventtime(2012-04-12, line 93). The "most recent verification" then comes after the transaction that it describes.The
phone_verification_timevalues (lines 119 and 136) have the same problem. They are not new in this PR.Suggested fix: use a verification time before the event time, or move the event time to a recent date.
🤖 Comment by Claude Opus 5.5.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reply by Claude Opus 5.5 on Greg's behalf.
Fixed in 6a19605. The example event time is now 2026-10-05, after every verification time. This also fixes the phone examples.