feat: entropy of user password is now checked by attributes passed from authsource
Created by: xpavlic
Merge request reports
Activity
45 46 return AuthSwitcher::DEFAULT_REQUESTED_CONTEXTS; 46 47 } 47 48 $supportedRequestedContexts = array_values(array_intersect($requestedContexts, AuthSwitcher::SUPPORTED)); 49 if (!$sfaEntropy) { 247 277 } 248 278 } 249 279 } 250 $result[] = AuthSwitcher::SFA; 280 if (!$this->check_entropy || $this->checkSfaEntropy($state)) { 281 $result[] = AuthSwitcher::SFA; 282 } Created by: melanger
I don't think this is right -
AuthSwitcher::SFA
here does not mean the REFEDS SFA profile, it just means the user can log in via password only. If you kept this check, then users with passwords that don't meet the REFEDS SFA profile would not be able to log in, this method would return an empty array.I think it would be better to move this check and keep the original version of this method.
174 184 } 175 185 } 176 186 187 private function checkSfaEntropy($attributes) 188 { 189 if (!$this->sfa_len_attr || !$this->sfa_alphabet_attr || !in_array( 190 $this->sfa_alphabet_attr, 191 $attributes, 192 true 193 ) || !in_array($this->sfa_len_attr, $attributes, true)) { 194 return false; Created by: melanger
This would mean a breaking change, because the original behavior is that anything satisfies SFA.
Please change the code - if
sfa_len_attr
orsfa_alphabet_attr
is not set, log a message with level INFO, that authswitcher could not check REFEDS SFA, so it assumes it was fulfiled. When the attribute config options are set, do the new behavior andreturn false
here.Also update README - mention that if those 2 new options are not set, it is assumed that all passwords satisfy REFEDS SFA, and explain how to use them.
Created by: github-actions[bot]
This PR is included in version 10.5.0The release is available on GitHub release
Your semantic-release bot :package::rocket: