Remove the ability to compute login urls directly

pac4j-cas/src/main/java/org/pac4j/cas/client/CasClient.java

@@ -435,11 +435,6 @@ public String toString() {
                 "allowedProxyChains", this.allowedProxyChains, "casProxyReceptor", this.casProxyReceptor);
     }
 
-    @Override
-    protected boolean isDirectRedirection() {
-        return true;
-    }
-
     @Override
     public ClientType getClientType() {
         return ClientType.CAS_PROTOCOL;

pac4j-cas/src/main/java/org/pac4j/cas/client/CasProxyReceptor.java

@@ -148,11 +148,6 @@ protected CasProfile retrieveUserProfile(final CasCredentials credentials, final
         throw new TechnicalException("Not supported by the CAS proxy receptor");
     }
 
-    @Override
-    protected boolean isDirectRedirection() {
-        return true;
-    }
-
     @Override
     public ClientType getClientType() {
         return ClientType.CAS_PROTOCOL;

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

@@ -73,12 +73,12 @@ public void testRenew() throws RequiresHttpAction {
         casClient.setCallbackUrl(CALLBACK_URL);
         casClient.setCasLoginUrl(LOGIN_URL);
         MockWebContext context = MockWebContext.create();
-		casClient.redirect(context, false);
+		casClient.redirect(context);
         assertFalse(context.getResponseLocation().indexOf("renew=true") >= 0);
         casClient.setRenew(true);
         casClient.reinit(null);
         context = MockWebContext.create();
-        casClient.redirect(context, false);
+        casClient.redirect(context);
         assertTrue(context.getResponseLocation().indexOf("renew=true") >= 0);
     }
 
@@ -87,11 +87,11 @@ public void testGateway() throws RequiresHttpAction {
         casClient.setCallbackUrl(CALLBACK_URL);
         casClient.setCasLoginUrl(LOGIN_URL);
         final MockWebContext context = MockWebContext.create();
-        casClient.redirect(context, false);
+        casClient.redirect(context);
         assertFalse(context.getResponseLocation().indexOf("gateway=true") >= 0);
         casClient.setGateway(true);
         casClient.reinit(null);
-        casClient.redirect(context, false);
+        casClient.redirect(context);
         assertTrue(context.getResponseLocation().indexOf("gateway=true") >= 0);
         final CasCredentials credentials = casClient.getCredentials(context);
         assertNull(credentials);

pac4j-core/src/main/java/org/pac4j/core/client/BaseClient.java

@@ -29,7 +29,7 @@
 import org.slf4j.LoggerFactory;
 
 /**
- * <p>This class is the default implementation of an authentication client (whatever the protocol). It has the core concepts:</p>
+ * <p>This class is the default implementation of an authentication client (whatever the mechanism). It has the core concepts:</p>
  * <ul>
  * <li>The initialization process is handled by the {@link InitializableWebObject} inheritance, the {@link #internalInit(WebContext)} must be implemented
  * in sub-classes. The {@link #init(WebContext)} method must be called implicitly by the main methods of the {@link Client} interface, so that no explicit call is
@@ -85,9 +85,6 @@ public String getName() {
         return this.name;
     }
 
-    /**
-     * {@inheritDoc}
-     */
     @Override
     public final U getUserProfile(final C credentials, final WebContext context) {
         init(context);

pac4j-core/src/main/java/org/pac4j/core/client/Client.java

@@ -21,16 +21,16 @@
 import org.pac4j.core.profile.UserProfile;
 
 /**
- * <p>This interface is the core of <code>pac4j</code>. It represents an authentication mechanism to validate user's credentials and
- * retrieve the user profile.</p>
+ * <p>This interface is the core class of the library. It represents an authentication mechanism to validate user's credentials and
+ * retrieve his user profile.</p>
  * <p>Clients can be "indirect": in that case, credentials are not provided with the HTTP request, but the user must be redirected to
  * an identity provider to perform login, the original requested url being saved and restored after the authentication process is done.</p>
- * <p>The {@link #redirect(WebContext, boolean)} method is called to redirect the user to the identity provider,
+ * <p>The {@link #redirect(WebContext)} method is called to redirect the user to the identity provider,
  * the {@link #getCredentials(WebContext)} method is used to retrieve the credentials provided by the remote identity provider and
  * the {@link #getUserProfile(Credentials, WebContext)} method is called to get the user profile from the identity provider and based
  * on the provided credentials.</p>
  * <p>Clients can be "direct": in that case, credentials are provided along with the HTTP request and validated by the application.</p>
- * <p>The {@link #redirect(WebContext, boolean)} method is not used, the {@link #getCredentials(WebContext)} method is used to retrieve
+ * <p>The {@link #redirect(WebContext)} method is not used, the {@link #getCredentials(WebContext)} method is used to retrieve
  * and validate the credentials provided and the {@link #getUserProfile(Credentials, WebContext)} method is called to get the user profile from
  * the appropriate system.</p>
  * 
@@ -44,16 +44,15 @@ public interface Client<C extends Credentials, U extends UserProfile> {
      * 
      * @return the name of the client
      */
-    public String getName();
+    String getName();
 
     /**
      * <p>Redirect to the authentication provider for an indirect client.</p>
      *
      * @param context the current web context
-     * @param protectedTarget whether the target url is protected
      * @throws RequiresHttpAction whether an additional HTTP action is required
      */
-    public void redirect(WebContext context, boolean protectedTarget) throws RequiresHttpAction;
+    void redirect(WebContext context) throws RequiresHttpAction;
 
     /**
      * <p>Get the credentials from the web context. If no validation was made remotely (direct client), credentials must be validated at this step.</p>
@@ -63,7 +62,7 @@ public interface Client<C extends Credentials, U extends UserProfile> {
      * @return the credentials
      * @throws RequiresHttpAction whether an additional HTTP action is required
      */
-    public C getCredentials(WebContext context) throws RequiresHttpAction;
+    C getCredentials(WebContext context) throws RequiresHttpAction;
 
     /**
      * Get the user profile based on the provided credentials.
@@ -72,5 +71,5 @@ public interface Client<C extends Credentials, U extends UserProfile> {
      * @param context web context 
      * @return the user profile
      */
-    public U getUserProfile(C credentials, WebContext context);
+    U getUserProfile(C credentials, WebContext context);
 }

pac4j-core/src/main/java/org/pac4j/core/client/Clients.java

@@ -204,7 +204,7 @@ public List<Client> getClients() {
 
     @Override
     public String toString() {
-        return CommonHelper.toString(this.getClass(), "callbackUrl", this.callbackUrl, "clientTypeParameter",
+        return CommonHelper.toString(this.getClass(), "callbackUrl", this.callbackUrl, "clientNameParameter",
                 this.clientNameParameter, "clients", getClients());
     }
 }

pac4j-core/src/main/java/org/pac4j/core/client/DirectClient.java

@@ -21,16 +21,16 @@
 import org.pac4j.core.profile.CommonProfile;
 
 /**
- * <p>This class is the default direct (stateless) implementation of an authentication client (whatever the protocol).
- * In that case, redirecting does not have any sense.</p>
+ * <p>This class is the default direct (stateless) implementation of an authentication client (whatever the mechanism).
+ * In that case, redirecting does not make any sense.</p>
  *
  * @author Jerome Leleu
  * @since 1.8.0
  */
 public abstract class DirectClient<C extends Credentials, U extends CommonProfile> extends BaseClient<C, U> {
 
     @Override
-    public final void redirect(final WebContext context, final boolean protectedTarget) {
+    public final void redirect(final WebContext context) {
         throw new TechnicalException("direct clients do not support redirections");
     }
 }

pac4j-core/src/main/java/org/pac4j/core/client/IndirectClient.java

@@ -29,26 +29,15 @@
 import org.pac4j.core.util.CommonHelper;
 
 /**
- * <p>This class is the default indirect (with redirection, stateful) implementation of an authentication client (whatever the protocol).
- * It has the core concepts:</p>
- * <ul>
- * <li>The callback url is handled through the {@link #setCallbackUrl(String)} and {@link #getCallbackUrl()} methods</li>
- * <li>The concept of "direct" redirection is defined through the {@link #isDirectRedirection()} method: if true, the
- * {@link #redirect(WebContext, boolean)} method will always return the redirection to the provider where as if it's false, the
- * redirection url will be the callback url with an additional parameter: {@link #NEEDS_CLIENT_REDIRECTION_PARAMETER} to require the
- * redirection, which will be handled <b>later</b> in the {@link #getCredentials(WebContext)} method.
- * To force a direct redirection, the {@link #getRedirectAction(WebContext, boolean)} must be used with <code>true</code> for the
- * <code>protectedTarget</code> parameter</li>
- * <li>The way the callback url is finally computed is handled by the {@link #callbackUrlResolver} which is by default the provided {@link #callbackUrl}.</li>
- * </ul>
+ * <p>This class is the default indirect (with redirection, stateful) implementation of an authentication client (whatever the mechanism).</p>
+ * <p>The callback url is managed via the {@link #setCallbackUrl(String)} and {@link #getCallbackUrl()} methods. The way the callback url
+ * is finally computed is done by the {@link #callbackUrlResolver} which returns by default the provided {@link #callbackUrl}.</p>
  *
  * @author Jerome Leleu
  * @since 1.8.0
  */
 public abstract class IndirectClient<C extends Credentials, U extends CommonProfile> extends BaseClient<C, U> {
 
-    public final static String NEEDS_CLIENT_REDIRECTION_PARAMETER = "needs_client_redirection";
-
     public final static String ATTEMPTED_AUTHENTICATION_SUFFIX = "$attemptedAuthentication";
 
     protected String callbackUrl;
@@ -59,30 +48,10 @@ public abstract class IndirectClient<C extends Credentials, U extends CommonProf
 
     protected CallbackUrlResolver callbackUrlResolver = new DefaultCallbackUrlResolver();
 
-    /**
-     * Define if this client has a direct redirection.
-     * 
-     * @return if this client has a direct redirection
-     */
-    protected abstract boolean isDirectRedirection();
-
-    /**
-     * <p>Redirect to the authentication provider by updating the WebContext accordingly.</p>
-     * <p>Though, if this client requires an indirect redirection, it will return a redirection to the callback url (with an additionnal parameter requesting a
-     * redirection). Whatever the kind of client's redirection, the <code>protectedTarget</code> parameter set to <code>true</code> enforces
-     * a direct redirection.
-     * <p>If an authentication has already been tried for this client and has failed (previous <code>null</code> credentials) and if the target
-     * is protected (<code>protectedTarget</code> set to <code>true</code>), a forbidden response (403 HTTP status code) is returned.</p>
-     * <p>If the request is an AJAX one, an authorized response (401 HTTP status code) is returned instead of a redirection.</p>
-     *
-     * @param context the current web context
-     * @param protectedTarget whether the target url is protected
-     * @throws RequiresHttpAction whether an additional HTTP action is required
-     */
     @Override
-    public final void redirect(final WebContext context, final boolean protectedTarget)
+    public final void redirect(final WebContext context)
             throws RequiresHttpAction {
-        final RedirectAction action = getRedirectAction(context, protectedTarget);
+        final RedirectAction action = getRedirectAction(context);
         if (action.getType() == RedirectType.REDIRECT) {
             context.setResponseStatus(HttpConstants.TEMP_REDIRECT);
             context.setResponseHeader(HttpConstants.LOCATION_HEADER, action.getLocation());
@@ -93,41 +62,32 @@ public final void redirect(final WebContext context, final boolean protectedTarg
     }
 
     /**
-     * Get the redirectAction computed for this client. All the logic is encapsulated here. It should not be called be directly, the
-     * {@link #redirect(WebContext, boolean)} should be generally called instead.
-     * 
+     * <p>Get the redirectAction computed for this client. All the logic is encapsulated here. It should not be called be directly, the
+     * {@link #redirect(WebContext)} should be generally called instead.</p>
+     * <p>If an authentication has already been tried for this client and has failed (<code>null</code> credentials), a forbidden response (403 HTTP status code) is returned.</p>
+     * <p>If the request is an AJAX one, an authorized response (401 HTTP status code) is returned instead of a redirection.</p>
+     *
      * @param context context
-     * @param protectedTarget requires authentication
      * @return the redirection action
      * @throws RequiresHttpAction requires an additional HTTP action
      */
-    public final RedirectAction getRedirectAction(final WebContext context, final boolean protectedTarget) throws RequiresHttpAction {
+    public final RedirectAction getRedirectAction(final WebContext context) throws RequiresHttpAction {
         // it's an AJAX request -> unauthorized (instead of a redirection)
         if (ajaxRequestResolver.isAjax(context)) {
             logger.info("AJAX request detected -> returning 401");
             cleanRequestedUrl(context);
             throw RequiresHttpAction.unauthorized("AJAX request -> 401", context, null);
         }
-        // authentication has already been tried
+        // authentication has already been tried -> forbidden
         final String attemptedAuth = (String) context.getSessionAttribute(getName() + ATTEMPTED_AUTHENTICATION_SUFFIX);
         if (CommonHelper.isNotBlank(attemptedAuth)) {
             cleanAttemptedAuthentication(context);
-            // protected target -> forbidden
-            if (protectedTarget) {
-                cleanRequestedUrl(context);
-                throw RequiresHttpAction.forbidden("authentication already tried -> forbidden", context);
-            }
-        }
-        // it's a direct redirection or force the redirection because the target is protected -> return the real redirection
-        if (isDirectRedirection() || protectedTarget) {
-            init(context);
-            return retrieveRedirectAction(context);
-        } else {
-            // return an intermediate url which is the callback url with a specific parameter requiring redirection
-            final String intermediateUrl = CommonHelper.addParameter(computeFinalCallbackUrl(context),
-                    NEEDS_CLIENT_REDIRECTION_PARAMETER, "true");
-            return RedirectAction.redirect(intermediateUrl);
+            cleanRequestedUrl(context);
+            throw RequiresHttpAction.forbidden("authentication already tried -> forbidden", context);
         }
+
+        init(context);
+        return retrieveRedirectAction(context);
     }
 
     private void cleanRequestedUrl(final WebContext context) {
@@ -143,26 +103,16 @@ public String computeFinalCallbackUrl(final WebContext context) {
     }
 
     /**
-     * Return the redirection url to the provider, requested from an anonymous page.
+     * Retrieve the redirect action.
      *
-     * @param context the current web context
-     * @return the redirection url to the provider.
+     * @param context the web context
+     * @return the redirection action
      */
-    public String getRedirectionUrl(final WebContext context) {
-        try {
-            return getRedirectAction(context, false).getLocation();
-        } catch (final RequiresHttpAction e) {
-            return null;
-        }
-    }
-
     protected abstract RedirectAction retrieveRedirectAction(final WebContext context);
 
     /**
-     * <p>Get the credentials from the web context. In some cases, a {@link RequiresHttpAction} may be thrown instead:</p>
+     * <p>Get the credentials from the web context. In some cases, a {@link RequiresHttpAction} may be thrown:</p>
      * <ul>
-     * <li>if this client requires an indirect redirection, the redirection will be actually performed by these method and not by the
-     * {@link #redirect(WebContext, boolean)} one (302 HTTP status code)</li>
      * <li>if the <code>CasClient</code> receives a logout request, it returns a 200 HTTP status code</li>
      * <li>for the <code>IndirectBasicAuthClient</code>, if no credentials are sent to the callback url, an unauthorized response (401 HTTP status
      * code) is returned to request credentials through a popup.</li>
@@ -175,30 +125,22 @@ public String getRedirectionUrl(final WebContext context) {
     @Override
     public final C getCredentials(final WebContext context) throws RequiresHttpAction {
         init(context);
-        final String value = context.getRequestParameter(NEEDS_CLIENT_REDIRECTION_PARAMETER);
-        // needs redirection -> return the redirection url
-        if (CommonHelper.isNotBlank(value)) {
-            final RedirectAction action = retrieveRedirectAction(context);
-            final String message = "Needs client redirection";
-            if (action.getType() == RedirectType.SUCCESS) {
-                throw RequiresHttpAction.ok(message, context, action.getContent());
-            } else {
-                // it's a redirect
-                throw RequiresHttpAction.redirect(message, context, action.getLocation());
-            }
+        final C credentials = retrieveCredentials(context);
+        // no credentials -> save this authentication has already been tried and failed
+        if (credentials == null) {
+            context.setSessionAttribute(getName() + ATTEMPTED_AUTHENTICATION_SUFFIX, "true");
         } else {
-            // else get the credentials
-            final C credentials = retrieveCredentials(context);
-            // no credentials -> save this authentication has already been tried and failed
-            if (credentials == null) {
-                context.setSessionAttribute(getName() + ATTEMPTED_AUTHENTICATION_SUFFIX, "true");
-            } else {
-                cleanAttemptedAuthentication(context);
-            }
-            return credentials;
+            cleanAttemptedAuthentication(context);
         }
+        return credentials;
     }
 
+    /**
+     * Retrieve the credentials.
+     *
+     * @param context the web context
+     * @return the credentials.
+     */
     protected abstract C retrieveCredentials(final WebContext context) throws RequiresHttpAction;
 
     /**

pac4j-core/src/test/java/org/pac4j/core/client/ClientIT.java

@@ -166,8 +166,7 @@ protected boolean isJavascriptEnabled() {
     protected HtmlPage getRedirectionPage(final WebClient webClient, final Client<?, ?> client, final J2EContext context)
             throws Exception {
         final BaseClient baseClient = (BaseClient) client;
-        // force immediate redirection for tests
-        baseClient.redirect(context, true);
+        baseClient.redirect(context);
         final String redirectionUrl = context.getResponse().getHeader(HttpConstants.LOCATION_HEADER);
         logger.debug("redirectionUrl : {}", redirectionUrl);
         final HtmlPage loginPage = webClient.getPage(redirectionUrl);