Skip to content

Commit

Permalink
Set the mfaSecretKey to null even before returning it when using the …
Browse files Browse the repository at this point in the history
…console module
  • Loading branch information
MDeLuise authored and Coduz committed Jun 18, 2021
1 parent d6b698c commit 1e49b7e
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ public void onFailure(Throwable caught) {
public void onSuccess(final GwtXSRFToken xsrfToken) {
getButtonBar().disable();

keyEnabled = gwtMfaCredentialOptions != null && gwtMfaCredentialOptions.getAuthenticationKey() != null;
keyEnabled = gwtMfaCredentialOptions != null;
if (!keyEnabled) {
doMask(MSGS.maskEnableMfa());
// MFA is disabled, so enable it
Expand Down Expand Up @@ -488,7 +488,7 @@ public void onSuccess(GwtMfaCredentialOptions gwtMfaCredentialOptions) {
}

private void updateUIComponents(GwtMfaCredentialOptions gwtMfaCredentialOptions) {
boolean multiFactorAuth = gwtMfaCredentialOptions != null && gwtMfaCredentialOptions.getAuthenticationKey() != null;
boolean multiFactorAuth = gwtMfaCredentialOptions != null;
boolean hasCredentialWrite = currentSession.hasPermission(CredentialSessionPermission.write());
boolean hasCredentialDelete = currentSession.hasPermission(CredentialSessionPermission.delete());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,17 @@ public GwtMfaCredentialOptions find(String scopeIdStr, String mfaCredentialOptio

@Override
public MfaOption call() throws Exception {
return MFA_OPTION_SERVICE.find(scopeId, mfaCredentialOptionsId);
return setSecretKeyNullIfObjExists(MFA_OPTION_SERVICE.find(scopeId, mfaCredentialOptionsId));
}

});
} else {
mfaCredentialOption = MFA_OPTION_SERVICE.find(scopeId, mfaCredentialOptionsId);
}
return mfaCredentialOption != null ? KapuaGwtAuthenticationModelConverter.convertMfaCredentialOptions(mfaCredentialOption) : null;

return mfaCredentialOption != null ?
KapuaGwtAuthenticationModelConverter.convertMfaCredentialOptions(setSecretKeyNullIfObjExists(mfaCredentialOption)) :
null;
} catch (Exception ex) {
KapuaExceptionHandler.handle(ex);
return null;
Expand All @@ -101,14 +104,18 @@ public GwtMfaCredentialOptions findByUserId(String scopeIdStr, String userIdStr,

@Override
public MfaOption call() throws Exception {
return MFA_OPTION_SERVICE.findByUserId(scopeId, userId);
return setSecretKeyNullIfObjExists(MFA_OPTION_SERVICE.findByUserId(scopeId, userId));
}

});
} else {
mfaCredentialOption = MFA_OPTION_SERVICE.findByUserId(scopeId, userId);
}
return mfaCredentialOption != null ? KapuaGwtAuthenticationModelConverter.convertMfaCredentialOptions(mfaCredentialOption) : null;

return mfaCredentialOption != null ?
KapuaGwtAuthenticationModelConverter.convertMfaCredentialOptions(setSecretKeyNullIfObjExists(mfaCredentialOption)) :
null;

} catch (Exception ex) {
KapuaExceptionHandler.handle(ex);
return null;
Expand Down Expand Up @@ -141,7 +148,9 @@ public MfaOption call() throws Exception {
} else {
mfaCredentialOptions = MFA_OPTION_SERVICE.create(mfaCredentialOptionCreator);
}
gwtMfaCredentialOptions = KapuaGwtAuthenticationModelConverter.convertMfaCredentialOptions(mfaCredentialOptions);

mfaCredentialOptions.setMfaSecretKey(null);
gwtMfaCredentialOptions = KapuaGwtAuthenticationModelConverter.convertMfaCredentialOptions(setSecretKeyNullIfObjExists(mfaCredentialOptions));
} catch (Throwable t) {
KapuaExceptionHandler.handle(t);
}
Expand Down Expand Up @@ -176,4 +185,16 @@ public void run() throws Exception {
}
}

/**
* Useful in order to improve security. When the secret key is not needed to be send,
* then it's better to apply this function. If not applied, a user who is sniffing the
* data send by/to the console can view the secret key.
*/
private MfaOption setSecretKeyNullIfObjExists(MfaOption mfaCredentialOption) {
if (mfaCredentialOption != null) {
mfaCredentialOption.setMfaSecretKey(null);
}
return mfaCredentialOption;
}

}

0 comments on commit 1e49b7e

Please sign in to comment.