Prefer 303 over 302 for redirecting after a POST (#1223)

* Prefer 303 over 302 for redirecting after a POST

* update release notes

* Update release-notes.md

* rename TemporaryRedirectAction as FoundAction (302), the real RFC name

documentation/docs/release-notes.md

@@ -10,6 +10,7 @@ title: Release notes:
 - Updated the OpenID Connect/JWT dependencies
 - A client can return any kind of profile (using a custom `AuthorizationGenerator` or `ProfileCreator`) and even a minimal user profile (`UserProfile`)
 - Multiple HTTP actions (inheriting from `HttpAction`) are created to handle the necessary HTTP actions. They are only applied to the web context by the appropriate `HttpActionAdapter`. The `RedirectAction` is replaced by the new HTTP actions inheriting from `RedirectionAction`
+- Use the 303 "See Other" HTTP action instead of the 302 "Temporary Redirect" HTTP action for any redirection after a POST request in the `DefaultCallbackLogic`
 
 
 **v3.5.0**:

pac4j-cas/src/main/java/org/pac4j/cas/client/direct/DirectCasClient.java

@@ -12,7 +12,7 @@
 import org.pac4j.core.credentials.extractor.ParameterExtractor;
 import org.pac4j.core.exception.CredentialsException;
 import org.pac4j.core.exception.TechnicalException;
-import org.pac4j.core.exception.http.TemporaryRedirectAction;
+import org.pac4j.core.exception.http.FoundAction;
 import org.pac4j.core.http.callback.CallbackUrlResolver;
 import org.pac4j.core.http.callback.NoParameterCallbackUrlResolver;
 import org.pac4j.core.http.url.DefaultUrlResolver;
@@ -77,7 +77,7 @@ protected TokenCredentials retrieveCredentials(final WebContext context) {
                 final String redirectionUrl = CommonUtils.constructRedirectUrl(loginUrl, CasConfiguration.SERVICE_PARAMETER,
                         callbackUrl, configuration.isRenew(), false);
                 logger.debug("redirectionUrl: {}", redirectionUrl);
-                throw new TemporaryRedirectAction(redirectionUrl);
+                throw new FoundAction(redirectionUrl);
             }
 
             // clean url from ticket parameter

pac4j-cas/src/main/java/org/pac4j/cas/credentials/extractor/TicketAndLogoutRequestExtractor.java

@@ -4,7 +4,7 @@
 import org.jasig.cas.client.util.CommonUtils;
 import org.pac4j.cas.config.CasConfiguration;
 import org.pac4j.core.exception.http.NoContentAction;
-import org.pac4j.core.exception.http.TemporaryRedirectAction;
+import org.pac4j.core.exception.http.FoundAction;
 import org.pac4j.core.logout.handler.LogoutHandler;
 import org.pac4j.core.context.ContextHelper;
 import org.pac4j.core.context.HttpConstants;
@@ -135,7 +135,7 @@ private void computeRedirectionToServerIfNecessary(final WebContext context) {
             buffer.append(CommonUtils.urlEncode(relayStateValue));
             final String redirectUrl = buffer.toString();
             logger.debug("Redirection url to the CAS server: {}", redirectUrl);
-            throw new TemporaryRedirectAction(redirectUrl);
+            throw new FoundAction(redirectUrl);
         }
     }
 }

pac4j-cas/src/main/java/org/pac4j/cas/redirect/CasRedirectionActionBuilder.java

@@ -4,7 +4,7 @@
 import org.pac4j.cas.client.CasClient;
 import org.pac4j.cas.config.CasConfiguration;
 import org.pac4j.core.exception.http.RedirectionAction;
-import org.pac4j.core.exception.http.TemporaryRedirectAction;
+import org.pac4j.core.exception.http.FoundAction;
 import org.pac4j.core.context.WebContext;
 import org.pac4j.core.redirect.RedirectionActionBuilder;
 import org.pac4j.core.util.CommonHelper;
@@ -39,6 +39,6 @@ public RedirectionAction redirect(final WebContext context) {
         final String redirectionUrl = CommonUtils.constructRedirectUrl(computeLoginUrl, CasConfiguration.SERVICE_PARAMETER,
                 computedCallbackUrl, configuration.isRenew(), configuration.isGateway());
         logger.debug("redirectionUrl: {}", redirectionUrl);
-        return new TemporaryRedirectAction(redirectionUrl);
+        return new FoundAction(redirectionUrl);
     }
 }

pac4j-cas/src/test/java/org/pac4j/cas/client/CasClientTests.java

@@ -6,7 +6,7 @@
 import org.pac4j.core.context.WebContext;
 import org.pac4j.core.credentials.TokenCredentials;
 import org.pac4j.core.exception.http.HttpAction;
