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

Add a CAPTCHA check during registration #460

Merged
merged 73 commits into from
Dec 6, 2021
Merged

Add a CAPTCHA check during registration #460

merged 73 commits into from
Dec 6, 2021

Conversation

nmattia
Copy link
Collaborator

@nmattia nmattia commented Nov 22, 2021

This adds a CAPTCHA check during the registration flow. More precisely, the user now has to call the (new) method create_challenge that returns a CAPTCHA-style challenge (image and key) and register takes some extra parameters (the chars supposedly shown in the image, and the key). This involved some changes, in this repo and in others:

type Challenge = record {
    png_base64: text;
    challenge_key: ChallengeKey;
};
type ChallengeKey = nat32;
type ChallengeResult = record {
    key : ChallengeKey;
    chars : text;
};
  • ... as well as an extra argument to register: register : (DeviceData, ProofOfWork, ChallengeResult) -> (RegisterResponse);,
  • A change of flow in the register frontend flow: the confirmation screen now shows a CAPTCHA,
  • The backend-tests were updated to accommodate the new create_challenge() call.
  • The rust build now has a dummy_captcha feature (enabled in selenium tests and backend-tests) which makes sure the CAPTCHA challenges expect a known value ("a"). This should of course NEVER be used in production, hence is opt-in.

This also led to some helpful, though not absolutely necessary changes for backend-tests:

TODOs:

  • Add backend-tests for CAPTCHA failure and "too many challenges inflight" Add backend tests for CAPTCHA #468
  • Add selenium tests for CAPTCHA retry (i.e. auth device was created but CAPTCHA challenge failed -> only the CAPTCHA is reloaded)
  • Add PoW to create_challenge
  • Differentiate a "bad captcha" error from an actual canister, unexpected error.
  • Make the challenge key a string

@nmattia nmattia force-pushed the nm-captcha-rebased branch 3 times, most recently from 66bc39b to 1174d67 Compare November 23, 2021 14:43
@nmattia nmattia changed the title rebase captcha Add a CAPTCHA check during registration Nov 23, 2021
@nmattia nmattia marked this pull request as ready for review November 23, 2021 16:41
@nmattia nmattia requested a review from bitdivine November 23, 2021 16:41
.github/workflows/backend-tests.yml Show resolved Hide resolved
src/frontend/src/styles/main.css Outdated Show resolved Hide resolved
src/internet_identity/src/main.rs Outdated Show resolved Hide resolved
src/internet_identity/src/main.rs Outdated Show resolved Hide resolved
src/internet_identity/src/main.rs Outdated Show resolved Hide resolved
@nmattia nmattia force-pushed the nm-captcha-rebased branch 2 times, most recently from e1f230f to ceb5295 Compare November 26, 2021 16:08
@nmattia
Copy link
Collaborator Author

nmattia commented Nov 26, 2021

Sorry about the size of this beast :(

src/frontend/src/flows/confirmRegister.ts Outdated Show resolved Hide resolved
src/frontend/src/flows/confirmRegister.ts Show resolved Hide resolved
src/internet_identity/src/main.rs Show resolved Hide resolved
src/internet_identity/src/main.rs Outdated Show resolved Hide resolved
src/internet_identity/src/main.rs Outdated Show resolved Hide resolved
backend-tests/backend-tests.hs Show resolved Hide resolved
src/frontend/src/flows/loginUnknown.ts Outdated Show resolved Hide resolved
src/internet_identity/Cargo.toml Outdated Show resolved Hide resolved
Copy link
Member

@bitdivine bitdivine left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

backend-tests/README.md Outdated Show resolved Hide resolved
@nmattia nmattia force-pushed the nm-captcha-rebased branch 2 times, most recently from 79096ff to d258415 Compare December 3, 2021 09:44
@nmattia nmattia force-pushed the nm-captcha-rebased branch from d258415 to 8171f11 Compare December 6, 2021 14:00
@nmattia nmattia merged commit 504649d into main Dec 6, 2021
@nmattia nmattia deleted the nm-captcha-rebased branch December 6, 2021 15:14
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.

3 participants