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.

Sign up for GitHub

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

Accept terms of Service at signup #8193

Merged
merged 23 commits into from
Nov 27, 2024
Merged

Accept terms of Service at signup #8193

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
a44f782
Adapt create org route in backend to accept terms of service version
frcroth Nov 13, 2024
b62721f
Merge branch 'master' into accept-tos-at-sign-up
frcroth Nov 13, 2024
94716a7
fix inconsistency
frcroth Nov 13, 2024
5607fcc
Fix formatting
frcroth Nov 13, 2024
05eed1c
start to implement frontend
knollengewaechs Nov 14, 2024
ae61dc6
delete old tos modal, add checkboxes to forms and add style
knollengewaechs Nov 14, 2024
5340755
improve styling and remove random new test file
knollengewaechs Nov 15, 2024
ce2396f
Merge branch 'master' into accept-tos-at-sign-up
knollengewaechs Nov 15, 2024
4ea0446
Supply user access context at sign up tos acceptance
frcroth Nov 18, 2024
77e2a59
send org name as id
knollengewaechs Nov 18, 2024
b1bb0b8
add tos modal again
knollengewaechs Nov 18, 2024
a4817e4
Merge branch 'master' into accept-tos-at-sign-up
knollengewaechs Nov 18, 2024
f52c528
Fix frontend lint
frcroth Nov 18, 2024
2602e5b
Add changelog
frcroth Nov 20, 2024
e5d5f19
Revert "send org name as id"
frcroth Nov 20, 2024
db456d0
Merge branch 'master' into accept-tos-at-sign-up
frcroth Nov 20, 2024
377d289
Require terms of service at sign up if enabled
frcroth Nov 20, 2024
8d618c4
invert logic for rendering tos checkbox
knollengewaechs Nov 21, 2024
6c819e3
Add explanation on tos failure because of non-acceptance
frcroth Nov 25, 2024
1d2ecb2
Revert changes to application.conf
frcroth Nov 25, 2024
94be416
Merge branch 'master' into accept-tos-at-sign-up
frcroth Nov 25, 2024
d60afaa
extract TOS checkbox to component
knollengewaechs Nov 25, 2024
ca427bc
Merge branch 'master' into accept-tos-at-sign-up
frcroth Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions app/controllers/AuthenticationController.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -621,8 +621,8 @@ class AuthenticationController @Inject()(
dataStoreToken <- bearerTokenAuthenticatorService.createAndInitDataStoreTokenForUser(user)
_ <- organizationService
.createOrganizationDirectory(organization._id, dataStoreToken) ?~> "organization.folderCreation.failed"
_ <- Fox.runOptional(signUpData.acceptedTermsOfService)(version =>
acceptTermsOfServiceForUser(user, version))
_ <- Fox.runIf(conf.WebKnossos.TermsOfService.enabled)(
acceptTermsOfServiceForUser(user, signUpData.acceptedTermsOfService))
} yield {
Mailer ! Send(defaultMails
.newOrganizationMail(organization.name, email, request.headers.get("Host").getOrElse("")))
Expand All @@ -639,8 +639,12 @@ class AuthenticationController @Inject()(
)
}

private def acceptTermsOfServiceForUser(user: User, termsOfServiceVersion: Int)(implicit m: MessagesProvider) =
organizationService.acceptTermsOfService(user._organization, termsOfServiceVersion)(DBAccessContext(Some(user)), m)
private def acceptTermsOfServiceForUser(user: User, termsOfServiceVersion: Option[Int])(
implicit m: MessagesProvider): Fox[Unit] =
for {
acceptedVersion <- Fox.option2Fox(termsOfServiceVersion)
Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer Nov 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

While being nitpicky 🙈
Could you please add an error message like

Suggested change
acceptedVersion <- Fox.option2Fox(termsOfServiceVersion)
acceptedVersion <- Fox.option2Fox(termsOfServiceVersion) ?~> "Terms of service must be accepted."

? 🙏

Copy link
Contributor

Choose a reason for hiding this comment

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

Else some kind of generic 400 error would be returned where an explanation would be useful IMO

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 can do this of course, no problem. Note that the user would not see 400 anyway because the frontend does not allow not accepting TOS.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, I just thought in case the logic changes in the frontend and a little bug or so is introduced, that the backend 1. ensures that tos is accepted and 2. in case it is not, the error returned is understandable.

I can do this of course, no problem.

Thanks a lot 🙏

_ <- organizationService.acceptTermsOfService(user._organization, acceptedVersion)(DBAccessContext(Some(user)), m)
} yield ()

case class CreateUserInOrganizationParameters(firstName: String,
lastName: String,
Expand Down
2 changes: 1 addition & 1 deletion conf/application.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ webKnossos {
"""
termsOfService {
enabled = true
knollengewaechs marked this conversation as resolved.
Show resolved Hide resolved
# The URL will be embedded into an iFrame
# The URL will be linked to or embedded into an iFrame
url = "https://webknossos.org/terms-of-service"
acceptanceDeadline = "2023-01-01T00:00:00Z"
version = 1
Expand Down