Item5014: EditTablePlugin complains about editing conflict when adding row

If I say edit anyway or try again I just end up in normal viewing mode.

The error message in the oops page says "c12179 is editing RobinLittleJohn3Costing. "

But my WikiName is KennethLavrsen. The c12179 is my login name.

It seems that we have maybe two bugs.

1. First I should never have an issue with having a conflict with myself. This could be a conflict between the c12179 and my wikiname. Normally a conflict situation warns using a WikiName.

2. And if I try again or edit anyway I should not end up in view mode.

I am using ApacheLogin when I test and I use TWikiUserMapping with login name different from WikiName.

Release blocker

-- TWiki:Main/KennethLavrsen - 23 Nov 2007

Update.

It is $TWiki::cfg{PasswordManager} = 'none'; that triggers the issue.

This is the typical SSO config that enterprises use so it needs to get fixed before we release. In my view before a release candidate is released. Our main TWiki Mission target group - the Enterprise - cannot use TWiki with this bug.

This is most likely a core bug and not an EditTablePlugin bug. And I fear other features will suffer from this bug.

-- TWiki:Main.KennethLavrsen - 23 Nov 2007

VERY EXCELLENT JOB you have indeed found 2 seperate bugs in one fell swoop. I'm looking into why the leases are confused, but the other will need attention too - from my oops html

<form action="/cgi-bin/TWikiRelease04x02/bin/view/Sandbox/EditTablePlugin" method="post">
To edit the topic anyway, click  

mmm, the oops is harder - it is using the view script, because editing / adding a row is a view operation, and because the plugin is written to use #edittable1 I don't know how this plugin would work in this instance.

The lease bug in TWiki::Func is fixed, and the EditTablePlugin issue (that can hopefully be solved by some fancy footwork in the oopsconflict.tmpl s) has been moved to Item5019

-- SvenDowideit - 24 Nov 2007

Does anything need to change to EditTablePlugin?

-- TWiki:Main.ArthurClemens - 24 Nov 2007

crepe, that fix is quite wrong, reverting, and looking further into whats up.

Arthur,there is stuff that you probly know more about in Item5019, this one has more to do with the usermapping code.

-- SvenDowideit - 24 Nov 2007

I am beginning to analyse this bug and look at the code.

But then while I test I notice another strange thing. In the normal edit table lease lock situation where someone else is editing and I hit the Edit Table button then I cam told that c12179 (the login ID) is editing in the oops. When I hit the normal Edit button to edit the entire topic then I am told that KennethLavrsen is editing the topic. And this is no matter if $TWiki::cfg{PasswordManager} is set to 'none' or to 'TWiki::Users::HtPasswdUser'. That is a bug and probably related.

In 4.1.2 when I do the same I am told that KennethLavrsen is already editing both when I am hitting the Edit Table button and when I hit the topic edit.

And surely the way it worked in 4.1.2 is the correct. Whenever a login ID can be mapped to a WikiName then it is the friendly WikiName that should be presented to the user.

The checking for edit lock works fine with normal topic editing. It is when a plugin does the checking it goes wrong.

It is not only when you want to add a row we have the problem.

If a user hits the Edit table button - goes back instead of cancelling leaving the lock - and then hit Edit again - then he is told that he himself is already editing the table. This also makes sense that the error is like this because it is the same code that checks for locks when you hit edit and when you want to add a row.

When you want to add a row you always have a lease lock. When you do normal table edit then it is only if you have backed out before and then come back.

-- TWiki:Main.KennethLavrsen - 28 Nov 2007

I have traced this far.