-import org.pac4j.core.exception.http.TemporaryRedirectAction;
+import org.pac4j.core.exception.http.FoundAction;
 import org.pac4j.core.http.callback.CallbackUrlResolver;
 import org.pac4j.core.http.url.UrlResolver;
 import org.pac4j.core.util.TestsConstants;
@@ -110,7 +110,7 @@ public void testRenewMissing() {
         final CasClient casClient = new CasClient(configuration);
         casClient.setCallbackUrl(CALLBACK_URL);
         final MockWebContext context = MockWebContext.create();
-        final TemporaryRedirectAction action = (TemporaryRedirectAction) casClient.redirect(context);
+        final FoundAction action = (FoundAction) casClient.redirect(context);
         assertFalse(action.getLocation().indexOf("renew=true") >= 0);
     }
 
@@ -122,7 +122,7 @@ public void testRenew() {
         casClient.setCallbackUrl(CALLBACK_URL);
         configuration.setRenew(true);
         final MockWebContext context = MockWebContext.create();
-        final TemporaryRedirectAction action = (TemporaryRedirectAction) casClient.redirect(context);
+        final FoundAction action = (FoundAction) casClient.redirect(context);
         assertTrue(action.getLocation().indexOf("renew=true") >= 0);
     }
 
@@ -133,7 +133,7 @@ public void testGatewayMissing() {
         final CasClient casClient = new CasClient(configuration);
         casClient.setCallbackUrl(CALLBACK_URL);
         final MockWebContext context = MockWebContext.create();
-        final TemporaryRedirectAction action = (TemporaryRedirectAction) casClient.redirect(context);
+        final FoundAction action = (FoundAction) casClient.redirect(context);
         assertFalse(action.getLocation().indexOf("gateway=true") >= 0);
     }
 
@@ -145,7 +145,7 @@ public void testGatewayOK() {
         casClient.setCallbackUrl(CALLBACK_URL);
         final MockWebContext context = MockWebContext.create();
         configuration.setGateway(true);
-        final TemporaryRedirectAction action = (TemporaryRedirectAction) casClient.redirect(context);
+        final FoundAction action = (FoundAction) casClient.redirect(context);
         assertTrue(action.getLocation().indexOf("gateway=true") >= 0);
         final TokenCredentials credentials = casClient.getCredentials(context);
         assertNull(credentials);
@@ -200,7 +200,7 @@ public void testFrontLogoutWithRelayState() {
                 .addRequestParameter(CasConfiguration.LOGOUT_REQUEST_PARAMETER, deflateAndBase64(LOGOUT_MESSAGE))
                 .addRequestParameter(CasConfiguration.RELAY_STATE_PARAMETER, VALUE).setRequestMethod(HTTP_METHOD.GET.name());
         final HttpAction action = (HttpAction) TestsHelper.expectException(() -> casClient.getCredentials(context));
-        assertEquals(TEMP_REDIRECT, action.getCode());
+        assertEquals(FOUND, action.getCode());
     }
 
     @Test

pac4j-cas/src/test/java/org/pac4j/cas/client/direct/DirectCasClientTests.java

@@ -8,7 +8,7 @@
 import org.pac4j.core.credentials.TokenCredentials;
 import org.pac4j.core.exception.http.HttpAction;
 import org.pac4j.core.exception.TechnicalException;
-import org.pac4j.core.exception.http.TemporaryRedirectAction;
+import org.pac4j.core.exception.http.FoundAction;
 import org.pac4j.core.profile.CommonProfile;
 import org.pac4j.core.util.TestsConstants;
 import org.pac4j.core.util.TestsHelper;
@@ -58,7 +58,7 @@ public void testNoTokenRedirectionExpected() {
         final HttpAction action = (HttpAction) TestsHelper.expectException(() -> client.getCredentials(context));
         assertEquals(302, action.getCode());
         assertEquals(addParameter(LOGIN_URL, CasConfiguration.SERVICE_PARAMETER, CALLBACK_URL),
-            ((TemporaryRedirectAction) action).getLocation());
+            ((FoundAction) action).getLocation());
     }
 
     @Test

pac4j-core/src/main/java/org/pac4j/core/authorization/authorizer/AbstractCheckAuthenticationAuthorizer.java

@@ -1,7 +1,7 @@
 package org.pac4j.core.authorization.authorizer;
 
 import org.pac4j.core.context.WebContext;
