Skip to content

Identity Zone Selection via X-Zid Header #3427

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
adrianhoelzl-sap wants to merge 16 commits into
base: develop
Choose a base branch
from
Open

Identity Zone Selection via X-Zid Header #3427

adrianhoelzl-sap wants to merge 16 commits into from

Conversation

@adrianhoelzl-sap
Copy link
Contributor

@adrianhoelzl-sap adrianhoelzl-sap commented Apr 29, 2025

No description provided.

@adrianhoelzl-sap adrianhoelzl-sap requested review from a team and tack-sap April 29, 2025 09:22
@fhanik
Copy link
Contributor

fhanik commented Apr 30, 2025

@adrianhoelzl-sap How does this impact cookies and security?

What If I do a request like this

GET /some/path
Host: some.host.com
Header: X-Zid=zone-id-for-someother.host.com

and this sets up my cookies (including authentication stored in the session)
and then I do this request (and continue using the same JSESSIONID since the host hasn't changed)

GET /some/path
Host: some.host.com

Do I just gain access across zones?

@adrianhoelzl-sap
Copy link
Contributor Author

adrianhoelzl-sap commented May 1, 2025

@adrianhoelzl-sap How does this impact cookies and security?

What If I do a request like this

GET /some/path
Host: some.host.com
Header: X-Zid=zone-id-for-someother.host.com

and this sets up my cookies (including authentication stored in the session) and then I do this request (and continue using the same JSESSIONID since the host hasn't changed)

GET /some/path
Host: some.host.com

Do I just gain access across zones?

This scenario should be addressed by this check here:

uaa/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/SessionResetFilter.java

Line 60 in df50723

if (!Objects.equals(IdentityZoneHolder.getCurrentZoneId(), authentication.getPrincipal().getZoneId())) {

The zone resolved through the subdomain or X-Zid header (IdentityZoneHolder.getCurrentIdentityZoneId()) is compared to the zone for which the session was set up (authentication.getPrincipal().getZoneId()). If they do not match, the session is invalidated.

@fhanik
Copy link
Contributor

fhanik commented May 8, 2025

The SessionResetFilter is added at the end of each filter chain. it is the last filter that is used before an MVC endpoint is invoked.

But all the filters before that, that may contain logic and send redirects prior to the completion of the filter chain are still vulnerable to this.

A lot of the SAML/OAuth/OIDC happens in just filters, and not in MVC endpoints, and those filters would be working under incorrect assumptions.

Given the risk, is this header really needed?

If the HTTP client can set a header, it can set the "Host" header, and if there is a browser involved, cookie handling will be accurate.

strehle
strehle reviewed May 27, 2025
View reviewed changes
Copy link
Member

@strehle strehle left a comment

Choose a reason for hiding this comment

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

My hope was that we can consolidate the amount of filter (per each request) and optimize but can we integration this logic into one of other IdentityZoneXYZ filters ? ... maybe rename one and do more ?

@adrianhoelzl-sap
Refactor IdentityZoneResolvingFilter.doFilterInternal
a5f2cf1
@adrianhoelzl-sap
Add reading of 'X-Zid' header
4c69c39
@adrianhoelzl-sap
Add distinction between lookup of zone by ID and by subdomain
5f8fa30
@adrianhoelzl-sap
Refactor initialization of DAO in IdentityZoneResolvingFilterTests
af0d726
@adrianhoelzl-sap
Add unit tests for X-Zid header
6400d42
@adrianhoelzl-sap
Add description of X-Zid header to overview section of API documentation
ffc8c16
@adrianhoelzl-sap
Add toggle for X-Zid header
527b2a6
@adrianhoelzl-sap
Add zidHeaderEnabled field to IdentityZoneResolvingFilter
35b048e
@adrianhoelzl-sap
Add check for flag being enabled to filter
eb9c21e
@adrianhoelzl-sap
Add IdentityZoneMismatchCheckFilter
8776e49
@adrianhoelzl-sap
Add IdentityZoneMismatchCheckFilter to filter chain
72bd132
@adrianhoelzl-sap
Remove IdZ check from SessionResetFilter
4a2d978
@adrianhoelzl-sap
Add unit test for IdentityZoneMismatchCheckFilter
1868852
@adrianhoelzl-sap
Add MockMvc tests for X-Zid header behavior
2b8cf32
@adrianhoelzl-sap
Add integration tests for checking invalidation of session when using…
0d90cfa 
… session cookie for wrong zone
strehle
strehle reviewed Jun 5, 2025
View reviewed changes
scripts/cargo/uaa.yml
- excluded-claim1
- excluded-claim2
login:
zidHeaderEnabled: true
Copy link
Member

@strehle strehle Jun 5, 2025

Choose a reason for hiding this comment

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

I checked docs and we have X-Identity-Zone-Id and X-Identity-Zone-Subdomain already , so what is X-zid in comparison ?

The header allows new possiblities, correct ? then we should have thing to tell admin, e.g.
allowZoneSwitchByHeader or allowZoneSwitchByZidHeader

server/src/main/java/org/cloudfoundry/identity/uaa/zone/IdentityZoneMismatchCheckFilter.java

/**
* Checks whether there is a mismatch between ...
* <ul>
Copy link
Member

@strehle strehle Jun 5, 2025

Choose a reason for hiding this comment

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

What is the difference to X-Identity-Zone-Id, X-Identity-Zone-Subdomain ?

Copy link
Contributor Author

@adrianhoelzl-sap adrianhoelzl-sap Jun 12, 2025

Choose a reason for hiding this comment

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

With the subdomain or the X-Zid header, one selects the identity zone to log in to. With the other two headers, IdZ switching can be performed (only possible if logged in to "uaa" zone).

Example for IdZ switching:

  • user has a group zones.custom.scim.read in the "uaa" zone
  • log in to "uaa" zone (here, the X-Zid can be used as an override), receive token
  • use the token for performing actions according to the scim.read scope in the zone with the ID "custom"
    • In the SCIM requests, he would then need to pass the X-Identity-Zone-Id header with the value "custom"

...src/test/java/org/cloudfoundry/identity/uaa/mock/zones/IdentityZoneResolvingMockMvcTest.java
void subdomainSetToZone1_ZidHeaderSetToZone2_BothZonesExist_ShouldReturnZone2() throws Exception {
mockMvc.perform(
get("/login")
.header("Host", zone1Subdomain + "." + HOST_NO_SUBDOMAIN)
Copy link
Member

@strehle strehle Jun 5, 2025

Choose a reason for hiding this comment

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

this means, even if we have a subdomain but if there is a X-Zid we overrule the subdomain ?

Copy link
Contributor Author

@adrianhoelzl-sap adrianhoelzl-sap Jun 12, 2025

Choose a reason for hiding this comment

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

Yes, the X-Zid header overrides the zone specified by the subdomain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@strehle strehle strehle left review comments

@tack-sap tack-sap Awaiting requested review from tack-sap

@fhanik fhanik Awaiting requested review from fhanik

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

Foundational Infrastructure Working Group
Status: Inbox

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@adrianhoelzl-sap @fhanik @strehle