feat: Remove 'active' field from OAuth credentials (#1193)

* Remove active field from OAuth credentials

* [autofix.ci] apply automated fixes

* Remove unused parameter, add test case

* Fix tests

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This commit is contained in:
boxbeam 2024-01-12 00:37:11 -05:00 committed by GitHub
parent 3ddbda8e82
commit 3841185d71
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 54 additions and 75 deletions

View File

@ -2,7 +2,6 @@ CREATE TABLE github_oauth_credential (
id INTEGER PRIMARY KEY AUTOINCREMENT,
client_id VARCHAR(32) NOT NULL,
client_secret VARCHAR(64) NOT NULL,
active BOOLEAN DEFAULT (1),
created_at TIMESTAMP DEFAULT (DATETIME('now')),
updated_at TIMESTAMP DEFAULT (DATETIME('now'))
);

View File

@ -9,7 +9,6 @@ const GITHUB_OAUTH_CREDENTIAL_ROW_ID: i32 = 1;
pub struct GithubOAuthCredentialDAO {
pub client_id: String,
pub client_secret: String,
pub active: bool,
pub created_at: DateTime<Utc>,
pub updated_at: DateTime<Utc>,
}
@ -19,9 +18,8 @@ impl GithubOAuthCredentialDAO {
Ok(Self {
client_id: row.get(0)?,
client_secret: row.get(1)?,
active: row.get(2)?,
created_at: row.get(3)?,
updated_at: row.get(4)?,
created_at: row.get(2)?,
updated_at: row.get(3)?,
})
}
}
@ -31,52 +29,39 @@ impl DbConn {
pub async fn update_github_oauth_credential(
&self,
client_id: &str,
client_secret: Option<&str>,
active: bool,
client_secret: &str,
) -> Result<()> {
let client_id = client_id.to_string();
if let Some(client_secret) = client_secret {
let client_secret = client_secret.to_string();
let sql = r#"INSERT INTO github_oauth_credential (id, client_id, client_secret, active)
VALUES (:id, :cid, :secret, :active) ON CONFLICT(id) DO UPDATE
SET client_id = :cid, client_secret = :secret, active = :active, updated_at = datetime('now')
let client_secret = client_secret.to_string();
let sql = r#"INSERT INTO github_oauth_credential (id, client_id, client_secret)
VALUES (:id, :cid, :secret) ON CONFLICT(id) DO UPDATE
SET client_id = :cid, client_secret = :secret, updated_at = datetime('now')
WHERE id = :id"#;
self.conn
.call(move |c| {
let mut stmt = c.prepare(sql)?;
stmt.insert(named_params! {
":id": GITHUB_OAUTH_CREDENTIAL_ROW_ID,
":cid": client_id,
":secret": client_secret,
":active": active,
})?;
Ok(())
})
.await?;
Ok(())
} else {
let sql = r#"
UPDATE github_oauth_credential SET client_id = :cid, active = :active, updated_at = datetime('now')
WHERE id = :id"#;
let rows = self
.conn
.call(move |c| {
let mut stmt = c.prepare(sql)?;
let rows = stmt.execute(named_params! {
":id": GITHUB_OAUTH_CREDENTIAL_ROW_ID,
":cid": client_id,
":active": active,
})?;
Ok(rows)
})
.await?;
if rows != 1 {
return Err(anyhow::anyhow!(
"failed to update: github credential not found"
));
}
Ok(())
}
self.conn
.call(move |c| {
let mut stmt = c.prepare(sql)?;
stmt.insert(named_params! {
":id": GITHUB_OAUTH_CREDENTIAL_ROW_ID,
":cid": client_id,
":secret": client_secret,
})?;
Ok(())
})
.await?;
Ok(())
}
pub async fn delete_github_oauth_credential(&self) -> Result<()> {
Ok(self
.conn
.call(move |c| {
c.execute(
"DELETE FROM github_oauth_credential WHERE id = ?",
[GITHUB_OAUTH_CREDENTIAL_ROW_ID],
)?;
Ok(())
})
.await?)
}
pub async fn read_github_oauth_credential(&self) -> Result<Option<GithubOAuthCredentialDAO>> {
@ -85,7 +70,7 @@ impl DbConn {
.call(|conn| {
Ok(conn
.query_row(
r#"SELECT client_id, client_secret, active, created_at, updated_at FROM github_oauth_credential WHERE id = ?"#,
r#"SELECT client_id, client_secret, created_at, updated_at FROM github_oauth_credential WHERE id = ?"#,
[GITHUB_OAUTH_CREDENTIAL_ROW_ID],
GithubOAuthCredentialDAO::from_row,
)
@ -105,37 +90,32 @@ mod tests {
async fn test_update_github_oauth_credential() {
let conn = DbConn::new_in_memory().await.unwrap();
// test update failure when no record exists
let res = conn
.update_github_oauth_credential("client_id", None, false)
.await;
assert!(res.is_err());
// test insert
conn.update_github_oauth_credential("client_id", Some("client_secret"), true)
conn.update_github_oauth_credential("client_id", "client_secret")
.await
.unwrap();
let res = conn.read_github_oauth_credential().await.unwrap().unwrap();
assert_eq!(res.client_id, "client_id");
assert_eq!(res.client_secret, "client_secret");
assert!(res.active);
// test update
conn.update_github_oauth_credential("client_id", Some("client_secret_2"), false)
conn.update_github_oauth_credential("client_id", "client_secret_2")
.await
.unwrap();
let res = conn.read_github_oauth_credential().await.unwrap().unwrap();
assert_eq!(res.client_id, "client_id");
assert_eq!(res.client_secret, "client_secret_2");
assert!(!res.active);
// test update without client_secret
conn.update_github_oauth_credential("client_id_2", None, true)
// test delete
conn.delete_github_oauth_credential().await.unwrap();
assert!(conn.read_github_oauth_credential().await.unwrap().is_none());
// test update after delete
conn.update_github_oauth_credential("client_id_2", "client_secret_2")
.await
.unwrap();
let res = conn.read_github_oauth_credential().await.unwrap().unwrap();
assert_eq!(res.client_id, "client_id_2");
assert_eq!(res.client_secret, "client_secret_2");
assert!(res.active);
}
}

View File

@ -344,7 +344,6 @@ pub enum OAuthProvider {
pub struct OAuthCredential {
pub provider: OAuthProvider,
pub client_id: String,
pub active: bool,
pub created_at: DateTime<Utc>,
pub updated_at: DateTime<Utc>,
}
@ -409,9 +408,10 @@ pub trait AuthenticationService: Send + Sync {
&self,
provider: OAuthProvider,
client_id: String,
client_secret: Option<String>,
active: bool,
client_secret: String,
) -> Result<()>;
async fn delete_oauth_credential(&self, provider: OAuthProvider) -> Result<()>;
}
#[cfg(test)]

View File

@ -48,7 +48,6 @@ impl From<GithubOAuthCredentialDAO> for OAuthCredential {
OAuthCredential {
provider: OAuthProvider::Github,
client_id: val.client_id,
active: val.active,
created_at: val.created_at,
updated_at: val.updated_at,
}

View File

@ -362,14 +362,13 @@ impl Mutation {
ctx: &Context,
provider: OAuthProvider,
client_id: String,
client_secret: Option<String>,
active: bool,
client_secret: String,
) -> Result<bool> {
if let Some(claims) = &ctx.claims {
if claims.is_admin {
ctx.locator
.auth()
.update_oauth_credential(provider, client_id, client_secret, active)
.update_oauth_credential(provider, client_id, client_secret)
.await?;
return Ok(true);
}

View File

@ -353,9 +353,6 @@ impl AuthenticationService for DbConn {
.read_github_oauth_credential()
.await?
.ok_or(GithubAuthError::CredentialNotActive)?;
if !credential.active {
return Err(GithubAuthError::CredentialNotActive);
}
let email = client.fetch_user_email(code, credential).await?;
@ -403,15 +400,20 @@ impl AuthenticationService for DbConn {
&self,
provider: OAuthProvider,
client_id: String,
client_secret: Option<String>,
active: bool,
client_secret: String,
) -> Result<()> {
match provider {
OAuthProvider::Github => Ok(self
.update_github_oauth_credential(&client_id, client_secret.as_deref(), active)
.update_github_oauth_credential(&client_id, &client_secret)
.await?),
}
}
async fn delete_oauth_credential(&self, provider: OAuthProvider) -> Result<()> {
match provider {
OAuthProvider::Github => self.delete_github_oauth_credential().await,
}
}
}
fn password_hash(raw: &str) -> password_hash::Result<String> {