[smarcet] - #13099

* (openid) nonce/asocciations table cleanup cron task
* refactoring redirect url ( openid)

cron_jobs_scheduler/_config/schedule.yml

@@ -110,4 +110,8 @@ jobs:
   - name: "RevocationExecutorTask"
     params: "batch_size=10000"
     cron_expression: "59 11 * * *"
-    enabled: 0
+    enabled: 0
+
+  - name: "OpenStackIdCleanInvalidNoncesAssocsTask"
+    cron_expression: "00 03 * * *" # run at 0300 AM every day
+    enabled: 1

openstack/code/utils/DB/CustomMySQLDatabase.php

@@ -97,7 +97,7 @@ public function transactionEnd($chain = false){
 
     public function query($sql, $errorLevel = E_USER_ERROR) {
         $query = parent::query($sql, $errorLevel);
-        SS_Log::log($sql, SS_Log::DEBUG);
+        //SS_Log::log($sql, SS_Log::DEBUG);
         return $query;
     }
 

openstackid/_config/injector.yml

@@ -9,10 +9,19 @@ Injector:
   OpenStackIdMySQLStore:
     constructor:
       0: %$OpenStackIdDatabaseConnection
-  MyOpenIDConsumer:
-    class: Auth_OpenID_Consumer
+  Auth_OpenID_Consumer:
     constructor:
-          0: %$OpenStackIdMySQLStore
-          1: %$SilverStripeSessionWrapper
+      0: '%$OpenStackIdMySQLStore'
+      1: '%$SilverStripeSessionWrapper'
   Security:
-    class: OpenStackIdSecurityController
+    class: OpenStackIdSecurityController
+    constructor:
+      0: '%$Auth_OpenID_Consumer'
+  OpenStackIdAuthenticator:
+    constructor:
+      0: '%$MemberRepository'
+      1: '%$OpenStackIdMySQLStore'
+      2: '%$Auth_OpenID_Consumer'
+  OpenStackIdCleanInvalidNoncesAssocsTask:
+    constructor:
+      0: '%$OpenStackIdMySQLStore'

openstackid/code/OpenStackIdCommon.php

@@ -21,21 +21,36 @@ public static function redirectToSSL($url){
         die("<h1>Your browser is not accepting header redirects</h1><p>Please <a href=\"$dest\">click here</a>");
     }
 
+    /**
+     * @return string
+     */
     public static function getReturnTo()
     {
         $trust_root    = self::getTrustRoot();
-        $return_to_url = $trust_root . '/OpenStackIdAuthenticator?url=/OpenStackIdAuthenticator';
-        $back_url      = Controller::curr()->getRequest()->getVar('BackURL');
-        if(!empty($back_url)){
-            $back_url = self::cleanBackUrl($back_url);
-            if(!Director::is_site_url($back_url)){
-                $back_url = Director::absoluteBaseURL();
-            }
-            $fragment = Controller::curr()->getRequest()->requestVar('fragment');
-            if(!empty($fragment)) $back_url .= $fragment;
-            $return_to_url .= '&BackURL='.urlencode($back_url);
-        }
-        return $return_to_url;
+        $return_to_url = "{$trust_root}/OpenStackIdAuthenticator?url=/OpenStackIdAuthenticator";
+        // check first on session ...
+        $back_url      = urlencode(self::getRedirectBackUrl());
+        return "{$return_to_url}&BackURL={$back_url}";
+    }
+
+    /**
+     * @return string
+     */
+    public static function getRedirectBackUrl(){
+        // check first on session ...
+        $back_url = Controller::curr()->getSession()->get("BackURL");
+        if(empty($back_url))
+            $back_url = Controller::curr()->getRequest()->requestVar('BackURL');
+        $fragment = Controller::curr()->getRequest()->requestVar('fragment');
+
+        if(empty($back_url))
+            $back_url = Director::baseURL();
+        if(!empty($fragment))
+            $back_url .= $fragment;
+
+        $back_url = Director::absoluteURL($back_url, true);
+
+        return $back_url;
     }
 
     public static function getTrustRoot()
@@ -47,28 +62,6 @@ public static function escape($thing) {
         return htmlentities($thing);
     }
 
-    public static function getRedirectBackUrl(){
-        $url = null;
-        // Don't cache the redirect back ever
-        HTTP::set_cache_age(0);
-        // In edge-cases, this will be called outside of a handleRequest() context; in that case,
-        // redirect to the homepage - don't break into the global state at this stage because we'll
-        // be calling from a test context or something else where the global state is inappropraite
-        if($request = Controller::curr()->getRequest()) {
-            if($request->requestVar('BackURL')) {
-                $url = $request->requestVar('BackURL');
-            } else if($request->isAjax() && $request->getHeader('X-Backurl')) {
-                $url = $request->getHeader('X-Backurl');
-            }
-        }
-
-        $url = self::cleanBackUrl($url);
-
-        if(strpos($url,'/Security/login') !== false ) $url = Director::baseURL();
-
-        return $url;
-    }
-
     public static function loginMember($member, $back_url){
 
         $back_url = self::cleanBackUrl($back_url);
@@ -107,4 +100,15 @@ public static function cleanBackUrl($back_url){
             $back_url = Director::baseURL();
         return $back_url;
     }
+
+    /**
+     * @param string $message
+     * @param string $back_url
+     * @return SS_HTTPResponse
+     */
+    public static function error($message, $back_url){
+        Session::set("Security.Message.message", $message);
+        Session::set("Security.Message.type", "bad");
+        return Controller::curr()->redirect("Security/error?BackURL={$back_url}");
+    }
 }

openstackid/code/cron_tasks/OpenStackIdCleanInvalidNoncesAssocsTask.php

@@ -0,0 +1,51 @@
+<?php
+/**
+ * Copyright 2017 OpenStack Foundation
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ **/
+
+/**
+ * Class OpenStackIdCleanInvalidNoncesAssocsTask
+ */
+final class OpenStackIdCleanInvalidNoncesAssocsTask extends CronTask
+{
+
+    /**
+     * @var Auth_OpenID_OpenIDStore
+     */
+    private $openid_repository;
+
+    public function __construct(Auth_OpenID_OpenIDStore $openid_repository)
+    {
+        parent::__construct();
+        $this->openid_repository = $openid_repository;
+    }
+
+    /**
+     * @return void
+     */
+    public function run()
+    {
+        try
+        {
+            $init_time  = time();
+            list($nonces_expired, $assoc_expired) = $this->openid_repository->cleanup();
+            $finish_time = time() - $init_time;
+            echo "nonces expired {$nonces_expired}".PHP_EOL;
+            echo "associations expired {$assoc_expired}".PHP_EOL;
+            echo "time elapsed : {$finish_time} seconds.".PHP_EOL;
+        }
+        catch(Exception $ex)
+        {
+            SS_Log::log($ex->getMessage(), SS_Log::ERR);
+        }
+    }
+}

openstackid/code/infrastructure/OpenStackIdDatabaseConnection.php

@@ -18,11 +18,25 @@
  */
 class OpenStackIdDatabaseConnection extends Auth_OpenID_DatabaseConnection {
 
+    /**
+     * @var
+     */
+    private $last_affected_rows;
+
     public function query($sql, $params = array()) {
         if(($sql = $this->generateQuery($sql, $params)) === false)
             return false;
 
-        return DB::query($sql);
+        $query = DB::query($sql);
+        $this->last_affected_rows = $query->numRecords();
+        return $query;
+    }
+
+    /**
+     * @return int
+     */
+    public function affectedRows(){
+        return is_null($this->last_affected_rows) ? 0 : intval($this->last_affected_rows);
     }
 
     public function getOne($sql, $params = array()) {
@@ -69,7 +83,6 @@ function begin()
         DB::getConn()->transactionStart();
     }
 
-
     public function commit() {
         DB::getConn()->transactionEnd();
     }

openstackid/code/ui/OpenStackIdAuthenticator.php

@@ -26,15 +26,37 @@ class OpenStackIdAuthenticator extends Controller
      */
     private $member_repository;
 
-    public function __construct()
+    /**
+     * @var Auth_OpenID_Consumer
+     */
+    private $openid_consumer;
+
+    /**
+     * @var Auth_OpenID_OpenIDStore
+     */
+    private $openid_repository;
+
+    /**
+     * OpenStackIdAuthenticator constructor.
+     * @param IMemberRepository $member_repository
+     * @param Auth_OpenID_OpenIDStore $openid_repository
+     * @param Auth_OpenID_Consumer $openid_consumer
+     */
+    public function __construct
+    (
+        IMemberRepository $member_repository,
+        Auth_OpenID_OpenIDStore $openid_repository,
+        Auth_OpenID_Consumer $openid_consumer
+    )
     {
         parent::__construct();
-        $this->member_repository = new SapphireCLAMemberRepository();
+        $this->member_repository = $member_repository;
+        $this->openid_repository = $openid_repository;
+        $this->openid_consumer   = $openid_consumer;
     }
 
     function index()
     {
-
         try {
 
             $member = Member::currentUser();
@@ -43,27 +65,27 @@ function index()
                 // user is already logged in
                 return $this->redirect(OpenStackIdCommon::getRedirectBackUrl());
             }
-
-            $consumer = Injector::inst()->get('MyOpenIDConsumer');
-
             $query       = Auth_OpenID::getQuery();
+
             $message     = Auth_OpenID_Message::fromPostArgs($query);
             $nonce       = $message->getArg(Auth_OpenID_OPENID2_NS,'response_nonce');
             list($timestamp, $salt) = Auth_OpenID_splitNonce($nonce);
             $claimed_id  = $message->getArg(Auth_OpenID_OPENID2_NS,'claimed_id');
 
-            SS_Log::log(sprintf('OpenStackIdAuthenticator : id %s - salt %s - timestamp %s',$claimed_id, $salt, $timestamp), SS_Log::DEBUG);
+            SS_Log::log(sprintf('OpenStackIdAuthenticator : id %s - salt %s - timestamp %s - query %s',$claimed_id, $salt, $timestamp, implode(', ',$query)), SS_Log::DEBUG);
 
             // Complete the authentication process using the server's response.
-            $response = $consumer->complete(OpenStackIdCommon::getReturnTo());
+            $response = $this->openid_consumer->complete(OpenStackIdCommon::getReturnTo());
 
             if ($response->status == Auth_OpenID_CANCEL) {
                 SS_Log ::log('OpenStackIdAuthenticator : Auth_OpenID_CANCEL', SS_Log::WARN);
                 throw new Exception('The verification was cancelled. Please try again.');
-
             } else if ($response->status == Auth_OpenID_FAILURE) {
-                SS_Log ::log('OpenStackIdAuthenticator : Auth_OpenID_FAILURE', SS_Log::WARN);
-                throw new Exception("The OpenID authentication failed.");
+                SS_Log ::log("OpenStackIdAuthenticator : Auth_OpenID_FAILURE {$response->message}", SS_Log::WARN);
+                // delete associations
+                SS_Log ::log("OpenStackIdAuthenticator : Auth_OpenID_FAILURE cleaning openid_repository ...", SS_Log::WARN);
+                $this->openid_repository->reset();
+                throw new Exception("The OpenID authentication failed");
 
             } else if ($response->status == Auth_OpenID_SUCCESS) {
                 SS_Log ::log('OpenStackIdAuthenticator : Auth_OpenID_SUCCESS', SS_Log::DEBUG);
@@ -98,10 +120,8 @@ function index()
                 throw new Exception("The OpenID authentication failed: can not find user ".$openid);
             }
         } catch (Exception $ex) {
-            Session::set("Security.Message.message", $ex->getMessage());
-            Session::set("Security.Message.type", "bad");
-            SS_Log ::log($ex, SS_Log::DEBUG);
-            return $this->redirect("Security/badlogin");
+            SS_Log ::log($ex, SS_Log::WARN);
+            return OpenStackIdCommon::error($ex->getMessage(), OpenStackIdCommon::getRedirectBackUrl());
         }
     }