This fix effectively prevents a double execution of bind() but, I if I
got it right, for a short time it might expose a wrong true value for
signedIn. Consider an initial condition where we have just one thread.
When "signedIn.compareAndSet(false, true)" signedIn is set to true but
later if 'authenticate' fails is set back to false. So we a have a short
time in which signedIn has a non reliable value.
On 10/09/2016 13:16, Martin Grigorov wrote:
Hi,
I've pushed a possible fix in a branch.
You can see the diff at
https://git1-us-west.apache.org/repos/asf?p=wicket.git;a=commitdiff;h=a384c6f7;hp=6f530a925c25006a80cc97c5a30bca66a471cfe8
Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov
On Thu, Sep 8, 2016 at 3:57 PM, Martin Grigorov <[email protected]>
wrote:
Hi,
I think it is a bug in Wicket, so it should be fixed in Wicket itself.
The only way an application can fix it is to add some logic in its
#authenticate() method - wrap the logic inside "if (!isSignedIn())".
Using AtomicBoolean is definetely better in this case.
#signIn() should call #authenticate() only if the value is changed.
The open question is what to return from #signIn() because now there are
three options:
- unsuccessful authentication
- successful authentication
- already signed in
Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov
On Thu, Sep 8, 2016 at 3:30 PM, Andrea Del Bene <[email protected]>
wrote:
In this case I think it would be enough if we use an atomic boolean and
if we change its value with compareAndSet.
Andrea.
On 08/09/2016 13:22, Sven Meier wrote:
Hi Martin,
how easy is it for the application to deal with this?
We could break some stuff by sprinkling 'synchronized' over Wicket's
session-handling methods now.
Regards
Sven
On 07.09.2016 14:59, Martin Grigorov wrote:
Hi,
Currently [1] uses a volatile boolean "signedIn" to control the state.
org.apache.wicket.authroles.authentication.panel.SignInPanel#onConfigure()
tries to make use of it.
IMO this implementation is a bit weak. There are big windows this state
to
change in the meantime.
Usually this shouldn't be a big problem, the application will
authenticate
the same user twice.
But if the application does something in ISessionListener#onBind() then
it
becomes a problem [2].
Do you think this is a problem in Wicket or the applications should deal
with it?
1.
https://github.com/apache/wicket/blob/master/wicket-auth-rol
es/src/main/java/org/apache/wicket/authroles/authenticatio
n/AuthenticatedWebSession.java
2. https://issues.apache.org/jira/browse/ISIS-1481
Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov
