improvments : use pac4j 1.4.0 release, avoid check on content, use An…

…yContent, better handle 403
rhudson · Apr 23, 2013 · 7880dd1 · 7880dd1
1 parent b4ae62a
commit 7880dd1
Show file tree

Hide file tree

Showing 8 changed files with 46 additions and 39 deletions.
diff --git a/play-pac4j_java/src/main/java/org/pac4j/play/CallbackController.java b/play-pac4j_java/src/main/java/org/pac4j/play/CallbackController.java
@@ -81,9 +81,6 @@ public static Result callback() {
             } else if (code == HttpConstants.OK) {
                 String content = context.getResponseContent();
                 logger.debug("render : {}", content);
-                if (content == null) {
-                    content = "";
-                }
                 return ok(content);
             }
             final String message = "Unsupported HTTP action : " + code;
@@ -97,6 +94,12 @@ public static Result callback() {
 
         // get or create sessionId
         final String sessionId = StorageHelper.getOrCreationSessionId(session());
+
+        if (profile == null) {
+            // save that this kind of authentication has already been attempted and returns a null profile
+            StorageHelper.save(sessionId, client.getName() + Constants.ATTEMPTED_AUTHENTICATION_SUFFIX, "true");
+        }
+
         // save user profile
         StorageHelper.saveProfile(sessionId, profile);
         // get requested url

diff --git a/play-pac4j_java/src/main/java/org/pac4j/play/Config.java b/play-pac4j_java/src/main/java/org/pac4j/play/Config.java
@@ -34,8 +34,8 @@ public final class Config {
     // 1 hour = 3600 seconds
     private static int profileTimeout = 3600;
 
-    // 10 minutes = 600 seconds
-    private static int sessionTimeout = 600;
+    // 1 minute = 60 second
+    private static int sessionTimeout = 60;
 
     // all the clients
     private static Clients clients;

diff --git a/play-pac4j_java/src/main/java/org/pac4j/play/Constants.java b/play-pac4j_java/src/main/java/org/pac4j/play/Constants.java
@@ -37,5 +37,5 @@ public interface Constants {
 
     public final static String SEPARATOR = "$";
 
-    public final static String START_AUTHENTICATION_SUFFIX = SEPARATOR + "startAuthentication";
+    public final static String ATTEMPTED_AUTHENTICATION_SUFFIX = SEPARATOR + "attemptedAuthentication";
 }
diff --git a/play-pac4j_java/src/main/java/org/pac4j/play/java/RequiresAuthenticationAction.java b/play-pac4j_java/src/main/java/org/pac4j/play/java/RequiresAuthenticationAction.java
@@ -75,13 +75,13 @@ public Result call(final Context context) throws Throwable {
         if (profile != null) {
             return this.delegate.call(context);
         }
-        // no profile -> should try authentication if it has not already been tried
-        final String startAuth = (String) StorageHelper.get(sessionId, clientName
-                                                                       + Constants.START_AUTHENTICATION_SUFFIX);
-        logger.debug("startAuth : {}", startAuth);
-        StorageHelper.remove(sessionId, clientName + Constants.START_AUTHENTICATION_SUFFIX);
-        if (CommonHelper.isNotBlank(startAuth)) {
-            logger.error("not authenticated successfully to access a protected area -> forbidden");
+        // no profile -> has this authentication already be attempted ?
+        final String triedAuth = (String) StorageHelper.get(sessionId, clientName
+                                                                       + Constants.ATTEMPTED_AUTHENTICATION_SUFFIX);
+        logger.debug("triedAuth : {}", triedAuth);
+        if (CommonHelper.isNotBlank(triedAuth)) {
+            StorageHelper.remove(sessionId, clientName + Constants.ATTEMPTED_AUTHENTICATION_SUFFIX);
+            logger.error("authentication already tried -> forbidden");
             return forbidden(Config.getErrorPage403()).as(Constants.HTML_CONTENT_TYPE);
         }
         // requested url to save
@@ -95,8 +95,6 @@ public Result call(final Context context) throws Throwable {
         final String redirectionUrl = client
             .getRedirectionUrl(new JavaWebContext(context.request(), context.response(), context.session()), true);
         logger.debug("redirectionUrl : {}", redirectionUrl);
-        // save that this kind of authentication has already been tried
-        StorageHelper.save(sessionId, clientName + Constants.START_AUTHENTICATION_SUFFIX, "true");
         return redirect(redirectionUrl);
     }
 }
diff --git a/play-pac4j_java/src/main/java/org/pac4j/play/scala/ScalaWebContext.java b/play-pac4j_java/src/main/java/org/pac4j/play/scala/ScalaWebContext.java
@@ -33,13 +33,13 @@
  * @author Jerome Leleu
  * @since 1.0.0
  */
-public class ScalaWebContext implements WebContext {
+public class ScalaWebContext<C> implements WebContext {
 
-    private final Request<AnyContent> request;
+    private final Request<C> request;
 
     private final Session session;
 
-    public ScalaWebContext(final Request<AnyContent> request, final Session session) {
+    public ScalaWebContext(final Request<C> request, final Session session) {
         this.request = request;
         this.session = session;
     }
@@ -58,8 +58,8 @@ public String getRequestParameter(final String name) {
         if (values.isDefined()) {
             value = values.get().head();
         }
-        if (value == null) {
-            Option<scala.collection.immutable.Map<String, Seq<String>>> formParameters = this.request.body()
+        if (value == null && this.request instanceof AnyContent) {
+            Option<scala.collection.immutable.Map<String, Seq<String>>> formParameters = ((Request<AnyContent>)(this.request)).body()
                 .asFormUrlEncoded();
             if (formParameters.isDefined()) {
                 values = formParameters.get().get(name);

diff --git a/play-pac4j_scala2.10/src/main/scala/org/pac4j/play/scala/ScalaController.scala b/play-pac4j_scala2.10/src/main/scala/org/pac4j/play/scala/ScalaController.scala
@@ -43,7 +43,6 @@ trait ScalaController extends Controller {
    * @return the (updated) session
    */
   protected def getOrCreateSessionId(request: RequestHeader): Session = {
-    var sessionId: String = null
     var newSession = request.session
     val optionSessionId = newSession.get(Constants.SESSION_ID)
     logger.debug("getOrCreateSessionId : {}", optionSessionId)
@@ -62,29 +61,33 @@ trait ScalaController extends Controller {
    * @param action
    * @return the current action to process or the redirection to the provider if the user is not authenticated
    */
-  protected def RequiresAuthentication(clientName: String, targetUrl: String = "")(action: CommonProfile => Action[AnyContent]) = Action { request =>
+  protected def RequiresAuthentication[A](clientName: String, targetUrl: String, parser:BodyParser[A])(action: CommonProfile => Action[A]) = Action(parser) { request =>
+    logger.debug("Entering RequiresAuthentication")
     var newSession = getOrCreateSessionId(request)
     val sessionId = newSession.get(Constants.SESSION_ID).get
     logger.debug("sessionId : {}", sessionId)
     val profile = getUserProfile(request)
     logger.debug("profile : {}", profile)
     if (profile == null) {
-      val startAuth = StorageHelper.get(sessionId, clientName + Constants.START_AUTHENTICATION_SUFFIX).asInstanceOf[String]
-      logger.debug("startAuth : {}", startAuth);
-      StorageHelper.remove(sessionId, clientName + Constants.START_AUTHENTICATION_SUFFIX)
-      if (CommonHelper.isNotBlank(startAuth)) {
-        logger.error("not authenticated successfully to access a protected area -> forbidden")
+      val triedAuth = StorageHelper.get(sessionId, clientName + Constants.ATTEMPTED_AUTHENTICATION_SUFFIX).asInstanceOf[String]
+      logger.debug("triedAuth : {}", triedAuth);
+      if (CommonHelper.isNotBlank(triedAuth)) {
+        StorageHelper.remove(sessionId, clientName + Constants.ATTEMPTED_AUTHENTICATION_SUFFIX)
+        logger.error("authentication already tried -> forbidden")
         Forbidden(Config.getErrorPage403()).as(HTML)
       } else {
         val redirectionUrl = getRedirectionUrl(request, newSession, clientName, targetUrl, true)
         logger.debug("redirectionUrl : {}", redirectionUrl)
-        StorageHelper.save(sessionId, clientName + Constants.START_AUTHENTICATION_SUFFIX, "true")
         Redirect(redirectionUrl).withSession(newSession)
       }
     } else {
       action(profile)(request)
     }
   }
+
+  protected def RequiresAuthentication(clientName: String, targetUrl: String = "")(action: CommonProfile => Action[AnyContent]): Action[AnyContent] = { 
+    RequiresAuthentication(clientName, targetUrl, parse.anyContent)(action)
+  }
 
   /**
    * Returns the redirection url to the provider for authentication.
@@ -96,7 +99,7 @@ trait ScalaController extends Controller {
    * @param forceDirectRedirection
    * @return the redirection url to the provider
    */
-  protected def getRedirectionUrl(request: Request[AnyContent], newSession: Session, clientName: String, targetUrl: String = "", forceDirectRedirection: Boolean = false): String = {
+  protected def getRedirectionUrl[A](request: Request[A], newSession: Session, clientName: String, targetUrl: String = "", forceDirectRedirection: Boolean = false): String = {
     val sessionId = newSession.get(Constants.SESSION_ID).get
     logger.debug("sessionId for getRedirectionUrl() : {}", sessionId)
     // save requested url to save

diff --git a/play-pac4j_scala2.9/src/main/scala/org/pac4j/play/scala/ScalaController.scala b/play-pac4j_scala2.9/src/main/scala/org/pac4j/play/scala/ScalaController.scala
@@ -43,7 +43,6 @@ trait ScalaController extends Controller {
    * @return the (updated) session
    */
   protected def getOrCreateSessionId(request: RequestHeader): Session = {
-    var sessionId: String = null
     var newSession = request.session
     val optionSessionId = newSession.get(Constants.SESSION_ID)
     logger.debug("getOrCreateSessionId : {}", optionSessionId)
@@ -62,29 +61,33 @@ trait ScalaController extends Controller {
    * @param action
    * @return the current action to process or the redirection to the provider if the user is not authenticated
    */
-  protected def RequiresAuthentication(clientName: String, targetUrl: String = "")(action: CommonProfile => Action[AnyContent]) = Action { request =>
+  protected def RequiresAuthentication[A](clientName: String, targetUrl: String, parser:BodyParser[A])(action: CommonProfile => Action[A]) = Action(parser) { request =>
+    logger.debug("Entering RequiresAuthentication")
     var newSession = getOrCreateSessionId(request)
     val sessionId = newSession.get(Constants.SESSION_ID).get
     logger.debug("sessionId : {}", sessionId)
     val profile = getUserProfile(request)
     logger.debug("profile : {}", profile)
     if (profile == null) {
-      val startAuth = StorageHelper.get(sessionId, clientName + Constants.START_AUTHENTICATION_SUFFIX).asInstanceOf[String]
-      logger.debug("startAuth : {}", startAuth);
-      StorageHelper.remove(sessionId, clientName + Constants.START_AUTHENTICATION_SUFFIX)
-      if (CommonHelper.isNotBlank(startAuth)) {
-        logger.error("not authenticated successfully to access a protected area -> forbidden")
+      val triedAuth = StorageHelper.get(sessionId, clientName + Constants.ATTEMPTED_AUTHENTICATION_SUFFIX).asInstanceOf[String]
+      logger.debug("triedAuth : {}", triedAuth);
+      if (CommonHelper.isNotBlank(triedAuth)) {
+        StorageHelper.remove(sessionId, clientName + Constants.ATTEMPTED_AUTHENTICATION_SUFFIX)
+        logger.error("authentication already tried -> forbidden")
         Forbidden(Config.getErrorPage403()).as(HTML)
       } else {
         val redirectionUrl = getRedirectionUrl(request, newSession, clientName, targetUrl, true)
         logger.debug("redirectionUrl : {}", redirectionUrl)
-        StorageHelper.save(sessionId, clientName + Constants.START_AUTHENTICATION_SUFFIX, "true")
         Redirect(redirectionUrl).withSession(newSession)
       }
     } else {
       action(profile)(request)
     }
   }
+
+  protected def RequiresAuthentication(clientName: String, targetUrl: String = "")(action: CommonProfile => Action[AnyContent]): Action[AnyContent] = { 
+    RequiresAuthentication(clientName, targetUrl, parse.anyContent)(action)
+  }
 
   /**
    * Returns the redirection url to the provider for authentication.
@@ -96,7 +99,7 @@ trait ScalaController extends Controller {
    * @param forceDirectRedirection
    * @return the redirection url to the provider
    */
-  protected def getRedirectionUrl(request: Request[AnyContent], newSession: Session, clientName: String, targetUrl: String = "", forceDirectRedirection: Boolean = false): String = {
+  protected def getRedirectionUrl[A](request: Request[A], newSession: Session, clientName: String, targetUrl: String = "", forceDirectRedirection: Boolean = false): String = {
     val sessionId = newSession.get(Constants.SESSION_ID).get
     logger.debug("sessionId for getRedirectionUrl() : {}", sessionId)
     // save requested url to save

diff --git a/pom.xml b/pom.xml
@@ -82,7 +82,7 @@
 	</modules>
 
 	<properties>
-		<pac4j.version>1.4.0-SNAPSHOT</pac4j.version>
+		<pac4j.version>1.4.0</pac4j.version>
 	</properties>
 
 	<dependencyManagement>