move GSSAPI code behind nsIAuthModule so it can be used by mail protocols

RESOLVED FIXED in mozilla1.8alpha1

Status

()

Core
Networking
--
enhancement
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: Darin Fisher, Assigned: Darin Fisher)

Tracking

Trunk
mozilla1.8alpha1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

68.26 KB, patch
Christopher Nebergall
: review+
Brian Ryner (not reading)
: superreview+
Details | Diff | Splinter Review
(Assignee)

Description

14 years ago
move GSSAPI code behind nsIAuthModule so it can be used by mail protocols.  this
means changing nsIAuthModule slightly.

e.g.,

we need to add parameters to specify the servicename and probably some flags.
(Assignee)

Updated

14 years ago
Blocks: 237586
(Assignee)

Updated

14 years ago
Severity: normal → enhancement
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8alpha

Comment 1

14 years ago
awesome. Is allowing mailnews to use auth digest orthogonal or related or
parallel...? :-)
(Assignee)

Comment 2

14 years ago
we'd need to do the same sort of thing to support auth digest.  i'd say that it
should be dependent on this bug since the patch for this bug will include
changes to nsIAuthModule that will matter for auth digest.  but, otherwise it is
a task that can be done in parallel.

i have a patch for this bug, which i should be attaching tonight or tomorrow.
(Assignee)

Comment 3

14 years ago
Created attachment 146880 [details] [diff] [review]
v1 patch

This patch refactors the negotiateauth code into two pieces:

  (1) an implementation of nsIAuthModule
  (2) an implementation of nsIHttpAuthenticator

This will allow IMAP, SMTP, etc. to use the nsIAuthModule implementation to
provide SPNEGO support.

The nsIAuthModule impl lives in nsNegotiateAuthGSSAPI.{h,cpp} and the
nsIHttpAuthenticator impl lives in nsHttpNegotiateAuth.{h,cpp}.  I've also
renamed the file containing the "xpcom module factory" code from
nsHttpGssapiAuthModule.cpp to nsNegotiateAuthFactory.cpp.  Finally, I moved the
logging stuff into a new header file, nsNegotiateAuth.h, that is used by the
.cpp files.

this patch changes nsIAuthModule by adding serviceName and serviceFlags
arguments to nsIAuthModule::Init.  these args are ignored by the NTLM impl. 
some flags are defined on nsIAuthModule.  we might define more flags in the
future.

most of the changes in this patch is just moving code around.
(Assignee)

Comment 4

14 years ago
one other comment about this patch.  the whitelist of allowed sites is not part
of the nsIAuthModule implementation.  that list is only applicable to web
browsing, so mailnews won't have to worry about setting up the whitelist to
allow imap:// URLs, etc.  the same is true of the delegation whitelist.  Mail
protocols would need to have their own prefs to control whether or not
delegation is used.  i'm not even sure if delegation is applicable to mail
protocols.
(Assignee)

Comment 5

14 years ago
Comment on attachment 146880 [details] [diff] [review]
v1 patch

cneberg, bryner: can you please review these changes?  see comment #3.
thx!
Attachment #146880 - Flags: superreview?(bryner)
Attachment #146880 - Flags: review?(cneberg)
(Assignee)

Comment 6

14 years ago
some bugs in the patch "as is"

  (1) changes to nsHttpNTLMAuth.cpp are missing.
  (2) extensions/negotiateauth/Makefile.in compares GSSAPI_INCLUDES against the
      empty string, but it should really compare $(GSSAPI_INCLUDES) instead.

i've fixed both of these bugs in my tree.
Attachment #146880 - Flags: superreview?(bryner) → superreview+

Comment 7

14 years ago
I'll have time to look at this tommorrow.

Comment 8

14 years ago
Comment on attachment 146880 [details] [diff] [review]
v1 patch

Looks good.  I tried it out on Linux and it worked fine.
Attachment #146880 - Flags: review?(cneberg) → review+
(Assignee)

Comment 9

14 years ago
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.