Skip to content

Commit

Permalink
Merge branch 'main' into new-sms-templates
Browse files Browse the repository at this point in the history
  • Loading branch information
djamg committed Oct 10, 2023
2 parents 1a62da1 + a593bf9 commit 157a71c
Show file tree
Hide file tree
Showing 21 changed files with 163 additions and 73 deletions.
8 changes: 4 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@ repos:
]
files: ^requirements/.*\.txt$
- repo: https://github.com/asottile/pyupgrade
rev: v3.13.0
rev: v3.15.0
hooks:
- id: pyupgrade
args: ['--keep-runtime-typing', '--py310-plus']
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.0.291
rev: v0.0.292
hooks:
- id: ruff
args: ['--fix', '--exit-non-zero-on-fix']
Expand Down Expand Up @@ -127,7 +127,7 @@ repos:
- id: flake8
additional_dependencies: *flake8deps
- repo: https://github.com/PyCQA/pylint
rev: v3.0.0a7
rev: v3.0.1
hooks:
- id: pylint
args: [
Expand All @@ -147,7 +147,7 @@ repos:
additional_dependencies:
- 'bandit[toml]'
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.4.0
rev: v4.5.0
hooks:
- id: check-added-large-files
- id: check-ast
Expand Down
9 changes: 6 additions & 3 deletions funnel/forms/organization.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,12 @@ def validate_name(self, field: forms.Field) -> None:
)
if reason == 'reserved':
raise forms.validators.ValidationError(_("This name is reserved"))
if self.edit_obj and field.data.lower() == self.edit_obj.name.lower():
# Name is not reserved or invalid under current rules. It's also not changed
# from existing name, or has only changed case. This is a validation pass.
if (
self.edit_obj
and self.edit_obj.name
and field.data.lower() == self.edit_obj.name.lower()
):
# Name has only changed case from previous name. This is a validation pass
return
if reason == 'user':
if (
Expand Down
7 changes: 5 additions & 2 deletions funnel/forms/sync_ticket.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,8 @@ class TicketParticipantForm(forms.Form):
)
email = forms.EmailField(
__("Email"),
validators=[forms.validators.DataRequired(), forms.validators.ValidEmail()],
filters=[forms.filters.strip()],
validators=[forms.validators.Optional(), forms.validators.ValidEmail()],
filters=[forms.filters.none_if_empty()],
)
phone = forms.StringField(
__("Phone number"),
Expand Down Expand Up @@ -219,6 +219,9 @@ def set_queries(self) -> None:
def validate(self, *args, **kwargs) -> bool:
"""Validate form."""
result = super().validate(*args, **kwargs)
if self.email.data is None:
self.user = None
return True
with db.session.no_autoflush:
accountemail = AccountEmail.get(email=self.email.data)
if accountemail is not None:
Expand Down
9 changes: 7 additions & 2 deletions funnel/models/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,14 @@ class PROFILE_STATE(LabeledEnum): # noqa: N801
class ZBase32Comparator(Comparator[str]): # pylint: disable=abstract-method
"""Comparator to allow lookup by Account.uuid_zbase32."""

def __eq__(self, other: str) -> sa.ColumnElement[bool]: # type: ignore[override]
def __eq__(self, other: object) -> sa.ColumnElement[bool]: # type: ignore[override]
"""Return an expression for column == other."""
return self.__clause_element__() == UUID(bytes=zbase32_decode(other))
try:
return self.__clause_element__() == UUID( # type: ignore[return-value]
bytes=zbase32_decode(str(other))
)
except ValueError: # zbase32 call failed, so it's not a valid string
return sa.false()


class Account(UuidMixin, BaseMixin, Model):
Expand Down
5 changes: 4 additions & 1 deletion funnel/models/project_membership.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from .membership_mixin import ImmutableUserMembershipMixin
from .project import Project

__all__ = ['ProjectMembership', 'project_child_role_map']
__all__ = ['ProjectMembership', 'project_child_role_map', 'project_child_role_set']

#: Roles in a project and their remapped names in objects attached to a project
project_child_role_map: dict[str, str] = {
Expand All @@ -24,6 +24,9 @@
'reader': 'reader',
}

#: A model that is indirectly under a project needs the role names without remapping
project_child_role_set: set[str] = set(project_child_role_map.values())

#: ProjectMembership maps project's `account_admin` role to membership's `editor`
#: role in addition to the recurring role grant map
project_membership_role_map: dict[str, str | set[str]] = {
Expand Down
6 changes: 4 additions & 2 deletions funnel/models/sync_ticket.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ class TicketParticipant(EmailAddressMixin, UuidMixin, BaseMixin, Model):
"""A participant in one or more events, synced from an external ticket source."""

__tablename__ = 'ticket_participant'
__email_optional__ = False
__email_optional__ = True
__email_for__ = 'participant'

fullname = with_roles(
Expand Down Expand Up @@ -376,7 +376,9 @@ def checkin_list(cls, ticket_event: TicketEvent) -> list: # TODO: List type?
TicketEventParticipant,
TicketParticipant.id == TicketEventParticipant.ticket_participant_id,
)
.join(EmailAddress, EmailAddress.id == TicketParticipant.email_address_id)
.outerjoin(
EmailAddress, EmailAddress.id == TicketParticipant.email_address_id
)
.outerjoin(
SyncTicket, TicketParticipant.id == SyncTicket.ticket_participant_id
)
Expand Down
6 changes: 2 additions & 4 deletions funnel/models/venue.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

from __future__ import annotations

import itertools

from sqlalchemy.ext.orderinglist import ordering_list

from coaster.sqlalchemy import add_primary_relationship, with_roles
Expand All @@ -19,7 +17,7 @@
)
from .helpers import MarkdownCompositeBasic, reopen
from .project import Project
from .project_membership import project_child_role_map
from .project_membership import project_child_role_map, project_child_role_set

__all__ = ['Venue', 'VenueRoom']

Expand Down Expand Up @@ -113,7 +111,7 @@ class VenueRoom(UuidMixin, BaseScopedNameMixin, Model):
venue: Mapped[Venue] = with_roles(
relationship(Venue, back_populates='rooms'),
# Since Venue already remaps Project roles, we just want the remapped role names
grants_via={None: set(itertools.chain(*project_child_role_map.values()))},
grants_via={None: project_child_role_set},
)
parent: Mapped[Venue] = sa.orm.synonym('venue')
description, description_text, description_html = MarkdownCompositeBasic.create(
Expand Down
2 changes: 1 addition & 1 deletion funnel/templates/contacts.html.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
<div class="mui--clearfix">
<div class="user mui--pull-left">
<div class="user__box">
{{ useravatar(contact.ticket_participant.account) }}
{{ useravatar(contact.ticket_participant.participant) }}
<div class="user__box__header">
<p class="mui--text-subhead zero-bottom-margin">{{ contact.ticket_participant.fullname }}</p>
<p class="mui--text-body2 zero-bottom-margin">{{ contact.ticket_participant.email }}</p>
Expand Down
16 changes: 8 additions & 8 deletions funnel/views/api/oauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
)
from ...registry import resource_registry
from ...typing import ReturnView
from ...utils import abort_null, make_redirect_url
from ...utils import make_redirect_url
from ..login_session import reload_for_cookies, requires_client_login, requires_login
from .resource import get_userinfo

Expand Down Expand Up @@ -415,20 +415,20 @@ def oauth_token_success(token: AuthToken, **params) -> ReturnView:
def oauth_token() -> ReturnView:
"""Provide token endpoint for OAuth2 server."""
# Always required parameters
grant_type = cast(Optional[str], abort_null(request.form.get('grant_type')))
grant_type = cast(Optional[str], request.form.get('grant_type'))
auth_client = current_auth.auth_client # Provided by @requires_client_login
scope = abort_null(request.form.get('scope', '')).split(' ')
scope = request.form.get('scope', '').split(' ')
# if grant_type == 'authorization_code' (POST)
code = cast(Optional[str], abort_null(request.form.get('code')))
redirect_uri = cast(Optional[str], abort_null(request.form.get('redirect_uri')))
code = cast(Optional[str], request.form.get('code'))
redirect_uri = cast(Optional[str], request.form.get('redirect_uri'))
# if grant_type == 'password' (POST)
username = cast(Optional[str], abort_null(request.form.get('username')))
password = cast(Optional[str], abort_null(request.form.get('password')))
username = cast(Optional[str], request.form.get('username'))
password = cast(Optional[str], request.form.get('password'))
# if grant_type == 'client_credentials'
buid = cast(
Optional[str],
# XXX: Deprecated userid parameter
abort_null(request.form.get('buid') or request.form.get('userid')),
request.form.get('buid') or request.form.get('userid'),
)

# Validations 1: Required parameters
Expand Down
3 changes: 1 addition & 2 deletions funnel/views/api/shortlink.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@

from ... import app, shortlinkapp
from ...models import Shortlink, db
from ...utils import abort_null
from ..helpers import app_url_for, validate_is_app_url


# Add future hasjobapp route here
@app.route('/api/1/shortlink/create', methods=['POST'])
@requestform(('url', abort_null), ('shorter', getbool), ('name', abort_null))
@requestform('url', ('shorter', getbool), 'name')
def create_shortlink(
url: str | furl, shorter: bool = True, name: str | None = None
) -> tuple[dict[str, str], int]:
Expand Down
3 changes: 1 addition & 2 deletions funnel/views/api/sms_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
)
from ...transports.sms import validate_exotel_token
from ...typing import ReturnView
from ...utils import abort_null


@app.route('/api/1/sms/twilio_event', methods=['POST'])
Expand Down Expand Up @@ -116,7 +115,7 @@ def process_exotel_event(secret_token: str) -> ReturnView:
# If there are too many rejects, then most likely a hack attempt.
statsd.incr('phone_number.event', tags={'engine': 'exotel', 'stage': 'received'})

exotel_to = abort_null(request.form.get('To', ''))
exotel_to = request.form.get('To', '')
if not exotel_to:
return {'status': 'eror', 'error': 'invalid_phone'}, 422
# Exotel sends back 0-prefixed phone numbers, not plus-prefixed intl. numbers
Expand Down
4 changes: 2 additions & 2 deletions funnel/views/contact.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from .. import app
from ..models import ContactExchange, Project, TicketParticipant, db, sa
from ..typing import ReturnRenderWith, ReturnView
from ..utils import abort_null, format_twitter_handle
from ..utils import format_twitter_handle
from .login_session import requires_login


Expand Down Expand Up @@ -141,7 +141,7 @@ def scan(self) -> ReturnView:

@route('scan/connect', endpoint='scan_connect', methods=['POST'])
@requires_login
@requestargs(('puk', abort_null), ('key', abort_null))
@requestargs('puk', 'key')
def connect(self, puk: str, key: str) -> ReturnView:
"""Verify a badge scan and create a contact."""
ticket_participant = TicketParticipant.query.filter_by(puk=puk, key=key).first()
Expand Down
10 changes: 10 additions & 0 deletions funnel/views/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,16 @@ def shortlink(url: str, actor: Account | None = None, shorter: bool = True) -> s
# --- Request/response handlers --------------------------------------------------------


@app.before_request
def no_null_in_form():
"""Disallow NULL characters in any form submit (but don't scan file attachments)."""
if request.method == 'POST':
for values in request.form.listvalues():
for each in values:
if each is not None and '\x00' in each:
abort(400)


@app.after_request
def commit_db_session(response: ResponseType) -> ResponseType:
"""Commit database session at the end of a request if asked to."""
Expand Down
13 changes: 6 additions & 7 deletions funnel/views/label.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from ..forms import LabelForm, LabelOptionForm
from ..models import Account, Label, Project, db
from ..typing import ReturnRenderWith, ReturnView
from ..utils import abort_null
from .helpers import render_redirect
from .login_session import requires_login, requires_sudo
from .mixins import AccountCheckMixin, ProjectViewMixin
Expand All @@ -29,7 +28,7 @@ class ProjectLabelView(ProjectViewMixin, UrlForView, ModelView):
def labels(self) -> ReturnRenderWith:
form = forms.Form()
if form.validate_on_submit():
namelist = [abort_null(x) for x in request.values.getlist('name')]
namelist = request.values.getlist('name')
for idx, lname in enumerate(namelist, start=1):
lbl = Label.query.filter_by(project=self.obj, name=lname).first()
if lbl is not None:
Expand All @@ -51,8 +50,8 @@ def new_label(self) -> ReturnRenderWith:
# and those values are also available at `form.data`.
# But in case there are options, the option values are in the list
# in the order they appeared on the create form.
titlelist = [abort_null(x) for x in request.values.getlist('title')]
emojilist = [abort_null(x) for x in request.values.getlist('icon_emoji')]
titlelist = request.values.getlist('title')
emojilist = request.values.getlist('icon_emoji')
# first values of both lists belong to the parent label
titlelist.pop(0)
emojilist.pop(0)
Expand Down Expand Up @@ -143,9 +142,9 @@ def edit(self) -> ReturnRenderWith:
return render_redirect(self.obj.project.url_for('labels'))

if form.validate_on_submit():
namelist = [abort_null(x) for x in request.values.getlist('name')]
titlelist = [abort_null(x) for x in request.values.getlist('title')]
emojilist = [abort_null(x) for x in request.values.getlist('icon_emoji')]
namelist = request.values.getlist('name')
titlelist = request.values.getlist('title')
emojilist = request.values.getlist('icon_emoji')

namelist.pop(0)
titlelist.pop(0)
Expand Down
2 changes: 1 addition & 1 deletion funnel/views/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ def login() -> ReturnView:
if request.method == 'GET':
loginmethod = request.cookies.get('login')

formid = abort_null(request.form.get('form.id'))
formid = request.form.get('form.id')
if request.method == 'POST' and formid == 'passwordlogin':
try:
success = loginform.validate()
Expand Down
2 changes: 1 addition & 1 deletion funnel/views/login_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ def wrapper(*args: P.args, **kwargs: P.kwargs) -> T | ReturnResponse:
return render_redirect(url_for('change_password'))

elif request.method == 'POST':
formid = abort_null(request.form.get('form.id'))
formid = request.form.get('form.id')
if formid == FORMID_SUDO_OTP:
try:
otp_session = OtpSession.retrieve('sudo')
Expand Down
6 changes: 3 additions & 3 deletions funnel/views/siteadmin.py
Original file line number Diff line number Diff line change
Expand Up @@ -453,8 +453,8 @@ def init_rq_dashboard():
"""Register RQ Dashboard Blueprint if available for import."""
if rq_dashboard is not None:
rq_dashboard.blueprint.before_request(
lambda: None
if current_auth and current_auth.user.is_sysadmin
else abort(403)
lambda: (
None if current_auth and current_auth.user.is_sysadmin else abort(403)
)
)
app.register_blueprint(rq_dashboard.blueprint, url_prefix='/siteadmin/rq')
16 changes: 5 additions & 11 deletions funnel/views/ticket_participant.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,7 @@
from ..proxies import request_wants
from ..signals import user_data_changed, user_registered
from ..typing import ReturnRenderWith, ReturnView
from ..utils import (
abort_null,
format_twitter_handle,
make_qrcode,
mask_email,
split_name,
)
from ..utils import format_twitter_handle, make_qrcode, mask_email, split_name
from .helpers import render_redirect
from .login_session import requires_login
from .mixins import AccountCheckMixin, ProjectViewMixin, TicketEventViewMixin
Expand Down Expand Up @@ -96,7 +90,9 @@ def ticket_participant_checkin_data(ticket_participant, project, ticket_event):
'puuid_b58': puuid_b58,
'fullname': ticket_participant.fullname,
'company': ticket_participant.company,
'email': mask_email(ticket_participant.email),
'email': mask_email(ticket_participant.email)
if ticket_participant.email
else None,
'badge_printed': ticket_participant.badge_printed,
'checked_in': ticket_participant.checked_in,
'ticket_type_titles': ticket_participant.ticket_type_titles,
Expand Down Expand Up @@ -269,9 +265,7 @@ def checkin(self) -> ReturnView:
form = forms.Form()
if form.validate_on_submit():
checked_in = getbool(request.form.get('checkin'))
ticket_participant_ids = [
abort_null(x) for x in request.form.getlist('puuid_b58')
]
ticket_participant_ids = request.form.getlist('puuid_b58')
for ticket_participant_id in ticket_participant_ids:
attendee = TicketEventParticipant.get(self.obj, ticket_participant_id)
attendee.checked_in = checked_in
Expand Down
Loading

0 comments on commit 157a71c

Please sign in to comment.