payara / Payara-Examples

Repository for Example Code to demonstrate Payara specific features
144 stars 177 forks source link

BUG: JWTAuthenticationMechanism.java #81

Open christiancandela opened 6 years ago

christiancandela commented 6 years ago

Hello, In the next line you have a error

isRememberMeExpression = "self.isRememberMe(httpMessageContext)"

change to

isRememberMeExpression = "#{self.isRememberMe(httpMessageContext)}")

OndroMih commented 6 years ago

I think both versions work with Soteria, but the Javadoc says that the value is an Expression and not an Expression in #{...}, so the current solution is better.

Github has lots of examples for both: https://github.com/search?p=2&q=isRememberMeExpression&type=Code

@arjantijms, do you know which is correct?

OndroMih commented 6 years ago

This is the code: https://github.com/payara/Payara-Examples/blob/ca6d0093cf0cea7ff059f454dcd1e75f65088889/Java-EE/security-jwt-example/src/main/java/fish/payara/examples/security/JWTAuthenticationMechanism.java#L60

arjantijms commented 6 years ago

@OndrejM it has to be an expression delimited with the #{} or ${} syntax.

The confusion comes perhaps from the earlier EE Security revision (pre 1.0) where I didn't put in the check for that syntax yet. The reasoning was indeed that if the attribute could only hold an expression, why require the syntax.

But, as we implemented EL support across the spec, it became clear that it looked quite inconsistent if for one annotation that had String members already the syntax was required (to distinguish from a plain string) and for other attributes (those that are specifically for expressions) not.

So at long lost we made the decision to simply require the syntax everywhere for expressions. The added benefit is that some IDE editors have an easier time recognising them as well.

In the code you see this here:

  public static boolean evalELExpression(ELProcessor getELProcessor, String expression, boolean defaultValue) {
        if (!isELExpression(expression)) {
            return defaultValue;
        }

        return (Boolean) getELProcessor(getELProcessor).eval(toRawExpression(expression));
    }

If it's not an EL expression, the default value is taken, which in this case is the value of the boolean isRememberMe. I'll try to clarify this in the JavaDoc.

christiancandela commented 6 years ago

Hello, i test in actuall form and it doesn’t funtion. It’s ever put de cookie as rememberme value was true. When i change for the #{}, it does funtion well.

Thanks for you example. In my case this example helps my much.

Enviado desde mi iPhone

El 6/06/2018, a la(s) 6:44 a. m., Arjan Tijms notifications@github.com escribió:

@OndrejM it has to be an expression delimited with the #{} or ${} syntax.

The confusion comes perhaps from the earlier EE Security revision (pre 1.0) where I didn't put in the check for that syntax yet. The reasoning was indeed that if the attribute could only hold an expression, why require the syntax.

But, as we implemented EL support across the spec, it became clear that it looked quite inconsistent if for one annotation that had String members already the syntax was required (to distinguish from a plain string) and for other attributes (those that are specifically for expressions) not.

So at long lost we made the decision to simply require the syntax everywhere for expressions. The added benefit is that some IDE editors have an easier time recognising them as well.

In the code you see this here:

public static boolean evalELExpression(ELProcessor getELProcessor, String expression, boolean defaultValue) { if (!isELExpression(expression)) { return defaultValue; }

    return (Boolean) getELProcessor(getELProcessor).eval(toRawExpression(expression));
}

If it's not an EL expression, the default value is taken, which in this case is the value of the boolean isRememberMe. I'll try to clarify this in the JavaDoc.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

christiancandela commented 6 years ago

Hello again…

i forget say that for generate the cookie I change

@RememberMe( cookieMaxAgeSeconds = REMEMBERME_VALIDITY_SECONDS, isRememberMeExpression = "self.isRememberMe(httpMessageContext)" )

to

@RememberMe( cookieMaxAgeSeconds = REMEMBERME_VALIDITY_SECONDS, cookieSecureOnly = false, isRememberMeExpression = "#{self.isRememberMe(httpMessageContext)}" )

This because the example don't work for http without cookieSecureOnly = false,

El 6/06/2018, a las 6:44 a. m., Arjan Tijms notifications@github.com escribió:

@OndrejM https://github.com/OndrejM it has to be an expression delimited with the #{} or ${} syntax.

The confusion comes perhaps from the earlier EE Security revision (pre 1.0) where I didn't put in the check for that syntax yet. The reasoning was indeed that if the attribute could only hold an expression, why require the syntax.

But, as we implemented EL support across the spec, it became clear that it looked quite inconsistent if for one annotation that had String members already the syntax was required (to distinguish from a plain string) and for other attributes (those that are specifically for expressions) not.

So at long lost we made the decision to simply require the syntax everywhere for expressions. The added benefit is that some IDE editors have an easier time recognising them as well.

In the code you see this here:

public static boolean evalELExpression(ELProcessor getELProcessor, String expression, boolean defaultValue) { if (!isELExpression(expression)) { return defaultValue; }

    return (Boolean) getELProcessor(getELProcessor).eval(toRawExpression(expression));
}

If it's not an EL expression, the default value is taken, which in this case is the value of the boolean isRememberMe. I'll try to clarify this in the JavaDoc.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/payara/Payara-Examples/issues/81#issuecomment-395040016, or mute the thread https://github.com/notifications/unsubscribe-auth/AD9Mmdto528z41Nr_7j7XDh2W0Ze2kxKks5t58CwgaJpZM4UcAKe.