googlegsa / manager.v3

Google Search Appliance Connector Manager
Apache License 2.0
10 stars 10 forks source link

"Domain" gets removed when CM creates AuthenticationIdentity for the Connector #131

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
When the user tries to search (secured) for a keyword using GSA search
interface and enters user name as "domain\username", the connector receives
only the username as part of authentication identity (the domain is
dropped) due to which the connector authentication fails for NTLM as the
domain name is required.

On investigation we found out that in CM code:

Class: com.google.enterprise.connector.servlet.Authenticate

Method: handleDoPost()

We get the "xmlbody" without domain\username

e.g.<AuthnRequest><Credentials><Username>administrator</Username><Password>dummy
-password</Password></Credentials></AuthnRequest>

Original-username: ps4312\administrator

--

Marty Gronberg writes:

The Authenticate Servlet is in the SVN code base. The Authenticate servlet
simply pulls out the XML data sent by the GSA.

Amit is observing that it the XML created by the GSA (i.e. the AuthN part
of the EFE), the <Username> element is missing the domain data. I can tell
you it's not stripped by the frontend.ConnectorAuthnQuerier class or the
servlet.Authenticate Servlet so it must be stripped somewhere in the Authn
process before the query is made.

Is that expected behavior for the GSA or is it a bug? If it's not unique to
Connectors then is it a general AuthN behavior on the EFE?

Is there anything they can do to preserve the domain as part of the
Username for AuthN?

It's causing a problem with the Authn for SharePoint because that Connector
needs the request coming from the GSA to contain the domain name as part of
the <Username>.

--

John Lacey writes: 

I ran into this six months ago with the Livelink connector. It was
definitely the GSA stripping the domain, because I looked at the HTTP
traffic between the GSA and the connector manager. What I ended up doing
for the Livelink connector was adding a windowsDomain property to
connectorInstance.xml, so that customers who had a single domain could at
least authenticate.

Curiously, DNS-style domain names did not get stripped, so the username
comes to the connector as "username@domain" in that case. I specifically
handle that, and use the DNS-style domain if one is present, and fall back
to the windowsDomain property otherwise. This was pre-5.0.4, so I don't
know if anything changed in 5.0.4.

If we modify the GSA and the connector manager, it seems reasonable to add
a getDomain method to the AuthenticationIdentity class, or possibly create
a subclass. Of course, I don't know the timeline for the SAML rewrite, so
that may affect how we handle this in the connector manager. I'd rather not
be paralyzed by perfection, however.

--

Amit Kagrawal writes:

This is currently the show stopper for the connector. Even by using
approach suggested by John, there will be a problem when no domain is
provided during configuring connector instance and user has entered the
username as "domain\user" during search time. In this case the connector
will NOT be able to do authorization and authentication.

Also, since the domain used by connector instance is for crawling the
SharePoint documents, it can be different from thedomain used for search.
E.g. one can used site collection admin account for crawling to cover all
the contents in a site collection and a less privileged user during search.
So, semantically these domains are separate.

--

Ken Stillson writes:

Apologies, I've been really swamped on other things, and technically this
is no longer my area - this sort of thing has been transferred to the
gsa-sec-features-eng team (con, voyager, cph, ziff), however, I think I
know the source of the issue.

//depot/google3/java/com/google/enterprise/frontend/AuthNChallengeBasicAuth.java
:
validateResponse() is where the basic auth response is parsed and stored. 
There's a bit of logic in here for specifcally handling the
"domain\username" form.

Basically there are all sorts of bits of the GSA that won't work if the
central user-name field is left in "domain\username" form.  For example,
LDAP verification will always fail, as just the "username" form is expected.

Therefore, this method separates the domain and username compoents, and
stores them into different parts of the session manager.  The domain
portion is stored directly into the SM key
AuthNConstants.AUTHN_MECH_BASIC_AUTH_USER_DOMAIN_KEY (line 77).   The
username portion is encoded into the AuthNResult instance (line 93), which
is returned by validateResponse and will eventually make its way to
AuthN.java:processAuthMechanismResults() where line 1025 stores it into the
key AuthN-Mech-BASIC_AUTH_Id.

Later on,  AuthN.java:establishAuthentication() on line 618 will call
runConnectorAuthMechanism(basicAuthStatus).  In
AuthN.java:runConnectorAuthMechanism(), line 1119, the userIdentity field
is pulled out of the basicAuthStatus all by itself - no domain component is
processed.

