Skip to content
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

feat: require valid admin email for platform start #2217

Closed
wants to merge 1 commit into from

Conversation

zerbitx
Copy link
Contributor

@zerbitx zerbitx commented Oct 10, 2024

What issue type does this pull request address? (keep at least one, remove the others)
/kind enhancement

What does this pull request do? Which issues does it resolve? (use resolves #<issue_number> if possible)
resolves #ENG-4850

@zerbitx zerbitx marked this pull request as draft October 10, 2024 01:17
Copy link

netlify bot commented Oct 10, 2024

Deploy Preview for vcluster-docs canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 984ba66
🔍 Latest deploy log https://app.netlify.com/sites/vcluster-docs/deploys/67072b1d7cbac70008ba4be5

@zerbitx zerbitx marked this pull request as ready for review October 10, 2024 01:55
@zerbitx zerbitx requested a review from a team October 10, 2024 01:55
Comment on lines +164 to +179
func validateEmail(email string) error {
ret, err := verifier.Verify(email)
if err != nil {
return err
}

if ret.Disposable {
return fmt.Errorf(`disposable address "%s" not allowed`, email)
}

if !ret.HasMxRecords {
return fmt.Errorf(`address "%s" without MX record not allowed`, email)
}

return nil
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will perform SMTP or MX lookups from a CLI, potentially blocking folks in air-gapped or restricted networks from spinning up the platform.

I'd expect this logic in the platform code if connections can be established.
The CLI should only perform a quick sanity check using a regex or comparable mechanics.

Copy link
Member

@FabianKramm FabianKramm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm heavily against adding 132k lines of code for verfying an email address

@zerbitx zerbitx marked this pull request as draft October 10, 2024 22:50
@zerbitx zerbitx closed this Oct 11, 2024
@zerbitx zerbitx deleted the ENG-4850 branch October 11, 2024 13:14
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.

3 participants