From a50144acb45f5197dda828430ca31011ce04a7ef Mon Sep 17 00:00:00 2001 From: Kiran Jonnalagadda Date: Mon, 1 Jul 2024 14:13:18 +0530 Subject: [PATCH 1/3] Let site editors manage placeholder accounts --- funnel/forms/profile.py | 17 ++++++- funnel/models/account.py | 52 ++++++++++++++------- funnel/registry.py | 11 ++--- funnel/templates/profile_layout.html.jinja2 | 34 ++++++++------ funnel/views/api/resource.py | 21 ++++++++- funnel/views/profile.py | 4 +- 6 files changed, 96 insertions(+), 43 deletions(-) diff --git a/funnel/forms/profile.py b/funnel/forms/profile.py index 43f644529..511cabcb3 100644 --- a/funnel/forms/profile.py +++ b/funnel/forms/profile.py @@ -64,11 +64,13 @@ def __post_init__(self) -> None: self.logo_url.profile = self.account.name or self.account.buid if self.account.is_user_profile: self.make_for_user() + elif self.account.is_placeholder_profile: + self.make_for_placeholder() if not self.account.is_verified: del self.description def make_for_user(self) -> None: - """Customise form for a user account.""" + """Customize form for a user account.""" self.title.label.text = __("Your name") self.title.description = __( "Your full name, in the form others can recognise you by" @@ -83,6 +85,19 @@ def make_for_user(self) -> None: "Optional – This message will be shown on the account’s page" ) + def make_for_placeholder(self) -> None: + """Customize form for a placeholder account.""" + self.title.label.text = __("Entity name") + self.title.description = __("A common name for this entity") + self.tagline.description = __("A brief statement about this entity") + self.name.description = __( + "A unique word for this entity’s account page. Alphabets, numbers and underscores are okay. Pick something permanent: changing it will break links" + ) + self.description.label.text = __("More about this entity") + self.description.description = __( + "Optional – This message will be shown on the account’s page" + ) + @Account.forms('transition') class ProfileTransitionForm(forms.Form): diff --git a/funnel/models/account.py b/funnel/models/account.py index 63cbaf5f6..a13fdcf2a 100644 --- a/funnel/models/account.py +++ b/funnel/models/account.py @@ -1466,21 +1466,6 @@ def default_email( # This user has no email addresses return None - @property - def _self_is_owner_of_self(self) -> Account | None: - """ - Return self in a user account. - - Helper method for :meth:`roles_for` and :meth:`actors_with` to assert that the - user is owner and admin of their own account. - """ - return self if self.is_user_profile else None - - with_roles( - _self_is_owner_of_self, - grants={'follower', 'member', 'admin', 'owner'}, - ) - def organizations_as_owner_ids(self) -> list[int]: """ Return the database ids of the organizations this user is an owner of. @@ -2025,6 +2010,21 @@ def __init__(self, **kwargs: Any) -> None: if self.joined_at is None: self.joined_at = sa.func.utcnow() + @property + def _self_is_owner_of_self(self) -> Self: + """ + Return self in a user account. + + Helper method for :meth:`roles_for` and :meth:`actors_with` to assert that the + user is owner and admin of their own account. + """ + return self + + with_roles( + _self_is_owner_of_self, + grants={'follower', 'member', 'admin', 'owner'}, + ) + # XXX: Deprecated, still here for Baseframe compatibility Account.userid = Account.uuid_b64 @@ -2139,7 +2139,8 @@ class Community(Account): """ A community account. - Communities differ from organizations in having open-ended membership. + Communities differ from organizations in having open-ended membership. This model + is currently not properly specified and therefore not exposed in UI. """ __mapper_args__ = {'polymorphic_identity': 'C'} @@ -2157,11 +2158,28 @@ def __init__(self, owner: User, **kwargs: Any) -> None: class Placeholder(Account): - """A placeholder account.""" + """ + A placeholder account. + + Placeholders are managed by site editors, typically on behalf of an external entity. + """ __mapper_args__ = {'polymorphic_identity': 'P'} is_placeholder_profile = True + @role_check('owner', 'admin') + def site_editor_owner( + self, actor: Account | None, _anchors: Sequence[Any] = () + ) -> bool: + """Grant 'owner' and related roles to site editors.""" + return actor is not None and actor.is_site_editor + + @site_editor_owner.iterable + def _(self) -> Iterable[Account]: + return Account.query.join( + SiteMembership, SiteMembership.member_id == Account.id + ).filter(SiteMembership.is_active, Account.state.ACTIVE) + class Team(UuidMixin, BaseMixin[int, Account], Model): """A team of users within an organization.""" diff --git a/funnel/registry.py b/funnel/registry.py index 275238635..391ebb578 100644 --- a/funnel/registry.py +++ b/funnel/registry.py @@ -206,13 +206,12 @@ class LoginProvider: :meth:`do` is called when the user chooses to login with the specified provider. :meth:`callback` is called with the response from the provider. - Both :meth:`do` and :meth:`callback` are called as part of a Flask - view and have full access to the view infrastructure. However, while - :meth:`do` is expected to return a Response to the user, - :meth:`callback` only returns information on the user back to Lastuser. + Both :meth:`do` and :meth:`callback` are called as part of a Flask view and have + full access to the view infrastructure. However, while :meth:`do` is expected to + return a Response to the user, :meth:`callback` only returns information on the user + back to Lastuser. - Implementations must take their configuration via the __init__ - constructor. + Implementations must take their configuration via the __init__ constructor. :param name: Name of the service (stored in the database) :param title: Title (shown to user) diff --git a/funnel/templates/profile_layout.html.jinja2 b/funnel/templates/profile_layout.html.jinja2 index 07a73ee9f..b82854268 100644 --- a/funnel/templates/profile_layout.html.jinja2 +++ b/funnel/templates/profile_layout.html.jinja2 @@ -371,17 +371,19 @@ {% endif %} -
- {%- if current_auth.is_anonymous %} - {% trans %}Follow{% endtrans %} - {%- elif profile != current_auth.user and not profile.features.is_private() %} - - {% if not hide_unfollow %} - - {% endif %} - - {%- endif %} -
+ {%- if not profile.features.is_private() %} +
+ {%- if current_auth.is_anonymous %} + {% trans %}Follow{% endtrans %} + {%- elif profile != current_auth.user %} + + {% if not hide_unfollow %} + + {% endif %} + + {%- endif %} +
+ {%- endif %} {% endmacro %} @@ -479,10 +481,14 @@ {% trans %}Admins{% endtrans %} {{ faicon(icon='chevron-right', icon_size='subhead') }} {% elif not profile.features.is_private() %} {% trans %}Sessions{% endtrans %} - {% trans %}Projects{% endtrans %}{{ faicon(icon='chevron-right', icon_size='subhead') }} - {% trans %}Submissions{% endtrans %}{{ faicon(icon='chevron-right', icon_size='subhead') }} + {%- if profile.is_user_profile %} + {% trans %}Projects{% endtrans %}{{ faicon(icon='chevron-right', icon_size='subhead') }} + {% trans %}Submissions{% endtrans %}{{ faicon(icon='chevron-right', icon_size='subhead') }} + {%- endif %} {%- if profile.current_roles.admin %}{# TODO: Remove after consent flow #} - {% trans %}Following{% endtrans %} {% if profile.features.following_count() %}{{ profile.features.following_count() }}{% endif %}{{ faicon(icon='chevron-right', icon_size='subhead') }} + {%- if not profile.is_placeholder_profile %} + {% trans %}Following{% endtrans %} {% if profile.features.following_count() %}{{ profile.features.following_count() }}{% endif %}{{ faicon(icon='chevron-right', icon_size='subhead') }} + {%- endif %} {% trans %}Followers{% endtrans %} {% if profile.features.followers_count() %}{{ profile.features.followers_count() }}{% endif %}{{ faicon(icon='chevron-right', icon_size='subhead') }} {%- endif %} {% endif %} diff --git a/funnel/views/api/resource.py b/funnel/views/api/resource.py index 1e85722b0..eded4b3da 100644 --- a/funnel/views/api/resource.py +++ b/funnel/views/api/resource.py @@ -22,6 +22,7 @@ AuthToken, LoginSession, Organization, + Placeholder, User, db, getuser, @@ -47,6 +48,7 @@ def get_userinfo( get_permissions: bool = True, ) -> ReturnResource: """Return userinfo for a given user, auth client and scope.""" + userinfo: dict[str, Any] if '*' in scope or 'id' in scope or 'id/*' in scope: userinfo = { 'userid': user.buid, @@ -92,6 +94,21 @@ def get_userinfo( for org in user.organizations_as_admin ], } + # If the user is a site editor, also include placeholder accounts. + # TODO: Remove after Imgee merger + if user.is_site_editor: + placeholders = [ + { + 'userid': p.buid, + 'buid': p.buid, + 'uuid': p.uuid, + 'name': p.urlname, + 'title': p.title, + } + for p in Placeholder.query.all() + ] + userinfo['organizations']['owner'].extend(placeholders) + userinfo['organizations']['admin'].extend(placeholders) if get_permissions: uperms = AuthClientPermissions.get(auth_client=auth_client, account=user) @@ -488,13 +505,13 @@ def resource_id( @app.route('/api/1/session/verify', methods=['POST']) -@resource_registry.resource('session/verify', __("Verify user session"), scope='id') +@resource_registry.resource('session/verify', __("Verify login session"), scope='id') def session_verify( authtoken: AuthToken, args: MultiDict, files: MultiDict | None = None, # noqa: ARG001 ) -> ReturnResource: - """Verify a UserSession.""" + """Verify a :class:`LoginSession`.""" sessionid = abort_null(args['sessionid']) login_session = LoginSession.authenticate(buid=sessionid, silent=True) if login_session is not None and login_session.account == authtoken.effective_user: diff --git a/funnel/views/profile.py b/funnel/views/profile.py index d072f2f72..0118e77d5 100644 --- a/funnel/views/profile.py +++ b/funnel/views/profile.py @@ -117,7 +117,7 @@ def view(self) -> ReturnRenderWith: ], } - elif self.obj.is_organization_profile: + else: template_name = 'profile.html.jinja2' # `order_by(None)` clears any existing order defined in relationship. @@ -248,8 +248,6 @@ def view(self) -> ReturnRenderWith: else None ), } - else: - abort(404) # Reserved account return ctx From aa8711fcd4ad92d83534ac98d44ae8f4494cd0e3 Mon Sep 17 00:00:00 2001 From: Kiran Jonnalagadda Date: Mon, 1 Jul 2024 15:27:44 +0530 Subject: [PATCH 2/3] Linter fixes --- funnel/models/__init__.py | 1 + funnel/models/auth_client.py | 4 +--- funnel/transports/sms/send.py | 4 +--- funnel/views/account_delete.py | 8 ++------ 4 files changed, 5 insertions(+), 12 deletions(-) diff --git a/funnel/models/__init__.py b/funnel/models/__init__.py index fad49d181..773fe8e6b 100644 --- a/funnel/models/__init__.py +++ b/funnel/models/__init__.py @@ -1,6 +1,7 @@ """Provide configuration for models and import all into a common `models` namespace.""" # pyright: reportUnsupportedDunderAll=false +# ruff: noqa: F811 # The second half of this file is dynamically generated. Run `make initpy` to # regenerate. Since lazy_loader is opaque to static type checkers, there's an overriding diff --git a/funnel/models/auth_client.py b/funnel/models/auth_client.py index 144d61f2f..07331c9eb 100644 --- a/funnel/models/auth_client.py +++ b/funnel/models/auth_client.py @@ -531,9 +531,7 @@ def is_valid(self) -> bool: if self.validity == 0: return True # This token is perpetually valid now = utcnow() - if self.created_at < now - timedelta(seconds=self.validity): - return False - return True + return not self.created_at < now - timedelta(seconds=self.validity) @classmethod def revoke_all_for(cls, account: Account) -> None: diff --git a/funnel/transports/sms/send.py b/funnel/transports/sms/send.py index 36e408621..fdb25435b 100644 --- a/funnel/transports/sms/send.py +++ b/funnel/transports/sms/send.py @@ -108,9 +108,7 @@ def validate_exotel_token(token: str, to: str) -> bool: def okay_to_message_in_india_right_now() -> bool: """Report if it's currently within messaging hours in India (9 AM to 7PM IST).""" now = utcnow().astimezone(indian_timezone) - if now.hour >= DND_END_HOUR and now.hour < DND_START_HOUR: - return True - return False + return bool(now.hour >= DND_END_HOUR and now.hour < DND_START_HOUR) def send_via_exotel( diff --git a/funnel/views/account_delete.py b/funnel/views/account_delete.py index 7c6250cc9..d456f367e 100644 --- a/funnel/views/account_delete.py +++ b/funnel/views/account_delete.py @@ -52,9 +52,7 @@ def decorator(func: ValidatorFunc) -> ValidatorFunc: ) def profile_is_protected(user: Account) -> bool: """Block deletion if the user has a protected account.""" - if user.is_protected: - return False - return True + return not user.is_protected @delete_validator( @@ -91,9 +89,7 @@ def profile_has_projects(user: Account) -> bool: ) def user_owns_apps(user: Account) -> bool: """Fail if user is the owner of client apps.""" - if user.clients: - return False - return True + return not user.clients # MARK: Delete validator view helper --------------------------------------------------- From a0287c7976e2e018216ada1c3948fe95f54a51a5 Mon Sep 17 00:00:00 2001 From: Kiran Jonnalagadda Date: Mon, 1 Jul 2024 15:28:46 +0530 Subject: [PATCH 3/3] Wrap lines --- funnel/forms/profile.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/funnel/forms/profile.py b/funnel/forms/profile.py index 511cabcb3..71610cdc8 100644 --- a/funnel/forms/profile.py +++ b/funnel/forms/profile.py @@ -91,7 +91,9 @@ def make_for_placeholder(self) -> None: self.title.description = __("A common name for this entity") self.tagline.description = __("A brief statement about this entity") self.name.description = __( - "A unique word for this entity’s account page. Alphabets, numbers and underscores are okay. Pick something permanent: changing it will break links" + "A unique word for this entity’s account page. Alphabets, numbers and" + " underscores are okay. Pick something permanent: changing it will break" + " links" ) self.description.label.text = __("More about this entity") self.description.description = __(