The key problem happens in EditTablePlugin/Core.pm line 1099

            my $loginUser = TWiki::Func::wikiToUserName($wikiUserName);
            if ( $lockUser ne $loginUser ) {

The $lockUser is correctly the login ID of the person that last edited.

But the $loginUser differ.

With $TWiki::cfg{PasswordManager} = 'TWiki::Users::HtPasswdUser' the TWiki::Func::wikiToUserName($wikiUserName) correctly returns the username.

With $TWiki::cfg{PasswordManager} = 'none' the TWiki::Func::wikiToUserName($wikiUserName) returns the wikiname which is wrong.

The spec for TWiki::Func::wikiToUserName says: "Translate a Wiki name to a login name based on TWikiUsers topic" and this does not happen when $TWiki::cfg{PasswordManager} = 'none' which is very odd.

-- TWiki:Main.KennethLavrsen - 29 Nov 2007

I am adding my findings step by step. If nothing else it is useful to keep my own thoughts straight.

In Func.pm line 732

my $cUID = $users->getCanonicalUserID($wiki);

is where I have traced the problem to so far. getCanonicalUserID($wiki) is where the wrong value comes from.

-- TWiki:Main.KennethLavrsen - 29 Nov 2007

I have reached as far as Users.pm lines 348 and forward. I can see that inside getCanonicalUserID - when called from Func wikiToUserName - there is a difference what happens on the line

my $testMapping = $this->_getMapping($login, $login, undef, 1);

It seems that in both bases we get the correct TWikiUserMapping but the

return forceCUID($login) if ($testMapping && $testMapping->getLoginName($login));

I have reached as far as Users.pm lines 348 and forward. I can see that inside getCanonicalUserID - when called from Func wikiToUserName where we reach the line that says

my $testMapping = $this->_getMapping($login, $login, undef, 1);

It seems that in both bases we get the correct TWikiUserMapping but on the next statement there is again a difference that depends on PasswordManager.

return forceCUID($login) if ($testMapping && $testMapping->getLoginName($login));

the $testMapping->getLoginName($login) returns nothing when PasswordManager is 'TWiki::Users::HtPasswdUser' and it returns the WikiName when it is 'none'

This is as far as I had time to debug. Spent 5 hours on it until now. This code is extremely complex. At least for me. And I have to change config each time between runs.

-- TWiki:Main.KennethLavrsen - 29 Nov 2007

Getting close but also getting into an area where I do not understand why the code is made as it is.

Following the track $testMapping->getLoginName leads us to Users/TWikiUserMapping round lines 187 and forward.

This function is supposed to return the login name but in reality it returns the WikiName.

But it depends on the line

return unless (($dontcheck) || defined($this->{L2U}->{$user}) || (defined($this->{passwords}->fetchPass( $user ))) );

Why is this function not returning anything based on defined($this->{passwords}->fetchPass( $user )? This is not documented in the code. And it does not make sense to me.

Dinner time. Will continue. Would be nice if the author of this code would explain the logic.

-- TWiki:Main.KennethLavrsen - 29 Nov 2007

Looking further at the code flow.

Users/TWikiUserMapping function getLoginName is called many times and successfully both when we have PasswordManager set for TWiki::Users::HtPasswdUser and when it is none.

When it is none the $this->{passwords} points to TWiki::Users::Password.

There is a major difference between how this function is normally called and how it is called when the scope is originally a call from TWiki::Func::wikiToUserName.

Normally the function is called with a login ID and it returns a login ID with some modification if the login contains special characters.

But from Func the getLoginName is called with a WikiName as input.

So it is when this function returns the WikiName things go wrong. When both defined($this->{L2U}->{$user}) and defined($this->{passwords}->fetchPass( $user )) are false the function returns nothing and then the Users function getCanonicalUserID returns the forceCUID of the WikiName and that is wrong.

Because I do not understand the logic yet I do not dare fixing anything yet. It would help if there had been some comments to explain the logic because it is not obvious.

-- TWiki:Main.KennethLavrsen - 29 Nov 2007

Going to bed now.

I did one small test

I changed TWiki:Func:wikiToUserName to

sub wikiToUserName {
    my( $wiki ) = @_;
    ASSERT($TWiki::Plugins::SESSION) if DEBUG;
    return '' unless $wiki;
    my $users = $TWiki::Plugins::SESSION->{users};
    
    my $usernames = $users->findUserByWikiName($wiki);
    return undef unless $usernames && scalar(@$usernames);
    return $usernames->[0];

and then I can suddenly do everything with EditTablePlugin.

But I guess the new extended spec for TWiki:Func:wikiToUserName is not met then. Ie. that it can also look up a cUID or login. Actually I did not try. Too late for me. But it shows that the plugin can be made to work and that I have found the area where the bug is.

Still hope to get some hint. Or better to that my analysis helped someone else with better perl skills than I to fix it.

-- TWiki:Main.KennethLavrsen - 30 Nov 2007

Yep, thats pretty much the same analysis I did last Sunday. Then, and when I've looked into it since make me worry that we could plaster over the root cause by patching TWiki::Func like I did in my initial very wrong fix, or in a similar way to what you have above.

And thats why I'm not rushing this, even though, like the SSO bug that tooke a few weeks to work though, I expect it will end up being a very simple code change, I'm simply tracing through all the code paths, verifying that they make sense, and so on.

-- TWiki:Main.SvenDowideit - 30 Nov 2007

wow, what a bloody saga - and the main part of the fix.... change Password::fecthPass() to return undef - like the docco says.

The commit to fix this makes Item4771 a little worse, but I've much more code qued up to fix that.

-- TWiki:Main.SvenDowideit - 06 Dec 2007

one bug i noticed post the latest Item4771 commits - if the authenticated, but non-registered user uses EditTablePlugin, it doesn't like it

FIXED that too, I think we're good to go.

-- TWiki:Main.SvenDowideit - 06 Dec 2007