OAuth2 improvements (#968)

* fixed requireAuth/@errorStatus : set content length to 0 * do not issue a session cookie where none is required
membrane · Mar 13, 2024 · cb9df18 · cb9df18
1 parent 066a414
commit cb9df18
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 5 deletions.
diff --git a/...n/java/com/predic8/membrane/core/interceptor/oauth2client/OAuth2Resource2Interceptor.java b/...n/java/com/predic8/membrane/core/interceptor/oauth2client/OAuth2Resource2Interceptor.java
@@ -238,7 +238,7 @@ private void applyBackendAuthorization(Exchange exc, Session s) {
     public Outcome respondWithRedirect(Exchange exc) throws Exception {
         Integer errorStatus = (Integer) exc.getProperty(ERROR_STATUS);
         if (errorStatus != null) {
-            exc.setResponse(Response.statusCode(errorStatus).build());
+            exc.setResponse(Response.statusCode(errorStatus).header(CONTENT_LENGTH, "0").build());
             return RETURN;
         }
 

diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/oauth2client/RequireAuth.java b/core/src/main/java/com/predic8/membrane/core/interceptor/oauth2client/RequireAuth.java
@@ -120,7 +120,7 @@ public Integer getErrorStatus() {
     }
 
     @MCAttribute
-    public void setErrorStatus(Integer errorStatus) {
+    public void setErrorStatus(int errorStatus) {
         this.errorStatus = errorStatus;
     }
 

diff --git a/...ava/com/predic8/membrane/core/interceptor/oauth2client/rf/token/AccessTokenRefresher.java b/...ava/com/predic8/membrane/core/interceptor/oauth2client/rf/token/AccessTokenRefresher.java
@@ -18,6 +18,7 @@
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
 import com.predic8.membrane.core.exchange.Exchange;
+import com.predic8.membrane.core.interceptor.oauth2.OAuth2AnswerParameters;
 import com.predic8.membrane.core.interceptor.oauth2.authorizationservice.AuthorizationService;
 import com.predic8.membrane.core.interceptor.session.Session;
 import org.slf4j.Logger;
@@ -58,16 +59,15 @@ public void refreshIfNeeded(Session session, Exchange exc) {
 
         synchronized (getTokenSynchronizer(session)) {
             try {
-                refreshAccessToken(session, wantedScope);
-                exc.setProperty(Exchange.OAUTH2, session.getOAuth2AnswerParameters(wantedScope));
+                exc.setProperty(Exchange.OAUTH2, refreshAccessToken(session, wantedScope));
             } catch (Exception e) {
                 log.warn("Failed to refresh access token, clearing session and restarting OAuth2 flow.", e);
                 session.clearAuthentication();
             }
         }
     }
 
-    private void refreshAccessToken(Session session, String wantedScope) throws Exception {
+    private OAuth2AnswerParameters refreshAccessToken(Session session, String wantedScope) throws Exception {
         var params = session.getOAuth2AnswerParameters();
         var response = auth.refreshTokenRequest(session, params, wantedScope);
 
@@ -91,6 +91,8 @@ private void refreshAccessToken(Session session, String wantedScope) throws Exce
         tokenResponseHandler.handleTokenResponse(session, wantedScope, json, params);
 
         session.setOAuth2Answer(wantedScope, params.serialize());
+
+        return params;
     }
 
     private boolean refreshingOfAccessTokenIsNeeded(Session session, String wantedScope) {

diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/session/SessionManager.java b/core/src/main/java/com/predic8/membrane/core/interceptor/session/SessionManager.java
@@ -143,6 +143,9 @@ private void createDefaultResponseIfNeeded(Exchange exc) {
     private void handleSetCookieHeaderForResponse(Exchange exc, Session session) throws Exception {
         Optional<Object> originalCookieValueAtBeginning = Optional.ofNullable(exc.getProperty(SESSION_COOKIE_ORIGINAL));
 
+        if (originalCookieValueAtBeginning.isEmpty() && !session.isDirty)
+            return;
+
         if(ttlExpiryRefreshOnAccess || session.isDirty() || originalCookieValueAtBeginning.isEmpty() || cookieRenewalNeeded(originalCookieValueAtBeginning.get().toString())){
             String currentCookieValueOfSession = getCookieValue(session);
             if (!ttlExpiryRefreshOnAccess && originalCookieValueAtBeginning.isPresent() &&

diff --git a/...t/java/com/predic8/membrane/core/interceptor/oauth2/client/b2c/OAuth2ResourceB2CTest.java b/...t/java/com/predic8/membrane/core/interceptor/oauth2/client/b2c/OAuth2ResourceB2CTest.java
@@ -65,6 +65,7 @@
 import java.util.function.Consumer;
 
 import static com.predic8.membrane.core.RuleManager.RuleDefinitionSource.MANUAL;
+import static com.predic8.membrane.core.http.Header.SET_COOKIE;
 import static com.predic8.membrane.core.http.MimeType.APPLICATION_JSON;
 import static org.junit.jupiter.api.Assertions.*;
 
@@ -402,6 +403,7 @@ public void returning4xx() throws Exception {
 
         assertEquals(403, exc.getResponse().getStatusCode());
         assertEquals("Forbidden", exc.getResponse().getStatusMessage());
+        assertNull(exc.getResponse().getHeader().getFirstValue(SET_COOKIE));
 
         browser.apply(new Request.Builder().get(getClientAddress() + "/pe/init").buildExchange());