Skip to content

AB#117049 Switch to Spring Boot method of setting SameSite#216

Merged
buckett merged 1 commit intomasterfrom
AB#117049
Apr 29, 2026
Merged

AB#117049 Switch to Spring Boot method of setting SameSite#216
buckett merged 1 commit intomasterfrom
AB#117049

Conversation

@buckett
Copy link
Copy Markdown
Member

@buckett buckett commented Apr 27, 2026

This drops our code so we just set the standard property.

This drops our code so we just set the standard property.
Copilot AI review requested due to automatic review settings April 27, 2026 15:05
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR switches SameSite cookie configuration from a custom embedded-Tomcat cookie processor to Spring Boot’s standard configuration property, aiming to support cross-origin OAuth flows.

Changes:

  • Configure session cookie SameSite via server.servlet.session.cookie.same-site=None.
  • Remove the custom TomcatConfiguration that previously set SameSite=None through Tomcat’s Rfc6265CookieProcessor.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/main/resources/application.properties Adds Spring Boot property to set session cookie SameSite to None.
src/main/java/uk/ac/ox/ctl/ltiauth/TomcatConfiguration.java Deletes Tomcat-specific SameSite cookie processor customization in favor of property-based config.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +66 to +67
# This is needed for the OAuth flow so that the cookie is sent in cross-origin requests.
server.servlet.session.cookie.same-site=None
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SameSite=None cookies are ignored/rejected by modern browsers unless they also have the Secure attribute. This property is set in the base application.properties, but server.servlet.session.cookie.secure=true is only configured in application-aws.properties, so the comment about cross-origin OAuth may not hold for non-aws profiles (e.g. docker/local) and could break the flow in those environments. Consider scoping same-site=None to the same profiles where cookie.secure (and HTTPS / forwarded-proto handling) are guaranteed, or explicitly document/enforce the Secure requirement for all deployments that rely on cross-site cookies.

Suggested change
# This is needed for the OAuth flow so that the cookie is sent in cross-origin requests.
server.servlet.session.cookie.same-site=None
# Default to a browser-compatible setting for non-HTTPS/local profiles.
# Profiles that require cross-origin OAuth must override this to `None` and also set
# `server.servlet.session.cookie.secure=true` so modern browsers will accept the cookie.
server.servlet.session.cookie.same-site=Lax

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@sebastianchristopher sebastianchristopher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@buckett buckett merged commit 0bd2e16 into master Apr 29, 2026
7 checks passed
@buckett buckett deleted the AB#117049 branch April 29, 2026 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants