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 2020/09/04 12:51:24 UTC
[Bug 64713] New: Principal is cached in session without
javax.servlet.http.registerSession being set
https://bz.apache.org/bugzilla/show_bug.cgi?id=64713
Bug ID: 64713
Summary: Principal is cached in session without
javax.servlet.http.registerSession being set
Product: Tomcat 9
Version: 9.0.37
Hardware: PC
Status: NEW
Severity: minor
Priority: P2
Component: JASPIC
Assignee: dev@tomcat.apache.org
Reporter: robert.rodewald@kopsis.com
Target Milestone: -----
The JASPIC 1.1 specification (section 3.8.4) states that the Prinicpal shall be
stored in the session after successful authentication by a ServerAuthModule if
javax.servlet.http.registerSession is set to a value of true. This works, but
if the value is set to false the current code is not compliant as only the
existence of the key is checked.
Here is the code from AuthenticatorBase:
Map map = state.messageInfo.getMap();
if (map != null && map.containsKey("javax.servlet.http.registerSession")) {
register(request, response, principal, "JASPIC", null, null, true, true);
} else {
register(request, response, principal, "JASPIC", null, null);
}
Furthermore this makes the behavior dependent on the configuration of the
authentication valves "cache" property it the property is not set, which is
very inflexible. The ServerAuthModule can therefore not decide to NOT cache it
in the session. Especially if you have more than one module which both want to
handle this differently.
I would suggest to evaluate the property if present and if it is set to "false"
disable any caching (ignore valve config). This makes it standard compliant and
more flexible without changing current behavior in the case of a "true" value.
Here is the changed code:
if (map != null && map.containsKey("javax.servlet.http.registerSession")) {
boolean registerSession =
Boolean.valueOf((String)map.get("javax.servlet.http.registerSession")).booleanValue();
register(request, response, principal, "JASPIC", null, null,
alwaysUseSession || registerSession, registerSession);
} else {
register(request, response, principal, "JASPIC", null, null);
}
--
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 64713] Principal is cached in session if
javax.servlet.http.registerSession is set to false
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64713
--- Comment #1 from Christopher Schultz <ch...@christopherschultz.net> ---
How about:
Map map = state.messageInfo.getMap();
if (map != null &&
Boolean.TRUE.equals(map.containsKey("javax.servlet.http.registerSession"))) {
register(request, response, principal, "JASPIC", null, null, true, true);
} else {
register(request, response, principal, "JASPIC", null, null);
}
??
Care to write a PR/patch for this and submit it? Its pretty trivial but you've
already done the real work involved in locating the issue and fixing it.
--
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 64713] Principal is cached in session if
javax.servlet.http.registerSession is set to false
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64713
--- Comment #6 from Mark Thomas <ma...@apache.org> ---
Thanks for this.
I'd looked at bug 64712 and come up with a patch for that that also (I thought)
addressed this bug but I had missed your case 3.
Using getOrDefault is tempting but given the importance of getting this right
I'd rather keep the code in sync across versions which means sticking to Java
1.7 methods.
It shouldn't take too long to merge my changes with your patch. I should get
the fix for this committed shortly.
--
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 64713] Principal is cached in session if
javax.servlet.http.registerSession is set to false
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64713
--- Comment #2 from Robert Rodewald <ro...@kopsis.com> ---
I suppose you meant:
Boolean.TRUE.equals(map.get("javax.servlet.http.registerSession")) ?
This solves the problem of the false value being interpreted as true, but
remains somewhat inflexible if you don't (really don't) want any caching as the
register-Method with the 6 parameters (as opposed to that with 8) uses the
valve configuration to determine the values for alwaysUseSession and cache.
What I was trying to achieve was (3 cases):
1. javax.servlet.http.registerSession set to TRUE -> do caching and always
create session (OK)
2. javax.servlet.http.registerSession NOT set -> use valve config for cache and
alwaysUseSession (OK)
AND
3. javax.servlet.http.registerSession set to FALSE -> don't do caching and use
config of valve for alwaysUseSession (MISSING)
The third option is important in my opinion. I'm thinking of Bearer
authentication. As soon as you don't send the token you should not be
authenticated any more (you would now be because of cached value in the
session).
I will try to produce the patch. Will be my first time but I'll do my best.
--
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 64713] Principal is cached in session if
javax.servlet.http.registerSession is set to false
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64713
Robert Rodewald <ro...@kopsis.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #37428|0 |1
is obsolete| |
--- Comment #5 from Robert Rodewald <ro...@kopsis.com> ---
Created attachment 37429
--> https://bz.apache.org/bugzilla/attachment.cgi?id=37429&action=edit
Combined patch for bugs 64712, 64713
combination of the patches for bug 64712 and 64713 to avoid merge conflicts
(hope that helps)
--
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 64713] Principal is cached in session if
javax.servlet.http.registerSession is set to false
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64713
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|NEW |RESOLVED
--- Comment #7 from Mark Thomas <ma...@apache.org> ---
Fixed in:
- master for 10.0.0-M8 onwards
- 9.0.x for 9.0.38 onwards
- 8.5.x for 8.5.58 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 64713] Principal is cached in session if
javax.servlet.http.registerSession is set to false
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64713
Robert Rodewald <ro...@kopsis.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
OS| |All
Summary|Principal is cached in |Principal is cached in
|session without |session if
|javax.servlet.http.register |javax.servlet.http.register
|Session being set |Session is set to false
--
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 64713] Principal is cached in session if
javax.servlet.http.registerSession is set to false
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64713
--- Comment #4 from Robert Rodewald <ro...@kopsis.com> ---
Created attachment 37428
--> https://bz.apache.org/bugzilla/attachment.cgi?id=37428&action=edit
Patch for Bug 64713
- extended evaluation of "jakarta.servlet.http.registerSession" (false value
disables caching of principal in session)
- removed unnecessary null-check for map (can not be null as per spec and
Tomcat implementation)
--
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 64713] Principal is cached in session if
javax.servlet.http.registerSession is set to false
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64713
--- Comment #3 from Christopher Schultz <ch...@christopherschultz.net> ---
(In reply to Robert Rodewald from comment #2)
> I suppose you meant:
> Boolean.TRUE.equals(map.get("javax.servlet.http.registerSession")) ?
Derp.
> The third option is important in my opinion. I'm thinking of Bearer
> authentication. As soon as you don't send the token you should not be
> authenticated any more (you would now be because of cached value in the
> session).
Aha, I get it, and yes, I agree. Although honestly Bearer authentication should
probably not be using a session but there's no need to be sloppy.
> I will try to produce the patch. Will be my first time but I'll do my best.
Excellent. Ask on the dev@ list if you need any help with that; we're happy to
help.
--
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