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

Introduce middleware for session handling #4266

Merged
merged 4 commits into from Jun 13, 2023
Merged

Conversation

juliushaertl
Copy link
Member

@juliushaertl juliushaertl commented Jun 8, 2023

📝 Summary

This PR introduces a middleware for checking if a valid text session is present. Controllers can annotate this requirement using the #[RequireDocumentSession] attribute and furtheron rely on the getSession method of the ASessionAwareController base class.

For now only two controllers are migrated as the others still pass around the request parameters so this will require some more refactoring in a follow up.

Integration test in cypress is added for the user api controller to cover the check for a valid session.

Also did some code cleanup with new PHP features along the way.

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
  • Documentation (README or documentation) has been updated or is not required

@cypress
Copy link

cypress bot commented Jun 8, 2023

1 flaky tests on run #10139 ↗︎

0 147 1 0 Flakiness 1

Details:

Introduce middleware for session handling
Project: Text Commit: b86e40e407
Status: Passed Duration: 08:09 💡
Started: Jun 12, 2023 2:23 PM Ended: Jun 12, 2023 2:31 PM
Flakiness  cypress/e2e/api/UsersApi.spec.js • 1 flaky test

View Output Video

Test Artifacts
The user mention API > fetches users with valid session Output Screenshots

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

Copy link
Collaborator

@max-nextcloud max-nextcloud left a comment

Choose a reason for hiding this comment

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

Great work. So much nicer than before.

Comment on lines 2 to 5
* @copyright Copyright (c) 2022 Max <max@nextcloud.com>
*
* @author Max <max@nextcloud.com>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume this is your copyright - or has this test only been moved?

Copy link
Member Author

Choose a reason for hiding this comment

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

I started with a copy of the other API one ;) Will update to keep us both

fixes #1943

Signed-off-by: Julius Härtl <jus@bitgrid.net>
…ntController and UserApiController

Signed-off-by: Julius Härtl <jus@bitgrid.net>
… to separate folder

Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliushaertl
Copy link
Member Author

/compile

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@juliushaertl juliushaertl merged commit 7fc6fa3 into main Jun 13, 2023
43 checks passed
@juliushaertl juliushaertl deleted the chore/session-middleware branch June 13, 2023 09:14
@mejo-
Copy link
Member

mejo- commented Dec 1, 2023

/backport f6f2b80,65e2e3b3f7af3941834f2a645fcd0585a5406bd9,9473eff478490de21e127ac9939e1efabc06acc3 to stable27

@mejo-
Copy link
Member

mejo- commented Dec 5, 2023

/backport f6f2b80 to stable27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants