This module is an excellent idea, but unfortunately not currently available for Drupal 6. I have begun work on this, but is currently stuck at the rewriting of cram_auth into cram_form_alter, which I'll have to get a better understanding of, before I dare venture into trying to untangle that.

I've attached what I've done so far, which, apart from updates, also contain a few code style corrections. Comments (to fill 'description' fields) for the schema in cram.install would be good.

CommentFileSizeAuthor
#6 cram.d6.patch9.19 KBFreso
#4 cram.d6.patch9.02 KBFreso
#1 cram.d6.patch6.04 KBFreso
cram.d6.patch8.48 KBFreso

Comments

Freso’s picture

Freso commented
Status: Active » Needs work
StatusFileSize
new cram.d6.patch6.04 KB

And I just realised I had worked on an old version of CRAM. This is pretty much the same stuff, but with a few corrected errors, without code style changes, and against latest HEAD.

I'm also not sure whether to replace $module_path with %module_path on line 53, and then have array('%module_path' => $module_path) as the third argument. This would seem to better follow how things are normally done, but... I don't know. I'm not sure how localisation currently affects $variables in such places.

selmanj’s picture

selmanj commented

Thanks! I really appreciate the help. I have been distracted by other things work-related lately so I haven't been able to do much development relating to this module at all.

I think I'll create a D6 branch tomorrow, so we can apply your patch and start getting a D6 version going. I'd like to support D5 and D6 though at least until a stable release comes out (I still use D5 on most of my sites), so I dont' want to create a branch until I'm pretty sure the feature set is solid and we're just working on ironing out all the bugs.

As for the %module_path usage, where are you talking about using it? With drupal_add_js in cram_form_alter() ?

Freso’s picture

Freso commented

A Drupal 6 branch would be great, so I wouldn't have to worry about Drupal 5 changes outdating my patches. I tried to convert cram_auth yesterday as well, but it was quite tricky (for me).

%module_path would be in watchdog('cram', 'md5.js was not found in %module_path. See the INSTALL.txt file for details.', array('%module_path' => $module_path), WATCHDOG_ERROR);, as opposed to the watchdog('cram', "md5.js was not found in $module_path. See the INSTALL.txt file for details.", array(), WATCHDOG_ERROR); currently in the patch. Note that the array has to be defined either way, as long as you wish to use WATCHDOG_ERROR due to new argument order in Drupal 6's watchdog().

Freso’s picture

Freso commented
Status: Needs work » Needs review
StatusFileSize
new cram.d6.patch9.02 KB

Hep. Thanks to the help of Mr. Joe Selman himself, and a good long look at user.module, I finally cracked the case, and am proud to attach the (probably/hopefully) final D6 porting patch. Works from my testing, but I've most likely not tested every use case. I've also not had a HTTP snoop on the case, so it'd probably be good if someone else could verify that it (still?) sends in non-plaintext.

Anyway, I'd deem it RTBC, but for the sake of properness, I'll keep it at CNR. :)

Freso’s picture

Freso commented
Status: Needs review » Needs work

D'oh. No. This code allows anyone to log in as any user, as long as the given username is correct.

Freso’s picture

Freso commented
Status: Needs work » Needs review
StatusFileSize
new cram.d6.patch9.19 KB

Okay. This patch passes all tests I've done on the other ones, and though there's a slight oddity with expired nonces and the clearing of the password field, I don't think there's much else I can do with regards to actual porting. Now it's more tuning and bug fixing. This patch works (and probably works as-good-as the Drupal 5 version).

selmanj’s picture

selmanj commented

Hmm, the path applies cleanly, but I am unable to login with cram enabled using an out of the box d6 installation... I'm going to review the new API changes to D6 and then see if I can't figure it out. Hopefully it's not something stupid on my end :)

Freso’s picture

Freso commented

Did you remember to copy over md5.js?

selmanj’s picture

selmanj commented

Yeah.

I think I see what the problem is. It looks like it's a bug with my last commit, which is unrelated to the d6 port. It seems to happen when the default is not to use cram. I'll file a separate issue.

selmanj’s picture

selmanj commented
Status: Needs review » Fixed

Branched and committed! Thanks for the patch Freso!

Freso’s picture

Freso commented
Status: Fixed » Reviewed & tested by the community

Oops. You forgot to commit the new file: cram.admin.inc!

Freso’s picture

Freso commented
Status: Reviewed & tested by the community » Fixed

Ehm. Never mind me. My CVS-fu just isn't as strong as I'd like it to be... >_>

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.