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

Mfa nodejs example #236

Closed
wants to merge 2 commits into from
Closed

Mfa nodejs example #236

wants to merge 2 commits into from

Conversation

debbly
Copy link
Contributor

@debbly debbly commented Oct 18, 2023

No description provided.

@CLAassistant
Copy link

CLAassistant commented Oct 18, 2023

CLA assistant check
All committers have signed the CLA.

@debbly debbly changed the base branch from master to feat/SDK-V3 October 18, 2023 04:15
Comment on lines 142 to 144
Lit.Actions.setResponse({response: JSON.stringify({"Lit.Auth": Lit.Auth})})
};

Choose a reason for hiding this comment

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

in this code we can check that the Lit.Auth object which is an object based on this struct

pub struct AuthContext {
    pub action_ipfs_ids: Vec<String>,
    pub auth_sig_address: Option<String>,
    pub auth_method_contexts: Vec<AuthMethodResponse>,
    pub resources: Vec<RiString<UriSpec>>,
}

where AuthMethodContexts is each auth method's authenticated response. so we can check each of these to confirm that we have the expected mfa.

Comment on lines +150 to +156
const authSig = {
sig: "0x2bdede6164f56a601fc17a8a78327d28b54e87cf3fa20373fca1d73b804566736d76efe2dd79a4627870a50e66e1a9050ca333b6f98d9415d8bca424980611ca1c",
derivedVia: "web3.eth.personal.sign",
signedMessage:
"localhost wants you to sign in with your Ethereum account:\n0x9D1a5EC58232A894eBFcB5e466E3075b23101B89\n\nThis is a key for Partiful\n\nURI: https://localhost/login\nVersion: 1\nChain ID: 1\nNonce: 1LF00rraLO4f7ZSIt\nIssued At: 2022-06-03T05:59:09.959Z",
address: "0x9D1a5EC58232A894eBFcB5e466E3075b23101B89",
};

Choose a reason for hiding this comment

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

Since we provide auth methods to this method. I dont think we have to also provide an auth sig. as we already have authentication context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unfortunately, when tracing it down (within the code) we are required to pass in an authSig

though reading the docs here, I should be able to pass in session sigs? https://lit-js-sdk-v3-api-docs.vercel.app/interfaces/types_src.ILitNodeClient.html#executeJs

Lit.Actions.setResponse({response: JSON.stringify({"Lit.Auth": Lit.Auth})})

// Check the Lit.Auth object for auth methods (both 7 for OTP auth)
if () {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is the meat of this PR that never got implemented - the if statement to check that both are present

@glitch003
Copy link
Collaborator

just wanna clear up the state of this PR. it needs some work to finish.

  • We need to write the if statement that enforces that at least 2 auth methods have been presented
  • We need to set the scope of the auth methods to []
  • We need to run addPermittedAction() on the lit action that enforces this, with scopes: [1]. this should be the only auth method on the PKP that has scopes: [1]

@Ansonhkg Ansonhkg closed this Nov 17, 2023
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.

5 participants