Skip to content

Commit cf06871

Browse files
jzheauxjzheaux
committed
Rework Logout Validation Logic
This commit simplifies the code, favoring the construction of a list of error codes over composing a set of consumers. This will simplify future analysis by making the code more readable. Signed-off-by: Josh Cummings <3627351+jzheaux@users.noreply.github.com>
1 parent e50c2a6 commit cf06871

10 files changed

Lines changed: 507 additions & 310 deletions

File tree

‎saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutRequestValidator.java‎

Lines changed: 59 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@
1616

1717
package org.springframework.security.saml2.provider.service.authentication.logout;
1818

19+
import java.util.ArrayList;
1920
import java.util.Collection;
20-
import java.util.function.Consumer;
21+
import java.util.Collections;
2122

2223
import org.opensaml.saml.saml2.core.LogoutRequest;
2324
import org.opensaml.saml.saml2.core.NameID;
@@ -56,97 +57,88 @@ public Saml2LogoutValidatorResult validate(Saml2LogoutRequestValidatorParameters
5657
LogoutRequest logoutRequest = this.saml.deserialize(Saml2Utils.withEncoded(request.getSamlRequest())
5758
.inflate(request.getBinding() == Saml2MessageBinding.REDIRECT)
5859
.decode());
59-
return Saml2LogoutValidatorResult.withErrors()
60-
.errors(verifySignature(request, logoutRequest, registration))
61-
.errors(validateRequest(logoutRequest, registration, authentication))
62-
.build();
60+
Collection<Saml2Error> errors = verifySignature(request, logoutRequest, registration);
61+
if (!errors.isEmpty()) {
62+
return Saml2LogoutValidatorResult.withErrors(errors.toArray(Saml2Error[]::new)).build();
63+
}
64+
errors = validateRequest(logoutRequest, registration, authentication);
65+
return errors.isEmpty() ? Saml2LogoutValidatorResult.success()
66+
: Saml2LogoutValidatorResult.withErrors(errors.toArray(Saml2Error[]::new)).build();
6367
}
6468

65-
private Consumer<Collection<Saml2Error>> verifySignature(Saml2LogoutRequest request, LogoutRequest logoutRequest,
69+
private Collection<Saml2Error> verifySignature(Saml2LogoutRequest request, LogoutRequest logoutRequest,
6670
RelyingPartyRegistration registration) {
6771
AssertingPartyMetadata details = registration.getAssertingPartyMetadata();
6872
Collection<Saml2X509Credential> credentials = details.getVerificationX509Credentials();
6973
VerificationConfigurer verify = this.saml.withVerificationKeys(credentials).entityId(details.getEntityId());
70-
return (errors) -> {
71-
if (logoutRequest.isSigned()) {
72-
errors.addAll(verify.verify(logoutRequest));
73-
}
74-
else {
75-
RedirectParameters params = new RedirectParameters(request.getParameters(),
76-
request.getParametersQuery(), logoutRequest);
77-
errors.addAll(verify.verify(params));
78-
}
79-
};
74+
if (logoutRequest.isSigned()) {
75+
return verify.verify(logoutRequest);
76+
}
77+
RedirectParameters params = new RedirectParameters(request.getParameters(), request.getParametersQuery(),
78+
logoutRequest);
79+
return verify.verify(params);
8080
}
8181

82-
private Consumer<Collection<Saml2Error>> validateRequest(LogoutRequest request,
83-
RelyingPartyRegistration registration, Authentication authentication) {
84-
return (errors) -> {
85-
validateIssuer(request, registration).accept(errors);
86-
validateDestination(request, registration).accept(errors);
87-
validateSubject(request, registration, authentication).accept(errors);
88-
};
82+
private Collection<Saml2Error> validateRequest(LogoutRequest request, RelyingPartyRegistration registration,
83+
Authentication authentication) {
84+
Collection<Saml2Error> errors = new ArrayList<>();
85+
errors.addAll(validateIssuer(request, registration));
86+
errors.addAll(validateDestination(request, registration));
87+
errors.addAll(validateSubject(request, registration, authentication));
88+
return errors;
8989
}
9090

91-
private Consumer<Collection<Saml2Error>> validateIssuer(LogoutRequest request,
92-
RelyingPartyRegistration registration) {
93-
return (errors) -> {
94-
if (request.getIssuer() == null) {
95-
errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to find issuer in LogoutRequest"));
96-
return;
97-
}
98-
String issuer = request.getIssuer().getValue();
99-
if (!issuer.equals(registration.getAssertingPartyMetadata().getEntityId())) {
100-
errors
101-
.add(new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to match issuer to configured issuer"));
102-
}
103-
};
91+
private Collection<Saml2Error> validateIssuer(LogoutRequest request, RelyingPartyRegistration registration) {
92+
if (request.getIssuer() == null) {
93+
return Collections.singletonList(
94+
new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to find issuer in LogoutRequest"));
95+
}
96+
String issuer = request.getIssuer().getValue();
97+
if (!issuer.equals(registration.getAssertingPartyMetadata().getEntityId())) {
98+
return Collections.singletonList(
99+
new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to match issuer to configured issuer"));
100+
}
101+
return Collections.emptyList();
104102
}
105103

106-
private Consumer<Collection<Saml2Error>> validateDestination(LogoutRequest request,
107-
RelyingPartyRegistration registration) {
108-
return (errors) -> {
109-
if (request.getDestination() == null) {
110-
errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION,
111-
"Failed to find destination in LogoutRequest"));
112-
return;
113-
}
114-
String destination = request.getDestination();
115-
if (!destination.equals(registration.getSingleLogoutServiceLocation())) {
116-
errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION,
117-
"Failed to match destination to configured destination"));
118-
}
119-
};
104+
private Collection<Saml2Error> validateDestination(LogoutRequest request, RelyingPartyRegistration registration) {
105+
if (request.getDestination() == null) {
106+
return Collections.singletonList(
107+
new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION, "Failed to find destination in LogoutRequest"));
108+
}
109+
String destination = request.getDestination();
110+
if (!destination.equals(registration.getSingleLogoutServiceLocation())) {
111+
return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION,
112+
"Failed to match destination to configured destination"));
113+
}
114+
return Collections.emptyList();
120115
}
121116

