Summary After upgrading play-pac4j from 12.0.0-PLAY3.0 to 12.0.1-PLAY3.0 (and higher), responses for pac4j-secured routes reissue cookies on every request: Session cookie is re-set each time with updated nbf/iat. CSRF cookie is also re-set each time

[kamonkaushals sd]

Summary

After upgrading play-pac4j from 12.0.0-PLAY3.0 to 12.0.1-PLAY3.0 (and higher), responses for pac4j-secured routes reissue cookies on every request:

Session cookie  is re-set each time with updated nbf/iat.

CSRF cookie is also re-set each time.

• This did not occur on 12.0.0-PLAY3.0. It now causes increased Set-Cookie traffic and unnecessary churn.

 

Scope

• Behavior occurs only on routes guarded by pac4j (@Secure). Non-secured routes do not reissue cookies.

 

Affected versions

• Good: 12.0.0-PLAY3.0

• Bad: 12.0.1-PLAY3.0 and later

 

Environment

• Play Framework 3.0.x (Java)

• Scala 2.13.16

• pac4j modules: core/http/saml/jwt 6.2.2

 

Dependencies (excerpt)

Dependencies.scala

        "org.pac4j" %% "play-pac4j" % "12.0.0-PLAY3.0",

        "org.pac4j" % "pac4j-core" % "6.2.2",

        "org.pac4j" % "pac4j-http" % "6.2.2",

        "org.pac4j" % "pac4j-saml" % "6.2.2",

    "org.pac4j" % "pac4j-jwt" % "6.2.2",

 

(Reproduces when the play-pac4j line is bumped to 12.0.1-PLAY3.0+.)

[Jérôme LELEU]

Hi,

Sorry for my late reply.

It wasn't easy to investigate this problem. It mainly comes from: https://github.com/pac4j/play-pac4j/pull/671

The session cookie recreation is a real problem and has been fixed.

The pac4jCsrfToken is actually renewed because of the rotateTokens of DefaultCsrfTokenGenerator and it is not a problem, but a mandatory fix.

Can you test with version 13.0.1-PLAY3.0-SNAPSHOT?

Thanks.

Best regards,

Jérôme

[Shane McDaniel]

I'm observing a related issue in 12.0.2-PLAY3.0, is that expected?

We have an @Secure handler that sets a session value but that value does not manifest in the browser cookie.  

Our current version is 12.0.0-PLAY3.0.

[Jérôme LELEU]

Hi,

The original problem, normally fixed in 13.0.1, was the recreation of the session.

But here, you have the opposite problem: the session is not recreated despite you setting a new value.

Can you do some debugging in the SecureAction class?

Thanks.

Best regards,

Jérôme

--
You received this message because you are subscribed to the Google Groups "Pac4j development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to pac4j-dev+...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/pac4j-dev/b3f9dc8c-e07d-49fa-85d9-0c9cd4c207b3n%40googlegroups.com.

[Shane McDaniel]

I will dig into the @Secure code more.

However, I've observed something from the browser side, does this give you any ideas?

As background: The user flow is:

1.  On a LIST page a user clicks a button to go to a PROGRAM page

2. The PROGRAM handler determines if we have the user language set and if not redirects them to a SET_LANG url with a redirectTo session cookie value

3. SET_LANG silently does its operation (setting the language from the browser request) then redirects to the redirectTo which is to the PROGRAM page

4. PROGRAM determines the users language is set and renders the page as expected.

In 12.0.1+ the redirectTo session value isn't present in SET_LANG, so SET_LANG sends the user back to LIST page as our default.

However if the user repeats the flow above by clicking the PROGRAM button it works as expected.  So the first time through the sessions is not updated, the second time it is.

What I observe is that in 12.0.1+ on the second load of LIST there is a pac4jCsrfToken, that is not present on the first load.  I would seem that cookie is necessary now.

In our existing service with 12.0.0 there is never a pac4jCsrfToken cookie in this flow.

Can you say anything about the presence of pac4jCsrfToken in 12.0.1+ and how it may be impacting our 12.0.0 based code?

[Shane McDaniel]

Apologies, ignore the second-time part of the previous.  The reason it "works" the second time is it does not do the SET_LANG redirect.  So that is completely misleading in what I said.

So the fundamental issue is just that values in the session.data are dropped

If I look at the code it does seem like that is what it does

r = r.withSession(session);

https://github.com/pac4j/play-pac4j/blob/master/shared/src/main/java/org/pac4j/play/PlayWebContext.java#L311

Appears to replace the Response session completely with the Request session.

A material change from 12.0.0 is the addition of: return playWebContext.supplementResponse(result);

https://github.com/pac4j/play-pac4j/blob/master/shared/src/main/java/org/pac4j/play/java/SecureAction.java#L104

I'm unsure if "supplement" is done in the 12.0.0 code base, but doing that after and replacing the response session with the request appears to be our issue and a bug.

[Shane McDaniel]

To be especially clear:

A material change from 12.0.0 is the addition of: return playWebContext.supplementResponse(result);

I mean from 12.0.0 on.  12.0.0 doesn't appear to do the supplement, or at least not in the same way, the versions after do.