From 48ab0355745e731a2fbc43628d611cce3a1c16db Mon Sep 17 00:00:00 2001 From: Dusan Jakub Date: Wed, 11 Oct 2023 16:17:47 +0200 Subject: [PATCH] 'state' is optional --- .../java/com/ysoft/geecon/OAuthResource.java | 14 +++++++------ .../geecon/error/OAuthRedirectException.java | 5 ++++- .../com/ysoft/geecon/AuthCodeGrantTest.java | 20 +++++++++++++++++++ .../geecon/helpers/AuthorizationCodeFlow.java | 10 +++++++++- 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/ysoft/geecon/OAuthResource.java b/src/main/java/com/ysoft/geecon/OAuthResource.java index c534b8c..6e510a6 100644 --- a/src/main/java/com/ysoft/geecon/OAuthResource.java +++ b/src/main/java/com/ysoft/geecon/OAuthResource.java @@ -10,7 +10,6 @@ import com.ysoft.geecon.repo.SessionsRepo; import com.ysoft.geecon.repo.UsersRepo; import io.quarkus.qute.CheckedTemplate; import io.quarkus.qute.TemplateInstance; -import io.quarkus.runtime.util.StringUtil; import io.quarkus.security.webauthn.WebAuthnLoginResponse; import io.quarkus.security.webauthn.WebAuthnRegisterResponse; import io.quarkus.security.webauthn.WebAuthnSecurity; @@ -153,9 +152,11 @@ public class OAuthResource { var responseTypes = session.params().getResponseTypes(); UriBuilder uri = UriBuilder.fromUri(redirectUri) - .fragment("") - .queryParam("state", session.params().getState()); + .fragment(""); + if (StringUtils.isNotBlank(session.params().getState())) { + uri.queryParam("state", session.params().getState()); + } if (responseTypes.contains(AuthParams.ResponseType.code)) { uri.queryParam("code", sessionsRepo.generateAuthorizationCode(sessionId)); } @@ -263,9 +264,10 @@ public class OAuthResource { // must NOT redirect to invalid redirect URI throw new OAuthUserVisibleException(ErrorResponse.Error.invalid_request, "Invalid redirect URI"); } - if (StringUtil.isNullOrEmpty(params.getState())) { - throw new OAuthRedirectException(params, ErrorResponse.Error.invalid_request, "Missing state"); - } + // state is optional +// if (StringUtil.isNullOrEmpty(params.getState())) { +// throw new OAuthRedirectException(params, ErrorResponse.Error.invalid_request, "Missing state"); +// } if (!params.validateResponseType()) { throw new OAuthRedirectException(params, ErrorResponse.Error.unsupported_response_type, "Unsupported response type"); diff --git a/src/main/java/com/ysoft/geecon/error/OAuthRedirectException.java b/src/main/java/com/ysoft/geecon/error/OAuthRedirectException.java index a6e437a..76d7ced 100644 --- a/src/main/java/com/ysoft/geecon/error/OAuthRedirectException.java +++ b/src/main/java/com/ysoft/geecon/error/OAuthRedirectException.java @@ -3,6 +3,7 @@ package com.ysoft.geecon.error; import com.ysoft.geecon.dto.AuthParams; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.UriBuilder; +import org.apache.commons.lang3.StringUtils; public class OAuthRedirectException extends OAuthApiException { private final AuthParams authParams; @@ -19,9 +20,11 @@ public class OAuthRedirectException extends OAuthApiException { public Response getResponse() { UriBuilder uri = UriBuilder.fromUri(authParams.getRedirectUri()) .fragment("") - .queryParam("state", authParams.getState()) .queryParam("error", response.error()) .queryParam("error_description", response.description()); + if (StringUtils.isNotBlank(authParams.getState())) { + uri.queryParam("state", authParams.getState()); + } return Response.seeOther(uri.build()).build(); } } diff --git a/src/test/java/com/ysoft/geecon/AuthCodeGrantTest.java b/src/test/java/com/ysoft/geecon/AuthCodeGrantTest.java index 4322c07..6d8c227 100644 --- a/src/test/java/com/ysoft/geecon/AuthCodeGrantTest.java +++ b/src/test/java/com/ysoft/geecon/AuthCodeGrantTest.java @@ -59,6 +59,26 @@ public class AuthCodeGrantTest { assertThat(accessTokenResponse.accessToken(), is(notNullValue())); } + @Test + public void authCodeGrant_stateIsOptional() throws IOException { + AuthorizationCodeFlow flow = new AuthorizationCodeFlow(authUrl, CLIENT) + .state(null) + .scope("scope1 scope2"); + LoginScreen loginScreen = flow.start().expectLogin(); + + ConsentScreen consentScreen = loginScreen.submit("bob", "password").expectSuccess(); + assertThat(consentScreen.getScopes(), is(List.of("scope1", "scope2"))); + + Document submit = consentScreen.submit(); + flow.expectSuccessfulRedirect(submit.connection().response()); + + assertThat(flow.getCode(), is(notNullValue())); + assertThat(flow.getAccessToken(), is(nullValue())); + AccessTokenResponse accessTokenResponse = flow.exchangeCode().expectTokens(); + + assertThat(accessTokenResponse.accessToken(), is(notNullValue())); + } + @Test public void badCredentials() throws IOException { AuthorizationCodeFlow flow = new AuthorizationCodeFlow(authUrl, CLIENT) diff --git a/src/test/java/com/ysoft/geecon/helpers/AuthorizationCodeFlow.java b/src/test/java/com/ysoft/geecon/helpers/AuthorizationCodeFlow.java index 9609908..73c06e1 100644 --- a/src/test/java/com/ysoft/geecon/helpers/AuthorizationCodeFlow.java +++ b/src/test/java/com/ysoft/geecon/helpers/AuthorizationCodeFlow.java @@ -37,7 +37,11 @@ public class AuthorizationCodeFlow { query = new HashMap<>(); query.put("client_id", client.clientId()); query.put("redirect_uri", client.redirectUris().get(0)); - query.put("state", state); + } + + public AuthorizationCodeFlow state(String state) { + this.state = state; + return this; } public AuthorizationCodeFlow param(String key, String value) { @@ -46,6 +50,10 @@ public class AuthorizationCodeFlow { } public Result start() throws IOException { + if (state != null) { + query.put("state", state); + } + Document document = Jsoup.connect(authUrl) .followRedirects(false) .data(query)