122-
private Consumer<Collection<Saml2Error>> validateSubject(LogoutRequest request,
123-
RelyingPartyRegistration registration, Authentication authentication) {
124-
return (errors) -> {
125-
if (authentication == null) {
126-
return;
127-
}
128-
NameID nameId = getNameId(request, registration);
129-
if (nameId == null) {
130-
errors
131-
.add(new Saml2Error(Saml2ErrorCodes.SUBJECT_NOT_FOUND, "Failed to find subject in LogoutRequest"));
132-
return;
133-
}
134-
135-
validateNameId(nameId, authentication, errors);
136-
};
117+
private Collection<Saml2Error> validateSubject(LogoutRequest request, RelyingPartyRegistration registration,
118+
Authentication authentication) {
119+
if (authentication == null) {
120+
return Collections.emptyList();
121+
}
122+
NameID nameId = getNameId(request, registration);
123+
if (nameId == null) {
124+
return Collections.singletonList(
125+
new Saml2Error(Saml2ErrorCodes.SUBJECT_NOT_FOUND, "Failed to find subject in LogoutRequest"));
126+
}
127+
return validateNameId(nameId, authentication);
137128
}
138129

139130
private NameID getNameId(LogoutRequest request, RelyingPartyRegistration registration) {
140131
this.saml.withDecryptionKeys(registration.getDecryptionX509Credentials()).decrypt(request);
141132
return request.getNameID();
142133
}
143134

144-
private void validateNameId(NameID nameId, Authentication authentication, Collection<Saml2Error> errors) {
135+
private Collection<Saml2Error> validateNameId(NameID nameId, Authentication authentication) {
145136
String name = nameId.getValue();
146137
if (!name.equals(authentication.getName())) {
147-
errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_REQUEST,
138+
return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_REQUEST,
148139
"Failed to match subject in LogoutRequest with currently logged in user"));
149140
}
141+
return Collections.emptyList();
150142
}
151143

152144
}

‎saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/logout/BaseOpenSamlLogoutResponseValidator.java‎

Lines changed: 74 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@
1616

1717
package org.springframework.security.saml2.provider.service.authentication.logout;
1818

19+
import java.util.ArrayList;
1920
import java.util.Collection;
20-
import java.util.function.Consumer;
21+
import java.util.Collections;
2122

