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.

Sign up for GitHub

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

Jump to bottom

OTP form issues #7

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

OTP form issues #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e4433f4
Fix:
kotnikd3 Apr 24, 2024
b0e114a
Fix The label "<< Back to Login" should be revised to something like …
kotnikd3 Apr 24, 2024
6d871d2
Change versions in pom.xml
kotnikd3 Apr 24, 2024
bac015f
Downgrade keycloak.version in pom.xml
kotnikd3 Apr 25, 2024
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
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<maven.compiler.target>${java.version}</maven.compiler.target>
<keycloak.version>22.0.5</keycloak.version>
<maven-shade.version>3.2.4</maven-shade.version>
<package.version>2.0.0</package.version>
<package.version>2.1.0</package.version>
Copy link
Contributor

Choose a reason for hiding this comment

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

This can just be 2.0.1, since it's just fixing the attempt logic when it's set as required.

</properties>

<dependencies>
Expand Down
27 changes: 11 additions & 16 deletions src/main/java/com/weare5stones/keycloak/authenticators/emailotp/EmailOTPAuthenticator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,23 +115,18 @@ public void action(AuthenticationFlowContext context) {
context.success();
}
} else {
// invalid
AuthenticationExecutionModel execution = context.getExecution();
if (execution.isRequired()) {
context.failureChallenge(AuthenticationFlowError.INVALID_CREDENTIALS,
context.form().setAttribute("realm", context.getRealm())
.setError("emailTOTPCodeInvalid").createForm(TOTP_FORM));
} else if (execution.isConditional() || execution.isAlternative()) {
if (remainingAttempts > 0) {
// decrement the remaining attempts
authSession.setAuthNote(AUTH_NOTE_REMAINING_RETRIES, Integer.toString(remainingAttempts - 1));
// display the error message
// Code is invalid
remainingAttempts--;
authSession.setAuthNote(AUTH_NOTE_REMAINING_RETRIES, Integer.toString(remainingAttempts));
Comment on lines +119 to +120
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this inside the the condition.


if (remainingAttempts > 0) {
// Inform user of the remaining attempts
context.failureChallenge(AuthenticationFlowError.INVALID_CREDENTIALS,
context.form().setAttribute("realm", context.getRealm())
.setError("emailTOTPCodeInvalid", remainingAttemptsStr).createForm(TOTP_FORM));
} else {
context.attempted();
}
context.form().setAttribute("realm", context.getRealm())
.setError("emailTOTPCodeInvalid", Integer.toString(remainingAttempts)).createForm(TOTP_FORM));
} else {
// Reset login
context.resetFlow();
Copy link
Contributor

Choose a reason for hiding this comment

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

Add an error here if you can. If you can't, I may take a look at that later.

}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/theme-resources/messages/messages_de.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ emailTOTPBodyHtml=Ihr Email Code lautet: <br/> <h2>{1}</h2></div> <br/> Sie ist

emailTOTPFormTitle=Email Code
emailTOTPFormLabel=Code
resendEmail=E-Mail zur\u00Fcksenden
emailTOTPFormInstruction=Geben Sie den Code ein, den wir an Ihr Ger\u00E4t gesendet haben.

emailTOTPCodeExpired=Die G\u00FCltigkeit des Codes ist abgelaufen.
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/theme-resources/messages/messages_en.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ emailTOTPBodyHtml=Your temporary login code is: <br/> <h2>{1}</h2></div> <br/> T

emailTOTPFormTitle=Temporary Login Code
emailTOTPFormLabel=Code
resendEmail=Resend email
emailTOTPFormInstruction=Enter the code we sent to your email address.

emailTOTPCodeExpired=The code has expired.
Expand Down
10 changes: 5 additions & 5 deletions src/main/resources/theme-resources/templates/totp-form.ftl
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert these changes. (The button probably should just be removed, but that could be a breaking change for some.)

We could add a resend button, but to get that working properly for everyone, it would still be removing the "back to login", and adding a new button with some logic in src/main/java/com/weare5stones/keycloak/authenticators/emailotp/EmailOTPAuthenticator.java

Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@
</div>
</div>
<div class="${properties.kcFormGroupClass!} ${properties.kcFormSettingClass!}">
<div id="kc-form-buttons" class="${properties.kcFormButtonsClass!}">
<input class="${properties.kcButtonClass!} ${properties.kcButtonPrimaryClass!} ${properties.kcButtonBlockClass!} ${properties.kcButtonLargeClass!}" type="submit" value="${msg("doSubmit")}"/>
</div>

<div id="kc-form-options" class="${properties.kcFormOptionsClass!}">
<div class="${properties.kcFormOptionsWrapperClass!}">
<span><a href="${url.loginUrl}">${kcSanitize(msg("backToLogin"))?no_esc}</a></span>
<span><a href="${url.loginUrl}">${msg("resendEmail")}</a></span>
</div>
</div>

<div id="kc-form-buttons" class="${properties.kcFormButtonsClass!}">
<input class="${properties.kcButtonClass!} ${properties.kcButtonPrimaryClass!} ${properties.kcButtonBlockClass!} ${properties.kcButtonLargeClass!}" type="submit" value="${msg("doSubmit")}"/>
</div>
</div>
</form>
<#elseif section = "info" >
Expand Down