sbglasius commented on code in PR #16174:
URL: https://github.com/apache/grails-core/pull/16174#discussion_r3825111026
##########
grails-spring-security/cas/plugin/src/main/groovy/grails/plugin/springsecurity/cas/SpringSecurityCasGrailsPlugin.groovy:
##########
@@ -35,25 +36,28 @@ import
org.springframework.security.cas.authentication.CasAuthenticationProvider
import org.springframework.security.cas.authentication.NullStatelessTicketCache
import org.springframework.security.cas.web.CasAuthenticationEntryPoint
import org.springframework.security.cas.web.CasAuthenticationFilter
+import
org.springframework.security.web.authentication.session.NullAuthenticatedSessionStrategy
+import grails.plugin.springsecurity.BeanTypeResolver
import grails.plugin.springsecurity.SecurityFilterPosition
import grails.plugin.springsecurity.SpringSecurityUtils
import grails.plugins.Plugin
+@Slf4j
@CompileStatic
class SpringSecurityCasGrailsPlugin extends Plugin {
- String grailsVersion = '7.0.0 > *'
+ String grailsVersion = '8.0.0-SNAPSHOT > *'
String author = 'Burt Beckwith'
Review Comment:
Done in 2391490. Removed both `author` and `authorEmail` (the latter was an
empty string and meaningless without the former).
##########
grails-doc/src/en/guide/security/securityPlugins/springSecurity/cas/usage.adoc:
##########
@@ -25,7 +25,7 @@ under the License.
Configuring your CAS server is beyond the scope of this document. There are
many different approaches and this will most likely be done by IT staff. It's
assumed here that you already have a running CAS server.
====
-https://www.jasig.org/cas[CAS] is a popular single sign-on implementation.
It's open source and has an Apache-like license, and is easy to get started
with but is also highly configurable. In addition it has clients written in
Java, .Net, PHP, Perl, and other languages.
+https://apereo.github.io/cas[CAS] is a popular single sign-on implementation.
It's open source and has an Apache-like license, and is easy to get started
with but is also highly configurable. In addition it has clients written in
Java, .Net, PHP, Perl, and other languages.
Review Comment:
Good catch, that section directly contradicted the change. Rewritten in
2391490: it now leads with the fact that it's opt-in and *why* (enabling it
disables session fixation prevention, because CAS maps the service ticket to
the session id), shows the `useSingleSignout: true` snippet, and mentions the
startup warning. The former "if you don't want the filter registered, disable
it" paragraph is now framed as returning to the default.
##########
grails-doc/src/en/guide/upgrading/upgrading80x.adoc:
##########
@@ -2632,3 +2632,36 @@ used to apply to its own message source.
Adding or removing a base name now needs a restart, because Spring Boot reads
the configured base names
once when it builds the message source.
+
+==== 46. CAS Single Sign-Out Is Opt-In
Review Comment:
Fixed in 2391490 — renumbered to 48.
Worth flagging that this was more than cosmetic: 45, 46 and 47 all landed
upstream while this PR was open, so the file actually had *two* sections
numbered 46 and would have produced duplicate asciidoctor anchors. I checked
the whole file for other collisions and there are none.
##########
grails-test-examples/spring-security/cas/test1/src/integration-test/groovy/grails/plugin/springsecurity/cas/test/AbstractCasSpec.groovy:
##########
@@ -0,0 +1,178 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package grails.plugin.springsecurity.cas.test
+
+import grails.testing.mixin.integration.Integration
+import spock.lang.Requires
+import spock.lang.Specification
+
+import java.net.http.HttpClient
+import java.net.http.HttpRequest
+import java.net.http.HttpResponse
+import java.time.Duration
+
+/**
+ * Drives the CAS protocol by hand against the containerised CAS server.
+ *
+ * <p>The app and CAS are both reached on {@code localhost}, and cookies
ignore ports, so a single
+ * cookie jar would send the app's session cookie to CAS and vice versa. Each
side therefore gets
+ * its own client, and redirects are followed explicitly so the right one is
used for each hop.</p>
+ */
+@Integration
+@Requires({ isDockerAvailable() })
+abstract class AbstractCasSpec extends Specification {
+
+ HttpClient appClient
+ HttpClient casClient
+
+ /**
+ * A cookie-less client for the calls CAS makes to the application on its
own connection, such
+ * as the back-channel logout request. Using the authenticated client
instead would hide which
+ * filter acted: an unconsumed POST to the CAS login path fails
authentication and clears the
+ * session by itself, which looks the same from outside as single signout
working.
+ */
+ HttpClient backChannelClient
+
+ /** The service ticket CAS issued during the most recent {@link #login}
call. */
+ String lastServiceTicket
+
+ void setup() {
+ appClient = newClient()
+ casClient = newClient()
+ backChannelClient = newClient()
+ }
+
+ /**
+ * Mirrors the probe used by the hibernate7 specs. Checking the socket
avoids the macOS failure
+ * mode where asking Testcontainers for a client throws when the daemon
API version differs.
+ */
+ static boolean isDockerAvailable() {
+ List<String> candidates = [
+ System.getProperty('user.home') + '/.docker/run/docker.sock',
+ '/var/run/docker.sock',
+ System.getenv('DOCKER_HOST') ?: ''
+ ]
+ candidates.any { it && new File(it).exists() }
+ }
+
+ String getAppBaseUrl() {
+ "http://localhost:${serverPort}"
+ }
+
+ String getCasBaseUrl() {
+ CasContainerHolder.serverUrlPrefix
+ }
+
+ /** CAS redirects to the container-visible host name; the test client has
to use localhost. */
+ static String toLocalUrl(String url) {
+ url.replace(CasTestConfig.CONTAINER_VISIBLE_HOST, 'localhost')
+ }
+
+ HttpResponse<String> get(HttpClient client, String url) {
+ client.send(HttpRequest.newBuilder(URI.create(url)).GET().build(),
+ HttpResponse.BodyHandlers.ofString())
+ }
+
+ HttpResponse<String> postForm(HttpClient client, String url, Map<String,
String> form) {
+ String body = form.collect { k, v -> "${encode(k)}=${encode(v)}"
}.join('&')
+ HttpRequest request = HttpRequest.newBuilder(URI.create(url))
+ .header('Content-Type', 'application/x-www-form-urlencoded')
+ .POST(HttpRequest.BodyPublishers.ofString(body))
+ .build()
+ client.send(request, HttpResponse.BodyHandlers.ofString())
+ }
+
+ static String location(HttpResponse<?> response) {
+ response.headers().firstValue('Location').orElse(null)
+ }
+
+ /**
+ * Authenticates at CAS and returns the service ticket URL it redirects
back to, already
+ * rewritten to localhost.
+ */
+ String authenticateAtCas(String loginUrl, String username, String
password) {
+ HttpResponse<String> form = get(casClient, loginUrl)
+ assert form.statusCode() == 200
+ String execution = extractExecution(form.body())
+ assert execution, 'CAS login form did not contain an execution token'
+
+ HttpResponse<String> submitted = postForm(casClient, loginUrl,
+ [username: username, password: password, execution: execution,
_eventId: 'submit'])
+ assert submitted.statusCode() == 302,
+ "expected CAS to redirect after login but got
${submitted.statusCode()}"
+ toLocalUrl(location(submitted))
+ }
+
+ /** Full login: hit a secured URL, authenticate at CAS, and follow the
ticket back to the app. */
+ HttpResponse<String> login(String securedPath, String username, String
password) {
+ HttpResponse<String> challenge = get(appClient, appBaseUrl +
securedPath)
+ assert challenge.statusCode() == 302,
+ "expected a redirect to CAS but got ${challenge.statusCode()}"
+ String ticketUrl = authenticateAtCas(location(challenge), username,
password)
+ lastServiceTicket = extractTicket(ticketUrl)
+ assert lastServiceTicket, "CAS redirect carried no service ticket:
${ticketUrl}"
+ followRedirects(get(appClient, ticketUrl))
+ }
+
+ HttpResponse<String> followRedirects(HttpResponse<String> response, int
limit = 5) {
+ HttpResponse<String> current = response
+ for (int i = 0; i < limit && current.statusCode() in [301, 302, 303,
307, 308]; i++) {
+ current = get(appClient, absolute(location(current)))
+ }
+ current
+ }
+
+ String absolute(String location) {
+ String local = toLocalUrl(location)
+ local.startsWith('http') ? local : appBaseUrl + local
+ }
+
+ /** The message CAS sends to a service on back-channel logout. */
+ static String logoutRequest(String serviceTicket) {
+ """<samlp:LogoutRequest
xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" \
+ID="LR-1-${System.nanoTime()}" Version="2.0"
IssueInstant="2026-01-01T00:00:00Z">\
+<saml:NameID
xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">@NOT_USED@</saml:NameID>\
+<samlp:SessionIndex>${serviceTicket}</samlp:SessionIndex>\
+</samlp:LogoutRequest>"""
+ }
+
+ static String extractTicket(String url) {
+ def matcher = url =~ /[?&]ticket=([^&]+)/
+ matcher.find() ? matcher.group(1) : null
+ }
+
+ private static String extractExecution(String html) {
+ def matcher = html =~ /name="execution"\s+value="([^"]+)"/
+ matcher.find() ? matcher.group(1) : null
+ }
+
+ private static String encode(String value) {
+ URLEncoder.encode(value, 'UTF-8')
+ }
+
+ private static HttpClient newClient() {
+ CookieManager cookieManager = new CookieManager(null,
CookiePolicy.ACCEPT_ALL)
+ HttpClient.newBuilder()
+ .cookieHandler(cookieManager)
+ .followRedirects(HttpClient.Redirect.NEVER)
+ .connectTimeout(Duration.ofSeconds(30))
+ .build()
Review Comment:
Agreed, and this is the failure mode most likely to bite in CI. Fixed in
2391490: added a `REQUEST_TIMEOUT` of 60s applied to both `get()` and
`postForm()` via `HttpRequest.timeout(...)`, so a container that accepts the
connection but never answers fails the spec in a minute instead of hanging
until the runner's job-level kill. Kept the 30s `connectTimeout` as-is.
I went with a per-request timeout rather than `@Timeout` so the failure
points at the specific stalled call rather than the whole feature method.
##########
grails-spring-security/cas/plugin/src/main/groovy/grails/plugin/springsecurity/cas/SpringSecurityCasGrailsPlugin.groovy:
##########
@@ -87,10 +91,29 @@ class SpringSecurityCasGrailsPlugin extends Plugin {
if (conf.cas.useSingleSignout) {
- // session fixation prevention breaks single signout because
- // the service ticket is mapped to the session id which changes
+ // Session fixation prevention breaks single signout because
the service ticket is
+ // mapped to the session id, which changes when the session is
replaced on login.
+ // Disabling it is a security trade-off the application has
opted into, so say so.
+ String message = '''
+ WARNING: cas.useSingleSignout is enabled, so session fixation prevention
has been disabled.
+ CAS maps the service ticket to the HTTP session id, and a logout request
cannot be matched to a
+ session that was replaced when the user authenticated. Set
cas.useSingleSignout to false to keep
+ session fixation prevention and handle logout in the application instead.
+ '''
+ println message
Review Comment:
You're right that the annotation was dead, and right about the reasoning:
the docs this PR adds say the plugin logs a warning, and an app capturing logs
but not stdout would never see a `println`. Restored `log.warn` alongside the
`println` in 2391490, matching `SpringSecurityCoreGrailsPlugin:423-424`.
Whitespace-only line trimmed too.
##########
grails-test-examples/spring-security/cas/README.md:
##########
@@ -14,21 +14,77 @@ See the License for the specific language governing
permissions and
limitations under the License.
-->
-This is a CAS-enabled test application. To run it successfully, a CAS
-server is required. The URL for the CAS server is configured in the
-[application.groovy](test1/grails-app/conf/application.groovy)
-file. Setting up a CAS server is out of the scope of this document, but
-good places to start are [Apereo CAS GitHub](https://github.com/apereo/cas)
-and the [CAS Initializr](https://getcas.apereo.org/ui) service.
+This is a CAS-enabled test application. It no longer needs a hand-run CAS
server: an
+[Apereo CAS](https://github.com/apereo/cas) server is started in a container by
+[CasContainerHolder](test1/src/main/groovy/grails/plugin/springsecurity/cas/test/CasContainerHolder.groovy),
+and
[CasTestEnvironmentPostProcessor](test1/src/main/groovy/grails/plugin/springsecurity/cas/test/CasTestEnvironmentPostProcessor.groovy)
+points the CAS plugin at it before the application context is built. Docker
(or a compatible
+container runtime) is therefore required to run or test this application.
-The test application can be run with:
+## Running the tests
-`./gradlew :testapp-spring-security-cas-test1:bootRun`
+The application is exercised under three configurations, selected with the
`TESTCONFIG` system
+property. Each has to be its own run, because the configuration is applied at
application startup.
+
+The specs only run when one of these is selected, so a build that covers every
Spring Security
+example does not repeat a configuration that the dedicated per-configuration
CI job already runs.
+Running `check` without `-DTESTCONFIG` therefore reports no CAS tests.
+
+| `TESTCONFIG` | Configuration | Covered by |
+|---|---|---|
+| `cas` (default) | `proxyCallbackUrl` and `proxyReceptorUrl` unset, single
signout enabled | `CasLoginSpec`, `CasNoProxyReceptorSpec`,
`CasSingleSignOutSpec` |
+| `casProxy` | both proxy settings configured, single signout enabled |
`CasLoginSpec`, `CasProxyTicketSpec`, `CasSingleSignOutSpec` |
+| `casNoSingleSignout` | `cas.useSingleSignout` left at its default |
`CasLoginSpec`, `CasNoProxyReceptorSpec`, `CasNoSingleSignOutSpec` |
+
+```
+./gradlew :grails-test-examples-spring-security-cas-test1:check
-DTESTCONFIG=cas
+./gradlew :grails-test-examples-spring-security-cas-test1:check
-DTESTCONFIG=casProxy
+./gradlew :grails-test-examples-spring-security-cas-test1:check
-DTESTCONFIG=casNoSingleSignout
+```
+
+`cas.useSingleSignout` is opt-in as of Grails 8. The app enables it for the
first two configurations
+so the single signout filter is exercised; enabling it disables session
fixation prevention, and the
+plugin warns about that at startup.
+
+The CAS image is pinned to a known-good version and can be overridden:
+
+```
+./gradlew :grails-test-examples-spring-security-cas-test1:check
-PcasContainerVersion=7.3.6
+```
+
+The specs skip themselves when no Docker daemon is available.
+
+## Running the application
+
+```
+./gradlew :grails-test-examples-spring-security-cas-test1:bootRun
+```
The test application URLs are:
* [http://localhost:8081/secure/admins](http://localhost:8081/secure/admins)
* [http://localhost:8081/secure/users](http://localhost:8081/secure/users)
+*
[http://localhost:8081/secure/proxyStatus](http://localhost:8081/secure/proxyStatus)
— asks CAS for a proxy ticket
Review Comment:
Correct, and the root cause was worse than the docs: `bootRun` forks its own
JVM, so `-DTESTCONFIG` never reached the app at all — there was no way to
exercise the endpoint by hand.
Rather than just note the limitation, `build.gradle` now forwards
`TESTCONFIG` and `casContainerVersion` to `bootRun`, and the README documents
`bootRun -DTESTCONFIG=casProxy` (2391490). Verified end-to-end: drove a login
against the container and `/secure/proxyStatus` returned
`PROXY_TICKET:PT-2-1B26izig…` instead of `NO_PROXY_TICKET`.
##########
grails-test-examples/spring-security/cas/test1/grails-app/controllers/grails/plugin/springsecurity/cas/test/SecureController.groovy:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package grails.plugin.springsecurity.cas.test
+
+import grails.plugin.springsecurity.annotation.Secured
+import org.apereo.cas.client.authentication.AttributePrincipal
+import org.springframework.security.cas.authentication.CasAuthenticationToken
+import org.springframework.security.core.Authentication
+import org.springframework.security.core.context.SecurityContextHolder
+
+class SecureController {
+
Review Comment:
Fixed in 2391490, 4 spaces per `.editorconfig`. `git diff
--ignore-all-space` on that file is empty, so it's purely whitespace.
##########
grails-test-examples/spring-security/cas/test1/src/integration-test/groovy/grails/plugin/springsecurity/cas/test/CasSingleSignOutSpec.groovy:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package grails.plugin.springsecurity.cas.test
+
+import spock.lang.IgnoreIf
+
+import java.net.http.HttpResponse
+
+/**
+ * Covers single sign-out, which the plugin enables by default via {@code
cas.useSingleSignout} by
Review Comment:
Reworded in 2391490 to "which the app opts into via `cas.useSingleSignout`",
and added a pointer to `CasNoSingleSignOutSpec` as the spec covering the
default.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]