Skip to content

Commit d2566af

Browse files
HejdaJakubHejdaJakub
authored
Merge pull request #3975 from HejdaJakub/mfaTimestampFix
fix(core): MFA timeout
2 parents 0e19122 + b67c23f commit d2566af

File tree

5 files changed

+67
-32
lines changed

5 files changed

+67
-32
lines changed

‎perun-base/src/main/java/cz/metacentrum/perun/core/api/PerunPrincipal.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ public class PerunPrincipal {
2525
// Specifies if the principal has initialized authZResolver
2626
private volatile boolean authzInitialized = false;
2727
// Keywords of additionalInformations
28-
public static final String MFA_TIMESTAMP = "mfaTimestamp";
28+
public static final String AUTH_TIME = "authTime";
29+
public static final String ACR_MFA = "acrMfa";
2930
public static final String ISSUER = "issuer";
3031
public static final String ACCESS_TOKEN = "accessToken";
3132

‎perun-base/src/main/resources/perun-base.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@
180180
<prop key="perun.userInfoEndpoint.extSourceLogin">eduperson_unique_id, eduperson_principal_name, saml2_nameid_persistent, eduperson_targeted_id, voperson_external_id</prop>
181181
<prop key="perun.userInfoEndpoint.extSourceName">target_issuer</prop>
182182
<prop key="perun.userInfoEndpoint.extSourceFriendlyName">target_backend, display_name, text</prop>
183-
<prop key="perun.introspectionEndpoint.mfaAuthTimeout">24</prop>
183+
<prop key="perun.introspectionEndpoint.mfaAuthTimeout">1440</prop>
184184
<prop key="perun.introspectionEndpoint.mfaAcrValue">https://refeds.org/profile/mfa</prop>
185185
<prop key="perun.enforceMfa">false</prop>
186186
</props>

‎perun-core/src/main/java/cz/metacentrum/perun/core/api/exceptions/MfaTimeoutException.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package cz.metacentrum.perun.core.api.exceptions;
2+
3+
import cz.metacentrum.perun.core.api.exceptions.rt.PerunRuntimeException;
4+
5+
public class MfaTimeoutException extends PerunRuntimeException {
6+
static final long serialVersionUID = 0;
7+
8+
public MfaTimeoutException(String message) {
9+
super(message);
10+
}
11+
12+
public MfaTimeoutException(String message, Throwable cause) {
13+
super(message, cause);
14+
}
15+
16+
public MfaTimeoutException(Throwable cause) {
17+
super(cause);
18+
}
19+
}

‎perun-core/src/main/java/cz/metacentrum/perun/core/blImpl/AuthzResolverBlImpl.java‎

Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
import cz.metacentrum.perun.core.api.exceptions.MfaInvalidRolesException;
5656
import cz.metacentrum.perun.core.api.exceptions.MfaPrivilegeException;
5757
import cz.metacentrum.perun.core.api.exceptions.MfaRolePrivilegeException;
58+
import cz.metacentrum.perun.core.api.exceptions.MfaTimeoutException;
5859
import cz.metacentrum.perun.core.api.exceptions.PolicyNotExistsException;
5960
import cz.metacentrum.perun.core.api.exceptions.ResourceNotExistsException;
6061
import cz.metacentrum.perun.core.api.exceptions.RoleAlreadySetException;
@@ -100,8 +101,9 @@
100101

101102
import static cz.metacentrum.perun.core.api.AuthzResolver.MFA_CRITICAL_ATTR;
102103
import static cz.metacentrum.perun.core.api.PerunPrincipal.ACCESS_TOKEN;
104+
import static cz.metacentrum.perun.core.api.PerunPrincipal.ACR_MFA;
105+
import static cz.metacentrum.perun.core.api.PerunPrincipal.AUTH_TIME;
103106
import static cz.metacentrum.perun.core.api.PerunPrincipal.ISSUER;
104-
import static cz.metacentrum.perun.core.api.PerunPrincipal.MFA_TIMESTAMP;
105107
import static org.apache.commons.lang3.StringUtils.isBlank;
106108

107109
/**
@@ -2562,7 +2564,7 @@ public static synchronized void refreshAuthz(PerunSession sess) {
25622564
sess.getPerunPrincipal().getRoles().clear();
25632565
}
25642566

2565-
if (isAuthorizedByMfa(sess)) {
2567+
if (isAuthorizedByMfa(sess, false)) {
25662568
sess.getPerunPrincipal().getRoles().putAuthzRole(Role.MFA);
25672569
}
25682570
}
@@ -2623,10 +2625,6 @@ public static void refreshMfa(PerunSession sess) throws ExpiredTokenException, M
26232625
throw new MFAuthenticationException("MFA enforcement is turned off");
26242626
}
26252627

2626-
if (!BeansUtils.getCoreConfig().getRequestUserInfoEndpoint()) {
2627-
throw new MFAuthenticationException("Cannot verify MFA - UserInfo endpoint not configured.");
2628-
}
2629-
26302628
String accessToken = sess.getPerunPrincipal().getAdditionalInformations().get(ACCESS_TOKEN);
26312629
if (accessToken == null) {
26322630
throw new MFAuthenticationException("Cannot verify MFA - access token is missing.");
@@ -2637,7 +2635,7 @@ public static void refreshMfa(PerunSession sess) throws ExpiredTokenException, M
26372635
throw new MFAuthenticationException("Cannot verify MFA - issuer is missing.");
26382636
}
26392637

2640-
if (isAuthorizedByMfa(sess)) {
2638+
if (isAuthorizedByMfa(sess, true)) {
26412639
sess.getPerunPrincipal().getRoles().putAuthzRole(Role.MFA);
26422640
}
26432641
}
@@ -4356,34 +4354,47 @@ private static Map<String, Integer> createMappingOfValues(PerunBean complementar
43564354

43574355
/**
43584356
* Checks, if principal was authorized by Multi-factor authentication.
4359-
* The information is resolved in UserInfoEndpointCall and stored in principal's additionalInformations.
4360-
* The timestamp of MFA must not be older than mfa timeout set in config.
4357+
* The information is resolved from headers (apache IntrospectionEndpoint call) and stored in principal's additionalInformations.
4358+
* Check if the auth time + mfa timeout is not older than the current time
4359+
* auth time = time of the first authentication
4360+
* mfa timeout = amount of time defined in the config for how long the MFA should be valid (since SFA)
43614361
*
43624362
* @param sess session
4363+
* @param throwError if this method should throw errors or just return boolean
43634364
* @return true if principal authorized by MFA in allowed limit, false otherwise
43644365
*/
4365-
private static boolean isAuthorizedByMfa(PerunSession sess) {
4366+
private static boolean isAuthorizedByMfa(PerunSession sess, boolean throwError) {
43664367
if (!BeansUtils.getCoreConfig().isEnforceMfa()) {
43674368
return false;
43684369
}
43694370

4370-
if (!sess.getPerunPrincipal().getAdditionalInformations().containsKey(MFA_TIMESTAMP)) {
4371-
return false;
4372-
}
4373-
4374-
String returnedTimestamp = sess.getPerunPrincipal().getAdditionalInformations().get(MFA_TIMESTAMP);
4375-
Instant parsedReturnedTimestamp;
4371+
String returnedAuthTime = sess.getPerunPrincipal().getAdditionalInformations().get(AUTH_TIME);
4372+
Instant parsedReturnedAuthTime;
43764373
try {
4377-
parsedReturnedTimestamp = Instant.parse(returnedTimestamp);
4374+
parsedReturnedAuthTime = Instant.parse(returnedAuthTime);
43784375
} catch (DateTimeParseException e) {
4379-
throw new InternalErrorException("MFA timestamp " + returnedTimestamp + " could not be parsed", e);
4376+
throw new InternalErrorException("MFA timestamp " + returnedAuthTime + " could not be parsed", e);
43804377
}
4381-
if (parsedReturnedTimestamp.isAfter(Instant.now())) {
4382-
throw new InternalErrorException("MFA auth timestamp " + returnedTimestamp + " was greater than current time");
4378+
if (parsedReturnedAuthTime.isAfter(Instant.now())) {
4379+
throw new InternalErrorException("MFA auth timestamp " + returnedAuthTime + " was greater than current time");
43834380
}
43844381

4385-
long mfaTimeoutInSec = Duration.ofHours(BeansUtils.getCoreConfig().getMfaAuthTimeout()).getSeconds();
4386-
Instant mfaValidUntil = parsedReturnedTimestamp.plusSeconds(mfaTimeoutInSec);
4387-
return mfaValidUntil.isAfter(Instant.now());
4382+
long mfaTimeoutInSec = Duration.ofMinutes(BeansUtils.getCoreConfig().getMfaAuthTimeout()).getSeconds();
4383+
Instant mfaValidUntil = parsedReturnedAuthTime.plusSeconds(mfaTimeoutInSec);
4384+
4385+
// check if the auth time + mfa timeout > the current time
4386+
if (mfaValidUntil.isAfter(Instant.now())) {
4387+
// if user has MFA and it is still valid
4388+
return sess.getPerunPrincipal().getAdditionalInformations().containsKey(ACR_MFA);
4389+
} else {
4390+
if (!throwError) return false;
4391+
if (sess.getPerunPrincipal().getAdditionalInformations().containsKey(ACR_MFA)) {
4392+
// MFA is no longer valid
4393+
throw new MfaTimeoutException("Your MFA timestamp " + returnedAuthTime + " is not valid anymore, you'll need to reauthenticate");
4394+
} else {
4395+
// user is authenticated by SFA but the mfa timeout would cause an error, so we need to reauthenticate this user
4396+
throw new MfaTimeoutException("Your single factor authentication timestamp " + returnedAuthTime + " is not valid anymore, you'll need to reauthenticate");
4397+
}
4398+
}
43884399
}
43894400
}

‎perun-rpc/src/main/java/cz/metacentrum/perun/rpc/Api.java‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,9 @@
6767
import java.util.regex.Pattern;
6868

6969
import static cz.metacentrum.perun.core.api.PerunPrincipal.ACCESS_TOKEN;
70+
import static cz.metacentrum.perun.core.api.PerunPrincipal.ACR_MFA;
71+
import static cz.metacentrum.perun.core.api.PerunPrincipal.AUTH_TIME;
7072
import static cz.metacentrum.perun.core.api.PerunPrincipal.ISSUER;
71-
import static cz.metacentrum.perun.core.api.PerunPrincipal.MFA_TIMESTAMP;
7273
import static org.apache.commons.lang3.StringUtils.isEmpty;
7374
import static org.apache.commons.lang3.StringUtils.isNotEmpty;
7475

@@ -271,14 +272,17 @@ else if (isNotEmpty(req.getHeader(OIDC_CLAIM_SUB))) {
271272
}
272273
extSourceLoaString = "-1";
273274

274-
// get MFA timestamp
275+
// store auth_time to additional information
276+
String authTimestamp = req.getHeader(OIDC_CLAIM_AUTH_TIME);
277+
if (isNotEmpty(authTimestamp)) {
278+
Instant authReadableTimestamp = Instant.ofEpochSecond(Long.parseLong(authTimestamp));
279+
additionalInformations.put(AUTH_TIME, authReadableTimestamp.toString());
280+
}
281+
282+
// store MFA flag to additional information
275283
String acr = req.getHeader(OIDC_CLAIM_ACR);
276284
if (isNotEmpty(acr) && acr.equals(BeansUtils.getCoreConfig().getIntrospectionEndpointMfaAcrValue())) {
277-
String mfaTimestamp = req.getHeader(OIDC_CLAIM_AUTH_TIME);
278-
if (isNotEmpty(mfaTimestamp)) {
279-
Instant mfaReadableTimestamp = Instant.ofEpochSecond(Long.parseLong(mfaTimestamp));
280-
additionalInformations.put(MFA_TIMESTAMP, mfaReadableTimestamp.toString());
281-
}
285+
additionalInformations.put(ACR_MFA, "mfa");
282286
}
283287

284288
if (BeansUtils.getCoreConfig().getRequestUserInfoEndpoint()) {

0 commit comments

Comments
 (0)