Skip to content

Allow injecting the user temporarily for direct editing - #44295

Merged
blizzz merged 2 commits into
masterfrom
fix/direct-edit-incognito
Mar 21, 2024
Merged

Allow injecting the user temporarily for direct editing#44295
blizzz merged 2 commits into
masterfrom
fix/direct-edit-incognito

Conversation

@juliusknorr

@juliusknorrjuliusknorr commented Mar 19, 2024•
edited
Loading

Copy link
Copy Markdown
Member
  • Drop usage of incognito mode
  • Allow to inject the current user into the session without writing it to the php session itself for direct editing

Groupfolders

FixONLYOFFICE/onlyoffice-nextcloud#900 with ONLYOFFICE/onlyoffice-nextcloud#967

  • Setup a groupfolder that has read only ACL set on a child folder (without share permission)
  • Try to open a file with a mobile app
  • The mobile app uses a webview without user session
  • Now the onlyoffice codebase uses getUserFolder()->getById() which is fine as they handle authentication internally, however the getById will fail as groupfolders is not aware of the current user the mountpoint is setup with and assumes that the file is a share (requiring share acls)

github-advanced-security[bot]
github-advanced-securityAI found potential problems Mar 19, 2024
View reviewed changes
Comment threadapps/files_external/lib/Migration/DummyUserSession.php Fixed
@juliusknorr
fix: Do not use incognito mode for direct editing
20dd80d
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
juliusknorrforce-pushed the fix/direct-edit-incognito branch from 7a6ee18 to e6331a7CompareMarch 19, 2024 10:30
@juliusknorrjuliusknorr mentioned this pull request Mar 19, 2024
Merged
@juliusknorr
juliusknorrforce-pushed the fix/direct-edit-incognito branch from e6331a7 to 7d65d93CompareMarch 19, 2024 10:35
@juliusknorr
juliusknorr requested review from a team, ArtificialOwl, ChristophWurst, artonge, blizzz, mejo- and nfebe and removed request for a teamMarch 19, 2024 10:36
@juliusknorrjuliusknorr added bug 3. to review Waiting for reviews labels Mar 19, 2024
@juliusknorrjuliusknorr changed the title fix: Do not use incognito mode for direct editingAllow injecting the user temporarily for direct editingMar 19, 2024
@juliusknorrjuliusknorr mentioned this pull request Mar 19, 2024
Merged
nfebe
nfebe reviewed Mar 19, 2024
View reviewed changes
Comment threadapps/files_external/lib/Migration/DummyUserSession.php Outdated
nfebe
nfebe approved these changes Mar 19, 2024
View reviewed changes
Comment threadlib/private/User/Session.php Outdated
@juliusknorrjuliusknorr self-assigned this Mar 19, 2024
@juliusknorr
juliusknorrforce-pushed the fix/direct-edit-incognito branch 2 times, most recently from 4131f98 to 07691d3CompareMarch 19, 2024 12:41
github-advanced-security[bot]
github-advanced-securityAI found potential problems Mar 19, 2024
View reviewed changes
Comment threadapps/files_external/lib/Migration/DummyUserSession.php Fixed
@juliusknorr
fix: Implement option to temporarily set the user session
e330efe
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
juliusknorrforce-pushed the fix/direct-edit-incognito branch from 07691d3 to e330efeCompareMarch 19, 2024 12:48
@blizzz
blizzz merged commit 42180a2 into masterMar 21, 2024
@blizzz
blizzz deleted the fix/direct-edit-incognito branch March 21, 2024 12:54
@AltahrimAltahrim mentioned this pull request Mar 21, 2024
Merged
@nickvergessennickvergessen mentioned this pull request Mar 22, 2024
Merged
4 tasks
nickvergessen
nickvergessen reviewed Mar 22, 2024
View reviewed changes
Comment threadlib/private/legacy/OC_User.php
*/
public static function getUser() {
$uid = \OC::$server->getSession() ? \OC::$server->getSession()->get('user_id') : null;
$uid = Server::get(IUserSession::class)->getUser()?->getUID();

@nickvergessennickvergessenMar 22, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This breaks logging with debug when ldap is enabled:
#44400

@joshtrichardsjoshtrichards mentioned this pull request Mar 26, 2024
Closed
@juliusknorr

juliusknorr commented Jul 22, 2024

Copy link
Copy Markdown
MemberAuthor

/backport to stable29

@juliusknorr

juliusknorr commented Jul 22, 2024

Copy link
Copy Markdown
MemberAuthor

/backport to stable28

@juliusknorr

juliusknorr commented Jul 22, 2024

Copy link
Copy Markdown
MemberAuthor

/backport to stable27

@backportbotbackportbotBot mentioned this pull request Jul 22, 2024
Closed
3 tasks
This was referenced Jul 22, 2024
Merged
Merged
@juliusknorrjuliusknorr mentioned this pull request Jul 29, 2024
Merged
3 tasks
@juliusknorrjuliusknorr mentioned this pull request Dec 19, 2024
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickvergessennickvergessennickvergessen left review comments
@blizzzblizzzblizzz approved these changes
@nfebenfebenfebe approved these changes
@ChristophWurstChristophWurstAwaiting requested review from ChristophWurst
@mejo-mejo-Awaiting requested review from mejo-
@ArtificialOwlArtificialOwlAwaiting requested review from ArtificialOwlArtificialOwl is a code owner automatically assigned from nextcloud/server-backend
@artongeartongeAwaiting requested review from artongeartonge is a code owner automatically assigned from nextcloud/server-backend

Assignees

@juliusknorrjuliusknorr

Labels

3. to reviewWaiting for reviewsbug

Projects

📝 Productivity team
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Error with opening documents in Nextcloud Mobile App with Groupfolders Advanced Permissions

5 participants

@juliusknorr@nickvergessen@blizzz@nfebe@github-advanced-security