Accept terms of Service at signup

frcroth commented 1 week ago

URL of deployed dev instance (used for testing):

https://___.webknossos.xyz

Accept terms of service already at sign up / organization creation.

Backend is done. Frontend needs to call /api/termsOfService route and add a checkbox / iframe to the account creation page. Then set "acceptedTermsOfService" in the submitted form to /api/auth/createOrganizationWithAdmin to the TOS VERSION. The Terms of Service acceptance afterward can be removed.

Steps to test:

Register as a new user
Try not to accept the Terms of Service, this should not be possible
Switch isWkOrgInstance in application.conf, it should behave the same regarding the ToS
Disable the ToS in application.conf, there should no longer be a ToS checkbox
After having signed up, increase the ToS version in application.conf. The user should now see the ToS modal and have to accept the ToS

TODOs:

[x] Frontend
- [x] wkorg
- [x] generic
- [x] styling
[ ] Remove dev-only changes to application.conf

Issues:

fixes #8149

(Please delete unneeded items, merge only when none are left open)

[x] Updated changelog
[ ] Updated migration guide if applicable
[ ] Updated documentation if applicable
[ ] Adapted wk-libs python client if relevant API parts change
[ ] Considered common edge cases

coderabbitai[bot] commented 1 week ago

📝 Walkthrough

## Walkthrough The changes in this pull request enhance the user registration process by integrating terms of service (TOS) acceptance directly into the signup form. The `SignUpData` case class now includes an optional field for TOS acceptance, and the `createOrganizationWithAdmin` method has been updated to process this information. The frontend components have been modified to include checkboxes for TOS agreement, and the configuration has been updated to enable TOS features. Overall, the modifications streamline the user experience by requiring TOS acceptance during registration. ## Changes | File Path | Change Summary | |-----------------------------------------------------|-------------------------------------------------------------------------------------------------------------------| | app/controllers/AuthenticationController.scala | Added `acceptTermsOfServiceForUser` method; updated `SignUpData` to include `acceptedTermsOfService`; modified `createOrganizationWithAdmin` to handle TOS acceptance. | | app/controllers/OrganizationController.scala | Refactored `acceptTermsOfService` to use `organizationService`; updated `create` and `getDefault` methods for user and privacy checks. | | app/models/organization/OrganizationService.scala | Added `acceptTermsOfService` method to handle TOS acceptance logic. | | conf/application.conf | Updated `termsOfService.enabled` and `features.isWkorgInstance` to `true`. | | frontend/javascripts/admin/auth/registration_form_generic.tsx | Added TOS checkbox and validation; modified structure for privacy statement checkbox. | | frontend/javascripts/admin/auth/registration_form_wkorg.tsx | Updated to include TOS acceptance handling and simplified privacy check logic. | | frontend/javascripts/messages.tsx | Added message key for TOS acceptance requirement. | | frontend/javascripts/router.tsx | Imported `CheckTermsOfServices` component for rendering in the router layout. | | frontend/stylesheets/main.less | Introduced `.registration-form-checkboxes` CSS class for styling checkboxes. | | CHANGELOG.unreleased.md | Documented changes related to TOS acceptance in user registration and other enhancements. | ## Assessment against linked issues | Objective | Addressed | Explanation | |---------------------------------------------------------------------------|-----------|-------------------------------------------------| | Integrate TOS acceptance into signup form (#8149) | ✅ | | | Adapt frontend to determine TOS necessity based on configuration (#8149) | ✅ | | ## Possibly related PRs - **#8127**: Related to user registration and terms acceptance, potentially involving similar input handling. - **#8181**: Focuses on improving error messages, aligning with user feedback enhancements in registration. - **#8200**: Involves user permissions and validation logic, connecting to updates on user registration. ## Suggested labels `new feature`, `frontend`, `backend` ## Suggested reviewers - MichaelBuessemeyer ## Poem > In the fields where bunnies play, > New terms now lead the way. > With a checkbox here, and a form so bright, > We hop through signup, all feels just right! > Consent is key, so don’t delay, > Join us now, come out and play! 🐰✨

