fix: show ToS on non-files public pages (calendar/collectives/talk), Files on Nextcloud 31 and Login-flow #1046

ShGKme · 2025-02-05T23:12:51Z

Resolves

ToS checks for public pages by `#sharingToken` element. It doesn't work:

In Nextcloud 31+ (must use initial state via @nextcloud/sharing)
For pages, that are public, but not Files, e.g.
- Calendar
- Collectives
- Talk

Instead - check for the current user.

How to test

Enable ToS, add terms of settings
Enable ToS for public pages
Create a calendar
Share calendar by link
Open as a guest

Changed behavior for logged-in users on public pages

Fix: Sending External Link to Internal users breaks things. #1013
Disable ToS for logged-in users
Enable ToS for public shares

Before:
- Logged-in users have to accept ToS on public shares (because it is a public share)
After:
- Logged-in users don't have to accept ToS on public shares (because they a logged-in)

Login flow

Fix: Accept ToS during login flow v2 #989
Because login pages are also "public" pages, they are now manually excluded
Except the login flow grant pages
As a side effect - it fixes the client

How to test

Enable ToS, add terms of settings
Enable ToS for logged-in users
Try to log in in the Files client (Login Flow v2)
- Before: login fails and you see login flow again
- After: you need to accept ToS on the last step
Try to log in in the Talk Desktop (Login Flow v1)
- Before: login is a success, but you cannot send/see files
- After: you need to accept ToS on the last step

Screenshots

Before	After

Signed-off-by: Grigorii K. Shartsev <[email protected]>

GretaD

tested and it works as described. I would like Joas to have a saying if the approach is what we want

nickvergessen

Changed behavior for logged-in users on public pages

✔️ Sounds like a desired change

There is one more change - it also applied to the login page, which might be unexpected

Similarly the registration app should be excluded, as it has a dedicated integration with the app.

src/public.js

nickvergessen · 2025-02-06T13:18:05Z

it also applied to the login page, which might be unexpected

To fix login and registration app, I suggest to check those parts in
https://github.com/nextcloud/terms_of_service/blob/master/lib/AppInfo/Application.php#L89
like /login/ is already checked for logged in users.

Also we need to check what happens with 2FA apps

ShGKme · 2025-02-07T16:32:58Z

it also applied to the login page, which might be unexpected

Fixed with an exception for login flow grant. It also fixes the issue with the clients.

ShGKme · 2025-02-07T16:36:55Z

lib/AppInfo/Application.php

+		// Skip login-related pages
+		// TODO: Add a universal way to specify skipped pages instead of hardcoding
+		$skipPatterns = [
+			// Login
+			'#^/login$#',
+			// Login Flow Grant must have the terms of service
+			// so that the user can accept them before using the app
+			// TODO: add a checkbox on the login instead, like on the Registration app
+			'#^/login/(?!flow/grant|v2/grant)#',
+			// SAML
+			'#^/saml$#',
+			'#^/saml/#',
+			// user_oidc
+			'#^/code$#',
+			'#^/sls$#',
+			'#^/id4me$#',
+			'#^/id4me/code$#',
+		];


cc @julien-nc for this part

Added /apps/<appname> prefixes in a fixup

ShGKme · 2025-02-07T16:46:36Z

Tested:

Signed-off-by: Grigorii K. Shartsev <[email protected]> Signed-off-by: nextcloud-command <[email protected]>

Signed-off-by: Grigorii K. Shartsev <[email protected]>

lib/AppInfo/Application.php

chore(deps): add @nextcloud/auth

05301a8

Signed-off-by: Grigorii K. Shartsev <[email protected]>

ShGKme added 3. to review bug labels Feb 5, 2025

ShGKme requested review from nickvergessen and GretaD February 5, 2025 23:12

ShGKme self-assigned this Feb 5, 2025

GretaD approved these changes Feb 6, 2025

View reviewed changes

ShGKme changed the title ~~fix(public): support non-files and Files on Nextcloud 31~~ fix(public): support non-files public pages (calendar/collectives) and Files on Nextcloud 31 Feb 6, 2025

This comment was marked as resolved.

Sign in to view

nextcloud-command force-pushed the fix/new-public-share-support branch from fc3d7e7 to 6ecb7da Compare February 6, 2025 10:51

This comment was marked as resolved.

Sign in to view

nickvergessen reviewed Feb 6, 2025

View reviewed changes

src/public.js Outdated Show resolved Hide resolved

ShGKme marked this pull request as draft February 6, 2025 15:22

ShGKme force-pushed the fix/new-public-share-support branch from 6ecb7da to 6c770c3 Compare February 7, 2025 16:22

ShGKme changed the title ~~fix(public): support non-files public pages (calendar/collectives) and Files on Nextcloud 31~~ fix(public): show ToS on non-files public pages (calendar/collectives) and Files on Nextcloud 31 Feb 7, 2025

ShGKme changed the title ~~fix(public): show ToS on non-files public pages (calendar/collectives) and Files on Nextcloud 31~~ fix: show ToS on non-files public pages (calendar/collectives/talk), Files on Nextcloud 31 and Login-flow Feb 7, 2025

ShGKme marked this pull request as ready for review February 7, 2025 16:33

ShGKme requested review from julien-nc and tobiasKaminsky February 7, 2025 16:36

ShGKme commented Feb 7, 2025

View reviewed changes

ShGKme added 3 commits February 7, 2025 17:55

fix(public): support non-files and Files on Nextcloud 31

0fd5702

Signed-off-by: Grigorii K. Shartsev <[email protected]> Signed-off-by: nextcloud-command <[email protected]>

fix: hide ToS on login except login flow for clients

048310e

Signed-off-by: Grigorii K. Shartsev <[email protected]>

chore(assets): Recompile assets

0ac77b5

Signed-off-by: Grigorii K. Shartsev <[email protected]>

ShGKme force-pushed the fix/new-public-share-support branch from 6c770c3 to 0ac77b5 Compare February 7, 2025 16:55

fixup! fix: hide ToS on login except login flow for clients

bed1701

nickvergessen reviewed Feb 7, 2025

View reviewed changes

lib/AppInfo/Application.php Show resolved Hide resolved

fixup! fix: hide ToS on login except login flow for clients

d7b15de

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: show ToS on non-files public pages (calendar/collectives/talk), Files on Nextcloud 31 and Login-flow #1046

fix: show ToS on non-files public pages (calendar/collectives/talk), Files on Nextcloud 31 and Login-flow #1046

ShGKme commented Feb 5, 2025 •

edited

Loading

GretaD left a comment

This comment was marked as resolved.

This comment was marked as resolved.

nickvergessen left a comment

nickvergessen commented Feb 6, 2025 •

edited

Loading

ShGKme commented Feb 7, 2025

ShGKme Feb 7, 2025

ShGKme Feb 7, 2025 •

edited

Loading

ShGKme commented Feb 7, 2025 •

edited

Loading

fix: show ToS on non-files public pages (calendar/collectives/talk), Files on Nextcloud 31 and Login-flow #1046

Are you sure you want to change the base?

fix: show ToS on non-files public pages (calendar/collectives/talk), Files on Nextcloud 31 and Login-flow #1046

Conversation

ShGKme commented Feb 5, 2025 • edited Loading

Resolves

ToS checks for public pages by #sharingToken element. It doesn't work:

How to test

Changed behavior for logged-in users on public pages

Login flow

How to test

Screenshots

GretaD left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

nickvergessen left a comment

Choose a reason for hiding this comment

nickvergessen commented Feb 6, 2025 • edited Loading

ShGKme commented Feb 7, 2025

ShGKme Feb 7, 2025

Choose a reason for hiding this comment

ShGKme Feb 7, 2025 • edited Loading

Choose a reason for hiding this comment

ShGKme commented Feb 7, 2025 • edited Loading

ShGKme commented Feb 5, 2025 •

edited

Loading

ToS checks for public pages by `#sharingToken` element. It doesn't work:

nickvergessen commented Feb 6, 2025 •

edited

Loading

ShGKme Feb 7, 2025 •

edited

Loading

ShGKme commented Feb 7, 2025 •

edited

Loading