Skip to content

New endpoints tests#3

Merged
smarcet merged 2 commits into
mainfrom
feature/users-update
Dec 21, 2021
Merged

New endpoints tests#3
smarcet merged 2 commits into
mainfrom
feature/users-update

Conversation

@romanetar

Copy link
Copy Markdown
Contributor

$user_registration_request = json_decode($content);

$this->assertTrue(!empty($user_registration_request->hash));
// $params = [

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove this line

);

//$this->assertResponseStatus(204);
$this->assertResponseStatus(201); //TODO: last operation should return 204

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please check here the response of the action and if its get the updated data

Comment thread tests/OAuth2UserUpdateApiTest.php Outdated
);

//$this->assertResponseStatus(204);
$this->assertResponseStatus(201); //TODO: last operation should return 204

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please check response here to see if updated or not

@romanetar
romanetar force-pushed the feature/users-update branch from 5b514c6 to a601ac0 Compare December 21, 2021 14:17
Signed-off-by: smarcet@gmail.com <smarcet@gmail.com>
Change-Id: I6b4a734de97c90afa9f09f0badefceeea1d6ba1a

@smarcet smarcet left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me

@smarcet
smarcet merged commit 8b6c871 into main Dec 21, 2021
smarcet added a commit that referenced this pull request Dec 28, 2021
* New endpoints tests

* Refactored test seeding

Signed-off-by: smarcet@gmail.com <smarcet@gmail.com>
Change-Id: I6b4a734de97c90afa9f09f0badefceeea1d6ba1a

Co-authored-by: romanetar <roman_ag@hotmail.com>
Co-authored-by: smarcet@gmail.com <smarcet@gmail.com>
smarcet added a commit that referenced this pull request Jul 23, 2026
Two related fixes to the MFA login flow:

1. Passwordless (flow=otp) login never checked shouldRequire2FA(), so an
   enforced-2FA user could bypass MFA entirely via emitOTP() + postLogin
   with flow=otp instead of flow=password (SDS idp-mfa.md §7.4 / Open
   Question #3 explicitly treats passwordless as single-factor). Now
   throws AuthenticationException before loginWithOTP(), reusing the
   existing errorLogin() redirect+flash path - the OTP form still submits
   as a native form POST, so this needed no new response contract.

2. challengeRequired()'s redirect-based implementations
   (DefaultLoginStrategy, DisplayResponseUserAgentStrategy) previously
   ignored the $params they received, silently depending on the caller
   having already flashed otp_length/otp_lifetime to session - an
   implicit contract that would silently break for any other caller.
   Both now flash their own $params (persistent, not one-shot, so it
   survives repeated refreshes) and set error_code, mirroring what
   DisplayResponseJsonStrategy already sends native clients in JSON.
   clearMFAUISessionState() now clears error_code too.

   The '2fa' flow value moves from a new ILoginStrategy constant to
   IAuthService::AuthenticationFlowMFA, alongside its siblings
   AuthenticationFlowPassword/AuthenticationFlowPasswordless - all three
   are the same session 'flow' enum (already flashed together in the
   AuthenticationException catch block), so splitting the third value
   into a different interface would have been inconsistent.

New test: OAuth2NativeMFALoginFlowTest gains a non-native (page/popup/
touch) case proving the 302+session-flash contract, alongside the
existing native 412+JSON case. TwoFactorLoginFlowTest covers the
passwordless-bypass rejection (including that it still reuses
errorLogin(), not a new JSON contract) and the error_code flash/clear.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants