You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Romain Manni-Bucau <rm...@gmail.com> on 2013/08/06 08:51:24 UTC

jaasrealm regression

Hi

it seems we can now configure jaasrealm to use a jaas config file from the
webapp. That's great but it would need to fallback to old behavior (jaas
system property to find its location) by default otherwise apps using an
older tomcat are broken.

wdyt?

*Romain Manni-Bucau*
*Twitter: @rmannibucau <https://twitter.com/rmannibucau>*
*Blog: **http://rmannibucau.wordpress.com/*<http://rmannibucau.wordpress.com/>
*LinkedIn: **http://fr.linkedin.com/in/rmannibucau*
*Github: https://github.com/rmannibucau*

Re: jaasrealm regression

Posted by Romain Manni-Bucau <rm...@gmail.com>.
you are probably right but having the check "the path you specified doesn't
exist" is quite easy and would be a nice enhancement  (it is common to
think the config is right when it is wrong).

*Romain Manni-Bucau*
*Twitter: @rmannibucau <https://twitter.com/rmannibucau>*
*Blog: **http://rmannibucau.wordpress.com/*<http://rmannibucau.wordpress.com/>
*LinkedIn: **http://fr.linkedin.com/in/rmannibucau*
*Github: https://github.com/rmannibucau*



2013/8/7 Mark Thomas <ma...@apache.org>

