Skip to content

Commit

Permalink
Return 'BadChallenge' on bad CAPTCHA
Browse files Browse the repository at this point in the history
  • Loading branch information
nmattia committed Nov 26, 2021
1 parent 0ac73a2 commit e1f230f
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 14 deletions.
1 change: 1 addition & 0 deletions src/frontend/generated/internet_identity_idl.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export const idlFactory = ({ IDL }) => {
'chars' : IDL.Text,
});
const RegisterResponse = IDL.Variant({
'bad_challenge' : IDL.Null,
'canister_full' : IDL.Null,
'registered' : IDL.Record({ 'user_number' : UserNumber }),
});
Expand Down
3 changes: 2 additions & 1 deletion src/frontend/generated/internet_identity_types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ export interface ProofOfWork { 'nonce' : bigint, 'timestamp' : Timestamp }
export type PublicKey = Array<number>;
export type Purpose = { 'authentication' : null } |
{ 'recovery' : null };
export type RegisterResponse = { 'canister_full' : null } |
export type RegisterResponse = { 'bad_challenge' : null } |
{ 'canister_full' : null } |
{ 'registered' : { 'user_number' : UserNumber } };
export type SessionKey = PublicKey;
export interface SignedDelegation {
Expand Down
24 changes: 20 additions & 4 deletions src/frontend/src/flows/confirmRegister.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const pageContent = html`
<p class="captcha-status-text"></p>
<img id="captchaImg" />
<input id="captchaInput" />
<p>Please confirm to add your device.</p>
<p class="confirm-paragraph">Please confirm to add your device.</p>
<button type="submit" class="primary" id="confirmRegisterButton" disabled>
Confirm
</button>
Expand Down Expand Up @@ -58,6 +58,13 @@ const tryRegister = (
displayUserNumber(result.userNumber).then(() => {
func(apiResultToLoginResult(result));
});
} else if (result.kind == "badChallenge") {
const confirmParagraph = document.querySelector(
".confirm-paragraph"
) as HTMLElement;
confirmParagraph.innerHTML =
"The value you entered is incorrect. A new challenge is generated.";
requestCaptcha();
} else {
// Currently if something goes wrong we only tell the user that
// something went wrong and then reload the page.
Expand All @@ -84,6 +91,16 @@ const requestCaptcha = (): void => {
) as HTMLElement;
captchaStatusText.innerHTML = "Creating CAPTCHA challenge…";

const captchaInput = document.querySelector(
"#captchaInput"
) as HTMLFormElement;
captchaInput.disabled = true;

const confirmRegisterButton = form.querySelector(
"#confirmRegisterButton"
) as HTMLFormElement;
confirmRegisterButton.disabled = true;

// Wrap this in a promise to avoid slowing things down
const makePow: Promise<ProofOfWork> = new Promise((resolve) => {
const now_in_ns = BigInt(Date.now()) * BigInt(1000000);
Expand All @@ -100,15 +117,14 @@ const requestCaptcha = (): void => {
"src",
`data:image/png;base64, ${captchaResp.png_base64}`
);
const confirmRegisterButton = form.querySelector(
"#confirmRegisterButton"
) as HTMLFormElement;
confirmRegisterButton.setAttribute(
"data-captcha-key",
`${captchaResp.challenge_key}`
);
captchaStatusText.innerHTML = "Please type in the characters you see.";
confirmRegisterButton.disabled = false;
captchaInput.disabled = false;
captchaInput.value = "";
}
});
};
Expand Down
8 changes: 8 additions & 0 deletions src/frontend/src/flows/loginUnknown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,14 @@ export const apiResultToLoginResult = (result: ApiResult): LoginResult => {
"Failed to register with Internet Identity, because there is no space left at the moment. We're working on increasing the capacity.",
};
}
case "badChallenge": {
return {
tag: "err",
title: "Failed to register",
message:
"Failed to register with Internet Identity, because the challenge wasn't successful",
};
}
case "seedPhraseFail": {
return {
tag: "err",
Expand Down
7 changes: 6 additions & 1 deletion src/frontend/src/utils/iiConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,16 @@ export type RegisterResult =
| LoginSuccess
| AuthFail
| ApiError
| RegisterNoSpace;
| RegisterNoSpace
| BadChallenge;

type LoginSuccess = {
kind: "loginSuccess";
connection: IIConnection;
userNumber: bigint;
};

type BadChallenge = { kind: "badChallenge" };
type UnknownUser = { kind: "unknownUser"; userNumber: bigint };
type AuthFail = { kind: "authFail"; error: Error };
type ApiError = { kind: "apiError"; error: Error };
Expand Down Expand Up @@ -121,6 +124,8 @@ export class IIConnection {
connection: new IIConnection(identity, delegationIdentity, actor),
userNumber,
};
} else if (hasOwnProperty(registerResponse, "bad_challenge")) {
return { kind: "badChallenge" };
} else {
console.error("unexpected register response", registerResponse);
throw Error("unexpected register response");
Expand Down
2 changes: 2 additions & 0 deletions src/internet_identity/internet_identity.did
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ type RegisterResponse = variant {
registered: record { user_number: UserNumber; };
// No more registrations are possible in this instance of the II service canister.
canister_full;
// The challenge was not successful.
bad_challenge;
};

type Delegation = record {
Expand Down
18 changes: 10 additions & 8 deletions src/internet_identity/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ enum RegisterResponse {
Registered { user_number: UserNumber },
#[serde(rename = "canister_full")]
CanisterFull,
// TODO: add BadCaptcha here
#[serde(rename = "bad_challenge")]
BadChallenge,
}

mod hash;
Expand Down Expand Up @@ -289,7 +290,10 @@ async fn init_salt() {

#[update]
async fn register(device_data: DeviceData, challenge_result: ChallengeAttempt) -> RegisterResponse {
check_challenge(challenge_result);
if let Err(()) = check_challenge(challenge_result) {
return RegisterResponse::BadChallenge;
}

check_entry_limits(&device_data);

if caller() != Principal::self_authenticating(device_data.pubkey.clone()) {
Expand Down Expand Up @@ -533,20 +537,18 @@ fn create_captcha<T: RngCore>(rng: T) -> (Base64, String) {
}

// just traps if challenge isn't OK, because when in rome...
fn check_challenge(res: ChallengeAttempt) {
fn check_challenge(res: ChallengeAttempt) -> Result<(),()> {

STATE.with(|s| {
let mut inflight_challenges = s.inflight_challenges.borrow_mut();
match inflight_challenges.remove(&res.key) {
Some(challenge) => {
if res.chars != challenge.chars {
// NOTE: we _could_ show the expected chars here (the key has been
// removed already so the user won't be able to re-submit the answer using that
// key).
trap("CAPTCHA challenge failed");
return Err(());
}
return Ok(());
},
None => trap("Could not find a CAPTCHA challenge with that key") ,
None => Err(()),
}
})
}
Expand Down

0 comments on commit e1f230f

Please sign in to comment.