handle the case where a provider doees not provide a Full Name

[dkg]

Without this patch, when an OpenID provider declines to provide a full
name, trac crashes by accessing the authname variable before it was
initialized.

If no full name is provided, fall back to the OpenID URL as the user's
identity.

[dairiki]

Thanks for the report and fix.

I've made a couple of comments/questions inline with your patch (see above.) Do they seem sane?

[dkg]

if remote_user is somehow not a string or evaluates to false, i think you'll want those checks.

[dkg]

hm, maybe you can ditch the first check and use:

if type(b) == str and self.lowercase_authname:

instead (as the lower check)

[dairiki]

My concern is that authname is the trac “username”. It is the key by which the trac “session” (to which all trac permissions and what-not are tied) is retrieved. I think (though I'm not quite positive) that if authname is not a string, trac (e.g. the DetachedSession constructor) will be unhappy.

If a check for an empty or non-string remote_user is necessary, I think it has to go earlier, before the if allowed:. (Or maybe the authname deduction logic needs to be moved to before the if allowed:, and the
if allowed: changed to if allowed and authname:?)

[dkg]

I don't think there's any sense in computing the authname if allowed is false, so i wouldn't move it outside of the "if allowed" check. And i agree that the plugin should do something more like bailing out cleanly if authname ends up not being a string.

[dairiki]

Okay, I think I’ve cleaned up the authname logic in 1c77fd3. Please take a look and see whether you like it.

If you have no major complaints I'll make a PyPI release.

[dkg]

On 05/21/2013 07:28 PM, dairiki wrote:

Okay, I think I’ve cleaned up the authname logic in 1c77fd3. Please take a look and see whether you like it.

thanks! I've just tested and it seems to work for me.

If you have no major complaints I'll make a PyPI release.

that would be great, thanks! please let me know when you've done that
and i'll try to get it into debian as well.

Regards,

--dkg

[dairiki]

Done. This is on PyPI as TracAuthOpenId==0.4.3.

Thanks for the kick to get this done!