-import org.pac4j.core.exception.http.TemporaryRedirectAction;
+import org.pac4j.core.exception.http.FoundAction;
 import org.pac4j.core.profile.UserProfile;
 
 /**
@@ -23,7 +23,7 @@ public AbstractCheckAuthenticationAuthorizer(final String redirectionUrl) {
     @Override
     protected boolean handleError(final WebContext context) {
         if (this.redirectionUrl != null) {
-            throw new TemporaryRedirectAction(this.redirectionUrl);
+            throw new FoundAction(this.redirectionUrl);
         } else {
             return false;
         }

pac4j-core/src/main/java/org/pac4j/core/context/HttpConstants.java

@@ -18,7 +18,9 @@ public interface HttpConstants {
 
     int FORBIDDEN = 403;
 
-    int TEMP_REDIRECT = 302;
+    int FOUND = 302;
+
+    int SEE_OTHER = 303;
 
     int BAD_REQUEST = 400;
 

pac4j-core/src/main/java/org/pac4j/core/engine/AbstractExceptionAwareLogic.java

@@ -2,7 +2,7 @@
 
 import org.pac4j.core.context.WebContext;
 import org.pac4j.core.exception.http.HttpAction;
-import org.pac4j.core.exception.http.TemporaryRedirectAction;
+import org.pac4j.core.exception.http.FoundAction;
 import org.pac4j.core.http.adapter.HttpActionAdapter;
 import org.pac4j.core.profile.factory.ProfileManagerFactoryAware;
 import org.pac4j.core.util.CommonHelper;
@@ -43,7 +43,7 @@ protected R handleException(final Exception e, final HttpActionAdapter<R, C> htt
             return httpActionAdapter.adapt(action, context);
         } else {
             if (CommonHelper.isNotBlank(errorUrl)) {
-                final HttpAction action = new TemporaryRedirectAction(errorUrl);
+                final HttpAction action = new FoundAction(errorUrl);
                 return httpActionAdapter.adapt(action, context);
             } else {
                 throw runtimeException(e);

pac4j-core/src/main/java/org/pac4j/core/engine/DefaultCallbackLogic.java

@@ -7,12 +7,14 @@
 import org.pac4j.core.client.finder.ClientFinder;
 import org.pac4j.core.client.finder.DefaultCallbackClientFinder;
 import org.pac4j.core.config.Config;
+import org.pac4j.core.context.ContextHelper;
 import org.pac4j.core.context.Pac4jConstants;
 import org.pac4j.core.context.WebContext;
 import org.pac4j.core.context.session.SessionStore;
 import org.pac4j.core.credentials.Credentials;
 import org.pac4j.core.exception.http.HttpAction;
-import org.pac4j.core.exception.http.TemporaryRedirectAction;
+import org.pac4j.core.exception.http.SeeOtherAction;
+import org.pac4j.core.exception.http.FoundAction;
 import org.pac4j.core.http.adapter.HttpActionAdapter;
 import org.pac4j.core.profile.ProfileManager;
 import org.pac4j.core.profile.UserProfile;
@@ -145,7 +147,11 @@ protected HttpAction redirectToOriginallyRequestedUrl(final C context, final Str
             redirectUrl = requestedUrl;
         }
         logger.debug("redirectUrl: {}", redirectUrl);
-        return new TemporaryRedirectAction(redirectUrl);
+        if (ContextHelper.isPost(context)) {
+            return new SeeOtherAction(redirectUrl);
+        } else {
+            return new FoundAction(redirectUrl);
+        }
     }
 
     public ClientFinder getClientFinder() {

pac4j-core/src/main/java/org/pac4j/core/engine/DefaultLogoutLogic.java

@@ -10,7 +10,7 @@
 import org.pac4j.core.exception.http.HttpAction;
 import org.pac4j.core.exception.http.NoContentAction;
 import org.pac4j.core.exception.http.RedirectionAction;
-import org.pac4j.core.exception.http.TemporaryRedirectAction;
+import org.pac4j.core.exception.http.FoundAction;
 import org.pac4j.core.http.adapter.HttpActionAdapter;
 import org.pac4j.core.profile.CommonProfile;
 import org.pac4j.core.profile.ProfileManager;
@@ -94,7 +94,7 @@ public R perform(final C context, final Config config, final HttpActionAdapter<R
             }
             logger.debug("redirectUrl: {}", redirectUrl);
             if (redirectUrl != null) {
-                action = new TemporaryRedirectAction(redirectUrl);
+                action = new FoundAction(redirectUrl);
             } else {
                 action = NoContentAction.INSTANCE;
             }

pac4j-core/src/main/java/org/pac4j/core/exception/http/TemporaryRedirectAction.java → pac4j-core/src/main/java/org/pac4j/core/exception/http/FoundAction.java

@@ -3,17 +3,17 @@
 import org.pac4j.core.context.HttpConstants;
 
 /**
- * A temporary redirect HTTP action.
+ * A "Found" HTTP action.
  *
  * @author Jerome Leleu
  * @since 4.0.0
  */
