Add an optional consent modal before login#3792
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an opt-in, configurable pre-login consent/notice modal to the UAA login flow (per identity zone), including server-side config validation, model wiring, and a client-side cookie-based “remember acceptance” mechanism.
Changes:
- Introduces
branding.loginConsentconfiguration (model + validator) and wires it into the login page model (LoginInfoEndpoint). - Adds login page UI (Thymeleaf modal markup) + new CSS/JS to display a blocking consent modal and persist acceptance in a cookie.
- Adds documentation and test coverage for hashing/duration parsing and zone validation behavior.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
model/src/main/java/.../LoginConsent.java |
New zone-config model for login consent (title/text/buttons/link/duration). |
model/src/main/java/.../BrandingInformation.java |
Adds loginConsent to branding configuration. |
server/src/main/java/.../LoginInfoEndpoint.java |
Adds consent config/hash/duration to the login page model. |
server/src/main/java/.../LoginConsentHashUtil.java |
Computes consent content hash + parses duration to seconds. |
server/src/main/java/.../LoginConsentValidator.java |
Validates required fields, duration, and decline URL. |
server/src/main/java/.../GeneralIdentityZoneConfigurationValidator.java |
Invokes login-consent validation when saving zone config. |
server/src/main/resources/templates/web/login.html |
Conditionally includes consent CSS/JS and renders the modal. |
uaa/src/main/webapp/resources/.../login-consent.css |
Styles for the modal. |
uaa/src/main/webapp/resources/.../consent.js |
Modal show/hide logic and consent cookie handling. |
docs/login-consent.md |
End-user/operator documentation for the feature. |
server/src/test/java/.../LoginConsentHashUtilTest.java |
Tests for hash + duration parsing behavior. |
server/src/test/java/.../LoginConsentValidatorTest.java |
Tests for config validation behavior. |
server/src/test/java/.../LoginInfoEndpointTests.java |
Tests ensuring consent config is added/omitted in the login model. |
model/src/test/java/.../LoginConsentTest.java |
Model tests (currently disabled). |
uaa/src/main/resources/uaa.yml |
Adds commented example configuration. |
settings.gradle |
Renames rootProject.name (unrelated to feature). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpoint.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginConsentHashUtil.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginConsentHashUtil.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/cloudfoundry/identity/uaa/login/LoginConsentHashUtil.java
Show resolved
Hide resolved
model/src/test/java/org/cloudfoundry/identity/uaa/zone/LoginConsentTest.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/cloudfoundry/identity/uaa/zone/LoginConsentValidatorTest.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java
Show resolved
Hide resolved
server/src/test/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java
Outdated
Show resolved
Hide resolved
1d79054 to
d32eaf0
Compare
adb326e to
24e7fd5
Compare
| String title = StringUtils.hasText(consent.getTitle()) ? consent.getTitle() : ""; | ||
| String text = StringUtils.hasText(consent.getText()) ? consent.getText() : ""; | ||
|
|
||
| String content = title + "|" + text; |
There was a problem hiding this comment.
In theory, there is the case where the values
title: 'abc|def'
content: 'ghi'
is changed to
title: 'abc'
content: 'def|ghi'
and would be considered equal.
(But I guess we can ignore that case)
| # <li>All communications are subject to routine monitoring, | ||
| # interception, and search, and may be disclosed or used for any purpose.</li> | ||
| # </ul> | ||
| # acceptButtonText: "I Accept" |
There was a problem hiding this comment.
Do we also need a configurable text for the decline button?
Implements #3692