review try-out: author the PR branch at the account's noreply address
The commit author's email was the forge account's own address, read with the site-admin token, which put an email the account may keep private into the target repo's history. It is now the address Forgejo itself uses for such an account's web commits, `<lowercased login>@<NO_REPLY_ADDRESS>` (models/user/user.go GetPlaceholderEmail). NO_REPLY_ADDRESS defaults to `noreply.<DOMAIN>` (modules/setting/service.go) and the forge module leaves it unset, with DOMAIN set from services.hyperhive.swarm.forge.domain, the same option SWARM_CONTROLLER_FORGE_URL is built from; so the domain is that URL's host. The account is still read, to refuse a user with no forge account, but its email no longer is. Refs #5259
This commit is contained in:
parent
36c3f38717
commit
c90bd705a7
2 changed files with 39 additions and 14 deletions
|
|
@ -484,8 +484,11 @@ branch, where you log in as usual and open the PR yourself.
|
||||||
- **The author** is the forge account named like your swarm UI login. A forge
|
- **The author** is the forge account named like your swarm UI login. A forge
|
||||||
account made by an SSO login carries the SSO username, so it's yours once
|
account made by an SSO login carries the SSO username, so it's yours once
|
||||||
you've logged in to the forge. The commit carries its full name, or the
|
you've logged in to the forge. The commit carries its full name, or the
|
||||||
login when it has none, and its email. The committer is the service
|
login when it has none, and its noreply address,
|
||||||
account.
|
`<login, lowercased>@noreply.<forge domain>`. It's the address the forge
|
||||||
|
itself uses for an account that keeps its email private, so the commit
|
||||||
|
links to your account and your email stays out of the repo's history. The
|
||||||
|
committer is the service account.
|
||||||
- **The token** is the service account's own, read from the store for each
|
- **The token** is the service account's own, read from the store for each
|
||||||
push (see [The service account](#the-service-account)), so the account
|
push (see [The service account](#the-service-account)), so the account
|
||||||
needs **write** access to the repo.
|
needs **write** access to the repo.
|
||||||
|
|
|
||||||
|
|
@ -185,22 +185,21 @@ impl Client {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The forge account `login` as a commit author: its full name, or its
|
/// The forge account `login` as a commit author: its full name, or its
|
||||||
/// login when it has none, and its email. `None` when the forge has no
|
/// login when it has none, and its [`noreply_address`], so the commit
|
||||||
/// such account. Read with this daemon's site-admin token, which sees an
|
/// links to the account without showing its email. `None` when the
|
||||||
/// email the account keeps private.
|
/// forge has no such account.
|
||||||
///
|
///
|
||||||
/// # Errors
|
/// # Errors
|
||||||
/// When the forge refuses the read, or the account has no email.
|
/// When the forge refuses the read, or this daemon's forge URL has no
|
||||||
|
/// host.
|
||||||
pub async fn commit_author(&self, login: &str) -> Result<Option<CommitAuthor>> {
|
pub async fn commit_author(&self, login: &str) -> Result<Option<CommitAuthor>> {
|
||||||
let user = match self.api.call(|api| api.user_get(login).into_future()).await {
|
let user = match self.api.call(|api| api.user_get(login).into_future()).await {
|
||||||
Ok(user) => user,
|
Ok(user) => user,
|
||||||
Err(e) if is_not_found(&e) => return Ok(None),
|
Err(e) if is_not_found(&e) => return Ok(None),
|
||||||
Err(e) => return Err(e).with_context(|| format!("read forge user {login}")),
|
Err(e) => return Err(e).with_context(|| format!("read forge user {login}")),
|
||||||
};
|
};
|
||||||
let email = user
|
let login = user.login.as_deref().unwrap_or(login);
|
||||||
.email
|
let email = noreply_address(&self.url, login)?;
|
||||||
.filter(|e| !e.trim().is_empty())
|
|
||||||
.with_context(|| format!("forge user {login} has no email"))?;
|
|
||||||
let name = user
|
let name = user
|
||||||
.full_name
|
.full_name
|
||||||
.map(|n| n.trim().to_owned())
|
.map(|n| n.trim().to_owned())
|
||||||
|
|
@ -301,6 +300,19 @@ impl Client {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The address the forge itself puts on a web commit by `login` when the
|
||||||
|
/// account keeps its email private: the lowercased login at
|
||||||
|
/// `NO_REPLY_ADDRESS`. The forge module leaves that at its default,
|
||||||
|
/// `noreply.<DOMAIN>`, and sets `DOMAIN` from
|
||||||
|
/// `services.hyperhive.swarm.forge.domain`, the option this daemon's forge
|
||||||
|
/// URL `forge` is built from, so the domain is that URL's host.
|
||||||
|
fn noreply_address(forge: &url::Url, login: &str) -> Result<String> {
|
||||||
|
let domain = forge
|
||||||
|
.host_str()
|
||||||
|
.with_context(|| format!("the forge URL {forge} names no host"))?;
|
||||||
|
Ok(format!("{}@noreply.{domain}", login.to_lowercase()))
|
||||||
|
}
|
||||||
|
|
||||||
/// Make `commit` with `forge`'s identity as committer, through the contents
|
/// Make `commit` with `forge`'s identity as committer, through the contents
|
||||||
/// API.
|
/// API.
|
||||||
async fn push_branch(forge: &Forgejo, owner: &str, repo: &str, commit: BranchCommit) -> Result<()> {
|
async fn push_branch(forge: &Forgejo, owner: &str, repo: &str, commit: BranchCommit) -> Result<()> {
|
||||||
|
|
@ -504,28 +516,29 @@ mod tests {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn a_commit_author_is_the_accounts_name_and_email_or_none() {
|
async fn a_commit_author_is_the_accounts_name_at_its_noreply_address_or_none() {
|
||||||
use super::super::tests::stub_forge;
|
use super::super::tests::stub_forge;
|
||||||
|
|
||||||
let (client, _) = stub_forge(
|
let (client, _) = stub_forge(
|
||||||
(404, "{}"),
|
(404, "{}"),
|
||||||
(
|
(
|
||||||
200,
|
200,
|
||||||
r#"{"login":"alice","full_name":" Alice A ","email":"a@x.test"}"#,
|
r#"{"login":"Alice","full_name":" Alice A ","email":"private@x.test"}"#,
|
||||||
),
|
),
|
||||||
(404, "{}"),
|
(404, "{}"),
|
||||||
)
|
)
|
||||||
.await;
|
.await;
|
||||||
|
let domain = client.url.host_str().expect("a host").to_owned();
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
client.commit_author("alice").await.expect("read"),
|
client.commit_author("alice").await.expect("read"),
|
||||||
Some(CommitAuthor {
|
Some(CommitAuthor {
|
||||||
name: "Alice A".to_owned(),
|
name: "Alice A".to_owned(),
|
||||||
email: "a@x.test".to_owned(),
|
email: format!("alice@noreply.{domain}"),
|
||||||
})
|
})
|
||||||
);
|
);
|
||||||
let (client, _) = stub_forge(
|
let (client, _) = stub_forge(
|
||||||
(404, "{}"),
|
(404, "{}"),
|
||||||
(200, r#"{"login":"bob","full_name":"","email":"b@x.test"}"#),
|
(200, r#"{"login":"bob","full_name":"","email":""}"#),
|
||||||
(404, "{}"),
|
(404, "{}"),
|
||||||
)
|
)
|
||||||
.await;
|
.await;
|
||||||
|
|
@ -540,6 +553,15 @@ mod tests {
|
||||||
assert_eq!(client.commit_author("nobody").await.expect("read"), None);
|
assert_eq!(client.commit_author("nobody").await.expect("read"), None);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn the_noreply_address_is_the_lowercased_login_at_noreply_dot_the_forge_host() {
|
||||||
|
let forge = url::Url::parse("https://Forge.Example:8443/").expect("a URL");
|
||||||
|
assert_eq!(
|
||||||
|
noreply_address(&forge, "Alice-B").expect("a host"),
|
||||||
|
"alice-b@noreply.forge.example"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn the_job_locks_down_the_classifiers_own_account() {
|
async fn the_job_locks_down_the_classifiers_own_account() {
|
||||||
use super::super::tests::{lockdown_patch, stub_forge};
|
use super::super::tests::{lockdown_patch, stub_forge};
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue