BryanDavis has uploaded a new change for review. https://gerrit.wikimedia.org/r/248488
Change subject: Convert logging to PSR3 standard ...................................................................... Convert logging to PSR3 standard Replace calls to wfDebug() and wfDebugLog() with direct usage of Psr\Log\LoggerInterface instances obtained from MediaWiki\Logger\LoggerFactory. Log event levels selected according to guidelines at https://www.mediawiki.org/wiki/Structured_logging. Bug: T109465 Change-Id: I92ffac553b6552b26cb6f531c2d10d7bca777900 --- M backend/MWOAuthDAO.php M backend/MWOAuthDataStore.php M backend/MWOAuthRequest.php M backend/MWOAuthServer.php M examples/testClient.php M examples/testClientHeaders.php M frontend/specialpages/SpecialMWOAuth.php M lib/OAuth.php 8 files changed, 71 insertions(+), 45 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/OAuth refs/changes/88/248488/1 diff --git a/backend/MWOAuthDAO.php b/backend/MWOAuthDAO.php index 5c13270..04123c2 100644 --- a/backend/MWOAuthDAO.php +++ b/backend/MWOAuthDAO.php @@ -21,12 +21,18 @@ http://www.gnu.org/copyleft/gpl.html */ +use MediaWiki\Logger\LoggerFactory; +use Psr\Log\LoggerInterface; + /** * Representation of a Data Access Object */ abstract class MWOAuthDAO implements \IDBAccessObject { private $daoOrigin = 'new'; // string; object construction origin private $daoPending = true; // boolean; whether fields changed or the field is new + + /** @var \\Psr\\Log\\LoggerInterface */ + protected $logger; /** * @throws \MWException @@ -36,6 +42,7 @@ if ( array_diff( $fields, $this->getFieldNames() ) ) { throw new \MWException( "Invalid field(s) defined in access check methods." ); } + $this->logger = LoggerFactory::getLogger( 'OAuth' ); } /** @@ -153,7 +160,7 @@ } if ( $this->daoOrigin === 'db' ) { if ( $this->daoPending ) { - wfDebug( get_class( $this ) . ': performing DB update; object changed.' ); + $this->logger->debug( get_class( $this ) . ': performing DB update; object changed.' ); $dbw->update( static::getTable(), $this->getRowArray( $dbw ), @@ -167,11 +174,11 @@ $this->daoPending = false; return $dbw->affectedRows() > 0; } else { - wfDebug( get_class( $this ) . ': skipping DB update; object unchanged.' ); + $this->logger->debug( get_class( $this ) . ': skipping DB update; object unchanged.' ); return false; // short-circuit } } else { - wfDebug( get_class( $this ) . ': performing DB update; new object.' ); + $this->logger->debug( get_class( $this ) . ': performing DB update; new object.' ); $dbw->insert( static::getTable(), $this->getRowArray( $dbw ), diff --git a/backend/MWOAuthDataStore.php b/backend/MWOAuthDataStore.php index c9498dc..74994c3 100644 --- a/backend/MWOAuthDataStore.php +++ b/backend/MWOAuthDataStore.php @@ -2,11 +2,17 @@ namespace MediaWiki\Extensions\OAuth; +use MediaWiki\Logger\LoggerFactory; +use Psr\Log\LoggerInterface; + class MWOAuthDataStore extends OAuthDataStore { /** @var \DBConnRef DB for the consumer/grant registry */ protected $centralDB; /** @var \BagOStuff Cache for Tokens and Nonces */ protected $cache; + + /** @var \\Psr\\Log\\LoggerInterface */ + protected $logger; /** * @param \DBConnRef $centralDB Central DB slave @@ -15,6 +21,7 @@ public function __construct( \DBConnRef $centralDB, \BagOStuff $cache ) { $this->centralDB = $centralDB; $this->cache = $cache; + $this->logger = LoggerFactory::getLogger( 'OAuth' ); } /** @@ -37,7 +44,7 @@ * @return MWOAuthToken */ public function lookup_token( $consumer, $token_type, $token ) { - wfDebugLog( 'OAuth', __METHOD__ . ": Looking up $token_type token '$token'" ); + $this->logger->debug( __METHOD__ . ": Looking up $token_type token '$token'" ); if ( $token_type === 'request' ) { $returnToken = $this->cache->get( MWOAuthUtils::getCacheKey( @@ -87,7 +94,7 @@ // Set timeout 5 minutes in the future of the timestamp as OAuthServer does. Use the // timestamp so the client can also expire their nonce records after 5 mins. if ( !$this->cache->add( $key, 1, $timestamp + 300 ) ) { - wfDebugLog( 'OAuth', "$key exists, so nonce has been used by this consumer+token" ); + $this->logger->info( "$key exists, so nonce has been used by this consumer+token" ); return true; } return false; @@ -121,7 +128,7 @@ $this->cache->add( $cacheConsumerKey, $consumer->key, 600 ); //10 minutes. Kindof arbitray. $this->cache->add( $cacheTokenKey, $token, 600 ); //10 minutes. Kindof arbitray. $this->cache->add( $cacheCallbackKey, $callback, 600 ); //10 minutes. Kindof arbitray. - wfDebugLog( 'OAuth', __METHOD__ . + $this->logger->debug( __METHOD__ . ": New request token {$token->key} for {$consumer->key} with callback {$callback}" ); return $token; } @@ -170,7 +177,7 @@ * @return MWOAuthToken the access token */ public function new_access_token( $token, $consumer, $verifier = null ) { - wfDebugLog( 'OAuth', __METHOD__ . + $this->logger->debug( __METHOD__ . ": Getting new access token for token {$token->key}, consumer {$consumer->key}" ); if ( !$token->getVerifyCode() || !$token->getAccessKey() ) { @@ -183,7 +190,7 @@ $consumer->get( 'consumerKey' ), 'request', $token->key ); $accessToken = $this->lookup_token( $consumer, 'access', $token->getAccessKey() ); $this->cache->set( $cacheKey, '**USED**' , 600 ); - wfDebugLog( 'OAuth', __METHOD__ . + $this->logger->debug( __METHOD__ . ": New access token {$accessToken->key} for {$consumer->key}" ); return $accessToken; } diff --git a/backend/MWOAuthRequest.php b/backend/MWOAuthRequest.php index 414ad2e..9861ca3 100644 --- a/backend/MWOAuthRequest.php +++ b/backend/MWOAuthRequest.php @@ -54,7 +54,7 @@ ) === 0 ) { $postData = OAuthUtil::parse_parameters( $request->getRawPostString() ); - wfDebugLog( 'OAuth', __METHOD__ . ': Post String = ' . implode( ',', $postData ) ); + $this->logger->debug( __METHOD__ . ': Post String = ' . implode( ',', $postData ) ); $parameters = array_merge( $parameters, $postData ); } @@ -68,7 +68,7 @@ ); $parameters = array_merge($parameters, $headerParameters); } - wfDebugLog( 'OAuth', __METHOD__ . ": parameters:\n" . print_r( $parameters, true) ); + $this->logger->debug( __METHOD__ . ": parameters:\n" . print_r( $parameters, true) ); return new self( $httpMethod, $httpUrl, $parameters, $request->getIP() ); } diff --git a/backend/MWOAuthServer.php b/backend/MWOAuthServer.php index b05dd51..be5b5a5 100644 --- a/backend/MWOAuthServer.php +++ b/backend/MWOAuthServer.php @@ -125,7 +125,7 @@ // Rev A change $verifier = $request->get_parameter( 'oauth_verifier' ); - wfDebugLog( 'OAuth', __METHOD__ . ": verify code is '$verifier'" ); + $this->logger->debug( __METHOD__ . ": verify code is '$verifier'" ); $new_token = $this->data_store->new_access_token( $token, $consumer, $verifier ); return $new_token; @@ -258,7 +258,7 @@ $requestToken->addAccessKey( $accessToken->key ); $this->data_store->updateRequestToken( $requestToken, $consumer ); - wfDebugLog( 'OAuth', "Verification code {$requestToken->getVerifyCode()} for $requestTokenKey (client: $consumerKey)" ); + $this->logger->debug( "Verification code {$requestToken->getVerifyCode()} for $requestTokenKey (client: $consumerKey)" ); return $consumer->generateCallbackUrl( $this->data_store, $requestToken->getVerifyCode(), $requestTokenKey ); } diff --git a/examples/testClient.php b/examples/testClient.php index b949257..5707534 100644 --- a/examples/testClient.php +++ b/examples/testClient.php @@ -30,11 +30,6 @@ * signature and you will get an error. */ -function wfDebugLog( $method, $msg) { - //echo "[$method] $msg\n"; -} - - require __DIR__ . '/../lib/OAuth.php'; /** diff --git a/examples/testClientHeaders.php b/examples/testClientHeaders.php index 3a1d318..344fae0 100644 --- a/examples/testClientHeaders.php +++ b/examples/testClientHeaders.php @@ -8,10 +8,6 @@ * A basic client for overall testing */ -function wfDebugLog( $method, $msg) { - //echo "[$method] $msg\n"; -} - require '../lib/OAuth.php'; @@ -82,7 +78,3 @@ } echo "Returned: $data\n\n"; - - - - diff --git a/frontend/specialpages/SpecialMWOAuth.php b/frontend/specialpages/SpecialMWOAuth.php index 42dcad3..522490a 100644 --- a/frontend/specialpages/SpecialMWOAuth.php +++ b/frontend/specialpages/SpecialMWOAuth.php @@ -21,12 +21,19 @@ http://www.gnu.org/copyleft/gpl.html */ +use MediaWiki\Logger\LoggerFactory; +use Psr\Log\LoggerInterface; + /** * Page that handles OAuth consumer authorization and token exchange */ class SpecialMWOAuth extends \UnlistedSpecialPage { + /** @var \\Psr\\Log\\LoggerInterface */ + protected $logger; + function __construct() { parent::__construct( 'OAuth' ); + $this->logger = LoggerFactory::getLogger( 'OAuth' ); } public function execute( $subpage ) { @@ -47,7 +54,7 @@ case 'initiate': $oauthServer = MWOAuthUtils::newMWOAuthServer(); $oauthRequest = MWOAuthRequest::fromRequest( $request ); - wfDebugLog( 'OAuth', __METHOD__ . ": Consumer " . + $this->logger->debug( __METHOD__ . ": Consumer " . "'{$oauthRequest->getConsumerKey()}' getting temporary credentials" ); // fetch_request_token does the version, freshness, and sig checks $token = $oauthServer->fetch_request_token( $oauthRequest ); @@ -60,7 +67,7 @@ $request->getVal( 'oauth_token' ) ); $consumerKey = $request->getVal( 'consumerKey', $request->getVal( 'oauth_consumer_key' ) ); - wfDebugLog( 'OAuth', __METHOD__ . ": doing '$subpage' with " . + $this->logger->debug( __METHOD__ . ": doing '$subpage' with " . "'$requestToken' '$consumerKey' for '{$user->getName()}'" ); // TODO? Test that $requestToken exists in memcache if ( $user->isAnon() ) { @@ -104,7 +111,7 @@ } $consumerKey = $oauthRequest->get_parameter( 'oauth_consumer_key' ); - wfDebugLog( 'OAuth', "/token: '{$consumerKey}' getting temporary credentials" ); + $this->logger->debug( "/token: '{$consumerKey}' getting temporary credentials" ); $token = $oauthServer->fetch_access_token( $oauthRequest ); if ( $isRsa ) { // RSA doesn't use the token secret, so don't return one. @@ -183,10 +190,10 @@ } } } catch ( MWOAuthException $exception ) { - wfDebugLog( 'OAuth', __METHOD__ . ": Exception " . $exception->getMessage() ); + $this->logger->warning( __METHOD__ . ": Exception " . $exception->getMessage(), array( 'exception' => $exception ) ); $this->showError( wfMessage( $exception->msg, $exception->params ), $format ); } catch ( OAuthException $exception ) { - wfDebugLog( 'OAuth', __METHOD__ . ": Exception " . $exception->getMessage() ); + $this->logger->warning( __METHOD__ . ": Exception " . $exception->getMessage(), array( 'exception' => $exception ) ); $this->showError( wfMessage( 'mwoauth-oauth-exception', $exception->getMessage() ), $format diff --git a/lib/OAuth.php b/lib/OAuth.php index dfb309f..3e86714 100755 --- a/lib/OAuth.php +++ b/lib/OAuth.php @@ -1,7 +1,4 @@ <?php - -namespace MediaWiki\Extensions\OAuth; - // vim: foldmethod=marker /** * The MIT License @@ -26,6 +23,12 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ + +namespace MediaWiki\Extensions\OAuth; + +use MediaWiki\Logger\LoggerFactory; +use Psr\Log\LoggerInterface; + /* Generic exception class */ class OAuthException extends \Exception { @@ -83,6 +86,14 @@ * See section 9 ( "Signing Requests" ) in the spec */ abstract class OAuthSignatureMethod { + + /** @var \\Psr\\Log\\LoggerInterface */ + protected $logger; + + public function __construct() { + $this->logger = LoggerFactory::getLogger( 'OAuth' ); + } + /** * Needs to return the name of the Signature Method ( ie HMAC-SHA1 ) * @return string @@ -110,9 +121,9 @@ * @return bool */ public function check_signature( $request, $consumer, $token, $signature ) { - wfDebugLog( 'OAuth', __METHOD__ . ": Expecting: '$signature'" ); + $this->logger->debug( __METHOD__ . ": Expecting: '$signature'" ); $built = $this->build_signature( $request, $consumer, $token ); - wfDebugLog( 'OAuth', __METHOD__ . ": Built: '$built'" ); + $this->logger->debug( __METHOD__ . ": Expecting: '$signature'" ); // Check for zero length, although unlikely here if ( strlen( $built ) == 0 || strlen( $signature ) == 0 ) { return false; @@ -146,7 +157,7 @@ public function build_signature( $request, $consumer, $token ) { $base_string = $request->get_signature_base_string(); - wfDebugLog( 'OAuth', __METHOD__ . ": Base string: '$base_string'" ); + $this->logger->debug( __METHOD__ . ": Base string: '$base_string'" ); $request->base_string = $base_string; $key_parts = array( @@ -156,7 +167,7 @@ $key_parts = OAuthUtil::urlencode_rfc3986( $key_parts ); $key = implode( '&', $key_parts ); - wfDebugLog( 'OAuth', __METHOD__ . ": HMAC Key: '$key'" ); + $this->logger->debug( __METHOD__ . ": HMAC Key: '$key'" ); return base64_encode( hash_hmac( 'sha1', $base_string, $key, true ) ); } } @@ -270,12 +281,16 @@ public static $version = '1.0'; public static $POST_INPUT = 'php://input'; + /** @var \\Psr\\Log\\LoggerInterface */ + protected $logger; + function __construct( $http_method, $http_url, $parameters = NULL ) { $parameters = ($parameters) ? $parameters : array(); $parameters = array_merge( OAuthUtil::parse_parameters(parse_url($http_url, PHP_URL_QUERY)), $parameters); $this->parameters = $parameters; $this->http_method = $http_method; $this->http_url = $http_url; + $this->logger = LoggerFactory::getLogger( 'OAuth' ); } @@ -400,7 +415,6 @@ * and the concated with &. */ public function get_signature_base_string() { - //wfDebugLog( 'OAuth', __METHOD__ . ": Generating base string when this->paramters:\n" . print_r( $this->parameters, true ) ); $parts = array( $this->get_normalized_http_method(), $this->get_normalized_http_url(), @@ -530,8 +544,12 @@ /** @var OAuthDataStore */ protected $data_store; + /** @var \\Psr\\Log\\LoggerInterface */ + protected $logger; + function __construct( $data_store ) { $this->data_store = $data_store; + $this->logger = LoggerFactory::getLogger( 'OAuth' ); } public function add_signature_method( $signature_method ) { @@ -646,7 +664,7 @@ if ( !$consumer_key ) { throw new OAuthException( "Invalid consumer key" ); } - wfDebugLog( 'OAuth', __METHOD__ . ": getting consumer for '$consumer_key'" ); + $this->logger->debug( __METHOD__ . ": getting consumer for '$consumer_key'" ); $consumer = $this->data_store->lookup_consumer( $consumer_key ); if ( !$consumer ) { throw new OAuthException( "Invalid consumer" ); @@ -698,7 +716,7 @@ ); if ( !$valid_sig ) { - wfDebugLog( 'OAuth', __METHOD__ . ': Signature check (' . get_class( $signature_method ) . ') failed' ); + $this->logger->info( __METHOD__ . ': Signature check (' . get_class( $signature_method ) . ') failed' ); throw new OAuthException( "Invalid signature" ); } } @@ -801,11 +819,12 @@ // May 28th, 2010 - method updated to tjerk.meesters for a speed improvement. // see http://code.google.com/p/oauth/issues/detail?id = 163 public static function split_header( $header, $only_allow_oauth_parameters = true ) { - wfDebugLog( 'OAuth', __METHOD__ . ": pulling headers from '$header'" ); + $logger = LoggerFactory::getLogger( 'OAuth' ); + $logger->debug( __METHOD__ . ": pulling headers from '$header'" ); $params = array(); if ( preg_match_all( '/(' . ( $only_allow_oauth_parameters ? 'oauth_' : '' ) . '[a-z_-]*)=(:?"([^"]*)"|([^,]*))/', $header, $matches ) ) { foreach ( $matches[1] as $i => $h ) { - wfDebugLog( 'OAuth', __METHOD__ . ": '$i' => '$h'" ); + $logger->debug( __METHOD__ . ": '$i' => '$h'" ); $params[$h] = OAuthUtil::urldecode_rfc3986( empty( $matches[3][$i] ) ? $matches[4][$i] : $matches[3][$i] ); } if ( isset( $params['realm'] ) ) { @@ -894,7 +913,8 @@ } public static function build_http_query( $params ) { - wfDebugLog( 'OAuth', __METHOD__ . " called with params:\n" . print_r( $params, true ) ); + LoggerFactory::getLogger( 'OAuth' )->debug( + __METHOD__ . " called with params:\n" . print_r( $params, true ) ); if ( !$params ) return ''; // Urlencode both keys and values @@ -925,5 +945,3 @@ return implode( '&', $pairs ); } } - -?> -- To view, visit https://gerrit.wikimedia.org/r/248488 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I92ffac553b6552b26cb6f531c2d10d7bca777900 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/OAuth Gerrit-Branch: master Gerrit-Owner: BryanDavis <[email protected]> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
