Dialog for when the user is about to share a resource publicly #1751
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a confirmation dialog that appears when users are about to share a resource publicly for the first time. The dialog warns users to check for sensitive data before making their document accessible to anyone with the link.
Key changes:
- Adds a new confirmation dialog for public sharing that warns about sensitive data exposure
- Refactors user management functions to support both organization and document-level permissions
- Creates comprehensive test coverage for the new dialog functionality
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/nbrowser/gristUtils.ts | Refactors user management functions to support document-level ACL operations and adds 'everyone' test user |
| test/nbrowser/UserManager.ts | Adds comprehensive tests for document sharing functionality including the new confirmation dialog |
| app/common/ResourceTypes.ts | Defines ResourceType union type for different resource categories |
| app/client/ui/UserManager.ts | Implements the public sharing confirmation dialog with warning message |
| app/client/models/UserManagerModel.ts | Adds logic to detect when public sharing is being enabled for the first time |
fflorent
force-pushed
the
warn-sharing-publicly
branch
from
6fa1947 to
2a304a3
Compare
| return await mainSession.loadDoc(`/doc/${docId}`, options); | ||
| } | ||
|
|
||
| async function checkAccessDenied(user: gu.UserData) { |
There was a problem hiding this comment.
I suspect this function makes the tests much slower (each of them takes around 8s). I hope it is still acceptable for you.
Alternatively, I can:
- try to use the API call instead, but I would need help;
- avoid checking the access for
user2,user3andanonas a prerequisite for the tests, maybe that's a bit overkill
fflorent
force-pushed
the
warn-sharing-publicly
branch
from
f4813bc to
4f19ee9
Compare
manuhabitela
moved this from Needs feedback
to Needs Internal Feedback
in French administration Board
app/client/ui/UserManager.ts
Outdated
| () => onConfirm(ctl, new Set([...acceptedWarnings, "public-sharing"])), | ||
| { | ||
| explanation: ( | ||
| markdown(t(`Your {{resourceType}} will be accessible to anyone \ |
There was a problem hiding this comment.
Can you please run npm run generate:translation
and verify that your new strings are correctly collected
and add the modification to your PR?
The user may not understand the consequences of sharing publicly a resource, especially a doc. This confirmation dialog is meant to ask them to be sure the document does not contain sensitive data before sharing.
This modal is meant to manage user access to resources. For now, only doc access is tested.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
fflorent
force-pushed
the
warn-sharing-publicly
branch
from
890489e to
af51757
Compare
fflorent
moved this from Needs Internal Feedback
to Needs feedback
in French administration Board
|
The language of the warning may suggest to a user that sharing a document will list it in search engines. Some websites make an active effort to get "user generated content" indexed by search engines by making easily consumed lists of such material for crawling. The reality of sharing for Grist is different, including a "noindex" meta element inserted in most document pages. 
Of course search engines can find documents in all sorts of ways as users paste the url here and there, and can ignore directives, so the warning has merit. Have you looked at the language used by other services that use link-sharing with the security dependent entirely on a secret in the url? |
No, we have instead worked together with members of our team to phrase this modal dialog to make the warning as clear and as meaningful possible. Should I take a look? |
3 participants
Context
The user may not understand the consequences of sharing publicly a resource, especially a doc.
Proposed solution
This confirmation dialog is meant to ask them to be sure the document does not contain sensitive data before sharing, and draw attention about the consequences.
It appears only when the resource is newly publicly shared (i.e. it does not appear if we change the permissions to everyone).
Related issues
Has this been tested?
Screenshots / Screencasts
recording.mp4