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

chore: drop validToken test token #93

Closed
wants to merge 1 commit into from

Conversation

miraclx
Copy link
Contributor

@miraclx miraclx commented Apr 21, 2023

Resolves #76. Provided FE no longer needs this, we can proceed to remove it.

If they're still dependent on this, we can make the process a bit more dynamic, to reduce the chances of an exploit.

@miraclx miraclx requested a review from volovyks April 21, 2023 04:44
@itegulov
Copy link
Contributor

The problem is that this breaks integration tests as we have not figured out how to imitate Firebase there. We could maybe hide these fake tokens behind a feature flag for now?

}
fn get_token_verifier_type(token: &str) -> Option<SupportedTokenVerifiers> {
// TODO: add real token type detection
(token.len() > 30).then(|| {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This logic does not make any sense anymore. Let's delete it and jsut use PagodaFirebaseTokeVerifier in verify token.

@volovyks
Copy link
Collaborator

@miraclx do you have time to continue working on this PR?

@miraclx
Copy link
Contributor Author

miraclx commented Apr 24, 2023

Stuck on this note in the sign_node /commit path: https://github.com/near/mpc-recovery/blob/21839cdc8478b9ed6a43c53328a94e24df64c53d/mpc-recovery/src/sign_node/mod.rs#L72-L74

It's unclear how to extract the access token from the payload. The structure of the payload is undocumented. In the two cases I can find;

  1. Random alphanumeric
    https://github.com/near/mpc-recovery/blob/21839cdc8478b9ed6a43c53328a94e24df64c53d/integration-tests/tests/mpc/positive.rs#L29
  2. Hash (so there's nothing to extract from here):
    https://github.com/near/mpc-recovery/blob/21839cdc8478b9ed6a43c53328a94e24df64c53d/mpc-recovery/src/transaction.rs#L127

I need context on how this is used in prod to define a proper resolution for this.

@itegulov
Copy link
Contributor

@miraclx try rebasing on my branch from #103, I am extracting the token there

@volovyks
Copy link
Collaborator

Looks like this PR is not relevant anymore. @miraclx feel free to reopen if you still want to merge something.

@volovyks volovyks closed this Apr 25, 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.

Disallow usage of the "validTOKEN" in production
3 participants