You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by "moresandeep (via GitHub)" <gi...@apache.org> on 2024/12/12 15:46:02 UTC
[PR] KNOX-3077 - Add pac4j.cookie.max.age param [knox]
moresandeep opened a new pull request, #972:
URL: https://github.com/apache/knox/pull/972
## What changes were proposed in this pull request?
This PR introduces a new parameter `pac4j.cookie.max.age` for the Pac4J provider that enforces cookie age on the cookies created by the pac4j provider.
e.g.
```
<provider>
<role>federation</role>
<name>pac4j</name>
<enabled>true</enabled>
<param>
<name>pac4j.callbackUrl</name>
<value>https://www.local.com:8443/gateway/knoxsso/api/v1/websso</value>
</param>
<!-- Unit is seconds -->
<param>
<name>pac4j.cookie.max.age</name>
<value>180</value>
</param>
.........
```
Note that these value needs to be same as `knoxsso.cookie.max.age` if you want Knox session to expire at the same time.
## How was this patch tested?
This patch was tested locally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-3077 - Add pac4j.cookie.max.age param [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #972:
URL: https://github.com/apache/knox/pull/972#discussion_r1882749429
##########
gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java:
##########
@@ -122,6 +122,12 @@ public class Pac4jDispatcherFilter implements Filter, SessionInvalidator {
private static final String PAC4J_OIDC_TYPE = "oidc.type";
+ /* property for specifying pac4j cookies ttl */
+ public static final String PAC4J_COOKIE_MAX_AGE = "pac4j.cookie.max.age";
+
+ /* default value is same is KNOXSSO token ttl default */
+ public static final int PAC4J_COOKIE_MAX_AGE_DEFAULT = -1;
Review Comment:
This could - should - be `private` (not used anywhere outside of this class) and `String` instead of `int` (i.e. `"-1"`, to avoid unnecessary conversion below)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-3077 - Add pac4j.cookie.max.age param [knox]
Posted by "moresandeep (via GitHub)" <gi...@apache.org>.
moresandeep commented on code in PR #972:
URL: https://github.com/apache/knox/pull/972#discussion_r1884187593
##########
gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java:
##########
@@ -122,6 +122,12 @@ public class Pac4jDispatcherFilter implements Filter, SessionInvalidator {
private static final String PAC4J_OIDC_TYPE = "oidc.type";
+ /* property for specifying pac4j cookies ttl */
+ public static final String PAC4J_COOKIE_MAX_AGE = "pac4j.cookie.max.age";
+
+ /* default value is same is KNOXSSO token ttl default */
+ public static final int PAC4J_COOKIE_MAX_AGE_DEFAULT = -1;
Review Comment:
@smolnar82 actually `PAC4J_COOKIE_MAX_AGE` is used outside of this class in `KnoxSessionStore` which needs this to be public.
I did update the `int` to `String` thanks for pointing it out!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-3077 - Add pac4j.cookie.max.age param [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #972:
URL: https://github.com/apache/knox/pull/972#discussion_r1884565011
##########
gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java:
##########
@@ -122,6 +122,12 @@ public class Pac4jDispatcherFilter implements Filter, SessionInvalidator {
private static final String PAC4J_OIDC_TYPE = "oidc.type";
+ /* property for specifying pac4j cookies ttl */
+ public static final String PAC4J_COOKIE_MAX_AGE = "pac4j.cookie.max.age";
+
+ /* default value is same is KNOXSSO token ttl default */
+ public static final int PAC4J_COOKIE_MAX_AGE_DEFAULT = -1;
Review Comment:
@moresandeep - yes, I know the parameter name is used elsewhere, that's why I added a comment on the default value only 😊
Thanks for applying the changes so quickly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-3077 - Add pac4j.cookie.max.age param [knox]
Posted by "moresandeep (via GitHub)" <gi...@apache.org>.
moresandeep commented on code in PR #972:
URL: https://github.com/apache/knox/pull/972#discussion_r1883054529
##########
gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java:
##########
@@ -122,6 +122,12 @@ public class Pac4jDispatcherFilter implements Filter, SessionInvalidator {
private static final String PAC4J_OIDC_TYPE = "oidc.type";
+ /* property for specifying pac4j cookies ttl */
+ public static final String PAC4J_COOKIE_MAX_AGE = "pac4j.cookie.max.age";
+
+ /* default value is same is KNOXSSO token ttl default */
+ public static final int PAC4J_COOKIE_MAX_AGE_DEFAULT = -1;
Review Comment:
Agreed! thanks for the review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-3077 - Add pac4j.cookie.max.age param [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on PR #972:
URL: https://github.com/apache/knox/pull/972#issuecomment-2542419222
Please don't forget to file a Doc Jira with this new config.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-3077 - Add pac4j.cookie.max.age param [knox]
Posted by "moresandeep (via GitHub)" <gi...@apache.org>.
moresandeep merged PR #972:
URL: https://github.com/apache/knox/pull/972
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org