7.x update

I don't have a D7 install, so I haven't investigated, and I haven't ported anything to D7, so I don't know what kind of unexpected gotchas there may be.

From a very quick look, it looks like the port should be pretty simple. Update the .info, split apart the hook_user method to make use of hook_user_login() and hook_user_logout() instead. If there's nothing else, there's much more effort in setting up a test sandbox and testing it than in the code changes.

If you're considering this for a production site, I hope you've read through the doc and understand the limitations, e.g. the fact that changes in usernames are not propagated, that wiki users aren't deleted when Drupal users are deleted, etc.

I don't have much to add to this excellent analysis other that I would encourage waiting for the git switchover of drupal.org, as branches are much easier to deal with in git. :)

I can take care of the branching voodoo if people want to work on this, patches are obviously very welcome.

Just for kicks I did a 10 min experiment and the module seems to work with minimal changes:

- change the .info file to say core = 7.x
- split mediawikiauth_user() into:

function mediawikiauth_user_login(&$edit, &$user)
function mediawikiauth_user_logout(&$edit, &$user)

Each gets the code that used to be in the matching switch ($op) condition.

It's possible that changes may be necessary to the store-settings-in-DB code that's been added; I don't run that code yet.

I only did bare-minimum testing, but this got me logged in to the wiki in my setup.

Here's a minimal-change patch against HEAD that works for me. (Minimal testing only. :-)

When committing this, I recommend also fixing up StaticUserLogout() as suggested here so that code is in sync with the MW-side code on mediawiki.org and so logout will work.