> On 07/08/2013 10:33, Romain Manni-Bucau wrote:
> > ok, sorry
> >
> > i basically just set up a simple war sample using tomee maven plugin (but
> > for the part we speak about only tomcat is relevant). This sample was
> using
> > JAASRealm with the default LoginModule of tomee (properties one if you
> > care).
> >
> > IIRC before when the setup was wrong you get an error message saying the
> > jaas file is not found or not correctly set. When i first tested with a
> > wrong path i didn't get it but my login just failed.
>
> I've just been through the JRE source and the code path is identical
> between:
> - prior to this fix
> - post this fix with no web-app specific config.
>
> Therefore, any error message you got before, you will get now.
>
> > The point is IMO tomcat impl totally relies on JAAS for error cases.
> Since
> > there is an init phase (getConfig()) i think it could check the config a
> > bit more to log an explicit error message (it should check configFile of
> > course but the jaas system property too).
> >
> > Is it clearer?
>
> Yes. What you describe is an enhancement request. Personally, I'd ensure
> that any exception that is thrown by LoginConfig is visible to in the
> Tomcat logs (if it isn't already) rather than adding additional checks
> to getConfig(). Obviously the user will just see a failed login.
>
> Mark
>
> >
> > *Romain Manni-Bucau*
> > *Twitter: @rmannibucau <https://twitter.com/rmannibucau>*
> > *Blog: **http://rmannibucau.wordpress.com/*<
> http://rmannibucau.wordpress.com/>
> > *LinkedIn: **http://fr.linkedin.com/in/rmannibucau*
> > *Github: https://github.com/rmannibucau*
> >
> >
> >
> > 2013/8/7 Mark Thomas <ma...@apache.org>
> >
> >> On 07/08/2013 10:21, Romain Manni-Bucau wrote:
> >>> well maybe it does (surely since that's the LoginContext behavior) but
> >>> error is not obvious at all.
> >>>
> >>> I didn't got time to dig deeper into it but i think you are right and
> the
> >>> main issue is the error message which should be more explicit saying
> >>> configFile was not set and system property is missing or wrong.
> >>
> >> What error message?
> >>
> >>> wdyt?
> >>
> >> I think you need to be a lot clearer about what the problem is. A good
> >> problem definition should include:
> >> - what you did from a clean install
> >> - what happened that you didn't expect to happen
> >> - what didn't happen that you expected to happen
> >>
> >> So far your messages on this topic make little sense to someone who is
> >> not sat in front of your computer and has no knowledge of what you have
> >> tested and how.
> >>
> >> Mark
> >>
> >>>
> >>> PS: i changed of JVM version too, maybe LoginContext was modified, i
> >> didn't
> >>> check this neither.
> >>>
> >>> *Romain Manni-Bucau*
> >>> *Twitter: @rmannibucau <https://twitter.com/rmannibucau>*
> >>> *Blog: **http://rmannibucau.wordpress.com/*<
> >> http://rmannibucau.wordpress.com/>
> >>> *LinkedIn: **http://fr.linkedin.com/in/rmannibucau*
> >>> *Github: https://github.com/rmannibucau*
> >>>
> >>>
> >>>
> >>> 2013/8/7 Mark Thomas <ma...@apache.org>
> >>>
> >>>> On 06/08/2013 08:51, Romain Manni-Bucau wrote:
> >>>>> Hi
> >>>>>
> >>>>> it seems we can now configure jaasrealm to use a jaas config file
> from
> >>>> the
> >>>>> webapp. That's great but it would need to fallback to old behavior
> >> (jaas
> >>>>> system property to find its location) by default otherwise apps using
> >> an
> >>>>> older tomcat are broken.
> >>>>>
> >>>>> wdyt?
> >>>>
> >>>> What makes you think it doesn't fall-back to the old behaviour if no
> >>>> webapp specific file is specified?
> >>>>
> >>>> Here is the commit:
> >>>> http://svn.apache.org/viewvc?view=revision&revision=1498498
> >>>>
> >>>> Mark
> >>>>
> >>>>
> >>>> ---------------------------------------------------------------------
> >>>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> >>>> For additional commands, e-mail: dev-help@tomcat.apache.org
> >>>>
> >>>>
> >>>
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> >> For additional commands, e-mail: dev-help@tomcat.apache.org
> >>
> >>
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>

Re: jaasrealm regression

Posted by Mark Thomas <ma...@apache.org>.
On 07/08/2013 10:33, Romain Manni-Bucau wrote:
> ok, sorry
> 
> i basically just set up a simple war sample using tomee maven plugin (but
> for the part we speak about only tomcat is relevant). This sample was using
> JAASRealm with the default LoginModule of tomee (properties one if you
> care).
> 
> IIRC before when the setup was wrong you get an error message saying the
> jaas file is not found or not correctly set. When i first tested with a
> wrong path i didn't get it but my login just failed.

I've just been through the JRE source and the code path is identical
between:
- prior to this fix
- post this fix with no web-app specific config.

Therefore, any error message you got before, you will get now.

> The point is IMO tomcat impl totally relies on JAAS for error cases. Since
> there is an init phase (getConfig()) i think it could check the config a
> bit more to log an explicit error message (it should check configFile of
> course but the jaas system property too).
> 
> Is it clearer?

Yes. What you describe is an enhancement request. Personally, I'd ensure
that any exception that is thrown by LoginConfig is visible to in the
Tomcat logs (if it isn't already) rather than adding additional checks
to getConfig(). Obviously the user will just see a failed login.

Mark

> 
> *Romain Manni-Bucau*
> *Twitter: @rmannibucau <https://twitter.com/rmannibucau>*
> *Blog: **http://rmannibucau.wordpress.com/*<http://rmannibucau.wordpress.com/>
> *LinkedIn: **http://fr.linkedin.com/in/rmannibucau*
> *Github: https://github.com/rmannibucau*
> 
> 
> 
> 2013/8/7 Mark Thomas <ma...@apache.org>
> 
>> On 07/08/2013 10:21, Romain Manni-Bucau wrote:
>>> well maybe it does (surely since that's the LoginContext behavior) but
>>> error is not obvious at all.
>>>
>>> I didn't got time to dig deeper into it but i think you are right and the
>>> main issue is the error message which should be more explicit saying
>>> configFile was not set and system property is missing or wrong.
>>
>> What error message?
>>
>>> wdyt?
>>
>> I think you need to be a lot clearer about what the problem is. A good
>> problem definition should include:
>> - what you did from a clean install
>> - what happened that you didn't expect to happen
>> - what didn't happen that you expected to happen
>>
>> So far your messages on this topic make little sense to someone who is
>> not sat in front of your computer and has no knowledge of what you have
>> tested and how.
>>
>> Mark
>>
>>>
>>> PS: i changed of JVM version too, maybe LoginContext was modified, i
>> didn't
>>> check this neither.
>>>
>>> *Romain Manni-Bucau*
>>> *Twitter: @rmannibucau <https://twitter.com/rmannibucau>*
>>> *Blog: **http://rmannibucau.wordpress.com/*<
>> http://rmannibucau.wordpress.com/>
>>> *LinkedIn: **http://fr.linkedin.com/in/rmannibucau*
>>> *Github: https://github.com/rmannibucau*
>>>
>>>
>>>
>>> 2013/8/7 Mark Thomas <ma...@apache.org>
>>>
>>>> On 06/08/2013 08:51, Romain Manni-Bucau wrote:
>>>>> Hi
>>>>>
>>>>> it seems we can now configure jaasrealm to use a jaas config file from
>>>> the
>>>>> webapp. That's great but it would need to fallback to old behavior
>> (jaas
>>>>> system property to find its location) by default otherwise apps using
>> an
>>>>> older tomcat are broken.
>>>>>
>>>>> wdyt?
>>>>
>>>> What makes you think it doesn't fall-back to the old behaviour if no
>>>> webapp specific file is specified?
>>>>
>>>> Here is the commit:
>>>> http://svn.apache.org/viewvc?view=revision&revision=1498498
>>>>
>>>> Mark
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>>>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>>>
>>>>
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>>
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: jaasrealm regression

Posted by Romain Manni-Bucau <rm...@gmail.com>.
ok, sorry

i basically just set up a simple war sample using tomee maven plugin (but
for the part we speak about only tomcat is relevant). This sample was using
JAASRealm with the default LoginModule of tomee (properties one if you
care).

IIRC before when the setup was wrong you get an error message saying the
jaas file is not found or not correctly set. When i first tested with a
wrong path i didn't get it but my login just failed.

The point is IMO tomcat impl totally relies on JAAS for error cases. Since
there is an init phase (getConfig()) i think it could check the config a
bit more to log an explicit error message (it should check configFile of
course but the jaas system property too).

Is it clearer?

*Romain Manni-Bucau*
*Twitter: @rmannibucau <https://twitter.com/rmannibucau>*
*Blog: **http://rmannibucau.wordpress.com/*<http://rmannibucau.wordpress.com/>
*LinkedIn: **http://fr.linkedin.com/in/rmannibucau*
*Github: https://github.com/rmannibucau*



2013/8/7 Mark Thomas <ma...@apache.org>

> On 07/08/2013 10:21, Romain Manni-Bucau wrote:
> > well maybe it does (surely since that's the LoginContext behavior) but
> > error is not obvious at all.
> >
> > I didn't got time to dig deeper into it but i think you are right and the
> > main issue is the error message which should be more explicit saying
> > configFile was not set and system property is missing or wrong.
>
> What error message?
>
> > wdyt?
>
> I think you need to be a lot clearer about what the problem is. A good
> problem definition should include:
> - what you did from a clean install
> - what happened that you didn't expect to happen
> - what didn't happen that you expected to happen
>
> So far your messages on this topic make little sense to someone who is
> not sat in front of your computer and has no knowledge of what you have
> tested and how.
>
> Mark
>
> >
> > PS: i changed of JVM version too, maybe LoginContext was modified, i
> didn't
> > check this neither.
> >
> > *Romain Manni-Bucau*
> > *Twitter: @rmannibucau <https://twitter.com/rmannibucau>*
> > *Blog: **http://rmannibucau.wordpress.com/*<
> http://rmannibucau.wordpress.com/>
> > *LinkedIn: **http://fr.linkedin.com/in/rmannibucau*
> > *Github: https://github.com/rmannibucau*
> >
> >
> >
> > 2013/8/7 Mark Thomas <ma...@apache.org>
> >
> >> On 06/08/2013 08:51, Romain Manni-Bucau wrote:
> >>> Hi
> >>>
> >>> it seems we can now configure jaasrealm to use a jaas config file from
> >> the
> >>> webapp. That's great but it would need to fallback to old behavior
> (jaas
> >>> system property to find its location) by default otherwise apps using
> an
> >>> older tomcat are broken.
> >>>
> >>> wdyt?
> >>
> >> What makes you think it doesn't fall-back to the old behaviour if no
> >> webapp specific file is specified?
> >>
> >> Here is the commit:
> >> http://svn.apache.org/viewvc?view=revision&revision=1498498
> >>
> >> Mark
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> >> For additional commands, e-mail: dev-help@tomcat.apache.org
> >>
> >>
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>

Re: jaasrealm regression

Posted by Mark Thomas <ma...@apache.org>.
On 07/08/2013 10:21, Romain Manni-Bucau wrote:
> well maybe it does (surely since that's the LoginContext behavior) but
> error is not obvious at all.
> 
> I didn't got time to dig deeper into it but i think you are right and the
> main issue is the error message which should be more explicit saying
> configFile was not set and system property is missing or wrong.

What error message?

> wdyt?

I think you need to be a lot clearer about what the problem is. A good
problem definition should include:
- what you did from a clean install
- what happened that you didn't expect to happen
- what didn't happen that you expected to happen

So far your messages on this topic make little sense to someone who is
not sat in front of your computer and has no knowledge of what you have
tested and how.

Mark

> 
> PS: i changed of JVM version too, maybe LoginContext was modified, i didn't
> check this neither.
> 
> *Romain Manni-Bucau*
> *Twitter: @rmannibucau <https://twitter.com/rmannibucau>*
> *Blog: **http://rmannibucau.wordpress.com/*<http://rmannibucau.wordpress.com/>
> *LinkedIn: **http://fr.linkedin.com/in/rmannibucau*
> *Github: https://github.com/rmannibucau*
> 
> 
> 
> 2013/8/7 Mark Thomas <ma...@apache.org>
> 
>> On 06/08/2013 08:51, Romain Manni-Bucau wrote:
>>> Hi
>>>
>>> it seems we can now configure jaasrealm to use a jaas config file from
>> the
>>> webapp. That's great but it would need to fallback to old behavior (jaas
>>> system property to find its location) by default otherwise apps using an
>>> older tomcat are broken.
>>>
>>> wdyt?
>>
>> What makes you think it doesn't fall-back to the old behaviour if no
>> webapp specific file is specified?
>>
>> Here is the commit:
>> http://svn.apache.org/viewvc?view=revision&revision=1498498
>>
>> Mark
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>>
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: jaasrealm regression

Posted by Romain Manni-Bucau <rm...@gmail.com>.
well maybe it does (surely since that's the LoginContext behavior) but
error is not obvious at all.

I didn't got time to dig deeper into it but i think you are right and the
main issue is the error message which should be more explicit saying
configFile was not set and system property is missing or wrong.

wdyt?

PS: i changed of JVM version too, maybe LoginContext was modified, i didn't
check this neither.

*Romain Manni-Bucau*
*Twitter: @rmannibucau <https://twitter.com/rmannibucau>*
*Blog: **http://rmannibucau.wordpress.com/*<http://rmannibucau.wordpress.com/>
*LinkedIn: **http://fr.linkedin.com/in/rmannibucau*
*Github: https://github.com/rmannibucau*



2013/8/7 Mark Thomas <ma...@apache.org>

> On 06/08/2013 08:51, Romain Manni-Bucau wrote:
> > Hi
> >
> > it seems we can now configure jaasrealm to use a jaas config file from
> the
> > webapp. That's great but it would need to fallback to old behavior (jaas
> > system property to find its location) by default otherwise apps using an
> > older tomcat are broken.
> >
> > wdyt?
>
> What makes you think it doesn't fall-back to the old behaviour if no
> webapp specific file is specified?
>
> Here is the commit:
> http://svn.apache.org/viewvc?view=revision&revision=1498498
>
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>

Re: jaasrealm regression

Posted by Mark Thomas <ma...@apache.org>.
On 06/08/2013 08:51, Romain Manni-Bucau wrote:
> Hi
> 
> it seems we can now configure jaasrealm to use a jaas config file from the
> webapp. That's great but it would need to fallback to old behavior (jaas
> system property to find its location) by default otherwise apps using an
> older tomcat are broken.
> 
> wdyt?

What makes you think it doesn't fall-back to the old behaviour if no
webapp specific file is specified?

Here is the commit:
http://svn.apache.org/viewvc?view=revision&revision=1498498

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org