I don't think we can modify the AuthN-Mech-BASIC_AUTH_Id key.  Too much
depends on that, and changing it will cause chaos.  If this is to be fixed,
I think we would have to add logic to runConnectorAuthMechanism to
reconstruct the domain-name form if the
AUTHN_MECH_BASIC_AUTH_USER_DOMAIN_KEY is set in the SM.  (This solution
also requires passing the sessionId to runConnectorAuthMechanism(), because
currently it doesn't know it, it only gets the AuthNResults structure,
which isn't sufficient information.

Note that this is effectively a contract change between EFE and the
connectors.  That is, while this may make the connector you're currently
working with work, it may also break all the other connectors.  I'm not
saying it will - I have no idea, but it is a contract change in the
semantics of the username field passed as param 1 to the
ConnectorAuthnQuerier.query() method.

Given this road-map of what would need to be changed to fix this - do we
want to do it?  and whose the right folks to do it?  I know both the
sec-features team and my team are working very hard on PRD requirements for
6.0.  A context switch will be expensive for either team.

--

Marty Gronberg writes:

It's probably true of any group tagged to help with this that a context
switch would be expensive. It seems the gsa-sec-features team is in the
best position to take this since it's their charter and they can determine
how it would affect the SAML work.  If anything needs to be done on the
Connector Manager side to handle this contract change I can help with that.

Seems we can do this without a contract change to username by extending to
the XML message to the CM to include a <domain> element rather than change
the value of the username.  A similar extension, as John talked about
above, would have to be done to the AuthenticationIdentity class in the CM.
 That way, those Connectors that don't care won't be bothered and those
that do can extract the domain and create the proper username.

--

John Lacey writes:

There are two things I'd like to see us get right.

1. We saw that adding methods to AuthenticationIdentity is actively
annoying, since connectors need mock implementations for testing. Even
though we originally talked about adding methods to SPI interfaces that
implemented by the Connector Manager as being safe, they are really
incompatible changes as far the connector implementations are concerned.
Choices seem to be:

A. Domains are core concepts, so let's change the interface and break the
connectors.
B. Add a concrete implementation to the SPI (SimpleAuthenticationIdentity?).
C. Add the getDomain method to a new subinterface of AuthenticationIdentity.

Note that B also breaks the connectors, with the idea that future changes
like this wouldn't, since the connectors could switch to the concrete class
instead of using a mock object in the tests.

2. I'd like to make sure the GSA handles DNS-style domains (user@domain) in
a consistent fashion, so that getUsername only returns "user" and getDomain
returns "domain" in that case, too. I do not want getUsername to return
"user@domain", that just seems broken and untidy.

I'm also curious, do we just mean Windows domains here, or could it be
interpreted more broadly? If the latter, how could a connector distinguish
among multiple alternatives?

--

Max Ziff writes:

FYI, the identity situation will become more complicated when the Security
Manager SPI merges with the Connector Manager SPI.  At present, the SM
needs a much richer form of identity that includes cookies (and may later
include kerberos tickets, etc.)  It is currently found in
c.g.e.connector.spi package, but only in the SM's code tree:
SecAuthnIdentity.  It is implemented as a extended interface, for the same
reason you suggested: so that it won't break the build of the CM or any
existing connectors.

I think your observation that "Domains are core concepts" is good, and I
like your approach B for the current situation.  In more detail, here's
what I propose:
Add getDomain() to AuthenticationIdentity
Add SimpleAuthenticationIdentity to the SPI -- using a clone of
UserPassIdentity,  plus a domain field
In the future, when the SM's SecAuthIdentity has matured, make it part of
the SM spi as is -- that is, as an extended interface.  At that time, add a
SimpleSecAuthIdentity as well.
How does that sound?

As for your second question, I agree about getting dns-style domains
through the same accessor.  I'm not sure about how a connector would tell
the difference between a dns-style domain and a windows domain (or kerberos
domain?).  How about a domain-type enumeration?  In addition to getDomain()
we could add getDomainType() that returns "DNS", "WINDOWS", "KERBEROS",
"UNKNOWN", and perhaps others.

Original issue reported on code.google.com by Brett.Mi...@gmail.com on 16 Mar 2009 at 7:09

GoogleCodeExporter commented 9 years ago
Max Ziff writes:

Description:
The main thrust of this change is to add domain to the
AuthenticationIdentity SPI interface, and to add plumbing so that if a
Domain is specified in the authenticate servlet call, then it gets sent
down to the Connector.

The GSA is now supplying domain (as of cl 10484431).  I first verified
that this will not bother an older Connector Manager that doesn't know
about domain.  It just sleepily ignores the Domain element in the xml.

This CL is a candidate for down-integration into 1.3.x

Description:
The main thrust of this change is to add domain to the
AuthenticationIdentity SPI interface, and to add plumbing so that if a
Domain is specified in the authenticate servlet call, then it gets sent
down to the Connector.

My apologies for using Rietveld instead of gvn -- my gvn install is
broken and I need to get this change in soon.  Thanks for working with
me.

The GSA is now supplying domain (as of cl 10484431).  I first verified
that this will not bother an older Connector Manager that doesn't know
about domain.  It just sleepily ignores the Domain element in the xml.

This CL is a candidate for down-integration into 1.3.x

Changes committed to the trunk as revision r1591:

Author: max ziff 
Date: Mon Mar 16 10:47:59 2009 
New Revision: 1591 

Added: 

trunk/projects/connector-manager/source/java/com/google/enterprise/connecto
r/spi/SimpleAuthenticationIdentity.java 

trunk/projects/connector-manager/source/javatests/com/google/enterprise/con
nector/spi/SimpleAuthenticationIdentityTest.java

Removed: 

trunk/projects/connector-manager/source/java/com/google/enterprise/connecto
r/manager/UserPassIdentity.java 

trunk/projects/connector-manager/source/javatests/com/google/enterprise/con
nector/manager/UserPassIdentityTest.java 

Modified: 

trunk/projects/connector-manager/source/java/com/google/enterprise/connecto
r/manager/Manager.java 

trunk/projects/connector-manager/source/java/com/google/enterprise/connecto
r/manager/ProductionManager.java 
trunk/projects/connector-manager/source/java/com/google/enterprise/connecto
r/servlet/Authenticate.java 

trunk/projects/connector-manager/source/java/com/google/enterprise/connecto
r/servlet/ServletUtil.java 
trunk/projects/connector-manager/source/java/com/google/enterprise/connecto
r/spi/AuthenticationIdentity.java 

trunk/projects/connector-manager/source/javatests/com/google/enterprise/con
nector/jcr/JcrAuthenticationManagerTest.java 
trunk/projects/connector-manager/source/javatests/com/google/enterprise/con
nector/jcr/JcrAuthorizationManagerTest.java 

trunk/projects/connector-manager/source/javatests/com/google/enterprise/con
nector/jcr/JcrConnector.java 
trunk/projects/connector-manager/source/javatests/com/google/enterprise/con
nector/manager/MockManager.java 

trunk/projects/connector-manager/source/javatests/com/google/enterprise/con
nector/pusher/DocPusherTest.java 
trunk/projects/connector-manager/source/javatests/com/google/enterprise/con
nector/servlet/AuthenticateTest.java 

Log: http://codereview.appspot.com/28043 

Original comment by Brett.Mi...@gmail.com on 16 Mar 2009 at 7:16

GoogleCodeExporter commented 9 years ago
Committed to the 1.3.x change branch as revision r1592

Original comment by Brett.Mi...@gmail.com on 16 Mar 2009 at 7:24

GoogleCodeExporter commented 9 years ago
See also:

Livelink Connector Issue 17:
http://code.google.com/p/google-enterprise-connector-otex/issues/detail?id=17

Documentum Connector Issue 34:
http://code.google.com/p/google-enterprise-connector-dctm/issues/detail?id=34

Sharepoint Connector Issues 56, 58, 44:
http://code.google.com/p/google-enterprise-connector-sharepoint/issues/list?can=
1&q=domain

Original comment by Brett.Mi...@gmail.com on 16 Mar 2009 at 9:26

GoogleCodeExporter commented 9 years ago

Original comment by Brett.Mi...@gmail.com on 17 Mar 2009 at 10:31

GoogleCodeExporter commented 9 years ago

Original comment by mgron...@gmail.com on 29 Apr 2009 at 9:01

GoogleCodeExporter commented 9 years ago

Original comment by mgron...@gmail.com on 29 Apr 2009 at 11:24

GoogleCodeExporter commented 9 years ago

Original comment by Brett.Mi...@gmail.com on 16 May 2009 at 9:22