Skip to content

Add an option for disabling the requirement to verify email before publishing #3048

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .env.sample
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,7 @@ export GH_CLIENT_SECRET=
# Credentials for connecting to the Sentry error reporting service.
# export SENTRY_DSN_API=
export SENTRY_ENV_API=local

# If you don't require users to complete email verification before
# they can publish a crate, uncomment this line.
# export DISABLE_EMAIL_VERIFICATION_REQUIREMENT=1
12 changes: 11 additions & 1 deletion docs/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -439,12 +439,22 @@ OAuth Applications](https://github.com/settings/developers) and click on the
- Homepage URL: `http://localhost:4200/`
- Authorization callback URL: `http://localhost:4200/authorize/github`

Create the application, then take the Client ID ad Client Secret values and use
Create the application, then take the Client ID and Client Secret values and use
them as the values of the `GH_CLIENT_ID` and `GH_CLIENT_SECRET` in your `.env`.

Then restart your backend, and you should be able to log in to your local
crates.io with your GitHub account.

The next step is that you need to verify your email address to be able
to publish crates. When you log in for the first time, an email will
be sent with a verification link. In development, the sending of
emails is simulated by a file representing the email being created in
your local `/tmp/` directory. You need to find that email file and
follow the link to verify your email. As an easier alternative, you
can disable requiring a verified email in development by setting
`DISABLE_EMAIL_VERIFICATION_REQUIREMENT` in your `.env` file and
restart your backend.

Go to http://localhost:4200/me to get your API token and run the `cargo login`
command as directed.

Expand Down
8 changes: 8 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub struct Config {
pub blocked_traffic: Vec<(String, Vec<String>)>,
pub domain_name: String,
pub allowed_origins: Vec<String>,
pub require_email_verification: bool,
}

impl Default for Config {
Expand All @@ -44,6 +45,8 @@ impl Default for Config {
/// - `READ_ONLY_REPLICA_URL`: The URL of an optional postgres read-only replica database.
/// - `BLOCKED_TRAFFIC`: A list of headers and environment variables to use for blocking
///. traffic. See the `block_traffic` module for more documentation.
/// - `DISABLE_EMAIL_VERIFICATION_REQUIREMENT`: Disable the requirement that email must be
///. verified before publishing a crate.
fn default() -> Config {
let api_protocol = String::from("https");
let mirror = if dotenv::var("MIRROR").is_ok() {
Expand Down Expand Up @@ -142,6 +145,7 @@ impl Default for Config {
blocked_traffic: blocked_traffic(),
domain_name: domain_name(),
allowed_origins,
require_email_verification: require_email_verification(),
}
}
}
Expand All @@ -150,6 +154,10 @@ pub(crate) fn domain_name() -> String {
dotenv::var("DOMAIN_NAME").unwrap_or_else(|_| "crates.io".into())
}

fn require_email_verification() -> bool {
dotenv::var("DISABLE_EMAIL_VERIFICATION_REQUIREMENT").is_err()
Copy link
Member

Choose a reason for hiding this comment

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

if one had DISABLE_EMAIL_VERIFICATION_REQUIREMENT=0, won't this give a surprising result

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it would be surprising, but I think consistency with the other fields in the config is more important. There are a few others that follow this same pattern in this config, where it's just checking set or unset. For example, MIRROR and HEROKU.

}

fn blocked_traffic() -> Vec<(String, Vec<String>)> {
let pattern_list = dotenv::var("BLOCKED_TRAFFIC").unwrap_or_default();
parse_traffic_patterns(&pattern_list)
Expand Down
29 changes: 20 additions & 9 deletions src/controllers/krate/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,25 @@ pub fn publish(req: &mut dyn RequestExt) -> EndpointResult {
let api_token_id = ids.api_token_id();
let user = ids.user();

let verified_email_address = user.verified_email(&conn)?;
let verified_email_address = verified_email_address.ok_or_else(|| {
cargo_err(&format!(
"A verified email address is required to publish crates to crates.io. \
Visit https://{}/me to set and verify your email address.",
app.config.domain_name,
))
})?;
let email_address = if app.config.require_email_verification {
let verified_email_address = user.verified_email(&conn)?;
verified_email_address.ok_or_else(|| {
cargo_err(&format!(
"A verified email address is required to publish crates to crates.io. \
Visit https://{}/me to set and verify your email address.",
app.config.domain_name,
))
})?
} else {
Copy link
Member

Choose a reason for hiding this comment

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

Hm, I think what I would expect is that if DISABLE_EMAIL_VERIFICATION_REQUIREMENT is set, then we don't need to require the user to set any email.. that is, this else block shouldn't exist.

let email_address = user.email(&conn)?;
email_address.ok_or_else(|| {
cargo_err(&format!(
"An email address is required to publish crates to crates.io. \
Visit https://{}/me to set and verify your email address.",
app.config.domain_name,
))
})?
};

// Create a transaction on the database, if there are no errors,
// commit the transactions to record a new or updated crate.
Expand Down Expand Up @@ -152,7 +163,7 @@ pub fn publish(req: &mut dyn RequestExt) -> EndpointResult {
file_length as i32,
user.id,
)?
.save(&conn, &new_crate.authors, &verified_email_address)?;
.save(&conn, &new_crate.authors, &email_address)?;

insert_version_owner_action(
&conn,
Expand Down
1 change: 1 addition & 0 deletions src/tests/all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ fn simple_config() -> Config {
blocked_traffic: Default::default(),
domain_name: "crates.io".into(),
allowed_origins: Vec::new(),
require_email_verification: true,
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
[
{
"request": {
"uri": "http://alexcrichton-test.s3.amazonaws.com/crates/foo_unverified_email/foo_unverified_email-1.0.0.crate",
"method": "PUT",
"headers": [
[
"accept",
"*/*"
],
[
"content-length",
"35"
],
[
"host",
"alexcrichton-test.s3.amazonaws.com"
],
[
"accept-encoding",
"gzip"
],
[
"content-type",
"application/x-tar"
],
[
"authorization",
"AWS AKIAICL5IWUZYWWKA7JA:uDc39eNdF6CcwB+q+JwKsoDLQc4="
],
[
"date",
"Fri, 15 Sep 2017 07:53:06 -0700"
]
],
"body": "H4sIAAAAAAAA/+3AAQEAAACCIP+vbkhQwKsBLq+17wAEAAA="
},
"response": {
"status": 200,
"headers": [
[
"x-amz-request-id",
"26589A5E52F8395C"
],
[
"ETag",
"\"f9016ad360cebb4fe2e6e96e5949f022\""
],
[
"date",
"Fri, 15 Sep 2017 14:53:07 GMT"
],
[
"content-length",
"0"
],
[
"x-amz-id-2",
"JdIvnNTw53aqXjBIqBLNuN4kxf/w1XWX+xuIiGBDYy7yzOSDuAMtBSrTW4ZWetcCIdqCUHuQ51A="
],
[
"Server",
"AmazonS3"
]
],
"body": ""
}
}
]
26 changes: 26 additions & 0 deletions src/tests/krate/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,32 @@ fn new_krate_with_unverified_email_fails() {
);
}

#[test]
fn new_krate_records_with_unverified_email_if_email_verification_disabled() {
let (app, _, _, token) = TestApp::full()
.with_config(|c| c.require_email_verification = false)
.with_token();

app.db(|conn| {
update(emails::table)
.set((emails::verified.eq(false),))
.execute(conn)
.unwrap();
});

let crate_to_publish = PublishBuilder::new("foo_unverified_email");

token.enqueue_publish(crate_to_publish).good();

app.db(|conn| {
let email: String = versions_published_by::table
.select(versions_published_by::email)
.first(conn)
.unwrap();
assert_eq!(email, "[email protected]");
});
}

#[test]
fn new_krate_records_verified_email() {
let (app, _, _, token) = TestApp::full().with_token();
Expand Down