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

Optional Account ID in signin flow #100

Merged
merged 6 commits into from
Apr 22, 2023
Merged
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: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ Newly created NEAR account will have two full access keys. One that was provided

URL: /add_key
Request parameters: {
near_account_id: String,
// in case NEAR AccointId is not provided,
// it will be determined using recovery PK and NEAR Wallet APIs
near_account_id: Option(String),
public_key: String,
oidc_token: String
}
Expand Down
14 changes: 7 additions & 7 deletions integration-tests/tests/mpc/negative.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ async fn test_invalid_token() -> anyhow::Result<()> {
let (status_code, add_key_response) = ctx
.leader_node
.add_key(AddKeyRequest {
near_account_id: account_id.to_string(),
near_account_id: Some(account_id.to_string()),
oidc_token: token::invalid(),
public_key: new_user_public_key.clone(),
})
Expand All @@ -54,7 +54,7 @@ async fn test_invalid_token() -> anyhow::Result<()> {
let (status_code, add_key_response) = ctx
.leader_node
.add_key(AddKeyRequest {
near_account_id: account_id.to_string(),
near_account_id: Some(account_id.to_string()),
oidc_token: token::valid(),
public_key: new_user_public_key.clone(),
})
Expand Down Expand Up @@ -114,7 +114,7 @@ async fn test_malformed_account_id() -> anyhow::Result<()> {
let (status_code, add_key_response) = ctx
.leader_node
.add_key(AddKeyRequest {
near_account_id: malformed_account_id.to_string(),
near_account_id: Some(malformed_account_id.to_string()),
oidc_token: token::valid(),
public_key: new_user_public_key.clone(),
})
Expand All @@ -126,7 +126,7 @@ async fn test_malformed_account_id() -> anyhow::Result<()> {
let (status_code, add_key_response) = ctx
.leader_node
.add_key(AddKeyRequest {
near_account_id: account_id.to_string(),
near_account_id: Some(account_id.to_string()),
oidc_token: token::valid(),
public_key: new_user_public_key.clone(),
})
Expand Down Expand Up @@ -183,7 +183,7 @@ async fn test_malformed_public_key() -> anyhow::Result<()> {
let (status_code, add_key_response) = ctx
.leader_node
.add_key(AddKeyRequest {
near_account_id: account_id.to_string(),
near_account_id: Some(account_id.to_string()),
oidc_token: token::valid(),
public_key: malformed_public_key.clone(),
})
Expand All @@ -197,7 +197,7 @@ async fn test_malformed_public_key() -> anyhow::Result<()> {
let (status_code, add_key_response) = ctx
.leader_node
.add_key(AddKeyRequest {
near_account_id: account_id.to_string(),
near_account_id: Some(account_id.to_string()),
oidc_token: token::valid(),
public_key: new_user_public_key.clone(),
})
Expand Down Expand Up @@ -227,7 +227,7 @@ async fn test_add_key_to_non_existing_account() -> anyhow::Result<()> {
let (status_code, add_key_response) = ctx
.leader_node
.add_key(AddKeyRequest {
near_account_id: account_id.to_string(),
near_account_id: Some(account_id.to_string()),
oidc_token: token::valid(),
public_key: user_public_key.clone(),
})
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/tests/mpc/positive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ async fn test_basic_action() -> anyhow::Result<()> {
let (status_code, add_key_response) = ctx
.leader_node
.add_key(AddKeyRequest {
near_account_id: account_id.to_string(),
near_account_id: Some(account_id.to_string()),
oidc_token: token::valid(),
public_key: new_user_public_key.clone(),
})
Expand Down
69 changes: 64 additions & 5 deletions mpc-recovery/src/leader_node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub struct Config {
pub account_creator_id: AccountId,
// TODO: temporary solution
pub account_creator_sk: SecretKey,
pub account_lookup_url: String,
}

pub async fn run(config: Config) {
Expand All @@ -52,6 +53,7 @@ pub async fn run(config: Config) {
near_root_account,
account_creator_id,
account_creator_sk,
account_lookup_url,
} = config;
let _span = tracing::debug_span!("run", id, port);
tracing::debug!(?sign_nodes, "running a leader node");
Expand Down Expand Up @@ -87,6 +89,7 @@ pub async fn run(config: Config) {
near_root_account: near_root_account.parse().unwrap(),
account_creator_id,
account_creator_sk,
account_lookup_url,
};

//TODO: not secure, allow only for testnet, whitelist endpoint etc. for mainnet
Expand Down Expand Up @@ -118,6 +121,7 @@ struct LeaderState {
account_creator_id: AccountId,
// TODO: temporary solution
account_creator_sk: SecretKey,
account_lookup_url: String,
}

async fn parse(response_future: ResponseFuture) -> anyhow::Result<SigShareResponse> {
Expand Down Expand Up @@ -303,10 +307,28 @@ enum AddKeyError {
OidcVerificationFailed(anyhow::Error),
#[error("relayer error: {0}")]
RelayerError(#[from] RelayerError),
#[error("failed to find associated account id for pk: {0}")]
AccountNotFound(String),
#[error("{0}")]
Other(#[from] anyhow::Error),
}

fn get_acc_id_from_pk(
public_key: PublicKey,
account_lookup_url: String,
) -> Result<AccountId, anyhow::Error> {
let url = format!("{}/publicKey/{}/accounts", account_lookup_url, public_key);
let client = reqwest::blocking::Client::new();
let response = client.get(url).send()?.text()?;
let accounts: Vec<String> = serde_json::from_str(&response)?;
Ok(accounts
.first()
.cloned()
.unwrap_or_default()
.parse()
.unwrap())
}

async fn process_add_key<T: OAuthTokenVerifier>(
state: LeaderState,
request: AddKeyRequest,
Expand All @@ -315,17 +337,26 @@ async fn process_add_key<T: OAuthTokenVerifier>(
.await
.map_err(AddKeyError::OidcVerificationFailed)?;
let internal_acc_id = get_internal_account_id(oidc_token_claims);
let user_account_id: AccountId = request
.near_account_id
.parse()
.map_err(|e| AddKeyError::MalformedAccountId(request.near_account_id, e))?;
let user_recovery_pk = get_user_recovery_pk(internal_acc_id.clone());
let user_recovery_sk = get_user_recovery_sk(internal_acc_id);
let new_public_key: PublicKey = request
.public_key
.parse()
.map_err(|e| AddKeyError::MalformedPublicKey(request.public_key, e))?;

let user_account_id: AccountId = match &request.near_account_id {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we handle the error case of it being a list of IDs with this controlling key?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We are just taking the first one now. There will be no error.
Also, we are using recovery PK. Since we allow anly one acc per our internal (firebase) Id, thre should always be one account.

Some(near_account_id) => near_account_id
.parse()
.map_err(|e| AddKeyError::MalformedAccountId(request.near_account_id.unwrap(), e))?,
None => match get_acc_id_from_pk(user_recovery_pk.clone(), state.account_lookup_url) {
Ok(near_account_id) => near_account_id,
Err(e) => {
tracing::error!(err = ?e);
return Err(AddKeyError::AccountNotFound(e.to_string()));
}
},
};

nar::retry(|| async {
// Get nonce and recent block hash
let (_hash, block_height, nonce) = state
Expand Down Expand Up @@ -381,7 +412,10 @@ async fn add_key<T: OAuthTokenVerifier>(
Json(request): Json<AddKeyRequest>,
) -> (StatusCode, Json<AddKeyResponse>) {
tracing::info!(
near_account_id = hex::encode(&request.near_account_id),
near_account_id = hex::encode(match &request.near_account_id {
Some(ref near_account_id) => near_account_id,
None => "not specified",
}),
public_key = hex::encode(&request.public_key),
iodc_token = format!("{:.5}...", request.oidc_token),
"add_key request"
Expand Down Expand Up @@ -529,3 +563,28 @@ async fn submit<T: OAuthTokenVerifier>(
(StatusCode::INTERNAL_SERVER_ERROR, Json(LeaderResponse::Err))
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_get_acc_id_from_pk_mainnet() {
let url = "https://api.kitwallet.app".to_string();
let public_key: PublicKey = "ed25519:2uF6ZUghFFUg3Kta9rW47iiJ3crNzRdaPD2rBPQWEwyc"
.parse()
.unwrap();
let first_account = get_acc_id_from_pk(public_key, url).unwrap();
assert_eq!(first_account.to_string(), "serhii.near".to_string());
}

#[test]
fn test_get_acc_id_from_pk_testnet() {
let url = "https://testnet-api.kitwallet.app".to_string();
let public_key: PublicKey = "ed25519:7WYR7ifUbdVo2soQCvzAHnfdGfDhUhF8Und5CKZYK9b8"
.parse()
.unwrap();
let first_account = get_acc_id_from_pk(public_key, url).unwrap();
assert_eq!(first_account.to_string(), "serhii.testnet".to_string());
}
}
8 changes: 8 additions & 0 deletions mpc-recovery/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ enum Cli {
/// TEMPORARY - Account creator ed25519 secret key
#[arg(long, env("MPC_RECOVERY_ACCOUNT_CREATOR_SK"))]
account_creator_sk: Option<String>,
#[arg(
long,
env("MPC_RECOVERY_ACCOUNT_LOOKUP_URL"),
default_value("https://api.kitwallet.app")
)]
account_lookup_url: String,
},
StartSign {
/// Node ID
Expand Down Expand Up @@ -129,6 +135,7 @@ async fn main() -> anyhow::Result<()> {
near_root_account,
account_creator_id,
account_creator_sk,
account_lookup_url,
} => {
let gcp_service = GcpService::new().await?;
let sk_share = load_sh_skare(&gcp_service, node_id, sk_share).await?;
Expand All @@ -151,6 +158,7 @@ async fn main() -> anyhow::Result<()> {
// TODO: Create such an account for testnet and mainnet in a secure way
account_creator_id,
account_creator_sk,
account_lookup_url,
})
.await;
}
Expand Down
2 changes: 1 addition & 1 deletion mpc-recovery/src/msg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl NewAccountResponse {

#[derive(Serialize, Deserialize, Debug)]
pub struct AddKeyRequest {
pub near_account_id: String,
pub near_account_id: Option<String>,
pub public_key: String,
pub oidc_token: String,
}
Expand Down