(Edited the title to reflect that we're not just talking about the timeline anymore.)

2thinkling:
I'm tried to apply patch to the AuthDrupal 0.7.2 (2010-12-23) version. Patch was not applied correctly, so I'm tried to make it by hands. After passing through installation instruction I tried to login and I've got such notice

* Notice: Undefined index: wgCookiePrefix в функции mediawikiauth_user_login() (строка 100 в файле /home/maestro/data/www/motoradio.com.ua/sites/all/modules/AuthDrupal/mediawikiauth.module).
* Notice: Undefined index: wgCookieSecure в функции mediawikiauth_user_login() (строка 100 в файле /home/maestro/data/www/motoradio.com.ua/sites/all/modules/AuthDrupal/mediawikiauth.module).

And I'm not become logged in to the wiki. Do you use this version, or get somewhere from CVS? May be you can attach to your message already patched full archive?

Thanks in advance!

P.s.: I've tried to use http://ftp.drupal.org/files/projects/mediawikiauth-6.x-0.7.tar.gz as source for the patch, it doesn't aplly patch too.

@CryAngel: As mentioned in my comment, the patch was made against the HEAD version of the code in CVS. This means the latest in-progress code that has not yet been released into a dev or stable version. If you want to look at that version, see these instructions:

http://drupal.org/node/420968/cvs-instructions/HEAD

But as I said also, HEAD code looks out of sync with the mediawiki-side code hosted on mediawiki.org, so I don't think you can get a fully working version that way.

If you are comfortable reading the patch and doing it by hand, I suggest getting the code from mediawiki.org, even for the Drupal side, and making the same changes to that code--until a new version is released here.

subscribe

I have committed your patch to HEAD, please test.

6.x development now on the 6.x-1.x branch.

subscribe

subscribe

i made a patch for the HEAD code, so that the module works in drupal 7
some of the hooks changed in drupal 7 for example hook_perm is now hook_permission.
this patch was tested with the last version of mediawiki 1.17

patch does not apply to master branch (but I'll be happy to test)
and note the following warning from git apply:

mediawikiauth7-1057708.patch:34: trailing whitespace.
mediawikiauth7-1057708.patch:128: space before tab in indent.

To the module maintainer: please add a 7.x branch so bug can be sorted correctly and curious people can look at git master branch.

PS: It currently works in Drupal 7 (not quite respecting hook prototypes though :))

+ we may want to mark #932590: StaticUserLogout sets wong cookieprefix in CVS version as a dup' when this one is fixed.

+ can we have the code indented please ?
+ can we have the variables as a serialized array, it would avoid cluttering {variables} and avoid some lines of code like $wikicount = variable_get('mediawiki_wikicount', 1);

( NB: git "HEAD" is not (always) origin/master )

This part of the patch of Joseph28 is very important :

- function mediawikiauth_user_logout(&$edit, &$user) {
+ function mediawikiauth_user_logout($user) {

without it D7 complains about:
Warning: Parameter 1 to mediawikiauth_user_logout() expected to be a reference,, the hook is not run, the cookies are not deleted and that's why people can't disconnect from mediawiki in the current state of git/master.

this is a new patch to port mediawikiauth to drupal 7.
this time it was made against 6.x branch.
hope we can have a 7.x branch soon.

The git repository is wrongly setup. HEAD is a confusing branch name. "master" would be clearer.
HEAD is one commit ahead of 6.x-1.x : a commit attempting a D7 port (you may have based your work on it)
http://drupalcode.org/project/mediawikiauth.git/commitdiff/26fcb9733a6df...

whatever; the patch seems fine to me. tested and working (and no more db prefix problem as said in #932590: StaticUserLogout sets wong cookieprefix in CVS version)

Notes:
as you're synch'ing AuthDrupalEncode, why not copying the whole file (AuthDrupal/AuthDrupalEncode.php) from the MediaWiki repository

I attach another patch which should apply on top of yours:

• it fixes some whitespaces/comments, some typos ('fieldset' descriptionS), some spellings (but I'm not a native english speaker either)
• removes mediawikiauth_permission()

Because if someone uses this extension he should be able to change the key in mediawiki LocalSettings.php, so he is probably the root. (or is it an edge-case I can't think about ?)

let's hope the maintainer will come back soon so we can get the repository fixed, patches pushed (and code indented) + bugs closed.

Please mark those as needs review when you upload a patch. I'll review the patch and the repo mess shortly.

It would be nice to get this to RTBC before commit, but I won't be a dick about it. :)

The branch layout is OK. I do not understand what you are saying about the "HEAD" branch, this is the current branch layout:

* 5.x-1.x - 5.x port
* 6.x-1.x - 6.x port
* master - development branch, 7.x port

I would gladly rename the master branch to 7.x, but unfortunately, it's not something that can be done on drupal.org, so we'll have to deal with this for now, unless someone wants to work on the 8.x port. :P

Basically, to rename the branch, i would first need to delete the master branch, to avoid confusion, which can't be done:

 ! [remote rejected] master (deletion of the current branch prohibited)

So let's stick to the master branch for the 7.x port.

@drzraf please do not mix whitespace changes with code changes. It makes the patch harder to review and I will not commit a patch that will mix both while not doing much other work.

@joseph28 - your patch seems to be sound, even though it suffers from the same problems (whitespace changes mixed) as drzraf. It also looks like the patch mixes a bunch of cleanups and the actual D7 port, which makes the patch hard to understand.

I have reverted the one commit on the master branch so that it is a clean slate like the 6.x branch. Please reroll the patches with the proper formatting and without extra changes so they can be properly reviewed and tested. Mark the issue as "needs review" when that is done, and "needs review and tested by the community" when testing is completed.

Thank you for your work, and sorry for the delay.

About my above post, it was probably wrote during a git-confusion time.
About 7.x-1.x you may want to simply copy "master" with git checkout -b.

The "7.x update" subject got AFAIK 3 (three) distinct attempts : yours in master, mine by email,
joesph28's patch in this issue (we probably both overlooked the "master" branch which was
not advertised as 7.x anywhere, thus duplicating work).

There's still a lot to do and getting the code to evolve while staying in
sync with AuthDrupal seems hazardous (at least for me).

I temporary forked it to keep both part in a single repository [1]
(starting with code cleanups + the D7 patch + MW-side code) at that point
this branch was tagged 0.7.2.
Then was tagged 0.7.3 after some more cleanups (especially in respect to $GLOBALS)
Then I moved to php-mcrypt which helped removing some configuration variables,
code and complexity altogether.

I also created a (not yet mature) "httpauth" branch which intends to switch
to an HTTP-based solution (probably solving #1080752: Should MW-side code directly access the Drupal DB? on the road) using hook_xmlrpc.
One goal is to bypass the restriction of same-origin setups and/or the need for a .wide-domain
cookie when using cross-subdomains instances without activating third-party cookies.

Both branches may evolve to the point where it is possible to forward credentials
indifferently through the encrypted cookie or a server-side HTTP bridge.

so far, cleaner code, better security and smaller codebase (most of the changes being
on the MW side though), less configuration needed and only one directory for both applications.

feel free to patch/merge

[1] http://gitorious.org/drzraf/drupal-mediawiki/commits/custom

Well, that's pretty cool!

It would be nice to see patches added to both the 6.x and 7.x branch however. And also, the D7 patch on gitorious suffers from similar problems that I outlined in my review above: whitespace are mixed with non-whitespace changes.

I feel that once this is resolved and proper patches are reroll (without whitespace and also with support for 6.x), I can open up maintainership to others here. (Unless people do not feel like maintaining 6.x at all, which would seem to be a bit daring in my opinion.)

So what are the steps we need to do to get out new D6 & D7 releases?

1) Run through Coder to clean out whitespace & formatting
2) Get reviewed with a media wiki implementation to verify that it works as expected
3) Mark patch RTBC

Which version of media wiki is this written against?

Anything else?

Step 0) do not mix functionality and porting patches, so that we can have a consistent implementation in D6 and D7.

Thanks for the summary Mike :)

this a new patch against the master branch.

@joseph28 - does this take care of steps 0 & 1? Thanks!

It seems some functionality and whitespaces changes remain, please refine.

index 904ca2a..7f1238d 100644
--- a/AuthDrupalEncode.php
+++ b/AuthDrupalEncode.phpundefined
@@ -51,9 +51,12 @@ function authdrupal_encode($plain_string)
  * see if you need to fix it there too. (Should really be shared.)
  */
 
-function StaticUserLogout($prefix = null, $path = null, $domain = null) {
-	if (is_null($prefix)) {
-		$prefix = $GLOBALS['wgCookiePrefix'];
+function StaticUserLogout($dbname = null, $dbprefix = null, $path = null, $domain = null, $cookie_secure = null) {
+	if (is_null($dbname)) {
+	    $dbname = $GLOBALS['wgDBname'];
+	}
+	if (is_null($dbprefix)) {
+		$dbprefix = $GLOBALS['wgDBprefix'];
 	}
 	if (is_null($path)) {
 		$path = $GLOBALS['wgCookiePath'];
@@ -61,30 +64,26 @@ function StaticUserLogout($prefix = null, $path = null, $domain = null) {
 	if (is_null($domain)) {
 		$domain = $GLOBALS['wgCookieDomain'];
 	}
+	if (is_null($cookie_secure)) {
+	    $cookie_secure = $GLOBALS['wgCookieSecure'];
+	}
 	// this lifted from wiki/includes/Setup.php which hasn't been included
 	// when we need these
-	if (is_null($prefix)) {
-	  if ( $GLOBALS['wgDBprefix'] ) {
-	    $prefix = $GLOBALS['wgDBname'] . '_' . $GLOBALS['wgDBprefix'];
-	  }
-	  elseif ( $GLOBALS['wgSharedDB'] ) {
-	    // This is not supported yet--haven't researched it--Maarten.
-	    // XXX should throw an error into watchdog log?
-	    $prefix = $GLOBALS['wgSharedDB'];
-	  } else {
-	    $prefix = $GLOBALS['wgDBname'];
-	  }
+	if ( ! empty($dbprefix) ) {
+		$prefix = $dbname . '_' . $dbprefix;
+	} else {
+		$prefix = $dbname;

non-porting change. unclear what it's for.

+++ b/AuthDrupalEncode.phpundefined
@@ -61,30 +64,26 @@ function StaticUserLogout($prefix = null, $path = null, $domain = null) {
-	setcookie( $prefix . '_session', '', time() - 3600, $path, $domain, $GLOBALS['wgCookieSecure'] );
+	setcookie( $prefix . '_session', '', time() - 3600, $path, $domain, $cookie_secure );
 
-	setcookie( $prefix . 'UserName', '', time() - 3600, $path, $domain, $GLOBALS['wgCookieSecure'] );
-	setcookie( $prefix . 'UserID', '', time() - 3600, $path, $domain, $GLOBALS['wgCookieSecure'] );
-	setcookie( $prefix . 'Token', '', time() - 3600, $path, $domain, $GLOBALS['wgCookieSecure'] );
+	setcookie( $prefix . 'UserName', '', time() - 3600, $path, $domain, $cookie_secure );
+	setcookie( $prefix . 'UserID', '', time() - 3600, $path, $domain, $cookie_secure );
+	setcookie( $prefix . 'Token', '', time() - 3600, $path, $domain, $cookie_secure );

whitespace change, but also non-porting change.

ok i removed these changes. the only changes are the one required for the module to work with drupal 7

Thank you! Great work!

The patch looks sound now! Can anybody test this? Mike?

Works for me.

patch committed thanks and sorry for the delay. will release this shortly.