New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AuthenticationFilter.getUser returns null for container based authentication #364

Closed
gitblit opened this Issue Aug 12, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@gitblit
Owner

gitblit commented Aug 12, 2015

Originally reported on Google Code with ID 68

What steps will reproduce the problem?
1. We run gitblit on our on-demand java platform 
2. The platform does a SAML2-authentication for us
3. We added an additional WicketFilter which does application-specific user authorization
4. zip-download does not work anymore

What is the expected output? What do you see instead?
The user is null when our filter runs after the DownloadZipFilter. The reason is that
the DownloadZipFilter wraps the request with a new one with a user obtained by the
method getUser which only works with BASIC authorization.

What version of the product are you using? On what operating system?
0.7.0

Please provide any additional information below.


Reported by stefanlay1 on 2012-02-22 16:01:39

@gitblit

This comment has been minimized.

Show comment
Hide comment
@gitblit

gitblit Aug 12, 2015

Owner
Zip, Git, RSS, RPC, and Pages all depend on AuthenticationFilter, so in your setup none
of them will work properly.  :(
AuthenticationFilter and those servlets have nothing to do with Wicket which is why
your WicketFilter does not work here.

Matthias and I discussed your SAML2 architecture a while ago.  In re-reading his message
it looks like there is a container-supplied UserPrincipal in the request which perhaps
can be directly checked/used and then mapped to a UserModel.  For now, maybe inject
something like this in AuthenticationFilter.getUser()...

protected UserModel getUser(HttpServletRequest httpRequest) {
    UserModel user = null;
    // look for a container-supplied user principal
    if (httpRequest.getUserPrincipal() != null) {
        Principal principal = httpRequest.getUserPrincipal();
        String name = principal.getName();
        if (!StringUtils.isEmpty(name)) {
            user = GitBlit.self().getUserModel(name);
            if (user == null) {
                // maybe create user? or at least instantiate a temporary UserModel?
            } else {
                return user;
            }
        }
    }

From my exchange with Matthias:
"We use SAML2 [1] to get single-sign-on as we have a couple more systems used by developers
which all require authentication. 
In order to prevent the need to manually login to all these different systems we use
the client certificate based single-sign-on
solution which uses our SAML2 based identity management server on the server side which
is provided by our IT department. 

This is transparent to the application, it requires to add some security constraints
to the application's web.xml to instruct
the web container that it should care for authentication. Requests which can't be authenticated
by the container will never
reach the application. Requests which have successfully authenticated reach the application
which gets the authenticated
user by calling HttpServletRequest.html.getUserPrincipal(). Gerrit allows to configure
which HTTP header will have the
user principal name, this allows integration with application servers which do not
strictly follow the servlet spec."

Reported by James.Moger on 2012-02-22 17:19:23

Owner

gitblit commented Aug 12, 2015

Zip, Git, RSS, RPC, and Pages all depend on AuthenticationFilter, so in your setup none
of them will work properly.  :(
AuthenticationFilter and those servlets have nothing to do with Wicket which is why
your WicketFilter does not work here.

Matthias and I discussed your SAML2 architecture a while ago.  In re-reading his message
it looks like there is a container-supplied UserPrincipal in the request which perhaps
can be directly checked/used and then mapped to a UserModel.  For now, maybe inject
something like this in AuthenticationFilter.getUser()...

protected UserModel getUser(HttpServletRequest httpRequest) {
    UserModel user = null;
    // look for a container-supplied user principal
    if (httpRequest.getUserPrincipal() != null) {
        Principal principal = httpRequest.getUserPrincipal();
        String name = principal.getName();
        if (!StringUtils.isEmpty(name)) {
            user = GitBlit.self().getUserModel(name);
            if (user == null) {
                // maybe create user? or at least instantiate a temporary UserModel?
            } else {
                return user;
            }
        }
    }

From my exchange with Matthias:
"We use SAML2 [1] to get single-sign-on as we have a couple more systems used by developers
which all require authentication. 
In order to prevent the need to manually login to all these different systems we use
the client certificate based single-sign-on
solution which uses our SAML2 based identity management server on the server side which
is provided by our IT department. 

This is transparent to the application, it requires to add some security constraints
to the application's web.xml to instruct
the web container that it should care for authentication. Requests which can't be authenticated
by the container will never
reach the application. Requests which have successfully authenticated reach the application
which gets the authenticated
user by calling HttpServletRequest.html.getUserPrincipal(). Gerrit allows to configure
which HTTP header will have the
user principal name, this allows integration with application servers which do not
strictly follow the servlet spec."

Reported by James.Moger on 2012-02-22 17:19:23

@gitblit

This comment has been minimized.

Show comment
Hide comment
@gitblit

gitblit Aug 12, 2015

Owner
Hi James,

thanks for your quick response. 

We found the following (easy) workaround: We ensure that our own Wicket-Filter is the
first one in the queue. Then it runs before any other Filter inheriting from AuthenticationFilter
wraps the request. We do it by putting the declaration of our Filter before all others
in the web.xml. I'm however not sure if we can rely on the order. But currently it
works.

Yes, we can get the user from the request with httpRequest.getUserPrincipal().

When I have time I can contribute something like you proposed. Do you think it would
be valuable also for other?

Stefan

Reported by stefanlay1 on 2012-02-23 07:55:34

Owner

gitblit commented Aug 12, 2015

Hi James,

thanks for your quick response. 

We found the following (easy) workaround: We ensure that our own Wicket-Filter is the
first one in the queue. Then it runs before any other Filter inheriting from AuthenticationFilter
wraps the request. We do it by putting the declaration of our Filter before all others
in the web.xml. I'm however not sure if we can rely on the order. But currently it
works.

Yes, we can get the user from the request with httpRequest.getUserPrincipal().

When I have time I can contribute something like you proposed. Do you think it would
be valuable also for other?

Stefan

Reported by stefanlay1 on 2012-02-23 07:55:34

@gitblit

This comment has been minimized.

Show comment
Hide comment
@gitblit

gitblit Aug 12, 2015

Owner
Good fix.  I'm 99% sure that order is preserved on servlet filters for precisely your
use-case.

Don't worry about contributing the above code, I will take care of that and test it
with a Tomcat realm or something like that.  I would be interested in your SAML2 WicketFilter,
if that is not too site-specific.

Reported by James.Moger on 2012-02-23 14:01:39

Owner

gitblit commented Aug 12, 2015

Good fix.  I'm 99% sure that order is preserved on servlet filters for precisely your
use-case.

Don't worry about contributing the above code, I will take care of that and test it
with a Tomcat realm or something like that.  I would be interested in your SAML2 WicketFilter,
if that is not too site-specific.

Reported by James.Moger on 2012-02-23 14:01:39

@gitblit

This comment has been minimized.

Show comment
Hide comment
@gitblit

gitblit Aug 12, 2015

Owner
Master now supports authentication for Wicket and the servlets via the servlet container
principal, if available.

Reported by James.Moger on 2012-12-03 22:24:08

  • Status changed: Queued
  • Labels added: Milestone-1.2.0
Owner

gitblit commented Aug 12, 2015

Master now supports authentication for Wicket and the servlets via the servlet container
principal, if available.

Reported by James.Moger on 2012-12-03 22:24:08

  • Status changed: Queued
  • Labels added: Milestone-1.2.0
@gitblit

This comment has been minimized.

Show comment
Hide comment
@gitblit

gitblit Aug 12, 2015

Owner
v1.2.0 has been deployed.

Reported by James.Moger on 2013-01-01 01:06:25

  • Status changed: Fixed
Owner

gitblit commented Aug 12, 2015

v1.2.0 has been deployed.

Reported by James.Moger on 2013-01-01 01:06:25

  • Status changed: Fixed

@gitblit gitblit closed this Aug 12, 2015

@fzs fzs modified the milestone: 1.2.0 Dec 13, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment