You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2017/11/21 13:17:06 UTC

[Bug 61795] New: Make JASPIC callback handler class configurable via a property of the authenticator

https://bz.apache.org/bugzilla/show_bug.cgi?id=61795

            Bug ID: 61795
           Summary: Make JASPIC callback handler class configurable via a
                    property of the authenticator
           Product: Tomcat 8
           Version: 8.5.23
          Hardware: PC
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Catalina
          Assignee: dev@tomcat.apache.org
          Reporter: lazar.kirchev@gmail.com
  Target Milestone: ----

As discussed in the mailing list, it could be useful to provide a property of
the authenticator with which to specify the name of a custom JASPIC callback
handler implementation.

This pull request https://github.com/apache/tomcat/pull/93 illustrates the
idea.

If we agree that this is OK and this will be the name of the property, I will
also add it to the documentation.

I have not included a test because I had some concerns. I want to test only the
instantiation of the Callback Handler instance, but this is done in a private
method. So I see several approaches:

- in such cases I sometimes make the method package private so that it can be
accessed by the test. However, I see that this is not an accepted practice in
the Tomcat code, so I do not want to use it

- call the method with reflection - I see that in some Tomcat tests reflection
is actually used

- use a lot of mocking and test via the authenticate method of a child class of
the AuthenticatorBase

- go for a test with a Tomcat instance and and a sample application and test
the whole scenario of JAPSIC authentication with custom callback handler 

Which of these approaches do you prefer?

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[Bug 61795] Make JASPIC callback handler class configurable via a property of the authenticator

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61795

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #3 from Mark Thomas <ma...@apache.org> ---
Yet again, many thanks. Patch applied.

Fixed in:
- trunk for 9.0.2 onwards
- 8.5.x for 8.5.24 onwards

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[Bug 61795] Make JASPIC callback handler class configurable via a property of the authenticator

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61795

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 OS|                            |All

--- Comment #1 from Mark Thomas <ma...@apache.org> ---
The approach looks good to me.

a) The package private approach has been used in a few places. It is correct
that it isn't often the preferred approach.

b) Reflection might be the simplest option here.

c) Mocking is also an option although not one that is used very often.

d) A test with a Tomcat instance is often used - especially when the amount of
re-use possible means minimal additional test code needs to be written.

Ultimately it is your call. My own preference would be for b) or d) followed by
c) then a) but I'm not the one implementing this. If you have a strong
preference, go with that.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

[Bug 61795] Make JASPIC callback handler class configurable via a property of the authenticator

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61795

Lazar Kirchev <la...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement

--- Comment #2 from Lazar Kirchev <la...@gmail.com> ---
Thanks Mark! As I want to test only the creation logic, I also prefer b). 

I updated the pull request with tests and added the property to the
documentation.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org