122495 - xml-rpc needs authentication

Status: RESOLVED INVALID

(Core :: XML, defect)

Description

[Samuel Sieb]

xml-rpc currently doesn't work well when you need a username and password to
access the server.  The API needs some way to specify the username and password
to be used.

Comment 1

[Samuel Sieb]

Attachment: add function to set username and password

I added a function called "setAuthentication".
I also removed some unused code and fixed several strict warnings.

Comment 2

[Samuel Sieb]

Attachment: new patch

The last patch was not finished.

Comment 3

[Samuel Sieb]

looking for review

Comment 4

[Samuel Sieb]

mj, do you think you could review this?

Comment 5

[Robert Ginda]

Comment on attachment 66997 [details] [diff] [review]
new patch

Looks clean to me, r=rginda

Comment 6

[Darin Fisher]

Comment on attachment 66997 [details] [diff] [review]
new patch

>Index: extensions/xml-rpc/src/nsXmlRpcClient.js

>+    // nsIInterfaceRequester interface
>+    getInterface: function(iid, result){
>+        if (iid.equals(Components.interfaces.nsIAuthPrompt)){
>+              return this;
>+        }

nit: indentation

otherwise sr=darin

Comment 7

[Martijn Pieters]

The code on the whole looks good to me. I do have some questions though:

- How does the user of the component clear the authentication credetials? There 
may be a need for this; there is a need to clear browser authentication in 
general, for example the Zope management interface has to force this on the 
server side as no browser has an UI for it. Having a method to set credentials, 
I'd expect a companion method to clear them too.

- How does the user of the component get notified if the authentication failed?

Comment 8

[Samuel Sieb]

I'm open to suggestions.  I just did this because it needed to be done.  If you
have ideas on a better API, let me know and I'll implement it.

Comment 9

[Darin Fisher]

one solution would be to use the password manager for storing all passwords. 
this would allow users to make use of the password manager UI to delete these.

see nsIPasswordManager.idl in netwerk/base/public

Comment 10

[Darin Fisher]

i should have added: if you have specific question regarding the password
manager, ask morse.

Comment 11

[Martijn Pieters]

Hmm, I think users of the XML-RPC compnent may want to add a layer in between 
the password manager and the component. Just add a clearAuthentication method, 
which sets this._useAuth to false.

As for authentication error problems; just pass back an error message through 
nsIXmlRpcClientListener.onError().

Comment 12

[Samuel Sieb]

I checked in the current patch as some people have immediate need of it. I will
work on adding the other functionality as I have time.

Comment 13

[Martijn Pieters]

It wasn't possible just to check in the fix for removing the newline on POSTs? 

I indeed see that your patch did various clean-ups besides the authentication code; it is generally a good idea to seperate developments like these into seperate checkins.

Comment 14

[Andreas Franke (gone)]

I agree with Martijn Pieters here. Especially the checkin log message "added
method to set authentication for xml-rpc" did not suggest that the checkin fixed
the additional newline problem (bug 124861). But it's nice to see that this code
is actively maintained. :-)

Comment 15

[Andrew Hagen]

The patch has maybe grown stale, but it still has review and super-review. Can
someone do a status update?

Comment 16

[Samuel Sieb]

Comment on attachment 66997 [details] [diff] [review]
new patch

Sorry, see comment 12.	This patch was checked in, but I'm leaving the bug open
in hopes of making it better.

Comment 17

[Samuel Sieb]

Anyone have any comments on whether bug 197087 being fixed makes this one irrelevant?