Skip to content

Commit 85d4d04

Browse files
authored
Merge pull request #1110 from OpenConext/fix/794-reject-app-after-linked-institution
#794 Moves the MFA check after the ACR check
2 parents a39607c + 8bb320a commit 85d4d04

2 files changed

Lines changed: 46 additions & 5 deletions

File tree

myconext-server/src/main/java/myconext/security/GuestIdpAuthenticationRequestFilter.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -730,17 +730,18 @@ private void sendAssertion(HttpServletRequest request,
730730
} else {
731731
authnContextClassRefValue = ACR.selectACR(authenticationContextClassReferences, hasStudentAffiliation);
732732
}
733-
} else if (samlAuthenticationRequest.isMfaProfileRequired()) {
733+
} else if (!applySsoMfa && !CollectionUtils.isEmpty(authenticationContextClassReferences)) {
734+
optionalMessage = String.format("The specified authentication context requirements '%s' cannot be met by the responder.",
735+
String.join(", ", authenticationContextClassReferences));
736+
samlStatus = SAMLStatus.NO_AUTHN_CONTEXT;
737+
}
738+
if (samlAuthenticationRequest.isMfaProfileRequired()) {
734739
if (samlAuthenticationRequest.isTiqrFlow() || applySsoMfa) {
735740
authnContextClassRefValue = ACR.selectACR(authenticationContextClassReferences, false);
736741
} else {
737742
optionalMessage = "The requesting service has indicated that a login with the eduID app is required to login.";
738743
samlStatus = SAMLStatus.NO_AUTHN_CONTEXT;
739744
}
740-
} else if (!applySsoMfa && !CollectionUtils.isEmpty(authenticationContextClassReferences)) {
741-
optionalMessage = String.format("The specified authentication context requirements '%s' cannot be met by the responder.",
742-
String.join(", ", authenticationContextClassReferences));
743-
samlStatus = SAMLStatus.NO_AUTHN_CONTEXT;
744745
}
745746
if (!samlStatus.equals(SAMLStatus.SUCCESS)) {
746747
authnContextClassRefValue = DefaultSAMLService.authnContextClassRefUnspecified;

myconext-server/src/test/java/myconext/api/UserControllerTest.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.apache.commons.io.IOUtils;
1717
import org.apache.http.client.CookieStore;
1818
import org.junit.Test;
19+
import org.opensaml.saml.saml2.core.StatusCode;
1920
import org.springframework.beans.factory.annotation.Autowired;
2021
import org.springframework.http.HttpHeaders;
2122
import org.springframework.http.HttpMethod;
@@ -197,6 +198,45 @@ public void accountLinkingAndMfa() throws IOException {
197198
assertTrue(samlResponse.contains(ACR.LINKED_INSTITUTION_MFA));
198199
}
199200

201+
@Test
202+
public void accountLinkingAndMfa_RejectMfa() throws IOException {
203+
String authnContext = readFile("request_authn_context_linked_institution_mfa.xml");
204+
Response response = samlAuthnRequestResponseWithLoa(null, "relay", authnContext);
205+
String authenticationRequestId = extractAuthenticationRequestIdFromAuthnResponse(response);
206+
207+
String location = response.getHeader("Location");
208+
assertTrue(location.contains("/login"));
209+
assertTrue(location.contains("stepup=true"));
210+
assertTrue(location.contains("mfa=true"));
211+
212+
// Linking institution
213+
User user = userRepository.findOneUserByEmail("mdoe@example.com");
214+
LinkedAccount linkedAccount = linkedAccount("John", "Doe", new Date());
215+
user.getLinkedAccounts().add(linkedAccount);
216+
userRepository.save(user);
217+
218+
ClientAuthenticationRequest clientAuthenticationRequest = new ClientAuthenticationRequest(authenticationRequestId, user, false, "repsonse");
219+
ClientAuthenticationResponse authenticationResponse = oneTimeLoginCodeRequest(clientAuthenticationRequest, HttpMethod.PUT);
220+
221+
SamlAuthenticationRequest samlAuthenticationRequest = authenticationRequestRepository
222+
.findById(authenticationResponse.authenticationRequestId).get();
223+
224+
Response magicResponse = given().redirects().follow(false)
225+
.when()
226+
.queryParam("h", samlAuthenticationRequest.getHash())
227+
.cookie(BROWSER_SESSION_COOKIE_NAME, "true")
228+
.get("/saml/guest-idp/magic");
229+
while (magicResponse.statusCode() == 302) {
230+
String redirectLocation = magicResponse.getHeader("Location");
231+
assertNotNull(redirectLocation);
232+
magicResponse = this.get302Response(magicResponse, Optional.empty(), "?force=true");
233+
}
234+
String samlResponse = samlAuthnResponse(magicResponse, Optional.empty());
235+
236+
assertTrue(samlResponse.contains(StatusCode.NO_AUTHN_CONTEXT));
237+
assertTrue(samlResponse.contains("The requesting service has indicated that a login with the eduID app is required to login."));
238+
}
239+
200240
@Test
201241
public void accountLinkingAndMfa_Flow() throws IOException {
202242
// Login

0 commit comments

Comments
 (0)