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.

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

Optional Terms Of Services APIs and libparsec support #8471

Merged
merged 7 commits into from
Oct 4, 2024
Merged

Conversation

touilleMan
Copy link
Member

@touilleMan touilleMan commented Sep 21, 2024

Changes:

  • an organization can be configured with a EULA
  • a default EULA can be provided in the server configuration for spontaneously created organization
  • authenticated cmds now check that the author has accepted the EULA, if not an HTTP 463 error is returned
  • a new cmds family is introduced: the "eula" cmds which is similar to to the authenticated cmds family but doesn't check for the EULA to be accepted.
  • the eula cmds family exposes two commands eula_get (to retreive the EULA url that should be displayed to the user) and eula_accept (to accept the EULA, once done the authenticated cmds can be used)
  • if organization's EULA is updated, then the user using authenticated API will get 463 until they accept the new EULA
  • if organization's EULA is removed, then user can use authenticated API no matter if they have accepted or not the previous EULA
  • From the bindings point of view, client_get_eula and client_accept_eula are now part of libparsec

What is left:

Currently libparsec_client doesn't correctly handle HTTP errors coming from the server (see #8472 ).

This means the HTTP 463 error coming from the server in case the EULA hasn't been accept is currently translated into an internal error (instead of being a proper dedicated error).

Copy link
Contributor

@AureliaDolo AureliaDolo left a comment

Choose a reason for hiding this comment

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

I don't get exactly why eula is it's own kind of commands. It's because it's only part of the auth process ?

libparsec/crates/client/src/client/eula.rs Outdated Show resolved Hide resolved
@AureliaDolo
Copy link
Contributor

@touilleMan see #8513 for the old invitation system artifacts

@touilleMan touilleMan changed the title Optional End User License Agreement APIs and libparsec support Optional Terms Of Services APIs and libparsec support Sep 25, 2024
@touilleMan
Copy link
Member Author

I don't get exactly why eula is it's own kind of commands. It's because it's only part of the auth process ?

@AureliaDolo the issue is the authenticated cmds family checks that the user has accepted the Terms Of Service

So the commands to retrieve and accept the Terms Of Service must go through a different cmds family that does the same authentication checks, except for the acceptation of the Terms Of Service part.

@touilleMan touilleMan force-pushed the eula branch 8 times, most recently from ff6d493 to 4441fa8 Compare September 27, 2024 08:49
@FirelightFlagboy FirelightFlagboy linked an issue Sep 30, 2024 that may be closed by this pull request
libparsec/crates/client/src/client/tos.rs Outdated Show resolved Hide resolved
libparsec/crates/client/src/client/tos.rs Outdated Show resolved Hide resolved
libparsec/crates/client/src/event_bus.rs Outdated Show resolved Hide resolved
libparsec/crates/client/src/client/tos.rs Outdated Show resolved Hide resolved
server/parsec/components/auth.py Outdated Show resolved Hide resolved
server/parsec/components/auth.py Outdated Show resolved Hide resolved
server/parsec/components/postgresql/organization_update.py Outdated Show resolved Hide resolved
server/parsec/components/postgresql/organization_update.py Outdated Show resolved Hide resolved
@mmmarcos
Copy link
Contributor

mmmarcos commented Oct 1, 2024

As discussed with @touilleMan we should add a (minimal?) RFC describing how this works

@touilleMan touilleMan force-pushed the eula branch 2 times, most recently from 9615886 to 4ac273e Compare October 2, 2024 14:46
@@ -11,8 +11,14 @@ pub type Integer = i64;
#[derive(serde::Deserialize, serde::Serialize, Clone, Debug, PartialEq, Eq)]
pub struct DeviceID(pub String);

#[allow(unused)]
pub enum ProtocolFamily {
Family,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this mock needed ?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's used for testing the crate itself

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the libparsec_serialization_format crate generates in its proc macros some code that makes uses of the types in libparsec_types.
So that means libparsec_serialization_format's tests requires libparsec_types as dependencies, but this is cumbersome (don't remember the specifics, but I guess it has to do with circular dependencies).
So instead of directly relying on libparsec_types, the generated code refers to something like super::libparsec_types. This way the user of the proc macro can define whath is libparsec_types should be:

  • a mock (the code you were pointing out) in case of libparsec_serialization_format's tests
  • the real libparsec_types crate for all other cases

@touilleMan touilleMan force-pushed the eula branch 2 times, most recently from 743b789 to d6fbd7b Compare October 3, 2024 10:40
@touilleMan touilleMan force-pushed the eula branch 5 times, most recently from c905abd to c459cc7 Compare October 3, 2024 14:48
@Max-7 Max-7 mentioned this pull request Oct 4, 2024
5 tasks
@touilleMan touilleMan added this pull request to the merge queue Oct 4, 2024
Merged via the queue into master with commit fe36a18 Oct 4, 2024
16 checks passed
@touilleMan touilleMan deleted the eula branch October 4, 2024 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement T&Cs acceptance server-side
5 participants