2223
import org.opensaml.saml.saml2.core.LogoutResponse;
2324
import org.opensaml.saml.saml2.core.StatusCode;
@@ -52,101 +53,90 @@ public Saml2LogoutValidatorResult validate(Saml2LogoutResponseValidatorParameter
5253
LogoutResponse logoutResponse = this.saml.deserialize(Saml2Utils.withEncoded(response.getSamlResponse())
5354
.inflate(response.getBinding() == Saml2MessageBinding.REDIRECT)
5455
.decode());
55-
return Saml2LogoutValidatorResult.withErrors()
56-
.errors(verifySignature(response, logoutResponse, registration))
57-
.errors(validateRequest(logoutResponse, registration))
58-
.errors(validateLogoutRequest(logoutResponse, request.getId()))
59-
.build();
56+
Collection<Saml2Error> errors = verifySignature(response, logoutResponse, registration);
57+
if (!errors.isEmpty()) {
58+
return Saml2LogoutValidatorResult.withErrors(errors.toArray(Saml2Error[]::new)).build();
59+
}
60+
errors = validateRequest(logoutResponse, registration, request.getId());
61+
return errors.isEmpty() ? Saml2LogoutValidatorResult.success()
62+
: Saml2LogoutValidatorResult.withErrors(errors.toArray(Saml2Error[]::new)).build();
6063
}
6164

62-
private Consumer<Collection<Saml2Error>> verifySignature(Saml2LogoutResponse response,
63-
LogoutResponse logoutResponse, RelyingPartyRegistration registration) {
64-
return (errors) -> {
65-
AssertingPartyMetadata details = registration.getAssertingPartyMetadata();
66-
Collection<Saml2X509Credential> credentials = details.getVerificationX509Credentials();
67-
VerificationConfigurer verify = this.saml.withVerificationKeys(credentials)
68-
.entityId(details.getEntityId())
69-
.entityId(details.getEntityId());
70-
if (logoutResponse.isSigned()) {
71-
errors.addAll(verify.verify(logoutResponse));
72-
}
73-
else {
74-
RedirectParameters params = new RedirectParameters(response.getParameters(),
75-
response.getParametersQuery(), logoutResponse);
76-
errors.addAll(verify.verify(params));
77-
}
78-
};
65+
private Collection<Saml2Error> verifySignature(Saml2LogoutResponse response, LogoutResponse logoutResponse,
66+
RelyingPartyRegistration registration) {
67+
AssertingPartyMetadata details = registration.getAssertingPartyMetadata();
68+
Collection<Saml2X509Credential> credentials = details.getVerificationX509Credentials();
69+
VerificationConfigurer verify = this.saml.withVerificationKeys(credentials).entityId(details.getEntityId());
70+
if (logoutResponse.isSigned()) {
71+
return verify.verify(logoutResponse);
72+
}
73+
RedirectParameters params = new RedirectParameters(response.getParameters(), response.getParametersQuery(),
74+
logoutResponse);
75+
return verify.verify(params);
7976
}
8077

81-
private Consumer<Collection<Saml2Error>> validateRequest(LogoutResponse response,
82-
RelyingPartyRegistration registration) {
83-
return (errors) -> {
84-
validateIssuer(response, registration).accept(errors);
85-
validateDestination(response, registration).accept(errors);
86-
validateStatus(response).accept(errors);
87-
};
78+
private Collection<Saml2Error> validateRequest(LogoutResponse response, RelyingPartyRegistration registration,
79+
String logoutRequestId) {
80+
Collection<Saml2Error> errors = new ArrayList<>();
81+
errors.addAll(validateIssuer(response, registration));
82+
errors.addAll(validateDestination(response, registration));
83+
errors.addAll(validateStatus(response));
84+
errors.addAll(validateLogoutRequest(response, logoutRequestId));
85+
return errors;
8886
}
8987

90-
private Consumer<Collection<Saml2Error>> validateIssuer(LogoutResponse response,
91-
RelyingPartyRegistration registration) {
92-
return (errors) -> {
93-
if (response.getIssuer() == null) {
94-
errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to find issuer in LogoutResponse"));
95-
return;
96-
}
97-
String issuer = response.getIssuer().getValue();
98-
if (!issuer.equals(registration.getAssertingPartyMetadata().getEntityId())) {
99-
errors
100-
.add(new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to match issuer to configured issuer"));
101-
}
102-
};
88+
private Collection<Saml2Error> validateIssuer(LogoutResponse response, RelyingPartyRegistration registration) {
89+
if (response.getIssuer() == null) {
90+
return Collections.singletonList(
91+
new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to find issuer in LogoutResponse"));
92+
}
93+
String issuer = response.getIssuer().getValue();
94+
if (!issuer.equals(registration.getAssertingPartyMetadata().getEntityId())) {
95+
return Collections.singletonList(
96+
new Saml2Error(Saml2ErrorCodes.INVALID_ISSUER, "Failed to match issuer to configured issuer"));
97+
}
98+
return Collections.emptyList();
10399
}
104100

105-
private Consumer<Collection<Saml2Error>> validateDestination(LogoutResponse response,
106-
RelyingPartyRegistration registration) {
107-
return (errors) -> {
108-
if (response.getDestination() == null) {
109-
errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION,
110-
"Failed to find destination in LogoutResponse"));
111-
return;
112-
}
113-
String destination = response.getDestination();
114-
if (!destination.equals(registration.getSingleLogoutServiceResponseLocation())) {
115-
errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION,
116-
"Failed to match destination to configured destination"));
117-
}
118-
};
101+
private Collection<Saml2Error> validateDestination(LogoutResponse response, RelyingPartyRegistration registration) {
102+
if (response.getDestination() == null) {
103+
return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION,
104+
"Failed to find destination in LogoutResponse"));
105+
}
106+
String destination = response.getDestination();
107+
if (!destination.equals(registration.getSingleLogoutServiceResponseLocation())) {
108+
return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_DESTINATION,
109+
"Failed to match destination to configured destination"));
110+
}
111+
return Collections.emptyList();
119112
}
120113