📜 Recent review details

**Configuration used: CodeRabbit UI** **Review profile: CHILL**

📥 Commits

Reviewing files that changed from the base of the PR and between 377d2897b3d8564bfd25bf50c932b3b446645649 and 8d618c4fff0a2a087cdf087704e75425551f30ca.

📒 Files selected for processing (2)

* `frontend/javascripts/admin/auth/registration_form_generic.tsx` (3 hunks) * `frontend/javascripts/admin/auth/registration_form_wkorg.tsx` (4 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

* frontend/javascripts/admin/auth/registration_form_generic.tsx * frontend/javascripts/admin/auth/registration_form_wkorg.tsx

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

🪧 Tips

### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit , please review it.` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (Invoked using PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` anywhere in the PR title to generate the title automatically. ### CodeRabbit Configuration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://docs.coderabbit.ai) for detailed information on how to use CodeRabbit. - Join our [Discord Community](http://discord.gg/coderabbit) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.

frcroth commented 1 week ago

Backend should be done. @dieknolle3333 do you want to take a look at this for the frontend?

dieknolle3333 commented 1 week ago

sounds good!

dieknolle3333 commented 1 week ago

I feel like adding an iframe increases the probability that users actually read the ToS before accepting them, but then the privacy statement should also be an iframe. And two iframes either make the page quite long or are quite small, so users would have to scroll a lot to read them. Thats why I chose to add a link for now. If anybody disagrees with this decision, please let me know! (pic below shows how it looks with the terms of service in an iframe, for illustrations purposes)

dieknolle3333 commented 1 week ago

this is how I built the frontend for now

frcroth commented 4 days ago

@dieknolle3333 Thank you so much! One thing I noticed is that the frontend sends an "organization" value used as the org id. However, it is a random string, not the slug-name of the organization as I expected (from the changes from #7386). So, if I create an account as "Monika Musterfrau" I would expect the organization value to be either empty (it will be handled by the backend then) or "monika-musterfrau-lab", not "3c98af371407d694"

dieknolle3333 commented 4 days ago

I will have a look

dieknolle3333 commented 4 days ago

@frcroth I implemented the org id generation similar to the org name generation now, and this id is sent to /api/auth/createOrganizationWithAdmin. I also added the ToS modal again thanks to Toms remark on slack. I did not yet test the latter, will do that later today. I did notice that the registration of users works now, thanks to your latest changes! :tada: edit: I think testing went well :+1:

MichaelBuessemeyer commented 2 days ago

@dieknolle3333 Thank you so much! One thing I noticed is that the frontend sends an "organization" value used as the org id. However, it is a random string, not the slug-name of the organization as I expected (from the changes from https://github.com/scalableminds/webknossos/pull/7386). So, if I create an account as "Monika Musterfrau" I would expect the organization value to be either empty (it will be handled by the backend then) or "monika-musterfrau-lab", not "3c98af371407d694"

@fm3 I need a little double-check here. I would have expected the frontend to not generate the orga id but letting the backend do this :thinking:. Because I would imagine that the new implementation:

function generateOrganizationId(firstName: string, lastName: string) {
  return `${firstName.toLowerCase()}-${lastName.toLowerCase()}-lab`;
}

can lead to naming collisions in case of using the same name when creating an orga. :thinking:.

MichaelBuessemeyer commented 2 days ago

Oh and during testing I happend to notice, that the wkorg sign up form only has a single password input :thinking:?! The second one to prevent accidental mistyping is missing. Was this removed intentionally? A small investigation showed that this has been the case for a long time. cf. pr https://github.com/scalableminds/webknossos/pull/5350/files where this was already the case.

But IMO there should be a second pwd field to prevent mistyping the password.

=> I opened an issue for this, so we can tackle this in another PR :D https://github.com/scalableminds/webknossos/issues/8215

dieknolle3333 commented 20 hours ago

from my perspective, this PR can be rereviewed :) I left the application.conf edits for testing purposes.

scalableminds / webknossos