-public class TemporaryRedirectAction extends RedirectionAction {
+public class FoundAction extends RedirectionAction {
 
     private final String location;
 
-    public TemporaryRedirectAction(final String location) {
-        super(HttpConstants.TEMP_REDIRECT);
+    public FoundAction(final String location) {
+        super(HttpConstants.FOUND);
         this.location = location;
     }
 

pac4j-core/src/main/java/org/pac4j/core/exception/http/SeeOtherAction.java

@@ -0,0 +1,23 @@
+package org.pac4j.core.exception.http;
+
+import org.pac4j.core.context.HttpConstants;
+
+/**
+ * A "See Other" HTTP action.
+ *
+ * @author Jerome Leleu
+ * @since 4.0.0
+ */
+public class SeeOtherAction extends RedirectionAction {
+
+    private final String location;
+
+    public SeeOtherAction(final String location) {
+        super(HttpConstants.SEE_OTHER);
+        this.location = location;
+    }
+
+    public String getLocation() {
+        return location;
+    }
+}

pac4j-core/src/main/java/org/pac4j/core/http/adapter/JEEHttpActionAdapter.java

@@ -3,10 +3,7 @@
 import org.pac4j.core.context.HttpConstants;
 import org.pac4j.core.context.JEEContext;
 import org.pac4j.core.exception.TechnicalException;
-import org.pac4j.core.exception.http.HttpAction;
-import org.pac4j.core.exception.http.NoContentAction;
-import org.pac4j.core.exception.http.OkAction;
-import org.pac4j.core.exception.http.TemporaryRedirectAction;
+import org.pac4j.core.exception.http.*;
 
 /**
  * The HTTP action adapter for the {@link JEEContext}.
@@ -27,8 +24,10 @@ public Object adapt(final HttpAction action, final JEEContext context) {
                 context.writeResponseContent("");
             } else if (action instanceof OkAction) {
                 context.writeResponseContent(((OkAction) action).getContent());
-            } else if (action instanceof TemporaryRedirectAction) {
-                context.setResponseHeader(HttpConstants.LOCATION_HEADER, ((TemporaryRedirectAction) action).getLocation());
+            } else if (action instanceof FoundAction) {
+                context.setResponseHeader(HttpConstants.LOCATION_HEADER, ((FoundAction) action).getLocation());
+            } else if (action instanceof SeeOtherAction) {
+                context.setResponseHeader(HttpConstants.LOCATION_HEADER, ((SeeOtherAction) action).getLocation());
             }
 
             return null;

pac4j-core/src/main/java/org/pac4j/core/http/ajax/DefaultAjaxRequestResolver.java

@@ -24,11 +24,11 @@ public boolean isAjax(final WebContext context) {
 
     @Override
     public HttpAction buildAjaxResponse(final RedirectionAction action, final WebContext context) {
-        if (!(action instanceof TemporaryRedirectAction)) {
+        if (!(action instanceof FoundAction)) {
             throw UnauthorizedAction.INSTANCE;
         }
 
-        final String url = ((TemporaryRedirectAction) action).getLocation();
+        final String url = ((FoundAction) action).getLocation();
 
         if ( CommonHelper.isBlank(context.getRequestParameter(FACES_PARTIAL_AJAX_PARAMETER))) {
             if (CommonHelper.isNotBlank(url)) {

pac4j-core/src/main/java/org/pac4j/core/logout/CasLogoutActionBuilder.java

@@ -1,7 +1,7 @@
 package org.pac4j.core.logout;
 
 import org.pac4j.core.exception.http.RedirectionAction;
-import org.pac4j.core.exception.http.TemporaryRedirectAction;
+import org.pac4j.core.exception.http.FoundAction;
 import org.pac4j.core.profile.UserProfile;
 import org.pac4j.core.context.WebContext;
 
@@ -43,7 +43,7 @@ public RedirectionAction getLogoutAction(final WebContext context, final UserPro
             redirectUrl = addParameter(redirectUrl, postLogoutUrlParameter, targetUrl);
         }
         logger.debug("redirectUrl: {}", redirectUrl);
-        return new TemporaryRedirectAction(redirectUrl);
+        return new FoundAction(redirectUrl);
     }
 
     @Override

pac4j-core/src/main/java/org/pac4j/core/logout/GoogleLogoutActionBuilder.java

@@ -2,7 +2,7 @@
 
 import org.pac4j.core.context.WebContext;
 import org.pac4j.core.exception.http.RedirectionAction;
-import org.pac4j.core.exception.http.TemporaryRedirectAction;
+import org.pac4j.core.exception.http.FoundAction;
 import org.pac4j.core.profile.UserProfile;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -22,6 +22,6 @@ public RedirectionAction getLogoutAction(final WebContext context, final UserPro
 
         final String redirectUrl = "https://accounts.google.com/Logout";
         logger.debug("redirectUrl: {}", redirectUrl);
-        return new TemporaryRedirectAction(redirectUrl);
+        return new FoundAction(redirectUrl);
     }
 }