121-
private Consumer<Collection<Saml2Error>> validateStatus(LogoutResponse response) {
122-
return (errors) -> {
123-
if (response.getStatus() == null) {
124-
return;
125-
}
126-
if (response.getStatus().getStatusCode() == null) {
127-
return;
128-
}
129-
if (StatusCode.SUCCESS.equals(response.getStatus().getStatusCode().getValue())) {
130-
return;
131-
}
132-
if (StatusCode.PARTIAL_LOGOUT.equals(response.getStatus().getStatusCode().getValue())) {
133-
return;
134-
}
135-
errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE, "Response indicated logout failed"));
136-
};
114+
private Collection<Saml2Error> validateStatus(LogoutResponse response) {
115+
if (response.getStatus() == null) {
116+
return Collections.emptyList();
117+
}
118+
if (response.getStatus().getStatusCode() == null) {
119+
return Collections.emptyList();
120+
}
121+
if (StatusCode.SUCCESS.equals(response.getStatus().getStatusCode().getValue())) {
122+
return Collections.emptyList();
123+
}
124+
if (StatusCode.PARTIAL_LOGOUT.equals(response.getStatus().getStatusCode().getValue())) {
125+
return Collections.emptyList();
126+
}
127+
return Collections
128+
.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE, "Response indicated logout failed"));
137129
}
138130

139-
private Consumer<Collection<Saml2Error>> validateLogoutRequest(LogoutResponse response, String id) {
140-
return (errors) -> {
141-
if (response.getInResponseTo() == null) {
142-
return;
143-
}
144-
if (response.getInResponseTo().equals(id)) {
145-
return;
146-
}
147-
errors.add(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE,
148-
"LogoutResponse InResponseTo doesn't match ID of associated LogoutRequest"));
149-
};
131+
private Collection<Saml2Error> validateLogoutRequest(LogoutResponse response, String id) {
132+
if (response.getInResponseTo() == null) {
133+
return Collections.emptyList();
134+
}
135+
if (response.getInResponseTo().equals(id)) {
136+
return Collections.emptyList();
137+
}
138+
return Collections.singletonList(new Saml2Error(Saml2ErrorCodes.INVALID_RESPONSE,
139+
"LogoutResponse InResponseTo doesn't match ID of associated LogoutRequest"));
150140
}
151141

152142
}

0 commit comments

Comments
 (0)