Skip to content

Commit

Permalink
Extract oauth error information and fail early if it exists (#3266)
Browse files Browse the repository at this point in the history
* extract error information and fail early if it exists

* parse specific login errors for more informative messages

* fix and add tests

* decode request string

* clean up formatted request and tests

* throw login specific exceptions

---------

Co-authored-by: obydog002 <>
  • Loading branch information
obydog002 authored Dec 3, 2024
1 parent 4fc4a42 commit d732ff0
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.faforever.client.login;

import lombok.Getter;

@Getter
public class KnownLoginErrorException extends RuntimeException {
private final String i18nKey;

public KnownLoginErrorException(String message, String i18nKey) {
super(message);
this.i18nKey = i18nKey;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
@Slf4j
@RequiredArgsConstructor
public class LoginController extends NodeController<Pane> {

private final OperatingSystem operatingSystem;
private final GameRunner gameRunner;
private final LoginService loginService;
Expand Down Expand Up @@ -292,6 +291,8 @@ private Void onLoginFailed(Throwable throwable) {
notificationService.addNotification(
new ServerNotification(i18n.get("login.failed"), loginException.getMessage(), Severity.ERROR,
List.of(new DismissAction(i18n))));
} else if (throwable instanceof KnownLoginErrorException loginException) {
notificationService.addImmediateErrorNotification(throwable, loginException.getI18nKey());
} else {
log.error("Could not log in", throwable);
notificationService.addImmediateErrorNotification(throwable, "login.failed");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.net.ServerSocket;
import java.net.Socket;
import java.net.URI;
import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CountDownLatch;
Expand All @@ -33,6 +34,9 @@ public class OAuthValuesReceiver {

private static final Pattern CODE_PATTERN = Pattern.compile("code=([^ &]+)");
private static final Pattern STATE_PATTERN = Pattern.compile("state=([^ &]+)");
private static final Pattern ERROR_PATTERN = Pattern.compile("error=([^ &]+)");
private static final Pattern ERROR_SCOPE_DENIED = Pattern.compile("scope_denied");
private static final Pattern ERROR_NO_CSRF = Pattern.compile("No\\+CSRF\\+value");

private final PlatformService platformService;
private final LoginService loginService;
Expand Down Expand Up @@ -89,6 +93,7 @@ private Values readValues(String state, String codeVerifier) {

// Do not try with resources as the socket needs to stay open.
try {
checkForError(request);
Values values = readValues(request, redirectUri);
success = true;
return values;
Expand Down Expand Up @@ -139,13 +144,32 @@ private Values readValues(String request, URI redirectUri) {
return new Values(code, state, redirectUri);
}

private String formatRequest(String request) {
return URLDecoder.decode(request, StandardCharsets.UTF_8);
}

private String extractValue(String request, Pattern pattern) {
Matcher matcher = pattern.matcher(request);
if (!matcher.find()) {
throw new IllegalStateException("Could not extract value with pattern '" + pattern + "' from: " + request);
throw new IllegalStateException("Could not extract value with pattern '" + pattern + "' from: " + formatRequest(request));
}
return matcher.group(1);
}

private void checkForError(String request) {
Matcher matcher = ERROR_PATTERN.matcher(request);
if (matcher.find()) {
String errorMessage = "Login failed with error '" + matcher.group(1) + "'. The full request is: " + formatRequest(request);
if (ERROR_SCOPE_DENIED.matcher(request).find()) {
throw new KnownLoginErrorException(errorMessage, "login.scopeDenied");
}

if (ERROR_NO_CSRF.matcher(request).find()) {
throw new KnownLoginErrorException(errorMessage, "login.noCSRF");
}
throw new IllegalStateException(errorMessage);
}
}

public record Values(String code, String state, URI redirectUri) {}
}
2 changes: 2 additions & 0 deletions src/main/resources/i18n/messages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -1089,6 +1089,8 @@ map.all = All Versions
map.current = Current Version
login.remember = Remember Me
login.failed = Error occurred during login
login.scopeDenied = Login failed. You did not accept the scopes on the login page.
login.noCSRF = Login failed. Likely your login timed out, please try again.
login.badState = State returned by user service does not match initial state if this continues to occur please reach out to technical help on the forum or discord channel
login.oauthBaseUrl = OAuth base URL
session.expired.title = Session Expired
Expand Down
17 changes: 15 additions & 2 deletions src/test/java/com/faforever/client/login/LoginControllerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public void testLoginFails() throws Exception {
@Test
public void testLoginFailsNoPorts() throws Exception {
when(oAuthValuesReceiver.receiveValues(anyString(), anyString()))
.thenReturn(CompletableFuture.failedFuture(new IllegalStateException()));
.thenReturn(CompletableFuture.failedFuture(new IllegalStateException("")));

instance.onLoginButtonClicked();
WaitForAsyncUtils.waitForFxEvents();
Expand All @@ -170,6 +170,19 @@ public void testLoginFailsNoPorts() throws Exception {
assertTrue(instance.loginFormPane.isVisible());
}

@Test
public void testLoginFailsKnownError() throws Exception {
when(oAuthValuesReceiver.receiveValues(anyString(), anyString()))
.thenReturn(CompletableFuture.failedFuture(new KnownLoginErrorException("", "login.known")));

instance.onLoginButtonClicked();
WaitForAsyncUtils.waitForFxEvents();

verify(notificationService).addImmediateErrorNotification(any(), eq("login.known"));
assertFalse(instance.loginProgressPane.isVisible());
assertTrue(instance.loginFormPane.isVisible());
}

@Test
public void testLoginFailsTimeout() throws Exception {
when(oAuthValuesReceiver.receiveValues(anyString(), anyString()))
Expand Down Expand Up @@ -236,7 +249,7 @@ public void testLoginRefreshFails() {
.thenReturn(CompletableFuture.completedFuture(ClientConfigurationBuilder.create().defaultValues().get()));
loginPrefs.setRememberMe(true);
loginPrefs.setRefreshToken("abc");
when(loginService.loginWithRefreshToken()).thenReturn(Mono.error(new Exception()));
when(loginService.loginWithRefreshToken()).thenReturn(Mono.error(new Exception("")));
runOnFxThreadAndWait(() -> reinitialize(instance));
verify(loginService).loginWithRefreshToken();
assertFalse(instance.loginProgressPane.isVisible());
Expand Down

0 comments on commit d732ff0

Please sign in to comment.