Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GDPR] | Invite to a role - Logged in user, warning popup is not matching designs and session Hijacking creates security risk and problems during workflow #11021

Open
1 task done
Tribunal33 opened this issue Mar 3, 2025 · 3 comments
Assignees
Labels
Bug:3:Critical A bug that prevents a substantial majority of users from using the software.
Milestone

Comments

@Tribunal33
Copy link
Contributor

Tribunal33 commented Mar 3, 2025

Valid Title

  • I have updated the title to accurately reflect the bug description

Description

This one has two issues. First one is the popup is not rendering variables correctly and does not match the Figma design which forces logout. But the more concerning problem is the session tokens can be hijacked which causes a security issue and leads to further problems for the workflow.

Steps to Reproduce

  1. Login with User/Role with access to invite to a role
  2. Invite a user to a role completing the workflow
  3. Without logging out check the mail and accept the new role
  4. Redirects to Step 1 of acceptance workflow (shows broken error popup 1st time)
  5. Select ok on the broken popup
  6. Fill in valid Username, password and tick privacy statement > Select Save and Continue
  7. Fill in the Ngnix server password if it pops up
  8. It will throw the broken popup error again. By pass this by selecting OK
  9. Fill in details with valid Given Name, Family Name, Affiliation and Country of Affiliation
  10. Select Save and Continue
  11. Fill in the Ngnix server password if it pops up
  12. Broken Popup shows another time > Select OK to bypass
  13. Observe Review & Create account page Roles table is empty
  14. Select Accept and Continue to OJS
  15. Fill in the Ngnix server password if it pops up
  16. Broken Popup comes up again and you cannot proceed

Expected Result

Figma designs have a separate modal (see screenshot). Beyond that the user should not be able to continue with the workflow without having been signed out initially.

Actual Result

See screenshot. Several broken popups throughout the workflow. User is able to bypass them and continue.

Environment Details

No response

Application Version

OJS stable-3_5_0

Logs

No response

Additional Information

Image


PRs

[pkp-lib][main]: #11055

@Tribunal33 Tribunal33 added the Bug:3:Critical A bug that prevents a substantial majority of users from using the software. label Mar 3, 2025
@Tribunal33 Tribunal33 added this to the 3.5 Internal milestone Mar 3, 2025
@Tribunal33
Copy link
Contributor Author

Tribunal33 commented Mar 3, 2025

@asmecher if you could take a look at the potential bug as a security risk. There is a figma design which would force a logout but only during this flow. I feel like session expiration and token hijacking could be a problem with phishing attacks.

@withanage if you could just take a look at the expected figma designs for this scenario I think we could downgrade the severity level to 2. I could even move the security risk idea to another ticket to investigate. I've noticed, since I sign in all the time with a variety of users, that there were times that my sessions get blocked. If I was a more sophisticated hacker I think I could exploit this issue.

@asmecher
Copy link
Member

asmecher commented Mar 3, 2025

Actually, I'll pass this to @defstat, who wrote the AnonymousUserPolicy that's at play here as part of #10459. That's where the named user.authorization.shouldBeAnonymous locale key is; looks like it was never added to the .po files.

@Tribunal33, I'm not sure I see a broader security issue here, but @defstat can double-check.

@asmecher asmecher assigned defstat and unassigned asmecher and withanage Mar 3, 2025
defstat added a commit to defstat/pkp-lib that referenced this issue Mar 10, 2025
@defstat
Copy link
Contributor

defstat commented Mar 10, 2025

@Tribunal33 @asmecher I have added the missing .po locale key. The PR is linked in the issue description.

Regarding UI and session management, I see the following distinct cases:

  1. The invitation is created for a specific user.

    • 1.1 A different user is logged in.
    • 1.2 The invited user is logged in.
    • 1.3 No user is logged in.

    Currently, in all these cases, we identify the invited user and log them in, logging out any other user in the process.

    I believe a better approach would be:

    • 1.1 The operation should not be allowed.
    • 1.2 The operation should proceed as expected.
    • 1.3 The invited user should be logged in, allowing the process to continue.
  2. The invitation is created for a person who is not yet a user in OJS/OMP/OPS.

    • 2.1 A user is already logged in.
      • In the back office, the shouldBeAnonymous policy is triggered, preventing the operation. @Tribunal33 is correct about the UI flow - currently, the user can dismiss the error popup and continue to the next step, which is not the expected behavior. The back office does not process any operations in this case, but the UI should account for the authorization error returned by the API. (tagging @withanage and @ipula for this particular case)
    • 2.2 No user is logged in.
      • The process works as expected: since there is no existing user, we create the new user at the end of the process.
    • 2.3 An existing user is found with the invitation's email.
      • In this case, we follow the logic from scenario (1).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug:3:Critical A bug that prevents a substantial majority of users from using the software.
Projects
None yet
Development

No branches or pull requests

4 participants